[FFmpeg-devel] [PATCH] avutil: Rename RSHIFT macro to ROUNDED_RSHIFT

[James Almer]

On 1/21/2019 4:09 PM, FeRD (Frank Dana) wrote:
> The RSHIFT macro in libavutil/common.h does not actually perform
> a bitwise right-shift, but rather a rounded version of the same
> operation, as is noted by a comment above the macro. The rounded
> divsion macro on the very next line is named ROUNDED_DIV, which
> seems far more clear.
> 
> This patch renames RSHIFT to ROUNDED_RSHIFT, then updates all
> uses of the macro to use the new name. (These are all located
> in three codec files under libavcodec/.)
> 
> Signed-off-by: FeRD (Frank Dana) <ferdnyc at gmail.com>
> ---
>  libavcodec/mpeg4videodec.c |  4 ++--
>  libavcodec/vp3.c           | 16 ++++++++--------
>  libavcodec/vp56.c          |  4 ++--
>  libavutil/common.h         |  2 +-
>  4 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
> index f44ee76bd4..5d63ba12ba 100644
> --- a/libavcodec/mpeg4videodec.c
> +++ b/libavcodec/mpeg4videodec.c
> @@ -601,7 +601,7 @@ static inline int get_amv(Mpeg4DecContext *ctx, int n)
>          if (ctx->divx_version == 500 && ctx->divx_build == 413 && a >= s->quarter_sample)
>              sum = s->sprite_offset[0][n] / (1 << (a - s->quarter_sample));
>          else
> -            sum = RSHIFT(s->sprite_offset[0][n] * (1 << s->quarter_sample), a);
> +            sum = ROUNDED_RSHIFT(s->sprite_offset[0][n] * (1 << s->quarter_sample), a);
>      } else {
>          dx    = s->sprite_delta[n][0];
>          dy    = s->sprite_delta[n][1];
> @@ -623,7 +623,7 @@ static inline int get_amv(Mpeg4DecContext *ctx, int n)
>                  v   += dx;
>              }
>          }
> -        sum = RSHIFT(sum, a + 8 - s->quarter_sample);
> +        sum = ROUNDED_RSHIFT(sum, a + 8 - s->quarter_sample);
>      }
>  
>      if (sum < -len)
> diff --git a/libavcodec/vp3.c b/libavcodec/vp3.c
> index a5d8c2ed0b..13b3d6e22a 100644
> --- a/libavcodec/vp3.c
> +++ b/libavcodec/vp3.c
> @@ -861,10 +861,10 @@ static int unpack_vectors(Vp3DecodeContext *s, GetBitContext *gb)
>  
>                  if (s->chroma_y_shift) {
>                      if (s->macroblock_coding[current_macroblock] == MODE_INTER_FOURMV) {
> -                        motion_x[0] = RSHIFT(motion_x[0] + motion_x[1] +
> -                                             motion_x[2] + motion_x[3], 2);
> -                        motion_y[0] = RSHIFT(motion_y[0] + motion_y[1] +
> -                                             motion_y[2] + motion_y[3], 2);
> +                        motion_x[0] = ROUNDED_RSHIFT(motion_x[0] + motion_x[1] +
> +                                                     motion_x[2] + motion_x[3], 2);
> +                        motion_y[0] = ROUNDED_RSHIFT(motion_y[0] + motion_y[1] +
> +                                                     motion_y[2] + motion_y[3], 2);
>                      }
>                      motion_x[0] = (motion_x[0] >> 1) | (motion_x[0] & 1);
>                      motion_y[0] = (motion_y[0] >> 1) | (motion_y[0] & 1);
> @@ -873,10 +873,10 @@ static int unpack_vectors(Vp3DecodeContext *s, GetBitContext *gb)
>                      s->motion_val[1][frag][1] = motion_y[0];
>                  } else if (s->chroma_x_shift) {
>                      if (s->macroblock_coding[current_macroblock] == MODE_INTER_FOURMV) {
> -                        motion_x[0] = RSHIFT(motion_x[0] + motion_x[1], 1);
> -                        motion_y[0] = RSHIFT(motion_y[0] + motion_y[1], 1);
> -                        motion_x[1] = RSHIFT(motion_x[2] + motion_x[3], 1);
> -                        motion_y[1] = RSHIFT(motion_y[2] + motion_y[3], 1);
> +                        motion_x[0] = ROUNDED_RSHIFT(motion_x[0] + motion_x[1], 1);
> +                        motion_y[0] = ROUNDED_RSHIFT(motion_y[0] + motion_y[1], 1);
> +                        motion_x[1] = ROUNDED_RSHIFT(motion_x[2] + motion_x[3], 1);
> +                        motion_y[1] = ROUNDED_RSHIFT(motion_y[2] + motion_y[3], 1);
>                      } else {
>                          motion_x[1] = motion_x[0];
>                          motion_y[1] = motion_y[0];
> diff --git a/libavcodec/vp56.c b/libavcodec/vp56.c
> index b69fe6c176..9359b48bc6 100644
> --- a/libavcodec/vp56.c
> +++ b/libavcodec/vp56.c
> @@ -197,8 +197,8 @@ static void vp56_decode_4mv(VP56Context *s, int row, int col)
>  
>      /* chroma vectors are average luma vectors */
>      if (s->avctx->codec->id == AV_CODEC_ID_VP5) {
> -        s->mv[4].x = s->mv[5].x = RSHIFT(mv.x,2);
> -        s->mv[4].y = s->mv[5].y = RSHIFT(mv.y,2);
> +        s->mv[4].x = s->mv[5].x = ROUNDED_RSHIFT(mv.x,2);
> +        s->mv[4].y = s->mv[5].y = ROUNDED_RSHIFT(mv.y,2);
>      } else {
>          s->mv[4] = s->mv[5] = (VP56mv) {mv.x/4, mv.y/4};
>      }
> diff --git a/libavutil/common.h b/libavutil/common.h
> index 8db0291170..0bff7f8f72 100644
> --- a/libavutil/common.h
> +++ b/libavutil/common.h
> @@ -51,7 +51,7 @@
>  #endif
>  
>  //rounded division & shift
> -#define RSHIFT(a,b) ((a) > 0 ? ((a) + ((1<<(b))>>1))>>(b) : ((a) + ((1<<(b))>>1)-1)>>(b))
> +#define ROUNDED_RSHIFT(a,b) ((a) > 0 ? ((a) + ((1<<(b))>>1))>>(b) : ((a) + ((1<<(b))>>1)-1)>>(b))

common.h is a public installed library, so this would be an API break.

There's also no good way to deprecate a define and replace it with
another while informing the library user, so for something purely
cosmetic like this i don't think it's worth the trouble.

>  /* assume b>0 */
>  #define ROUNDED_DIV(a,b) (((a)>0 ? (a) + ((b)>>1) : (a) - ((b)>>1))/(b))
>  /* Fast a/(1<<b) rounded toward +inf. Assume a>=0 and b>=0 */
>