Repository navigation
feat(irl-source): video delay follows the audio playout offset - #36
Merged
Merged
Conversation
…p's chunk and lead
The audio hold's monotonic min/max queue moves to irl_core::window so the video delay can judge its relax window the same way.
The "Video lead is beyond what OBS can queue" line assumed frames reach libobs as early as the audio mapping places them. Pacing hands each frame over a couple of canvas ticks before it is due, so libobs never holds more than that and the warning described a queue that does not exist. Its frame budget, throttle and default frame interval go with it; the frame-interval EMA stays for the stats line's vfps.
…d of tracking it by hand The video delay was a stored offset on top of the audio playout offset, kept in step by hand-offs: the pump credited hold growth to AudioState::hold_built_ns, the video thread absorbed it, and raises and ramps shifted the queue. Those missed cases. After a Target Buffer edit during a hold build the delay stood at 66 ms for good, and concealment growth during an outage was never absorbed, so the picture stayed some 300 ms behind the sound for 17 to 24 s after the link came back. irl_core::video_delay now keeps an absolute floor F under the schedule: the worst arrival minus PTS seen, plus a tick for the decode. A frame is due at pts + max(O, min(F, O + max)), so the delay is clamp(F - O, 0, max) and is derived wherever it is needed. The raise, window, relax and ceiling rules are unchanged but act on F, and a raise still sets a whole number of ticks of delay against the offset in force. The liveness check moves into the same type, and a PTS timeline reset now forgets it with the floor. The relax window slides instead of running in fixed blocks, and the part of the floor below the offset, which is not on screen, drops at once instead of slewing. Otherwise a hold released for a recovered sender leaves a stale floor showing for up to ten seconds. Removed: VideoDelay::absorb, hold::credit_build, AudioState::hold_built_ns and hold_unbuilt_ns, VideoThread::absorb_audio_hold, PacingQueue::shift and shift_earlier. Queued frames keep their undelayed due time, so reschedule re-derives every due time, fallback-scheduled ones included. The audio hold itself is unchanged. video_delay_ms and vdelay= report the derived delay, republished every video cycle. When the playout alone takes it to or from zero, one info line says so. The fallback adds the startup audio lead whenever no mapping is published, matching the test that picks the mapping, so a chunk whose PTS ends at or below zero no longer leaves video with neither. Tests whose numbers moved: - a_delay_set_by_a_bad_first_frame_ramps_back_without_a_jump settles at 0, not one tick: the relax target is what frames need plus a tick of headroom in floor terms, and 100 ms in hand covers both. - the hand-back test drives only the mapping; nothing credits the build. - av_sync_sim: a Target Buffer edit during a build ends in sync (82.8 ms to 16.1 ms, the anchoring tick) and is no longer ignored. A stall during a build is in sync 0.1 s after it (was 17.4 s), a loss under the delay 1.6 s after it (was 24.4 s); both assert that now, and their settled windows start at 34 s and 30 s instead of 55 s and 60 s. The loss scenario checks that the delay stood before the loss rather than at the re-anchor, since concealment now takes it away first. - network_sim checks the built hold by the buffer level and the video margin instead of the removed credit. - video_delay unit tests are restated against the floor; the absorb ones go, and tests for a raise against a moved offset, the derived delay, slew visibility and reset are added.
The Video delay and Audio hold sections describe the floor the delay is derived from, why the relax window slides, and the hold building without any hand-off to the video thread.
…th the mapping the output clock restarts on a reconfigure, so the offset it primes on is not comparable with the baseline of the old format, and the re-anchor check measured drift against it.
Contributor
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (32)
✨ Finishing Touches📝 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #35. Merge that first; this PR's base then moves to master.
Why
The video delay and the audio hold estimate the same thing (how far video trails its audio) in two ways, and three hand-offs kept them in step: the pump credited hold growth, the video thread absorbed it from the delay, and the re-anchor netted the hold out of its drift. The hand-offs missed cases. A new end-to-end simulation shows two of them:
What changed
pts + max(O, F), capped, so the visible delaymax(0, F − O)is derived and never stored. Raise, relax and ceiling policies stay, applied to F. The absorb and hold credit plumbing is gone.video_delay_msreports the live picture behind sound error.irl-core:OutputClock(one restart path for prime, stall and re-anchor) andPlayoutMapping(the offset and its drift baseline, shared by the pump andav_drift).VideoThreadreads one injected clock, which removes the timing flake intests/video_pipeline.rs.tests/av_sync_sim.rsdrives the real pump and video thread on one virtual clock and measures the actual offset between picture and sound in 10 scenarios.crates/irl-core/examples/skew-relay.rsanddocs/skew-testing.mdreproduce late video against a real OBS.Simulation results
How to review
crates/irl-core/src/video_delay.rsand its tests: that is the model.crates/irl-source/src/video/thread.rsandvideo/output.rs, where due times are derived.crates/irl-source/tests/av_sync_sim.rs, the scenarios and their bounds.The commits before
94d82b4change no playback behaviour, apart from removing the lead warning infbd03e6.94d82b4is the model change, and its body explains every test whose numbers moved.Risks
docs/skew-testing.mdis the recipe; real Moblin or pocketSRT captures would be the stronger check.