Apply the subtitle delay on ExoPlayer #74

Merged

justinzeus merged 3 commits from fm/sub-offset-core into fm/tv-round2 2026-11-08 01:45:22 +01:00

Conversation 1 Commits 3 Files changed 8 +776 -5

justinzeus commented 2026-11-08 01:40:40 +01:00

Owner

Copy link

The subtitle delay control (gear menu → Subtitle delay, −1 s … +1 s, saved per item and per subtitle track) now works on ExoPlayer, the default player. Until now it only showed and worked on MPV.

Commits

1. Add a subtitle delay renderer wrapper for ExoPlayer: new services/SubtitleDelayRenderer.kt.
   • SubtitleDelayRenderer is a ForwardingRenderer. It gives the wrapped renderer the playback position minus the delay in enable, render, resetPosition, getDurationToProgressUs and supportsResetPositionWithoutKeyFrameReset. A positive delay shows subtitles later, the same sign as mpv sub-delay. The delay changes only through a MSG_SET_SUBTITLE_DELAY renderer message, which runs on the playback thread.
   • End of stream: media3's TextRenderer reads its end of stream only within 1 s of the period end. With a delay above +1 s, STATE_ENDED would arrive late by the delay minus 1 s, so next-up would come late. The wrapper reports ended once the real position passes the period end (worked out from the timeline and MediaPeriodId) while the delay is positive.
   • SubtitleDelayRenderersFactory wraps the text renderer and libass's AssRenderer. AssRenderer draws at positionUs - 10^12, so the same shift also delays ASS/SSA under libass.
   • Unit tests: SubtitleDelayRendererTest.
2. Apply the subtitle delay on ExoPlayer and show the delay control
   • PlayerFactory wraps the final renderers factory, after the libass wrap.
   • Player.applySubtitleDelay(delay) sets MpvPlayer.subtitleDelay, or sends the message to every SubtitleDelayRenderer of an ExoPlayer.
   • PlaybackPage: the apply effect now keys on (player, subtitleDelay). The gate is now subtitleDelaySupported(player, currentPlayback): always true for MPV, and true for ExoPlayer unless the selected subtitle stream has DeliveryMethod = Encode (burned in by the server).
   • Tests in SubtitleDelayControlTest:
     • the gate for MPV, ExoPlayer with embedded, external, burned-in and no subtitles, and for other players;
     • Settings lists "Subtitle delay" only when the delay is enabled;
     • pressing "+250ms" reports 250 ms.
   • Jellyfin sets DeliveryMethod on every subtitle stream of a PlaybackInfo response (Jellyfin.Api/Helpers/MediaInfoHelper.cs:504-511). The fixture external_subs.json carries it (External and Embed), and the burned-in case swaps Embed for Encode.
3. Add an end-to-end subtitle delay playback test
   • Adds media3-test-utils and media3-test-utils-robolectric 1.11.1 as test dependencies (Apache-2.0, same version as media3).
   • SubtitleDelayPlaybackTest plays a generated 4 s WAV with a sideloaded SRT through a real ExoPlayer (DefaultMediaSourceFactory, CapturingRenderersFactory, fake clock):
     • +2 s: the first line shows at 3 s. The line that would land after the end never shows. STATE_ENDED arrives after 4 s of playback (±100 ms).
     • −0.5 s: lines show 0.5 s early.
     • 0 → +1 s at 0.5 s: the change re-times the next lines, and the states are only BUFFERING → READY → ENDED, with no rebuffer.
   • This commit is separate so it can stay fork-only if upstream does not want the test dependency.

Resetting the delay when switching to a track with no saved value landed separately: "Reset subtitle delay when the new track has none saved", already on fm/tv-round2.

What it covers and what it does not

• Covered:

  • embedded and external text subtitles (SRT, VTT, TTML, mov_text);
  • image subtitles (PGS, VobSub, DVB);
  • ASS/SSA under libass.

  All of them go through the same renderer shift, and a change applies while playing.

