Repository navigation
refactor!: smaller codebase, stats cut to what diagnosed real issues - #35
Merged
Merged
Conversation
…e old decode path
…clock the reset recovery hold and the packet keyframe gate timeout move into irl_core::consts with the other tuning values.
…ing queue to tests
…s, fix safety comments and doc links
get_stats and GetStats keep buffer_fill_ms, current_speed, total_audio_frames, total_video_frames, pts_max_gap_ms, audio_underruns, audio_output_restarts, video_delay_ms, av_skew_ms, audio_hold_ms and reconnecting. removed: adaptive_latency_control, low_latency_audio, reconnect_count, pts_repairs, pts_normalizations, pts_interpolations, pts_resets, pts_last_gap_ms, silence_insertions, audio_resync_skipped_chunks, audio_hidden_trimmed_chunks, audio_quality_events, obs_lead_ms, audio_decoder_flushes, video_corrupt_frames, video_corrupt_held, video_lead_ms, video_lead_excess and stream_delay_ms, along with the counters and mirrors that only fed them. stream_delay_ms measured schedule drift since the first frame, not end-to-end delay. the 30 s stats line drops the pts split, trims, quality events, obs lead, chunk sizes, decoder flushes, corrupt/held, video lead, early and reanchors. it gains ll= for low latency audio and prints ctrl=off only when adaptive latency control is disabled. the readme table and sample line, irl-stats.lua and the docs follow. BREAKING CHANGE: the get_stats proc and the GetStats vendor request no longer report adaptive_latency_control, low_latency_audio, reconnect_count, pts_repairs, pts_normalizations, pts_interpolations, pts_resets, pts_last_gap_ms, silence_insertions, audio_resync_skipped_chunks, audio_hidden_trimmed_chunks, audio_quality_events, obs_lead_ms, audio_decoder_flushes, video_corrupt_frames, video_corrupt_held, video_lead_ms, video_lead_excess or stream_delay_ms; overlays reading them must be updated.
…onger returns an option
…tive to docs/architecture.md the deviation list against the 1.x C plugin is gone; current behaviour is the spec.
Contributor
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (82)
💤 Files with no reviewable changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThis pull request narrows public wrapper APIs, centralizes shared timing and DSP helpers, reduces published statistics, updates audio/video recovery behavior, and aligns tests, documentation, and examples with the revised interfaces. ChangesRuntime and API consolidation
Priority: ➖ Normal Estimated code review effort: 5 (Critical) | ~90 minutes Change: Refactor · Severity of issue fixed: Low Merge Risk: ⚪ Minimal · up to No actionable merge-blocking issue is established. The change is mergeable after normal checks, with real-stream comparison still to be completed. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change reduces statistics and low-level APIs. Reviewed cancellation, recovery, and resource-accounting behavior retains its existing safeguards. Older integrations need migration, and incomplete review coverage leaves limited residual uncertainty. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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. A rabbit trims the wrapper’s gate 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.
Why
The codebase carried a lot of weight from the port: functions nothing called, generality in
obsandffmpegno other plugin uses, 30 stats of which most never helped diagnose anything, comments that described the 1.x C plugin, and a 274 line CLAUDE.md that every agent session had to load. This PR removes that weight without changing settings, stat names that remain, or media controls.What changed
obsandffmpegtrimmed to what the plugin calls. Theobs-sysABI mirrors stay complete, sincelayout-testchecks them.NS_TIME_BASEconstant. ThePacedFrametrait is gone.irl_core::consts, andAudioBuffer::newno longer returns anOptionthat could never beNone.ll=andctrl=off. README,irl-stats.luaand the bug template are updated.crates/irl-source/tests/common/, repeated cases are table driven, and the constants test pins current values instead of the C plugin's.docs/architecture.md, and the list of deviations from the C plugin is deleted: current behaviour is the spec.Rust code goes from 28,219 to 26,572 lines.
How to review
Go commit by commit; each one passes
make checkon its own. Most are deletions or mechanical moves. Look closely at:58bbae2, the stats prune. Check that the kept internal counters (audible_skipped_chunks,audio_offset_reanchors,video_corrupt_held) are the ones the tests and log lines need.d71587b, the only behaviour fix in the low latency path.9540fd1, CLAUDE.md anddocs/architecture.md.How to release
Release as 3.0.0. The release notes must list the removed
get_statsandGetStatsfields, since overlays that read them will see them missing. TheBREAKING CHANGEfooter on58bbae2carries the list.Risks
Summary by CodeRabbit