[FFmpeg-devel] [PATCH] lavc: avoid rounding errors in float constants

[FFmpeg-devel] [PATCH] lavc: avoid rounding errors in float constants

[remi]

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;
         }
-- 
2.37.2

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

Re: [FFmpeg-devel] [PATCH] lavc: avoid rounding errors in float constants

[Andreas Rheinhardt]

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

Re: [FFmpeg-devel] [PATCH] lavc: avoid rounding errors in float constants

[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.

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

Re: [FFmpeg-devel] [PATCH] lavc: avoid rounding errors in float constants

[Andreas Rheinhardt]

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