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 90E9A4B0A1 for ; Mon, 27 May 2024 19:43:48 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 3313968D4C1; Mon, 27 May 2024 22:43:46 +0300 (EEST) Received: from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net [217.70.183.199]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id B287668C704 for ; Mon, 27 May 2024 22:43:39 +0300 (EEST) Received: by mail.gandi.net (Postfix) with ESMTPSA id D5986FF803 for ; Mon, 27 May 2024 19:43:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc; s=gm1; t=1716839019; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=utqqzDZ5hteF8tR0RbZZpaBohjA/yZh5MwPLYEJJGas=; b=GwqDGSd61IMyZka+JTVz0aJWIDuGsQSD+ZfA7KU8ZY41T5Fnqc4L26MXjC/ukYVvGaTmel hcJXpqJ9ozKdMBCEj3hnXgBYpRnrkwHtPjK9WVg/NScTsSY4+FLN6waNuem84F0ivI/SQA +EjTAD/LJ2J14rNClHF4AgRyf+UyF1SBH79wV4I2Idpd7/Am7oRBJMr2XdWI5BDkyn08KM CLek/VqNK9v4y/aSz+sAQARHzTS5Nr/HE8M/1mYPTQ4z3ZX+OxC0P3iemMVnV8Tt7deT7K M2XxmLpiN8s+kZAc9OQVDTTo7CFVB78krccKDCUs2wDTQ5HugkZqxq0UF4mclg== Date: Mon, 27 May 2024 21:43:37 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20240527194337.GQ2821752@pb2> References: <20240427003623.118199-1-michael@niedermayer.cc> <171679774316.28895.4366606367617414113@lain.khirnov.net> <20240527181105.GN2821752@pb2> <703eafba-1d9a-458b-a4ee-f2092a73bd83@gmail.com> <20240527192055.GO2821752@pb2> MIME-Version: 1.0 In-Reply-To: X-GND-Sasl: michael@niedermayer.cc Subject: Re: [FFmpeg-devel] [PATCH] avformat/framecrcenc: compute the checksum for side data 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="===============2692008546162530510==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============2692008546162530510== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="UuuTb5hZdWr4w/Ub" Content-Disposition: inline --UuuTb5hZdWr4w/Ub Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, May 27, 2024 at 04:32:43PM -0300, James Almer wrote: > On 5/27/2024 4:20 PM, Michael Niedermayer wrote: > > On Mon, May 27, 2024 at 03:17:15PM -0300, James Almer wrote: > > > On 5/27/2024 3:11 PM, Michael Niedermayer wrote: > > > > On Mon, May 27, 2024 at 10:15:43AM +0200, Anton Khirnov wrote: > > > > > Quoting Michael Niedermayer (2024-04-27 02:36:23) > > > > > > This allows detecting issues in side data related code, same as= what > > > > > > framecrc does for before already for packet data itself. > > > > > >=20 > > > > > > Signed-off-by: Michael Niedermayer > > > > > > --- > > > > >=20 > > > > > I am against this patch. Checksumming side data is a fundamentall= y wrong > > > > > thing to do. > > > >=20 > > > > It, or something equivalent is neccessary for regression testing. > > > > (and it was you who asked also for the tests i run to be part of > > > > fate. But here you object to it) > > > >=20 > > > > You know, not checking side data is not checking it so differences = would then not be > > > > detected allowing for unintended changes to be introduced (aka bugs) > > >=20 > > > You have seen how much code is needed to get hashing to work for all = targets > > > with some types, > >=20 > > framecrcenc.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++--- > > 1 file changed, 73 insertions(+), 3 deletions(-) > >=20 > > 70 more lines of code, in my patch > >=20 > > If we need another 70 to handle some corner cases, no idea if we do, th= ats > > still negligible >=20 > IAMF and video_enc_params. And potential future types. It's not negligibl= e. >=20 > >=20 > >=20 > > > so it does feel like it's not the right thing to do. > >=20 > > I dont think i can follow that logic >=20 > Extra custom code to take into account specific side data types in order = to > output a common hash value. Do we really need to add that when we already > have side data type specific parsing code to in ffprobe? side data specific code doesnt belong in ffprobe what should instead be done is to have side data parsing code where the sid= e data struct is Side data can be represented in at least 4 ways 1. one or more C structs 2. a serialized bytestream 3. json data 4. a human readable key value printout We have some of this in ffprobe, it should be closer to iamf.c/h and then used by framecrcenc (whatever format framecrcenc uses is not important) [...] thx --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I know you won't believe me, but the highest form of Human Excellence is to question oneself and others. -- Socrates --UuuTb5hZdWr4w/Ub Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEKAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZlTiYgAKCRBhHseHBAsP qzhvAJ9CJ8zCf1N36drWsbQrCLKXN9VhqgCeKu/9TAyeRLxSKBkznO/uYO8+0BI= =diTu -----END PGP SIGNATURE----- --UuuTb5hZdWr4w/Ub-- --===============2692008546162530510== 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". --===============2692008546162530510==--