Skip to content

Commit 51cd3fc

Browse files
authored
fix(amf): disable LTR by default to match FFmpeg amfenc behavior (#631)
* fix(amf): disable LTR by default to match FFmpeg amfenc behavior PR #630 enabled native AMF LTR with 4 slots by default to improve weak-network RFI recovery, but this introduced a regression: static screen regions retain encoder artifacts (color blocks) until motion forces a refresh. Root cause: with LTR_MODE_RESET_UNUSED + ULTRA_LOW_LATENCY usage, the IDR-time LTR baseline (slot 0) is permanently referenced by P-frames for static macroblocks. The slot-0 quantization noise from IDR is never overwritten, so static regions inherit it forever; only motion forces a fresh intra block. FFmpeg's libavcodec/amfenc.c does NOT use the native AMF LTR API at all (it only optionally uses Pre-Analysis LTR which is a different mechanism). That is why the legacy FFmpeg-AMF path never exhibited this artifact. Fix: revert default amd_ltr_frames to 0 so out-of-the-box behavior matches FFmpeg. Users who actually need RFI on lossy networks can opt in via amd_ltr_frames>=1; the slot rotation logic from #630 still applies in that case. Refs #630 * docs(amf): explain LTR opt-in trade-off and dead-code rationale Add inline comments at the three LTR enable sites and the effective_ltr_slots calc explaining: (1) why we keep the implementation despite default-disabled, (2) the IDR storm risk if RFI is removed entirely, (3) the static-region color-block trade-off when users opt in.
1 parent 31f9a0d commit 51cd3fc

3 files changed

Lines changed: 32 additions & 5 deletions

File tree

src/amf/amf_config.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,11 @@ namespace amf {
5555
// Enforce HRD
5656
std::optional<int> enforce_hrd;
5757

58-
// Number of LTR frames for RFI
59-
int max_ltr_frames = 4;
58+
// Number of LTR frames for RFI (0 = disabled, matches FFmpeg amfenc behavior).
59+
// When enabled, static screen regions may retain encoder artifacts from the
60+
// baseline LTR frame until motion forces a refresh; only opt in when the
61+
// network actually needs reference-frame invalidation recovery.
62+
int max_ltr_frames = 0;
6063

6164
// --- Pre-Analysis sub-system ---
6265
// PAQ mode (AMF_PA_PAQ_MODE_ENUM): 0=none, 1=CAQ

src/amf/amf_d3d11.cpp

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,23 @@ namespace amf {
152152
encoder->SetProperty(AMF_VIDEO_ENCODER_INPUT_QUEUE_SIZE, (amf_int64) 1);
153153
encoder->SetProperty(AMF_VIDEO_ENCODER_QUERY_TIMEOUT, (amf_int64) 1);
154154

155-
// LTR for RFI
155+
// LTR for RFI (Reference Frame Invalidation, weak-network recovery).
156+
//
157+
// Disabled by default (max_ltr_frames == 0) to match FFmpeg amfenc behavior:
158+
// FFmpeg's libavcodec/amfenc.c never sets MAX_LTR_FRAMES / LTR_MODE, so static
159+
// screen regions are not pinned to a baseline LTR frame and never accumulate
160+
// color-block artifacts.
161+
//
162+
// Trade-off when the user opts in (amd_ltr_frames >= 1):
163+
// + On lossy links, client-side reference invalidation can recover by
164+
// sending a P-frame referencing a known-good LTR slot instead of a full
165+
// IDR. IDRs are 10-20x larger than P-frames and themselves more likely
166+
// to be lost on weak networks, which can cascade into an "IDR storm".
167+
// - Static regions may inherit the IDR-time quantization noise of the
168+
// baseline LTR slot until motion forces a fresh intra block.
169+
//
170+
// The slot rotation / IDR-baseline preservation logic below (PR #630) only
171+
// takes effect when LTR is opted in.
156172
max_ltr_frames = config.max_ltr_frames;
157173
if (max_ltr_frames > 0) {
158174
encoder->SetProperty(AMF_VIDEO_ENCODER_MAX_LTR_FRAMES, (amf_int64) max_ltr_frames);
@@ -217,6 +233,8 @@ namespace amf {
217233
encoder->SetProperty(AMF_VIDEO_ENCODER_HEVC_PROFILE, (amf_int64) AMF_VIDEO_ENCODER_HEVC_PROFILE_MAIN);
218234
}
219235

236+
// LTR for RFI - see H.264 block above for detailed trade-off rationale.
237+
// Disabled by default; opt-in via amd_ltr_frames config.
220238
max_ltr_frames = config.max_ltr_frames;
221239
if (max_ltr_frames > 0) {
222240
encoder->SetProperty(AMF_VIDEO_ENCODER_HEVC_MAX_LTR_FRAMES, (amf_int64) max_ltr_frames);
@@ -298,6 +316,8 @@ namespace amf {
298316
encoder->SetProperty(AMF_VIDEO_ENCODER_AV1_AQ_MODE, (amf_int64) *config.pa_paq_mode);
299317
}
300318

319+
// LTR for RFI - see H.264 block above for detailed trade-off rationale.
320+
// Disabled by default; opt-in via amd_ltr_frames config.
301321
max_ltr_frames = config.max_ltr_frames;
302322
if (max_ltr_frames > 0) {
303323
encoder->SetProperty(AMF_VIDEO_ENCODER_AV1_MAX_LTR_FRAMES, (amf_int64) max_ltr_frames);
@@ -595,7 +615,11 @@ namespace amf {
595615

596616

597617

598-
// Clamp effective LTR slots to what the encoder actually reserves
618+
// Clamp effective LTR slots to what the encoder actually reserves.
619+
// When max_ltr_frames == 0 (default), the entire LTR/RFI subsystem becomes
620+
// a no-op: the IDR baseline marking, slot rotation, and invalidate handling
621+
// below all gate on `effective_ltr_slots > 0`. The fallback for client-side
622+
// invalidate_ref_frames in this case is force_idr=true (see video.cpp).
599623
effective_ltr_slots = (max_ltr_frames > 0) ? std::min(max_ltr_frames, MAX_LTR_SLOTS) : 0;
600624

601625
// Reset LTR state

src/config.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ namespace config {
7373
std::optional<int> amd_vbaq;
7474
int amd_coder;
7575
int amd_qvbr_quality = 23; // QVBR quality level 1-51 (lower=better, default=23)
76-
int amd_ltr_frames = 4; // LTR frames for RFI (0=disabled, default=4)
76+
int amd_ltr_frames = 0; // LTR frames for RFI (0=disabled by default; matches FFmpeg amfenc behavior to avoid static-region color blocks)
7777
int amd_slices_per_frame = 0; // Slices/tiles per frame (0=client decides, 1-4=minimum)
7878
} amd;
7979

0 commit comments

Comments
 (0)