From d1ee9379e070cd86a260e8c2861e22d5db6ada03 Mon Sep 17 00:00:00 2001 From: Kayos Date: Mon, 25 May 2026 13:43:45 -0700 Subject: [PATCH] =?UTF-8?q?vc=3D36:=20audit-fix=20tail=20=E2=80=94=20atomi?= =?UTF-8?q?c=20setPlayingFrom,=20cache=20wipe,=20polish?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deferred items from the vc=35 audit-fix sprint. Smaller surface, real impact: HIGH-C6 — atomic setPlayingFrom claim StrawMediaController.setPlayingFrom previously did if (NowPlaying.current.value?.streamUrl == streamUrl) return setMediaItem(...); prepare(); play() NowPlaying.set(...) When the inline player and fullscreen Player effects fired in the same composition pass (an inline → fullscreen transition), both checks could see the stale NowPlaying value, both passed the guard, both ran setMediaItem + prepare + play. Result: an audible "did the video just restart?" stutter that was hard to reproduce. New: NowPlaying.claim(item) uses MutableStateFlow.compareAndSet in a CAS loop. Returns true ONLY for the caller that won the race; losing caller bails before touching the controller. The guard is now actually atomic, not a check-then-set. MED-Q11 — minibar surfaces playback errors Background button takes the user to Home with audio continuing in the foreground service. If that audio then fails (transient network drop on the resolved URL), neither the inline-player error listener nor PlayerScreen's exist anymore — only the minibar is observing. Added onPlayerError to MinibarOverlay's listener: Toast the errorCodeName + clear NowPlaying so the minibar hides itself rather than claiming a dead session is loaded. MED-Q15 — pre-compute recencyScore once mergeFromCache's compareByDescending invoked recencyScore() twice per pair (compareBy semantics), so ~1800 regex matches on a 900- item merge. Pair the score with the item once, sort the pair, take the items back. N matches. MED-C13 — Settings cache-wipe also clears in-memory VM SubscriptionFeedViewModel.clearInMemoryCache() exposed; Settings's Switch.onCheckedChange(false) now calls it alongside the disk wipe. Without this the feed kept rendering its in-memory mirror until process death. MED-C5 — drop StrawHome.formatDurationShort Near-duplicate of util.formatDuration. Used util's version + the existing `if (durationSeconds > 0)` guard at the call site already produces identical output (util returns "" on sec <= 0). MED-C19 — drop unused Surface import in StrawHome. NowPlaying gained one public method (claim). Everything else is internal-only churn. --- buildSrc/src/main/kotlin/ProjectConfig.kt | 4 +-- .../main/kotlin/com/sulkta/straw/StrawHome.kt | 11 ++----- .../feature/feed/SubscriptionFeedViewModel.kt | 31 ++++++++++++++----- .../straw/feature/player/MinibarOverlay.kt | 14 +++++++++ .../sulkta/straw/feature/player/NowPlaying.kt | 25 +++++++++++++++ .../feature/player/StrawMediaController.kt | 15 ++++++--- .../straw/feature/settings/SettingsScreen.kt | 7 +++++ 7 files changed, 83 insertions(+), 24 deletions(-) diff --git a/buildSrc/src/main/kotlin/ProjectConfig.kt b/buildSrc/src/main/kotlin/ProjectConfig.kt index 4ba83af96..901e66c2e 100644 --- a/buildSrc/src/main/kotlin/ProjectConfig.kt +++ b/buildSrc/src/main/kotlin/ProjectConfig.kt @@ -55,6 +55,6 @@ const val NEWPIPE_APPLICATION_ID_NEW = "net.newpipe.app" // vc=19 / 0.1.0-AE — rust pipeline cutover. Extraction via // strawcore-core (Sulkta-Coop/strawcore) via the UniFFI wrapper; no // NewPipeExtractor in the runtime path. -const val STRAW_VERSION_CODE = 35 -const val STRAW_VERSION_NAME = "0.1.0-AU" +const val STRAW_VERSION_CODE = 36 +const val STRAW_VERSION_NAME = "0.1.0-AV" const val STRAW_APPLICATION_ID = "com.sulkta.straw" diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/StrawHome.kt b/strawApp/src/main/kotlin/com/sulkta/straw/StrawHome.kt index ff2d26d34..2461a8182 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/StrawHome.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/StrawHome.kt @@ -48,7 +48,6 @@ import androidx.compose.material3.ModalDrawerSheet import androidx.compose.material3.ModalNavigationDrawer import androidx.compose.material3.NavigationDrawerItem import androidx.compose.material3.Scaffold -import androidx.compose.material3.Surface import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.material3.TopAppBar @@ -81,6 +80,7 @@ import com.sulkta.straw.data.WatchHistoryItem import com.sulkta.straw.feature.feed.SubscriptionFeedViewModel import com.sulkta.straw.feature.search.StreamItem import com.sulkta.straw.OverlayDimColor +import com.sulkta.straw.util.formatDuration import com.sulkta.straw.util.formatViews import kotlinx.coroutines.launch @@ -524,7 +524,7 @@ private fun ThumbnailWithDuration( ) if (durationSeconds > 0) { Text( - text = formatDurationShort(durationSeconds), + text = formatDuration(durationSeconds), style = MaterialTheme.typography.labelSmall, color = androidx.compose.ui.graphics.Color.White, modifier = Modifier @@ -538,13 +538,6 @@ private fun ThumbnailWithDuration( } } -private fun formatDurationShort(totalSec: Long): String { - val h = totalSec / 3600 - val m = (totalSec % 3600) / 60 - val s = totalSec % 60 - return if (h > 0) "%d:%02d:%02d".format(h, m, s) else "%d:%02d".format(m, s) -} - @Composable private fun SubChip( ch: ChannelRef, diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/feed/SubscriptionFeedViewModel.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/feed/SubscriptionFeedViewModel.kt index fbdff939b..e5a6408c4 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/feed/SubscriptionFeedViewModel.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/feed/SubscriptionFeedViewModel.kt @@ -214,19 +214,34 @@ class SubscriptionFeedViewModel : ViewModel() { private fun mergeFromCache(channels: List): List { val subUrls = channels.map { it.url }.toSet() // Drop cache entries for unsubscribed channels so removed subs - // fall out of the feed immediately. + // fall out of the feed immediately. (Has a side effect on the + // ConcurrentHashMap — kept here for atomicity vs. a separate + // pass.) channelCache.keys.toList().forEach { if (it !in subUrls) channelCache.remove(it) } + // Newest-first across channels. Pre-compute recencyScore once + // per item — vc=35 audit MED-Q15: sortedWith's comparator was + // invoking the regex twice per pair, so ~1800 regex matches on + // a 900-item merge. Pairing the score before sort drops that + // to N matches. return channels.flatMap { ch -> channelCache[ch.url]?.items.orEmpty() } - // Newest-first across channels. Falls back to viewCount when - // we couldn't parse the relative date (older items + live - // streams come back without one). + .map { it to it.recencyScore() } .sortedWith( - compareByDescending { it.recencyScore() } - .thenByDescending { it.viewCount }, + compareByDescending> { it.second } + .thenByDescending { it.first.viewCount }, ) - // Generous cap. Anything past this is almost certainly noise - // for a feed view; pagination in the UI further slices this. .take(500) + .map { it.first } + } + + /** + * Clear in-memory cache. Called from Settings when the user flips + * off the local-cache toggle — disk wipe via FeedCacheStore.clear() + * was already there, but the VM kept its in-memory mirror so items + * stayed visible until process death. vc=35 audit MED-C13. + */ + fun clearInMemoryCache() { + channelCache.clear() + _ui.value = _ui.value.copy(items = emptyList(), lastFetchedAt = 0L) } } diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/MinibarOverlay.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/MinibarOverlay.kt index b9dbdcda6..a66cff9a6 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/MinibarOverlay.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/MinibarOverlay.kt @@ -69,11 +69,25 @@ fun MinibarOverlay( // Reflect the controller's play state in the play/pause icon. Listening // is the only reliable way; isPlaying snapshots stale between events. var isPlaying by remember { mutableStateOf(controller.isPlaying) } + val ctx = androidx.compose.ui.platform.LocalContext.current DisposableEffect(controller) { val listener = object : Player.Listener { override fun onIsPlayingChanged(playing: Boolean) { isPlaying = playing } + // vc=35 audit MED-Q11: if Background-button took the user + // to Home and the foreground audio fails, the only Player + // surface still listening is this minibar. Surface a Toast + // + clear NowPlaying so the minibar hides itself rather + // than claiming an already-dead session is "loaded". + override fun onPlayerError(error: androidx.media3.common.PlaybackException) { + android.widget.Toast.makeText( + ctx, + "playback error: ${error.errorCodeName}", + android.widget.Toast.LENGTH_LONG, + ).show() + NowPlaying.clear() + } } controller.addListener(listener) isPlaying = controller.isPlaying diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/NowPlaying.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/NowPlaying.kt index 75e00f29c..7c9cf6420 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/NowPlaying.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/NowPlaying.kt @@ -36,6 +36,31 @@ object NowPlaying { _current.value = item } + /** + * Atomically claim playback for `streamUrl`. Returns true if this + * call WON the claim (caller should now do setMediaItem + prepare + + * play). Returns false if someone else has already set the same + * streamUrl — typically because the inline-player effect and the + * fullscreen Player effect both fired in the same window during + * an inline→fullscreen transition. The losing caller does nothing; + * the winning caller's playback is already in flight. + * + * Uses MutableStateFlow.compareAndSet for the race-free transition. + * vc=35 audit HIGH-C6 — the previous "check NowPlaying then + * setPlayingFrom" sequence had a window where both checks could + * pass before either NowPlaying.set ran. + */ + fun claim(item: NowPlayingItem): Boolean { + while (true) { + val cur = _current.value + if (cur?.streamUrl == item.streamUrl) return false + if (_current.compareAndSet(cur, item)) return true + // Lost the CAS to a concurrent writer — retry against the + // fresh state. Bounded: at most a handful of competing + // callers in practice. + } + } + fun clear() { _current.value = null } diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/StrawMediaController.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/StrawMediaController.kt index 498dc96a9..5c3013c5a 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/StrawMediaController.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/StrawMediaController.kt @@ -82,11 +82,12 @@ fun MediaController.setPlayingFrom( resolved: ResolvedPlayback, startPositionMs: Long = 0L, ) { - val item = buildMediaItem(title, uploader, thumbnail, resolved) ?: return - setMediaItem(item, startPositionMs) - prepare() - playWhenReady = true - NowPlaying.set( + val mediaItem = buildMediaItem(title, uploader, thumbnail, resolved) ?: return + // Atomic claim BEFORE any controller mutation. If a concurrent + // caller already set this URL (inline player + fullscreen Player + // racing each other on the same transition), we bail before + // double-priming the player. vc=35 audit HIGH-C6. + val claimed = NowPlaying.claim( NowPlayingItem( streamUrl = streamUrl, title = title, @@ -95,6 +96,10 @@ fun MediaController.setPlayingFrom( segments = resolved.segments, ), ) + if (!claimed) return + setMediaItem(mediaItem, startPositionMs) + prepare() + playWhenReady = true } @UnstableApi diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/settings/SettingsScreen.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/settings/SettingsScreen.kt index c5e834837..bc5e3583e 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/settings/SettingsScreen.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/settings/SettingsScreen.kt @@ -41,6 +41,7 @@ import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.unit.dp import android.widget.Toast +import androidx.lifecycle.viewmodel.compose.viewModel import com.sulkta.straw.data.FeedCache import com.sulkta.straw.data.History import com.sulkta.straw.data.MaxResolution @@ -50,6 +51,7 @@ import com.sulkta.straw.data.Settings import com.sulkta.straw.data.ThemeMode import com.sulkta.straw.feature.dataimport.ImportResult import com.sulkta.straw.feature.dataimport.SettingsImport +import com.sulkta.straw.feature.feed.SubscriptionFeedViewModel import com.sulkta.straw.util.LogDump import kotlinx.coroutines.launch @@ -200,6 +202,7 @@ fun SettingsScreen() { style = MaterialTheme.typography.bodyLarge, fontWeight = FontWeight.SemiBold, ) + val feedVm: SubscriptionFeedViewModel = viewModel() Switch( checked = cacheEnabled, onCheckedChange = { checked -> @@ -209,6 +212,10 @@ fun SettingsScreen() { // defeats the purpose of opting out. FeedCache.get().clear() SearchCache.get().clear() + // vc=35 audit MED-C13 — wipe the in-memory + // copy too, otherwise items stayed visible + // until process death. + feedVm.clearInMemoryCache() } }, )