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 16EA74B092 for ; Mon, 27 May 2024 19:32:05 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 75BA368D4A6; Mon, 27 May 2024 22:32:04 +0300 (EEST) Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 4EBA168D1C4 for ; Mon, 27 May 2024 22:31:57 +0300 (EEST) Received: by mail.gandi.net (Postfix) with ESMTPSA id 9C39C1C0002 for ; Mon, 27 May 2024 19:31:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=niedermayer.cc; s=gm1; t=1716838316; 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=R8k9Xk3jAj7NGxeX8jpm5uIFSU0rHeSWb6slbuXvh/4=; b=OqmHoBLg1qTvpZiLhMmcanbtYKAYPxiZm9fUrBehWUS8WQi3L1mm11Z4VDFJrMqAYpUeOw hr89+tJQdeADOERQnCjLEZhLDOphkIvrQFGmCL8sv55OFa0kwCtERykSr6nn0r+2OYvxTu zryvh5VKqVn62lEnDAcCDOoGqj4qiElrRX28EeF5tIjWUzw7RGgIXYv2K8Dz12f4kwd3sy WAp3SEOjVsMPMpwzBIzPTVAQq8MzF2GSCs5PFXlFMuO9JITJMmFcua7Y82B2ukMLGh8H1M 2GG9xyzO0EmeN0zB4F1MjY8rWL5pJ/FKzRRin852vam75/T+0chzF4/yPvPaDg== Date: Mon, 27 May 2024 21:31:55 +0200 From: Michael Niedermayer To: FFmpeg development discussions and patches Message-ID: <20240527193155.GP2821752@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: <20240527192055.GO2821752@pb2> 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="===============6925975085316886048==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============6925975085316886048== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="2VmY8g3XFtK+hx8d" Content-Disposition: inline --2VmY8g3XFtK+hx8d Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, May 27, 2024 at 09:20:55PM +0200, 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 w= hat > > > > > 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 fundamentally = 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 wo= uld 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 ta= rgets > > 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, thats > still negligible >=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 >=20 > > ffprobe (and f_sidedata) are what should be used for actual integrity > > checks. >=20 > ffprobe cannot test ffmpeg, ffmpeg is a seperate excutable >=20 > If you suggest that side data should not be tested in FFmpeg while packet= =2Edata > should be tested. That position seems inconsistant to me >=20 > If you suggest that neither side data nor packet.data should be tested in= FFmpeg > iam confident that there would be a majority disagreeing. >=20 > f_sidedata is not at the output of ffmpeg so even if it could test it, it > does not test the ffmpeg output. > We also dont replace running md5sum and framecrc on ffmpeg output by a bi= tstream > filter. >=20 > Again, there is need to test what comes out of FFmpeg, thats at the muxer= level > thats what framecrcenc does. There is also an additional aspect and that is efficiency or "time taken by all fate tests" framecrcenc already has all the side data, it costs basically 0 time to pri= nt that any ffprobe based check needs to run everything a 2nd time, so it will be s= lower also ffprobe is only good for side data from the demuxer. my patch tests all cases including side data from the encoder or any other source that gets forwarded to the muxer in each testcase. thx [...] --=20 Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Never trust a computer, one day, it may think you are the virus. -- Compn --2VmY8g3XFtK+hx8d Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iF0EABEKAB0WIQSf8hKLFH72cwut8TNhHseHBAsPqwUCZlTfpAAKCRBhHseHBAsP q+fzAJ9+d+Z1xiorJNvgA+R3fZifOTq0swCfbQyr1nraQf3L29ibotHlBJpcaKw= =ua4U -----END PGP SIGNATURE----- --2VmY8g3XFtK+hx8d-- --===============6925975085316886048== 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". --===============6925975085316886048==--