• Not covered:

  • Burned-in subtitles. A server transcode draws them into the video, so nothing on the client can move them. The control is hidden for them.
  • Seeking with a positive delay. A line that started less than the delay before the seek point may be skipped.
  • libass while paused. Whether libass repaints while paused is not verified.

• media3 internals: wrapping hides the renderer from two instanceof TextRenderer checks in RendererHolder: the clipped-stream end position and server-side ad transitions. Wholphin uses neither.

Tests

Run through docker/gradle.sh debug :app:testDefaultDebugUnitTest --tests 'com.github.damontecres.wholphin.services.SubtitleDelay*' --tests 'com.github.damontecres.wholphin.ui.playback.SubtitleDelayControlTest':

Class	Tests	Result
SubtitleDelayRendererTest	6	pass
SubtitleDelayControlTest	3	pass
SubtitleDelayPlaybackTest	3	pass

• End-of-stream check. A throwaway variant of the end-to-end test bypassed the end-of-stream fix: with +2 s, the 4 s item reached STATE_ENDED only after 5010 ms of playback, at position 5010 > duration 4000. The committed test requires 4000 ± 100 ms, so it fails without the fix. The throwaway was not committed.
• Base: the tests ran on the commits based on 5f5b5ffd. The branch was then rebased without conflicts onto 709ca291, which only adds the reset-delay change in PlaybackViewModel and Discover changes. None of the files in this PR changed in that rebase.
• Not run: the full suite, an APK build and any device.

Manual check for later, on a real player

Play a title with an external SRT, set the delay to +2 s and let it run to the end of the film. It must reach the end and show the end screen or next-up straight away, without hanging or waiting at the end.

