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 5812F4B09D for ; Mon, 27 May 2024 19:52:37 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id B665C68D3CA; Mon, 27 May 2024 22:52:35 +0300 (EEST) Received: from mail-pl1-f173.google.com (mail-pl1-f173.google.com [209.85.214.173]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 63DD168D01E for ; Mon, 27 May 2024 22:52:29 +0300 (EEST) Received: by mail-pl1-f173.google.com with SMTP id d9443c01a7336-1f4a0050b9aso1086265ad.2 for ; Mon, 27 May 2024 12:52:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1716839546; x=1717444346; darn=ffmpeg.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id:from :to:cc:subject:date:message-id:reply-to; bh=1mD7b8kDNsJHlc2YhJWhiOnoLrE46lGmkrtuqvprKbY=; b=k+YeGfCBumOO7y2NF97Kw9YM6u83uOT0E87bHVi3G1UFWDBzvAlcrAfCZvpHtrzA/8 wAqb4fy8ekHIKvYIvBfykB+Y3HuLmWXRfswUV0hdzXydLY8w7YtBtAwmOrQ/DXd0KsWk 7zHoJks/yDlhfsP1fW7i1RR79GdqoHWuVviLkoPppFc6DQD002E8i+iLj303G7uTTV1i 8wlm86TPgd6onA8KyWO9P9xtrvXYQ3P+7Rxyd8q0jLHbJFQnwDhnbENYW95V8X73+Ffr WIyfYIQY1fdrAECn9Toey4hcvReewJzWjZuTN++Z+sDJh3T5cxmxflAPmxeQi8MyRQCG fwZg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716839546; x=1717444346; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=1mD7b8kDNsJHlc2YhJWhiOnoLrE46lGmkrtuqvprKbY=; b=J5a3fo5tGUUKt25mWXptIRFxoheUepYqR2nqT8+ZOqOvWh24CuSCzDCKTl8kDOntla jVDG6sQVo8hV3CwJCb1ycoYk87tfvMcj/duINV0W0GIAOfDTw/o4VUlPUw6bam//Zmle 0MEHWHtVuJbzIr4bb4Z6VUvmNFRXsfVJ2iOPSvIOcgqMu/Tg5IDZ/kqPv8QlQuNSrKCF pvmbv0x7HTGWwOdiGGjHJaF7k5yjWvnz5eFaIhJPF2T3BR4r19nbhFirxviccQ18TiIu erNhlV7RaluASHvK9jhIUpB2ZU9uXoI08rB/pQfU3Z1lN7QPwsZ/WMusoYhktmTMrO1J /Jlg== X-Gm-Message-State: AOJu0YxaRFiiP5gcJsBzDGgx65IbxC/w3UWhmnHC0SBurn7888hbzLC7 HUhA+f7S0Jrv8UfFFjv0+ARDLyLWRjrHD2aYzoeOcVjqUQTxGF529fo/UQ== X-Google-Smtp-Source: AGHT+IH/6FG3MRCOkB7tSBPip4HwO19mW1l6MqkJJ7KxcyeVPB7PySbVfnA5fv+A8IhQ0BpCOS16tg== X-Received: by 2002:a17:902:da8d:b0:1f3:35d9:432b with SMTP id d9443c01a7336-1f4486bd0f3mr103700865ad.10.1716839546430; Mon, 27 May 2024 12:52:26 -0700 (PDT) Received: from [192.168.0.10] ([190.194.167.233]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1f44c96f26bsm66904135ad.127.2024.05.27.12.52.25 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 27 May 2024 12:52:26 -0700 (PDT) Message-ID: Date: Mon, 27 May 2024 16:52:22 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: ffmpeg-devel@ffmpeg.org 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> <20240527193155.GP2821752@pb2> <20240527195034.GR2821752@pb2> Content-Language: en-US From: James Almer In-Reply-To: <20240527195034.GR2821752@pb2> 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 5/27/2024 4:50 PM, Michael Niedermayer wrote: > On Mon, May 27, 2024 at 04:33:21PM -0300, James Almer wrote: >> On 5/27/2024 4:31 PM, Michael Niedermayer wrote: >>> 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 what >>>>>>>> framecrc does for before already for packet data itself. >>>>>>>> >>>>>>>> Signed-off-by: Michael Niedermayer >>>>>>>> --- >>>>>>> >>>>>>> I am against this patch. Checksumming side data is a fundamentally wrong >>>>>>> thing to do. >>>>>> >>>>>> 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) >>>>>> >>>>>> 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) >>>>> >>>>> You have seen how much code is needed to get hashing to work for all targets >>>>> with some types, >>>> >>>> framecrcenc.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- >>>> 1 file changed, 73 insertions(+), 3 deletions(-) >>>> >>>> 70 more lines of code, in my patch >>>> >>>> If we need another 70 to handle some corner cases, no idea if we do, thats >>>> still negligible >>>> >>>> >>>>> so it does feel like it's not the right thing to do. >>>> >>>> I dont think i can follow that logic >>>> >>>> >>>>> ffprobe (and f_sidedata) are what should be used for actual integrity >>>>> checks. >>>> >>>> ffprobe cannot test ffmpeg, ffmpeg is a seperate excutable >>>> >>>> If you suggest that side data should not be tested in FFmpeg while packet.data >>>> should be tested. That position seems inconsistant to me >>>> >>>> If you suggest that neither side data nor packet.data should be tested in FFmpeg >>>> iam confident that there would be a majority disagreeing. >>>> >>>> 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 bitstream >>>> filter. >>>> >>>> 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 print that >>> >>> any ffprobe based check needs to run everything a 2nd time, so it will be slower >>> >>> 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. >> >> We could extend showinfo_bsf to print side data information. > > Well, you argued a moment ago that its too much code (in framecrcenc) > its not going to be less code if the same or more detailed information > is printed in a showinfo_bsf > > again, my suggestion is that this code should go to where side data is > and then showinfo_bsf, framecrcenc and ffprobe can use it I mean, showinfo_bsf could be adapted in a way ffprobe can invoke/parse, so all the related ffprobe code can be moved there. > > thx > > [...] > > > _______________________________________________ > 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". _______________________________________________ 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".