[FFmpeg-devel] [PATCH 21/35] avdevice: capabilities API details no longer public
Andreas Rheinhardt
andreas.rheinhardt at outlook.com
Tue Jun 8 02:54:05 EEST 2021
Diederick Niehorster:
> NB: will break build, makes needed corresponding changes to avformat.
>
Then said changes should be part of this patch. Patches should be
logically self-contained and atomic; this is not the same as splitting
by file/library.
But that is a moot point: James already told you a better way to fix this.
> Signed-off-by: Diederick Niehorster <dcnieho at gmail.com>
> ---
> libavdevice/avdevice.c | 34 ++++++++++++++++++++--------------
> libavdevice/avdevice.h | 42 +++++++++---------------------------------
> libavdevice/internal.h | 33 +++++++++++++++++++++++++++++++++
> libavdevice/version.h | 2 +-
> 4 files changed, 63 insertions(+), 48 deletions(-)
>
> diff --git a/libavdevice/avdevice.c b/libavdevice/avdevice.c
> index d73d36e1dd..b521516ff5 100644
> --- a/libavdevice/avdevice.c
> +++ b/libavdevice/avdevice.c
> @@ -96,51 +96,57 @@ 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,
> +int avdevice_capabilities_create(void** opaque, AVFormatContext *s,
> AVDictionary **device_options)
> {
> int ret;
> + AVDeviceCapabilitiesQuery *caps = NULL;
> av_assert0(s);
> - av_assert0(caps);
> + av_assert0(opaque);
> av_assert0(s->iformat || s->oformat);
> + *opaque = NULL;
> if ((s->oformat && !s->oformat->create_device_capabilities) ||
> (s->iformat && !s->iformat->create_device_capabilities)) {
> - *caps = NULL;
> return AVERROR(ENOSYS);
> }
> - *caps = av_mallocz(sizeof(AVDeviceCapabilitiesQuery));
> - if (!(*caps))
> + *opaque = caps = av_mallocz(sizeof(AVDeviceCapabilitiesQuery));
> + if (!caps)
> return AVERROR(ENOMEM);
> - (*caps)->device_context = s;
> + 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)
> + if ((ret = s->iformat->create_device_capabilities(s, caps)) < 0)
> goto fail;
> } else {
> - if ((ret = s->oformat->create_device_capabilities(s, *caps)) < 0)
> + if ((ret = s->oformat->create_device_capabilities(s, caps)) < 0)
> goto fail;
> }
> - av_opt_set_defaults(*caps);
> + av_opt_set_defaults(caps);
> return 0;
> fail:
> av_freep(caps);
> + *opaque = NULL;
> return ret;
> }
>
> -void avdevice_capabilities_free(AVDeviceCapabilitiesQuery **caps, AVFormatContext *s)
> +void avdevice_capabilities_free(void **opaque, AVFormatContext *s)
> {
> - if (!s || !caps || !(*caps))
> + AVDeviceCapabilitiesQuery *caps;
> + if (!s || !opaque)
> + return;
> + caps = *(AVDeviceCapabilitiesQuery **) opaque;
> + if (!caps)
> return;
> av_assert0(s->iformat || s->oformat);
> if (s->iformat) {
> if (s->iformat->free_device_capabilities)
> - s->iformat->free_device_capabilities(s, *caps);
> + s->iformat->free_device_capabilities(s, caps);
> } else {
> if (s->oformat->free_device_capabilities)
> - s->oformat->free_device_capabilities(s, *caps);
> + s->oformat->free_device_capabilities(s, caps);
> }
> - av_freep(caps);
> + av_freep(opaque);
> }
>
> int avdevice_list_devices(AVFormatContext *s, AVDeviceInfoList **device_list)
> diff --git a/libavdevice/avdevice.h b/libavdevice/avdevice.h
> index 7c5e77df00..389ac0b5f2 100644
> --- a/libavdevice/avdevice.h
> +++ b/libavdevice/avdevice.h
> @@ -401,40 +401,15 @@ int avdevice_dev_to_app_control_message(struct AVFormatContext *s,
> * @endcode
> */
>
> -/**
> - * Structure describes device capabilities.
> - *
> - * It is used by devices in conjunction with av_device_capabilities AVOption table
> - * to implement capabilities probing API based on AVOption API. Should not be used directly.
> - */
> -typedef struct AVDeviceCapabilitiesQuery {
> - const AVClass *av_class;
> - AVFormatContext *device_context;
> - enum AVCodecID codec;
> - enum AVSampleFormat sample_format;
> - enum AVPixelFormat pixel_format;
> - int sample_rate;
> - int channels;
> - int64_t channel_layout;
> - int window_width;
> - int window_height;
> - int frame_width;
> - int frame_height;
> - AVRational fps;
> -} AVDeviceCapabilitiesQuery;
> -
> -/**
> - * AVOption table used by devices to implement device capabilities API. Should not be used by a user.
> - */
> -extern const AVOption av_device_capabilities[];
> -
> /**
> * Initialize capabilities probing API based on AVOption API.
> *
> * avdevice_capabilities_free() must be called when query capabilities API is
> * not used anymore.
> - *
> - * @param[out] caps Device capabilities data. Pointer to a NULL pointer must be passed.
> + *
> + * @param[out] opaque A pointer where the capabilities API state will be stored. Pointer
> + * to a NULL pointer must be passed and caller must not touch this in
> + * any way.
> * @param s Context of the device.
> * @param device_options An AVDictionary filled with device-private options.
> * On return this parameter will be destroyed and replaced with a dict
> @@ -445,16 +420,17 @@ extern const AVOption av_device_capabilities[];
> *
> * @return >= 0 on success, negative otherwise.
> */
> -int avdevice_capabilities_create(AVDeviceCapabilitiesQuery **caps, AVFormatContext *s,
> +int avdevice_capabilities_create(void **opaque, AVFormatContext *s,
> AVDictionary **device_options);
>
> /**
> * Free resources created by avdevice_capabilities_create()
> *
> - * @param caps Device capabilities data to be freed.
> - * @param s Context of the device.
> + * @param[out] opaque Pointer to be freed that was returned from
> + * avdevice_capabilities_create.
> + * @param s Context of the device.
> */
> -void avdevice_capabilities_free(AVDeviceCapabilitiesQuery **caps, AVFormatContext *s);
> +void avdevice_capabilities_free(void **opaque, AVFormatContext *s);
>
> /**
> * Structure describes basic parameters of the device.
> diff --git a/libavdevice/internal.h b/libavdevice/internal.h
> index 67c90e1f87..fe4be64ee7 100644
> --- a/libavdevice/internal.h
> +++ b/libavdevice/internal.h
> @@ -20,9 +20,42 @@
> #define AVDEVICE_INTERNAL_H
>
> #include "libavformat/avformat.h"
> +#include "libavcodec/codec_id.h"
> +#include "libavutil/log.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/pixfmt.h"
> +#include "libavutil/rational.h"
> +#include "libavutil/samplefmt.h"
We prefer the order libavutil-libavcodec-libavformat (the reverse of
linking order).
>
> av_warn_unused_result
> int ff_alloc_input_device_context(struct AVFormatContext **avctx, const AVInputFormat *iformat,
> const char *format);
>
> +/**
> + * Structure describes device capabilities.
> + *
> + * It is used by devices in conjunction with av_device_capabilities AVOption table
> + * to implement capabilities probing API based on AVOption API. Should not be used directly.
> + */
> +typedef struct AVDeviceCapabilitiesQuery {
> + const AVClass *av_class;
> + AVFormatContext *device_context;
> + enum AVCodecID codec;
> + enum AVSampleFormat sample_format;
> + enum AVPixelFormat pixel_format;
> + int sample_rate;
> + int channels;
> + int64_t channel_layout;
> + int window_width;
> + int window_height;
> + int frame_width;
> + int frame_height;
> + AVRational fps;
> +} AVDeviceCapabilitiesQuery;
> +
> +/**
> + * AVOption table used by devices to implement device capabilities API. Should not be used by a user.
That last part is superfluous, as (external) users by definition have no
business looking into internal.h.
> + */
> +extern const AVOption av_device_capabilities[];
Don't use this name: av_* is our namespace for public symbols. Our
linker script makes sure that all symbols that start with av_* are
exported; whether they are in a public header is irrelevant.
Not exporting the symbol in the first place also solves the problem of
users using it despite it not being part of the API.
(I btw don't know whether said linker script is used on Windows.)
> +
> #endif
> diff --git a/libavdevice/version.h b/libavdevice/version.h
> index 0381d6cd0d..53af6fa0d0 100644
> --- a/libavdevice/version.h
> +++ b/libavdevice/version.h
> @@ -28,7 +28,7 @@
> #include "libavutil/version.h"
>
> #define LIBAVDEVICE_VERSION_MAJOR 59
> -#define LIBAVDEVICE_VERSION_MINOR 2
> +#define LIBAVDEVICE_VERSION_MINOR 3
> #define LIBAVDEVICE_VERSION_MICRO 100
>
> #define LIBAVDEVICE_VERSION_INT AV_VERSION_INT(LIBAVDEVICE_VERSION_MAJOR, \
>
More information about the ffmpeg-devel
mailing list