On 5/18/2025 6:23 PM, Michael Niedermayer wrote:
You're seeing this the wrong way. You want this feature, and every day it's not upstreamed, it's potential extra work in your local repo. That's on you, same as other people probably have their own local repos or public forks they need to be kept in sync with extra personal changes that are not upstreamed. The way this should be handled is: Does the project need it, or do developers want it? Because the extra work i talked about earlier is potentially adding a case to the switch for every new side data type added from now on, that you're putting on other developers' shoulders.On Sun, May 18, 2025 at 11:43:14AM -0300, James Almer wrote:On 5/18/2025 9:25 AM, Michael Niedermayer wrote:This allows detecting changes and regressions 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> --- libavformat/framecrcenc.c | 116 +- tests/ref/fate/autorotate | 2 +- tests/ref/fate/cover-art-mp3-id3v2-remux | 2 +- tests/ref/fate/ffmpeg-bsf-input | 10 +- tests/ref/fate/ffmpeg-spec-disposition | 2 +- tests/ref/fate/force_key_frames-source | 784 ++++++------ tests/ref/fate/force_key_frames-source-drop | 34 +- tests/ref/fate/force_key_frames-source-dup | 1224 +++++++++---------- tests/ref/fate/gapless-mp3 | 6 +- tests/ref/fate/h264_redundant_pps-side_data | 2 +- tests/ref/fate/id3v2-priv-remux | 2 +- tests/ref/fate/matroska-hdr10-plus-remux | 2 +- tests/ref/fate/matroska-ogg-opus-remux | 2 +- tests/ref/fate/matroska-opus-remux | 2 +- tests/ref/fate/matroska-vp8-alpha-remux | 14 +- tests/ref/fate/mov-cover-image | 2 +- tests/ref/fate/segment-mp4-to-ts | 250 ++-- tests/ref/fate/shortest | 100 +- tests/ref/fate/webm-hdr10-plus-remux | 2 +- tests/ref/fate/webm-webvtt-remux | 24 +- 20 files changed, 1346 insertions(+), 1236 deletions(-) diff --git a/libavformat/framecrcenc.c b/libavformat/framecrcenc.c index 2ba20f3aab4..96c2a82eb2a 100644 --- a/libavformat/framecrcenc.c +++ b/libavformat/framecrcenc.c @@ -21,8 +21,11 @@ #include <inttypes.h> +#include "config.h" #include "libavutil/adler32.h" #include "libavutil/avstring.h" +#include "libavutil/intreadwrite.h" +#include "libavutil/hdr_dynamic_metadata.h" #include "libavcodec/codec_id.h" #include "libavcodec/codec_par.h" @@ -48,6 +51,20 @@ static int framecrc_write_header(struct AVFormatContext *s) return ff_framehash_write_header(s); } +static av_unused void inline bswap(char *buf, int offset, int size) +{ + if (size == 8) { + uint64_t val = AV_RN64(buf + offset); + AV_WN64(buf + offset, av_bswap64(val)); + } else if (size == 4) { + uint32_t val = AV_RN32(buf + offset); + AV_WN32(buf + offset, av_bswap32(val)); + } else if (size == 2) { + uint16_t val = AV_RN16(buf + offset); + AV_WN16(buf + offset, av_bswap16(val)); + } +} + static int framecrc_write_packet(struct AVFormatContext *s, AVPacket *pkt) { uint32_t crc = av_adler32_update(0, pkt->data, pkt->size); @@ -58,11 +75,104 @@ static int framecrc_write_packet(struct AVFormatContext *s, AVPacket *pkt) if (pkt->flags != AV_PKT_FLAG_KEY) av_strlcatf(buf, sizeof(buf), ", F=0x%0X", pkt->flags); if (pkt->side_data_elems) { + int i; av_strlcatf(buf, sizeof(buf), ", S=%d", pkt->side_data_elems); - for (int i = 0; i < pkt->side_data_elems; i++) { - av_strlcatf(buf, sizeof(buf), ", %8"SIZE_SPECIFIER, - pkt->side_data[i].size); + for (i=0; i<pkt->side_data_elems; i++) { + const AVPacketSideData *const sd = &pkt->side_data[i]; + const uint8_t *data = sd->data; + uint32_t side_data_crc = 0; + + switch (sd->type) {Wont this potentially introduce extra work for when we add new types?no, it will be less work because maintaining this in a seperate branch is the identical work but in addition its also the extra work due to the seperate branch. and it adds complexity if one wants to bisect. Every hour i have to spend maintaining this or anything else in a seperate branch is an hour less i spend on the ffmpeg core repository.
That said, I'm not against this in principle, but again, it will mean extra work when adding future side data types that encapsulate long elaborate structs.
OpenPGP_signature.asc
Description: OpenPGP digital signature
_______________________________________________ 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".
