Skip to content

fix(video): treat NOT_READY as transient with one IDR on resume#594

Open
mariotaku wants to merge 2 commits into
mainfrom
fix/video-feed-not-ready
Open

fix(video): treat NOT_READY as transient with one IDR on resume#594
mariotaku wants to merge 2 commits into
mainfrom
fix/video-feed-not-ready

Conversation

@mariotaku

Copy link
Copy Markdown
Owner

Summary

Companion to ss4s #16 (FeedGuard exclusive primitive) — closes the original race that #593 worked around, but does it correctly so HDR toggle and resolution changes actually work mid-stream instead of being silently suppressed.

Background

SS4S_PlayerVideoSetHDRInfo(NULL) on ndl-webos5 (and SS4S_PlayerVideoSizeChanged on most LG/NDL backends) internally calls NDL_DirectMediaUnload + NDL_DirectMediaLoad. Before ss4s #16, an in-flight driver->Feed could be running on the half-torn-down decoder and crash the stream. PR #593 worked around this by skipping the SetHDRInfo(NULL) call entirely for MAIN10 streams.

ss4s #16 added a proper drain primitive (BeginExclusive / EndExclusive on the FeedGuard): during the destructive op, Feed returns SS4S_VIDEO_FEED_NOT_READY so it never reaches the driver. But the app side still tears down the stream on NOT_READY because it falls into the default error path:

} else {
    commons_log_error("Session", "Video feed error %d", result);
    session_interrupt(session, false, STREAMING_INTERRUPT_DECODER);
    return DR_OK;
}

So the race is gone, but every HDR toggle still kills the stream.

Fix

Handle NOT_READY as a transient signal in vdec_delegate_submit:

Feed return New behavior
OK (first one after NOT_READY) Return DR_NEED_IDR — server resends a keyframe so the decoder resyncs from known-good data instead of a P-frame referencing the gap
OK (steady state) DR_OK, normal stats bookkeeping
NOT_READY Drop the frame silently (DR_OK), set need_idr_on_resume = true. No session_interrupt, no DR_NEED_IDR spam — exactly one IDR request per outage no matter how long it lasts.
REQUEST_KEYFRAME DR_NEED_IDR (existing)
Anything else session_interrupt(STREAMING_INTERRUPT_DECODER) (existing — real driver errors still tear the stream down)

Resets need_idr_on_resume = false in vdec_delegate_setup so stream restarts don't inherit stale state.

Bumps

Submodule pin 60d980a2ab442770. Picks up:

  • fix(video): drain Feed during SetHDRInfo and SizeChanged (#16) — the exclusive primitive this PR pairs with
  • ci: run tests on every push and pull request (#17)
  • ci: add AddressSanitizer pipeline (with dynamic libasan) (#18)

Comparison with #593

Aspect #593 (skip on MAIN10) this PR + ss4s #16
Fixes the crash on MAIN10 HDR-off
Same fix for SizeChanged race ✅ (free — same primitive)
HDR can actually be disabled mid-stream on 8-bit streams unchanged
HDR can be disabled mid-stream on MAIN10 streams ❌ (suppressed)
Brief frame drop + IDR resync during toggle n/a ~ms of dropped frames, single IDR request, no tear-down

Recommend closing #593 in favor of this combined fix.

Test plan

🤖 Generated with Claude Code

Pairs with the ss4s FeedGuard exclusive primitive (ss4s #16) that
drains in-flight Feeds across SetHDRInfo and SizeChanged. During
those brief windows, SS4S_PlayerVideoFeed returns
SS4S_VIDEO_FEED_NOT_READY instead of running on a half-torn-down
decoder.

vdec_delegate_submit previously treated any non-OK / non-KEYFRAME
return as fatal and called session_interrupt(STREAMING_INTERRUPT_DECODER),
which tore the stream down on every HDR toggle and every
resolution change.

Handle NOT_READY as transient:
- Drop the current frame silently (return DR_OK), set
  need_idr_on_resume = true.
- On the next successful Feed, consume the flag and return one
  DR_NEED_IDR so Limelight resends a keyframe and the decoder
  resyncs from a known-good frame instead of decoding the next
  P-frame against the data it never saw.
- Exactly one IDR request per outage, regardless of how many
  frames got dropped during the exclusive window.

Reset the flag in vdec_delegate_setup so stream restarts don't
inherit stale state.

Bumps the ss4s submodule to pick up #16 (FeedGuard exclusive),
#17 (CI test pipeline), and #18 (ASan CI pipeline).

Supersedes #593 (which suppressed the SetHDRInfo(NULL) call on
MAIN10 as a workaround). With this fix the toggle works
correctly without suppression, and SizeChanged gets the same
treatment for free.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant