[FFmpeg-devel] [PATCH 4/5] avcodec/parser: Schedule av_parser_change for removal

[James Almer]

On 3/1/2021 4:35 PM, James Almer wrote:
> On 3/1/2021 12:36 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> On 2/26/2021 10:18 AM, Andreas Rheinhardt wrote:
>>>> Originally deprecated in 748c2fca7e4d99357c234936aa71212a6282be36,
>>>> publically deprecated in 9a07c1332cfe092b57b5758f22b686ca58806c60
>>>> (merged into FFmpeg in 1885ffb03d0af28e6bac2bcc8725fa15b93f6ac9).
>>>>
>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>>>> ---
>>>>    libavcodec/avcodec.h | 6 +++++-
>>>>    libavcodec/parser.c  | 3 ++-
>>>>    libavcodec/version.h | 3 +++
>>>>    3 files changed, 10 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>>>> index 3178c163b9..a8741df04b 100644
>>>> --- a/libavcodec/avcodec.h
>>>> +++ b/libavcodec/avcodec.h
>>>> @@ -3576,14 +3576,18 @@ int av_parser_parse2(AVCodecParserContext *s,
>>>>                         int64_t pts, int64_t dts,
>>>>                         int64_t pos);
>>>>    +#if FF_API_PARSER_CHANGE
>>>>    /**
>>>>     * @return 0 if the output buffer is a subset of the input, 1 if it
>>>> is allocated and must be freed
>>>> - * @deprecated use AVBitStreamFilter
>>>> + * @deprecated Use dump_extradata, remove_extra or extract_extradata
>>>> + *             bitstream filters instead.
>>>>     */
>>>> +attribute_deprecated
>>>>    int av_parser_change(AVCodecParserContext *s,
>>>>                         AVCodecContext *avctx,
>>>>                         uint8_t **poutbuf, int *poutbuf_size,
>>>>                         const uint8_t *buf, int buf_size, int 
>>>> keyframe);
>>>> +#endif
>>>>    void av_parser_close(AVCodecParserContext *s);
>>>>      /**
>>>> diff --git a/libavcodec/parser.c b/libavcodec/parser.c
>>>> index a63f532c48..f4bc00da7d 100644
>>>> --- a/libavcodec/parser.c
>>>> +++ b/libavcodec/parser.c
>>>> @@ -186,6 +186,7 @@ int av_parser_parse2(AVCodecParserContext *s,
>>>> AVCodecContext *avctx,
>>>>        return index;
>>>>    }
>>>>    +#if FF_API_PARSER_CHANGE
>>>>    int av_parser_change(AVCodecParserContext *s, AVCodecContext *avctx,
>>>>                         uint8_t **poutbuf, int *poutbuf_size,
>>>>                         const uint8_t *buf, int buf_size, int keyframe)
>>>> @@ -220,7 +221,7 @@ int av_parser_change(AVCodecParserContext *s,
>>>> AVCodecContext *avctx,
>>>>          return 0;
>>>>    }
>>>> -
>>>> +#endif
>>>>    void av_parser_close(AVCodecParserContext *s)
>>>>    {
>>>>        if (s) {
>>>> diff --git a/libavcodec/version.h b/libavcodec/version.h
>>>> index 488def4c23..815df15628 100644
>>>> --- a/libavcodec/version.h
>>>> +++ b/libavcodec/version.h
>>>> @@ -150,6 +150,9 @@
>>>>    #ifndef FF_API_MPV_RC_STRATEGY
>>>>    #define FF_API_MPV_RC_STRATEGY     (LIBAVCODEC_VERSION_MAJOR < 59)
>>>>    #endif
>>>> +#ifndef FF_API_PARSER_CHANGE
>>>> +#define FF_API_PARSER_CHANGE       (LIBAVCODEC_VERSION_MAJOR < 59)
>>>
>>> A doxygen deprecation notice is not enough to consider the function
>>> deprecated. There was no APIChanges entry and no attribute added to the
>>> function prototype.
>>> So this needs the APIChanges entry, and therefore be scheduled as < 60.
>>
>> I disagree. Of course Doxygen deprecations count and being on the list
>> of deprecated things [1] for more than eight years is enough.
> 
> If there was no APIChanges entry that references a library version, then 
> this can not be considered deprecated and can not be removed until a 
> bump at least 2 years from now. Even more so when there was no 
> deprecation attribute on the function to warn its users.
> 
> So please, change it to < 60. It's an absolutely harmless standalone 
> function and can stay around for a proper deprecation period.

What could also be done as part of this deprecation is doing the same to 
AVCodecParser.split(). av_parser_change() is the only function that 
makes use of it, so with the function gone it has no reason to exist 
anymore, and it can all be removed at the same time.

You'd have to also wrap the relevant split functions on each parser that 
implements it.

> 
>>
>> - Andreas
>>
>> [1]: https://ffmpeg.org/doxygen/trunk/deprecated.html#_deprecated000007
>>
>>>
>>> LGTM otherwise.
>>>
>>>> +#endif
>>>>    #ifndef FF_API_THREAD_SAFE_CALLBACKS
>>>>    #define FF_API_THREAD_SAFE_CALLBACKS (LIBAVCODEC_VERSION_MAJOR < 60)
>>>>    #endif
>>>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>>
>