[FFmpeg-devel] [PATCH 2/4] lavd: add device capabilities API
Nicolas George
george at nsup.org
Tue Feb 4 17:25:44 CET 2014
Le quintidi 15 pluviôse, an CCXXII, Lukasz Marek a écrit :
> Signed-off-by: Lukasz Marek <lukasz.m.luki at gmail.com>
> ---
> libavdevice/avdevice.c | 191 +++++++++++++++++++++++++++++++++++++++
> libavdevice/avdevice.h | 238 +++++++++++++++++++++++++++++++++++++++++++++++++
> libavdevice/version.h | 2 +-
> libavformat/avformat.h | 12 +++
> libavformat/version.h | 2 +-
> 5 files changed, 443 insertions(+), 2 deletions(-)
I do not have a full view of how you want the API to work yet, but I can
still produce a few remarks that will, hopefully, help clarify things.
>
> diff --git a/libavdevice/avdevice.c b/libavdevice/avdevice.c
> index 51617fb..fa524a2 100644
> --- a/libavdevice/avdevice.c
> +++ b/libavdevice/avdevice.c
> @@ -17,9 +17,48 @@
> */
>
> #include "libavutil/avassert.h"
> +#include "libavcodec/avcodec.h"
> #include "avdevice.h"
> #include "config.h"
>
> +#define AVDEVICE_AV_PARAM AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_VIDEO_PARAM
> +#define AVDEVICE_DECENC_PARAM AV_OPT_FLAG_DECODING_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> +#define AVDEVICE_ALL_PARAM AVDEVICE_AV_PARAM | AVDEVICE_DECENC_PARAM
Any reason to use such long names for purely local macros? Other files with
similar features use "D", "E", etc. A macro for the repeated offsetof would
be advisable too.
> +
> +const AVOption av_device_capabilities[] = {
> + { "__device_name", "device name", offsetof(AVDeviceCapabilities, device_name), AV_OPT_TYPE_STRING,
> + {.str = NULL}, 0, 0, AVDEVICE_ALL_PARAM },
Are you sure about the "__" prefix as a namespace isolation? Something more
explicit, like, maybe "device." or "device/", or "devcap.", seems nicer.
> + { "__device_context", "device context", offsetof(AVDeviceCapabilities, device_context), AV_OPT_TYPE_POINTER,
> + {.str = NULL}, 0, 0, AVDEVICE_ALL_PARAM },
> + { "__codec", "codec", offsetof(AVDeviceCapabilities, codec), AV_OPT_TYPE_INT,
> + {.i64 = -1}, -1, INT_MAX, AVDEVICE_ALL_PARAM },
> + { "__format", "format", offsetof(AVDeviceCapabilities, format), AV_OPT_TYPE_INT,
> + {.i64 = -1}, -1, INT_MAX, AVDEVICE_ALL_PARAM },
> +
> + { "__sample_rate", "sample rate", offsetof(AVDeviceCapabilities, sample_rate), AV_OPT_TYPE_INT,
> + {.i64 = -1}, -1, INT_MAX, AV_OPT_FLAG_AUDIO_PARAM | AVDEVICE_DECENC_PARAM },
> + { "__channels", "channels", offsetof(AVDeviceCapabilities, channels), AV_OPT_TYPE_INT,
> + {.i64 = -1}, -1, INT_MAX, AV_OPT_FLAG_AUDIO_PARAM | AVDEVICE_DECENC_PARAM },
> + { "__channel_layout", "channel layout", offsetof(AVDeviceCapabilities, channel_layout), AV_OPT_TYPE_INT,
> + {.i64 = -1}, -1, INT_MAX, AV_OPT_FLAG_AUDIO_PARAM | AVDEVICE_DECENC_PARAM },
Should be INT64, no?
> +
> + { "__window_width", "window width", offsetof(AVDeviceCapabilities, window_width), AV_OPT_TYPE_INT,
> + {.i64 = -1}, -1, INT_MAX, AV_OPT_FLAG_VIDEO_PARAM | AVDEVICE_DECENC_PARAM },
> + { "__window_height", "window height", offsetof(AVDeviceCapabilities, window_height), AV_OPT_TYPE_INT,
> + {.i64 = -1}, -1, INT_MAX, AV_OPT_FLAG_VIDEO_PARAM | AVDEVICE_DECENC_PARAM },
> + { "__frame_width", "frame width", offsetof(AVDeviceCapabilities, frame_width), AV_OPT_TYPE_INT,
> + {.i64 = -1}, -1, INT_MAX, AV_OPT_FLAG_VIDEO_PARAM | AVDEVICE_DECENC_PARAM },
> + { "__frame_height", "frame height", offsetof(AVDeviceCapabilities, frame_height), AV_OPT_TYPE_INT,
> + {.i64 = -1}, -1, INT_MAX, AV_OPT_FLAG_VIDEO_PARAM | AVDEVICE_DECENC_PARAM },
How does it handle situations where a small number frame sizes are possible?
> + { "__fps", "fps", offsetof(AVDeviceCapabilities, fps), AV_OPT_TYPE_RATIONAL,
> + {.dbl = -1}, -1, INT_MAX, AV_OPT_FLAG_VIDEO_PARAM | AVDEVICE_DECENC_PARAM },
> + { NULL }
> +};
> +
> +#undef AVDEVICE_AV_PARAM
> +#undef AVDEVICE_DECENC_PARAM
> +#undef AVDEVICE_ALL_PARAM
> +
> unsigned avdevice_version(void)
> {
> av_assert0(LIBAVDEVICE_VERSION_MICRO >= 100);
> @@ -52,3 +91,155 @@ int avdevice_dev_to_app_control_message(struct AVFormatContext *s, enum AVDevToA
> return AVERROR(ENOSYS);
> return s->control_message_cb(s, type, data, data_size);
> }
> +
> +static const char * get_opt_name_from_cap_enum(enum AVDeviceCapability capability)
> +{
> + switch (capability) {
> + case AV_DEV_CAP_DEVICE_NAME:
> + return "__device_name";
> + case AV_DEV_CAP_CODEC_ID:
> + return "__codec";
> + case AV_DEV_CAP_FORMAT:
> + return "__format";
> + case AV_DEV_CAP_SAMPLE_RATE:
> + return "__sample_rate";
> + case AV_DEV_CAP_CHANNELS:
> + return "__channels";
> + case AV_DEV_CAP_CHANNEL_LAYOUT:
> + return "__channel_layout";
> + case AV_DEV_CAP_WINDOW_WIDTH:
> + return "__window_width";
> + case AV_DEV_CAP_WINDOW_HEIGHT:
> + return "__window_height";
> + case AV_DEV_CAP_FRAME_WIDTH:
> + return "__frame_width";
> + case AV_DEV_CAP_FRAME_HEIGHT:
> + return "__frame_height";
> + case AV_DEV_CAP_FPS:
> + return "__fps";
> + default:
> + break;
> + }
> + return NULL;
> +}
> +int avdevice_init_device_capabilities(AVFormatContext *s, AVDictionary **device_options)
> +{
> + int ret;
> + if ((ret = av_opt_set_pointer(s->priv_data, "__device_context", s,
> + AV_OPT_SEARCH_CHILDREN)) < 0)
> + return (ret == AVERROR_OPTION_NOT_FOUND) ? AVERROR(ENOSYS) : ret;
> + if ((ret = av_opt_set_dict(s->priv_data, device_options)) < 0)
> + return ret;
> + return 0;
> +}
> +
> +int avdevice_finish_device_capabilities(AVFormatContext *s,
> + AVDeviceCapabilities **spec,
> + enum AVDeviceApplyStrategy strategy)
> +{
> + if (!s->oformat || !s->oformat->apply_configuration)
> + return AVERROR(ENOSYS);
> + return s->oformat->apply_configuration(s, (void **)spec, strategy);
Why the cast to void?
> +}
> +
> +void avdevice_free_device_capabilities(AVDeviceCapabilities **spec)
> +{
> + if (!spec || !(*spec))
> + return;
> + av_free((*spec)->device_name);
> + av_freep(spec);
> +}
I do not see the corresponding alloc function, is it normal?
> +
> +int avdevice_get_device_capability(AVFormatContext *s, enum AVDeviceCapability capability,
> + AVOptionRanges **allowed_values)
> +{
> + const char *opt_name;
> + if (!s || !allowed_values ||
> + !(opt_name = get_opt_name_from_cap_enum(capability)))
> + return AVERROR(EINVAL);
IMHO, since these errors are obviously invalid use of the API, an assert
failure is more adapted.
> + return av_opt_query_ranges(allowed_values, s->priv_data, opt_name, AV_OPT_SEARCH_CHILDREN);
Here and in the following functions, it seems you are just wrapping /
duplicating the options code. Why not just allow to use the options API
directly? Less code for you, less new API to learn for the others.
Maybe I am missing something, but the API could be something like that:
AVDeviceCapability *cap;
avdevice_capability_create(&cap, fmt_ctx, options);
av_opt_set_type(cap, "fps", 30);
avdevice_capability_compute(cap);
av_opt_query_ranges(cap, "frame_width", &width);
avdevice_capability_free(&cap);
> +}
> +
> +int avdevice_set_device_capability_int(AVFormatContext *s,
> + enum AVDeviceCapability capability, int64_t value)
> +{
> + const char *opt_name;
> + if (!s || !(opt_name = get_opt_name_from_cap_enum(capability)))
> + return AVERROR(EINVAL);
> + switch (capability) {
> + case AV_DEV_CAP_CODEC_ID:
> + case AV_DEV_CAP_FORMAT:
> + case AV_DEV_CAP_SAMPLE_RATE:
> + case AV_DEV_CAP_CHANNELS:
> + case AV_DEV_CAP_CHANNEL_LAYOUT:
> + case AV_DEV_CAP_WINDOW_WIDTH:
> + case AV_DEV_CAP_WINDOW_HEIGHT:
> + case AV_DEV_CAP_FRAME_WIDTH:
> + case AV_DEV_CAP_FRAME_HEIGHT:
> + return av_opt_set_int(s->priv_data, opt_name, value, AV_OPT_SEARCH_CHILDREN);
> + default:
> + break;
> + }
> + av_log(s, AV_LOG_ERROR, "Capability %s is not of integer type.\n", opt_name);
> + return AVERROR(EINVAL);
Unless I am mistaken, you are duplicating checks that are already done by
the options API.
(On the other hand, since you are relying on enums, calling set_int on a
non-integer option is an obviously invalid use of the API and deserves an
assert failure.)
> +}
> +
> +int avdevice_set_device_capability_string(AVFormatContext *s,
> + enum AVDeviceCapability capability,
> + const char *value)
> +{
> + const char *opt_name;
> + if (!s || !(opt_name = get_opt_name_from_cap_enum(capability)))
> + return AVERROR(EINVAL);
> + switch (capability) {
> + case AV_DEV_CAP_DEVICE_NAME:
> + return av_opt_set(s->priv_data, opt_name, value, AV_OPT_SEARCH_CHILDREN);
> + default:
> + break;
> + }
> + av_log(s, AV_LOG_ERROR, "Capability %s is not of string type.\n", opt_name);
> + return AVERROR(EINVAL);
> +}
> +
> +int avdevice_set_device_capability_q(AVFormatContext *s,
> + enum AVDeviceCapability capability,
> + AVRational value)
> +{
> + const char *opt_name;
> + if (!s || !(opt_name = get_opt_name_from_cap_enum(capability)))
> + return AVERROR(EINVAL);
> + switch (capability) {
> + case AV_DEV_CAP_FPS:
> + return av_opt_set_q(s->priv_data, opt_name, value, AV_OPT_SEARCH_CHILDREN);
> + default:
> + break;
> + }
> + av_log(s, AV_LOG_ERROR, "Capability %s is not of AVRational type.\n", opt_name);
> + return AVERROR(EINVAL);
> +}
> +
> +int avdevice_list_devices(AVFormatContext *s, AVDeviceInfoList **device_list)
This part and the related changes could go in a separate patch. It could
probably be applied much faster.
> +{
> + if (!s->oformat || !s->oformat->get_device_list)
> + return AVERROR(ENOSYS);
> + return s->oformat->get_device_list(s, (void **)device_list);
> +}
> +
> +void avdevice_free_list_devices(AVDeviceInfoList **device_list)
> +{
> + AVDeviceInfoList *list;
> + AVDeviceInfo *dev;
> + int i;
> +
> + if (!device_list || !(*device_list))
> + return;
> + list = *device_list;
> +
> + for (i = 0; i < list->nb_devices; i++) {
> + dev = &list->devices[i];
> + av_free(dev->device_name);
> + av_free(dev->device_description);
> + av_free(dev);
> + }
> + av_freep(device_list);
> +}
> diff --git a/libavdevice/avdevice.h b/libavdevice/avdevice.h
> index a6408ea..bfcca35 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"
>
> /**
> @@ -186,4 +189,239 @@ int avdevice_dev_to_app_control_message(struct AVFormatContext *s,
> enum AVDevToAppMessageType type,
> void *data, size_t data_size);
>
> +/**
> + * Structure describes device capabilites.
> + *
> + * It is used by devices in conjuntion with av_device_capabilities AVOption table
> + * to to implement capabilities probing API.
> + */
> +typedef struct AVDeviceCapabilities {
> + const AVClass *class;
> + char *device_name;
> + AVFormatContext *device_context;
> + enum AVCodecID codec;
> + int format; /**< AVSampleFormat or AVPixelFormat */
> + int sample_rate;
> + int channels;
> + int64_t channel_layout;
> + int window_width;
> + int window_height;
> + int frame_width;
> + int frame_height;
> + AVRational fps;
> +} AVDeviceCapabilities;
> +
> +extern const AVOption av_device_capabilities[];
> +
> +/**
> + * Enumerates device capabilities that can be probed.
> + */
> +enum AVDeviceCapability {
> + /**
> + * Device name.
> + *
> + * set: set the device to read capability of.
> + * get: value previously set, use avdevice_list_devices()
> + * to get full list of the devices.
> + * type: string.
> + */
> + AV_DEV_CAP_DEVICE_NAME,
> +
> + /**
> + * Supported codecs.
> + *
> + * set: limit following queries to configurations supporting the codec.
> + * get: list all supported codecs.
> + * type: int (enum AVCodecID).
> + */
> + AV_DEV_CAP_CODEC_ID,
> +
> + /**
> + * Supported sample/pixel formats.
> + *
> + * set: limit following queries to configurations supporting the format.
> + * get: list all supported formats.
> + * type: int (enum AVSampleFormat / enum AVPixelFormat).
> + */
> + AV_DEV_CAP_FORMAT,
> +
> + /**
> + * Supported sample/pixel formats.
> + *
> + * set: limit following queries to configurations supporting the format.
> + * get: list all supported formats.
> + * type: int (enum AVSampleFormat / enum AVPixelFormat).
> + */
> + AV_DEV_CAP_SAMPLE_RATE,
> +
> + /**
> + * Supported channels count.
> + *
> + * set: limit following queries to configurations supporting the cannels count.
> + * get: list all supported channels count.
> + * type: int.
> + */
> + AV_DEV_CAP_CHANNELS,
> +
> + /**
> + * Supported cannel layouts.
> + *
> + * set: limit following queries to configurations supporting the cannel layouts.
> + * get: list all supported cannel layouts.
> + * type: int.
> + */
> + AV_DEV_CAP_CHANNEL_LAYOUT,
> +
> + /**
> + * Supported window width/height.
> + *
> + * set: limit following queries to configurations supporting the window width/height.
> + * get: list range of supported window width/height.
> + * type: int.
> + */
> + AV_DEV_CAP_WINDOW_WIDTH,
> + AV_DEV_CAP_WINDOW_HEIGHT,
> +
> + /**
> + * Supported frame width/height.
> + *
> + * set: limit following queries to configurations supporting the frame width/height.
> + * get: list range of supported frame width/height.
> + * type: int.
> + */
> + AV_DEV_CAP_FRAME_WIDTH,
> + AV_DEV_CAP_FRAME_HEIGHT,
> +
> + /**
> + * Supported frames per second.
> + *
> + * set: limit following queries to configurations supporting the fps.
> + * get: list range of supported fps.
> + * type: int.
> + */
> + AV_DEV_CAP_FPS
> +};
> +
> +enum AVDeviceApplyStrategy {
> + AVDeviceApplyStrategyAbandon, /**< don't apply settings to device */
> + AVDeviceApplyStrategyAbandonNotValid, /**< don't apply settings to device when invalid */
> + AVDeviceApplyFixToTheNearestValidValue, /**< adjust values to the nearest valid value */
> + AVDeviceApplyFixToTheBestValidValue /**< adjust values to the best valid value */
> +};
> +
> +/**
> + * Function prepares the device to be probed.
> + *
> + * This function must be called before using av_device_get_device_capability()
> + * or av_device_set_device_capability_*().
> + * avdevice_finish_device_capabilities() must be called afterwards.
> + *
> + * @param s device context.
> + * @param device_options device-specific options.
> + * @return >= 0 on success, negative otherwise.
> + */
> +int avdevice_init_device_capabilities(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
> + * working then adjustment takes place according to provided strategy.
> + * After potential adjustments, set capabilities are applied and device configuration.
> + * 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.
> + *
> + * @note: This may be useful to validate if input stream may be passed directly
> + * to output device, but usually capabilites should be tested one by one
> + * and correct values should be provided.
> + *
> + * @param s device context
> + * @param[out] spec returns device configuration. May be NULL. If not NULL
> + * then must be freed with avdevice_free_device_capabilities().
> + * @param strategy fixing values strategy. Ignored when spec is not provided.
> + * @return >= 0 on success, negative otherwise.
> + * AVERROR(EINVAL) when strategy is not implemented.
> + */
> +int avdevice_finish_device_capabilities(AVFormatContext *s,
> + AVDeviceCapabilities **spec,
> + enum AVDeviceApplyStrategy strategy);
> +
> +/**
> + * Free AVDeviceCapabilities struct and its allocated data.
> + *
> + * @param spec structure to be freed
> + */
> +void avdevice_free_device_capabilities(AVDeviceCapabilities **spec);
> +
> +/**
> + * Return range(s) of valid values for device capability.
> + *
> + * This function allow to get ranges of values for device capability.
> + * Returned ranges takes into account real device capablities and
> + * values previously set by avdevice_set_device_capability_* functions.
> + * For example list of supported formats may dependes on the codec set previously,
> + * fps may depends on resolution or codec set previously.
> + *
> + * @param s device context
> + * @param capability capability to be tested.
> + * @param[out] ranges list of allowed ranges for selected capability.
> + * @return >= 0 on success, negative otherwise.
> + */
> +int avdevice_get_device_capability(AVFormatContext *s,
> + enum AVDeviceCapability capability,
> + AVOptionRanges **ranges);
> +
> +/**
> + * Set device capability.
> + *
> + * @param s device context
> + * @param value new value for capability
> + * @return >= 0 on success, negative otherwise.
> + */
> +int avdevice_set_device_capability_int(AVFormatContext *s,
> + enum AVDeviceCapability capability,
> + int64_t value);
> +int avdevice_set_device_capability_string(AVFormatContext *s,
> + enum AVDeviceCapability capability,
> + const char *value);
> +int avdevice_set_device_capability_q(AVFormatContext *s,
> + enum AVDeviceCapability capability,
> + AVRational value);
> +
> +/**
> + * Structure describes basic parameters of the device.
> + */
> +typedef struct AVDeviceInfo {
> + char *device_name; /**< device name, format depends on device */
> + char *device_description; /**< human friendly name */
> +} AVDeviceInfo;
> +
> +/**
> + * List of available devices.
> + */
> +typedef struct AVDeviceInfoList {
> + AVDeviceInfo *devices; /**< list of autodetected devices */
> + int nb_devices; /**< number of autodetected devices */
> + int default_device; /**< index of default device */
... "or -1 if no default"?
> +} AVDeviceInfoList;
> +
> +/**
> + * List available devices.
Please remember to explain that some devices can accept both predefined
names and synthetic ones, and this function will only list the former.
> + *
> + * @param ofmt device format.
> + * @param[out] devices list of autodetected devices.
> + * @return count of autodetected devices, negative on error.
> + */
> +int avdevice_list_devices(struct AVFormatContext *s, AVDeviceInfoList **device_list);
> +
> +/**
> + * Convinient function to free result of avdevice_list_devices().
> + *
> + * @param devices device list to be freed.
> + */
> +void avdevice_free_list_devices(AVDeviceInfoList **device_list);
> +
> #endif /* AVDEVICE_AVDEVICE_H */
> diff --git a/libavdevice/version.h b/libavdevice/version.h
> index a621775..0dedc73 100644
> --- a/libavdevice/version.h
> +++ b/libavdevice/version.h
> @@ -28,7 +28,7 @@
> #include "libavutil/version.h"
>
> #define LIBAVDEVICE_VERSION_MAJOR 55
> -#define LIBAVDEVICE_VERSION_MINOR 7
> +#define LIBAVDEVICE_VERSION_MINOR 8
> #define LIBAVDEVICE_VERSION_MICRO 100
>
> #define LIBAVDEVICE_VERSION_INT AV_VERSION_INT(LIBAVDEVICE_VERSION_MAJOR, \
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 50b7108..58bed4e 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -458,6 +458,18 @@ typedef struct AVOutputFormat {
> */
> int (*control_message)(struct AVFormatContext *s, int type,
> void *data, size_t data_size);
> + /**
> + * Returns device list with it properties.
> + * @see avdevice_list_devices() for more details.
> + */
> + int (*get_device_list)(struct AVFormatContext *s, void **device_list);
> + /**
> + * Allows to apply device configuration via avdevice_set_device_capability_*
> + * API with posibility to adjust not matching configuration.
> + * @see avdevice_finish_device_capabilities() for more details.
> + */
> + int (*apply_configuration)(struct AVFormatContext *s, void **configuration,
> + int strategy);
> } AVOutputFormat;
> /**
> * @}
> diff --git a/libavformat/version.h b/libavformat/version.h
> index 38945a5..0fcbe60 100644
> --- a/libavformat/version.h
> +++ b/libavformat/version.h
> @@ -30,7 +30,7 @@
> #include "libavutil/version.h"
>
> #define LIBAVFORMAT_VERSION_MAJOR 55
> -#define LIBAVFORMAT_VERSION_MINOR 29
> +#define LIBAVFORMAT_VERSION_MINOR 30
You probably should just note "TODO minor bump" somewhere, and actually add
it when the patch is almost ready for inclusion. In particular, this hunk
could silently disappear during a rebase if another patch did exactly the
same.
> #define LIBAVFORMAT_VERSION_MICRO 100
>
> #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
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/20140204/d3808448/attachment.asc>
More information about the ffmpeg-devel
mailing list