On Wed, Jan 22, 2020 at 06:53:13PM +0100, Andreas Rheinhardt wrote:
> On Wed, Jan 22, 2020 at 6:44 PM Michael Niedermayer <mich...@niedermayer.cc>
> wrote:
>
> > On Wed, Jan 22, 2020 at 03:42:25PM +0100, Andreas Rheinhardt wrote:
> > > On Tue, Jan 21, 2020 at 11:18 PM Michael Niedermayer
> > <mich...@niedermayer.cc>
> > > wrote:
> > >
> > > > Fixes: Invalid shifts
> > > > Fixes: #8140
> > > > Fixes: #8146
> > > >
> > > > Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
> > > > ---
> > > > libswscale/input.c | 8 ++++----
> > > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/libswscale/input.c b/libswscale/input.c
> > > > index 064f8da314..159f70307d 100644
> > > > --- a/libswscale/input.c
> > > > +++ b/libswscale/input.c
> > > > @@ -286,8 +286,8 @@ static av_always_inline void
> > > > rgb16_32ToUV_c_template(int16_t *dstU,
> > > > int gsh, int bsh,
> > > > int S,
> > > > int32_t *rgb2yuv)
> > > > {
> > > > - const int ru = rgb2yuv[RU_IDX] << rsh, gu = rgb2yuv[GU_IDX]
> > <<
> > > > gsh, bu = rgb2yuv[BU_IDX] << bsh,
> > > > - rv = rgb2yuv[RV_IDX] << rsh, gv = rgb2yuv[GV_IDX]
> > <<
> > > > gsh, bv = rgb2yuv[BV_IDX] << bsh;
> > > > + const int ru = rgb2yuv[RU_IDX] * (1 << rsh), gu =
> > > > rgb2yuv[GU_IDX] * (1 << gsh), bu = rgb2yuv[BU_IDX] * (1 << bsh),
> > > > + rv = rgb2yuv[RV_IDX] * (1 << rsh), gv =
> > > > rgb2yuv[GV_IDX] * (1 << gsh), bv = rgb2yuv[BV_IDX] * (1 << bsh);
> > > > const unsigned rnd = (256u<<((S)-1)) + (1<<(S-7));
> > > > int i;
> > > >
> > > > @@ -314,8 +314,8 @@ static av_always_inline void
> > > > rgb16_32ToUV_half_c_template(int16_t *dstU,
> > > > int gsh, int
> > > > bsh, int S,
> > > > int32_t
> > > > *rgb2yuv)
> > > > {
> > > > - const int ru = rgb2yuv[RU_IDX] << rsh, gu = rgb2yuv[GU_IDX]
> > <<
> > > > gsh, bu = rgb2yuv[BU_IDX] << bsh,
> > > > - rv = rgb2yuv[RV_IDX] << rsh, gv = rgb2yuv[GV_IDX]
> > <<
> > > > gsh, bv = rgb2yuv[BV_IDX] << bsh,
> > > > + const int ru = rgb2yuv[RU_IDX] * (1 << rsh), gu =
> > > > rgb2yuv[GU_IDX] * (1 << gsh), bu = rgb2yuv[BU_IDX] * (1 << bsh),
> > > > + rv = rgb2yuv[RV_IDX] * (1 << rsh), gv =
> > > > rgb2yuv[GV_IDX] * (1 << gsh), bv = rgb2yuv[BV_IDX] * (1 << bsh),
> > > > maskgx = ~(maskr | maskb);
> > > > const unsigned rnd = (256U<<(S)) + (1<<(S-6));
> > > > int i;
> > > > --
> > >
> > > When I looked into this last year, I was unable to ascertain that the
> > > multiplication wouldn't overflow (in which case one would need to go the
> > > unsigned route, most easily by making the rgb2yuv parameter a uint32_t
> > *).
> > > Can you?
> >
> > If there is a situation where these overflow then this needs to be looked
> > at with a testcase. Simply using unsigned would likely not produce a
> > outcome the user wants
> >
> > The outcome would not change at all (presuming the ordinary implementation
> specified conversion unsigned->int): rgb2yuv is only used to initialize
> these const ints that you see in the diff and using unsigned would just
> mean that the conversion needs to wrap around.
> > But I agree that there is no reason to use unsigned for the calculation if > there is no testcase showing an overflow. So proceed as you wish. ok thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If you think the mosad wants you dead since a long time then you are either wrong or dead since a long time.
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
