Re: [FFmpeg-devel] [PATCH 1/6] avformat/avidec: support huge durations

[Anton Khirnov <anton@khirnov.net>]

Quoting Michael Niedermayer (2023-09-30 16:31:43)
> On Sat, Sep 30, 2023 at 04:04:03PM +0200, Michael Niedermayer wrote:
> > On Sat, Sep 30, 2023 at 11:35:19AM +0200, Marton Balint wrote:
> > > 
> > > 
> > > On Sat, 30 Sep 2023, Michael Niedermayer wrote:
> > > 
> > > > Fixes: signed integer overflow: 109817402400 * 301990077 cannot be represented in type 'long long'
> > > > Fixes: 51896/clusterfuzz-testcase-minimized-ffmpeg_dem_AVI_fuzzer-6706191715139584
> > > > 
> > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > ---
> > > > libavformat/avidec.c | 12 ++++++++++--
> > > > 1 file changed, 10 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/libavformat/avidec.c b/libavformat/avidec.c
> > > > index 00bd7a98a9d..cfc693842b7 100644
> > > > --- a/libavformat/avidec.c
> > > > +++ b/libavformat/avidec.c
> > > > @@ -27,6 +27,7 @@
> > > > #include "libavutil/avstring.h"
> > > > #include "libavutil/opt.h"
> > > > #include "libavutil/dict.h"
> > > > +#include "libavutil/integer.h"
> > > > #include "libavutil/internal.h"
> > > > #include "libavutil/intreadwrite.h"
> > > > #include "libavutil/mathematics.h"
> > > > @@ -476,7 +477,7 @@ static int calculate_bitrate(AVFormatContext *s)
> > > >         AVStream *st = s->streams[i];
> > > >         FFStream *const sti = ffstream(st);
> > > >         int64_t duration;
> > > > -        int64_t bitrate;
> > > > +        int64_t bitrate = 0;
> > > > 
> > > >         for (j = 0; j < sti->nb_index_entries; j++)
> > > >             len += sti->index_entries[j].size;
> > > > @@ -484,7 +485,14 @@ static int calculate_bitrate(AVFormatContext *s)
> > > >         if (sti->nb_index_entries < 2 || st->codecpar->bit_rate > 0)
> > > >             continue;
> > > >         duration = sti->index_entries[j-1].timestamp - sti->index_entries[0].timestamp;
> > > > -        bitrate = av_rescale(8*len, st->time_base.den, duration * st->time_base.num);
> > > > +        if (INT64_MAX / duration >= st->time_base.num) {
> > > > +            bitrate = av_rescale(8*len, st->time_base.den, duration * st->time_base.num);
> > > 
> > > Why not always use the AVInteger version? This is not performance sensitive
> > > as far as I see.
> > 
> > We can, i will have to fix the rounding though so it matches av_rescale()
> 
> will apply this with just AVInteger and fixed rounding with my next push probably

This seems MUCH less readable to me.

Do we expect such huge durations in actual files rather than fuzzed
ones? Would it not be better to just not export the duration at all when
the computation would overflow?

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