From c18d79c656b11d9bc2f5655f8feaabb95896548b Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Wed, 19 Aug 2026 17:29:31 +0200 Subject: [PATCH] fix(android): stop background audio rewinding to where it started MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A video handed off to background audio (UR-040) streams a live mp3 transcode over plain HTTP. That response is chunked, so there is no Content-Length, and a live encode carries no Xing header, so the extractor establishes no duration — on device every position tick reads " / 0.0". ProgressiveMediaPeriod.configureRetry resumes a failed load in place only when the content length is known or the seek map has a duration. With neither it assumes the source is live, sets pendingDeferredRetry, and when the sample queues next run dry resets them and re-requests the URL from offset 0. Our URL carries StartTimeTicks = the handoff point, so "offset 0" is where audio-only mode began: a transient load error armed a retry that fired minutes later, when the buffer finally drained, and playback resumed at the handoff point and ran on from there. A successful retry raises no error and ends nothing, so neither arm of DR-129 was consulted and no discontinuity handler existed — the only trace was a position that went backwards, which is why it read as random, and why the two earlier fixes for the same symptom (DR-129's phantom end, DR-159's relative-timeline leak) left it standing. A retry that can only restart the stream is worth less than no retry at all. player_retry_restarts_stream marks a Remote audio-only video item, loadWithMetadata carries the answer to Kotlin, and the pure StreamRetryDecision holds it for a DefaultLoadErrorHandlingPolicy that returns C.TIME_UNSET — making onLoadError answer DONT_RETRY_FATAL before it reaches configureRetry. The rewind becomes a recoverable error, which recoverable_error_resume already answers by re-opening at the position playback reached, StartTimeTicks rewritten so the selected audio track survives. Every other source keeps the player's retry: a static file and an HLS playlist declare their timeline and are resumed where the load stopped. onPositionDiscontinuity is added for its log line alone, loud for DISCONTINUITY_REASON_INTERNAL, which is the rewind's own signature. Verified on device (FP5), same procedure both runs — handoff, 60s to fill the buffer, a 45s radio outage: before 13:54:52 BUFFERING, then "Media ready! Duration: -9.22e15" (C.TIME_UNSET) and position 1165.4s -> 840.349s, exactly the handoff base, 3.5 minutes after the outage with nothing logged between after 14:05:08 "declining the player's retry", playback undisturbed off the buffer for 69s (a fatal load error is only raised when the renderer next needs data), then ERROR_CODE_IO_NETWORK_CONNECTION_FAILED -> re-opening at 785.6s -> READY, and no rewind in the following 7 min Kotlin tests run with ./gradlew :app:testUniversalDebugUnitTest. TRACES: UR-040, UR-004 | DR-203 | UT-200 --- docs/defect-windows.md | 1 + docs/requirements.md | 6 +- docs/traceability.md | 8034 +++++++++-------- scripts/extract-traces.test.ts | 7 +- .../jellytau/player/JellyTauPlayer.kt | 83 +- .../jellytau/player/StreamRetryDecision.kt | 50 + .../player/StreamRetryDecisionTest.kt | 47 + src-tauri/src/player/android/mod.rs | 7 +- src-tauri/src/player/mod.rs | 3 +- src-tauri/src/player/stream_end.rs | 131 + 10 files changed, 4441 insertions(+), 3928 deletions(-) create mode 100644 src-tauri/android/src/main/java/com/dtourolle/jellytau/player/StreamRetryDecision.kt create mode 100644 src-tauri/android/src/test/java/com/dtourolle/jellytau/player/StreamRetryDecisionTest.kt diff --git a/docs/defect-windows.md b/docs/defect-windows.md index e5c1c9c5..43c418c0 100644 --- a/docs/defect-windows.md +++ b/docs/defect-windows.md @@ -80,6 +80,7 @@ silently correct an out-of-range index — which is exactly why it was reported | Stop-report path never fed the sync queue that existed for it (DR-154) | v0.4.6 | **v0.5.1** | feature (queue + drain landed with no producer) | | Background-audio base applied in two display-only places (DR-159) | v0.2.9 | **v0.5.3** | pickaxe | | Positions reported as 0 before the first tick, and always 0 for webview media (DR-178/179/180) | v0.5.3 | **v0.5.5** | feature (DR-159's tick boundary) | +| Length-less handoff transcode left to the player's own load-error retry, which can only restart it (DR-203) | v0.0.16 | **v0.8.2** | feature (the handoff's progressive-mp3 choice) | Three of these are worth separating out, because the defect is not a mistake in the code so much as **plumbing that was built and never connected**: diff --git a/docs/requirements.md b/docs/requirements.md index 5faeee00..9cee1835 100644 --- a/docs/requirements.md +++ b/docs/requirements.md @@ -376,6 +376,7 @@ Internal architecture, components, and application logic. | DR-200 | The lockscreen notification is exempt from `POST_NOTIFICATIONS`, because of the **session token**, not because it belongs to a foreground service — and the difference is what the code now records. `POST_NOTIFICATIONS` was declared in the manifest and requested nowhere, so on Android 13+ it sat permanently denied; an audit read that as a threat to UR-006, since the media notification is what carries the lockscreen transport controls. It is not. Android's own wording is that the permission covers "non-exempt (including Foreground Services (FGS)) notifications", with denied users seeing FGS notices "in the Task Manager but [not] in the notification drawer" — so an FGS notification is explicitly *not* exempt — while separately "Notifications related to media sessions are exempt from this behavior change". The platform predicate is `Notification.isMediaNotification()`, which requires `MediaStyle` **and** a non-null `EXTRA_MEDIA_SESSION`, and it is byte-identical across API 33–36. `NotificationManagerService` uses it to decide whether to drop the post, and SystemUI's media carousel (`MediaDataProcessor.onNotificationAdded`) is gated on the *same* predicate — so a token-less notification is not merely absent from the shade, it never reaches the notification listener and the lockscreen/Quick-Settings controls do not exist at all. Confirmed on device (HONOR ROD2-W09, Android 16 / SDK 36): appops `POST_NOTIFICATION: ignore`, `granted=false`, and the service simultaneously `isForeground=true` with `foregroundNoti=Notification(category=transport actions=3 vis=PUBLIC)`. So **no runtime permission request is added** — a prompt the app does not need is a prompt that can be permanently denied for nothing — and no `checkSelfPermission` gate is placed on `startForeground`, which would trade a cosmetic problem for the "did not then call Service.startForeground()" kill. What is added is the guard that matches the real precondition: `mediaSessionCompat?.sessionToken` is a null-safe call, and the exemption hangs entirely on it, so both builders now bind the token once and log an error if it is ever null while the permission is denied — converting a failure that is invisible unless the tester happened to deny the permission (most grant it reflexively) into a logcat line. The manifest declaration is *kept*, unrequested, and documented: media3 does not need it (media3-session declares no permissions and the `MediaSessionService` guide asks only for the two `FOREGROUND_SERVICE` ones), but the exemption covers media and self-managed-call notifications only, so a download-completion notice (UR-011) would be an ordinary notification and silently dropped — keeping the declaration is what makes adding one a one-file change | Android | UR-006 | Done | | DR-201 | A lockscreen skip means different things depending on what is playing, and the backend decides which. `onSkipToNext`/`onSkipToPrevious` forwarded a bare `"next"`/`"previous"` to Rust, which always advanced the queue — correct for music, wrong for a video whose audio is running through a background-audio handoff (UR-040), where the buttons should scrub. Pressing skip to re-hear a line jumped to the next *episode* instead. `resolve_skip_action` in `player/seek.rs` maps the command to either `Advance` or `SeekTo`, and `is_background_audio_active()` is the whole test: the handoff exists only for video, and an episode played through it reports `MediaType::Audio`, so media type cannot distinguish the case. Forward jumps 30s, back 10s — asymmetric because the back button replays dialogue just missed rather than travels — and both clamp to `[0, duration]`, since a negative offset is rejected by backends and a seek past the end reads as EOF and would advance, the very outcome being prevented. Routed through the same spawn-then-`seek_absolute` path as the scrubber, because a handoff seek re-opens the stream and must not run under the blocking lock (DR-159). The Kotlin keeps sending the same opaque command; only the `PlaybackStateCompat` gains `ACTION_FAST_FORWARD`/`ACTION_REWIND` so the system draws seek affordances rather than skip arrows that lie about what they do | Playback | UR-040, UR-006 | Done | | DR-202 | Video keeps the display awake. Android counts its display timeout from the last *user input*, and watching something is exactly the case where there is none, so the screen dimmed and slept mid-film unless the user kept tapping it. Nothing held it: `FLAG_KEEP_SCREEN_ON` appeared nowhere in the app, and neither renderer supplies a hold for free — ExoPlayer's `setWakeMode` is a CPU/wifi wake lock that says nothing about the display, and it draws into the `TextureView` this app owns (DR-192) rather than media3's `PlayerView`, which is the widget that would otherwise set `keepScreenOn` itself; the WebView `