From 5d5837c4dbcc1a526fdf8ef6d9f834c6721d0a2c Mon Sep 17 00:00:00 2001 From: Alitleis123 Date: Sun, 11 Oct 2026 18:14:39 -0400 Subject: [PATCH 1/2] Honour keyframe requests on AMD and survive display mode changes FFmpeg 7.1.1's AMF wrapper never reads AVFrame::pict_type, so every forced keyframe on an AMD GPU was ignored. After a reconnect, a keyframe request or dropped USB frames, the iPad waited for AMF's own IDR, which comes every 30 encoded frames. On an idle desktop the capture side resends a frame only every 750 ms, so that wait could reach about 22 seconds. The encode loop now reopens the AMF session instead, because a new session always starts with an IDR that carries its parameter sets. These reopens happen at most once per 500 ms, the same limit the host applies to client keyframe requests. A display mode change that resized the capture (Radeon Super Resolution, VSR, GPU scaling, a resized virtual display) left the encoder on its old frame buffers. A larger mode panicked the encode thread on a slice bound and a smaller one sent a garbled picture. The loop now reopens the encoder at the new size, and restarts the pipeline if that reopen fails. host/tests/amf_encoder.rs drives the real encode stage with synthetic frames and decodes the output with a strict FFmpeg decoder. ETERNAL_AMF_TEST=hardware requires a working AMF encoder, for the Radeon reference PC. CI's Windows job sets fallback, which proves AMF fails cleanly without an AMD driver and the stage falls back to libx264. libx264 runs the same keyframe and mode-change checks on every platform. Unit tests pin the h264_amf and hevc_amf options the iPad decoder needs, the reopen rate limit, and the rule that AMF all-intra access units go out as keyframes. --- .github/workflows/ci.yml | 4 + docs/beta-testing.md | 13 ++ host/src/control.rs | 7 +- host/src/encoder/mod.rs | 136 +++++++++++-- host/tests/amf_encoder.rs | 403 ++++++++++++++++++++++++++++++++++++++ proto/src/h264.rs | 38 ++++ 6 files changed, 585 insertions(+), 16 deletions(-) create mode 100644 host/tests/amf_encoder.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 30fc3d8..9b1f1c5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -70,6 +70,10 @@ jobs: run: cargo clippy --workspace --all-targets --locked -- -D warnings - name: Test workspace + env: + # The runner has no AMD driver, so AMF must fail to open and the + # encode stage must fall back to libx264 (host/tests/amf_encoder.rs). + ETERNAL_AMF_TEST: fallback run: cargo test --workspace --locked - name: Test virtual display task scripts diff --git a/docs/beta-testing.md b/docs/beta-testing.md index fe2801c..d1c5563 100644 --- a/docs/beta-testing.md +++ b/docs/beta-testing.md @@ -213,6 +213,19 @@ option needs both color comparison and ten-minute NVENC/AMF runs before its baseline can change. AMF normalization, startup-IDR retries and forced-intra handling remain in place. +On a PC with an AMD GPU, run the AMF encoder tests from a source checkout +before signing off an AMD build: + +```powershell +$env:ETERNAL_AMF_TEST = "hardware" +cargo test -p eternal-host --locked --test amf_encoder +``` + +They open `h264_amf` and `hevc_amf` on the installed driver, request a +keyframe in the middle of a GOP, change the capture size mid-stream, and +decode every packet with FFmpeg. CI runs the same file with `fallback`. That +proves a PC without an AMD driver drops to libx264 instead of failing. + An amber software-fallback banner means a hardware encoder failed to open. Capture that error before changing drivers or settings. A fallback that produces video is not proof that the requested hardware encoder worked. diff --git a/host/src/control.rs b/host/src/control.rs index 1b1838d..8298a0e 100644 --- a/host/src/control.rs +++ b/host/src/control.rs @@ -22,9 +22,10 @@ pub struct SharedControl { pub encoder_input: Arc>, pub max_dgram: Arc, pub target_addr: Arc>, - /// Set by transport on iPad re-handshake (same target). Encoder - /// swaps it back to false on the next frame and forces an IDR. AMD only — NVENC - /// ignores it because NVENC keyframe cadence is already correct. + /// Set when the client needs a keyframe: a new session, a keyframe request + /// or frames dropped on a USB link. The encoder clears it on the next + /// frame and forces an IDR, reopening AMF sessions because AMF ignores + /// per-frame keyframe requests (see `encoder::ignores_forced_keyframes`). pub force_next_idr: Arc, /// Encoder name override from the Settings tab, applied on next pipeline restart. /// `None` means use auto-detected encoder from GpuInfo. diff --git a/host/src/encoder/mod.rs b/host/src/encoder/mod.rs index 99df1fa..ae693b1 100644 --- a/host/src/encoder/mod.rs +++ b/host/src/encoder/mod.rs @@ -131,6 +131,7 @@ fn run_encode_loop( // session changes (hevc_wanted going false resets it). let mut hevc_unavailable_warned = false; let mut hevc_open_failed = false; + let mut last_keyframe_reopen: Option = None; // Guarantee the very first frame of every pipeline is an IDR for all encoders, so the // iPad never sits in `waitingForSyncSample` after a connect or pipeline restart. @@ -204,6 +205,21 @@ fn run_encode_loop( } } + // Capture rebuilds at the new size after a display mode change (Radeon + // Super Resolution, VSR, GPU scaling, a resized virtual display). The + // open session's frame buffers no longer fit. + let size_changed = encoder_state + .as_ref() + .is_some_and(|state| !state.accepts_frame(raw_frame.width, raw_frame.height)); + let keyframe_reopen = shared.force_next_idr.load(Ordering::SeqCst) + && encoder_state.as_ref().is_some_and(|state| { + reopen_for_keyframe( + &state.opened_encoder, + state.frames_submitted, + last_keyframe_reopen.map(|at| at.elapsed()), + ) + }); + if encoder_state.is_none() { match EncoderState::new( &desired_encoder, @@ -255,16 +271,23 @@ fn run_encode_loop( } Err(e) => return Err(e), } - } else if encoder_state.as_ref().is_some_and(|state| { - state.opened_encoder != desired_encoder - || ((state.opened_bitrate != desired_bitrate || state.opened_fps != target_fps) - && failed_reopen != Some((desired_bitrate, target_fps))) - }) { - // Bitrate changed (ABR rung or the user's slider) or the - // negotiated codec changed (HEVC toggle / client capability): - // hardware encoders ignore parameter pokes on an open context, so - // either means a session reopen (~50-200ms hiccup, no transport - // teardown, same stream epoch) followed by a forced IDR. + } else if size_changed + || keyframe_reopen + || encoder_state.as_ref().is_some_and(|state| { + state.opened_encoder != desired_encoder + || ((state.opened_bitrate != desired_bitrate || state.opened_fps != target_fps) + && failed_reopen != Some((desired_bitrate, target_fps))) + }) + { + // Bitrate changed (ABR rung or the user's slider), the + // negotiated codec changed (HEVC toggle / client capability), the + // capture size changed (display mode change), or AMF owes a + // keyframe: hardware encoders ignore parameter pokes on an open + // context, so each means a session reopen (~50-200ms hiccup, no + // transport teardown, same stream epoch) followed by a forced IDR. + if keyframe_reopen { + last_keyframe_reopen = Some(Instant::now()); + } let previous = encoder_state .as_ref() .map(|s| s.opened_bitrate) @@ -282,6 +305,10 @@ fn run_encode_loop( from = previous, to = desired_bitrate, encoder = %desired_encoder, + width = raw_frame.width, + height = raw_frame.height, + size_changed, + keyframe = keyframe_reopen, "Reopened encoder session" ); encoder_state = Some(state); @@ -289,6 +316,16 @@ fn run_encode_loop( shared.force_next_idr.store(true, Ordering::SeqCst); note_encoder_opened(&shared, &desired_encoder); } + // The old session cannot take a frame of the new size. A + // pipeline restart opens a fresh encoder, with the software + // fallback if the hardware one rejects this size. + Err(e) if size_changed => { + return Err(format!( + "{desired_encoder} could not reopen at {}x{}: {e}", + raw_frame.width, raw_frame.height + ) + .into()); + } Err(e) if is_hevc_encoder(&desired_encoder) => { warn!( error = %e, @@ -378,6 +415,7 @@ fn run_encode_loop( let encode_start = Instant::now(); encoder.encoder.send_frame(&encoder.frame)?; + encoder.frames_submitted += 1; loop { match encoder.encoder.receive_packet(&mut encoder.packet) { @@ -419,9 +457,8 @@ fn run_encode_loop( &mut encoder.h264, &encoder.opened_encoder, ); - let is_keyframe = packet_is_key - || au_info.contains_idr - || (is_amf_encoder(&encoder.opened_encoder) && au_info.intra_only); + let is_keyframe = + au_info.is_keyframe(packet_is_key, &encoder.opened_encoder); (nal_data, is_keyframe) }; if !opened_is_hevc { @@ -510,6 +547,8 @@ struct EncoderState { /// Last applied bitrate. Hardware encoders reopen when this changes. opened_bitrate: u32, opened_fps: u32, + /// Frames sent to this session. Its first frame is always an IDR. + frames_submitted: u64, /// First frame's capture instant — the PTS epoch for this session. pts_epoch: Option, last_pts: i64, @@ -668,12 +707,17 @@ impl EncoderState { opened_encoder: encoder_name.to_string(), opened_bitrate: bitrate_bps, opened_fps: target_fps, + frames_submitted: 0, pts_epoch: None, last_pts: -1, legacy_pts, }) } + fn accepts_frame(&self, width: u32, height: u32) -> bool { + self.frame.width() == width && self.frame.height() == height + } + fn observe_amf_diagnostics( &mut self, normalized_packet: &[u8], @@ -703,6 +747,30 @@ pub fn is_hevc_encoder(encoder_name: &str) -> bool { encoder_name.starts_with("hevc_") || encoder_name == "libx265" } +/// True for encoders that cannot be asked for a keyframe on a given frame. +/// FFmpeg 7.1's AMF wrapper never reads `AVFrame::pict_type` (8.0 added that, +/// behind its `forced_idr` option), so AMF keeps encoding P-frames until its +/// own IDR period. Every new session starts with an IDR carrying its +/// parameter sets, so the encode loop reopens AMF to honour a request. +pub fn ignores_forced_keyframes(encoder_name: &str) -> bool { + matches!(encoder_name, "h264_amf" | "hevc_amf") +} + +/// Whether a pending keyframe request needs a session reopen: the encoder +/// ignores per-frame requests, the session has already sent its opening IDR, +/// and the last such reopen is older than the client's own request limit. +fn reopen_for_keyframe( + encoder_name: &str, + frames_submitted: u64, + since_last_reopen: Option, +) -> bool { + ignores_forced_keyframes(encoder_name) + && frames_submitted > 0 + && since_last_reopen.is_none_or(|elapsed| { + elapsed >= crate::transport::session::KEYFRAME_REQUEST_MIN_INTERVAL + }) +} + fn codec_display_name_for(encoder_name: &str) -> &str { match encoder_name { "h264_nvenc" => "H.264 (NVENC)", @@ -1371,6 +1439,48 @@ mod tests { assert_eq!(hevc_variant_of("mystery_codec"), None); } + #[test] + fn amf_keyframe_requests_reopen_the_session_at_most_twice_a_second() { + let long_ago = Some(std::time::Duration::from_secs(5)); + let just_now = Some(std::time::Duration::from_millis(100)); + for name in ["h264_amf", "hevc_amf"] { + assert!(reopen_for_keyframe(name, 1, None), "{name}"); + assert!(reopen_for_keyframe(name, 300, long_ago), "{name}"); + // A fresh session's first frame is already an IDR. + assert!(!reopen_for_keyframe(name, 0, None), "{name}"); + assert!(!reopen_for_keyframe(name, 300, just_now), "{name}"); + } + // These encoders turn AVFrame::pict_type into a keyframe themselves. + for name in ["h264_nvenc", "hevc_nvenc", "h264_qsv", "libx264", "libx265"] { + assert!(!reopen_for_keyframe(name, 300, None), "{name}"); + } + } + + #[test] + fn amf_options_keep_the_settings_the_ipad_decoder_needs() { + let h264 = encoder_options("h264_amf"); + for (key, value) in [ + ("usage", "lowlatency"), + ("profile", "constrained_baseline"), + ("level", "5.1"), + ("coder", "cavlc"), + ("aud", "0"), + ("header_spacing", "1"), + ("rc", "cbr"), + ] { + assert_eq!(h264.get(key), Some(value), "h264_amf {key}"); + } + let hevc = encoder_options("hevc_amf"); + for (key, value) in [ + ("usage", "lowlatency"), + ("aud", "0"), + ("header_insertion_mode", "idr"), + ("rc", "cbr"), + ] { + assert_eq!(hevc.get(key), Some(value), "hevc_amf {key}"); + } + } + #[test] fn hevc_detection_matches_the_variant_table() { for h264 in ["h264_nvenc", "h264_amf", "h264_qsv", "libx264"] { diff --git a/host/tests/amf_encoder.rs b/host/tests/amf_encoder.rs new file mode 100644 index 0000000..27f47ce --- /dev/null +++ b/host/tests/amf_encoder.rs @@ -0,0 +1,403 @@ +//! The real encode stage under AMD's AMF encoder, fed synthetic frames and +//! checked by decoding its output with FFmpeg's software decoders. +//! +//! `ETERNAL_AMF_TEST` says what the AMF cases must prove on this machine: +//! - unset: whatever the linked FFmpeg allows. Skips when it has no AMF. +//! - `hardware`: AMF must open. Use this on a PC with an AMD GPU. +//! - `fallback`: AMF is compiled in but cannot open, so the stage must fall +//! back to libx264. CI's Windows runner has no AMD driver. +//! +//! The mode-change cases also run on libx264 everywhere. Radeon Super +//! Resolution, Virtual Super Resolution and GPU scaling all change the +//! desktop mode under a running stream. + +use std::sync::atomic::Ordering; +use std::sync::{Arc, Mutex}; +use std::time::{Duration, Instant}; + +use eternal_host::capture::synthetic::{decode_frame_counter_from_luma, render_synthetic_frame}; +use eternal_host::capture::{frame_slot, RawFrame}; +use eternal_host::control::SharedControl; +use eternal_host::encoder::{run_encode_stage, NALUnit, NAL_CHANNEL_CAPACITY}; +use eternal_host::gpu::GpuInfo; +use eternal_host::stats::PIPELINE_STATS; +use eternal_wire::h264::{contains_nal_type, nal_type, parse_annex_b_units}; +use ffmpeg_next::codec::Id; + +/// The stage and PIPELINE_STATS are process-global, so stages run one at a time. +static STAGE_LOCK: Mutex<()> = Mutex::new(()); + +const FRAME_INTERVAL: Duration = Duration::from_millis(20); +const FRAME_COUNT: u64 = 90; +/// Mid-GOP. The encoder's own IDRs land on frames 0, 30 and 60. +const KEYFRAME_REQUEST_AT: u64 = 40; +/// How far a requested keyframe's picture may trail the request. The encoder +/// takes the picture before any session reopen, so reopen time does not count. +/// Anything inside this window still beats the next GOP IDR. +const KEYFRAME_WINDOW: u64 = 5; +const SMALL: (u32, u32) = (640, 360); +const LARGE: (u32, u32) = (1280, 720); + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum AmfExpectation { + Any, + Hardware, + Fallback, +} + +/// What this run must prove about AMF, or `None` to skip. +fn amf_expectation() -> Option { + ffmpeg_next::init().unwrap(); + let compiled = ffmpeg_next::encoder::find_by_name("h264_amf").is_some(); + let mode = std::env::var("ETERNAL_AMF_TEST").unwrap_or_default(); + let expectation = match mode.trim() { + "" => AmfExpectation::Any, + "hardware" => AmfExpectation::Hardware, + "fallback" => AmfExpectation::Fallback, + other => panic!("unknown ETERNAL_AMF_TEST={other}; use hardware or fallback"), + }; + if compiled { + Some(expectation) + } else { + assert_eq!( + expectation, + AmfExpectation::Any, + "ETERNAL_AMF_TEST={mode} needs an FFmpeg build with h264_amf" + ); + eprintln!("skipping: the linked FFmpeg has no h264_amf"); + None + } +} + +/// Whether an AMF encoder opens on this machine. Cases that need a working +/// AMF skip when it does not, except under `ETERNAL_AMF_TEST=hardware`. +fn amf_available(name: &str) -> bool { + let Some(expectation) = amf_expectation() else { + return false; + }; + let opens = amf_opens(name); + if expectation == AmfExpectation::Hardware { + assert!( + opens, + "{name} did not open; check the AMD driver and amfrt64.dll" + ); + } + if !opens { + eprintln!("skipping: {name} does not open on this machine"); + } + opens +} + +fn amf_opens(name: &str) -> bool { + let Some(codec) = ffmpeg_next::encoder::find_by_name(name) else { + return false; + }; + let Ok(mut encoder) = ffmpeg_next::codec::Context::new_with_codec(codec) + .encoder() + .video() + else { + return false; + }; + encoder.set_width(LARGE.0); + encoder.set_height(LARGE.1); + encoder.set_format(ffmpeg_next::format::Pixel::YUV420P); + encoder.set_time_base(ffmpeg_next::Rational(1, 60)); + encoder.open().is_ok() +} + +struct StageRun { + packets: Vec, + software_fallback: bool, +} + +/// Feeds `FRAME_COUNT` frames through a fresh encode stage at +/// `FRAME_INTERVAL`, sized by `size_of`, and raises a keyframe request just +/// before frame `keyframe_at`. Panics if the stage fails or panics. +fn run_stage( + encoder: &str, + size_of: impl Fn(u64) -> (u32, u32), + keyframe_at: Option, +) -> StageRun { + ffmpeg_next::init().unwrap(); + let (producer, consumer) = frame_slot(); + let (tx, mut rx) = tokio::sync::mpsc::channel(NAL_CHANNEL_CAPACITY); + let shared = SharedControl::new(0, 8_000_000); + shared.running.store(true, Ordering::SeqCst); + let gpu = GpuInfo { + encoder_name: encoder.to_string(), + codec_display_name: encoder.to_string(), + ..GpuInfo::software_fallback() + }; + let generation = eternal_host::supervisor::CURRENT_GENERATION.load(Ordering::SeqCst); + let stage_shared = shared.clone(); + let stage = std::thread::spawn(move || { + run_encode_stage(consumer, tx, stage_shared, gpu, generation).map_err(|e| e.to_string()) + }); + // The output channel holds only a few frames, so drain it as we go. + let collector = std::thread::spawn(move || { + let mut packets = Vec::new(); + while let Some(nal) = rx.blocking_recv() { + packets.push(nal); + } + packets + }); + + for number in 0..FRAME_COUNT { + if keyframe_at == Some(number) { + shared.force_next_idr.store(true, Ordering::SeqCst); + } + let (width, height) = size_of(number); + let mut data = vec![0u8; (width * height * 4) as usize]; + render_synthetic_frame(&mut data, width, height, number); + producer.publish(RawFrame { + frame_number: number, + timestamp: Instant::now(), + data: Arc::new(data), + width, + height, + }); + std::thread::sleep(FRAME_INTERVAL); + } + producer.close(); + + match stage.join() { + Ok(Ok(())) => {} + Ok(Err(error)) => panic!("{encoder} encode stage failed: {error}"), + Err(_) => panic!("{encoder} encode stage panicked"), + } + let packets = collector.join().unwrap(); + assert!(!packets.is_empty(), "{encoder} produced no packets"); + StageRun { + packets, + software_fallback: PIPELINE_STATS.lock().using_software_fallback, + } +} + +/// Decodes `packets` with a fresh, strict software decoder and returns each +/// picture's size and frame counter. Any decode error fails the test. +fn decode(codec: Id, packets: &[&NALUnit]) -> Vec<(u32, u32, u64)> { + let codec = ffmpeg_next::decoder::find(codec).unwrap(); + let mut options = ffmpeg_next::Dictionary::new(); + options.set("err_detect", "explode"); + let mut decoder = ffmpeg_next::codec::Context::new_with_codec(codec) + .decoder() + .open_as_with(codec, options) + .unwrap() + .video() + .unwrap(); + let mut frame = ffmpeg_next::frame::Video::empty(); + let mut pictures = Vec::new(); + let mut receive = |decoder: &mut ffmpeg_next::decoder::Video| { + while decoder.receive_frame(&mut frame).is_ok() { + let counter = + decode_frame_counter_from_luma(frame.data(0), frame.stride(0), frame.width()); + pictures.push((frame.width(), frame.height(), counter)); + } + }; + for packet in packets { + decoder + .send_packet(&ffmpeg_next::Packet::copy(&packet.data)) + .unwrap_or_else(|error| panic!("packet {} did not decode: {error}", packet.sequence)); + receive(&mut decoder); + } + decoder.send_eof().unwrap(); + receive(&mut decoder); + pictures +} + +fn assert_h264_keyframe(packet: &NALUnit, what: &str) { + assert!(packet.is_keyframe, "{what} is not flagged as a keyframe"); + for (kind, name) in [(7, "SPS"), (8, "PPS"), (5, "IDR slice")] { + assert!( + contains_nal_type(&packet.data, kind), + "{what} has no {name}" + ); + } +} + +fn assert_hevc_keyframe(packet: &NALUnit, what: &str) { + assert!(packet.is_keyframe, "{what} is not flagged as a keyframe"); + assert!( + eternal_wire::hevc::contains_keyframe(&packet.data), + "{what} has no IRAP slice" + ); + assert!( + eternal_wire::hevc::contains_parameter_sets(&packet.data), + "{what} lacks VPS/SPS/PPS" + ); +} + +/// A keyframe request in the middle of a GOP must produce a keyframe whose +/// picture is at most `KEYFRAME_WINDOW` frames past the request, and a decoder +/// that joins there must decode everything after it. +fn assert_keyframe_on_request(run: &StageRun, codec: Id) { + let keyframes: Vec = run + .packets + .iter() + .filter(|packet| packet.is_keyframe) + .map(|packet| packet.sequence) + .collect(); + let start = run + .packets + .iter() + .position(|packet| packet.is_keyframe && packet.sequence >= KEYFRAME_REQUEST_AT) + .unwrap_or_else(|| panic!("no keyframe after the request; keyframes at {keyframes:?}")); + let joined = &run.packets[start]; + match codec { + Id::H264 => assert_h264_keyframe(joined, "the requested keyframe"), + _ => assert_hevc_keyframe(joined, "the requested keyframe"), + } + let tail: Vec<&NALUnit> = run.packets[start..].iter().collect(); + let pictures = decode(codec, &tail); + assert!( + pictures.len() >= 10, + "only {} pictures decoded", + pictures.len() + ); + let counters: Vec = pictures.iter().map(|picture| picture.2).collect(); + let window = KEYFRAME_REQUEST_AT..=KEYFRAME_REQUEST_AT + KEYFRAME_WINDOW; + assert!( + window.contains(&counters[0]), + "the first keyframe after the request shows frame {}, so the request \ + waited for the encoder's own IDR; keyframes at {keyframes:?}", + counters[0] + ); + assert!( + counters.windows(2).all(|pair| pair[0] < pair[1]), + "frame counters do not advance: {counters:?}" + ); +} + +/// Size schedule for the mode-change runs: small, then large, then small. +fn mode_changes(number: u64) -> (u32, u32) { + if (30..60).contains(&number) { + LARGE + } else { + SMALL + } +} + +/// After each mode change the stream must restart on a keyframe at the new +/// size that a fresh decoder can join. +fn assert_follows_mode_changes(run: &StageRun) { + for (from, size) in [(30u64, LARGE), (60, SMALL)] { + let start = run + .packets + .iter() + .position(|packet| packet.sequence >= from) + .unwrap_or_else(|| panic!("no packets after the mode change at frame {from}")); + assert_h264_keyframe( + &run.packets[start], + &format!("the first packet after the mode change at frame {from}"), + ); + let segment: Vec<&NALUnit> = run.packets[start..] + .iter() + .take_while(|packet| packet.sequence < from + 30) + .collect(); + let pictures = decode(Id::H264, &segment); + assert!(!pictures.is_empty(), "nothing decoded after frame {from}"); + for (width, height, counter) in pictures { + assert_eq!( + (width, height), + size, + "frame {counter} decoded at the wrong size" + ); + } + } +} + +#[test] +fn software_encoder_follows_capture_mode_changes() { + let _guard = STAGE_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let run = run_stage("libx264", mode_changes, None); + assert_follows_mode_changes(&run); +} + +#[test] +fn software_encoder_honours_keyframe_requests() { + let _guard = STAGE_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let run = run_stage("libx264", |_| LARGE, Some(KEYFRAME_REQUEST_AT)); + assert_h264_keyframe(&run.packets[0], "the first packet"); + assert_keyframe_on_request(&run, Id::H264); +} + +#[test] +fn amf_honours_keyframe_requests() { + let Some(expectation) = amf_expectation() else { + return; + }; + let _guard = STAGE_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let run = run_stage("h264_amf", |_| LARGE, Some(KEYFRAME_REQUEST_AT)); + match expectation { + AmfExpectation::Hardware => assert!( + !run.software_fallback, + "AMF did not open; check the AMD driver and amfrt64.dll" + ), + AmfExpectation::Fallback => assert!( + run.software_fallback, + "AMF opened, so this machine cannot test the missing-driver fallback" + ), + AmfExpectation::Any => {} + } + eprintln!( + "h264_amf {}", + if run.software_fallback { + "could not open; checked the libx264 fallback" + } else { + "opened on this GPU" + } + ); + assert_h264_keyframe(&run.packets[0], "the first packet"); + assert_keyframe_on_request(&run, Id::H264); + + if !run.software_fallback { + // The iPad's VideoToolbox decoder depends on these h264_amf options. + let sps = parse_annex_b_units(&run.packets[0].data) + .into_iter() + .find(|unit| nal_type(unit) == Some(7)) + .unwrap(); + assert_eq!(sps[1], 66, "profile is not baseline"); + assert_ne!(sps[2] & 0x40, 0, "baseline profile is not constrained"); + assert!(sps[3] >= 51, "level {} is below 5.1", sps[3]); + assert!( + run.packets + .iter() + .all(|packet| !contains_nal_type(&packet.data, 9)), + "AMF emitted access unit delimiters" + ); + } +} + +#[test] +fn amf_follows_capture_mode_changes() { + if !amf_available("h264_amf") { + return; + } + let _guard = STAGE_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let run = run_stage("h264_amf", mode_changes, None); + assert!(!run.software_fallback, "AMF fell back to libx264"); + assert_follows_mode_changes(&run); +} + +#[test] +fn amf_hevc_honours_keyframe_requests() { + if !amf_available("hevc_amf") { + return; + } + let _guard = STAGE_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let run = run_stage("hevc_amf", |_| LARGE, Some(KEYFRAME_REQUEST_AT)); + assert_hevc_keyframe(&run.packets[0], "the first packet"); + assert_keyframe_on_request(&run, Id::HEVC); +} + +#[test] +fn amf_fails_cleanly_without_an_amd_driver() { + if amf_expectation() != Some(AmfExpectation::Fallback) { + return; + } + // Without an AMD driver, both AMF encoders must fail to open rather than + // hang or crash. The HEVC one is chosen at runtime for HEVC clients. + for name in ["h264_amf", "hevc_amf"] { + assert!(!amf_opens(name), "{name} opened without an AMD driver"); + } +} diff --git a/proto/src/h264.rs b/proto/src/h264.rs index a6812a5..d4d0704 100644 --- a/proto/src/h264.rs +++ b/proto/src/h264.rs @@ -88,6 +88,15 @@ pub struct AuInfo { pub intra_only: bool, } +impl AuInfo { + /// Whether the access unit goes out flagged as a keyframe. AMF's + /// all-intra access units without an IDR NAL count too: they carry + /// SPS/PPS after normalization and the iPad starts decoding on them. + pub fn is_keyframe(&self, packet_is_key: bool, encoder_name: &str) -> bool { + packet_is_key || self.contains_idr || (is_amf_encoder(encoder_name) && self.intra_only) + } +} + /// Rewrites one encoded packet into clean Annex B, injecting cached SPS/PPS /// according to the per-encoder rules the iPad decoder depends on. pub fn normalize_h264_payload( @@ -796,6 +805,35 @@ mod tests { } } + #[test] + fn amf_intra_access_units_go_out_as_keyframes() { + let extradata = [ + 1, 66, 0, 30, 0xFF, 0xE1, 0x00, 0x04, 0x67, 0x42, 0x00, 0x1E, 0x01, 0x00, 0x02, 0x68, + 0xCE, + ]; + let intra = [0, 0, 0, 1, 0x41, 0xB8]; + let inter = [0, 0, 0, 1, 0x41, 0xE0]; + let idr = [0, 0, 0, 1, 0x65, 0x88, 0x84]; + let keyframe = |payload: &[u8], packet_is_key: bool, encoder: &str| { + let mut state = H264BitstreamState::default(); + state.refresh_parameter_sets_from_extradata(Some(extradata.to_vec())); + state.packet_count = 90; + let (_, info) = + normalize_h264_payload_with_info(payload, packet_is_key, &mut state, encoder); + info.is_keyframe(packet_is_key, encoder) + }; + + assert!(keyframe(&intra, false, "h264_amf")); + assert!(!keyframe(&inter, false, "h264_amf")); + assert!(keyframe(&idr, false, "h264_amf")); + // Other encoders keep the strict IDR rule, and packet flags still count. + assert!(!keyframe(&intra, false, "h264_nvenc")); + assert!(keyframe(&idr, false, "h264_nvenc")); + assert!(keyframe(&inter, true, "libx264")); + // HEVC goes through its own path, so the H.264 AMF rules stop at h264_amf. + assert!(!is_amf_encoder("hevc_amf")); + } + #[test] fn rejects_predicted_access_unit_as_random_access() { let payload = [0, 0, 0, 1, 0x09, 0x30, 0, 0, 0, 1, 0x41, 0xE0]; From 6091bf7bb1c95ef9cdeb6997a9736e8fdc7a6925 Mon Sep 17 00:00:00 2001 From: Alitleis123 Date: Sun, 11 Oct 2026 18:35:23 -0400 Subject: [PATCH 2/2] Preserve deferred AMF keyframe requests and strengthen decoding checks Keep requests pending through the AMF reopen rate limit and failed reopen attempts. Test the limit boundary and both codec paths, strictly validate every output picture, reject broken streams, and cover HEVC resolution changes. Document the remaining driver and iPad hardware checks. Apply the existing rustfmt correction so workspace formatting passes. --- docs/beta-testing.md | 9 +++++ host/src/encoder/mod.rs | 56 +++++++++++++++++++++++++++--- host/src/updates.rs | 3 +- host/tests/amf_encoder.rs | 73 ++++++++++++++++++++++++++++++++------- 4 files changed, 123 insertions(+), 18 deletions(-) diff --git a/docs/beta-testing.md b/docs/beta-testing.md index d1c5563..860464c 100644 --- a/docs/beta-testing.md +++ b/docs/beta-testing.md @@ -226,6 +226,15 @@ keyframe in the middle of a GOP, change the capture size mid-stream, and decode every packet with FFmpeg. CI runs the same file with `fallback`. That proves a PC without an AMD driver drops to libx264 instead of failing. +With `ETERNAL_AMF_TEST` unset, machines whose FFmpeg has no AMF skip the +hardware cases. Use `hardware` for AMD sign-off and retain the GPU model, +AMD driver version, Windows build and FFmpeg version with the test output. +These synthetic-frame tests exercise the encoder. Run the existing +`R-amf-h264` and `R-amf-hevc` reference-PC rows for desktop capture and iPad +decoding, and check reconnects on an idle desktop over both Wi-Fi and USB. +Also resize the captured display up and down during a stream, then test +lock/unlock and display sleep/wake for capture recovery on that driver. + An amber software-fallback banner means a hardware encoder failed to open. Capture that error before changing drivers or settings. A fallback that produces video is not proof that the requested hardware encoder worked. diff --git a/host/src/encoder/mod.rs b/host/src/encoder/mod.rs index ae693b1..47e93fd 100644 --- a/host/src/encoder/mod.rs +++ b/host/src/encoder/mod.rs @@ -132,6 +132,7 @@ fn run_encode_loop( let mut hevc_unavailable_warned = false; let mut hevc_open_failed = false; let mut last_keyframe_reopen: Option = None; + let mut pending_keyframe = false; // Guarantee the very first frame of every pipeline is an IDR for all encoders, so the // iPad never sits in `waitingForSyncSample` after a connect or pipeline restart. @@ -211,7 +212,10 @@ fn run_encode_loop( let size_changed = encoder_state .as_ref() .is_some_and(|state| !state.accepts_frame(raw_frame.width, raw_frame.height)); - let keyframe_reopen = shared.force_next_idr.load(Ordering::SeqCst) + // Latch requests before deciding whether to reopen. Requests received + // during the rate limit must survive until AMF can emit an IDR. + pending_keyframe |= shared.force_next_idr.swap(false, Ordering::SeqCst); + let keyframe_reopen = pending_keyframe && encoder_state.as_ref().is_some_and(|state| { reopen_for_keyframe( &state.opened_encoder, @@ -313,7 +317,7 @@ fn run_encode_loop( ); encoder_state = Some(state); failed_reopen = None; - shared.force_next_idr.store(true, Ordering::SeqCst); + pending_keyframe = true; note_encoder_opened(&shared, &desired_encoder); } // The old session cannot take a frame of the new size. A @@ -403,8 +407,12 @@ fn run_encode_loop( pts }; encoder.frame.set_pts(Some(pts)); - let force_idr = shared.force_next_idr.swap(false, Ordering::SeqCst) - || test_idr_period.is_some_and(|period| raw_frame.frame_number.is_multiple_of(period)); + let force_idr = take_keyframe_request( + &mut pending_keyframe, + &encoder.opened_encoder, + encoder.frames_submitted, + ) || test_idr_period + .is_some_and(|period| raw_frame.frame_number.is_multiple_of(period)); let _forced_intra = prepare_frame_for_encode( &mut encoder.frame, raw_frame.frame_number, @@ -771,6 +779,15 @@ fn reopen_for_keyframe( }) } +/// AMF can satisfy a request only on a fresh session's first frame. Keep it +/// pending while a reopen is rate limited or has failed. +fn take_keyframe_request(pending: &mut bool, encoder_name: &str, frames_submitted: u64) -> bool { + if ignores_forced_keyframes(encoder_name) && frames_submitted > 0 { + return false; + } + std::mem::take(pending) +} + fn codec_display_name_for(encoder_name: &str) -> &str { match encoder_name { "h264_nvenc" => "H.264 (NVENC)", @@ -1456,6 +1473,37 @@ mod tests { } } + #[test] + fn amf_keeps_keyframe_requests_pending_until_a_fresh_session() { + for name in ["h264_amf", "hevc_amf"] { + let mut pending = true; + for millis in [0, 100, 499] { + assert!(!reopen_for_keyframe( + name, + 10, + Some(std::time::Duration::from_millis(millis)) + )); + assert!(!take_keyframe_request(&mut pending, name, 10)); + assert!(pending, "{name} lost the deferred request"); + } + assert!(reopen_for_keyframe( + name, + 10, + Some(std::time::Duration::from_millis(500)) + )); + // A failed reopen leaves the old session in place. + assert!(!take_keyframe_request(&mut pending, name, 10)); + assert!(pending); + // A successful reopen starts its frame counter at zero. + assert!(take_keyframe_request(&mut pending, name, 0)); + assert!(!pending); + assert!(!take_keyframe_request(&mut pending, name, 0)); + } + let mut pending = true; + assert!(take_keyframe_request(&mut pending, "libx264", 10)); + assert!(!pending); + } + #[test] fn amf_options_keep_the_settings_the_ipad_decoder_needs() { let h264 = encoder_options("h264_amf"); diff --git a/host/src/updates.rs b/host/src/updates.rs index 9e70417..646752c 100644 --- a/host/src/updates.rs +++ b/host/src/updates.rs @@ -8,7 +8,8 @@ use parking_lot::Mutex; use serde::{Deserialize, Serialize}; const CHECK_INTERVAL: u64 = 24 * 60 * 60; -const LATEST_URL: &str = "https://api.github.com/repos/sidebandstudio/EternalMonitor/releases/latest"; +const LATEST_URL: &str = + "https://api.github.com/repos/sidebandstudio/EternalMonitor/releases/latest"; static CHECK_RUNNING: AtomicBool = AtomicBool::new(false); pub type AvailableUpdate = Arc>>; diff --git a/host/tests/amf_encoder.rs b/host/tests/amf_encoder.rs index 27f47ce..ec4a573 100644 --- a/host/tests/amf_encoder.rs +++ b/host/tests/amf_encoder.rs @@ -187,11 +187,18 @@ fn decode(codec: Id, packets: &[&NALUnit]) -> Vec<(u32, u32, u64)> { .unwrap(); let mut frame = ffmpeg_next::frame::Video::empty(); let mut pictures = Vec::new(); - let mut receive = |decoder: &mut ffmpeg_next::decoder::Video| { - while decoder.receive_frame(&mut frame).is_ok() { - let counter = - decode_frame_counter_from_luma(frame.data(0), frame.stride(0), frame.width()); - pictures.push((frame.width(), frame.height(), counter)); + let mut receive = |decoder: &mut ffmpeg_next::decoder::Video| loop { + match decoder.receive_frame(&mut frame) { + Ok(()) => { + let counter = + decode_frame_counter_from_luma(frame.data(0), frame.stride(0), frame.width()); + pictures.push((frame.width(), frame.height(), counter)); + } + Err(ffmpeg_next::Error::Eof) => break, + Err(ffmpeg_next::Error::Other { errno }) if errno == ffmpeg_next::error::EAGAIN => { + break; + } + Err(error) => panic!("decoder rejected an access unit: {error}"), } }; for packet in packets { @@ -202,6 +209,7 @@ fn decode(codec: Id, packets: &[&NALUnit]) -> Vec<(u32, u32, u64)> { } decoder.send_eof().unwrap(); receive(&mut decoder); + assert_eq!(pictures.len(), packets.len(), "decoder lost pictures"); pictures } @@ -231,6 +239,7 @@ fn assert_hevc_keyframe(packet: &NALUnit, what: &str) { /// picture is at most `KEYFRAME_WINDOW` frames past the request, and a decoder /// that joins there must decode everything after it. fn assert_keyframe_on_request(run: &StageRun, codec: Id) { + decode(codec, &run.packets.iter().collect::>()); let keyframes: Vec = run .packets .iter() @@ -279,22 +288,24 @@ fn mode_changes(number: u64) -> (u32, u32) { /// After each mode change the stream must restart on a keyframe at the new /// size that a fresh decoder can join. -fn assert_follows_mode_changes(run: &StageRun) { +fn assert_follows_mode_changes(run: &StageRun, codec: Id) { + decode(codec, &run.packets.iter().collect::>()); for (from, size) in [(30u64, LARGE), (60, SMALL)] { let start = run .packets .iter() .position(|packet| packet.sequence >= from) .unwrap_or_else(|| panic!("no packets after the mode change at frame {from}")); - assert_h264_keyframe( - &run.packets[start], - &format!("the first packet after the mode change at frame {from}"), - ); + let what = format!("the first packet after the mode change at frame {from}"); + match codec { + Id::H264 => assert_h264_keyframe(&run.packets[start], &what), + _ => assert_hevc_keyframe(&run.packets[start], &what), + } let segment: Vec<&NALUnit> = run.packets[start..] .iter() .take_while(|packet| packet.sequence < from + 30) .collect(); - let pictures = decode(Id::H264, &segment); + let pictures = decode(codec, &segment); assert!(!pictures.is_empty(), "nothing decoded after frame {from}"); for (width, height, counter) in pictures { assert_eq!( @@ -310,7 +321,7 @@ fn assert_follows_mode_changes(run: &StageRun) { fn software_encoder_follows_capture_mode_changes() { let _guard = STAGE_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let run = run_stage("libx264", mode_changes, None); - assert_follows_mode_changes(&run); + assert_follows_mode_changes(&run, Id::H264); } #[test] @@ -321,6 +332,31 @@ fn software_encoder_honours_keyframe_requests() { assert_keyframe_on_request(&run, Id::H264); } +#[test] +fn strict_decoder_rejects_broken_access_units() { + let _guard = STAGE_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let run = run_stage("libx264", |_| SMALL, None); + let predicted = run + .packets + .iter() + .position(|packet| !packet.is_keyframe) + .expect("the stream has no predicted frames"); + let tail: Vec<&NALUnit> = run.packets[predicted..].iter().collect(); + assert!( + std::panic::catch_unwind(|| decode(Id::H264, &tail)).is_err(), + "a decoder joining without a keyframe accepted the stream" + ); + + let mut broken = run.packets[predicted].clone(); + broken.data = vec![0, 0, 0, 1, 0x65, 0xff]; + let mut packets: Vec<&NALUnit> = run.packets.iter().collect(); + packets[predicted] = &broken; + assert!( + std::panic::catch_unwind(|| decode(Id::H264, &packets)).is_err(), + "a decoder accepted a corrupt slice" + ); +} + #[test] fn amf_honours_keyframe_requests() { let Some(expectation) = amf_expectation() else { @@ -376,7 +412,18 @@ fn amf_follows_capture_mode_changes() { let _guard = STAGE_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let run = run_stage("h264_amf", mode_changes, None); assert!(!run.software_fallback, "AMF fell back to libx264"); - assert_follows_mode_changes(&run); + assert_follows_mode_changes(&run, Id::H264); +} + +#[test] +fn amf_hevc_follows_capture_mode_changes() { + if !amf_available("hevc_amf") { + return; + } + let _guard = STAGE_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let run = run_stage("hevc_amf", mode_changes, None); + assert!(!run.software_fallback, "AMF fell back to libx264"); + assert_follows_mode_changes(&run, Id::HEVC); } #[test]