diff mbox series

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

Message ID 20210226131806.1990485-4-andreas.rheinhardt@gmail.com
State Accepted
Commit dc20b27802a4db3e46952b53c3aa8f331beffd37
Headers show
Series [FFmpeg-devel,1/5] fftools/ffmpeg_filter: Don't use deprecated function
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt Feb. 26, 2021, 1:18 p.m. UTC
Originally deprecated in 748c2fca7e4d99357c234936aa71212a6282be36,
publically deprecated in 9a07c1332cfe092b57b5758f22b686ca58806c60
(merged into FFmpeg in 1885ffb03d0af28e6bac2bcc8725fa15b93f6ac9).

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavcodec/avcodec.h | 6 +++++-
 libavcodec/parser.c  | 3 ++-
 libavcodec/version.h | 3 +++
 3 files changed, 10 insertions(+), 2 deletions(-)

Comments

James Almer Feb. 26, 2021, 1:23 p.m. UTC | #1
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@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.

LGTM otherwise.

> +#endif
>   #ifndef FF_API_THREAD_SAFE_CALLBACKS
>   #define FF_API_THREAD_SAFE_CALLBACKS (LIBAVCODEC_VERSION_MAJOR < 60)
>   #endif
>
Andreas Rheinhardt March 1, 2021, 3:36 p.m. UTC | #2
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@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.

- 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
>>
James Almer March 1, 2021, 7:35 p.m. UTC | #3
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@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.

> 
> - 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@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Paul B Mahol March 1, 2021, 7:42 p.m. UTC | #4
Nope, remove it ASAP!

On Mon, Mar 1, 2021 at 8:36 PM James Almer <jamrial@gmail.com> 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@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.
>
> >
> > - 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@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
James Almer March 1, 2021, 7:47 p.m. UTC | #5
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@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@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>
>
Andreas Rheinhardt March 2, 2021, 5:12 a.m. UTC | #6
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@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@ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>>
>>
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Andreas Rheinhardt March 2, 2021, 5:14 a.m. UTC | #7
James Almer:
> 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@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.
> 

I disagree; I just don't see why a doxygen deprecation should count for
nothing.

> So please, change it to < 60. It's an absolutely harmless standalone
> function and can stay around for a proper deprecation period.
> 
>>
>> - 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@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Hendrik Leppkes March 2, 2021, 7:26 a.m. UTC | #8
On Tue, Mar 2, 2021 at 6:14 AM Andreas Rheinhardt
<andreas.rheinhardt@gmail.com> wrote:
>
> James Almer:
> > 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@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.
> >
>
> I disagree; I just don't see why a doxygen deprecation should count for
> nothing.
>

