[FFmpeg-devel] [PATCH 01/48] avcodec/packet: deprecate av_init_packet()

Marton Balint cus at passwd.hu
Sun Mar 21 20:15:28 EET 2021



On Sun, 21 Mar 2021, James Almer wrote:

> On 3/21/2021 11:16 AM, Anton Khirnov wrote:
>> Quoting James Almer (2021-03-21 14:54:22)
>>> On 3/21/2021 9:28 AM, Anton Khirnov wrote:
>>>> Quoting James Almer (2021-03-05 17:32:52)
>>>>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>>>>> index 7da2f3d98e..783cc1b591 100644
>>>>> --- a/libavformat/avformat.h
>>>>> +++ b/libavformat/avformat.h
>>>>> @@ -954,7 +954,11 @@ typedef struct AVStream {
>>>>>         * decoding: set by libavformat, must not be modified by the 
> caller.
>>>>>         * encoding: unused
>>>>>         */
>>>>> +#if FF_API_INIT_PACKET
>>>>>        AVPacket attached_pic;
>>>>> +#else
>>>>> +    AVPacket *attached_pic;
>>>>> +#endif
>>>>
>>>> Sorry I'm late to the party, but as we are changing the type of an
>>>> existing field, we need to explicitly spell out a way for the callers to
>>>> make their code forward compatible. E.g. similarly to what I made for
>>>> thread_safe_callbacks deprecation.
>>>
>>> What do you suggest? The field is not printing deprecation warnings, so
>>> would a doxy comment be enough for people to notice it?
>>> Have we done a similar change before? I know at least for symbols like
>>> public functions we never change their signature in an incompatible way,
>>> and instead replace them altogether.
>>>
>>> Maybe we could add the deprecated attribute to attached_pic, introduce a
>>> temporary getter function that returns a pointer to the packet, mention
>>> in the doxy that the field is not going away, is just changing types,
>>> and they have the option of using the getter for a fire-and-forget
>>> migration process, then maybe deprecate and eventually remove the getter
>>> after the switch is done.
>> 
>> This seems like a lot of hoops to jump through. Wouldn't it then be
>> better to just add a new field with a new name?
>
> A name change just because it's now a pointer seems overkill.

I don't think it is. A name change guarantees that existing code won't 
compile to something that will surely crash. IMHO the clean solution is to 
keep the old field with deprecation, and add a new field. Same what I did 
with AVFormat->filename / AVFormat->url.

Regards,
Marton


More information about the ffmpeg-devel mailing list