[FFmpeg-devel] [PATCHv2 4/4] libavcodec: v4l2: add support for v4l2 mem2mem codecs

[Jorge Ramirez]

On 08/02/2017 02:43 PM, Hendrik Leppkes wrote:
> On Wed, Aug 2, 2017 at 11:39 AM, Jorge Ramirez
> <jorge.ramirez-ortiz at linaro.org> wrote:
>> On 08/02/2017 09:33 AM, Hendrik Leppkes wrote:
>>> On Tue, Aug 1, 2017 at 2:54 PM, Jorge Ramirez-Ortiz
>>> <jorge.ramirez-ortiz at linaro.org> wrote:
>>>> From: Alexis Ballier <aballier at gentoo.org>
>>>>
>>>> This patchset enhances Alexis Ballier's original patch and validates
>>>> it using Qualcomm's Venus hardware (driver recently landed upstream
>>>> [1]).
>>>>
>>>> This has been tested on Qualcomm's DragonBoard 410c and 820c
>>>>
>>>> Tested decoders:
>>>>          - h264
>>>>          - mpeg4
>>>>          - vp8
>>>>          - vp9
>>>>          - hevc
>>>>
>>>> Tested encoders:
>>>>          -h264
>>>>          -h263
>>>>          -mpeg4
>>>>
>>>> Tested transcoding (concurrent encoding/decoding)
>>>>
>>>> Some of the changes introduced:
>>>> - v4l2: code cleanup.
>>>> - v4l2: follow the decode api.
>>>> - v4l2: fix display size for NV12 output pool.
>>>> - v4l2: handle EOS.
>>>> - v4l2: vp8 and mpeg4 decoding.
>>>> - v4l2: hevc and vp9 support.
>>>> - v4l2: generate EOF on dequeue errors.
>>>> - v4l2: h264_mp4toannexb filtering.
>>>> - v4l2: import compat/v4l2 header files.
>>>>
>>>> [1] https://lwn.net/Articles/697956/
>>>>
>>>> Reviewed-by: Jorge Ramirez <jorge.ramirez-ortiz at linaro.org>
>>>> Reviewed-by: Alexis Ballier <aballier at gentoo.org>
>>>> Tested-by: Jorge Ramirez <jorge.ramirez-ortiz at linaro.org>
>>>> ---
>>>>    Changelog                     |    3 +-
>>>>    compat/v4l2/v4l2-common.h     |  107 ++
>>>>    compat/v4l2/v4l2-controls.h   |  987 +++++++++++++++++
>>>>    compat/v4l2/videodev2.h       | 2402
>>>> +++++++++++++++++++++++++++++++++++++++++
>>> As commented on IRC before, I'm not a fan of importing Linux kernel
>>> headers for the only reason because its convenient to always have
>>> recent headers available.
>> Hi Hendrik,
>>
>> but what is wrong with convenience? as a matter of fact, this will not add
>> any 'major' maintenance task.
>> Only when the ffmpeg v4l2 support is modified to add a more recent call (or
>> some new fourcc) these headers will have to be updated accordingly (ie do a
>> copy paste for files that are easily avaialable)
>>
>> IMO, this seems much easier, less error prone - and consistent - than
>> modifying configure every time looking for what needs to be checked before
>> building.
>>
> You could bring that argument for every single external library/API we
> support, and that is just not something we should be doing.
> We require headers to be available in the system for practically every
> other external API/library with only very few exceptions which have
> extraordinary circumstances beyond "I don't want to maintain configure
> checks".

but that is not a reason I have given - ie not wanting to maintain 
"configure", that is some interpretation of the conclusion.. (I think 
you got causation the other way around on this :)) The way I see it 
maintaining configure for V4L2 API just seems less efficient and more 
error prone - and more work for everyone with no real gains.

Let me try highlighting some benefits IMHO of importing the V4L2 API 
(also notice that this is the linux kernel API - we are not talking 
about some HW vendor specs that we want to import through some backdoor, 
these files are well maintained)

1. reduction in the frequency of the maintenance tasks.
When they need to be performed (ie new format or fourcc or whatever), 
the user will be updating for the future since it will import many other 
updates.
-> You can't say the same about maintaining configure.

2. the build environment is always sane no matter where you build.
This translates in more extensive testing since it enables building on 
more environments.
-> You can't say the same about checking against whatever API was 
installed in the build environment (could be 8 years old)

3. you can build encoders natively on a 14.04 server running a 3.3 
kernel and execute them on a target running a recent kernel
-> that can't be done the way you propose

And since the kernel guarantees that will not break userspace, there is 
zero risk to the users if we import the V4L2 API just as other projects 
have done.

> So no, we should not be doing this.

could you help me understand why what I am proposing is a no-go on ffmpeg?
what are the benefits of 'configure' checking for the kernel API in the 
build environment?

Anyhow, if this set in stone by the maintainers, just let me know and I 
will just update configure.
I hate wasting people's time :)


>
> - Hendrik