Add setting to show nothing when the stream ends instead of freezing the last frame - #7
Conversation
The last decoded frame stayed frozen on screen after a disconnect, for the whole reconnect loop and indefinitely if the stream never returned. irl_handle_stream_read_error() faded out the audio and flushed its buffer but never touched video. The only path that cleared the frame was hide/deactivate, which returns early unless Close Stream When Inactive is on, so with it off a scene switch did not clear it either. Port OBS's own media source behaviour: clear_on_media_end there, "Show Nothing When the Stream Ends" here, likewise on by default and hot-swappable. It gates every path where the stream stops: the disconnect in receiver-stream.c, hide/deactivate, and a restart-forcing settings edit (which is decided against the config just installed, not the one being replaced). The no-URL case still clears unconditionally, since there is no stream for the frame to have come from. The clear runs on the video thread rather than the receiver thread. Calling obs_source_output_video(NULL) from the receiver would race a frame already inside format conversion, and that frame would repaint the frozen image right after the clear. Instead the receiver drops the queue and raises video_clear_pending under video_queue_lock, and the video thread re-checks it after each output. Note this means a transient read error now blanks the source for the reconnect delay instead of holding the last frame, which is exactly what the media source does.
|
Warning Review limit reached
Next review available in: 50 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
WalkthroughThe change adds ChangesStream-End Video Clearing
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Receiver as receiver-stream.c
participant Request as irl_video_request_clear
participant Video as receiver-video.c
participant Output as Video output
Receiver->>Request: Request clear after stream teardown
Request->>Video: Set pending flag and signal thread
Video->>Output: Output NULL frame
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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.
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 `@README.md`:
- Line 90: Qualify the README statement about hot-setting changes so it does not
claim that Close Stream When Inactive always keeps the connection and statistics
active. Limit the no-reconnect and continuous-counting behavior to settings that
do not intentionally stop an inactive source, or explicitly document the
receiver stop and statistics reset exception.
In `@src/irl-source.c`:
- Around line 515-522: Update the restart-clear logic in irl_source_update to
request clearing through irl_video_request_clear() before starting the
replacement receiver, ensuring the remaining video queue is drained before the
new thread runs. Avoid invoking clear_async_video() directly from the settings
update callback or otherwise ensure its NULL-frame output occurs only after the
queue is drained.
In `@src/settings.c`:
- Around line 106-109: Update src/settings.c lines 106-109 to state that Close
Stream When Inactive clears the source only when Show Nothing When the Stream
Ends (clear_on_disconnect) is enabled; update CLAUDE.md line 82 to remove the
unconditional wording that the frame is cleared to black.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 94f0c946-497c-497b-8874-74e3d2ba6dcc
📒 Files selected for processing (8)
CLAUDE.mdREADME.mdinclude/irl-source.hsrc/irl-source.csrc/receiver-internal.hsrc/receiver-stream.csrc/receiver-video.csrc/settings.c
The restart-forcing path in irl_source_update() cleared after start_receiver(), so the NULL frame could in principle land after the replacement stream delivered its first one and blank a live picture. Move it ahead of the restart. Also correct two docs claims the new setting invalidated: the properties help and CLAUDE.md said Close Stream When Inactive clears the frame unconditionally, and the README said every hot setting keeps the connection and the stats counters alive — turning Close Stream When Inactive on while the source is hidden stops the receiver and resets them, by design.
Reported on Discord: after ending a stream manually (Moblin → srtla_rec → SRT proxy → MediaMTX → OBS), the last decoded frame stayed on screen. Switching scenes and back cleared it, but only with Close Stream When Inactive enabled.
Cause
irl_handle_stream_read_error()faded out the audio and flushed its buffer but never touched video, so the last frame sat there for the whole reconnect loop — indefinitely if the stream never came back. The only code path that cleared video washide/deactivate, and those return early unless Close Stream When Inactive is on, which is why toggling it off made the scene switch stop clearing too.Not a refactor regression:
clear_async_videoonly ever existed as part ofa8f6715 feat: add close-when-inactive source behavior. No earlier commit cleared video on disconnect.Change
Ports OBS's own media source behaviour. What it calls
clear_on_media_end("Show nothing when playback ends",obs-ffmpeg-source.c:192, default on) is Show Nothing When the Stream Ends here — likewise on by default, and hot-swappable without dropping the connection.It gates every path where the stream stops:
receiver-stream.c, alongside the existing audio fade-outhide/deactivateunder Close Stream When InactiveThe no-URL case still clears unconditionally — there is no stream for the frame to have come from.
Why the clear runs on the video thread
obs_source_output_video(source, NULL)called straight from the receiver thread would race a frame already inside format conversion, and that frame would repaint the frozen image a few ms after the clear. Instead the receiver drops the queue and raisesvideo_clear_pendingundervideo_queue_lock, and the video thread re-checks the flag after each output. The flag is reset inreset_runtime_state(), which only runs with the workers stopped.Behaviour note
With the 2s default Reconnect Delay, a transient read error now blanks the source for the reconnect window instead of holding the last frame. That is exactly what the media source does. It cuts against the "prefer frames over blanks" line in
docs/viewer-quality-plan.md, but that stance is about decoder damage mid-stream, not a dead connection. If it flickers too much in practice, a grace period before the clear is the knob — not the default.Testing
Not compiled locally (no FFmpeg/libobs dev headers available,
deps/unbuilt), so CI here is the first real compile. Needs a manual check that ending the stream blanks the source, that it comes back cleanly on reconnect, and that unchecking the new box restores the old frozen-frame behaviour.Summary by CodeRabbit
New Features
Documentation