[FFmpeg-devel] [PATCH] avcodec/v4l2: set sizeimage param for non-raw buffers [fixes #6716]

[Jorge Ramirez-Ortiz]

On 10/04/2017 06:20 PM, wm4 wrote:
>>>>>> Isn't this something that should be fixed in the driver?
>>>>> yes but it might take forever and I dont know how many other drivers might
>>>>> need it.
>>>>>      
>>>>>> Why 2MB?
>>>>> no analysis done but seems to be enough to hold an encoded frame. Should it be
>>>>> any bigger?
>>>> I could use the calculations below if a generic magic number is a problem:
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/platform/qcom/venus/venc.c#n52
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/platform/qcom/venus/vdec.c#n49
>>>>
>>>> please let me know
>>> Well, I don't think there's any reason why the frame size would be
>>> limited to 2MB. I also can't tell if this is for uncompressed or
>>> compressed frames. For uncompressed frames, you could easily compute a
>>> good guess (the exact size depends on alignment and padding). For
>>> compressed frames it's probably impossible.
>> yes this is for compressed frames
>>
>>> If the kernel driver somehow can't be fixed and if this is a show
>>> stopper, it's probably OK if this is done to unbreak it, but it should
>> I doubt the kernel driver will be fixed any time soon - I can try posting a
>> patch there.
>>
>> But even then if it gets merged people using older kernels will have to back
>> port to their kernels and it ends up being a pain for everyone. Since in this
>> case userspace can easily take care of it - is a minor change- I think it should
>> be merged in ffmpeg.
> So would it break for better drivers if a packet of over 2 MB is fed to
> them?

any good driver should encapsulate its own restrictions and not export them to 
the client as it is the case on s5p-mfc - so drivers properly written will 
ignore the sizeimage field.
This commit is just a workaround for a particular driver (since this driver was 
upstreamed more than 5 years ago it is hard to say if what they did followed the 
API at the time).

>
> If yes, you'd have to detect the driver and explicitly use the
> workaround for the buggy driver only.

no, it will not affect properly - recently? - written drivers which should use 
the frame dimensions to allocate the compressed buffers.

>
>> I pull the calculation from venc.c and vdec.c instead of the magic number.
> To be honest this heuristic isn't better than the fixed constant
> anyway. Both can break, except that the heuristic is more complex.

um maybe- different hardware might have different alignment restrictions - But 
since the calculation seemed to roughly match Mark Thompson's in VAAPI I suppose 
it is good enough.