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.

Signed-off-by: Michael Niedermayer <mich...@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

IAMF and video_enc_params. And potential future types. It's not negligible.



so it does feel like it's not the right thing to do.

I dont think i can follow that logic

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?



ffprobe (and f_sidedata) are what should be used for actual integrity
checks.

ffprobe cannot test ffmpeg, ffmpeg is a seperate excutable

I didn't say ffprobe should be used to test ffmpeg, i said ffprobe should be used to test side data integrity.


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.

I meant {a,f}_showinfo, sorry. And it does not apply to packet side data, obviously.

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.

Packet side data does not come out of ffmpeg, it comes out of libavformat (or libavcodec/libavfilter in the case of frame side data). A user of lavf can look at side data instead of needing a lavf muxer to print a hash that needs ad hoc code for it to match across targets.


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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to