Add periodic intra refreshes to mitigate HEVC errors - #274
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a host-configurable IDR interval setting, persisted to state.json, exposed in the Host Settings UI (checkbox + slider), threaded into StreamConfiguration, and used by FFmpegDecoder to request periodic IDR frames based on decoded-frame counting. ChangesPeriodic IDR Frame Request Support
Sequence Diagram(s)sequenceDiagram
participant User as User/UI
participant Page as HostSettingsPage
participant Host as MoonlightHost
participant App as AppPage
participant Client as MoonlightClient
participant Decoder as FFmpegDecoder
User->>Page: Configure periodic refresh (checkbox/slider)
Page->>Host: Save IdrInterval
App->>Host: Read host settings on connect
App->>Client: StartStreaming(streamConfig with idrInterval)
Client->>Decoder: CompleteInitialization(..., idrInterval)
loop During decode
Decoder->>Decoder: count frames
alt count >= fps * idrInterval
Decoder->>Client: LiRequestIdrFrame()
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds a user-configurable “periodic decoder refresh” feature that requests IDR frames at a fixed interval to mitigate HEVC artifact accumulation on Xbox (issue #190).
Changes:
- Plumbs a new
idrIntervalsetting from host settings UI → stream configuration → FFmpeg decoder initialization. - Implements periodic
LiRequestIdrFrame()calls based on decoded-frame counting. - Persists the new setting in
state.jsonviaidr_interval.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Streaming/FFmpegDecoder.h | Extends decoder initialization API and stores IDR interval state. |
| Streaming/FFmpegDecoder.cpp | Implements periodic IDR requests during decode based on interval. |
| State/StreamConfiguration.h | Adds idrInterval to stream configuration object. |
| State/MoonlightHost.h | Adds host-level IdrInterval property for settings/UI binding. |
| State/MoonlightClient.cpp | Passes configured interval into decoder initialization before starting connection. |
| State/ApplicationState.cpp | Loads/saves idr_interval in persisted host state JSON. |
| Pages/HostSettingsPage.xaml.h | Adds checkbox event handler declarations for the new setting. |
| Pages/HostSettingsPage.xaml.cpp | Initializes UI state and toggles slider enabled/min/value for interval. |
| Pages/HostSettingsPage.xaml | Adds UI controls (checkbox + slider) to configure the interval. |
| Pages/AppPage.xaml.cpp | Copies host setting into the stream configuration used to start streaming. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Pages/HostSettingsPage.xaml.cpp`:
- Around line 125-132: The page initialization sets
EnableIdrIntervalCheckbox->IsChecked = true which triggers the checkbox Checked
handler and overwrites the saved host->IdrInterval (causing
IdrIntervalSlider->Value to become 10); fix by initializing the slider from
host->IdrInterval and preventing the Checked/Unchecked handler from firing
during navigation—either (a) set IdrIntervalSlider->Value = host->IdrInterval
(and IdrIntervalSlider->IsEnabled = host->IdrInterval > 0) before setting
EnableIdrIntervalCheckbox->IsChecked, or (b) temporarily unsubscribe the
checkbox event handlers (e.g.,
EnableIdrIntervalCheckbox_Checked/EnableIdrIntervalCheckbox_Unchecked) while you
set IsChecked and Value, then re-subscribe them afterwards.
In `@State/MoonlightHost.h`:
- Around line 289-296: The IdrInterval setter currently stores raw input into
the field idrInterval; update it to normalize/clamp inputs so only 0 (disabled)
or values >= 2 (enabled) are persisted: if the incoming value is less than 2,
set idrInterval to 0, otherwise set it to the incoming value; then call
OnPropertyChanged("IdrInterval") only after assigning the (possibly normalized)
value. Reference: property IdrInterval, backing field idrInterval, and
OnPropertyChanged.
In `@Streaming/FFmpegDecoder.cpp`:
- Around line 283-295: The periodic IDR-request logic should only run for HEVC
streams and when fps is a valid positive value; update the block that uses
idrInterval, m_FramesSinceIDR, fps, frame->pict_type and LiRequestIdrFrame() to
first verify the codec is HEVC (e.g., codec id or codec name check) and that fps
> 0 before computing (fps * idrInterval), and only then increment/reset
m_FramesSinceIDR and call LiRequestIdrFrame(); this prevents firing on every
frame when fps is unset or for non-HEVC streams.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cf6b8b27-9f8f-4e77-af82-0c61a51ac746
📒 Files selected for processing (10)
Pages/AppPage.xaml.cppPages/HostSettingsPage.xamlPages/HostSettingsPage.xaml.cppPages/HostSettingsPage.xaml.hState/ApplicationState.cppState/MoonlightClient.cppState/MoonlightHost.hState/StreamConfiguration.hStreaming/FFmpegDecoder.cppStreaming/FFmpegDecoder.h
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
The quick menu option Reset Decoder lets the user fix this as needed, this patch wouldn't be any better because there's no way to detect when the issue occurs. |
|
Hard disagree, Moonlight on the Xbox is literally unusable for me and probably most other people in #190 without this. Sure I can reset the decoder from the quick menu but I would have to do that every 2-3 minutes, if not even more often. So unless someone can figure out why this happens, this is the only way I can use Moonlight on my Xbox at all. I also made it a toggle so it's completely optional. |
|
There's something wrong with the AMD encoder that makes this worse. If sending large keyframes on a regular basis is really the only solution, this can live on the server side with an option to run the encoder more like a traditional encoder: regular IDR keyframes, higher latency. There is also some work on an AMF encoder but it's limited to the Vibe* forks at the moment, and also doesn't have working intra-refresh. BTW I would title this PR, "request periodic IDR frames", this isn't using intra-refresh. |
|
Hi @razorlikes , I contacted @andygrundman about a week ago and he advised me to look at the host implementation. I took the most recent Vibeshine as the basis and with the help of Google I came up with 2 inserts for amf_d3d11.cpp. My prototype now works perfectly for me, 4k HDR 60fps, real AMF GDR intra refresh is at work and no additional stutters have been introduced. The dreaded Xbox artefacts in slow moving scenes are gone. |
|
If the version of the amf encoder that got merged into Vibe* doesn't work for you, and you have a fix, I'd encourage you to submit a PR to that project. Or at least open an issue about it. |
|
I opened an in issue for Vibeshine with all the details, let's see if someone wants to pick it up. |
Fix/Workaround for #190.
Requests IDR frames periodically to get rid of the video artifacts that keep accumulating when using HEVC.
Tested on my Series S against Apollo with a 9070 XT.
Summary by CodeRabbit