Skip to content

Commit a229243

Browse files
hoangkien1703claude
hoangkien1703
and
claude
committed
Keep the spoken-word highlight from jumping backward
The playback window slides every few rows. Each slide reset the highlight resolver and live tracker and recomputed the row from timestamps, which often pointed at the overlapping previous sentence, while live progress restarted at the first word. Shift the held positions by the rows the window dropped instead. A backward correction inside a row now also needs the timestamps to agree, so live progress alone can no longer pull the underline to the first word. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent b1e4715 commit a229243

5 files changed

Lines changed: 231 additions & 19 deletions

File tree

‎app/src/main/java/com/kienhoang/dualsubreplay/ui/AppViewModel.kt‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,28 +1156,33 @@ class AppViewModel internal constructor(
11561156
rows: List<SubtitleSegment>,
11571157
preparing: Boolean,
11581158
) {
1159-
if (_state.value.segments
1160-
.firstOrNull()
1161-
?.id != rows.firstOrNull()?.id
1162-
) {
1159+
// The window slides forward every few rows during playback. Keep the highlight on the same
1160+
// row instead of recomputing it from timestamps, which can point at the previous sentence.
1161+
val shift = windowShift(_state.value.segments, rows)
1162+
if (shift == null) {
11631163
liveCaptionTracker.reset()
11641164
captionHighlightResolver.reset()
1165+
} else if (shift > 0) {
1166+
liveCaptionTracker.shift(shift)
1167+
captionHighlightResolver.shift(shift)
11651168
}
11661169
_state.update { current ->
11671170
if (!isCurrentLoad(current, videoId, generation)) return@update current
11681171
val index = activeSubtitleIndex(rows, latestPlaybackSecondMs)
1169-
// Preserve live karaoke corrections while only a translation changes.
1170-
val sameWindow = current.segments.firstOrNull()?.id == rows.firstOrNull()?.id
1172+
val keptIndex =
1173+
when {
1174+
shift == null -> null
1175+
current.currentIndex < 0 -> -1
1176+
else -> (current.currentIndex - shift).takeIf { it in rows.indices }
1177+
}
11711178
current.copy(
11721179
segments = rows,
1173-
currentIndex = if (sameWindow) current.currentIndex else index,
1180+
currentIndex = keptIndex ?: index,
11741181
activeWordIndex =
1175-
if (sameWindow) {
1176-
current.activeWordIndex
1177-
} else if (current.wordHighlightEnabled) {
1178-
activeWordIndex(rows, index, latestPlaybackSecondMs)
1179-
} else {
1180-
-1
1182+
when {
1183+
keptIndex != null -> current.activeWordIndex
1184+
current.wordHighlightEnabled -> activeWordIndex(rows, index, latestPlaybackSecondMs)
1185+
else -> -1
11811186
},
11821187
stage = if (preparing) LoadStage.TRANSLATING else LoadStage.READY,
11831188
statusMessage =

‎app/src/main/java/com/kienhoang/dualsubreplay/ui/KaraokeTiming.kt‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,12 @@ internal data class CaptionHighlightPosition(
5757
*
5858
* YouTube often reveals several auto-caption words in one update. The live position then marks
5959
* only the newest word, so inside that revealed range the timestamp word decides; outside it the
60-
* live range bounds the result. When live progress keeps reporting an earlier word of the same
61-
* sentence for [HIGHLIGHT_BACKWARD_CORRECTION_MS] of playback, it is accepted, so a wrong live match
62-
* (a repeated "the" or "you") recovers without a seek. The highlight never returns to an earlier
63-
* sentence without a seek: live captions briefly disappear between lines, and the timestamp
64-
* fallback can still point at the previous sentence then.
60+
* live range bounds the result. When live progress and the timestamps both keep reporting an
61+
* earlier word of the same sentence for [HIGHLIGHT_BACKWARD_CORRECTION_MS] of playback, it is
62+
* accepted, so a wrong live match (a repeated "the" or "you") recovers without a seek. Live progress
63+
* alone never moves the highlight back: after a reset it restarts at the first word of YouTube's
64+
* line. The highlight never returns to an earlier sentence without a seek: live captions briefly
65+
* disappear between lines, and the timestamp fallback can still point at the previous sentence then.
6566
*/
6667
internal class CaptionHighlightResolver {
6768
private var lastPosition: CaptionHighlightPosition? = null
@@ -72,6 +73,13 @@ internal class CaptionHighlightResolver {
7273
behindSinceMs = null
7374
}
7475

76+
/** The playback window dropped [delta] rows from its start; keep holding the same row. */
77+
fun shift(delta: Int) {
78+
val held = lastPosition ?: return
79+
val segmentIndex = held.segmentIndex - delta
80+
if (segmentIndex < 0) reset() else lastPosition = held.copy(segmentIndex = segmentIndex)
81+
}
82+
7583
fun resolve(
7684
generatedCaptions: Boolean,
7785
wordHighlightEnabled: Boolean,
@@ -92,7 +100,13 @@ internal class CaptionHighlightResolver {
92100
(if (fromLive) liveBoundedPosition(checkNotNull(livePosition), timedPosition) else timedPosition)
93101
?: return null
94102
val held = lastPosition
95-
val correctable = fromLive && held != null && selected.segmentIndex == held.segmentIndex
103+
// Live progress restarts at word 0 after a reset; only go back when the timestamps agree.
104+
val timedAgrees =
105+
held != null &&
106+
timedPosition != null &&
107+
timedPosition.segmentIndex == held.segmentIndex &&
108+
timedPosition.wordIndex in 0 until held.wordIndex
109+
val correctable = fromLive && timedAgrees && selected.segmentIndex == held?.segmentIndex
96110
val resolved =
97111
when {
98112
held == null || selected >= held -> selected
@@ -303,6 +317,13 @@ internal class LiveCaptionTracker {
303317
lastMappedMediaTimeMs = Long.MIN_VALUE
304318
}
305319

320+
/** The playback window dropped [delta] rows from its start; live progress itself is unchanged. */
321+
fun shift(delta: Int) {
322+
val mapped = lastPosition ?: return
323+
val segmentIndex = mapped.segmentIndex - delta
324+
if (segmentIndex < 0) reset() else lastPosition = mapped.copy(segmentIndex = segmentIndex)
325+
}
326+
306327
fun resolve(
307328
sample: LiveCaptionSample?,
308329
segments: List<SubtitleSegment>,

‎app/src/main/java/com/kienhoang/dualsubreplay/ui/PlaybackTranslation.kt‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,3 +100,16 @@ internal fun nextWindowTranslation(
100100
rows[it].translatedText == null && rows[it].endMs > behind && rows[it].startMs <= request.timeMs
101101
}
102102
}
103+
104+
/**
105+
* Rows [rows] dropped from the start of [previous] when the playback window slid forward, 0 for the
106+
* same window, or null when [rows] does not start inside [previous] (a seek, a reload, the first window).
107+
* Row ids are store indices, so equal ids are the same row in both windows.
108+
*/
109+
internal fun windowShift(
110+
previous: List<SubtitleSegment>,
111+
rows: List<SubtitleSegment>,
112+
): Int? {
113+
val firstId = rows.firstOrNull()?.id ?: return null
114+
return previous.indexOfFirst { it.id == firstId }.takeIf { it >= 0 }
115+
}

‎app/src/test/java/com/kienhoang/dualsubreplay/ui/KaraokeTimingTest.kt‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,61 @@ class KaraokeTimingTest {
8585
assertEquals(CaptionHighlightPosition(5, 3), resolver.resolve(true, true, 5, 2, KaraokePosition(5, 2), 11_300))
8686
}
8787

88+
@Test
89+
fun liveProgressAloneCannotPullTheHighlightBackToTheFirstWord() {
90+
// PR #76 phone report: after a reset, live progress restarts at word 0 while the
91+
// timestamps are still ahead. Without both signals agreeing, the highlight holds.
92+
val resolver = CaptionHighlightResolver()
93+
assertEquals(CaptionHighlightPosition(3, 5), resolver.resolve(true, true, 3, 5, KaraokePosition(3, 5), 10_000))
94+
listOf(10_100L, 11_000L, 12_500L, 15_000L).forEach { time ->
95+
assertEquals(CaptionHighlightPosition(3, 5), resolver.resolve(true, true, 3, 6, KaraokePosition(3, 0), time))
96+
}
97+
// Timestamps in the previous row do not count as agreement either.
98+
listOf(15_100L, 17_000L).forEach { time ->
99+
assertEquals(CaptionHighlightPosition(3, 5), resolver.resolve(true, true, 2, 4, KaraokePosition(3, 0), time))
100+
}
101+
}
102+
103+
@Test
104+
fun windowShiftKeepsHoldingTheSameRow() {
105+
val resolver = CaptionHighlightResolver()
106+
assertEquals(CaptionHighlightPosition(6, 3), resolver.resolve(true, true, 6, 3, null, 30_000))
107+
resolver.shift(2)
108+
// Timestamps point at the overlapping previous row (was 5, now 3): the highlight stays.
109+
assertEquals(CaptionHighlightPosition(4, 3), resolver.resolve(true, true, 3, 7, null, 30_100))
110+
assertEquals(CaptionHighlightPosition(4, 4), resolver.resolve(true, true, 4, 4, null, 30_400))
111+
}
112+
113+
@Test
114+
fun windowShiftPastTheHeldRowResets() {
115+
val resolver = CaptionHighlightResolver()
116+
assertEquals(CaptionHighlightPosition(1, 3), resolver.resolve(false, true, 1, 3, null))
117+
resolver.shift(2)
118+
assertEquals(CaptionHighlightPosition(0, 0), resolver.resolve(false, true, 0, 0, null))
119+
}
120+
121+
@Test
122+
fun liveTrackerShiftKeepsProgressWithoutWarmUp() {
123+
val tracker = LiveCaptionTracker()
124+
val old = listOf(segment(0, 0, "we start here"), segment(1, 1_200, "you explain it kind now"))
125+
tracker.resolve(sample("you explain", 1, 1_600), old, 1, 1_600)
126+
assertEquals(KaraokePosition(1, 2), tracker.resolve(sample("you explain it", 2, 2_000), old, 1, 2_000))
127+
tracker.shift(1)
128+
val slid = old.drop(1)
129+
// The next revision is emitted at once, in the shifted window, continuing forward.
130+
assertEquals(KaraokePosition(0, 3), tracker.resolve(sample("you explain it kind", 3, 2_400), slid, 0, 2_400))
131+
}
132+
133+
@Test
134+
fun windowShiftFindsTheNewFirstRowInThePreviousWindow() {
135+
val previous = listOf(segment(10, 0, "a"), segment(11, 1_000, "b"), segment(12, 2_000, "c"))
136+
assertEquals(0, windowShift(previous, previous))
137+
assertEquals(2, windowShift(previous, listOf(segment(12, 2_000, "c"), segment(13, 3_000, "d"))))
138+
assertNull(windowShift(previous, listOf(segment(9, 0, "z"), segment(10, 0, "a"))))
139+
assertNull(windowShift(emptyList(), previous))
140+
assertNull(windowShift(previous, emptyList()))
141+
}
142+
88143
@Test
89144
fun highlightNeverReturnsToThePreviousSentenceWithoutASeek() {
90145
// Phone report: live captions vanish between lines while timestamps still point at the

‎docs/specs/2026-09-25-highlight-never-jumps-back.md‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
# Spoken-word highlight never jumps backward during playback
2+
3+
## Status
4+
5+
Implemented. Local checks pass. Final-head CI and the owner's phone check are pending. The owner
6+
asked for a separate PR from `main`. That request does not authorize merging or publishing.
7+
8+
## Context / problem
9+
10+
On the phone build of PR #76, the owner saw the spoken-word underline sometimes jump back to the
11+
first word of the line or to the previous sentence. It felt jerky. The same highlight code is on
12+
`main` (see [sentence translation and highlight sync](2026-09-24-sentence-translation-and-highlight-sync.md)).
13+
14+
There are two causes in the code:
15+
16+
1. **Every playback-window slide wiped the highlight state.**
17+
- `SubtitleStore.windowIndices` keeps rows from 30 s before playback, so the window's first row
18+
changes every few seconds.
19+
- When it did, `AppViewModel.publishSubtitleWindow` reset `CaptionHighlightResolver` and
20+
`LiveCaptionTracker`, then set the row and word from timestamps alone.
21+
- Auto-caption lines overlap in time, so the timestamp row was often the previous sentence.
22+
- The reset tracker also restarted live progress at word 0 of YouTube's line, which is the jump
23+
to the first word.
24+
- The reset only existed because highlight positions are indices into the current window.
25+
2. **One signal could move the highlight back.** The 1 s backward correction inside a row trusted
26+
live progress alone. `liveBoundedPosition` clamps the timestamp word down to the live word, so
27+
live progress at word 0 pulled the underline back to the start of the row even while the
28+
timestamps were ahead.
29+
30+
## Goals
31+
32+
- During normal playback, the highlight only moves forward.
33+
- It moves backward only for a real reason:
34+
- a seek back;
35+
- a new video;
36+
- rebuilt rows (caption format, highlight toggle, reload, settings reset);
37+
- live progress **and** timestamps both agreeing, for 1 s, that it is ahead in the same row.
38+
39+
## Non-goals
40+
41+
- No change to live-caption matching, timestamps, window size or translation scheduling.
42+
43+
## User-visible behavior
44+
45+
- **Before:** every few seconds the underline could jump back to the previous sentence or to the
46+
first word of the line, then catch up.
47+
- **After:** the underline stays in place or moves forward. A backward seek still moves it back
48+
at once. A wrong live match still recovers after 1 s, because the timestamps also point earlier.
49+
50+
## Technical constraints / invariants
51+
52+
- Row ids are store indices (`SubtitleMerger.merge`, `prepareCaptionDisplayStore`). The same id in
53+
two windows is the same row.
54+
- Unit tests stay plain JUnit4.
55+
56+
## Proposed approach / plan
57+
58+
1. `windowShift(previous, rows)` in `PlaybackTranslation.kt` returns how many rows the window
59+
dropped from its start: 0 for the same window, null when the new window does not start inside
60+
the old one (seek, reload, first window).
61+
2. `CaptionHighlightResolver.shift` and `LiveCaptionTracker.shift` move their held positions by
62+
that amount. They reset only when the held row left the window. The tracker keeps its live
63+
progress, so there is no 2-revision warm-up after a slide.
64+
3. `publishSubtitleWindow` shifts the held position instead of resetting it, and keeps the
65+
current row and word. It still resets and recomputes from timestamps when `windowShift` is null.
66+
4. `CaptionHighlightResolver` accepts a backward correction only when the timestamp word is in the
67+
held row and before the held word, in addition to the existing 1 s, same-row, live-only rules.
68+
69+
## Acceptance criteria
70+
71+
- [x] Live progress at word 0 while timestamps are ahead, or in the previous row, never moves the
72+
highlight back, however long it lasts
73+
(`KaraokeTimingTest.liveProgressAloneCannotPullTheHighlightBackToTheFirstWord`; fails with the
74+
old rule).
75+
- [x] After a window slide, timestamps pointing at the overlapping previous row do not move the
76+
highlight back (`windowShiftKeepsHoldingTheSameRow`).
77+
- [x] A slide past the held row resets it (`windowShiftPastTheHeldRowResets`).
78+
- [x] The live tracker continues after a slide without the warm-up
79+
(`liveTrackerShiftKeepsProgressWithoutWarmUp`).
80+
- [x] `windowShift` handles the same window, a forward slide, a backward window and empty lists
81+
(`windowShiftFindsTheNewFirstRowInThePreviousWindow`).
82+
- [x] A wrong live match still recovers after 1 s when the timestamps agree, and existing highlight
83+
tests pass unchanged (`wrongLiveWordMatchRecoversInsideTheSentenceAfterSustainedDisagreementOnly`).
84+
- [ ] Final-head CI `verify-build` and `managed-device-tests` pass.
85+
- [ ] Owner phone check: two minutes or more of an auto-captioned video with no backward jump.
86+
A backward seek still moves the highlight back at once.
87+
88+
## Validation plan
89+
90+
| Category | Command/scenario and expected result | Environment / applicability |
91+
| --- | --- | --- |
92+
| Unit tests | `testDebugUnitTest` passes, including the new `KaraokeTimingTest` cases | Local Windows + CI |
93+
| Android lint/build | `formatCheck complexityCheck lintDebug assembleDebug assembleDebugAndroidTest` pass | Local Windows + CI |
94+
| Managed-device/emulator | Existing `pixel2Api36DebugAndroidTest` suite | CI |
95+
| Physical-device / live YouTube | Scenario in the last acceptance criterion | Owner's phone |
96+
97+
## Risks / edge cases
98+
99+
- A wrong live match whose timestamps also point ahead is now held until the speech catches up,
100+
or until a seek. Before, it corrected after 1 s. Staying slightly ahead is less jarring than
101+
jumping back.
102+
103+
## Release intent
104+
105+
`release:patch`: a user-visible bug fix with no new setting (project default).
106+
107+
## Implementation result
108+
109+
Implemented as planned.
110+
111+
## Validation result
112+
113+
- Local (Windows, JDK 17, Android SDK 36): `formatCheck complexityCheck testDebugUnitTest lintDebug
114+
assembleDebug assembleDebugAndroidTest` passed, with 281 unit tests and 0 failures.
115+
- With the old backward-correction rule put back, the new first-word test fails (1 of 20 in
116+
`KaraokeTimingTest`), so it covers the reported jump.
117+
- Managed-device tests: not run locally. CI runs them.
118+
- Physical phone / live YouTube: not run. Pending owner acceptance.

0 commit comments

Comments
 (0)