[FFmpeg-devel] [PATCH 21/35] avdevice: capabilities API details no longer public

Andreas Rheinhardt andreas.rheinhardt at outlook.com
Tue Jun 8 03:01:03 EEST 2021


James Almer:
> On 6/7/2021 8:04 PM, Diederick Niehorster wrote:
>> NB: will break build, makes needed corresponding changes to avformat.
>>
>> 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;
> 
> Instead of removing the struct altogether, just keep its typedef here.
> That way the functions below and any libavformat struct can still use
> AVDeviceCapabilitiesQuery as an incomplete type.
> 
> So in short:
> 
> typedef struct AVDeviceCapabilitiesQuery AVDeviceCapabilitiesQuery;
> 
> This is nonetheless technically an API break, but since nothing used
> this stuff, i guess we could make an exception. Not sure what other
> developers think.
> 

See the "Should not be used directly." above. It was not allowed to be
used by a user, ergo it is not an API break; same goes for
av_device_capabilites (which I somehow forgot to remove during the
bump). Given that they are not allowed to be used, it never made sense
to have them public.

- Andreas


More information about the ffmpeg-devel mailing list