Because its hard to get notified about changes there, and thus people
don't notice. I might read doxygen when I write new code, but for
existing code I wouldn't go around reading docs for functions that I
am already using checking for changes.
The entire point of the deprecation period is to inform users of an
upcoming change, but putting it only in doxygen is just not effective
at that task at all. Users can be assumed to be reading APIchanges, a
central place for all relevant changes (even if most probably don't),
or even better get compiler deprecation warnings. Either of those
would be good, both even better.

- Hendrik
Anton Khirnov March 2, 2021, 8:39 a.m. UTC | #9
Quoting James Almer (2021-03-01 20:35:55)
> 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@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

I do not agree that an APIChanges entry is necessary for a deprecation.
APIChanges is for listing API changes. A deprecation is not by itself an
API change, merely a statement of intent wrt future API changes.

From the perspective of an API user it does not make much sense to ask
whether a feature X is deprecated in version Y, only whether a
replacement API is present. In this specific case, a replacement API has
been present since 2006 - for more than half the project's existence.
Surely that should be enough.
Andreas Rheinhardt March 2, 2021, 12:25 p.m. UTC | #10
Hendrik Leppkes:
> On Tue, Mar 2, 2021 at 6:14 AM Andreas Rheinhardt
> <andreas.rheinhardt@gmail.com> wrote:
>>
>> James Almer:
>>> 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@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.
>>>
>>
>> I disagree; I just don't see why a doxygen deprecation should count for
>> nothing.
>>
> 
> Because its hard to get notified about changes there, and thus people
> don't notice. I might read doxygen when I write new code, but for
> existing code I wouldn't go around reading docs for functions that I
> am already using checking for changes.
> The entire point of the deprecation period is to inform users of an
> upcoming change, but putting it only in doxygen is just not effective
> at that task at all. Users can be assumed to be reading APIchanges, a
> central place for all relevant changes (even if most probably don't),
> or even better get compiler deprecation warnings. Either of those
> would be good, both even better.

I don't see a reason why
https://ffmpeg.org/doxygen/trunk/deprecated.html should not count; if
both APIchanges as well as doxygen were to be properly curated, the
former would be even better to know about the current state of deprecations.
There is currently no requirement anywhere (that I could find) that says
that deprecations should be put into APIchanges or that API users should
search in APIchanges for deprecations. The only place mentioning
doc/APIchanges is libavutil/avutil.h:
"However, new public symbols may be added and new members may be
appended to public structs whose size is not part of public ABI (most
public structs in FFmpeg). New macros and enum values may be added.
Behavior in undocumented situations may change slightly (and be
documented). All those are accompanied by an entry in doc/APIchanges and
incrementing either the minor or micro version number."
As you can see, there is no mention of deprecations. This is
intentional: It was written by Anton who thinks that deprecations are no
API changes at all (a view I don't share).
On the other hand, API users are explicitly encouraged to read the
doxygen documentation. From doc/developer.texi:
"This document is mostly useful for internal FFmpeg developers.
External developers who need to use the API in their application should
refer to the API doxygen documentation in the public headers, and
check the examples in @file{doc/examples} and in the source code to
see how the public API is employed."

In particular, in case of av_parser_change there was no documentation
for this function in avcodec.h before
9a07c1332cfe092b57b5758f22b686ca58806c60. Anyone who wanted to use this
function therefore had to either use doxygen (which picks up
documentation outside of headers, too) or look at the source directly.
In both of these instances, such a user would see the deprecation since
748c2fca7e4d99357c234936aa71212a6282be36. I therefore consider the
actual deprecation to have happened in 2006, before APIchanges was
introduced at all. In this time the doxygen deprecation list was
undoubtedly the list to look at for deprecations.

- Andreas
James Almer March 2, 2021, 12:50 p.m. UTC | #11
On 3/2/2021 5:39 AM, Anton Khirnov wrote:
> Quoting James Almer (2021-03-01 20:35:55)
>> 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@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
> 
> I do not agree that an APIChanges entry is necessary for a deprecation.
> APIChanges is for listing API changes. A deprecation is not by itself an
> API change, merely a statement of intent wrt future API changes.
> 
>  From the perspective of an API user it does not make much sense to ask
> whether a feature X is deprecated in version Y, only whether a
> replacement API is present. In this specific case, a replacement API has
> been present since 2006 - for more than half the project's existence.
> Surely that should be enough.

Yet the function was used in ffmpeg.c until 2018 when i removed it. It 
was dead code, but it was there and nobody noticed because it made no 
noise, so is it too crazy to think some project down there may still be 
using it because they were never nagged to make their code use the 
*_extradata bsfs?

Look, I don't really care about this function, and it's so old and part 
of AVCodecParserContext (which nobody outside the project likes) that 
probably nobody is using it (Google seems to agree), so if you really 
want it gone now and if it really predates APIChanges then just nuke it. 
But for future deprecations, an APIChanges entry and an attribute if 
applicable must be required, not just doxy. Doxy should mainly inform 
the developer of a potential replacement, or details about the 
deprecation (Like you wrote about what happens after 
thread_safe_callbacks is gone). APIChanges should be what keeps a 
history of additions and deprecations (Doxygen only keeps track of 
active deprecations). And of course the attribute so some actual noise 
is made.

Can we document this to prevent future pointless discussions about what 
is and what is not the correct process? And for the sake of everyone's 
sanity, don't summon the TC to solve a conflict about if we should add a 
line to two files or just one.
Andreas Rheinhardt March 2, 2021, 1:44 p.m. UTC | #12
James Almer:
> On 3/2/2021 5:39 AM, Anton Khirnov wrote:
>> Quoting James Almer (2021-03-01 20:35:55)
>>> 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@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
>>
>> I do not agree that an APIChanges entry is necessary for a deprecation.
>> APIChanges is for listing API changes. A deprecation is not by itself an
>> API change, merely a statement of intent wrt future API changes.
>>
>>  From the perspective of an API user it does not make much sense to ask
>> whether a feature X is deprecated in version Y, only whether a
>> replacement API is present. In this specific case, a replacement API has
>> been present since 2006 - for more than half the project's existence.
>> Surely that should be enough.
> 
> Yet the function was used in ffmpeg.c until 2018 when i removed it. It
> was dead code, but it was there and nobody noticed because it made no
> noise, so is it too crazy to think some project down there may still be
> using it because they were never nagged to make their code use the
> *_extradata bsfs?
> 
If someone didn't manage to read the documentation once every 15 years,
I don't have much pity for them.

> Look, I don't really care about this function, and it's so old and part
> of AVCodecParserContext (which nobody outside the project likes) that
> probably nobody is using it (Google seems to agree), so if you really

I also couldn't find any user of this function.

> want it gone now and if it really predates APIChanges then just nuke it.

Ok. But of course I won't nuke it now, only at the bump.

> But for future deprecations, an APIChanges entry and an attribute if
> applicable must be required, not just doxy. Doxy should mainly inform
> the developer of a potential replacement, or details about the
> deprecation (Like you wrote about what happens after
> thread_safe_callbacks is gone). APIChanges should be what keeps a
> history of additions and deprecations (Doxygen only keeps track of
> active deprecations). And of course the attribute so some actual noise
> is made.
> 
I agree that the deprecation process needs to be way better defined (as
long as the rules are only applied for future deprecations).
Just one suggestion in addition to what you said: For deprecated
AVOptions whose targets are ordinary users (not API users) it is enough
(and required) to add the AV_OPT_FLAG_DEPRECATED and to update the
documentation; an APIchanges entry should not be added as that would
just be noise for API users reading said file.

> Can we document this to prevent future pointless discussions about what
> is and what is not the correct process? And for the sake of everyone's
> sanity, don't summon the TC to solve a conflict about if we should add a
> line to two files or just one.

I won't summon the TC for this.

- Andreas

PS: I don't think it's only outside developers that don't like the
current parsing API. At least I don't like it either (it is not based
upon AVPackets, so it breaks side data; it is not refcounted).
Andreas Rheinhardt March 3, 2021, 2:08 p.m. UTC | #13
Andreas Rheinhardt:
> Originally deprecated in 748c2fca7e4d99357c234936aa71212a6282be36,
> publically deprecated in 9a07c1332cfe092b57b5758f22b686ca58806c60
> (merged into FFmpeg in 1885ffb03d0af28e6bac2bcc8725fa15b93f6ac9).
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@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)
> +#endif
>  #ifndef FF_API_THREAD_SAFE_CALLBACKS
>  #define FF_API_THREAD_SAFE_CALLBACKS (LIBAVCODEC_VERSION_MAJOR < 60)
>  #endif
> 
Will apply this patch tomorrow unless there are objections.

- Andreas
Anton Khirnov March 4, 2021, 11:27 a.m. UTC | #14
Quoting James Almer (2021-03-02 13:50:58)
> Can we document this to prevent future pointless discussions about what 
> is and what is not the correct process? And for the sake of everyone's 
> sanity, don't summon the TC to solve a conflict about if we should add a 
> line to two files or just one.

I am all for documenting development practices, though I don't think
they have to be applied retroactively to old changes.

In any case, this is a minor difference of opinion in my eyes. I do not
absolutely insist this function be removed RIGHT NOW, I would just
prefer to see it gone soon, since we have way too much old cruft as is.
TC is supposed to get involved only in cases of irreconcilable conflict,
not when people just happen to disagree on something.
diff mbox series

Patch

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)
+#endif
 #ifndef FF_API_THREAD_SAFE_CALLBACKS
 #define FF_API_THREAD_SAFE_CALLBACKS (LIBAVCODEC_VERSION_MAJOR < 60)
 #endif