From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id B5F4B47BBB for ; Tue, 3 Oct 2023 18:53:44 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 4FB0F68CB08; Tue, 3 Oct 2023 21:53:42 +0300 (EEST) Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 4B86C68CA73 for ; Tue, 3 Oct 2023 21:53:35 +0300 (EEST) Received: by mail.gandi.net (Postfix) with ESMTPSA id 8AD9960003 for ; Tue, 3 Oct 2023 18:53:34 +0000 (UTC) Date: Tue, 3 Oct 2023 20:53:33 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20231003185333.GG3543730@pb2> References: <20230929232001.23197-1-michael@niedermayer.cc> <47b96a8-db6b-aa20-d48-a6d60f0e84e@passwd.hu> <20230930140403.GK3543730@pb2> <20230930143143.GL3543730@pb2> <169610511838.6638.13529422310033823704@lain.khirnov.net> <20230930222856.GN3543730@pb2> <169623766726.6638.12142285893799599279@lain.khirnov.net> <20231002190333.GV3543730@pb2> MIME-Version: 1.0 In-Reply-To: X-GND-Sasl: michael@niedermayer.cc Subject: Re: [FFmpeg-devel] [PATCH 1/6] avformat/avidec: support huge durations X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: multipart/mixed; boundary="===============3977382307576284722==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============3977382307576284722== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="0EphSjNCSB1asamE" Content-Disposition: inline --0EphSjNCSB1asamE Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Oct 03, 2023 at 11:10:20AM +0200, Tomas H=E4rdin wrote: > m=E5n 2023-10-02 klockan 21:03 +0200 skrev Michael Niedermayer: > > On Mon, Oct 02, 2023 at 11:07:47AM +0200, Anton Khirnov wrote: > > > Quoting Michael Niedermayer (2023-10-01 00:28:56) > > > > On Sat, Sep 30, 2023 at 10:18:38PM +0200, Anton Khirnov wrote: > > > > > 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: > > > > > > > >=20 > > > > > > > >=20 > > > > > > > > On Sat, 30 Sep 2023, Michael Niedermayer wrote: > > > > > > > >=20 > > > > > > > > > Fixes: signed integer overflow: 109817402400 * > > > > > > > > > 301990077 cannot be represented in type 'long long' > > > > > > > > > Fixes: 51896/clusterfuzz-testcase-minimized- > > > > > > > > > ffmpeg_dem_AVI_fuzzer-6706191715139584 > > > > > > > > >=20 > > > > > > > > > Found-by: continuous fuzzing process > > > > > > > > > https://github.com/google/oss-fuzz/tree/master/projects/f= fmpeg > > > > > > > > > Signed-off-by: Michael Niedermayer > > > > > > > > > > > > > > > > > > --- > > > > > > > > > libavformat/avidec.c | 12 ++++++++++-- > > > > > > > > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > > > > > >=20 > > > > > > > > > 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) > > > > > > > > > =A0=A0=A0=A0=A0=A0=A0 AVStream *st =3D s->streams[i]; > > > > > > > > > =A0=A0=A0=A0=A0=A0=A0 FFStream *const sti =3D ffstream(st= ); > > > > > > > > > =A0=A0=A0=A0=A0=A0=A0 int64_t duration; > > > > > > > > > -=A0=A0=A0=A0=A0=A0=A0 int64_t bitrate; > > > > > > > > > +=A0=A0=A0=A0=A0=A0=A0 int64_t bitrate =3D 0; > > > > > > > > >=20 > > > > > > > > > =A0=A0=A0=A0=A0=A0=A0 for (j =3D 0; j < sti->nb_index_ent= ries; j++) > > > > > > > > > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 len +=3D sti->index_ent= ries[j].size; > > > > > > > > > @@ -484,7 +485,14 @@ static int > > > > > > > > > calculate_bitrate(AVFormatContext *s) > > > > > > > > > =A0=A0=A0=A0=A0=A0=A0 if (sti->nb_index_entries < 2 || st= ->codecpar- > > > > > > > > > >bit_rate > 0) > > > > > > > > > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 continue; > > > > > > > > > =A0=A0=A0=A0=A0=A0=A0 duration =3D sti->index_entries[j-1= ].timestamp - > > > > > > > > > sti->index_entries[0].timestamp; > > > > > > > > > -=A0=A0=A0=A0=A0=A0=A0 bitrate =3D av_rescale(8*len, st->= time_base.den, > > > > > > > > > duration * st->time_base.num); > > > > > > > > > +=A0=A0=A0=A0=A0=A0=A0 if (INT64_MAX / duration >=3D st->= time_base.num) > > > > > > > > > { > > > > > > > > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 bitrate =3D av_rescale= (8*len, st- > > > > > > > > > >time_base.den, duration * st->time_base.num); > > > > > > > >=20 > > > > > > > > Why not always use the AVInteger version? This is not > > > > > > > > performance sensitive > > > > > > > > as far as I see. > > > > > > >=20 > > > > > > > We can, i will have to fix the rounding though so it > > > > > > > matches av_rescale() > > > > > >=20 > > > > > > will apply this with just AVInteger and fixed rounding with > > > > > > my next push probably > > > > >=20 > > > > > This seems MUCH less readable to me. > > > >=20 > > > > we can add a av_rescale_2den() > > >=20 > > > Won't av_rescale_q(len, (AVRational){8, duration}, st->time_base) > > > achieve the same effect? > >=20 > > duration is 64bit AVRational is 32/32 bit, so i would expect that to > > not > > work. > > If duration was fitting in 32bit then duration * st->time_base.num > > would not > > have overflowed >=20 > Maybe we need a 64/64-bit version of AVRational.. Iam not convinced at this point that we need that. But its not hard to do, our AVInteger code will do any size integers by just changing a single number. The real annoyance is if you have 32/32 and 64/64 rationals then suddenly you need many functions to handle both. Sofar only a fuzzed file exceeded our 64 * 64 / 64 =3D=3D 64 * 32/32 * 32/32 rescale in one place adding a 64 * 64 / (64 * 64) rescale would fix that one which seems more limited in spreading complexity through the codebase thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many things microsoft did are stupid, but not doing something just because microsoft did it is even more stupid. If everything ms did were stupid they would be bankrupt already. --0EphSjNCSB1asamE Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZRxjKQAKCRBhHseHBAsP q/0OAJ4p03hI9CvL5JQdy+Hx8qMxOlBsXwCfaeQ0W3ZXvvi+6eA0+VRr3bA1F10= =iTHk -----END PGP SIGNATURE----- --0EphSjNCSB1asamE-- --===============3977382307576284722== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ 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". --===============3977382307576284722==--