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 832B347E06 for ; Fri, 27 Oct 2023 18:20:25 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8EC5368CB4A; Fri, 27 Oct 2023 21:20:22 +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 4FBB568CAB5 for ; Fri, 27 Oct 2023 21:20:16 +0300 (EEST) Received: by mail.gandi.net (Postfix) with ESMTPSA id AB90360002 for ; Fri, 27 Oct 2023 18:20:15 +0000 (UTC) Date: Fri, 27 Oct 2023 20:20:14 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20231027182014.GV3543730@pb2> References: <20230809174657.76-1-tcChlisop0@gmail.com> <20231026214400.GM3543730@pb2> <43bfe941-5150-4692-bb6f-170b1c0ec833@gmail.com> MIME-Version: 1.0 In-Reply-To: <43bfe941-5150-4692-bb6f-170b1c0ec833@gmail.com> X-GND-Sasl: michael@niedermayer.cc Subject: Re: [FFmpeg-devel] [PATCH v3] avcodec/dovi_rpu: verify RPU data CRC32 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="===============4999073922511377089==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============4999073922511377089== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="R5qH07xgj4wTg3Ty" Content-Disposition: inline --R5qH07xgj4wTg3Ty Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Oct 27, 2023 at 07:46:31AM -0400, quietvoid wrote: >=20 > On 26/10/2023 17.44, Michael Niedermayer wrote: > > On Wed, Aug 09, 2023 at 01:46:57PM -0400, quietvoid wrote: > > > The Dolby Vision RPU contains a CRC32 to validate the payload against. > > > The implementation is CRC32/MPEG-2. > > >=20 [...] > > > - /* FIXME: verify CRC32, requires implementation of AV_CRC_32_MPE= G_2 */ > > > + if (!(err_recognition & AV_EF_CRCCHECK)) > > > + return 0; > > > + > > > + /* Skip unsupported until CRC32 */ > > > + skip_bits_long(gb, get_bits_left(gb) - 32); > > > + > > > + rpu_data_crc32 =3D get_bits_long(gb, 32); > > > + > > > + /* Verify CRC32, buffer excludes the prefix, CRC32 and trailing = byte */ > > > + computed_crc32 =3D av_bswap32(av_crc(av_crc_get_table(AV_CRC_32_= IEEE), > > > + -1, rpu + 1, actual_rpu_size = - 6)); > > > + > > > + if (rpu_data_crc32 !=3D computed_crc32) { > > > + av_log(s->logctx, AV_LOG_ERROR, > > > + "RPU CRC mismatch! Expected %"PRIu32", received %"PRI= u32"\n", > > > + rpu_data_crc32, computed_crc32); > > > + > > > + if (err_recognition & AV_EF_EXPLODE) > > > + goto fail; > > > + } > > (correctly designed) CRCs have the beautifull symmetry that you can mer= ge > > the crc32 value into the crc computation and then a 0 means no CRC miss= match > > (there are many other cool properties but this one allows to simplify t= he code) > >=20 > > This works too: (and is simpler) > >=20 > > /* Skip unsupported until CRC32 */ > > skip_bits_long(gb, get_bits_left(gb)); > >=20 > > /* Verify CRC32, buffer excludes the prefix, CRC32 and trailing by= te */ > > computed_crc32 =3D av_bswap32(av_crc(av_crc_get_table(AV_CRC_32_IE= EE), > > -1, rpu + 1, actual_rpu_size - = 2)); > >=20 > > if (computed_crc32) { > > av_log(s->logctx, AV_LOG_ERROR, "RPU CRC mismatch! %"PRIX32"\n= ", > > computed_crc32); > >=20 > > if (err_recognition & AV_EF_EXPLODE) > > goto fail; > > } >=20 > Hi Michael. I like the idea and it's a cool property. >=20 > However the then printed CRC on mismatch is not a useful value, so I'm > unsure if it's better to simplify here. teh value printed is the syndrome (https://en.wikipedia.org/wiki/Decoding_methods#Syndrome_decoding) Its not a useless value. What is it usefull for? well take 2 pieces of different and random data add a correct CRC at the end of each now both will give a 0 of course on CRC checking now flip the same bits in both blocks (whichever and howmany you want) after that very likely the CRC check will not give 0 but it will give the same value for both blocks the syndrome only depends on the damage not the data, which is kind of cool. > I like having the expected CRC logged here. how do you know that expected CRC ? i mean if theres a missmatch you _know_ there is some damage but you do not know how much or where the damage is, the "expected CRC" stored there can be the damaged and in fact even the only damaged part or maybe everything is damaged. the syndrome can be used for correcting an error if you can narrow the possible errors down enough but the CRC itself iam not so sure. Maybe one could google it in hopes to find a undamaged file but for this to work the CRC would need to be printed on undamaged files and long enough. thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The worst form of inequality is to try to make unequal things equal. -- Aristotle --R5qH07xgj4wTg3Ty Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEIAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZTv/UAAKCRBhHseHBAsP q/PnAJ422tZQ/YDom8ZkIteK0CH2/QONNwCffzwGP4JIdkKA+YrwS4JqQIndOOc= =JLRU -----END PGP SIGNATURE----- --R5qH07xgj4wTg3Ty-- --===============4999073922511377089== 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". --===============4999073922511377089==--