[FFmpeg-devel] [PATCH 2/9] avcodec/gdv: Replace divisions by shifts in rescale()

[James Almer]

On 8/5/2018 5:29 PM, Michael Niedermayer wrote:
> Divisions tend to be slower than shifts unless the compiler optimizes them out.
> And some of these are in inner loops.

This patch makes the code slightly less readable, IMO. What compiler
doesn't optimize out an integer division by 2 into a right shift?
Is this part of the cause for the timeouts you mention in patch 9/9? Do
those run on builds with compiler optimizations disabled then?

The other patches in this set look good since they effectively simplify
and optimize the code for all scenarios (memcpy vs loop, reduced amount
of iterations, etc). But this one is trying to work around a decision
made by a compiler in a non real world scenario with specific runtime
constrains.
Does this timeout still happen if you apply the entire set except for
this patch?

> 
> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> ---
>  libavcodec/gdv.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/libavcodec/gdv.c b/libavcodec/gdv.c
> index e52a637610..79ca157dde 100644
> --- a/libavcodec/gdv.c
> +++ b/libavcodec/gdv.c
> @@ -85,14 +85,14 @@ static void rescale(GDVContext *gdv, uint8_t *dst, int w, int h, int scale_v, in
>              int y = h - j - 1;
>              for (i = 0; i < w; i++) {
>                  int x = w - i - 1;
> -                dst[PREAMBLE_SIZE + x + y * w] = dst[PREAMBLE_SIZE + x/2 + (y/2) * (w/2)];
> +                dst[PREAMBLE_SIZE + x + y * w] = dst[PREAMBLE_SIZE + (x>>1) + (y>>1) * (w>>1)];
>              }
>          }
>      } else if (gdv->scale_h) {
>          for (j = 0; j < h; j++) {
>              int y = h - j - 1;
>              for (x = 0; x < w; x++) {
> -                dst[PREAMBLE_SIZE + x + y * w] = dst[PREAMBLE_SIZE + x + (y/2) * w];
> +                dst[PREAMBLE_SIZE + x + y * w] = dst[PREAMBLE_SIZE + x + (y>>1) * w];
>              }
>          }
>      } else if (gdv->scale_v) {
> @@ -100,26 +100,26 @@ static void rescale(GDVContext *gdv, uint8_t *dst, int w, int h, int scale_v, in
>              int y = h - j - 1;
>              for (i = 0; i < w; i++) {
>                  int x = w - i - 1;
> -                dst[PREAMBLE_SIZE + x + y * w] = dst[PREAMBLE_SIZE + x/2 + y * (w/2)];
> +                dst[PREAMBLE_SIZE + x + y * w] = dst[PREAMBLE_SIZE + (x>>1) + y * (w>>1)];
>              }
>          }
>      }
>  
>      if (scale_h && scale_v) {
> -        for (y = 0; y < h/2; y++) {
> -            for (x = 0; x < w/2; x++) {
> -                dst[PREAMBLE_SIZE + x + y * (w/2)] = dst[PREAMBLE_SIZE + x*2 + y*2 * w];
> +        for (y = 0; y < (h>>1); y++) {
> +            for (x = 0; x < (w>>1); x++) {
> +                dst[PREAMBLE_SIZE + x + y * (w>>1)] = dst[PREAMBLE_SIZE + x*2 + y*2 * w];
>              }
>          }
>      } else if (scale_h) {
> -        for (y = 0; y < h/2; y++) {
> +        for (y = 0; y < (h>>1); y++) {
>              for (x = 0; x < w; x++) {
>                  dst[PREAMBLE_SIZE + x + y * w] = dst[PREAMBLE_SIZE + x + y*2 * w];
>              }
>          }
>      } else if (scale_v) {
>          for (y = 0; y < h; y++) {
> -            for (x = 0; x < w/2; x++) {
> +            for (x = 0; x < (w>>1); x++) {
>                  dst[PREAMBLE_SIZE + x + y * w] = dst[PREAMBLE_SIZE + x*2 + y * w];
>              }
>          }
>