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

[Andreas Rheinhardt]

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.
> 

The remove_extradata bsf also uses these. I actually intended to move
all the split functions to remove_extradata_bsf.c after the bump.

- Andreas

>>
>>>
>>> - 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".
>>>
>>
> 
> _______________________________________________
> 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".