[FFmpeg-devel] [PATCH 2/2] avcodec: deprecate AVCodecContext.debug_mv

[Andreas Rheinhardt]

James Almer:
> On 1/24/2021 8:14 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> It's been unused for almost three years now.
>>>
>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>> ---
>>>   doc/APIchanges             |  4 ++++
>>>   libavcodec/avcodec.h       |  7 ++++---
>>>   libavcodec/pthread_frame.c |  4 ++++
>>>   libavcodec/version.h       | 10 +++++-----
>>>   4 files changed, 17 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/doc/APIchanges b/doc/APIchanges
>>> index bbf56a5385..51f97b2759 100644
>>> --- a/doc/APIchanges
>>> +++ b/doc/APIchanges
>>> @@ -15,6 +15,10 @@ libavutil:     2017-10-21
>>>     API changes, most recent first:
>>>   +2021-01-xx - xxxxxxxxxx - lavc 58.118.100 - avcodec.h
>>> +  deprecate AVCodecContext.debug_mv, FF_DEBUG_VIS_MV_P_FOR,
>>> FF_DEBUG_VIS_MV_B_FOR,
>>> +  FF_DEBUG_VIS_MV_B_BACK
>>> +
>>>   2021-01-11 - xxxxxxxxxx - lavc 58.116.100 - avcodec.h
>>>     Add FF_PROFILE_VVC_MAIN_10 and FF_PROFILE_VVC_MAIN_10_444.
>>>   diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>>> index dc8738c819..eafc2e84e3 100644
>>> --- a/libavcodec/avcodec.h
>>> +++ b/libavcodec/avcodec.h
>>> @@ -2141,15 +2141,16 @@ typedef struct AVCodecContext {
>>>        */
>>>       int seek_preroll;
>>>   +#if FF_API_DEBUG_MV
>>>       /**
>>> -     * debug motion vectors
>>> -     * - encoding: Set by user.
>>> -     * - decoding: Set by user.
>>> +     * @deprecated unused
>>>        */
>>> +    attribute_deprecated
>>>       int debug_mv;
>>>   #define FF_DEBUG_VIS_MV_P_FOR  0x00000001 //visualize forward
>>> predicted MVs of P frames
>>>   #define FF_DEBUG_VIS_MV_B_FOR  0x00000002 //visualize forward
>>> predicted MVs of B frames
>>>   #define FF_DEBUG_VIS_MV_B_BACK 0x00000004 //visualize backward
>>> predicted MVs of B frames
>>> +#endif
>>>         /**
>>>        * custom intra quantization matrix
>>> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
>>> index a570e25e0d..3abb691ac7 100644
>>> --- a/libavcodec/pthread_frame.c
>>> +++ b/libavcodec/pthread_frame.c
>>> @@ -342,7 +342,11 @@ static int
>>> update_context_from_user(AVCodecContext *dst, AVCodecContext *src)
>>>         dst->opaque   = src->opaque;
>>>       dst->debug    = src->debug;
>>> +#if FF_API_DEBUG_MV
>>> +FF_DISABLE_DEPRECATION_WARNINGS
>>>       dst->debug_mv = src->debug_mv;
>>> +FF_ENABLE_DEPRECATION_WARNINGS
>>> +#endif
>>
>> If debug_mv is unused, then why don't you just remove the assignment?
> 
> It's a "set by user" field, so even if whatever value it may contain is
> never used by libavcodec, removing this would be a change in behavior.
> Admittedly harmless, but a change nonetheless, so I'm inclined to just
> leave it there.
> 

What user-visible behaviour would change?

>>
>>>         dst->slice_flags = src->slice_flags;
>>>       dst->flags2      = src->flags2;
>>> diff --git a/libavcodec/version.h b/libavcodec/version.h
>>> index 9f80caa9e0..f3974c2cb3 100644
>>> --- a/libavcodec/version.h
>>> +++ b/libavcodec/version.h
>>> @@ -28,8 +28,8 @@
>>>   #include "libavutil/version.h"
>>>     #define LIBAVCODEC_VERSION_MAJOR  58
>>> -#define LIBAVCODEC_VERSION_MINOR 117
>>> -#define LIBAVCODEC_VERSION_MICRO 101
>>> +#define LIBAVCODEC_VERSION_MINOR 118
>>> +#define LIBAVCODEC_VERSION_MICRO 100
>>>     #define LIBAVCODEC_VERSION_INT 
>>> AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
>>>                                                 
>>> LIBAVCODEC_VERSION_MINOR, \
>>> @@ -54,9 +54,6 @@
>>>   #ifndef FF_API_LOWRES
>>>   #define FF_API_LOWRES            (LIBAVCODEC_VERSION_MAJOR < 59)
>>>   #endif
>>> -#ifndef FF_API_DEBUG_MV
>>> -#define FF_API_DEBUG_MV          (LIBAVCODEC_VERSION_MAJOR < 58)
>>> -#endif
>>
>> Why is this hunk not in the preceding patch?
> 
> I figured I'd leave the actual macro in place instead of removing it and
> then re-adding it in a following commit, but i can change it if you
> prefer that way.
> 
Keeping it would make sense if you just replaced 58 by 60; but not if
you are moving this to the end anyway.

>>
>>>   #ifndef FF_API_AVCTX_TIMEBASE
>>>   #define FF_API_AVCTX_TIMEBASE    (LIBAVCODEC_VERSION_MAJOR < 59)
>>>   #endif
>>> @@ -150,5 +147,8 @@
>>>   #ifndef FF_API_THREAD_SAFE_CALLBACKS
>>>   #define FF_API_THREAD_SAFE_CALLBACKS (LIBAVCODEC_VERSION_MAJOR < 60)
>>>   #endif
>>> +#ifndef FF_API_DEBUG_MV
>>> +#define FF_API_DEBUG_MV          (LIBAVCODEC_VERSION_MAJOR < 60)
>>> +#endif
>>>     #endif /* AVCODEC_VERSION_H */
>>>