Re: [PATCH v2] media: venus: Fix reported frame intervals

[Stanimir Varbanov <stanimir.varbanov@linaro.org>]

Hi Loic,

Thanks for the patch!

On 8/4/20 3:21 PM, Loic Poulain wrote:
> On dragonboard-410c (apq8016) with HFI_VERSION_1XX, the reported
> framerate is in unit of 1/65535 fps (for fine grained control).
> So the current reported supported frame intervals is wrong (max
> is 1/65535 fps), leading to encoding issues or format negotiation
> failures with gstreamer.
> 
> Fix that by setting the framerate denominator to coherent value
> based on the the framerate factor.
> 
> The factor is not always the same, e.g. with db820c (apq8096) HFI
> reports framerate in fps unit. So only apply that for HFI_VERSION_1XX.
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
>  v2: Use IS_V1 helper for retrieveing venus core version
>      Adjust denominator instead of numerator
> 
>  drivers/media/platform/qcom/venus/venc.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)

Acked-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>

> 
> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
> index 9981a2a..5b3df09 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -576,6 +576,7 @@ static int venc_enum_frameintervals(struct file *file, void *fh,
>  {
>  	struct venus_inst *inst = to_inst(file);
>  	const struct venus_format *fmt;
> +	unsigned int framerate_factor = 1;
>  
>  	fival->type = V4L2_FRMIVAL_TYPE_STEPWISE;
>  
> @@ -600,12 +601,17 @@ static int venc_enum_frameintervals(struct file *file, void *fh,
>  	    fival->height < frame_height_min(inst))
>  		return -EINVAL;
>  
> +	if (IS_V1(inst->core)) {
> +		/* framerate is reported in 1/65535 fps unit */
> +		framerate_factor = (1 << 16);
> +	}
> +
>  	fival->stepwise.min.numerator = 1;
> -	fival->stepwise.min.denominator = frate_max(inst);
> +	fival->stepwise.min.denominator = frate_max(inst) / framerate_factor;
>  	fival->stepwise.max.numerator = 1;
> -	fival->stepwise.max.denominator = frate_min(inst);
> +	fival->stepwise.max.denominator = frate_min(inst) / framerate_factor;
>  	fival->stepwise.step.numerator = 1;
> -	fival->stepwise.step.denominator = frate_max(inst);
> +	fival->stepwise.step.denominator = frate_max(inst) / framerate_factor;
>  
>  	return 0;
>  }
> 

-- 
regards,
Stan