Fix AMD keyframe recovery and test encoder mode changes

.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

docs/beta-testing.md

@@ -213,6 +213,28 @@ 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.
+
+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.

host/src/control.rs

@@ -22,9 +22,10 @@ pub struct SharedControl {
     pub encoder_input: Arc<Mutex<crate::encoder::input::EncoderInput>>,
     pub max_dgram: Arc<AtomicU32>,
     pub target_addr: Arc<Mutex<PeerId>>,
-    /// 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<AtomicBool>,
     /// Encoder name override from the Settings tab, applied on next pipeline restart.
     /// `None` means use auto-detected encoder from GpuInfo.

host/src/encoder/mod.rs

@@ -131,6 +131,8 @@ 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<Instant> = 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.
@@ -204,6 +206,24 @@ 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));
+        // 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,
+                    state.frames_submitted,
+                    last_keyframe_reopen.map(|at| at.elapsed()),
+                )
+            });
+
         if encoder_state.is_none() {
             match EncoderState::new(
                 &desired_encoder,
@@ -255,16 +275,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,13 +309,27 @@ 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);
                     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
+                // 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,
@@ -366,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,
@@ -378,6 +423,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 +465,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 +555,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<std::time::Instant>,
     last_pts: i64,
@@ -668,12 +715,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 +755,39 @@ 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<std::time::Duration>,
+) -> bool {
+    ignores_forced_keyframes(encoder_name)
+        && frames_submitted > 0
+        && since_last_reopen.is_none_or(|elapsed| {
+            elapsed >= crate::transport::session::KEYFRAME_REQUEST_MIN_INTERVAL
+        })
+}
+
+/// 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)",
@@ -1371,6 +1456,79 @@ 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_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");
+        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"] {

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<Mutex<Option<String>>>;