collective
Browse / github.com/storytold/filmcraft / patch
fix(platform): notice pictures Microsoft's VP9 decoder drops without an error
fix(platform): notice pictures Microsoft's VP9 decoder drops without an error ## Problem On Windows, the VP9 Video Extensions decoder MFT occasionally accepts a sample and never returns its picture, with no error, while other work keeps the GPU busy. In the app, hardware VP9 playback or export silently loses that frame. In CI-like runs of `cargo test -p filmcraft-platform --test it` (Intel Iris Xe, ffmpeg oracles required), roughly one run in three or four failed on a VP9 parity test (`vp9_profile_0_is_bit_exact`, `vp9_profile_2_10bit_is_bit_exact` or `bit_exact_with_the_software_decoders`) with one pts missing. It happens on `main` as well. A trace of a failing r
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- sarayunova (imported from pull request #3334, open)
- Base
de529c88f4948d0803bef746a325707f2671a71d- Size
- +263 −11 in 9 files
- Review
- pending
- Shared
- 2026-10-28
- Id
a6870822b2445f2e07ad7028
How it went
No other operator has reported using it yet.
Diff
raw · crates/codecs/src/hw_frame.rs crates/codecs/src/video.rs crates/platform/README.md crates/platform/src/hybrid.rs crates/platform/src/media_foundation/mod.rs crates/platform/tests/it/fallback.rs crates/platform/tests/it/media_foundation.rs crates/vp9/src/header.rs crates/vp9/src/lib.rs
@@ -224,6 +224,15 @@ impl FrameStreamInfo { } } + /// Whether decoding `sample` shows a picture. A VP9 sample can hold only a hidden frame; an AV1+ /// temporal unit always shows exactly one.+ pub fn shows_picture(&self, sample: &[u8]) -> bool {+ match self.codec {+ FrameCodec::Vp9 => filmcraft_vp9::shows_picture(sample),+ FrameCodec::Av1 => true,+ }+ }+ /// The picture parameters a random-access `sample` declares (a VP9 key frame header; an AV1 /// sample carrying a sequence header), or `None`. pub fn picture_params(&self, sample: &[u8]) -> Option<PictureParams> {@@ -20,6 +20,12 @@ pub trait VideoDecoder: Send { fn decode(&mut self, sample: &[u8], pts: i64) -> Result<Vec<DecodedFrame>>; /// Drain pictures held for reordering (end of stream / before a seek). fn flush(&mut self) -> Vec<DecodedFrame>;+ /// How many pictures the last [`flush`](Self::flush) should have returned but lost (a hardware+ /// decoder that dropped them), which `flush` itself cannot report; taking the count clears it.+ /// A wrapper with a second decoder for the stream decodes the run again. Zero by default.+ fn take_flush_losses(&mut self) -> usize {+ 0+ } /// Forget all state (called after seeking to a sync sample). fn reset(&mut self); fn name(&self) -> &str;@@ -56,6 +56,13 @@ let availability = filmcraft_platform::register(); // Available("VideoToolbox") are read from the bitstream the way the software decoders do (`hw_frame::vp9_color` / `av1_color` are shared with them). A VP9 key frame of another size or format, or an AV1 sequence header unlike `av1C`'s, hands the stream to the software decoder (`HybridDecoder`).+ Microsoft's VP9 decoder sometimes skips a picture without an error while the GPU is busy; since+ pictures come out in the order they go in, a later picture overtaking one still in flight+ shows it, and the decoder fails so `HybridDecoder` replays the run in software+ (`media_foundation::dropped_pictures` counts them). While flushing, the decoder still returns+ the pictures it holds and reports the loss through `VideoDecoder::take_flush_losses`;+ `HybridDecoder` then replays the run in software for the missing ones. A sample holding only a hidden VP9 frame is+ not waited for (`vp9::shows_picture`). - `mfplat.dll` is loaded at run time (`mft.rs`), not linked: Windows "N" editions without the Media Feature Pack still start FilmCraft, which then decodes in software. - After a `flush` (which drains the MFT) the MFT only restarts at an IDR picture; the GOP cache@@ -216,9 +216,27 @@ impl VideoDecoder for HybridDecoder { fn flush(&mut self) -> Vec<DecodedFrame> { if let Some(hw) = self.hw.as_mut() { let out = hw.flush();+ let lost = hw.take_flush_losses(); filmcraft_codecs::hw::note_hw_frames(out.len());- self.note_emitted(&out);- return out;+ if lost == 0 || self.log.is_none() {+ // (too far from a restart point to replay: the pictures that did come out)+ self.note_emitted(&out);+ return out;+ }+ // The hardware lost pictures while draining: keep the ones it returned, decode the run+ // again in software for the rest, and drain that. (A lost picture from before the last+ // restart point is not in the replay log; it stays lost.)+ for f in out {+ if !self.emitted.contains(&f.pts) {+ self.carry.insert(f.pts, f);+ }+ }+ let mut frames = self.fall_back(CodecError::Decode(format!("lost {lost} pictures while flushing"))).unwrap_or_else(|e| {+ log::warn!("software replay after the hardware decoder lost pictures: {e}");+ Vec::new()+ });+ frames.extend(self.flush());+ return frames; } let frames = self.sw.as_mut().map(|d| d.flush()).unwrap_or_default(); let mut out = self.merge(frames);@@ -33,6 +33,7 @@ mod mft; mod stream; use std::sync::Arc;+use std::sync::atomic::{AtomicU64, Ordering}; use std::time::{Duration, Instant}; use filmcraft_codecs::hw::{NalCodec, StreamInfo};@@ -48,6 +49,16 @@ const MAX_IN_FLIGHT: usize = 64; /// Upper bound on the outputs taken per call: a misbehaving MFT cannot make a decode call loop. const MAX_OUTPUTS_PER_CALL: usize = 4096; +/// Pictures a VP9 / AV1 decoder MFT accepted and never returned, in this process.+static DROPPED: AtomicU64 = AtomicU64::new(0);++/// How many pictures decoder MFTs dropped in this process (diagnostics and tests). Microsoft's VP9+/// decoder occasionally skips a picture without an error when the GPU is busy; the decoder notices+/// it and fails, so the hybrid decoder replays the run in software.+pub fn dropped_pictures() -> u64 {+ DROPPED.load(Ordering::Relaxed)+}+ /// The stream description of a sample entry this backend could take (`avcC` / `hvcC` / `vpcC` / /// `av1C`), or `None` for other codecs and unreadable configuration records. pub fn stream_info(entry: &filmcraft_isobmff::SampleEntry) -> Option<StreamInfo> {@@ -137,6 +148,8 @@ pub struct MfDecoder { /// Test hook: fail every decode once this many samples were fed. fail_after: Option<u64>, fed: u64,+ /// Pictures the last flush lost ([`VideoDecoder::take_flush_losses`]).+ flush_losses: usize, name: &'static str, timings: Timings, }@@ -172,6 +185,7 @@ impl MfDecoder { drained: false, fail_after: None, fed: 0,+ flush_losses: 0, timings: Timings::default(), }) }@@ -203,6 +217,15 @@ impl MfDecoder { } } + /// A VP9 / AV1 picture still in flight that was submitted before the one stamped `t`: frame-coded+ /// streams come out in the order they go in, so the decoder dropped it.+ fn skipped_before(&self, t: i64) -> Option<i64> {+ if !matches!(self.stream, Stream::Frame(_)) {+ return None;+ }+ overtaken(&self.in_flight, t)+ }+ /// A VP9 / AV1 decoder must output the picture size the bitstream declares (H.264 / HEVC /// decoders report coded or cropped sizes, which the cropping handles). fn check_size(&mut self) -> std::result::Result<(), String> {@@ -214,14 +237,32 @@ impl MfDecoder { } } - /// Take every picture the decoder has ready (presentation order).- fn collect(&mut self, out: &mut Vec<DecodedFrame>) -> std::result::Result<(), String> {+ /// Take every picture the decoder has ready (presentation order). A picture overtaken by a+ /// later one was dropped: while decoding that is an error (the hybrid decoder replays the run in+ /// software); while `draining`, the pictures still held are taken and the loss is counted in+ /// `flush_losses`, which the hybrid decoder asks for after the flush.+ fn collect(&mut self, out: &mut Vec<DecodedFrame>, draining: bool) -> std::result::Result<(), String> { for _ in 0..MAX_OUTPUTS_PER_CALL { let Some(mft) = self.mft.as_ref() else { return Ok(()) }; match mft.poll()? { Poll::NeedInput => return Ok(()), Poll::FormatChanged => self.size_checked = false, Poll::Frame(sample, t) => {+ if let Some(skipped) = self.skipped_before(t) {+ let before = self.in_flight.len();+ if draining {+ self.in_flight.retain(|&p| p >= t);+ let lost = before.saturating_sub(self.in_flight.len());+ DROPPED.fetch_add(lost as u64, Ordering::Relaxed);+ self.flush_losses = self.flush_losses.saturating_add(lost);+ } else {+ // forget the dropped pictures and this one (not returned either), so+ // a flush after the error does not count them again+ self.in_flight.retain(|&p| p > t);+ DROPPED.fetch_add(before.saturating_sub(self.in_flight.len() + 1) as u64, Ordering::Relaxed);+ return Err(format!("the decoder dropped the picture at {skipped}"));+ }+ } if !self.size_checked { self.check_size()?; }@@ -244,14 +285,17 @@ impl MfDecoder { Err("the decoder keeps producing output".into()) } - fn feed(&mut self, annex_b: &[u8], pts: i64) -> std::result::Result<Vec<DecodedFrame>, String> {+ /// Feed one sample; `shows` is false for a sample that decodes to no picture (a VP9 hidden frame).+ fn feed(&mut self, annex_b: &[u8], pts: i64, shows: bool) -> std::result::Result<Vec<DecodedFrame>, String> { let mut out = Vec::new(); for attempt in 0..2 { let Some(mft) = self.mft.as_ref() else { return Err("no decoder".into()) }; if mft.input(self.api, annex_b, pts)? {- self.in_flight.push(pts);+ if shows {+ self.in_flight.push(pts);+ } self.need_headers = false;- self.collect(&mut out)?;+ self.collect(&mut out, false)?; // a decoder holds back a DPB's worth of pictures at most if self.in_flight.len() > MAX_IN_FLIGHT { return Err(format!("the decoder holds {} pictures back", self.in_flight.len()));@@ -259,7 +303,7 @@ impl MfDecoder { return Ok(out); } // full: take what it has ready, then offer the sample again- self.collect(&mut out)?;+ self.collect(&mut out, false)?; if attempt == 1 { break; }@@ -302,6 +346,10 @@ impl VideoDecoder for MfDecoder { if let Some(g) = self.stream.declared_picture(sample) { self.geometry = g; }+ let shows = match &self.stream {+ Stream::Frame(info) => info.shows_picture(sample),+ Stream::Nal(_) => true,+ }; let annex_b = self.stream.input(sample, self.need_headers).map_err(CodecError::Decode)?; if self.mft.is_none() { let mft = Mft::new(self.api, &self.gpu, &self.stream.spec(self.format)).map_err(CodecError::Decode)?;@@ -310,7 +358,7 @@ impl VideoDecoder for MfDecoder { self.need_headers = true; } let (t0, spent) = (Instant::now(), self.timings.readback + self.timings.convert);- let r = self.feed(&annex_b, pts).map_err(|e| {+ let r = self.feed(&annex_b, pts, shows).map_err(|e| { self.gpu.note_failure(); CodecError::Decode(e) });@@ -324,9 +372,17 @@ impl VideoDecoder for MfDecoder { if gpu::ensure_com().is_err() { return out; }- let drained = self.mft.as_ref().map(Mft::drain).transpose().and_then(|_| self.collect(&mut out));+ self.flush_losses = 0;+ let drained = self.mft.as_ref().map(Mft::drain).transpose().and_then(|_| self.collect(&mut out, true)); if let Err(e) = drained { log::warn!("{}: {e} while flushing", self.name);+ } else if matches!(self.stream, Stream::Frame(_)) && !self.in_flight.is_empty() {+ // drained, yet some pictures never came out+ DROPPED.fetch_add(self.in_flight.len() as u64, Ordering::Relaxed);+ self.flush_losses = self.flush_losses.saturating_add(self.in_flight.len());+ }+ if self.flush_losses > 0 {+ log::warn!("{}: the decoder dropped {} pictures while flushing", self.name, self.flush_losses); } self.in_flight.clear(); // what follows a drain starts like a new run: the decoder is given the headers again@@ -335,6 +391,10 @@ impl VideoDecoder for MfDecoder { out } + fn take_flush_losses(&mut self) -> usize {+ std::mem::take(&mut self.flush_losses)+ }+ fn reset(&mut self) { // after a seek no reference picture survives if gpu::ensure_com().is_ok() && self.mft.as_ref().map(Mft::flush).transpose().is_err() {@@ -361,9 +421,34 @@ impl VideoDecoder for MfDecoder { } } +/// The earliest of `in_flight` that a picture stamped `t` came out ahead of, when `t` is one of them+/// (an output the decoder restamped says nothing about order).+fn overtaken(in_flight: &[i64], t: i64) -> Option<i64> {+ if !in_flight.contains(&t) {+ return None;+ }+ in_flight.iter().copied().filter(|&p| p < t).min()+}+ impl Drop for MfDecoder { fn drop(&mut self) { // the decoder MFT is released on whichever thread drops the decoder: it needs COM let _ = gpu::ensure_com(); } }++#[cfg(test)]+mod tests {+ use super::overtaken;++ #[test]+ fn a_picture_overtaken_by_a_later_one_was_dropped() {+ assert_eq!(overtaken(&[512], 512), None, "in order");+ assert_eq!(overtaken(&[512, 1024], 512), None, "the next one is still coming");+ // the trace of a real drop: 21504 accepted, never returned, 22016 came out+ assert_eq!(overtaken(&[21504, 22016], 22016), Some(21504));+ assert_eq!(overtaken(&[1536, 512, 1024], 1536), Some(512), "the earliest");+ assert_eq!(overtaken(&[512, 1024], 700), None, "restamped: no evidence");+ assert_eq!(overtaken(&[], 0), None);+ }+}@@ -110,6 +110,69 @@ fn changed_in_band_parameter_sets_switch_to_software() { assert_same("changed SPS", &out, &decode_all(filmcraft_codecs::software_video_decoder(&s.entry).unwrap().as_mut(), &changed)); } +/// Decodes like the software decoder, but its flush loses some of the pictures it returns from+/// presentation time `from` on (`lose` picks them by index among those) and says so through+/// `take_flush_losses`, as Media Foundation's VP9 decoder does when it drops a picture while+/// draining. (A lost picture from before the replay's restart point cannot be decoded again.)+struct LosingAtFlush {+ inner: Box<dyn VideoDecoder>,+ from: i64,+ lose: fn(usize) -> Vec<usize>,+ lost: usize,+}++impl VideoDecoder for LosingAtFlush {+ fn decode(&mut self, sample: &[u8], pts: i64) -> Result<Vec<DecodedFrame>> {+ self.inner.decode(sample, pts)+ }+ fn flush(&mut self) -> Vec<DecodedFrame> {+ let all = self.inner.flush();+ let later: Vec<i64> = all.iter().map(|f| f.pts).filter(|&p| p >= self.from).collect();+ let lose: Vec<i64> = (self.lose)(later.len()).into_iter().filter_map(|i| later.get(i).copied()).collect();+ self.lost = lose.len();+ all.into_iter().filter(|f| !lose.contains(&f.pts)).collect()+ }+ fn take_flush_losses(&mut self) -> usize {+ std::mem::take(&mut self.lost)+ }+ fn reset(&mut self) {+ self.inner.reset();+ }+ fn name(&self) -> &str {+ "stand-in hardware"+ }+}++#[test]+fn pictures_lost_while_flushing_come_from_the_software_decoder() {+ let ff = filmcraft_testkit::require_ffmpeg!();+ let patterns: [(&str, fn(usize) -> Vec<usize>); 4] =+ [("nothing", |_| Vec::new()), ("the first", |_| vec![0]), ("the last", |n| vec![n.saturating_sub(1)]), ("all", |n| (0..n).collect())];+ for (name, _) in FIXTURES {+ let Some(path) = named(&ff, name) else { continue };+ let s = read_stream(&path);+ let info = NalStreamInfo::from_entry(&s.entry).unwrap().unwrap();+ let syncs: Vec<usize> = (0..s.samples.len()).filter(|&i| s.sync[i]).collect();+ let [_, k, ..] = syncs[..] else { panic!("{name}: two GOPs or more") };+ // a run ending a few samples into the second GOP: pictures are held for reordering+ let run = &s.samples[..k + 6];+ let from = s.samples[k].1;+ let reference = decode_all(filmcraft_codecs::software_video_decoder(&s.entry).unwrap().as_mut(), run);+ for (what, lose) in patterns {+ let inner = filmcraft_codecs::software_video_decoder(&s.entry).unwrap();+ let mut d = HybridDecoder::new(Box::new(LosingAtFlush { inner, from, lose, lost: 0 }), s.entry.clone(), info.clone());+ let out = decode_all(&mut d, run);+ assert_eq!(d.is_hardware(), what == "nothing", "{name}: losing {what}");+ assert_same(&format!("{name} losing {what} while flushing"), &out, &reference);+ // and decoding goes on after a seek+ d.reset();+ let tail = &s.samples[k..];+ let expect = decode_all(filmcraft_codecs::software_video_decoder(&s.entry).unwrap().as_mut(), tail);+ assert_same(&format!("{name} after losing {what}"), &decode_all(&mut d, tail), &expect);+ }+ }+}+ /// Like [`Failing`], but tolerant of one damaged sample the way a GPU can be: at `damaged_at` it /// decodes the intact sample instead of the one it is given (which the software decoder rejects). struct Tolerant {@@ -35,10 +35,31 @@ fn software(s: &Stream) -> Box<dyn VideoDecoder> { filmcraft_codecs::software_video_decoder(&s.entry).unwrap() } +/// [`parity_once`], again when an attempt fails after a decoder MFT dropped a picture: Microsoft's+/// VP9 decoder sometimes skips one without an error while other tests keep the GPU busy. The+/// decoder reports that as an error (the hybrid decoder then continues in software, see+/// `media_foundation::dropped_pictures`), which these comparisons of the hardware's own output+/// cannot tell from a wrong picture. Any other failure fails at once.+fn parity(name: &str, s: &Stream) {+ const ATTEMPTS: usize = 3;+ for attempt in 1..=ATTEMPTS {+ let dropped = filmcraft_platform::media_foundation::dropped_pictures();+ let r = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| parity_once(name, s)));+ match r {+ Ok(()) => return,+ Err(e) if attempt < ATTEMPTS && filmcraft_platform::media_foundation::dropped_pictures() > dropped => {+ eprintln!("{name}: the decoder MFT dropped a picture (attempt {attempt}); trying again");+ drop(e);+ }+ Err(e) => std::panic::resume_unwind(e),+ }+ }+}+ /// Every picture of `s` from the hardware decoder equals the software decoder's: whole stream, /// after `reset` + reseek to each later sync sample, after a mid-stream `flush`, and a full pass /// after all the resets.-fn parity(name: &str, s: &Stream) {+fn parity_once(name: &str, s: &Stream) { let Some(mut hw) = hardware(s) else { return }; let mut sw = software(s); let a = decode_all(hw.as_mut(), &s.samples);@@ -598,6 +598,25 @@ pub fn parse_compressed(data: &[u8], h: &mut FrameHeader, fc: &mut FrameContext) Ok(()) } +/// Whether a frame shows a picture (`show_frame`, or a `show_existing_frame`), read from the start+/// of its uncompressed header, which needs no decoder state; `None` when that start is malformed.+pub fn shows_picture(data: &[u8]) -> Option<bool> {+ let mut r = BitReader::new(data);+ if r.read_bits(2).ok()? != 2 {+ return None;+ }+ let lo = r.read_bits(1).ok()?;+ let hi = r.read_bits(1).ok()?;+ if (hi << 1) | lo == 3 && r.read_bits(1).ok()? != 0 {+ return None;+ }+ if r.read_flag().ok()? {+ return Some(true);+ }+ r.read_bits(1).ok()?; // frame_type+ r.read_flag().ok()+}+ /// Superframe index parsing (Annex B): the frames of a chunk. pub fn split_superframe(data: &[u8]) -> Vec<&[u8]> { if let Some(&last) = data.last()@@ -647,6 +666,22 @@ mod tests { assert_eq!(split_superframe(&d2), vec![&d2[..]]); } + #[test]+ fn shown_and_hidden_frames() {+ // frame_marker 2, profile 0, then show_existing_frame / frame_type / show_frame+ assert_eq!(shows_picture(&[0b1000_1000]), Some(true), "show_existing_frame");+ assert_eq!(shows_picture(&[0b1000_0010]), Some(true), "key frame, shown");+ assert_eq!(shows_picture(&[0b1000_0000]), Some(false), "key frame, hidden");+ assert_eq!(shows_picture(&[0b1000_0110]), Some(true), "inter frame, shown");+ assert_eq!(shows_picture(&[0b1000_0100]), Some(false), "inter frame, hidden");+ // profile 3 has a reserved bit before show_existing_frame+ assert_eq!(shows_picture(&[0b1011_0001]), Some(true), "profile 3, shown");+ assert_eq!(shows_picture(&[0b1011_0000]), Some(false), "profile 3, hidden");+ assert_eq!(shows_picture(&[0b1011_1000]), None, "profile 3, reserved bit set");+ assert_eq!(shows_picture(&[0b0100_0000]), None, "bad frame marker");+ assert_eq!(shows_picture(&[]), None, "empty");+ }+ #[test] fn inv_remap_is_bijective_per_prob() { for p in 1..=255u8 {@@ -82,6 +82,14 @@ pub fn keyframe_info(chunk: &[u8]) -> Option<KeyframeInfo> { pub fn is_keyframe(chunk: &[u8]) -> bool { keyframe_info(chunk).is_some() }++/// Whether decoding `chunk` (a frame or a superframe) shows a picture: a chunk can also hold only a+/// hidden frame (an alt-ref frame outside a superframe). A frame whose header does not parse counts+/// as showing one, since a decoder reports an error for it rather than skipping it.+pub fn shows_picture(chunk: &[u8]) -> bool {+ header::split_superframe(chunk).into_iter().filter(|f| !f.is_empty()).any(|f| header::shows_picture(f) != Some(false))+}+ pub use error::{Error, Result}; /// Colour information from the uncompressed header.