fix(player): keep playback alive on bad input, and stop sessions outliving their screen - #170
Merged
Merged
Conversation
…utliving it First batch from a four-pass adversarial review of the playback subsystem (~140 files, 38k lines). These are the ones that cost a viewer the film or cost the server a stream slot. A MALFORMED SUBTITLE COULD KILL THE APP. PgsSupExtractor bounded byte length and segment count, then handed the display set to Media3's PGS parser with nothing catching what came back. That parser trusts the set's own 16-bit width and height: it allocates IntArray(width * height) and applies RLE runs with no pixel bound of its own, so a few corrupt bytes can declare an enormous bitmap. The result is NegativeArraySizeException, an oversized-run failure, or an allocation big enough to take the process down. Playback ran fine until the damaged caption arrived. Bounding upstream cannot help — the danger is in what the bytes DECLARE, not how many there are — so the parse is contained and a bad caption now costs one missing subtitle. A STALE PROGRESS REPLY COULD HIJACK THE NEXT EPISODE. The reporter is cancelled on adoption, but the network layer catches cancellation and returns a NetworkError, so the cancelled reporter kept going and acted on an answer about a session nobody was watching. It could publish Reconnecting over the new episode, restore the old session, or start a duplicate one from the current start params. Ownership is now re-checked after the call rather than before it. TWO WAYS A SERVER SESSION OUTLIVED THE VIEWER. Exiting while a direct-play fallback was completing left the manager owning a transcode that nothing ever stopped — abandonActiveVideoSession had no caller at all — so playback exited while the server kept the stream slot until timeout. And teardown read the session id only from Active, while Reconnecting and Failed carry none, so leaving during an outage or after the 90-second timeout never told the server anything. Both release explicitly now. A FROZEN PICTURE SAID NOTHING. Exhausted post-resume recovery reported Failed exactly once, and the TV answered it with telemetry alone — audio kept advancing over a still frame with no message and no reason to press anything. It tells the viewer now. A FIRST FRAME FROM THE OUTGOING STREAM VOUCHED FOR THE NEW ONE. The callbacks were not attempt-qualified, so one queued by the previous item could land after a replan and disarm the replacement's watchdog before it had rendered anything — a black picture with its own safety net switched off, and diagnostics recording a first frame that never happened. Tests cover the guard and fail without it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ck identity Second batch from the playback review, all of it argued through with the reviewer rather than applied on first suggestion — five of my first seven fixes were wrong, and three of these are corrections to those corrections. PGS BITMAPS ARE NOW JUDGED BEFORE MEDIA3 SIZES AN ALLOCATION FROM THEM. I had argued against this, on the grounds that reading the format ourselves means a second parser that disagrees with the first. That was wrong: it is two 16-bit fields at a fixed offset in the object header, and RLE correctness stays Media3's problem. Catching the failure afterwards was never containment — the allocation has already happened, and eleven bytes can ask for six gigabytes. A full-frame 1080p caption still plays; a 40000x40000 one is dropped. Separating decode from publication stays, because an exception between sampleData and sampleMetadata leaves uncommitted bytes and the next sample lands on a boundary the queue disagrees about. A dropped caption is recoverable; a corrupt queue is not. MANUAL AUDIO WAS BEING SET BY SUBTITLE CHANGES. Every commit carries the current audio index, so testing it for non-null marked the server default as the viewer's choice after any successful subtitle change — and then carried that non-choice into the next episode. CommittedSubtitle now records whether audio was what changed. FAILURES ARE IDENTIFIED, NOT COMPARED BY TEXT. Two failures can read the same; a mount deadline reported twice is identical prose. Acknowledging by string let an old acknowledgement clear a new failure that merely said the same thing. PHONE TEARDOWN COULD STOP THE SESSION SOMEONE WAS WATCHING. Phone navigation REPLACES the player back-stack entry, so a new view model can adopt a session before the outgoing one tears down, and both stop paths were unqualified. TV had already solved this; phone had not. LANGUAGES ARE CANONICALISED BY THE ONE CANONICALISER. My own attempt ran the tag through a normaliser that strips '-', so en-US became enus; passed three-letter codes through unchanged, so fre and fra never met; and could throw on unrecognised input, turning odd metadata into a failed start. Left deliberately unresolved and documented in place: whether the plan index or the Media3 ordinal should win when they disagree. The reviewer first said plan, an existing test says ordinal, and it withdrew when shown the test — the real answer is conditional on whether the mounted topology is catalog-shaped, which is a larger change than this batch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round 4 confirmed the PGS offsets, the language canonicaliser and the commit propagation, then found four defects — two of them created by the fixes in fcc6ed1. Recorded plainly because the pattern matters: each round of fixes has so far introduced roughly as many problems as it closed, and only the next round has caught them. THE PHONE OWNERSHIP FIX HAD A HOLE THE SIZE OF THE ORIGINAL BUG. Teardown runs in two stages and the first clears sessionId, so onCleared's "early snapshot" read null — and a null expectedSessionId does not mean "be careful", it disables the ownership guard entirely and stops whatever is playing. The qualified stop I added was therefore unqualified on exactly the path that matters, an explicit Back. Ownership is now a retained token recorded when the session is adopted, and a missing token means do not stop rather than stop anything. AN UNRESOLVABLE CARRIED CHOICE BECAME A PERMANENT PREFERENCE. Seeding the manual-audio flag from the handoff INTENT ignored that resolution can fail: when a carried track matches nothing in the next episode the server default plays, and marking that manual made the following auto-advance capture the default as deliberate. One episode without a matching track turned a server default into a preference for the rest of the series. The flag is raised only where the choice actually resolved to a track. A SECOND FAILURE WAS SILENT WHILE A FIRST WAS SHOWING. The id only advanced when the previous slot was empty, so a different failure arriving during one inherited its id — and the screen keys on the id alone. Acknowledgement also reset the counter to zero, letting a re-emitted old failure manufacture id 1 and replay something already dismissed. Ids come from a monotonic seed now and acknowledgement clears the message, not the generator. THE HOME GENERATION CHECK RAN BEFORE TWO SUSPENSIONS. Caching and the local overlay both suspend, and a newer fetch can complete and publish during either, so the check proved only that the reply was current when it arrived. It is re-checked before publishing, and a superseded fetch no longer writes its sections to the cache where a cold start would serve them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…adopted Round 5 found that two of the previous batch's changes were regressions rather than improvements. Both are corrected here. THE FRAME BASELINE COULD SWALLOW THE FIRST FRAME. It was captured on the first SAMPLE, and sampling runs every 500ms and only while the screen is started — so a stream that rendered before its first sample made its own frames the baseline, the detector never recognised a first frame, and a perfectly healthy stream ran into the twenty-second decoder deadline. That is worse than the unattributed callback it replaced. The count is now read at mount, which is the moment that separates this attempt's frames from the previous one's. OWNERSHIP WAS RECORDED AT ONE ADOPTION SITE OUT OF FOUR. Initial playback, seek recovery and transactional replacement all adopt a session too, and none of them retained the token — so "skip the stop when ownership is unknown" meant skipping it in ordinary cases, leaving an active lifecycle and its reporter running after the viewer had gone. Every publication that takes a session now records it. Both were introduced by the fix for the previous round's findings, which is the third time in this sequence that a correction has needed correcting. Worth stating rather than smoothing over: on this subsystem, a fix written and not re-reviewed should be assumed broken. Still open and deliberately not attempted tonight: first-frame provenance via AnalyticsListener EventTime, and the conditional audio resolver using role flags. Both are policy-sensitive Media3 integration work whose failure modes are device-specific, and neither can be verified without hardware. The current ordinal precedence is unchanged and therefore no worse than before this work began; the frame baseline is now genuinely no worse. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…Ready Two attempts at first-frame provenance, both rejected on review, both now removed. The bug they were meant to fix is left in place and documented, because every cheap fix for it makes something else worse. WHY THE COUNTER BASELINE HAD TO GO. Media3 creates fresh DecoderCounters when a renderer is enabled, so a baseline of five thousand frames taken at mount can be compared against an incoming counter that restarted at zero — and the new stream would have to render five thousand more before it registered as having rendered at all. A healthy stream would sit there looking frozen until the decoder deadline fired. That trades a rare undetected freeze for a common invented one. Falling back to null when the count cannot be read is no better: it restores the first-sample capture, which swallows the first frame of any stream that renders promptly. An integer cannot tell cumulative continuation from counter replacement, and the earlier key-based attempt could not tell delivery time from provenance. Both were plausible and both were wrong. The callback goes back exactly as it was, with the reasoning recorded where the next person will look — the real fix is AnalyticsListener.onRenderedFirstFrame(EventTime) carried through a mount key on the MediaItem tag, and it is device-specific Media3 integration that should not be written without hardware to check it on. OWNERSHIP IS RETAINED WHERE THE SESSION IS FIRST KNOWN. The starter installs the lifecycle owner and starts its reporter before the view model publishes anything, and the publication path suspends. An exit inside that window found no session anywhere and skipped teardown entirely, stranding the lifecycle and its reporter. The id is now kept the moment Ready arrives, ahead of that suspension. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…orting Three of the findings that needed no hardware to settle. AUDIO DELAY EMITTED CHOPPED SOUND. The processor prepended silence AND passed the input through on every buffer until the head was paid off, so any delay longer than one decoder buffer came out as silence, audio, silence, audio — audible as half-rate, stuttering sound for the length of the offset, on exactly the correction someone applies when their lip sync is out. Silence now comes out in one unbroken run with the input held back until the head is paid, and frame-aligned so a partial frame cannot shift the channels against each other. The old test could not have caught it: it fed a single buffer larger than the entire delay, which is the one shape where the bug does not appear. The new test feeds the streaming shape and fails without the fix. SUBTITLE FILES LOST THEIR ACCENTS. Payloads were decoded leniently as UTF-8, which substitutes U+FFFD for anything else — and because a rewrite re-encodes what it decoded, the damage was permanent. A Windows-1252 subtitle needing cue renumbering came back with José as Jos<?> in the cues the viewer read. Decoding is strict now, with a Windows-1252 fallback for the legacy files this actually affects, and a payload that decodes as neither is left untouched rather than corrupted. A FAILED REFRESH REPORTED SUCCESS. refreshSubtitles returned Unit, so both callers incremented the completion nonce regardless — and both dialogs read that nonce as "the track merged and was selected" and closed themselves. A subtitle that downloaded or translated on the server, followed by a failed list request, therefore looked exactly like success while producing nothing. The outcome is returned and reported. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every episode transition on Android TV dead-ended on "Playback start was superseded." with a Retry button, despite auto-play being enabled. Reproduced 2/2 on a Google TV Streamer, with and without the HUD open. The screen has three teardown routes and two of them ran for the same exit. stopSessionForExit() is awaited before navigation, but onCleared() then issued a SECOND stop on the settlement scope. That one is untracked -- it does not go through stopAsync(), so acquireOwnershipEpoch()'s awaitPendingStop() cannot see it -- and it landed after the NEXT episode's start had captured its ownership epoch. By then the lifecycle had neither Active state nor a lastAdoptedSessionId, so the stop's ownership guard could not prove ownership had moved, fell through, and bumped stopEpoch. The incoming adoption was then rejected. PlaybackTeardownGate keeps that to exactly one stop per screen. It lives in android-shared because the concern is generic and phone has the same three-route shape. The detached routes go through the lifecycle-owned tracked job rather than a direct suspend stop: nothing awaits them -- onCleared's settlement callback even wraps its body in runCatching -- so a direct stop that threw would be swallowed with the claim consumed and no owner left to retry. That relocation made an unexpected throw escape stopAsync's launch, whose scope has a SupervisorJob but no CoroutineExceptionHandler, which on Android is process death rather than a lingering session. stopAsync now catches and logs non-cancellation failures. Also: Up Next rendered on top of an open HUD, since all three routes set showNextUp without clearing hudOpen and the HUD renders on hudOpen alone. Clearing it re-armed the 5s controls auto-hide underneath the 10s Up Next countdown, which hid the controls and pulled focus off the primary action, so the auto-hide effect now keys on and guards against showNextUp. Verified on device: E9 -> E10 advances cleanly. Not fixed here: phone has a similar non-awaited stop in onExit, but its onCleared fallback already uses the tracked stopAsync, and there is no phone available to verify a teardown change on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115uaQ6FTQZK8KYvazjefaW
Four things found by testing the player HUD on a Google TV Streamer. Label and value collided with literally zero gap -- "BackgroundNo background", "SubtitlesDanish - SRT - E...", and the same in the Info tab's STREAM list. Both rows separated label from value with a single Box(weight(1f)); Compose measures unweighted children first, so once the two texts filled the row that spacer resolved to 0dp. The gap is now fixed and unconditional, and the value region carries the weight so it is the side that ellipsizes. Inside HudFocusedSettingRow the value text is weighted too, otherwise a long value squeezes the trailing chevron toward zero. The "No background" row is gone. It was a second control over backgroundStyle -- the Background picker already offers "No background" -- and turning it off could not know what the style had been, so it hard-coded Box and destroyed the choice: Drop Shadow -> On -> Off left you on Box, persisted immediately. Its comment claimed Apple parity, but tvOS TVPlayerInfoHUD has no such toggle, only a Style picker and a Background color row. The Version picker rendered two identical "4K - DV" rows for two different files, since versionShortLabel is built from resolution and HDR/DV alone. versionPickerLabels disambiguates a list against itself, widening the attribute tuple (codec, then size, then container) until the colliding group is actually separated rather than requiring any one attribute to be unique. Non-colliding labels are untouched, and versions with nothing to tell them apart stay honestly duplicated. The detail selector already shows codec/size detail, so it is left alone. "Bitrate" was Media3's onBandwidthEstimate -- measured network throughput, not media bitrate -- which read as 151.3 Mbps for a ~19 Mbps stream on a fast LAN. Renamed to "Estimated bandwidth" on both TV and phone. Verified on device: gap and chevron restored on both row types, toggle gone, stats row renamed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115uaQ6FTQZK8KYvazjefaW
|
Warning Review limit reached
Next review available in: 40 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughPlayback lifecycle ownership, media parsing, subtitle decoding, TV playback handoff, HUD behavior, recovery feedback, and home-fetch ordering were updated. Regression tests cover teardown, delayed audio, PGS payload validation, subtitle decoding, version labels, and persisted audio preferences. ChangesPlayback lifecycle ownership
Media processing hardening
TV playback continuity and UI
Home fetch ordering
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant TvPlayerViewModel
participant PlaybackTeardownGate
participant PlaybackSessionLifecycle
participant NewPlaybackSession
TvPlayerViewModel->>PlaybackTeardownGate: stopOrdered(expectedSessionId)
PlaybackTeardownGate->>PlaybackSessionLifecycle: stop(expectedSessionId)
PlaybackTeardownGate->>PlaybackSessionLifecycle: claim detached teardown if ordered stop fails
PlaybackTeardownGate->>PlaybackSessionLifecycle: stopAsync(expectedSessionId)
NewPlaybackSession->>PlaybackSessionLifecycle: adopt new session
sequenceDiagram
participant TvPlayerViewModel
participant TvVideoPlaybackStarter
participant EpisodeSelectionHandoff
participant V3SessionRequest
TvPlayerViewModel->>EpisodeSelectionHandoff: capture explicit audio intent
TvVideoPlaybackStarter->>EpisodeSelectionHandoff: resolve target episode candidates
EpisodeSelectionHandoff-->>TvVideoPlaybackStarter: unique audioTrackIndex or null
TvVideoPlaybackStarter->>V3SessionRequest: send explicit or resolved audio selection
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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. Comment |
Contributor
Author
|
@coderabbitai review |
|
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/video/EpisodeSelectionHandoff.kt (1)
261-274: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winResolve and return the audio selection.
Line 270 constructs
ResolvedEpisodeSelectionwithoutaudioTrackIndex. This shared resolver drops an explicithandoff.audiochoice. Its callers then use the server-default audio track during an episode transition.Build candidates from the selected target version, call
resolveEpisodeAudioIntent, and assign the result toaudioTrackIndex. Add a focused test for an explicit audio handoff.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/video/EpisodeSelectionHandoff.kt` around lines 261 - 274, Update resolveEpisodeSelectionHandoff to derive audio candidates from the selected target version, resolve handoff?.audio with resolveEpisodeAudioIntent, and populate ResolvedEpisodeSelection.audioTrackIndex while preserving the existing source and subtitle resolution. Add a focused test verifying that an explicit audio handoff is retained during episode transition.
🧹 Nitpick comments (6)
android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/subtitle/PgsSupExtractor.kt (1)
240-249: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBind the caught
OutOfMemoryErroronly if you use it.detekt reports
SwallowedExceptionat Line 240. The log message does not referencee. Rename the binding to_to silence the rule, or include the throwable detail in the message.♻️ Proposed fix
- } catch (e: OutOfMemoryError) { + } catch (_: OutOfMemoryError) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/subtitle/PgsSupExtractor.kt` around lines 240 - 249, Update the OutOfMemoryError catch in the subtitle extraction block to avoid binding an unused exception: rename e to _ if the error details are intentionally omitted, or include e in the existing SubDiag.log call. Preserve the current malformed-set counting and null return behavior.Source: Linters/SAST tools
android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/subtitle/PgsSupExtractorTest.kt (1)
222-223: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that the accepted ODS payload reaches the parser.
The test asserts only the parse-call count. If a future change discards a valid ODS but still flushes the PCS and END segments, the parser is still called once and this test stays green. Assert that the captured bytes contain the ODS segment type to close that gap.
💚 Proposed addition
assertEquals(1, factory.parsed.size) + // The accepted object must survive into the set handed to the parser. + assertTrue( + factory.parsed.first().any { it == PgsSupExtractor.SEGMENT_TYPE_OBJECT.toByte() }, + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/subtitle/PgsSupExtractorTest.kt` around lines 222 - 223, Strengthen the test around the accepted ODS payload by asserting that the bytes captured in factory.parsed include the ODS segment type, not just that one parse call occurred. Keep the existing parsed.size assertion and use the test’s existing segment-type or byte-inspection symbols to verify the valid ODS reaches the parser.android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/PlaybackSessionLifecycleTest.kt (1)
979-1004: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd one positive assertion so the test cannot pass vacuously.
The test proves containment through
runTestfailing on an uncaught exception, which is sound. It does not prove the stop was attempted. If a regression madestopAsyncinert — no job launched — thenacquireOwnershipEpoch()would join a nullpendingStopJob, return immediately, and the test would still pass.Assert that
stopSessionwas actually called.💚 Proposed addition
fun `a failing async stop does not escape as an uncaught exception`() = runTest { + var attempts = 0 val sessionMgr = object : FakeSessionManager() { - override suspend fun stopSession(sessionId: String): ApiResult<Unit> = - throw IllegalStateException("stop failed") + override suspend fun stopSession(sessionId: String): ApiResult<Unit> { + attempts++ + throw IllegalStateException("stop failed") + } }lifecycle.stopAsync(expectedSessionId = "sess-a") // Joins the tracked job. If the failure escaped, runTest reports it. lifecycle.acquireOwnershipEpoch() + assertEquals(1, attempts, "the stop must actually have been attempted") }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/PlaybackSessionLifecycleTest.kt` around lines 979 - 1004, Update the test `a failing async stop does not escape as an uncaught exception` to track whether the overridden `stopSession` is invoked, then assert that it was called after `stopAsync` completes via `acquireOwnershipEpoch()`. Keep the existing exception-containment behavior and use the tracked invocation to prevent a vacuous pass.androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvPlayerViewModel.kt (1)
141-162: 📐 Maintainability & Code Quality | 🔵 TrivialTrack the UNRESOLVED audio-index precedence question outside the source.
This block documents a suspected defect — audio reverting to the first language after a replan — and states it is deliberately not fixed because
PlayerTrackEntriesTest.replanSelectionMapsMedia3OrdinalToStableServerAudioIndexpins the current order. A doc comment is not discoverable as work.Do you want me to open an issue that captures the two competing behaviours and the test that pins the current one?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvPlayerViewModel.kt` around lines 141 - 162, Remove the unresolved audio-index precedence discussion from the source comment and track it through an external issue instead. Keep only documentation that accurately describes the current behavior around the server catalog index, and preserve the existing implementation and PlayerTrackEntriesTest behavior.androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/performance/MobilePlayerLifecyclePerformanceSourceTest.kt (1)
29-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a negative assertion so an unqualified
stopAsynccannot return.The assertion confirms a qualified call exists. It does not prevent an unqualified
sessionLifecycle.stopAsync()from being added back elsewhere inPlayerViewModel.kt— both calls would coexist and the test would still pass.The TV counterpart pairs its positive check with
assertFalse(clearBody.contains("sessionLifecycle.stop"))(androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/player/TvSubtitleSettlementOwnershipTest.kt, Line 848). Mirror that here.💚 Proposed addition
assertTrue(viewModel.contains("sessionLifecycle.stopAsync(expectedSessionId =")) + // An unqualified stop disables the ownership guard entirely, which after a + // player-to-player navigation stops the session a newer screen adopted. + assertTrue(!viewModel.contains("sessionLifecycle.stopAsync()"))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/performance/MobilePlayerLifecyclePerformanceSourceTest.kt` around lines 29 - 33, Add a negative assertion alongside the existing positive check in MobilePlayerLifecyclePerformanceSourceTest to reject any unqualified sessionLifecycle.stopAsync call, mirroring the TV counterpart’s guard while preserving the qualified expectedSessionId assertion.androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/player/PlayerViewModel.kt (1)
953-965: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClear the retained token when the ready session is stopped as stale.
Line 960 retains
playbackState.sessionIdbefore the ownership re-check. IfownsLoad(loadOwner)is false, line 962 stops that session, butretainedOwnedSessionIdkeeps naming it. A lateronExit()then callssessionLifecycle.stop(expectedSessionId = <already-stopped id>).The lifecycle ownership guard prevents that stale stop from killing a newer adopted session, so this is not a cross-session kill. It still issues a redundant
stopSessionfor an id the server already closed, and it breaks the invariant this PR relies on: the retained token names a session this view model owns. The same gap exists on thestopStaleReadySessionbail-outs insideapplyCoordinatorStateToUi.♻️ Proposed change to release the token with the session
if (!ownsLoad(loadOwner)) { stopStaleReadySession(playbackState.sessionId) + if (retainedOwnedSessionId == playbackState.sessionId) { + retainedOwnedSessionId = null + } unpublishedReadySessionId = null return@launch }Apply the same release inside
stopStaleReadySessionitself if you prefer one place to own it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/player/PlayerViewModel.kt` around lines 953 - 965, Clear retainedOwnedSessionId whenever the ready session is stopped as stale, including the stopStaleReadySession bail-outs in applyCoordinatorStateToUi. Ensure the stale-session cleanup releases the token together with stopping playback, so onExit cannot later reuse an already-stopped session ID.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/subtitle/PgsSupExtractor.kt`:
- Around line 408-409: Update MAX_PGS_BITMAP_PIXELS in PgsSupExtractor to
support legitimate UHD PGS objects up to 3840×2160, preventing valid display
sets from being rejected by the pixel check. Preserve the allocation bound
implied by the UHD frame size; if the existing 1080p limit is intentionally
required for TV memory, retain it and document that tradeoff instead.
In
`@android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/video/PlaybackStartupStallDetector.kt`:
- Around line 47-55: Implement a per-session decoder-output baseline across
onMounted() and sample(): capture the existing decoderRenderedOutputBufferCount
when mounting, then determine first-frame output using the session delta (or an
equivalent session-scoped signal) so reused-player counts do not satisfy the
check. Preserve correct behavior when the counter resets, and add a regression
test covering a new session mounted with a pre-existing non-zero count.
In
`@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/detail/TvPlaybackFormatting.kt`:
- Around line 141-144: Update VERSION_DISCRIMINATORS to use
resolvedVideoCodec(v) for codec suffix generation, preserving the existing
blank-value and uppercase handling. Add a test covering versions with null
codecVideo values but distinct VideoTrack.codec values, verifying
versionPickerLabels produces distinct progressively disambiguated labels.
In
`@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvPlayerViewModel.kt`:
- Around line 2450-2498: Move the bounded-recovery KDoc above
onPlaybackRecoveryExhausted(), move the subtitle-acknowledgement KDoc above
onSubtitleFailureShown(), and leave the audio-selection KDoc above
onAudioSelectionCommitted(). Update onPlaybackRecoveryExhausted() so its
terminal error state also clears isLoading and isBuffering, matching the
existing terminal error publication behavior.
- Around line 2126-2151: Update the abandoned-session cleanup in the recovery
flow around abandonedSessionId to schedule abandonActiveVideoSession through
PlaybackSessionManager’s lifetime-independent sessionCleanupScope (or an async
release API that uses it), rather than viewModelScope.launch(NonCancellable).
After scheduling the cleanup, add an explicit return@launch so the abandoned
path exits without continuing the surrounding work.
In `@shared/src/commonMain/kotlin/org/siloserver/silo/viewmodel/HomeViewModel.kt`:
- Around line 78-90: Update loadSections() to capture fetchGeneration before any
cached bootstrap or suspension, and validate that captured generation before
publishing cached UI state. Pass the captured generation into fetchSections()
and remove the later generation increment there, ensuring older loads cannot
publish cache or network results after a newer refresh.
- Around line 166-168: Update fetchSections() and refresh() so refresh ownership
is tied to the fetch generation: return whether the fetch token is still
current, and move isRefreshing = false into refresh()’s finally block only when
that token remains current. Apply the same currentness handling to the early
returns around generation checks at the referenced locations, preventing
superseded refreshes from clearing state owned by a newer fetch.
---
Outside diff comments:
In
`@android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/video/EpisodeSelectionHandoff.kt`:
- Around line 261-274: Update resolveEpisodeSelectionHandoff to derive audio
candidates from the selected target version, resolve handoff?.audio with
resolveEpisodeAudioIntent, and populate ResolvedEpisodeSelection.audioTrackIndex
while preserving the existing source and subtitle resolution. Add a focused test
verifying that an explicit audio handoff is retained during episode transition.
---
Nitpick comments:
In
`@android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/subtitle/PgsSupExtractor.kt`:
- Around line 240-249: Update the OutOfMemoryError catch in the subtitle
extraction block to avoid binding an unused exception: rename e to _ if the
error details are intentionally omitted, or include e in the existing
SubDiag.log call. Preserve the current malformed-set counting and null return
behavior.
In
`@android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/PlaybackSessionLifecycleTest.kt`:
- Around line 979-1004: Update the test `a failing async stop does not escape as
an uncaught exception` to track whether the overridden `stopSession` is invoked,
then assert that it was called after `stopAsync` completes via
`acquireOwnershipEpoch()`. Keep the existing exception-containment behavior and
use the tracked invocation to prevent a vacuous pass.
In
`@android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/subtitle/PgsSupExtractorTest.kt`:
- Around line 222-223: Strengthen the test around the accepted ODS payload by
asserting that the bytes captured in factory.parsed include the ODS segment
type, not just that one parse call occurred. Keep the existing parsed.size
assertion and use the test’s existing segment-type or byte-inspection symbols to
verify the valid ODS reaches the parser.
In
`@androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/player/PlayerViewModel.kt`:
- Around line 953-965: Clear retainedOwnedSessionId whenever the ready session
is stopped as stale, including the stopStaleReadySession bail-outs in
applyCoordinatorStateToUi. Ensure the stale-session cleanup releases the token
together with stopping playback, so onExit cannot later reuse an already-stopped
session ID.
In
`@androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/performance/MobilePlayerLifecyclePerformanceSourceTest.kt`:
- Around line 29-33: Add a negative assertion alongside the existing positive
check in MobilePlayerLifecyclePerformanceSourceTest to reject any unqualified
sessionLifecycle.stopAsync call, mirroring the TV counterpart’s guard while
preserving the qualified expectedSessionId assertion.
In
`@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvPlayerViewModel.kt`:
- Around line 141-162: Remove the unresolved audio-index precedence discussion
from the source comment and track it through an external issue instead. Keep
only documentation that accurately describes the current behavior around the
server catalog index, and preserve the existing implementation and
PlayerTrackEntriesTest behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 71209d7f-bc85-415d-bd11-fceb66709b77
📒 Files selected for processing (27)
android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/PlaybackSessionLifecycle.ktandroid-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/PlaybackTeardownGate.ktandroid-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/audio/DelayAudioProcessor.ktandroid-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/subtitle/PgsSupExtractor.ktandroid-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/subtitle/SubripPayloadNormalizer.ktandroid-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/video/EpisodeSelectionHandoff.ktandroid-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/video/PlaybackStartupStallDetector.ktandroid-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/video/PostResumeVideoStallDetector.ktandroid-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/PlaybackSessionLifecycleTest.ktandroid-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/audio/DelayAudioProcessorTest.ktandroid-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/subtitle/PgsSupExtractorTest.ktandroid-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/subtitle/SubripPayloadNormalizerTest.ktandroid-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/video/PostResumeVideoStallDetectorTest.ktandroidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/player/PlaybackStatsSheet.ktandroidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/player/PlayerViewModel.ktandroidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/performance/MobilePlayerLifecyclePerformanceSourceTest.ktandroidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/player/MobileSubtitleTransactionAdapterTest.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/detail/TvPlaybackFormatting.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvPlayerHud.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvPlayerScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvPlayerViewModel.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvVideoPlaybackStarter.ktandroidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/detail/TvPlaybackFormattingTest.ktandroidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/player/TvSubtitleSettlementOwnershipTest.ktandroidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/player/TvSubtitleTransactionAdapterTest.ktshared/src/commonMain/kotlin/org/siloserver/silo/model/playback/SubtitleTransition.ktshared/src/commonMain/kotlin/org/siloserver/silo/viewmodel/HomeViewModel.kt
💤 Files with no reviewable changes (1)
- android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/player/video/PostResumeVideoStallDetectorTest.kt
Comment on lines
+408
to
+409
| /** A full-frame 1080p caption is allowed; a 4K one is not, on TV memory. */ | ||
| private const val MAX_PGS_BITMAP_PIXELS = 1920L * 1080L |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Confirm the 1080p pixel budget does not drop legitimate UHD captions.
MAX_PGS_BITMAP_PIXELS is 1920 * 1080. UHD Blu-ray PGS sidecars commonly declare objects up to 3840x2160. Such an object fails the pixel check at Line 403, so the extractor drops the display set and the caption never renders. The user sees missing subtitles with only a diagnostic log.
If the budget is a deliberate TV-memory limit, keep it and record the tradeoff. If not, raise the budget to the UHD frame size, which still bounds the allocation at about 33 MB for the IntArray.
🔧 Proposed change
-/** A full-frame 1080p caption is allowed; a 4K one is not, on TV memory. */
-private const val MAX_PGS_BITMAP_PIXELS = 1920L * 1080L
+/** A full-frame UHD caption is allowed; anything larger is not real content. */
+private const val MAX_PGS_BITMAP_PIXELS = 3840L * 2160L📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /** A full-frame 1080p caption is allowed; a 4K one is not, on TV memory. */ | |
| private const val MAX_PGS_BITMAP_PIXELS = 1920L * 1080L | |
| /** A full-frame UHD caption is allowed; anything larger is not real content. */ | |
| private const val MAX_PGS_BITMAP_PIXELS = 3840L * 2160L |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/subtitle/PgsSupExtractor.kt`
around lines 408 - 409, Update MAX_PGS_BITMAP_PIXELS in PgsSupExtractor to
support legitimate UHD PGS objects up to 3840×2160, preventing valid display
sets from being rejected by the pixel check. Preserve the allocation bound
implied by the UHD frame size; if the existing 1080p limit is intentionally
required for TV memory, retain it and document that tradeoff instead.
Comment on lines
47
to
55
| // Re-baselined on arm. The counters are cumulative on the player, so | ||
| // "greater than zero" would be satisfied instantly by the previous | ||
| // attempt's frames when a player is reused. | ||
| this.decoderStartupAtMs = null | ||
| this.paused = false | ||
| this.lastProgressPositionMs = this.startPositionMs | ||
| this.lastBufferedPositionMs = this.startPositionMs | ||
| this.lastProgressAtMs = nowMs | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Implement the per-session decoder baseline that this comment describes.
onMounted() resets decoderStartupAtMs, but it does not store or apply a decoder-output baseline. sample() still treats any raw decoderRenderedOutputBufferCount > 0 as current-session output at Line [99]. If a reused player carries a prior non-zero count, the new session sets firstFrameRendered before it renders. This bypasses the decoder startup deadline and can prevent recovery for a stalled track.
Pass a session-scoped first-frame signal or use a counter delta with a counter-reset-safe contract. Add a regression test for mounting a new session with a pre-existing non-zero counter.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@android-shared/src/androidMain/kotlin/org/siloserver/silo/common/player/video/PlaybackStartupStallDetector.kt`
around lines 47 - 55, Implement a per-session decoder-output baseline across
onMounted() and sample(): capture the existing decoderRenderedOutputBufferCount
when mounting, then determine first-frame output using the session delta (or an
equivalent session-scoped signal) so reused-player counts do not satisfy the
check. Preserve correct behavior when the counter resets, and add a regression
test covering a new session mounted with a pre-existing non-zero count.
Comment on lines
+166
to
+168
| // Superseded while in flight: a newer fetch has already | ||
| // answered, so this reply describes a home nobody is looking at. | ||
| if (generation != fetchGeneration) return |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Keep isRefreshing owned by the current refresh.
When an older refresh() reaches one of these early returns, its caller still executes the isRefreshing = false update at Line 127. A newer fetch can still be active. The UI can hide the refresh state, and refreshFromRealtime() can start an extra request.
Return the fetch token or currentness from fetchSections(). Clear isRefreshing in finally only when that token is still current.
Also applies to: 210-211, 222-222
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@shared/src/commonMain/kotlin/org/siloserver/silo/viewmodel/HomeViewModel.kt`
around lines 166 - 168, Update fetchSections() and refresh() so refresh
ownership is tied to the fetch generation: return whether the fetch token is
still current, and move isRefreshing = false into refresh()’s finally block only
when that token remains current. Apply the same currentness handling to the
early returns around generation checks at the referenced locations, preventing
superseded refreshes from clearing state owned by a newer fetch.
HomeViewModel — two ordering defects - loadSections() read and overlaid the cache before fetchSections() claimed a generation. That read suspends, so a refresh could publish fresh sections while it was in there and the cached overlay would then put stale rows back on screen. The generation is captured before the read; if it moved, the cached publish is skipped and we go straight to the fetch. - refresh() cleared isRefreshing unconditionally, so a superseded refresh hid the spinner while a newer fetch was still running — and re-opened refreshFromRealtime's single-flight gate, letting it fire a redundant request. fetchSections now reports the generation it ran as, and only the newest clears the flag. TvPlayerViewModel - Abandoned sessions are released on the manager's own cleanup scope instead of viewModelScope.launch(NonCancellable), which severs the parent link to produce a coroutine nothing can await or observe failures from. That scope already outlives any screen. The new manager entry point only stops while the manager still owns the session: the unconditional variant stops even when it failed to disown, and stopSession's predecessor branch then clears a newer pending publication and stops its replacement — a real hazard once the release is dispatched rather than inline, because the window widens. When ownership has moved on the id is recorded as an orphan instead. - Two KDoc blocks sat in a run of three before one function, so Kotlin attached only the last and the other two documented nothing. Moved onto onSubtitleFailureShown and onPlaybackRecoveryExhausted, which had no docs of their own. TvPlaybackFormatting - Version labels resolve the codec through resolvedVideoCodec, so a version whose codec lives only on its video track can discriminate. Doing that alone regressed the "indistinguishable versions stay equal" contract: two identical versions both gained the same suffix, a fabricated difference that separates nothing. An attribute tuple is now kept only when it actually distinguishes something. PlaybackStartupStallDetector — comment corrected, code deliberately not - The review asked for the per-session decoder baseline the comment described. That baseline was implemented and reverted earlier on this same branch (6bd3bd5): Media3 creates fresh DecoderCounters when a renderer is enabled, so one captured at mount can be compared against a counter that restarted at zero, and a healthy stream then looks frozen until it has rendered as many frames again — trading a rare missed freeze for a common invented one. The comment claiming "re-baselined on arm" was what invited the suggestion; it now states plainly that only the startup deadline is cleared, what was tried, why it went, and that the real fix is AnalyticsListener.onRenderedFirstFrame(EventTime) through a mount key, which needs hardware to validate. Full suite green on all four modules. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115uaQ6FTQZK8KYvazjefaW
Contributor
Author
|
Addressed in 14e1a4b. Six of the seven are fixed; one is a deliberate decline with reasoning. Fixed
Declined, with reasoning: the decoder baseline in That baseline was implemented and reverted earlier on this same branch, in 6bd3bd5. Media3 creates fresh The comment claiming "re-baselined on arm" is what invited the suggestion, and it was wrong — only the startup deadline is cleared. It now says so plainly, records what was tried and why it went, and names the real fix: Not changed: the PGS 1080p pixel budget Deliberate, and the constant's comment already says so. Raising it to admit a 3840×2160 object means a ~33 MB ARGB_8888 bitmap per cue on memory-constrained TV hardware. If UHD PGS sidecars turn out to matter in practice, the right fix is downscaling at decode rather than lifting the ceiling — happy to take that as a follow-up if you've seen it drop real captions. Full suite green on all four modules. One flake surfaced under parallel load in |
This was referenced Aug 6, 2026
This was referenced Aug 6, 2026
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.
First of three follow-ups to #168, split by theme so each stays reviewable. Player robustness; the audio-selection and profiles/navigation work follows in its own PR.
Playback survives bad input
A malformed caption could kill playback outright. Subtitle bitmaps arriving from the wire are now rejected rather than trusted, and track identity is no longer misread — both cases previously took the whole session down rather than dropping the track.
Sessions stop when their screen does
Server sessions could outlive the screen that started them: cancelled mid-start, superseded by a replan, or abandoned on an error path that returned before teardown. Ownership is now retained at Ready, and every session the screen adopts is owned — so it is stopped exactly once, by whoever owns it.
A frame-baseline attempt at the same problem is included here as its own commit and its revert. It compared decoder counters against a mount baseline, which fails because Media3 creates fresh
DecoderCounterswhen a renderer is enabled — an outgoing count against a restarted counter makes a healthy stream look frozen until it has rendered as many frames again. That trades a rare missed freeze for a common false one. It is left in the history rather than squashed away because the reasoning is worth keeping.Overlapping refreshes
fetchSections()publishers could overlap — a resume observer firing while an initial load is still running — and each capturedhadSectionsbefore its network call. An older partial response could then land after a newer complete one, replace good sections, and mark them not fully resolved, which also told TV focus restoration to keep waiting for rows that had already arrived. AfetchGenerationguard now serialises them: every fetch is stamped, and a result is dropped if a newer one has started.This closes the
HomeViewModelfinding CodeRabbit raised on #168, which merged before the fix.Also
Audio delay, subtitle encodings and refresh reporting were unbroken; auto-play next episode was unbroken; the TV HUD's row layout, version labels and stats naming were repaired.
Verification
All four module suites pass and both APKs assemble. Fixes carry regression tests where the boundary is testable.
Summary by CodeRabbit