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

[James Almer]

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. Library 
users that want to support both lavc 59 and 60 will have to add 
preprocessor checks to their code anyway, so using the same name or 
another will not make any difference in that regard. Only a getter would 
prevent ifdeffery, but i agree it's also annoying, and we're about to 
get rid of a dozen of those in the upcoming bump. It also couldn't be 
added to 4.4 since that was already branched out.

How about adding the deprecation attribute to prompt people to read the 
doxy, where we state the field is not going away, just changing types? 
Otherwise i don't think people will notice.