From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH] avformat/framecrcenc: List types and checksums for for side data
Date: Sun, 18 May 2025 16:43:01 +0200
Message-ID: <AS8P250MB0744E22E5884BCA6C8523A278F9DA@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <20250518122639.2958966-1-michael@niedermayer.cc>
Michael Niedermayer:
> 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 <michael@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"
Use proper alphabetical ordering
>
> #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++) {
This change is wrong.
> + const AVPacketSideData *const sd = &pkt->side_data[i];
> + const uint8_t *data = sd->data;
> + uint32_t side_data_crc = 0;
> +
> + switch (sd->type) {
> +#if HAVE_BIGENDIAN
> + uint8_t bswap_buf[FFMAX3(sizeof(AVCPBProperties),
> + sizeof(AVProducerReferenceTime),
> + sizeof(AVDynamicHDRPlus))];
> + case AV_PKT_DATA_PALETTE:
> + case AV_PKT_DATA_REPLAYGAIN:
> + case AV_PKT_DATA_DISPLAYMATRIX:
> + case AV_PKT_DATA_STEREO3D:
> + case AV_PKT_DATA_AUDIO_SERVICE_TYPE:
> + case AV_PKT_DATA_FALLBACK_TRACK:
> + case AV_PKT_DATA_MASTERING_DISPLAY_METADATA:
> + case AV_PKT_DATA_SPHERICAL:
> + case AV_PKT_DATA_CONTENT_LIGHT_LEVEL:
> + case AV_PKT_DATA_S12M_TIMECODE:
> + for (size_t j = 0; j < sd->size / 4; j++) {
> + uint8_t buf[4];
> + AV_WL32(buf, AV_RB32(sd->data + 4 * j));
> + side_data_crc = av_adler32_update(side_data_crc, buf, 4);
> + }
> + break;
> + case AV_PKT_DATA_CPB_PROPERTIES:
> +#define BSWAP(struct, field) bswap(bswap_buf, offsetof(struct, field), sizeof(((struct){0}).field))
> +#define BSWAP_RAT(struct, field) do{BSWAP(struct, field.num); BSWAP(struct, field.den);}while(0)
Missing spaces.
> + if (sd->size == sizeof(AVCPBProperties)) {
> + memcpy(bswap_buf, sd->data, sizeof(AVCPBProperties));
> + data = bswap_buf;
> + BSWAP(AVCPBProperties, max_bitrate);
> + BSWAP(AVCPBProperties, min_bitrate);
> + BSWAP(AVCPBProperties, avg_bitrate);
> + BSWAP(AVCPBProperties, buffer_size);
> + BSWAP(AVCPBProperties, vbv_delay);
> + }
> + goto pod;
> + case AV_PKT_DATA_PRFT:
> + if (sd->size == sizeof(AVProducerReferenceTime)) {
> + memcpy(bswap_buf, sd->data, sizeof(AVProducerReferenceTime));
> + data = bswap_buf;
> + BSWAP(AVProducerReferenceTime, wallclock);
> + BSWAP(AVProducerReferenceTime, flags);
> + }
> + goto pod;
> + case AV_PKT_DATA_DYNAMIC_HDR10_PLUS:
> + if (sd->size == sizeof(AVDynamicHDRPlus)) {
> + memcpy(bswap_buf, sd->data, sizeof(AVDynamicHDRPlus));
> + data = bswap_buf;
> + for(int i = 0; i<3; i++) {
Missing space after for and around "<". Also, why not use FF_ARRAY_ELEMS?
Same below.
> + BSWAP_RAT(AVDynamicHDRPlus, params[i].window_upper_left_corner_x);
> + BSWAP_RAT(AVDynamicHDRPlus, params[i].window_upper_left_corner_y);
> + BSWAP_RAT(AVDynamicHDRPlus, params[i].window_lower_right_corner_x);
> + BSWAP_RAT(AVDynamicHDRPlus, params[i].window_lower_right_corner_y);
> + BSWAP(AVDynamicHDRPlus, params[i].center_of_ellipse_x);
> + BSWAP(AVDynamicHDRPlus, params[i].center_of_ellipse_y);
> + BSWAP(AVDynamicHDRPlus, params[i].semimajor_axis_internal_ellipse);
> + BSWAP(AVDynamicHDRPlus, params[i].semimajor_axis_external_ellipse);
> + BSWAP(AVDynamicHDRPlus, params[i].semiminor_axis_external_ellipse);
> + //!!! overlap_process_option
> + for(int j=0; j<3; j++)
> + BSWAP_RAT(AVDynamicHDRPlus, params[i].maxscl[j]);
> + BSWAP_RAT(AVDynamicHDRPlus, params[i].average_maxrgb);
> + BSWAP_RAT(AVDynamicHDRPlus, params[i].fraction_bright_pixels);
> + BSWAP_RAT(AVDynamicHDRPlus, params[i].knee_point_x);
> + BSWAP_RAT(AVDynamicHDRPlus, params[i].knee_point_y);
> + for(int j=0; j<15; j++) {
> + BSWAP_RAT(AVDynamicHDRPlus, params[i].distribution_maxrgb[j].percentile);
> + BSWAP_RAT(AVDynamicHDRPlus, params[i].bezier_curve_anchors[j]);
> + }
> + BSWAP_RAT(AVDynamicHDRPlus, params[i].color_saturation_weight);
> + }
> + BSWAP_RAT(AVDynamicHDRPlus, targeted_system_display_maximum_luminance);
> + for(int i = 0; i<25; i++)
> + for(int j = 0; j<25; j++) {
> + BSWAP_RAT(AVDynamicHDRPlus, targeted_system_display_actual_peak_luminance[i][j]);
> + BSWAP_RAT(AVDynamicHDRPlus, mastering_display_actual_peak_luminance[i][j]);
> + }
> + }
> + pod:
> +#endif
> +
> + default:
> + side_data_crc = av_adler32_update(0, data, sd->size);
> + break;
> + case AV_PKT_DATA_IAMF_MIX_GAIN_PARAM:
> + case AV_PKT_DATA_IAMF_DEMIXING_INFO_PARAM:
> + case AV_PKT_DATA_IAMF_RECON_GAIN_INFO_PARAM:
> + side_data_crc = 0;
> + }
> +
> + av_strlcatf(buf, sizeof(buf), ", %s, %8"SIZE_SPECIFIER", 0x%08"PRIx32,
> + av_packet_side_data_name(sd->type), sd->size, side_data_crc);
> }
> }
> av_strlcatf(buf, sizeof(buf), "\n");
_______________________________________________
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".
next prev parent reply other threads:[~2025-05-18 14:43 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-18 12:25 Michael Niedermayer
2025-05-18 12:40 ` Michael Niedermayer
2025-05-18 14:43 ` Andreas Rheinhardt [this message]
2025-05-18 19:51 ` Michael Niedermayer
2025-06-07 18:01 ` Michael Niedermayer
2025-05-18 14:43 ` James Almer
2025-05-18 21:23 ` Michael Niedermayer
2025-06-08 2:54 ` James Almer
2025-06-09 20:24 ` Michael Niedermayer
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=AS8P250MB0744E22E5884BCA6C8523A278F9DA@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM \
--to=andreas.rheinhardt@outlook.com \
--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