Skip to content

perf: avoid redundant external subtitle scans and updates - #36

Merged
blurbery merged 1 commit into
perf/artwork-cancellation-playback-statsfrom
perf/subtitle-cue-updates
Sep 26, 2026
Merged

blurbery merged 1 commit into
perf/artwork-cancellation-playback-statsfrom
perf/subtitle-cue-updates

Conversation

@blurbery

@blurbery blurbery commented Sep 26, 2026 •

Copy link
Copy Markdown
Owner

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

  • Index cue start and end boundaries once per external text track. Advance during playback and rebuild the active selection after backward seeks.
  • Preserve overlapping cues, file order, repeated IDs and inclusive-start/exclusive-end timing. Publish only changed selections, with track identity included to distinguish different tracks that reuse cue IDs.
  • Keep parsing, appearance and native embedded/ASS rendering unchanged. No metadata-cache changes.
  • Add 12 shared regression tests and include them in the tvOS test target.

Validation

Combined source ef893c40a231c0e95d9f4b65f412b9e4ec5514fe passed 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-testing followed by xcodebuild test-without-building, with test parallelism disabled. The new suite is VividSubtitleCueCursorTests in both VividTests and VividTVTests. git diff --check passed.

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

  • Tested: Targeted simulator suites and final combined CI passed in perf: reduce artwork, subtitle and playback statistics work #35.
  • Environment: iOS 27.0 and tvOS 27.0 simulators.
  • Revision: Combined source ef893c40a231c0e95d9f4b65f412b9e4ec5514fe, including the parent statistics review fixes.
  • Checks and results: 12 new regression cases passed on each simulator, alongside the existing targeted checks described above.
  • Limitations: No physical-device subtitle feedback or exhaustive real-world subtitle-file testing.

Design impact and approval

  • Design impact: No design changes.
  • Approval from blurbery: Not applicable, no design changes.

Documentation impact

Updated the existing playback architecture guide with cue indexing and publication behaviour.

AI disclosure

Implemented these changes with Codex as a tool.

@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 29 minutes.

Check out review usage here.

View limit details

Limit 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.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: blurbery/vivid/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 2b76f4b0-93eb-4ae2-8d4f-f2959b3f1608

📥 Commits

Reviewing files that changed from the base of the PR and between 8f65713 and a534002.

📒 Files selected for processing (5)
  • docs/playback/architecture.md
  • iosApp/Tests/VividSubtitleCueCursorTests.swift
  • iosApp/iosApp/Playback/MPV/VividMPVPlayer.swift
  • iosApp/iosApp/Playback/VividSubtitleCueCursor.swift
  • iosApp/project.yml

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@blurbery

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@blurbery
blurbery merged commit ef893c4 into perf/artwork-cancellation-playback-stats Sep 26, 2026
2 of 3 checks passed
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