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

Lukasz Marek lukasz.m.luki at gmail.com
Thu Jan 30 01:56:39 CET 2014

On 26.01.2014 21:27, Michael Niedermayer wrote:
> On Sun, Jan 26, 2014 at 09:06:48PM +0100, Lukasz Marek wrote:
>> 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?
> API but iam quite aware that query_codec() is insufficient
>> If API then you may see (and other entries around it)
>> http://ffmpeg.org/pipermail/ffmpeg-devel/2013-November/150496.html
> yes, query_codec() sucks
>>>>> +
>>>>> +/**
>>>>> + * 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:
>>>> 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:
>>>> 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.
> well it simply allows querrying a device (or other AVClass struct)
> about what it supports so for example it could be querried about
> supported frame rates and that could produce a different list
> depending on if width/height have been already set or which
> actual hardware was detected.
> it doesnt support picking the closest valid configuration.
> Though that could be implemented on top of it
> like
> if current configuration that has been set is invalid
>      for each parameter
>          get list of valid choices (this depends on other already set ones)
>          find choice that is closest to what the user did set
>      return to the user the smallest change needed (name of option and value)
> The user app could then just call av_opt_set() with this to make things
> consistent
> Also you can extend AVOptionRange(s) if needed and add new
> functionality if that what exists isnt doing the job you need but
> it does fall in the area of use cases that AVOptionRange(s) should
> cover

I attached another draft. This base on AVOption API and it seems to be 
good direction. Please have a look on it, just want to make sure I do 
what you had in mind. I made some dummy implementation for opengl device 
and simple test app.

In simple words I added new AVOption struct that should be used as 
nested structure to device private context structure. It is very 
convenient all devices have the same names of the params. The only 
disadvantage of this are options that may get duplicated with already 
existing options. Input devices probably provide most of them, but 
output devices usually take them from the stream. I'm not sure it can 
cause some problems. Some method to link them would be nice.

maybe AVClass can be extended by following callbacks:
- option set - device may get informed duplicated value is changed and 
copy value of duplicated param to second one)
- validation - device (or any other class in fact) may validate new 
value before it is set. av_opt_set* would return AVERROR(EINVAL) if 
rejected by device.

Other problem I notice there is no way to free nested structure.
Following code will leave a leak:

avformat_alloc_output_context2(&oc, NULL, device_name, NULL);
av_opt_set_int(oc->priv_data, "width", 100, AV_OPT_SEARCH_CHILDREN);

I left avdevice_list_devices() method with change suggested by Nicolas.
So far AVDeviceInfo have only 2 fields, but there may be more and I 
think they should be obtained together. In other way application may get 
for example device_name of 2 devices and in-between time user connects 
headset that adds 3rd device, and list of device_description would 
contains 3 items.

Best Regards,
Lukasz Marek

You can avoid reality, but you cannot avoid the consequences of avoiding 
reality. - Ayn Rand
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavd-probe-api-draft.patch
Type: text/x-patch
Size: 12879 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140130/aa3a3f51/attachment.bin>

More information about the ffmpeg-devel mailing list