[FFmpeg-devel] [PATCH 5/8] lavd: add device capabilities API

[Lukasz Marek]

On 3 April 2014 16:56, Michael Niedermayer <michaelni at gmx.at> wrote:
>
>  > +void avdevice_capabilities_free(AVDeviceCapabilitiesQuery **caps,
> AVFormatContext *s)
> > +{
> > +    if (!s || !caps || !(*caps))
> > +        return;
> > +    av_assert0(s->iformat || s->oformat);
> > +    if (s->iformat) {
> > +        if (s->iformat->free_device_capabilities)
> > +            s->iformat->free_device_capabilities(s, *caps);
> > +    } else {
> > +        if (s->oformat->free_device_capabilities)
> > +            s->oformat->free_device_capabilities(s, *caps);
> > +    }
>
> > +    av_free((*caps)->device_name);
>
> should use av_freep() for saftey
>

Does it make a difference when caps is freed in next line?


>  > +    av_freep(caps);
> > +}
>

[...]


> > +> +/**
> > + * Initialize capabilities probing API based on AVOption API.
> > + *
> > + * avdevice_capabilities_free() must be called when query capabilities
> API is
> > + * not used anymore.
> > + *
>
> > + * @note: It is not allowed to free device context before calling
> avdevice_capabilities_free().
>
> why ?
>

This API have to open device to have it's private context and options.
API may cache results when obtaining them is slow (it can be stored in
private context)
Also, which is more important, this API needs device options to operate.
For example, pulse device need information which server should be probed.
When you free in the middle you lost this information. Technically device
context can be free before avdevice_capabilites_free, but no other API can
be used.


>
>
> > + *
> > + * @param caps           device configuration
>
> caps is a pointer to a pointer to AVDeviceCapabilitiesQuery
> what should the user set that to ?
> NULL ?
> some pointer that points to a NULL pointer ?
>
>
> > + * @param s              device context
>
> > + * @param device_options device options
>
> same question ?
> is this use for inputing or outputing values ?
>
> these things should be documented
> also what are device options?
> its important to explain this as its a free form key-value list where
> the user otherwise would probably have no clue what it is
>

OK, I will add more information, and probably a short example would be the
good here too.



>
>
> > + * @return >= 0 on success, negative otherwise.
> > + */
> > +int avdevice_capabilities_create(AVDeviceCapabilitiesQuery **caps,
> AVFormatContext *s,
> > +                                 AVDictionary **device_options);
> > +
> > +/**
> > + * Apply set parameters to device context and release data allocated
> > + * by avdevice_init_device_capabilities().
> > + *
> > + * All set capabilities are validated and tested. When configuration is
> not
> > + * valid then adjustment takes place according to provided strategy.
> > + * When it is required by provided strategy, set capabilities are
> applied into
> > + * device context. Mapping between capablities and device settings are
> device-specific.
> > + * In particular output device may not apply all parameters to the
> context,
> > + * but use stream properties when avformat_write_header() is called.
> > + *
> > + * @param caps     device configuration
> > + * @param s        device context
> > + * @param strategy apply strategy
> > + * @return 0 when configuration is not applied, 1 when configuration is
> applied,
> > + *         negative on error.
> > + */
> > +int avdevice_capabilities_apply(AVDeviceCapabilitiesQuery *caps,
> AVFormatContext *s,
> > +                                enum AVDeviceCapabilitiesApplyStrategy
> strategy);
>
> also can you explain in which cases this api improves over the
> existing situation where parameters are set in AVFormatContext
> and the devices private context via avoptions?
>
>
> I mean now one could set sample_rate, fps, ... via AVOptions into
> some input devices context (and with an AVDeviceCapabilitiesApplyStrategy
> field the device could on init/open choose the closest supported
>
> and with this API one create a seperate context fills the wanted
> parameters in there and applies it to the same effect
>
> I must be missing something as i dont see what advantage this
> additional step has


I'm not sure you have problem with applying something to device context, or
with the function at all
You don't miss anything, but:
- this allows to adjust parameters by device (it doesn't mean they have to
be applied to device tho)
- different devices sometimes use different names for the same option. It
is helpful for user to have generic code that should work for all that
implements this API.
Note that this step is not mandatory.