The subtitle delay control (gear menu → Subtitle delay, −1 s … +1 s, saved per item and per subtitle track) now works on ExoPlayer, the default player. Until now it only showed and worked on MPV. ## Commits 1. **Add a subtitle delay renderer wrapper for ExoPlayer**: new `services/SubtitleDelayRenderer.kt`. - `SubtitleDelayRenderer` is a `ForwardingRenderer`. It gives the wrapped renderer the playback position minus the delay in `enable`, `render`, `resetPosition`, `getDurationToProgressUs` and `supportsResetPositionWithoutKeyFrameReset`. A positive delay shows subtitles later, the same sign as mpv `sub-delay`. The delay changes only through a `MSG_SET_SUBTITLE_DELAY` renderer message, which runs on the playback thread. - End of stream: media3's `TextRenderer` reads its end of stream only within 1 s of the period end. With a delay above +1 s, `STATE_ENDED` would arrive late by the delay minus 1 s, so next-up would come late. The wrapper reports ended once the real position passes the period end (worked out from the timeline and `MediaPeriodId`) while the delay is positive. - `SubtitleDelayRenderersFactory` wraps the text renderer and libass's `AssRenderer`. `AssRenderer` draws at `positionUs - 10^12`, so the same shift also delays ASS/SSA under libass. - Unit tests: `SubtitleDelayRendererTest`. 2. **Apply the subtitle delay on ExoPlayer and show the delay control** - `PlayerFactory` wraps the final renderers factory, after the libass wrap. - `Player.applySubtitleDelay(delay)` sets `MpvPlayer.subtitleDelay`, or sends the message to every `SubtitleDelayRenderer` of an `ExoPlayer`. - `PlaybackPage`: the apply effect now keys on `(player, subtitleDelay)`. The gate is now `subtitleDelaySupported(player, currentPlayback)`: always true for MPV, and true for ExoPlayer unless the selected subtitle stream has `DeliveryMethod = Encode` (burned in by the server). - Tests in `SubtitleDelayControlTest`: - the gate for MPV, ExoPlayer with embedded, external, burned-in and no subtitles, and for other players; - Settings lists "Subtitle delay" only when the delay is enabled; - pressing "+250ms" reports 250 ms. - Jellyfin sets `DeliveryMethod` on every subtitle stream of a PlaybackInfo response (`Jellyfin.Api/Helpers/MediaInfoHelper.cs:504-511`). The fixture `external_subs.json` carries it (`External` and `Embed`), and the burned-in case swaps `Embed` for `Encode`. 3. **Add an end-to-end subtitle delay playback test** - Adds `media3-test-utils` and `media3-test-utils-robolectric` 1.11.1 as test dependencies (Apache-2.0, same version as media3). - `SubtitleDelayPlaybackTest` plays a generated 4 s WAV with a sideloaded SRT through a real `ExoPlayer` (`DefaultMediaSourceFactory`, `CapturingRenderersFactory`, fake clock): - **+2 s:** the first line shows at 3 s. The line that would land after the end never shows. `STATE_ENDED` arrives after 4 s of playback (±100 ms). - **−0.5 s:** lines show 0.5 s early. - **0 → +1 s at 0.5 s:** the change re-times the next lines, and the states are only BUFFERING → READY → ENDED, with no rebuffer. - This commit is separate so it can stay fork-only if upstream does not want the test dependency. Resetting the delay when switching to a track with no saved value landed separately: "Reset subtitle delay when the new track has none saved", already on `fm/tv-round2`. ## What it covers and what it does not - **Covered:** - embedded and external text subtitles (SRT, VTT, TTML, mov_text); - image subtitles (PGS, VobSub, DVB); - ASS/SSA under libass. All of them go through the same renderer shift, and a change applies while playing. - **Not covered:** - **Burned-in subtitles.** A server transcode draws them into the video, so nothing on the client can move them. The control is hidden for them. - **Seeking with a positive delay.** A line that started less than the delay before the seek point may be skipped. - **libass while paused.** Whether libass repaints while paused is not verified. - **media3 internals:** wrapping hides the renderer from two `instanceof TextRenderer` checks in `RendererHolder`: the clipped-stream end position and server-side ad transitions. Wholphin uses neither. ## Tests Run through `docker/gradle.sh debug :app:testDefaultDebugUnitTest --tests 'com.github.damontecres.wholphin.services.SubtitleDelay*' --tests 'com.github.damontecres.wholphin.ui.playback.SubtitleDelayControlTest'`: | Class | Tests | Result | |---|---|---| | `SubtitleDelayRendererTest` | 6 | pass | | `SubtitleDelayControlTest` | 3 | pass | | `SubtitleDelayPlaybackTest` | 3 | pass | - **End-of-stream check.** A throwaway variant of the end-to-end test bypassed the end-of-stream fix: with +2 s, the 4 s item reached `STATE_ENDED` only after 5010 ms of playback, at position 5010 > duration 4000. The committed test requires 4000 ± 100 ms, so it fails without the fix. The throwaway was not committed. - **Base:** the tests ran on the commits based on `5f5b5ffd`. The branch was then rebased without conflicts onto `709ca291`, which only adds the reset-delay change in `PlaybackViewModel` and Discover changes. None of the files in this PR changed in that rebase. - **Not run:** the full suite, an APK build and any device. ## Manual check for later, on a real player Play a title with an external SRT, set the delay to +2 s and let it run to the end of the film. It must reach the end and show the end screen or next-up straight away, without hanging or waiting at the end.

justinzeus added 3 commits 2026-11-08 01:40:40 +01:00

Add a subtitle delay renderer wrapper for ExoPlayer 0c9bffc3ab

Apply the subtitle delay on ExoPlayer and show the delay control 7c03a3fe0d

Add an end-to-end subtitle delay playback test 0a7c059f24

justinzeus commented 2026-11-08 01:43:49 +01:00

Author

Owner

Copy link

Review of the subtitle delay on ExoPlayer at head 0a7c059f241360bb76914ff53721237f2c4abd38 (3 commits, base fm/tv-round2).

