diff mbox series

[FFmpeg-devel,3/3] swscale/output: Fix several invalid shifts in yuv2rgb_full_1_c_template()

Message ID 20200121221454.17655-3-michael@niedermayer.cc
State New
Headers show
Series [FFmpeg-devel,1/3] swscale/input: Fix several invalid shifts related to rgb2yuv constants
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Michael Niedermayer Jan. 21, 2020, 10:14 p.m. UTC
Fixes: Invalid shifts
Fixes: #8320

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libswscale/output.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Andreas Rheinhardt Jan. 22, 2020, 5:13 a.m. UTC | #1
On Tue, Jan 21, 2020 at 11:24 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> Fixes: Invalid shifts
> Fixes: #8320
>
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libswscale/output.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libswscale/output.c b/libswscale/output.c
> index d192ea854b..439de04a05 100644
> --- a/libswscale/output.c
> +++ b/libswscale/output.c
> @@ -2111,8 +2111,8 @@ yuv2rgb_full_1_c_template(SwsContext *c, const
> int16_t *buf0,
>          int A = 0; //init to silence warning
>          for (i = 0; i < dstW; i++) {
>              int Y = buf0[i] << 2;
>

buf0[i] is an int16_t. Is it certain (in the sense of: checked, not in the
sense of "negative luminance is nonsense") that it can't be negative?
LGTM apart from that.

- Andreas
Michael Niedermayer Jan. 22, 2020, 1:02 p.m. UTC | #2
On Wed, Jan 22, 2020 at 06:13:52AM +0100, Andreas Rheinhardt wrote:
> On Tue, Jan 21, 2020 at 11:24 PM Michael Niedermayer <michael@niedermayer.cc>
> wrote:
> 
> > Fixes: Invalid shifts
> > Fixes: #8320
> >
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libswscale/output.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/libswscale/output.c b/libswscale/output.c
> > index d192ea854b..439de04a05 100644
> > --- a/libswscale/output.c
> > +++ b/libswscale/output.c
> > @@ -2111,8 +2111,8 @@ yuv2rgb_full_1_c_template(SwsContext *c, const
> > int16_t *buf0,
> >          int A = 0; //init to silence warning
> >          for (i = 0; i < dstW; i++) {
> >              int Y = buf0[i] << 2;
> >
> 
> buf0[i] is an int16_t. Is it certain (in the sense of: checked, not in the
> sense of "negative luminance is nonsense") that it can't be negative?
> LGTM apart from that.

ill change the luma cases too and will apply

Thanks


[...]
diff mbox series

Patch

diff --git a/libswscale/output.c b/libswscale/output.c
index d192ea854b..439de04a05 100644
--- a/libswscale/output.c
+++ b/libswscale/output.c
@@ -2111,8 +2111,8 @@  yuv2rgb_full_1_c_template(SwsContext *c, const int16_t *buf0,
         int A = 0; //init to silence warning
         for (i = 0; i < dstW; i++) {
             int Y = buf0[i] << 2;
-            int U = (ubuf0[i] + ubuf1[i] - (128<<8)) << 1;
-            int V = (vbuf0[i] + vbuf1[i] - (128<<8)) << 1;
+            int U = (ubuf0[i] + ubuf1[i] - (128<<8)) * 2;
+            int V = (vbuf0[i] + vbuf1[i] - (128<<8)) * 2;
 
             if (hasAlpha) {
                 A = (abuf0[i] + 64) >> 7;