Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Drew Dunne <asdunne-at-google.com@ffmpeg.org>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 1/2] swscale/output: Bias 16bps output calculations to improve non overflowing range
Date: Thu, 3 Nov 2022 10:07:45 -0400
Message-ID: <CAHHpSXagd69TNJGy4YtF87D5Docjj_UXWidNr8pMFOJs8uxYWQ@mail.gmail.com> (raw)
In-Reply-To: <20221102210239.1689-1-michael@niedermayer.cc>

I verified this fixes the overflow I was encountering.

On Wed, Nov 2, 2022 at 5:02 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> Fixes: integer overflow
> Fixes: ./ffmpeg   -f rawvideo -video_size 66x64 -pixel_format
> yuva420p10le   -i ~/videos/overflow_input_w66h64.yuva420p10le
>  -filter_complex
> "scale=flags=bicubic+full_chroma_int+full_chroma_inp+bitexact+accurate_rnd:in_color_matrix=bt2020:out_color_matrix=bt2020:in_range=full:out_range=full,format=rgba64[out]"
>  -pixel_format rgba64 -map '[out]'   -y overflow_w66h64.png
>
> Found-by: Drew Dunne <asdunne@google.com>
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libswscale/output.c | 120 ++++++++++++++++++++++----------------------
>  1 file changed, 60 insertions(+), 60 deletions(-)
>
> diff --git a/libswscale/output.c b/libswscale/output.c
> index 0e1c1225a0..df4647adde 100644
> --- a/libswscale/output.c
> +++ b/libswscale/output.c
> @@ -1100,8 +1100,8 @@ yuv2rgba64_X_c_template(SwsContext *c, const int16_t
> *lumFilter,
>          Y2 -= c->yuv2rgb_y_offset;
>          Y1 *= c->yuv2rgb_y_coeff;
>          Y2 *= c->yuv2rgb_y_coeff;
> -        Y1 += 1 << 13; // 21
> -        Y2 += 1 << 13;
> +        Y1 += (1 << 13) - (1 << 29); // 21
> +        Y2 += (1 << 13) - (1 << 29);
>          // 8 bits: 17 + 13 bits = 30 bits, 16 bits: 17 + 13 bits = 30 bits
>
>          R = V * c->yuv2rgb_v2r_coeff;
> @@ -1109,20 +1109,20 @@ yuv2rgba64_X_c_template(SwsContext *c, const
> int16_t *lumFilter,
>          B =                            U * c->yuv2rgb_u2b_coeff;
>
>          // 8 bits: 30 - 22 = 8 bits, 16 bits: 30 bits - 14 = 16 bits
> -        output_pixel(&dest[0], av_clip_uintp2(R_B + Y1, 30) >> 14);
> -        output_pixel(&dest[1], av_clip_uintp2(  G + Y1, 30) >> 14);
> -        output_pixel(&dest[2], av_clip_uintp2(B_R + Y1, 30) >> 14);
> +        output_pixel(&dest[0], av_clip_uintp2(((R_B + Y1) >> 14) +
> (1<<15), 16));
> +        output_pixel(&dest[1], av_clip_uintp2(((  G + Y1) >> 14) +
> (1<<15), 16));
> +        output_pixel(&dest[2], av_clip_uintp2(((B_R + Y1) >> 14) +
> (1<<15), 16));
>          if (eightbytes) {
>              output_pixel(&dest[3], av_clip_uintp2(A1      , 30) >> 14);
> -            output_pixel(&dest[4], av_clip_uintp2(R_B + Y2, 30) >> 14);
> -            output_pixel(&dest[5], av_clip_uintp2(  G + Y2, 30) >> 14);
> -            output_pixel(&dest[6], av_clip_uintp2(B_R + Y2, 30) >> 14);
> +            output_pixel(&dest[4], av_clip_uintp2(((R_B + Y2) >> 14) +
> (1<<15), 16));
> +            output_pixel(&dest[5], av_clip_uintp2(((  G + Y2) >> 14) +
> (1<<15), 16));
> +            output_pixel(&dest[6], av_clip_uintp2(((B_R + Y2) >> 14) +
> (1<<15), 16));
>              output_pixel(&dest[7], av_clip_uintp2(A2      , 30) >> 14);
>              dest += 8;
>          } else {
> -            output_pixel(&dest[3], av_clip_uintp2(R_B + Y2, 30) >> 14);
> -            output_pixel(&dest[4], av_clip_uintp2(  G + Y2, 30) >> 14);
> -            output_pixel(&dest[5], av_clip_uintp2(B_R + Y2, 30) >> 14);
> +            output_pixel(&dest[3], av_clip_uintp2(((R_B + Y2) >> 14) +
> (1<<15), 16));
> +            output_pixel(&dest[4], av_clip_uintp2(((  G + Y2) >> 14) +
> (1<<15), 16));
> +            output_pixel(&dest[5], av_clip_uintp2(((B_R + Y2) >> 14) +
> (1<<15), 16));
>              dest += 6;
>          }
>      }
> @@ -1160,8 +1160,8 @@ yuv2rgba64_2_c_template(SwsContext *c, const int32_t
> *buf[2],
>          Y2 -= c->yuv2rgb_y_offset;
>          Y1 *= c->yuv2rgb_y_coeff;
>          Y2 *= c->yuv2rgb_y_coeff;
> -        Y1 += 1 << 13;
> -        Y2 += 1 << 13;
> +        Y1 += (1 << 13) - (1 << 29);
> +        Y2 += (1 << 13) - (1 << 29);
>
>          R = V * c->yuv2rgb_v2r_coeff;
>          G = V * c->yuv2rgb_v2g_coeff + U * c->yuv2rgb_u2g_coeff;
> @@ -1175,20 +1175,20 @@ yuv2rgba64_2_c_template(SwsContext *c, const
> int32_t *buf[2],
>              A2 += 1 << 13;
>          }
>
> -        output_pixel(&dest[0], av_clip_uintp2(R_B + Y1, 30) >> 14);
> -        output_pixel(&dest[1], av_clip_uintp2(  G + Y1, 30) >> 14);
> -        output_pixel(&dest[2], av_clip_uintp2(B_R + Y1, 30) >> 14);
> +        output_pixel(&dest[0], av_clip_uintp2(((R_B + Y1) >> 14) +
> (1<<15), 16));
> +        output_pixel(&dest[1], av_clip_uintp2(((  G + Y1) >> 14) +
> (1<<15), 16));
> +        output_pixel(&dest[2], av_clip_uintp2(((B_R + Y1) >> 14) +
> (1<<15), 16));
>          if (eightbytes) {
>              output_pixel(&dest[3], av_clip_uintp2(A1      , 30) >> 14);
> -            output_pixel(&dest[4], av_clip_uintp2(R_B + Y2, 30) >> 14);
> -            output_pixel(&dest[5], av_clip_uintp2(  G + Y2, 30) >> 14);
> -            output_pixel(&dest[6], av_clip_uintp2(B_R + Y2, 30) >> 14);
> +            output_pixel(&dest[4], av_clip_uintp2(((R_B + Y2) >> 14) +
> (1<<15), 16));
> +            output_pixel(&dest[5], av_clip_uintp2(((  G + Y2) >> 14) +
> (1<<15), 16));
> +            output_pixel(&dest[6], av_clip_uintp2(((B_R + Y2) >> 14) +
> (1<<15), 16));
>              output_pixel(&dest[7], av_clip_uintp2(A2      , 30) >> 14);
>              dest += 8;
>          } else {
> -            output_pixel(&dest[3], av_clip_uintp2(R_B + Y2, 30) >> 14);
> -            output_pixel(&dest[4], av_clip_uintp2(  G + Y2, 30) >> 14);
> -            output_pixel(&dest[5], av_clip_uintp2(B_R + Y2, 30) >> 14);
> +            output_pixel(&dest[3], av_clip_uintp2(((R_B + Y2) >> 14) +
> (1<<15), 16));
> +            output_pixel(&dest[4], av_clip_uintp2(((  G + Y2) >> 14) +
> (1<<15), 16));
> +            output_pixel(&dest[5], av_clip_uintp2(((B_R + Y2) >> 14) +
> (1<<15), 16));
>              dest += 6;
>          }
>      }
> @@ -1217,8 +1217,8 @@ yuv2rgba64_1_c_template(SwsContext *c, const int32_t
> *buf0,
>              Y2 -= c->yuv2rgb_y_offset;
>              Y1 *= c->yuv2rgb_y_coeff;
>              Y2 *= c->yuv2rgb_y_coeff;
> -            Y1 += 1 << 13;
> -            Y2 += 1 << 13;
> +            Y1 += (1 << 13) - (1 << 29);
> +            Y2 += (1 << 13) - (1 << 29);
>
>              if (hasAlpha) {
>                  A1 = abuf0[i * 2    ] << 11;
> @@ -1232,20 +1232,20 @@ yuv2rgba64_1_c_template(SwsContext *c, const
> int32_t *buf0,
>              G = V * c->yuv2rgb_v2g_coeff + U * c->yuv2rgb_u2g_coeff;
>              B =                            U * c->yuv2rgb_u2b_coeff;
>
> -            output_pixel(&dest[0], av_clip_uintp2(R_B + Y1, 30) >> 14);
> -            output_pixel(&dest[1], av_clip_uintp2(  G + Y1, 30) >> 14);
> -            output_pixel(&dest[2], av_clip_uintp2(B_R + Y1, 30) >> 14);
> +            output_pixel(&dest[0], av_clip_uintp2(((R_B + Y1) >> 14) +
> (1<<15), 16));
> +            output_pixel(&dest[1], av_clip_uintp2(((  G + Y1) >> 14) +
> (1<<15), 16));
> +            output_pixel(&dest[2], av_clip_uintp2(((B_R + Y1) >> 14) +
> (1<<15), 16));
>              if (eightbytes) {
>                  output_pixel(&dest[3], av_clip_uintp2(A1      , 30) >>
> 14);
> -                output_pixel(&dest[4], av_clip_uintp2(R_B + Y2, 30) >>
> 14);
> -                output_pixel(&dest[5], av_clip_uintp2(  G + Y2, 30) >>
> 14);
> -                output_pixel(&dest[6], av_clip_uintp2(B_R + Y2, 30) >>
> 14);
> +                output_pixel(&dest[4], av_clip_uintp2(((R_B + Y2) >> 14)
> + (1<<15), 16));
> +                output_pixel(&dest[5], av_clip_uintp2(((  G + Y2) >> 14)
> + (1<<15), 16));
> +                output_pixel(&dest[6], av_clip_uintp2(((B_R + Y2) >> 14)
> + (1<<15), 16));
>                  output_pixel(&dest[7], av_clip_uintp2(A2      , 30) >>
> 14);
>                  dest += 8;
>              } else {
> -                output_pixel(&dest[3], av_clip_uintp2(R_B + Y2, 30) >>
> 14);
> -                output_pixel(&dest[4], av_clip_uintp2(  G + Y2, 30) >>
> 14);
> -                output_pixel(&dest[5], av_clip_uintp2(B_R + Y2, 30) >>
> 14);
> +                output_pixel(&dest[3], av_clip_uintp2(((R_B + Y2) >> 14)
> + (1<<15), 16));
> +                output_pixel(&dest[4], av_clip_uintp2(((  G + Y2) >> 14)
> + (1<<15), 16));
> +                output_pixel(&dest[5], av_clip_uintp2(((B_R + Y2) >> 14)
> + (1<<15), 16));
>                  dest += 6;
>              }
>          }
> @@ -1263,8 +1263,8 @@ yuv2rgba64_1_c_template(SwsContext *c, const int32_t
> *buf0,
>              Y2 -= c->yuv2rgb_y_offset;
>              Y1 *= c->yuv2rgb_y_coeff;
>              Y2 *= c->yuv2rgb_y_coeff;
> -            Y1 += 1 << 13;
> -            Y2 += 1 << 13;
> +            Y1 += (1 << 13) - (1 << 29);
> +            Y2 += (1 << 13) - (1 << 29);
>
>              if (hasAlpha) {
>                  A1 = abuf0[i * 2    ] << 11;
> @@ -1278,20 +1278,20 @@ yuv2rgba64_1_c_template(SwsContext *c, const
> int32_t *buf0,
>              G = V * c->yuv2rgb_v2g_coeff + U * c->yuv2rgb_u2g_coeff;
>              B =                            U * c->yuv2rgb_u2b_coeff;
>
> -            output_pixel(&dest[0], av_clip_uintp2(R_B + Y1, 30) >> 14);
> -            output_pixel(&dest[1], av_clip_uintp2(  G + Y1, 30) >> 14);
> -            output_pixel(&dest[2], av_clip_uintp2(B_R + Y1, 30) >> 14);
> +            output_pixel(&dest[0], av_clip_uintp2(((R_B + Y1) >> 14) +
> (1<<15), 16));
> +            output_pixel(&dest[1], av_clip_uintp2(((  G + Y1) >> 14) +
> (1<<15), 16));
> +            output_pixel(&dest[2], av_clip_uintp2(((B_R + Y1) >> 14) +
> (1<<15), 16));
>              if (eightbytes) {
>                  output_pixel(&dest[3], av_clip_uintp2(A1      , 30) >>
> 14);
> -                output_pixel(&dest[4], av_clip_uintp2(R_B + Y2, 30) >>
> 14);
> -                output_pixel(&dest[5], av_clip_uintp2(  G + Y2, 30) >>
> 14);
> -                output_pixel(&dest[6], av_clip_uintp2(B_R + Y2, 30) >>
> 14);
> +                output_pixel(&dest[4], av_clip_uintp2(((R_B + Y2) >> 14)
> + (1<<15), 16));
> +                output_pixel(&dest[5], av_clip_uintp2(((  G + Y2) >> 14)
> + (1<<15), 16));
> +                output_pixel(&dest[6], av_clip_uintp2(((B_R + Y2) >> 14)
> + (1<<15), 16));
>                  output_pixel(&dest[7], av_clip_uintp2(A2      , 30) >>
> 14);
>                  dest += 8;
>              } else {
> -                output_pixel(&dest[3], av_clip_uintp2(R_B + Y2, 30) >>
> 14);
> -                output_pixel(&dest[4], av_clip_uintp2(  G + Y2, 30) >>
> 14);
> -                output_pixel(&dest[5], av_clip_uintp2(B_R + Y2, 30) >>
> 14);
> +                output_pixel(&dest[3], av_clip_uintp2(((R_B + Y2) >> 14)
> + (1<<15), 16));
> +                output_pixel(&dest[4], av_clip_uintp2(((  G + Y2) >> 14)
> + (1<<15), 16));
> +                output_pixel(&dest[5], av_clip_uintp2(((B_R + Y2) >> 14)
> + (1<<15), 16));
>                  dest += 6;
>              }
>          }
> @@ -1343,7 +1343,7 @@ yuv2rgba64_full_X_c_template(SwsContext *c, const
> int16_t *lumFilter,
>          // 8bit: 27 -> 17bit, 16bit: 31 - 14 = 17bit
>          Y -= c->yuv2rgb_y_offset;
>          Y *= c->yuv2rgb_y_coeff;
> -        Y += 1 << 13; // 21
> +        Y += (1 << 13) - (1<<29); // 21
>          // 8bit: 17 + 13bit = 30bit, 16bit: 17 + 13bit = 30bit
>
>          R = V * c->yuv2rgb_v2r_coeff;
> @@ -1351,9 +1351,9 @@ yuv2rgba64_full_X_c_template(SwsContext *c, const
> int16_t *lumFilter,
>          B =                            U * c->yuv2rgb_u2b_coeff;
>
>          // 8bit: 30 - 22 = 8bit, 16bit: 30bit - 14 = 16bit
> -        output_pixel(&dest[0], av_clip_uintp2(R_B + Y, 30) >> 14);
> -        output_pixel(&dest[1], av_clip_uintp2(  G + Y, 30) >> 14);
> -        output_pixel(&dest[2], av_clip_uintp2(B_R + Y, 30) >> 14);
> +        output_pixel(&dest[0], av_clip_uintp2(((R_B + Y)>>14) + (1<<15),
> 16));
> +        output_pixel(&dest[1], av_clip_uintp2(((  G + Y)>>14) + (1<<15),
> 16));
> +        output_pixel(&dest[2], av_clip_uintp2(((B_R + Y)>>14) + (1<<15),
> 16));
>          if (eightbytes) {
>              output_pixel(&dest[3], av_clip_uintp2(A, 30) >> 14);
>              dest += 4;
> @@ -1392,7 +1392,7 @@ yuv2rgba64_full_2_c_template(SwsContext *c, const
> int32_t *buf[2],
>
>          Y -= c->yuv2rgb_y_offset;
>          Y *= c->yuv2rgb_y_coeff;
> -        Y += 1 << 13;
> +        Y += (1 << 13) - (1 << 29);
>
>          R = V * c->yuv2rgb_v2r_coeff;
>          G = V * c->yuv2rgb_v2g_coeff + U * c->yuv2rgb_u2g_coeff;
> @@ -1404,9 +1404,9 @@ yuv2rgba64_full_2_c_template(SwsContext *c, const
> int32_t *buf[2],
>              A += 1 << 13;
>          }
>
> -        output_pixel(&dest[0], av_clip_uintp2(R_B + Y, 30) >> 14);
> -        output_pixel(&dest[1], av_clip_uintp2(  G + Y, 30) >> 14);
> -        output_pixel(&dest[2], av_clip_uintp2(B_R + Y, 30) >> 14);
> +        output_pixel(&dest[0], av_clip_uintp2(((R_B + Y) >> 14) +
> (1<<15), 16));
> +        output_pixel(&dest[1], av_clip_uintp2(((  G + Y) >> 14) +
> (1<<15), 16));
> +        output_pixel(&dest[2], av_clip_uintp2(((B_R + Y) >> 14) +
> (1<<15), 16));
>          if (eightbytes) {
>              output_pixel(&dest[3], av_clip_uintp2(A, 30) >> 14);
>              dest += 4;
> @@ -1436,7 +1436,7 @@ yuv2rgba64_full_1_c_template(SwsContext *c, const
> int32_t *buf0,
>
>              Y -= c->yuv2rgb_y_offset;
>              Y *= c->yuv2rgb_y_coeff;
> -            Y += 1 << 13;
> +            Y += (1 << 13) - (1 << 29);
>
>              if (hasAlpha) {
>                  A = abuf0[i] << 11;
> @@ -1448,9 +1448,9 @@ yuv2rgba64_full_1_c_template(SwsContext *c, const
> int32_t *buf0,
>              G = V * c->yuv2rgb_v2g_coeff + U * c->yuv2rgb_u2g_coeff;
>              B =                            U * c->yuv2rgb_u2b_coeff;
>
> -            output_pixel(&dest[0], av_clip_uintp2(R_B + Y, 30) >> 14);
> -            output_pixel(&dest[1], av_clip_uintp2(  G + Y, 30) >> 14);
> -            output_pixel(&dest[2], av_clip_uintp2(B_R + Y, 30) >> 14);
> +            output_pixel(&dest[0], av_clip_uintp2(((R_B + Y) >> 14) +
> (1<<15), 16));
> +            output_pixel(&dest[1], av_clip_uintp2(((  G + Y) >> 14) +
> (1<<15), 16));
> +            output_pixel(&dest[2], av_clip_uintp2(((B_R + Y) >> 14) +
> (1<<15), 16));
>              if (eightbytes) {
>                  output_pixel(&dest[3], av_clip_uintp2(A, 30) >> 14);
>                  dest += 4;
> @@ -1469,7 +1469,7 @@ yuv2rgba64_full_1_c_template(SwsContext *c, const
> int32_t *buf0,
>
>              Y -= c->yuv2rgb_y_offset;
>              Y *= c->yuv2rgb_y_coeff;
> -            Y += 1 << 13;
> +            Y += (1 << 13) - (1 << 29);
>
>              if (hasAlpha) {
>                  A = abuf0[i] << 11;
> @@ -1481,9 +1481,9 @@ yuv2rgba64_full_1_c_template(SwsContext *c, const
> int32_t *buf0,
>              G = V * c->yuv2rgb_v2g_coeff + U * c->yuv2rgb_u2g_coeff;
>              B =                            U * c->yuv2rgb_u2b_coeff;
>
> -            output_pixel(&dest[0], av_clip_uintp2(R_B + Y, 30) >> 14);
> -            output_pixel(&dest[1], av_clip_uintp2(  G + Y, 30) >> 14);
> -            output_pixel(&dest[2], av_clip_uintp2(B_R + Y, 30) >> 14);
> +            output_pixel(&dest[0], av_clip_uintp2(((R_B + Y) >> 14) +
> (1<<15), 16));
> +            output_pixel(&dest[1], av_clip_uintp2(((  G + Y) >> 14) +
> (1<<15), 16));
> +            output_pixel(&dest[2], av_clip_uintp2(((B_R + Y) >> 14) +
> (1<<15), 16));
>              if (eightbytes) {
>                  output_pixel(&dest[3], av_clip_uintp2(A, 30) >> 14);
>                  dest += 4;
> --
> 2.17.1
>
> _______________________________________________
> 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".
>


-- 
Drew Dunne
asdunne@google.com
_______________________________________________
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".

  parent reply	other threads:[~2022-11-03 14:08 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-02 21:02 Michael Niedermayer
2022-11-02 21:02 ` [FFmpeg-devel] [PATCH 2/2] swscale/output: Bias 16bps output calculations to improve non overflowing range for GBRP16/GBRPF32 Michael Niedermayer
2022-11-02 21:07   ` Michael Niedermayer
2022-11-02 21:16     ` Andreas Rheinhardt
2022-11-02 21:31       ` Michael Niedermayer
2022-11-02 21:36         ` Martin Storsjö
2022-11-02 21:42         ` Michael Niedermayer
2022-11-02 22:52   ` Mark Reid
2022-11-02 22:55     ` Mark Reid
2022-11-03 14:07 ` Drew Dunne [this message]
2022-11-04 21:45   ` [FFmpeg-devel] [PATCH 1/2] swscale/output: Bias 16bps output calculations to improve non overflowing range Michael Niedermayer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHHpSXagd69TNJGy4YtF87D5Docjj_UXWidNr8pMFOJs8uxYWQ@mail.gmail.com \
    --to=asdunne-at-google.com@ffmpeg.org \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git