| Message ID | 20220913145103.3595-1-remi@remlab.net |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] lavc: avoid rounding errors in float constants | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_fate_loongarch64 | fail | Make fate failed |
| andriy/make_fate_x86 | fail | Make fate failed |
| andriy/make_x86 | warning | New warnings during build |
| yinshiyou/make_loongarch64 | warning | New warnings during build |
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 and therefore -INT_MIN on the right is UB by C11 6.5 (5). - Andreas
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: > 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. > Incorrect. -INT_MIN is the unary minus operator applied to INT_MIN. Regardless of how INT_MIN is defined, it is not an integer constant, because they just don't start with '-'. See the syntax in 6.4.4.1. Anyway, my limits.h here defines INT_MIN as follows: "#define INT_MIN (-__INT_MAX__ -1)". As you can see, this is definitely not even close to an integer constant as defined in 6.4.4.1. - Andreas
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; }
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(-)