diff mbox series

[FFmpeg-devel,2/3] avcodec/mpegvideo_enc: Limit bitrate tolerance to the representable

Message ID 20210530180255.23554-2-michael@niedermayer.cc
State New
Headers show
Series [FFmpeg-devel,1/3] avcodec/svq1enc: Do not print debug RD value before it has been computed
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Michael Niedermayer May 30, 2021, 6:02 p.m. UTC
Fixes: error: 1.66789e+11 is outside the range of representable values of type 'int'
Fixes: Ticket8201

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/mpegvideo_enc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

James Almer May 30, 2021, 6:20 p.m. UTC | #1
On 5/30/2021 3:02 PM, Michael Niedermayer wrote:
> Fixes: error: 1.66789e+11 is outside the range of representable values of type 'int'
> Fixes: Ticket8201
> 
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>   libavcodec/mpegvideo_enc.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
> index c01488f483..13618394a5 100644
> --- a/libavcodec/mpegvideo_enc.c
> +++ b/libavcodec/mpegvideo_enc.c
> @@ -455,9 +455,13 @@ av_cold int ff_mpv_encode_init(AVCodecContext *avctx)
>       if (!s->fixed_qscale &&
>           avctx->bit_rate * av_q2d(avctx->time_base) >
>               avctx->bit_rate_tolerance) {
> +        double nbt = avctx->bit_rate * av_q2d(avctx->time_base) * 5;
>           av_log(avctx, AV_LOG_WARNING,
>                  "bitrate tolerance %d too small for bitrate %"PRId64", overriding\n", avctx->bit_rate_tolerance, avctx->bit_rate);
> -        avctx->bit_rate_tolerance = 5 * avctx->bit_rate * av_q2d(avctx->time_base);
> +        if (nbt <= INT_MAX) {
> +            avctx->bit_rate_tolerance = nbt;
> +        } else
> +            avctx->bit_rate_tolerance = INT_MAX;

Maybe bit_rate_tolerance can be made an int64_t? We have done that with 
all bitrate fields in AVCodecContext and similar structs.
We're still in the open ABI period, so it can be done right now.

>       }
>   
>       if (avctx->rc_max_rate &&
>
Michael Niedermayer June 2, 2021, 3:48 p.m. UTC | #2
On Sun, May 30, 2021 at 03:20:06PM -0300, James Almer wrote:
> On 5/30/2021 3:02 PM, Michael Niedermayer wrote:
> > Fixes: error: 1.66789e+11 is outside the range of representable values of type 'int'
> > Fixes: Ticket8201
> > 
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >   libavcodec/mpegvideo_enc.c | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
> > index c01488f483..13618394a5 100644
> > --- a/libavcodec/mpegvideo_enc.c
> > +++ b/libavcodec/mpegvideo_enc.c
> > @@ -455,9 +455,13 @@ av_cold int ff_mpv_encode_init(AVCodecContext *avctx)
> >       if (!s->fixed_qscale &&
> >           avctx->bit_rate * av_q2d(avctx->time_base) >
> >               avctx->bit_rate_tolerance) {
> > +        double nbt = avctx->bit_rate * av_q2d(avctx->time_base) * 5;
> >           av_log(avctx, AV_LOG_WARNING,
> >                  "bitrate tolerance %d too small for bitrate %"PRId64", overriding\n", avctx->bit_rate_tolerance, avctx->bit_rate);
> > -        avctx->bit_rate_tolerance = 5 * avctx->bit_rate * av_q2d(avctx->time_base);
> > +        if (nbt <= INT_MAX) {
> > +            avctx->bit_rate_tolerance = nbt;
> > +        } else
> > +            avctx->bit_rate_tolerance = INT_MAX;
> 
> Maybe bit_rate_tolerance can be made an int64_t? We have done that with all
> bitrate fields in AVCodecContext and similar structs.
> We're still in the open ABI period, so it can be done right now.

for master i agree but this patch is for fixing this bug and backporting.
It is strictly speaking undefined behavior, We cannot backport a 64bit API
update.
We can after applying this immedeatly replace it by a 64bit solution
which is more correct.
Sadly i have more of these to fix so i suspect i will be working on more
of these before i would get to changing this to 64bit so if you want it
dont hesitate to push a matching change after this

thx

[...]
diff mbox series

Patch

diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
index c01488f483..13618394a5 100644
--- a/libavcodec/mpegvideo_enc.c
+++ b/libavcodec/mpegvideo_enc.c
@@ -455,9 +455,13 @@  av_cold int ff_mpv_encode_init(AVCodecContext *avctx)
     if (!s->fixed_qscale &&
         avctx->bit_rate * av_q2d(avctx->time_base) >
             avctx->bit_rate_tolerance) {
+        double nbt = avctx->bit_rate * av_q2d(avctx->time_base) * 5;
         av_log(avctx, AV_LOG_WARNING,
                "bitrate tolerance %d too small for bitrate %"PRId64", overriding\n", avctx->bit_rate_tolerance, avctx->bit_rate);
-        avctx->bit_rate_tolerance = 5 * avctx->bit_rate * av_q2d(avctx->time_base);
+        if (nbt <= INT_MAX) {
+            avctx->bit_rate_tolerance = nbt;
+        } else
+            avctx->bit_rate_tolerance = INT_MAX;
     }
 
     if (avctx->rc_max_rate &&