[FFmpeg-devel] [PATCH 2/4] lavd: add device capabilities API

Nicolas George george at nsup.org
Sun Feb 9 10:22:42 CET 2014


Le nonidi 19 pluviôse, an CCXXII, Lukasz Marek a écrit :
> TOTO: bump minors, update APIchages
> 
> Signed-off-by: Lukasz Marek <lukasz.m.luki at gmail.com>
> ---
>  libavdevice/avdevice.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++
>  libavdevice/avdevice.h |  79 ++++++++++++++++++++++++++++++++++++++
>  libavformat/avformat.h |  36 ++++++++++++++++++
>  3 files changed, 216 insertions(+)
> 
> diff --git a/libavdevice/avdevice.c b/libavdevice/avdevice.c
> index a418ef8..eb533e2 100644
> --- a/libavdevice/avdevice.c
> +++ b/libavdevice/avdevice.c
> @@ -17,9 +17,50 @@
>   */
>  
>  #include "libavutil/avassert.h"
> +#include "libavcodec/avcodec.h"
>  #include "avdevice.h"
>  #include "config.h"
>  
> +#define E AV_OPT_FLAG_ENCODING_PARAM
> +#define D AV_OPT_FLAG_DECODING_PARAM
> +#define A AV_OPT_FLAG_AUDIO_PARAM
> +#define V AV_OPT_FLAG_VIDEO_PARAM
> +#define OFFSET(x) offsetof(AVDeviceCapabilitiesQuery, x)
> +

