Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH] avformat/framecrcenc: compute the checksum for side data
Date: Mon, 27 May 2024 21:20:55 +0200
Message-ID: <20240527192055.GO2821752@pb2> (raw)
In-Reply-To: <703eafba-1d9a-458b-a4ee-f2092a73bd83@gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 2377 bytes --]

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 <michael@niedermayer.cc>
> > > > ---
> > > 
> > > 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.

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is a danger to trust the dream we wish for rather than
the science we have, -- Dr. Kenneth Brown

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

  reply	other threads:[~2024-05-27 19:21 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-27  0:36 Michael Niedermayer
2024-04-27 10:44 ` Andreas Rheinhardt
2024-04-27 12:07   ` Michael Niedermayer
2024-04-28  3:43     ` James Almer
2024-04-30 23:25       ` Michael Niedermayer
2024-04-30 23:29         ` James Almer
2024-05-01  0:40           ` Michael Niedermayer
2024-05-01  0:45             ` James Almer
2024-05-27  8:15 ` Anton Khirnov
2024-05-27 14:11   ` James Almer
2024-05-31  7:39     ` Anton Khirnov
2024-05-27 18:11   ` Michael Niedermayer
2024-05-27 18:17     ` James Almer
2024-05-27 19:20       ` Michael Niedermayer [this message]
2024-05-27 19:31         ` Michael Niedermayer
2024-05-27 19:33           ` James Almer
2024-05-27 19:50             ` Michael Niedermayer
2024-05-27 19:52               ` James Almer
2024-05-30 19:33                 ` Michael Niedermayer
2024-05-27 19:32         ` James Almer
2024-05-27 19:43           ` Michael Niedermayer
2024-05-31  7:32     ` Anton Khirnov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240527192055.GO2821752@pb2 \
    --to=michael@niedermayer.cc \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git