Re: [PR] mxfdec: Populate frame rate info for streams (PR #23929)

Tomas Härdin via ffmpeg-devel <[email protected]>
Newsgroups gmane.comp.video.ffmpeg.devel
Message-ID <[email protected]>
mån 2026-07-27 klockan 18:20 +0000 skrev anders-mjoll via ffmpeg-devel:
PR #23929 opened by anders-mjoll
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23929
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23929.patch

# Summary of changes

Briefly describe what this PR does and why.

MXF files containing PAFF encoded interlaced h264 will be misreported
by ffprobe to have half the frame rate it actually has. This is
because for most container formats PAFF interlaced h264 produces one
AVPacket per field. Two AVPackets pushed into the h264 decoder will
produce one AVFrame. In order to compensate for this common behavior
the general probing code will divide the packet rate by two in order
to calculate the frame rate. For MXF files this is not the behavior
as both fields are returned as a single AVPacket. So for MXF the
packet rate is equal to the frame rate and the PAFF compensation code
results in half the frame rate being reported. Instead of fixing the
MXF demuxer so that it returns one AVPacket for each field OR fixing
the probing code so that it accounts for one AVPacket having two
fields I just modified the MXF demuxer so that it provides the frame
rate explicitly. For MXF files this information is always known
anyway.


From f6b4a29d96d4b74697da746915b07e84e749a6e2 Mon Sep 17 00:00:00
2001
From: Anders Rein <[email protected]>
Date: Mon, 27 Jul 2026 20:11:31 +0200
Subject: [PATCH] mxfdec: Populate frame rate info for streams

---
 libavformat/mxfdec.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 4ba4af1fa7..19731a5c2a 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -2995,6 +2995,16 @@ static int
mxf_parse_structural_metadata(MXFContext *mxf)
                 }
             }
 
+            /* Frame wrapped H.264 stores exactly one coded frame
per edit unit,
+             * so the edit rate is the frame rate. Without this,
PAFF streams get
+             * a halved avg_frame_rate: the packet holds two field
pictures, but
+             * the parser only reports the first one and thus a
single field. */
+            if (st->codecpar->codec_id == AV_CODEC_ID_H264 &&
+                source_track->wrapping == FrameWrapped) {
+                st->avg_frame_rate = source_track->edit_rate;
+                st->r_frame_rate = st->avg_frame_rate;
+            }

We have to be very careful here, because the source track's EditRate is
not the "frame rate" per se. In MXF the frame rate is whatever rate the
file should be rendered as, which is determined at the MaterialPackage
level. It's possible for the MaterialPackage(s) and the FilePackage(s)
to use different EditRates.

Or, it would be, were it not for this cursed line:

        /* ensure SourceTrack EditRate == MaterialTrack EditRate
since only
         * the former is accessible via st->priv_data */
        source_track->edit_rate = material_track->edit_rate;


With PAFF it seems like it's possible to mix progressive and interlaced
essence, because interlacing was not cursed enough as it is.

Can this really only happen for H.264? What about MPEG-2 (H.262),
H.265, H.266 etc? For MPEG-2 specifically, S377m mentions that
SampleRate may be different from EditRate (page 141 in the 2009
version):

For MPEG-2 encoded video applying field coding for all frames in the
Essence Container, the value of Sample Rate equals the video field
rate.

Given this mess I'd prefer if we added more checks, along with loud
complaints if the file doesn't look like what we'd expect. For example,
it doesn't seem like we'll handle different MP and FP EditRate, so we
should probably error out in that case rather than produce garbage.

/Tomas
_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.