> +const AVOption av_device_capabilities[] = {
> +    { "devcap_device_name", "device name", OFFSET(device_name), AV_OPT_TYPE_STRING,
> +        {.str = NULL}, 0, 0, E|D|A|V },
> +    { "devcap_codec", "codec", OFFSET(codec), AV_OPT_TYPE_INT,
> +        {.i64 = -1}, -1, INT_MAX, E|D|A|V },
> +    { "devcap_format", "format", OFFSET(format), AV_OPT_TYPE_INT,
> +        {.i64 = -1}, -1, INT_MAX, E|D|A|V },
> +
> +    { "devcap_sample_rate", "sample rate", OFFSET(sample_rate), AV_OPT_TYPE_INT,
> +        {.i64 = -1}, -1, INT_MAX, E|D|A },
> +    { "devcap_channels", "channels", OFFSET(channels), AV_OPT_TYPE_INT,
> +        {.i64 = -1}, -1, INT_MAX, E|D|A },
> +    { "devcap_channel_layout", "channel layout", OFFSET(channel_layout), AV_OPT_TYPE_INT64,
> +        {.i64 = -1}, -1, INT_MAX, E|D|A },
> +
> +    { "devcap_window_width", "window width", OFFSET(window_width), AV_OPT_TYPE_INT,
> +        {.i64 = -1}, -1, INT_MAX, E|D|V },
> +    { "devcap_window_height", "window height", OFFSET(window_height), AV_OPT_TYPE_INT,
> +        {.i64 = -1}, -1, INT_MAX, E|D|V },
> +    { "devcap_frame_width", "frame width", OFFSET(frame_width), AV_OPT_TYPE_INT,
> +        {.i64 = -1}, -1, INT_MAX, E|D|V },
> +    { "devcap_frame_height", "frame height", OFFSET(frame_height), AV_OPT_TYPE_INT,
> +        {.i64 = -1}, -1, INT_MAX, E|D|V },
> +    { "devcap_fps", "fps", OFFSET(fps), AV_OPT_TYPE_RATIONAL,
> +        {.dbl = -1}, -1, INT_MAX, E|D|V },

Unless I am mistaken, these options are no longer included as child object
options. Therefore, the namespace separation should not be necessary now.

> +    { NULL }
> +};
> +
> +#undef E
> +#undef D
> +#undef A
> +#undef V
> +#undef OFFSET
> +
>  unsigned avdevice_version(void)
>  {
>      av_assert0(LIBAVDEVICE_VERSION_MICRO >= 100);
> @@ -53,6 +94,66 @@ int avdevice_dev_to_app_control_message(struct AVFormatContext *s, enum AVDevToA
>      return s->control_message_cb(s, type, data, data_size);
>  }
>  
> +int avdevice_capabilities_create(AVDeviceCapabilitiesQuery **caps, AVFormatContext *s,
> +                                 AVDictionary **device_options)
> +{
> +    int ret;
> +    av_assert1(s && caps);
> +    av_assert1(s->iformat || s->oformat);
> +    if ((s->oformat && !s->oformat->create_device_capabilities) ||
> +        (s->iformat && !s->iformat->create_device_capabilities))
> +        return AVERROR(ENOSYS);

> +    *caps = av_mallocz(sizeof(AVDeviceCapabilitiesQuery));

sizeof(**caps)

> +    if (!(*caps))
> +        return AVERROR(ENOMEM);

> +    av_opt_set_defaults(*caps);
> +    (*caps)->device_context = s;
> +    if ((ret = av_opt_set_dict(s->priv_data, device_options)) < 0)
> +        goto fail;
> +    if (s->iformat) {
> +        if ((ret = s->iformat->create_device_capabilities(s, *caps)) < 0)
> +            goto fail;
> +    } else {
> +        if ((ret = s->oformat->create_device_capabilities(s, *caps)) < 0)
> +            goto fail;
> +    }

Please correct me if I am wrong, but I believe that (*caps)->class is set by
create_device_capabilities; therefore, the av_opt_set_defaults() need to
come after that.

> +    return 0;
> +  fail:
> +    av_freep(caps);
> +    return ret;
> +}
> +
> +int avdevice_capabilities_apply(AVDeviceCapabilitiesQuery *caps, AVFormatContext *s,
> +                               enum AVDeviceSettingsApplyStrategy strategy)
> +{
> +    av_assert1(s);
> +    av_assert1(s->iformat || s->oformat);
> +    if (s->iformat) {
> +        if (!s->iformat->apply_device_capabilities)
> +            return AVERROR(ENOSYS);
> +        return s->iformat->apply_device_capabilities(s, caps, strategy);
> +    } else {
> +        if (!s->oformat->apply_device_capabilities)
> +            return AVERROR(ENOSYS);
> +        return s->oformat->apply_device_capabilities(s, caps, strategy);
> +    }
> +}
> +
> +void avdevice_capabilities_free(AVDeviceCapabilitiesQuery **caps, AVFormatContext *s)
> +{
> +    av_assert1(s && caps);
> +    av_assert1(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);
> +    av_freep(caps);
> +}
> +
>  int avdevice_list_devices(AVFormatContext *s, AVDeviceInfoList **device_list)
>  {
>      av_assert1(s);
> diff --git a/libavdevice/avdevice.h b/libavdevice/avdevice.h
> index 2392cac..504cf1c 100644
> --- a/libavdevice/avdevice.h
> +++ b/libavdevice/avdevice.h
> @@ -43,6 +43,9 @@
>   * @}
>   */
>  
> +#include "libavutil/log.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/dict.h"
>  #include "libavformat/avformat.h"
>  
>  /**
> @@ -187,6 +190,82 @@ int avdevice_dev_to_app_control_message(struct AVFormatContext *s,
>                                          void *data, size_t data_size);
>  
>  /**
> + * Structure describes device settings.
> + *
> + * It is used by devices in conjuntion with av_device_capabilities AVOption table
> + * to to implement capabilities probing API based on AVOption API.
> + */
> +typedef struct AVDeviceCapabilitiesQuery {
> +    const AVClass *class;
> +    char *device_name;
> +    AVFormatContext *device_context;
> +    enum AVCodecID codec;

> +    int format;                          /**< AVSampleFormat or AVPixelFormat */

I am not sure which is best: sharing the field or not sharing the field. It
does not matter much.

> +    int sample_rate;
> +    int channels;
> +    int64_t channel_layout;
> +    int window_width;
> +    int window_height;
> +    int frame_width;
> +    int frame_height;
> +    AVRational fps;
> +} AVDeviceCapabilitiesQuery;
> +
> +extern const AVOption av_device_capabilities[];
> +
> +enum AVDeviceSettingsApplyStrategy {

> +    AVDeviceApplyStrategyApplyValid,            /**< apply settings when valid */
> +    AVDeviceApplyStrategyFixToTheNearest,       /**< adjust structure only to the nearest valid value */
> +    AVDeviceApplyStrategyFixToTheBest,          /**< adjust structure only to the best valid value */
> +    AVDeviceApplyStrategyApplyFixToTheNearest,  /**< adjust values to the nearest valid value and apply */
> +    AVDeviceApplyStrategyApplyFixToTheBest      /**< adjust values to the best valid value and apply */

The rest of the API uses caps for enum values, not camelcase; thus
AVDEVICE_APPLY_STRATEGY_APPLY_VALID and so on. APPLY_STRATEGY could be just
"STRATEGY".

Also, maybe explain a bit more the difference between "apply settings" and
"adjust structure": I assume that in the former, the settings are actually
sent to the hardware, but what consequences does it have on an unopened
device?

> +};
> +
> +/**
> + * Initialize capabilities probing API based on AVOption API.
> + *
> + * @param caps           device configuration
> + * @param s              device context
> + * @param device_options device options
> + * @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.

> + * After potential adjustments, set capabilities are applied and device configuration.

There is something strange in that sentence.

> + * 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.
> + *
> + * 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().
> + *
> + * @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 AVDeviceSettingsApplyStrategy strategy);
> +
> +/**
> + * Free resources created by avdevice_capabilities_create()
> + *
> + * @param caps device configuration
> + * @param s    device context
> + */
> +void avdevice_capabilities_free(AVDeviceCapabilitiesQuery **caps, AVFormatContext *s);
> +
> +/**
>   * Structure describes basic parameters of the device.
>   */
>  typedef struct AVDeviceInfo {
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index dc9aeee..322bc17 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -465,6 +465,22 @@ typedef struct AVOutputFormat {
>       * @see avdevice_list_devices() for more details.
>       */
>      int (*get_device_list)(struct AVFormatContext *s, void **device_list);
> +    /**
> +     * Initialize device capabilities submodule.
> +     * @see avdevice_capabilities_create() for more details.
> +     */

> +    int (*create_device_capabilities)(struct AVFormatContext *s, void *caps);

Same as my previous mail: I am in favour of pre-declaring the structure
instead of type pruning.

> +    /**
> +     * Apply currently set device capabilities to device context.
> +     * @see avdevice_capabilities_apply() for more details.
> +     */
> +    int (*apply_device_capabilities)(struct AVFormatContext *s, void *caps,
> +                                     int strategy);
> +    /**
> +     * Free device capabilities submodule.
> +     * @see avdevice_capabilities_free() for more details.
> +     */
> +    int (*free_device_capabilities)(struct AVFormatContext *s, void *caps);
>  } AVOutputFormat;
>  /**
>   * @}
> @@ -593,11 +609,31 @@ typedef struct AVInputFormat {
>       * Active streams are all streams that have AVStream.discard < AVDISCARD_ALL.
>       */
>      int (*read_seek2)(struct AVFormatContext *s, int stream_index, int64_t min_ts, int64_t ts, int64_t max_ts, int flags);

> +

Spurious. Probably belongs in the previous patch.

(What is the good way of moving a hunk from a patch to another?)

>      /**
>       * Returns device list with it properties.
>       * @see avdevice_list_devices() for more details.
>       */
>      int (*get_device_list)(struct AVFormatContext *s, void **device_list);

> +
> +    /**
> +     * Initialize device capabilities submodule.
> +     * @see avdevice_capabilities_create() for more details.
> +     */
> +    int (*create_device_capabilities)(struct AVFormatContext *s, void *caps);
> +
> +    /**
> +     * Apply currently set device capabilities to device context.
> +     * @see avdevice_capabilities_apply() for more details.
> +     */
> +    int (*apply_device_capabilities)(struct AVFormatContext *s, void *caps,
> +                                     int strategy);
> +
> +    /**
> +     * Free device capabilities submodule.
> +     * @see avdevice_capabilities_free() for more details.
> +     */
> +    int (*free_device_capabilities)(struct AVFormatContext *s, void *caps);

Would it make sense to share these between in and out formats?

>  } AVInputFormat;
>  /**
>   * @}

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140209/f602219a/attachment.asc>


More information about the ffmpeg-devel mailing list