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 41434422B9 for ; Mon, 29 Aug 2022 22:49:01 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 1025F68BADB; Tue, 30 Aug 2022 01:48:59 +0300 (EEST) Received: from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net [217.70.183.198]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 06BD568BA89 for ; Tue, 30 Aug 2022 01:48:52 +0300 (EEST) Received: (Authenticated sender: michael@niedermayer.cc) by mail.gandi.net (Postfix) with ESMTPSA id 247E7C0002 for ; Mon, 29 Aug 2022 22:48:51 +0000 (UTC) Date: Tue, 30 Aug 2022 00:48:51 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20220829224851.GL2088045@pb2> References: <20220828172545.8185-1-michael@niedermayer.cc> MIME-Version: 1.0 In-Reply-To: Subject: Re: [FFmpeg-devel] [PATCH] avcodec/vpx_rac: Adjust vpx_rac_is_end) threshold 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="===============7767085098950810571==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============7767085098950810571== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="CMSRreXTSU7rpCqF" Content-Disposition: inline --CMSRreXTSU7rpCqF Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Aug 29, 2022 at 05:29:03PM +0200, Andreas Rheinhardt wrote: > Michael Niedermayer: > > A threshold of 180 is needed and sufficient for the sample, twice this = is used to > > cover potentially worse samples > >=20 > > fate/vp5 changes as the sample file is truncated and the damaged part i= s handled > > differently > >=20 > > Fixes: ticket #9754 > >=20 > > Signed-off-by: Michael Niedermayer > > --- > > libavcodec/vpx_rac.h | 2 +- > > tests/ref/fate/vp5 | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > >=20 > > diff --git a/libavcodec/vpx_rac.h b/libavcodec/vpx_rac.h > > index b158cc0754..2f5486f501 100644 > > --- a/libavcodec/vpx_rac.h > > +++ b/libavcodec/vpx_rac.h > > @@ -52,7 +52,7 @@ static av_always_inline int vpx_rac_is_end(VPXRangeCo= der *c) > > { > > if (c->end <=3D c->buffer && c->bits >=3D 0) > > c->end_reached ++; > > - return c->end_reached > 10; > > + return c->end_reached > 360; >=20 > Is the file from #9754 defective? Or is it our decoder that is > overcautious?=20 > Your commit message sounds as if it were the latter. Is it > guaranteed that is now enough for all spec-compliant samples? Does the > answer depend upon the codec? (vpx_rac_is_end() is shared between > several VPx codecs.); I presume you can't give any guarantees. The file in question is VP8 looking at rfc6386, i see no obvious end of data check in the AC decoder in the RFC that would hint toward defective but the renorm code only tries reading once over the end. Our large threshold is the result of avoiding adding code in the ac reader itself It would require more investigation to be really sure though. Looking at this with the reference decoder and some instrumentation in it to see if it overreads too for example.=20 But as the awnser to this wouldnt really affect the solution i think thats wasted time. Also heres a better looking fix: sadly i cannot replicate most of the timeouts with git master also i do not remember why i did not choose this simpler solution originally Author: Michael Niedermayer Date: Mon Aug 29 23:43:41 2022 +0200 libavcodec/vpx_rac: Change end detection logic =20 fate/vp5 changes as the sample file is truncated and the damaged part i= s handled differently =20 Fixes: ticket #9754 =20 This is a different and simpler solution to b6b9ac5698c8f911841b469af77= 199153278c55c =20 Signed-off-by: Michael Niedermayer diff --git a/libavcodec/vpx_rac.c b/libavcodec/vpx_rac.c index cf02e9a19c9..bbfc829431c 100644 --- a/libavcodec/vpx_rac.c +++ b/libavcodec/vpx_rac.c @@ -45,7 +45,6 @@ int ff_vpx_init_range_decoder(VPXRangeCoder *c, const uin= t8_t *buf, int buf_size c->bits =3D -16; c->buffer =3D buf; c->end =3D buf + buf_size; - c->end_reached =3D 0; if (buf_size < 1) return AVERROR_INVALIDDATA; c->code_word =3D bytestream_get_be24(&c->buffer); diff --git a/libavcodec/vpx_rac.h b/libavcodec/vpx_rac.h index b158cc0754d..392e59904bd 100644 --- a/libavcodec/vpx_rac.h +++ b/libavcodec/vpx_rac.h @@ -39,7 +39,6 @@ typedef struct VPXRangeCoder { const uint8_t *buffer; const uint8_t *end; unsigned int code_word; - int end_reached; } VPXRangeCoder; =20 extern const uint8_t ff_vpx_norm_shift[256]; @@ -50,9 +49,7 @@ int ff_vpx_init_range_decoder(VPXRangeCoder *c, const uin= t8_t *buf, int buf_size */ static av_always_inline int vpx_rac_is_end(VPXRangeCoder *c) { - if (c->end <=3D c->buffer && c->bits >=3D 0) - c->end_reached ++; - return c->end_reached > 10; + return c->end <=3D c->buffer && c->bits >=3D 1; } =20 static av_always_inline unsigned int vpx_rac_renorm(VPXRangeCoder *c) diff --git a/tests/ref/fate/vp5 b/tests/ref/fate/vp5 index 09ebe62b25e..2469a3ec21a 100644 --- a/tests/ref/fate/vp5 +++ b/tests/ref/fate/vp5 @@ -249,4 +249,4 @@ 0, 243, 243, 1, 233472, 0x6f530ac6 0, 244, 244, 1, 233472, 0x94f7466c 0, 245, 245, 1, 233472, 0xa8c1d365 -0, 246, 246, 1, 233472, 0x4f3ef38c +0, 246, 246, 1, 233472, 0xbf73f1b7 --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Observe your enemies, for they first find out your faults. -- Antisthenes --CMSRreXTSU7rpCqF Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCYw1CTgAKCRBhHseHBAsP q0KYAJsG2sDovfL+HcHCnfbknnvSZO+E+QCfa4P00f6cMIQbdjC7tjBXwY4TZLo= =xJ4c -----END PGP SIGNATURE----- --CMSRreXTSU7rpCqF-- --===============7767085098950810571== 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". --===============7767085098950810571==--