Sulkta-Coop/straw
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

vc=62: audit-fix sprint on playback regressions

Browse source 
Opus max-effort audit on vc=53-vc=61 diff caught four interlocked
playback bugs. Cobb's 'video bugs are back' likely lived in the
intersection of #1+#2 — stale auto-resume seek + no recovery path.

BUG-1: setPlayingFrom clamps auto-resume against entry.durationMs.
YouTube can replace a video at the same videoId with a shorter cut
(live->VOD trim, premiere edit). Without the clamp, setMediaItem
seeks past the new end, ExoPlayer fires onPlayerError, NowPlaying
clears, surface locks at thumbnail+spinner. Clamp at lookup uses
the recorded duration with a 5s safety margin; falls back to 0
when out of range.

BUG-2: InlinePlayer adds a Retry button on the playback-error
branch. Tapping it nulls playbackError + bumps a retryVersion that
re-keys the setPlayingFrom LaunchedEffect. Previously the screen
locked into the error message forever (no UI affordance to
re-attempt; LaunchedEffect's keys never changed). Bonus protection:
the manual retry path avoids the infinite-error-loop risk a
NowPlaying-keyed auto-retry would have created.

BUG-4: captureResumePosition now gates strictly on STATE_READY.
STATE_BUFFERING during a fresh setMediaItem reports the PREVIOUS
item's position via currentPosition — the 5s poll was happily
writing A's tail position under B's videoId in that window. Next
auto-resume would drop the user mid-A on a fresh open of B.

BUG-5: onMediaItemTransition falls back to MediaItem.mediaMetadata
when Queue.at(idx) is null. Without the fallback, a Queue/controller
desync would leave NowPlaying stuck on the previous item forever,
freezing controllerOnThisVideo at false and locking the inline
player into thumbnail+spinner on the next screen.
This commit is contained in:
Kayos 2026-05-26 12:31:27 -07:00
parent 6cc789a8a0
commit 6775f8252f
4 changed files with 74 additions and 14 deletions
Download patch file Download diff file Expand all files Collapse all files

30
strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailScreen.kt
View file

@ -67,6 +67,7 @@ import androidx.compose.runtime.DisposableEffect
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableIntStateOf
import androidx.compose.runtime.mutableFloatStateOf
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
@ -741,8 +742,14 @@ private fun InlinePlayer(
// Push the resolved stream into the shared controller if it isn't
// already playing this URL. We don't kick off a new fetch — the
// outer VideoDetailScreen already called vm.load(streamUrl).
//
// retryVersion lets the user manually re-fire setPlayingFrom after
// a playback error. Without it, the screen used to lock into the
// thumbnail+spinner branch once NowPlaying.clear() fired from
// onPlayerError. vc=62 audit BUG-2.
val resolved = state.resolved
LaunchedEffect(controller, resolved, streamUrl) {
var retryVersion by remember(streamUrl) { mutableIntStateOf(0) }
LaunchedEffect(controller, resolved, streamUrl, retryVersion) {
val c = controller ?: return@LaunchedEffect
val r = resolved ?: return@LaunchedEffect
// Optimization, not safety. claim() guards the race.
@ -792,11 +799,24 @@ private fun InlinePlayer(
color = MaterialTheme.colorScheme.error,
modifier = Modifier.padding(16.dp),
)
playbackError != null -> Text(
"playback error: $playbackError",
color = MaterialTheme.colorScheme.error,
playbackError != null -> Column(
modifier = Modifier.padding(16.dp),
)
horizontalAlignment = Alignment.CenterHorizontally,
) {
Text(
"playback error: $playbackError",
color = MaterialTheme.colorScheme.error,
)
Spacer(modifier = Modifier.height(12.dp))
OutlinedButton(onClick = {
// Clear the error AND nudge the LaunchedEffect to
// re-attempt setPlayingFrom. vc=62 audit BUG-2 —
// without this the screen used to lock on the
// error forever after NowPlaying.clear().
playbackError = null
retryVersion += 1
}) { Text("Retry") }
}
resolved?.isPlayable != true -> Text(
"no playable stream",
color = Color.White,

39
strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlaybackService.kt
View file

@ -158,12 +158,32 @@ class PlaybackService : MediaSessionService() {
override fun onMediaItemTransition(item: MediaItem?, reason: Int) {
if (item == null) return
val idx = player.currentMediaItemIndex
val queued = Queue.at(idx) ?: return
NowPlaying.claim(queued)
if (queued.segments.isEmpty()) {
val videoId = com.sulkta.straw.feature.detail.extractYtVideoId(queued.streamUrl)
if (!videoId.isNullOrBlank()) fetchSbForQueued(queued, videoId)
val queued = Queue.at(idx)
if (queued != null) {
NowPlaying.claim(queued)
if (queued.segments.isEmpty()) {
val videoId =
com.sulkta.straw.feature.detail.extractYtVideoId(queued.streamUrl)
if (!videoId.isNullOrBlank()) fetchSbForQueued(queued, videoId)
}
return
}
// Queue desync — MediaItem was added by a path that
// bypassed enqueueInternal, OR the queue was cleared
// while a transition was pending. Fall back to the
// MediaItem's own metadata so NowPlaying doesn't stay
// stuck on the previous video forever (would freeze
// VideoDetail's controllerOnThisVideo guard at false
// and lock the inline player into thumbnail+spinner).
// vc=62 audit BUG-5.
val uri = item.localConfiguration?.uri?.toString() ?: return
val fallback = NowPlayingItem(
streamUrl = uri,
title = item.mediaMetadata.title?.toString().orEmpty(),
uploader = item.mediaMetadata.artist?.toString().orEmpty(),
thumbnail = item.mediaMetadata.artworkUri?.toString(),
)
NowPlaying.claim(fallback)
}
override fun onIsPlayingChanged(isPlaying: Boolean) {
@ -205,10 +225,17 @@ class PlaybackService : MediaSessionService() {
* ResumePositionsStore. Bails on idle/ended states and unknown
* durations (live streams). The store itself enforces minimum-
* position + near-end-clear thresholds.
*
* Gates STRICTLY on STATE_READY. STATE_BUFFERING during a fresh
* setMediaItem still reports the PREVIOUS item's position via
* currentPosition until prepare finishes and the new timeline
* lands — without the gate we'd record A's tail position under
* B's videoId and auto-resume the user mid-A on next open.
* vc=62 audit BUG-4.
*/
private fun captureResumePosition(player: Player) {
val state = player.playbackState
if (state == Player.STATE_IDLE || state == Player.STATE_ENDED) return
if (state != Player.STATE_READY) return
val item = NowPlaying.current.value ?: return
val videoId = com.sulkta.straw.feature.detail.extractYtVideoId(item.streamUrl) ?: return
val pos = player.currentPosition

15
strawApp/src/main/kotlin/com/sulkta/straw/feature/player/StrawMediaController.kt
View file

@ -121,9 +121,22 @@ fun Player.setPlayingFrom(
// an app update / process death. The store skips trivial
// positions and clears near-end so we don't auto-resume to 0:03
// or to the credits.
//
// Clamp the resume position against the RECORDED duration with a
// safety margin. vc=62 audit BUG-1: YouTube can replace a video
// at the same videoId with a shorter cut (live→VOD trim, premiere
// edit, channel replace) — without the clamp, setMediaItem seeks
// past the new end, ExoPlayer fires onPlayerError, the screen
// ends up stuck on the thumbnail+spinner (BUG-2 cascade).
val effectiveStart = if (startPositionMs == 0L && Settings.get().autoResume.value) {
val videoId = extractYtVideoId(streamUrl)
videoId?.let { Resume.get().get(it)?.positionMs } ?: 0L
val saved = videoId?.let { Resume.get().get(it) }
if (saved == null) {
0L
} else {
val safeCeiling = saved.durationMs - 5_000L
if (saved.positionMs in 1L..safeCeiling) saved.positionMs else 0L
}
} else {
startPositionMs
}