On Sun, May 18, 2025 at 09:51:16PM +0200, Michael Niedermayer wrote:
> Hi
>
> On Sun, May 18, 2025 at 04:43:01PM +0200, Andreas Rheinhardt wrote:
> > 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 <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"
> >
> > Use proper alphabetical ordering
>
> Is there a inproper alphabetical ordering ?
>
>
>
> [...]
>
> >
> > > + 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.
>
> Ill try to find them and tell them to return, i hope they will pick the
> right places, because iam not sure which are the right places
>
>
> >
> > > + 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.
>
> I could do:
> FF_ARRAY_ELEMS(((AVDynamicHDRPlus*)0)->params)will apply the patch with these changes [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB There will always be a question for which you do not know the correct answer.
signature.asc
Description: PGP 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".
