perf: avoid redundant external subtitle scans and updates - #36
Conversation
|
Warning Review limit reachedNext included review available in 29 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: blurbery/vivid/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (5)
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 |
|
@coderabbitai review |
|
ef893c4
into
perf/artwork-cancellation-playback-stats
Problem and reproduction
Every playback tick scanned both external subtitle cue lists and published the results, even when the same captions remained visible or subtitles were off.
This PR targets #35's branch so the subtitle change can be reviewed separately, then included in one combined release when #35 reaches
main.Implementation and impact
Validation
Combined source
ef893c40a231c0e95d9f4b65f412b9e4ec5514fepassed 43 targeted iOS tests and 40 tvOS tests on OS 27.0 simulators with Xcode 27.0. Both runners finished successfully with complete result bundles. All 12 subtitle cases passed on each platform.The first CI run passed the tvOS build and all subtitle cases but failed one pre-existing exact seek-time assertion from #35. The parent branch now allows a 0.05-second frame tolerance while still checking seek completion, pause intent and load identity. The final combined #35 CI run passed: 1,077 iOS tests with 2 skipped and 0 failures, plus the tvOS simulator build and the workflow’s focused checks.
New tests compare the cursor against the original full filter across deterministic generated timelines, unsorted and overlapping cues, gaps, exact boundaries, forward/backward seeks, invalid inputs and parsed SRT/WebVTT files. Engine integration tests cover repeated publications, track switching, dual subtitles, delays, stop/reload and failed or cancelled loads. Existing artwork, statistics and iOS ASS-detection checks were also included.
Commands used:
xcodebuild build-for-testingfollowed byxcodebuild test-without-building, with test parallelism disabled. The new suite isVividSubtitleCueCursorTestsin bothVividTestsandVividTVTests.git diff --checkpassed.Performance
Across 1,000 clock updates, the engine tests observe no empty subtitle publications while subtitles are off, and one publication while a selected caption stays unchanged. These are deterministic work-count checks, not measured frame-rate or application benchmarks. Boundary indexes add two integer arrays per loaded text track; backward seeks can still scan the cues that have started.
Testing status
ef893c40a231c0e95d9f4b65f412b9e4ec5514fe, including the parent statistics review fixes.Design impact and approval
Documentation impact
Updated the existing playback architecture guide with cue indexing and publication behaviour.
AI disclosure
Implemented these changes with Codex as a tool.