[FFmpeg-devel] [PATCH 1/4] lavf: add probe device API

[Lukasz Marek]

On 26.01.2014 20:32, Michael Niedermayer wrote:
> On Sun, Jan 26, 2014 at 07:40:56PM +0100, Nicolas George wrote:
>> Le sextidi 6 pluviôse, an CCXXII, Lukasz Marek a écrit :
>>> >From c1e66d75f698fbd301743cd0664733a5d48f03e8 Mon Sep 17 00:00:00 2001
>>> From: Lukasz Marek <lukasz.m.luki at gmail.com>
>>> Date: Sat, 25 Jan 2014 22:46:03 +0100
>>> Subject: [PATCH] lavd: add probe device API
>>>
>>> ---
>>>   libavformat/avformat.h | 103 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 103 insertions(+)
>>>
>>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>>> index a495ee0..0ff4560 100644
>>> --- a/libavformat/avformat.h
>>> +++ b/libavformat/avformat.h
>>> @@ -323,6 +323,101 @@ typedef struct AVFrac {
>>>       int64_t val, num, den;
>>>   } AVFrac;
>>>
>>
>>> +//TODO: Move this stuff to libavdevice.
>>
>> I wonder. Being able to query the list of codecs supported by RTP, for
>> example, would be nice.
>
> also its a bit redudant with query_codec()

You refer to API or Nicolas' comment?
If API then you may see (and other entries around it)
http://ffmpeg.org/pipermail/ffmpeg-devel/2013-November/150496.html

>>
>>> +
>>> +/**
>>> + * TODO: desc
>>> + */
>>> +typedef struct AVDeviceFormat
>>> +{
>>> +    enum AVCodecID codec;  /**< codec */
>>> +    int *formats;          /**< list of formats supported with codec.
>>> +                                AVPixelFormat/AVSampleFormat terminated with -1 */
>>> +} AVDeviceFomat;
>>
>> First, I suggest, here and everywhere, to replace "terminated with X" lists
>> with a count indicator: "int *formats; unsigned nb_formats;". I believe this
>> is more practical for everyone. Also, some list are terminated by -1, some
>> by 0, some by NULL, that takes effort to remember.
>>
>
>> Second, I am not sure whether codec/format is the only pair that needs to be
>> linked. That is the most obvious one, but I can easily imagine a camera with
>> limited bandwidth supporting 50 FPS in MJPEG mode but only 30 FPS in
>> RAWVIDEO mode.
>
> +1
>
> also see  AVClass.query_ranges() and av_opt_query_ranges()
> its more generic and simpler

Not obvious goal I want to archive is to make use of it simple.
You call a function and you get configuration you may pass to filter 
chain sink for example. I agree this API is not enough because of issues 
Nicolas pointed. I would tend to add some flags like "pick the best 
configuration", "pick the nearest to wanted spec", "pick matching wanted 
spec". When user gets spammed with large amount of supported 
configurations, it will require possibly a lot of work to pick the 
correct. This "pick" algorithm will need to be repeated in every code 
that uses the device. Also, the device may possibly take device specific 
factors when deciding which is the best, that are not known to user.

You mention before about AVOptionRange, I had a short look on it, and I 
don't see how it could make it simpler? Maybe some hint, because I may 
miss something.

>
>
>>
>> Basically, we have the full Cartesian product:
>> CODECS × PIXEL_FORMAT × RESOLUTIONS × FRAME_RATES
>> CODECS × SAMPLE_FORMAT × CHANNEL_COUNTS × SAMPLE_RATES
>> and we need to be able to express a part of the set.
>>
>> The obvious simple idea is to consider that the subset is itself a Cartesian
>> product:
>>
>> SUPPORTED_FORMATS × SUPPORTED_RESOLUTIONS × SUPPORTED_FRAME_RATES \subset
>> PIXEL_FORMAT × RESOLUTIONS × FRAME_RATES
>>
>> Except for CODECS and FORMAT, because they always are strongly linked.
>>
>> I can suggest two solutions, one simple and one powerful.
>>
>> The simple one: each device can return a small list of AVDeviceCapabilities.
>> Each device expands its list the way it is most convenient. For example:
>>
>> [
>>    { codec = MJPEG, format = YUV420, list of supported resolutions and rates },
>>    { codec = MJPEG, format = YUV422, list of supported resolutions and rates },
>>    { codec = RAWVIDEO, format = RGB, list of supported resolutions and rates },
>> ]
>>
>> The powerful one: the AVDeviceCapabilities can leave fields unset and point
>> to a list of AVDeviceCapabilities to define them. Something like that:
>>
>> [
>>    { codec = MJPEG, pointer to sublist by format for MJPEG },
>>    { codec = RAWVIDEO, idem },
>> ]
>>
>> sublist = [
>>    { format = YUV420, pointer to sublist of frame sizes and rates },
>>    { format = YUV422, pointer to sublist of frame sizes and rates },
>> ]
>
> even more complex and still a subset of the existing
> AVOptionRange(s) API

I would appreciate some more information how you see it.

-- 
Best Regards,
Lukasz Marek

I may be drunk, Miss, but in the morning I will be sober and you will 
still be ugly. - Winston Churchill