Code verdict: APPROVE (review/code-claude = success).
Design verdict: PASS (review/design-claude = success). No new UI: the existing "Subtitle delay" row and its steps now appear for ExoPlayer, with no new string, token or layout.

Some media3-internal behaviour below rests on the lane's tests and the plan, not on my running media3. Those points are marked [INFERENCE].

Zero delay is identical to today.

• With a zero delay each shifted argument is positionUs - 0. Stream start and offset are untouched, and only the one custom message is intercepted.
• isEnded widens only when the delay is positive.
• The wrapper never withholds render, so it cannot stall or drop cues.
• No test plays through the real player at zero delay (note 3).

Shift is exact.

• Each entry point (enable, render, resetPosition, getDurationToProgressUs, supportsResetPositionWithoutKeyFrameReset) is shifted once, and nothing else shifts ExoPlayer text.
• A positive delay shows cues later and a negative delay earlier, as in mpv.
• The unit test pins all five calls. The real-player test shows +2 s at 3 s, and -0.5 s at 0.5 s and 2.0 s, within 100 ms.

End of stream (the lane's correction).

• Without the fix, +2 s ends 1 s late (5010 ms against 4000 ms), not a hang.
• The committed test asserts 4000 ± 100 ms and that the cue landing after the end never shows, so it fails without isEnded and passes with it.
• The override compares the unshifted position with the period end (stream offset plus period duration) in renderer time. It applies only for a positive delay with a known duration, and a seek back un-ends it. [INFERENCE on media3's text renderer]

Change while playing.

• The delay is set by a playback-thread message. The test sees only BUFFERING, READY, ENDED, with later cues re-timed.

Track kinds.

• Every TEXT renderer is wrapped (embedded and external text, plus PGS/VobSub/DVB image cues) and libass's AssRenderer. Video, audio and metadata renderers are untouched.
• Burned-in streams are unsupported as planned, and their control is hidden.

MPV.

• It maps to the same setter and the same gate as before, and the effect now also keys on the player.

Gate.

• MPV is true. ExoPlayer is true unless the selected stream's delivery method is ENCODE. No stream, or no playback yet, is true. Other players are false. It recomputes on track switch.
• SubtitleDelayControlTest covers these cases, the row appearing only when enabled, and "+250ms" reporting 250 ms.

Merge and dependencies.

• A merge-tree check against current fm/tv-round2 is clean.
• The media3-test-utils dependencies are test-only, at the existing version 1.11.1.

Non-blocking notes:

1. A delay change mid-playback does not reset the renderer. Lines already decoded keep their old timing for up to one 250 ms step. [INFERENCE]
2. There is no real-player test for embedded text in a container, PGS or libass ASS. libass repaint while paused is unverified, as the PR states.
3. One zero-delay assertion in the real-player test would pin the unchanged path.
4. After a seek with a positive delay, cues that started within the delay before the seek point are skipped. The PR documents this.
5. Wrapping hides the renderer from two media3 instanceof TextRenderer checks (clipped streams, server-side ad insertion). Wholphin uses neither.
6. Check on the box:
   • an external SRT at +2 s played to the end, where the end screen and next-up must come promptly;
   • one embedded text, one PGS and one libass title at ±500 ms, including pause and a seek;
   • a burned-in transcode, which must show no delay row;
   • the MPV backend, which must be unchanged.

Full report (review 12): /root/.treehouse/firstmate-7bab20/2/firstmate/data/round2-review/report.md. Not run: Gradle/make, nothing on the box.

Review of the subtitle delay on ExoPlayer at head `0a7c059f241360bb76914ff53721237f2c4abd38` (3 commits, base `fm/tv-round2`). **Code verdict: APPROVE (`review/code-claude` = success).** **Design verdict: PASS (`review/design-claude` = success).** No new UI: the existing "Subtitle delay" row and its steps now appear for ExoPlayer, with no new string, token or layout. Some media3-internal behaviour below rests on the lane's tests and the plan, not on my running media3. Those points are marked [INFERENCE]. **Zero delay is identical to today.** - With a zero delay each shifted argument is `positionUs - 0`. Stream start and offset are untouched, and only the one custom message is intercepted. - `isEnded` widens only when the delay is positive. - The wrapper never withholds `render`, so it cannot stall or drop cues. - No test plays through the real player at zero delay (note 3). **Shift is exact.** - Each entry point (`enable`, `render`, `resetPosition`, `getDurationToProgressUs`, `supportsResetPositionWithoutKeyFrameReset`) is shifted once, and nothing else shifts ExoPlayer text. - A positive delay shows cues later and a negative delay earlier, as in mpv. - The unit test pins all five calls. The real-player test shows +2 s at 3 s, and -0.5 s at 0.5 s and 2.0 s, within 100 ms. **End of stream (the lane's correction).** - Without the fix, +2 s ends 1 s late (5010 ms against 4000 ms), not a hang. - The committed test asserts 4000 ± 100 ms and that the cue landing after the end never shows, so it fails without `isEnded` and passes with it. - The override compares the unshifted position with the period end (stream offset plus period duration) in renderer time. It applies only for a positive delay with a known duration, and a seek back un-ends it. [INFERENCE on media3's text renderer] **Change while playing.** - The delay is set by a playback-thread message. The test sees only BUFFERING, READY, ENDED, with later cues re-timed. **Track kinds.** - Every TEXT renderer is wrapped (embedded and external text, plus PGS/VobSub/DVB image cues) and libass's `AssRenderer`. Video, audio and metadata renderers are untouched. - Burned-in streams are unsupported as planned, and their control is hidden. **MPV.** - It maps to the same setter and the same gate as before, and the effect now also keys on the player. **Gate.** - MPV is true. ExoPlayer is true unless the selected stream's delivery method is `ENCODE`. No stream, or no playback yet, is true. Other players are false. It recomputes on track switch. - `SubtitleDelayControlTest` covers these cases, the row appearing only when enabled, and "+250ms" reporting 250 ms. **Merge and dependencies.** - A merge-tree check against current `fm/tv-round2` is clean. - The `media3-test-utils` dependencies are test-only, at the existing version 1.11.1. **Non-blocking notes:** 1. A delay change mid-playback does not reset the renderer. Lines already decoded keep their old timing for up to one 250 ms step. [INFERENCE] 2. There is no real-player test for embedded text in a container, PGS or libass ASS. libass repaint while paused is unverified, as the PR states. 3. One zero-delay assertion in the real-player test would pin the unchanged path. 4. After a seek with a positive delay, cues that started within the delay before the seek point are skipped. The PR documents this. 5. Wrapping hides the renderer from two media3 `instanceof TextRenderer` checks (clipped streams, server-side ad insertion). Wholphin uses neither. 6. Check on the box: - an external SRT at +2 s played to the end, where the end screen and next-up must come promptly; - one embedded text, one PGS and one libass title at ±500 ms, including pause and a seek; - a burned-in transcode, which must show no delay row; - the MPV backend, which must be unchanged. Full report (review 12): `/root/.treehouse/firstmate-7bab20/2/firstmate/data/round2-review/report.md`. Not run: Gradle/make, nothing on the box.

justinzeus merged commit 12a81ca8e1 into fm/tv-round2 2026-11-08 01:45:22 +01:00

justinzeus referenced this pull request from a commit 2026-11-08 01:45:22 +01:00

Merge pull request 'Apply the subtitle delay on ExoPlayer' (#74) from fm/sub-offset-core into fm/tv-round2

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

exploring

Idea with a proof gate; not scheduled until the gate passes

No labels

exploring

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: justinzeus/wolfskin#74

No description provided.