diff --git a/crates/oak-codec/src/ffmpeg.rs b/crates/oak-codec/src/ffmpeg.rs index ad7a17350..364930444 100644 --- a/crates/oak-codec/src/ffmpeg.rs +++ b/crates/oak-codec/src/ffmpeg.rs @@ -474,6 +474,10 @@ struct VideoDecodeState { cache_at_eof: bool, /// One second in the stream's time base. second_ts: i64, + /// Whether the EOF-fallback warning has been printed for this session + /// (the warning fires once, when the last cached frame is returned with + /// a PTS far from the requested one). + eof_fallback_warned: bool, } /// Cached swscale context (recreated when any dimension/format changes). @@ -602,6 +606,7 @@ impl DecoderState { cache_at_zero: false, cache_at_eof: false, second_ts, + eof_fallback_warned: false, }) } else { None @@ -754,6 +759,24 @@ impl DecoderState { Ok(()) } + /// The smallest positive PTS gap between consecutive cached frames, in + /// the stream's time base. `None` when it cannot be derived (fewer than + /// two frames, or all frames share a PTS). + fn frame_interval_ts(video: &VideoDecodeState) -> Option { + let mut min: Option = None; + for pair in video.cache.iter().zip(video.cache.iter().skip(1)) { + let a = pair.0.pts().unwrap_or(AV_NOPTS_VALUE); + let b = pair.1.pts().unwrap_or(AV_NOPTS_VALUE); + if a != AV_NOPTS_VALUE && b != AV_NOPTS_VALUE { + let gap = (b - a).abs(); + if gap > 0 { + min = Some(min.map_or(gap, |m: i64| m.min(gap))); + } + } + } + min + } + /// Retrieve the video frame at (or before) `time`, mirroring the C++ /// `retrieve_frame` seek/cache logic. fn retrieve_frame( @@ -850,6 +873,26 @@ impl DecoderState { return Err(fail("unexpected codec EOF - unable to retrieve frame")); } return_frame = video.cache.back().cloned(); + // The returned frame is the last cached one, which may + // sit far from the requested timestamp (e.g. a corrupt + // or truncated file whose frames all decode to the + // start). Warn once per session with the actual offsets + // so the mismatch is diagnosable; skip files whose + // frames carry no usable PTS spacing (still images). + if !video.eof_fallback_warned { + video.eof_fallback_warned = true; + if let (Some(frame), Some(gap)) = + (return_frame.as_ref(), Self::frame_interval_ts(&video)) + { + let got = frame.pts().unwrap_or(AV_NOPTS_VALUE); + if got != AV_NOPTS_VALUE && (got - target_ts).abs() > gap { + eprintln!( + "oak-codec: EOF fallback in '{}': target {} got {} (frame gap {})", + self.filename, target_ts, got, gap + ); + } + } + } break; } Pull::Eagain => { @@ -1794,6 +1837,12 @@ struct VideoEncoderState { /// One frame in the encoder's time base (the last packet's duration; /// see `FFmpegEncoder::open`). frame_duration: i64, + /// The output stream's time base as left by `write_header`. The muxer + /// may re-set it while writing the header (mp4/mov force a video + /// timescale >= 10000) and FFmpeg 9 no longer rescales packet + /// timestamps to match, so every packet must be rescaled into this + /// value before `write_interleaved` (see `EncoderState::open`). + stream_time_base: FfRational, } /// Opened audio encoder + its conversion resampler. @@ -1982,11 +2031,11 @@ impl EncoderState { encoder.set_frame_rate(Some(frame_rate)); // The codecs' packet timestamps use a fine tick (x264 encodes at // 1024 ticks per frame); give H.264 an encoder time base scaled - // to that so the frame PTS stay integral, and sync the stream to - // the encoder's ACTUAL post-open time base (the value the muxer - // reads) so the container timing is `seconds * frame_rate`. - // Other codecs (e.g. MPEG-2) reject the scaled rate and keep the - // nominal frame-duration time base. + // to that so the frame PTS stay integral. Other codecs (e.g. + // MPEG-2) reject the scaled rate and keep the nominal + // frame-duration time base. The stream is synced to this value + // below (pre-header); packets are rescaled into the stream's + // post-header time base when written (`stream_time_base`). let tick = if codec_id == ffmpeg::codec::Id::H264 { FfRational(time_base.0, time_base.1 * 1024) } else { @@ -2010,9 +2059,14 @@ impl EncoderState { stream.set_parameters(&opened); // The encoder may adjust the time base during `open` (x264 // picks its own); sync the stream to the encoder's ACTUAL time - // base so the container timing matches the frame PTS computed - // from it (a mismatched stream time base crams the whole video - // into the first milliseconds). + // base. This is only the pre-header value though: the muxer + // re-sets the stream time base inside `write_header` (mp4/mov + // force a video timescale >= 10000, so a 10 fps stream's 1/10 + // becomes 1/10240) and FFmpeg 9 no longer rescales packet + // timestamps for us, so the packets written after that point + // must already be in the stream's final time base. `open` + // records that post-header value and every video packet is + // rescaled into it (identity when both match, e.g. mkv). let time_base = opened.time_base(); stream.set_time_base(time_base); // One frame in the encoder's time base, used to fill the last @@ -2042,6 +2096,8 @@ impl EncoderState { height, time_base, frame_duration, + // Overwritten below with the stream's real post-header value. + stream_time_base: time_base, }); } @@ -2098,6 +2154,15 @@ impl EncoderState { output.write_header().map_err(ffmpeg_err)?; + // The muxer may have adjusted the stream time base while writing + // the header (see the note above); read the value the container + // actually uses so packets can be rescaled into it. + if let Some(video) = video.as_mut() { + if let Some(tb) = stream_time_base_after_header(&output, video.stream_index) { + video.stream_time_base = tb; + } + } + self.output = Some(OutputState { output, video, @@ -2162,9 +2227,11 @@ impl EncoderState { // # CPP-PARITY // `FFmpegEncoder::write_frame` passes the frame time in seconds; the - // Rust `Frame` carries the timestamp as a rational. The PTS is - // expressed in the encoder's own time base (captured at open), so - // the container timing is exactly `seconds * rate`. + // Rust `Frame` carries the timestamp as a rational. The frame PTS + // given to the encoder stays in the encoder's own time base + // (captured at open) — the encoder validates them against its own + // rate — and the packets it emits are rescaled into the stream's + // post-`write_header` time base when written (`drain_video_packets`). let secs = frame.timestamp().to_f64(); let tb = video.time_base; let pts = (secs * tb.1 as f64 / tb.0 as f64).round() as i64; @@ -2287,6 +2354,10 @@ fn drain_video_encoder( if pkt.duration() <= 0 { pkt.set_duration(video.frame_duration); } + // Same rescale as `drain_video_packets`: the final flushed + // frame is emitted with encoder time base, which the muxer + // cannot consume directly. + pkt.rescale_ts(video.time_base, video.stream_time_base); pkt.write_interleaved(output).map_err(ffmpeg_err)?; } Err(e) if is_eof_or_eagain(&e) => break, @@ -2332,6 +2403,11 @@ fn drain_video_packets( if pkt.duration() <= 0 { pkt.set_duration(video.frame_duration); } + // The encoder emits timestamps in its own time base; the + // muxer expects them in the stream's post-`write_header` + // time base (see `EncoderState::open`), so rescale the + // whole packet (pts/dts/duration) before writing. + pkt.rescale_ts(video.time_base, video.stream_time_base); pkt.write_interleaved(output).map_err(ffmpeg_err)?; } Err(e) if is_eof_or_eagain(&e) => break, @@ -2360,6 +2436,30 @@ fn drain_audio_packets( Ok(()) } +/// The stream's time base as recorded in the container after +/// `write_header` (the muxer may have re-set it). `None` if the stream is +/// missing or the muxer left the time base unset (num/den == 0). +fn stream_time_base_after_header( + output: &ffmpeg::format::context::Output, + index: usize, +) -> Option { + // SAFETY: `output` is a live `AVFormatContext`; `nb_streams` bounds the + // `streams` array, and `index` came from the same context's + // `add_stream`. Reading the (muxer-owned) stream time base is a plain + // field read with no aliasing. + unsafe { + let ctx = output.as_ptr(); + let streams = std::slice::from_raw_parts((*ctx).streams, (*ctx).nb_streams as usize); + let st = *streams.get(index)?; + let tb = (*st).time_base; + if tb.num == 0 || tb.den == 0 { + None + } else { + Some(FfRational(tb.num, tb.den)) + } + } +} + /// Map an `ExportCodec::Codec` raw value to an FFmpeg codec id /// (CPP-PARITY `FFmpegEncoder::export_codec_to_bridge`). Values are the /// documented `ExportCodec::Codec` discriminants (see `exportcodec.rs`). diff --git a/crates/oak-codec/src/realmedia_tests.rs b/crates/oak-codec/src/realmedia_tests.rs index 537d3d760..3308a9df8 100644 --- a/crates/oak-codec/src/realmedia_tests.rs +++ b/crates/oak-codec/src/realmedia_tests.rs @@ -254,29 +254,72 @@ fn audio_conform_writes_planar_pcm() { let _ = std::fs::remove_dir_all(&dir); } +/// Count red-dominant pixels on `row` within the left half of the frame +/// (`x < width/2`). The test pattern's red/blue boundary sweeps right, so +/// the left half is solid red for exactly `32 - shift` columns; the right +/// half contains the wrap-around red strip, which is excluded here. MPEG-2's +/// lossy YUV round-trip keeps the dominance, shifting the boundary by at +/// most a couple of columns. +fn red_row_count(f: &Frame, row: usize) -> usize { + let stride = f.linesize_bytes() as usize; + let data = f.data().expect("allocated frame data"); + let width = f.width() as usize; + let mut count = 0; + for x in 0..width / 2 { + let off = row * stride + x * 16; + let r = f32::from_le_bytes(data[off..off + 4].try_into().unwrap()); + let b = f32::from_le_bytes(data[off + 8..off + 12].try_into().unwrap()); + if r > b { + count += 1; + } + } + count +} + #[test] fn testmedia_clip_probe_roundtrip() { let out = std::env::temp_dir().join(format!("oakcodec_tm3_{}.mp4", std::process::id())); crate::testmedia::write_test_clip(&out, 64, 64, 10, 10).expect("generate"); + println!("out: {}", out.display()); let d = FFmpegDecoder::new(); + + // The container duration must be ~1s (10 frames at 10 fps) in the video + // stream's own time base. This is the regression test for the encoder + // timestamp fix: when the muxer's `write_header` time-base change was + // ignored, the whole clip was crammed into ~1ms and this failed. + let desc = d.probe(&out.to_string_lossy(), None).expect("probe"); + let vp = desc.get_video_stream(0).expect("video stream"); + let (tb_num, tb_den) = vp.time_base(); + let duration_secs = vp.duration() as f64 * tb_num as f64 / tb_den as f64; + assert!( + (0.8..=1.2).contains(&duration_secs), + "container duration {duration_secs}s is not ~1s (tb {tb_num}/{tb_den}, {} ticks)", + vp.duration() + ); + + // Every frame must decode to its own content: the pattern's red/blue + // boundary sweeps right `shift` columns per frame, so the red-run + // length in the left half of row 32 is 32 - shift (tolerance for + // MPEG-2 loss). let s = CodecStream::with_block(out.to_string_lossy().into_owned(), 0, None); d.open(&s).expect("open"); - for t in [0i64, 3, 5, 9] { + for t in 0..10i64 { let f = d .retrieve_video_frame(&video_params(s.clone(), Rational::new(t, 10))) .unwrap_or_else(|e| panic!("decode t={t}: {e:?}")); - let stride = f.linesize_bytes() as usize; - let data = f.data().unwrap(); - let off = 32 * stride + 8 * 16; - let px = [ - f32::from_le_bytes(data[off..off + 4].try_into().unwrap()), - f32::from_le_bytes(data[off + 4..off + 8].try_into().unwrap()), - f32::from_le_bytes(data[off + 8..off + 12].try_into().unwrap()), - f32::from_le_bytes(data[off + 12..off + 16].try_into().unwrap()), - ]; - println!("t={t}/10 px(8,32): {px:?}"); + let red = red_row_count(&f, 32); + let shift = (t * 64 / (2 * 10)) % 64; + let expected = 32 - shift; + assert!( + (red as i64 - expected).abs() <= 3, + "t={t}/10 red pixels on row 32: {red}, expected ~{expected} (shift {shift})" + ); + } + + // OAK_KEEP_TESTMEDIA keeps the clip for external inspection (ffprobe). + if std::env::var_os("OAK_KEEP_TESTMEDIA").is_none() { + let _ = std::fs::remove_file(&out); } - let _ = std::fs::remove_file(&out); } #[test]