Re: [FFmpeg-devel] [PATCH] lavc: avoid rounding errors in float constants - Rémi Denis-Courmont

From: "Rémi Denis-Courmont" <remi@remlab.net>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH] lavc: avoid rounding errors in float constants
Date: Tue, 13 Sep 2022 18:39:38 +0300
Message-ID: <5600795.DvuYhMxLoT@basile.remlab.net> (raw)
In-Reply-To: <DU0P250MB0747C505A9B1114352BCA61D8F479@DU0P250MB0747.EURP250.PROD.OUTLOOK.COM>

Le tiistaina 13. syyskuuta 2022, 18.11.35 EEST Andreas Rheinhardt a écrit :
> remi@remlab.net:
> > From: Rémi Denis-Courmont <remi@remlab.net>
> > 
> > INT_MAX is (typically) a value with 31 significant bits but float can
> > only represent 23 significant bits, leading to a rounding error.
> > 
> > This substitutes the actual rounded value to avoid a clang warning:
> >  warning: implicit conversion from 'int' to 'float' changes value from
> >  
> >   2147483647 to 2147483648 [-Wimplicit-const-int-float-conversion]
> > 
> > ---
> > 
> >  libavcodec/aaccoder.c | 2 +-
> >  libavcodec/imc.c      | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavcodec/aaccoder.c b/libavcodec/aaccoder.c
> > index e3b6b2f02c..877558c91c 100644
> > --- a/libavcodec/aaccoder.c
> > +++ b/libavcodec/aaccoder.c
> > @@ -531,7 +531,7 @@ static void search_for_quantizers_anmr(AVCodecContext
> > *avctx, AACEncContext *s,> 
> >              int nz = 0;
> >              
> >              bandaddr[idx] = w * 16 + g;
> > 
> > -            qmin = INT_MAX;
> > +            qmin = -INT_MIN;
> > 
> >              qmax = 0.0f;
> >              for (w2 = 0; w2 < sce->ics.group_len[w]; w2++) {
> >              
> >                  FFPsyBand *band =
> >                  &s->psy.ch[s->cur_channel].psy_bands[(w+w2)*16+g];
> > 
> > diff --git a/libavcodec/imc.c b/libavcodec/imc.c
> > index 92f9980ded..d4dfe3222c 100644
> > --- a/libavcodec/imc.c
> > +++ b/libavcodec/imc.c
> > @@ -917,7 +917,7 @@ static int imc_decode_block(AVCodecContext *avctx,
> > IMCContext *q, int ch)> 
> >                                         chctx->flcoeffs1,
> >                                         chctx->flcoeffs2);
> >      
> >      for(i=0; i<BANDS; i++) {
> > 
> > -        if(chctx->flcoeffs1[i] > INT_MAX) {
> > +        if(chctx->flcoeffs1[i] > -INT_MIN) {
> > 
> >              av_log(avctx, AV_LOG_ERROR, "scalefactor out of range\n");
> >              return AVERROR_INVALIDDATA;
> >          
> >          }
> 
> -INT_MIN can't be represented in an int

Sure, but that's irrelevant.

> and therefore -INT_MIN on the right is UB by C11 6.5 (5).

Of course not. The type of an integer constant is always large enough to fit 
the value. In this case, it will either be long int or long long int, the 
later being large enough on any platform.

See C11 §6.4.4.1.

-- 
Rémi Denis-Courmont
http://www.remlab.net/

_______________________________________________
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".