From 21fc81ee7730b7dacd1d6548f13e65db751f7600 Mon Sep 17 00:00:00 2001 From: Kayos Date: Mon, 25 May 2026 17:01:10 +0000 Subject: [PATCH] =?UTF-8?q?vc=3D25:=20audit-fix=20sprint=20=E2=80=94=20CRI?= =?UTF-8?q?T=20+=20HIGH=20+=20MED=20+=20LOW=20cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opus max-effort audit of the vc=23 post-MediaController-unification codebase surfaced two CRITs, both in my own recent code. CRIT-1 + 1b: inline-position-threading band-aid deleted. After the V-2 controller unification, seeking the live controller to its own currentPosition was always 0-500ms backwards — every inline-to- fullscreen and minibar-expand handoff jerked playback backward. The whole `inlinePositionMs` / `onPositionChanged` / `startPositionMs` / `seekTo` chain is gone. The controller is one player; no handoff needed. CRIT-2: PlayerLeaveHandler removed. The registry was fully orphaned — nothing ever assigned to handler. Media3 handles HOME-to-background natively via the foreground service. Dropped the file, the onUserLeaveHint override, and the import. HIGH-1 + 5: PlayerViewModel collapsed into VideoDetailViewModel. Both fetched the same streamInfo for the same URL; PlayerScreen used to spin up two VMs to lift uploader + thumbnail from one and stream URLs from the other. One VM now exposes both `detail` and `resolved`. Drops a redundant network fetch and the double-spinner UX on PlayerScreen. HIGH-2: AndroidView { PlayerView } in PlayerScreen + InlinePlayer now has onRelease { it.player = null } so PlayerView surfaces stop retaining the controller after the composable leaves composition. HIGH-3: SubscriptionFeedViewModel switched to a per-channel cache. Each channel's entries refresh on their own TTL — adding one new subscription no longer invalidates the other 49. Failed/timed-out channel fetches leave the prior cache entry intact instead of blanking the feed for that channel. HIGH-4: onNewIntent override added. singleTask was silently dropping shared-from-Chrome YT URLs whenever Straw was already running. New intents now feed pendingDeepLink which the Compose tree drains into Screen.VideoDetail. MED-3, MED-8, MED-10, LOW batch: PlayerView control-overlay overlap fixed by going through one strategy; SearchViewModel.recordSearch moved into the success branch so errored queries don't pollute recent searches; Downloader's host whitelist tightened to *.googlevideo.com only; SubscriptionsStore.clear + HistoryStore.clearWatches/Searches now use updateAndGet for atomic clear consistent with the other writers; phase/path/audit-ticket markers stripped from comments (kept the technical commentary, dropped sprint tags); 4x duplicated Color(0xCC222222) overlay color extracted to OverlayChromeColor named constant in StrawTheme; HtmlText + StrawActivity NewPipeExtractor references replaced with the current extractor. Net: ~80 LOC deleted overall (the position-threading + handler registry + duplicate VM more than offset the cache + onNewIntent additions). --- buildSrc/src/main/kotlin/ProjectConfig.kt | 4 +- .../src/main/kotlin/com/sulkta/straw/Nav.kt | 14 +- .../kotlin/com/sulkta/straw/StrawActivity.kt | 167 +++++++++--------- .../kotlin/com/sulkta/straw/StrawTheme.kt | 6 + .../com/sulkta/straw/data/HistoryStore.kt | 20 +-- .../sulkta/straw/data/SubscriptionsStore.kt | 15 +- .../straw/feature/detail/VideoDetailScreen.kt | 153 +++++++--------- .../feature/detail/VideoDetailViewModel.kt | 96 ++++++++-- .../straw/feature/download/Downloader.kt | 23 +-- .../feature/feed/SubscriptionFeedViewModel.kt | 131 ++++++++------ .../feature/player/PlayerLeaveHandler.kt | 17 -- .../straw/feature/player/PlayerScreen.kt | 137 ++++++-------- .../straw/feature/player/PlayerViewModel.kt | 103 ----------- .../feature/player/StrawMediaController.kt | 1 + .../straw/feature/search/SearchViewModel.kt | 11 +- .../kotlin/com/sulkta/straw/util/HtmlText.kt | 6 +- 16 files changed, 403 insertions(+), 501 deletions(-) delete mode 100644 strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerLeaveHandler.kt delete mode 100644 strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerViewModel.kt diff --git a/buildSrc/src/main/kotlin/ProjectConfig.kt b/buildSrc/src/main/kotlin/ProjectConfig.kt index c5fbc9d7f..12e988a7a 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 = 24 -const val STRAW_VERSION_NAME = "0.1.0-AJ" +const val STRAW_VERSION_CODE = 25 +const val STRAW_VERSION_NAME = "0.1.0-AK" const val STRAW_APPLICATION_ID = "com.sulkta.straw" diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/Nav.kt b/strawApp/src/main/kotlin/com/sulkta/straw/Nav.kt index bf91616fa..9763aa727 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/Nav.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/Nav.kt @@ -3,7 +3,7 @@ * SPDX-License-Identifier: GPL-3.0-or-later * * Tiny in-app nav model — sealed Screen + a stack. No nav library; pure - * state. Good enough for day-2's home → search → detail → player flow. + * state. */ package com.sulkta.straw @@ -19,11 +19,7 @@ sealed interface Screen { data object Playlists : Screen data object Downloads : Screen data class VideoDetail(val streamUrl: String, val title: String) : Screen - data class Player( - val streamUrl: String, - val title: String, - val startPositionMs: Long = 0L, - ) : Screen + data class Player(val streamUrl: String, val title: String) : Screen data class Channel(val channelUrl: String, val name: String) : Screen data class PlaylistView(val playlistId: String, val name: String) : Screen } @@ -36,7 +32,11 @@ class Navigator(initial: Screen) { stack.add(s) } - /** @return false if we couldn't pop (root), true otherwise. */ + /** + * Pop the current screen off the stack. Returns false at root so the + * caller can defer to the system back behavior (exit the app); true + * otherwise. + */ fun pop(): Boolean { if (stack.size <= 1) return false stack.removeAt(stack.lastIndex) diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/StrawActivity.kt b/strawApp/src/main/kotlin/com/sulkta/straw/StrawActivity.kt index 7d16e4650..dea41ffea 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/StrawActivity.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/StrawActivity.kt @@ -19,6 +19,9 @@ import androidx.compose.material3.Surface import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.getValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.media3.common.util.UnstableApi @@ -27,7 +30,7 @@ import com.sulkta.straw.feature.detail.VideoDetailScreen import com.sulkta.straw.feature.download.DownloadsScreen import com.sulkta.straw.feature.player.LocalStrawController import com.sulkta.straw.feature.player.MinibarOverlay -import com.sulkta.straw.feature.player.PlayerLeaveHandler +import com.sulkta.straw.feature.player.NowPlaying import com.sulkta.straw.feature.player.PlayerScreen import com.sulkta.straw.feature.player.SponsorBlockSkipLoop import com.sulkta.straw.feature.player.rememberStrawController @@ -35,6 +38,7 @@ import com.sulkta.straw.feature.playlist.PlaylistViewScreen import com.sulkta.straw.feature.playlist.PlaylistsScreen import com.sulkta.straw.feature.search.SearchScreen import com.sulkta.straw.feature.settings.SettingsScreen +import kotlinx.coroutines.flow.MutableStateFlow private val YT_HOSTS = setOf( "youtube.com", "www.youtube.com", "m.youtube.com", @@ -46,6 +50,15 @@ private val YT_URL_RE = Regex( ) class StrawActivity : ComponentActivity() { + + /** + * Newly-arrived deep-link URL while the activity is already running. + * `onNewIntent` writes here; the Compose tree observes and pushes a + * VideoDetail screen. Without this the singleTask flag silently drops + * every share-to-Straw after the first. + */ + private val pendingDeepLink = MutableStateFlow(null) + @OptIn(UnstableApi::class) override fun onCreate(savedInstanceState: Bundle?) { enableEdgeToEdge() @@ -55,10 +68,9 @@ class StrawActivity : ComponentActivity() { setContent { val scheme = if (isSystemInDarkTheme()) strawDarkColors() else strawLightColors() - // Build one MediaController for the whole activity. Every screen - // pulls it via LocalStrawController, every PlayerView binds to - // it, and the minibar overlay (rendered below) uses it too. - // Single player, single source of truth. + // One MediaController for the whole activity. Every screen pulls + // it via LocalStrawController; the minibar overlay below uses it + // too. Single player, single source of truth. val controller = rememberStrawController() MaterialTheme(colorScheme = scheme) { CompositionLocalProvider(LocalStrawController provides controller) { @@ -80,6 +92,15 @@ class StrawActivity : ComponentActivity() { onDispose { cb.remove() } } + // Drain newly-arrived deep links. Consumed (cleared) once + // pushed so we don't re-navigate on every recomposition. + val pending by pendingDeepLink.collectAsState() + LaunchedEffect(pending) { + val url = pending ?: return@LaunchedEffect + nav.push(Screen.VideoDetail(url, "")) + pendingDeepLink.value = null + } + // SponsorBlock skip loop runs at the activity level so it // applies whether the user is fullscreen, in the minibar, // or away from the player surface. @@ -87,21 +108,13 @@ class StrawActivity : ComponentActivity() { Box(modifier = Modifier.fillMaxSize()) { ScreenContent(nav, s = nav.current) - // Persistent minibar overlay — visible on every screen - // except Player itself (fullscreen has its own UI). + // Persistent minibar — visible on every non-Player + // screen whenever something is loaded. if (nav.current !is Screen.Player) { MinibarOverlay( onExpand = { - val item = com.sulkta.straw.feature.player.NowPlaying.current.value - if (item != null) { - nav.push( - Screen.Player( - item.streamUrl, - item.title, - controller?.currentPosition ?: 0L, - ) - ) - } + val item = NowPlaying.current.value ?: return@MinibarOverlay + nav.push(Screen.Player(item.streamUrl, item.title)) }, modifier = Modifier.align(Alignment.BottomCenter), ) @@ -113,97 +126,79 @@ class StrawActivity : ComponentActivity() { } } + /** + * `launchMode="singleTask"` means a fresh VIEW/SEND from Chrome lands + * on the already-running activity instead of creating a new instance. + * Forward the URL into the Compose tree via the pending-link flow. + */ + override fun onNewIntent(intent: Intent) { + super.onNewIntent(intent) + setIntent(intent) + pickYouTubeUrl(intent)?.let { pendingDeepLink.value = it } + } + @Composable private fun ScreenContent(nav: Navigator, s: Screen) { when (s) { - is Screen.Home -> StrawHome( - onOpenSearch = { nav.push(Screen.Search) }, - onOpenSettings = { nav.push(Screen.Settings) }, - onOpenPlaylists = { nav.push(Screen.Playlists) }, - onOpenDownloads = { nav.push(Screen.Downloads) }, - onOpenVideo = { url, title -> - nav.push(Screen.VideoDetail(url, title)) - }, - onOpenChannel = { url, name -> - nav.push(Screen.Channel(url, name)) - }, - ) - is Screen.Downloads -> DownloadsScreen() - is Screen.Settings -> SettingsScreen() - is Screen.Search -> SearchScreen( - onOpenVideo = { url, title -> - nav.push(Screen.VideoDetail(url, title)) - }, - ) - is Screen.VideoDetail -> VideoDetailScreen( - streamUrl = s.streamUrl, - initialTitle = s.title, - onPlay = { startPositionMs -> - nav.push(Screen.Player(s.streamUrl, s.title, startPositionMs)) - }, - onOpenChannel = { url, name -> - nav.push(Screen.Channel(url, name)) - }, - onOpenVideo = { url, title -> - nav.push(Screen.VideoDetail(url, title)) - }, - ) - is Screen.Channel -> ChannelScreen( - channelUrl = s.channelUrl, - initialName = s.name, - onOpenVideo = { url, title -> - nav.push(Screen.VideoDetail(url, title)) - }, - ) - is Screen.Player -> PlayerScreen( - streamUrl = s.streamUrl, - title = s.title, - startPositionMs = s.startPositionMs, - onMinimize = { nav.pop() }, - ) - is Screen.Playlists -> PlaylistsScreen( - onOpenPlaylist = { id, name -> - nav.push(Screen.PlaylistView(id, name)) - }, - ) + is Screen.Home -> StrawHome( + onOpenSearch = { nav.push(Screen.Search) }, + onOpenSettings = { nav.push(Screen.Settings) }, + onOpenPlaylists = { nav.push(Screen.Playlists) }, + onOpenDownloads = { nav.push(Screen.Downloads) }, + onOpenVideo = { url, title -> nav.push(Screen.VideoDetail(url, title)) }, + onOpenChannel = { url, name -> nav.push(Screen.Channel(url, name)) }, + ) + is Screen.Downloads -> DownloadsScreen() + is Screen.Settings -> SettingsScreen() + is Screen.Search -> SearchScreen( + onOpenVideo = { url, title -> nav.push(Screen.VideoDetail(url, title)) }, + ) + is Screen.VideoDetail -> VideoDetailScreen( + streamUrl = s.streamUrl, + initialTitle = s.title, + onPlay = { nav.push(Screen.Player(s.streamUrl, s.title)) }, + onOpenChannel = { url, name -> nav.push(Screen.Channel(url, name)) }, + onOpenVideo = { url, title -> nav.push(Screen.VideoDetail(url, title)) }, + ) + is Screen.Channel -> ChannelScreen( + channelUrl = s.channelUrl, + initialName = s.name, + onOpenVideo = { url, title -> nav.push(Screen.VideoDetail(url, title)) }, + ) + is Screen.Player -> PlayerScreen( + streamUrl = s.streamUrl, + title = s.title, + onMinimize = { nav.pop() }, + ) + is Screen.Playlists -> PlaylistsScreen( + onOpenPlaylist = { id, name -> nav.push(Screen.PlaylistView(id, name)) }, + ) is Screen.PlaylistView -> PlaylistViewScreen( playlistId = s.playlistId, initialName = s.name, - onOpenVideo = { url, title -> - nav.push(Screen.VideoDetail(url, title)) - }, + onOpenVideo = { url, title -> nav.push(Screen.VideoDetail(url, title)) }, ) } } - /** - * HOME / recents → seamless hand-off to background audio when the - * player screen is active. PlayerScreen registers the handler; any - * other screen leaves it null and home behaves normally. - */ - override fun onUserLeaveHint() { - super.onUserLeaveHint() - PlayerLeaveHandler.handler?.invoke() - } - - /** Pull a YouTube URL out of an incoming Intent (VIEW or SEND). */ + /** Pull a YouTube URL out of an incoming VIEW or SEND intent. */ private fun pickYouTubeUrl(intent: Intent?): String? { intent ?: return null return when (intent.action) { Intent.ACTION_VIEW -> { val data = intent.data?.toString() ?: return null // Explicit scheme + host check — defense in depth vs the - // manifest intent-filter (apps can synth intents that - // bypass filter scheme matching when activity is exported). + // manifest intent-filter; apps can synth intents that + // bypass filter scheme matching on exported activities. if (intent.scheme?.lowercase() !in setOf("https", "http")) return null if (!looksLikeYouTube(data)) return null data } Intent.ACTION_SEND -> { val shared = intent.getStringExtra(Intent.EXTRA_TEXT) ?: return null - // Regex extracts a YT-looking substring from arbitrary - // attacker-controlled text. Re-validate via URI parse + host - // check before we hand it to NewPipeExtractor. + // Extract a YT-looking substring from attacker-controlled + // text, then re-validate via URI parse + host check before + // handing it to the extractor. val candidate = YT_URL_RE.find(shared)?.value ?: return null val truncated = candidate.substringBefore('#').trim() if (!looksLikeYouTube(truncated)) return null diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/StrawTheme.kt b/strawApp/src/main/kotlin/com/sulkta/straw/StrawTheme.kt index 74ad7be59..c3ee116ec 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/StrawTheme.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/StrawTheme.kt @@ -62,3 +62,9 @@ fun strawDarkColors(): ColorScheme = darkColorScheme( tertiary = DarkGreenTertiary, onTertiary = DarkGreenOnTertiary, ) + +// Semi-transparent overlays for chrome (overlay buttons, the SB badge, +// the inline-player fullscreen pill) and for the dimmed area behind the +// minibar thumbnail. Kept here so a theme tweak touches one place. +val OverlayChromeColor = androidx.compose.ui.graphics.Color(0xCC222222) +val OverlayDimColor = androidx.compose.ui.graphics.Color(0xCC000000) diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/data/HistoryStore.kt b/strawApp/src/main/kotlin/com/sulkta/straw/data/HistoryStore.kt index 8ed488e48..cc04a788c 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/data/HistoryStore.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/data/HistoryStore.kt @@ -2,9 +2,9 @@ * SPDX-FileCopyrightText: 2026 Sulkta-Coop * SPDX-License-Identifier: GPL-3.0-or-later * - * SharedPreferences-backed recent watches + recent search store. Day-3. - * Day-4 graduates to Room when there's a real query pattern (date ranges, - * full-text search, etc.) that SharedPreferences can't serve. + * Recent watches + recent searches backed by SharedPreferences JSON + * blobs. Capped to MAX_WATCHES / MAX_SEARCHES. Graduates to Room when + * a real query pattern (date ranges, full-text search) shows up. */ package com.sulkta.straw.data @@ -46,9 +46,9 @@ class HistoryStore(context: Context) { fun recordWatch(item: WatchHistoryItem) { val now = item.copy(watchedAt = System.currentTimeMillis()) - // Atomic read-modify-write via StateFlow.updateAndGet — fixes - // AUD-HIGH race where two concurrent recordWatch calls would - // each read the old list and one would clobber the other. + // Atomic read-modify-write — two concurrent recordWatch calls + // both reading the same `current` and one clobbering the other + // is exactly the bug updateAndGet avoids. val next = _watches.updateAndGet { current -> val without = current.filterNot { it.videoId == item.videoId } (listOf(now) + without).take(MAX_WATCHES) @@ -67,13 +67,13 @@ class HistoryStore(context: Context) { } fun clearWatches() { - _watches.value = emptyList() - sp.edit().remove(KEY_WATCHES).apply() + _watches.updateAndGet { emptyList() } + sp.edit().putString(KEY_WATCHES, json.encodeToString(emptyList())).apply() } fun clearSearches() { - _searches.value = emptyList() - sp.edit().remove(KEY_SEARCHES).apply() + _searches.updateAndGet { emptyList() } + sp.edit().putString(KEY_SEARCHES, json.encodeToString(emptyList())).apply() } private fun loadWatches(): List = runCatching { diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/data/SubscriptionsStore.kt b/strawApp/src/main/kotlin/com/sulkta/straw/data/SubscriptionsStore.kt index 3a9fc8162..b90d9b406 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/data/SubscriptionsStore.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/data/SubscriptionsStore.kt @@ -2,8 +2,8 @@ * SPDX-FileCopyrightText: 2026 Sulkta-Coop * SPDX-License-Identifier: GPL-3.0-or-later * - * SharedPreferences-lite subscription list. Day-4 graduates to Room when - * we want background feed fetching for new uploads. + * Subscription list backed by a single JSON blob in SharedPreferences. + * Graduates to Room when background feed fetching arrives. */ package com.sulkta.straw.data @@ -38,7 +38,9 @@ class SubscriptionsStore(context: Context) { _subs.value.any { it.url == channelUrl } fun toggle(ref: ChannelRef) { - // Atomic toggle via updateAndGet — see AUD-HIGH note in HistoryStore. + // updateAndGet makes the read-modify-write atomic vs. concurrent + // toggles (e.g. one channel subscribed from the feed while another + // is unsubscribed from VideoDetail). val next = _subs.updateAndGet { cur -> if (cur.any { it.url == ref.url }) { cur.filterNot { it.url == ref.url } @@ -50,8 +52,11 @@ class SubscriptionsStore(context: Context) { } fun clear() { - _subs.value = emptyList() - sp.edit().remove(KEY).apply() + // Same atomic-update path as toggle — protects against a concurrent + // toggle racing the clear and persisting [new-item] after the + // remove() call has fired. + _subs.updateAndGet { emptyList() } + persist(emptyList()) } private fun persist(list: List) { diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailScreen.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailScreen.kt index cc33676fe..499f232b1 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailScreen.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailScreen.kt @@ -5,6 +5,9 @@ package com.sulkta.straw.feature.detail +import android.content.Intent +import android.widget.Toast +import androidx.annotation.OptIn import androidx.compose.foundation.background import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Arrangement @@ -23,8 +26,12 @@ import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.statusBarsPadding import androidx.compose.foundation.layout.width import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.shape.CircleShape import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.foundation.verticalScroll +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.filled.PlayArrow +import androidx.compose.material3.AlertDialog import androidx.compose.material3.AssistChip import androidx.compose.material3.AssistChipDefaults import androidx.compose.material3.Button @@ -35,44 +42,40 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.OutlinedButton import androidx.compose.material3.OutlinedTextField import androidx.compose.material3.Text -import android.content.Intent -import android.widget.Toast -import androidx.annotation.OptIn -import androidx.compose.material3.AlertDialog +import androidx.compose.material3.TextButton +import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState -import androidx.compose.runtime.mutableLongStateOf +import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue -import com.sulkta.straw.data.PlaylistItem -import com.sulkta.straw.data.Playlists -import kotlinx.coroutines.delay -import androidx.compose.ui.platform.LocalContext -import androidx.compose.ui.viewinterop.AndroidView -import com.sulkta.straw.feature.download.DownloadKind -import com.sulkta.straw.feature.download.Downloader -import com.sulkta.straw.feature.player.PlayerViewModel -import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.filled.PlayArrow -import androidx.compose.runtime.Composable -import androidx.compose.runtime.LaunchedEffect -import androidx.compose.runtime.getValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip import androidx.compose.ui.graphics.Color +import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.text.font.FontWeight +import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp +import androidx.compose.ui.viewinterop.AndroidView import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.lifecycle.viewmodel.compose.viewModel import androidx.media3.common.Player import androidx.media3.common.util.UnstableApi import androidx.media3.ui.PlayerView import coil3.compose.AsyncImage +import com.sulkta.straw.OverlayChromeColor +import com.sulkta.straw.OverlayDimColor +import com.sulkta.straw.data.PlaylistItem +import com.sulkta.straw.data.Playlists +import com.sulkta.straw.feature.download.DownloadKind +import com.sulkta.straw.feature.download.Downloader import com.sulkta.straw.feature.player.LocalStrawController import com.sulkta.straw.feature.player.NowPlaying import com.sulkta.straw.feature.player.setPlayingFrom +import com.sulkta.straw.feature.search.StreamItem import com.sulkta.straw.util.formatCount import com.sulkta.straw.util.formatViews import com.sulkta.straw.util.stripHtml @@ -82,7 +85,7 @@ import com.sulkta.straw.util.stripHtml fun VideoDetailScreen( streamUrl: String, initialTitle: String, - onPlay: (startPositionMs: Long) -> Unit, + onPlay: () -> Unit, onOpenChannel: (channelUrl: String, name: String) -> Unit, onOpenVideo: (url: String, title: String) -> Unit, vm: VideoDetailViewModel = viewModel(), @@ -91,13 +94,8 @@ fun VideoDetailScreen( val context = LocalContext.current var showDownloadDialog by remember { mutableStateOf(false) } var showSaveToPlaylistDialog by remember { mutableStateOf(false) } - // Inline-play state. Resets when the user navigates to a different - // video (keyed on streamUrl). + // Inline-play state resets when navigating to a different video. var inlinePlaying by remember(streamUrl) { mutableStateOf(false) } - // V-2: inline player's current position, polled into here so the - // outer can pass it through when the user taps Play / ⛶. Resets to 0 - // when the inline player isn't active. - var inlinePositionMs by remember(streamUrl) { mutableLongStateOf(0L) } LaunchedEffect(streamUrl) { vm.load(streamUrl) } Column( @@ -120,17 +118,13 @@ fun VideoDetailScreen( else -> { val d = state.detail ?: return@Column - // Tap the thumbnail to play inline. Fullscreen button (top-right - // overlay on the inline player) jumps to the fullscreen Player - // screen which has the full toolset. if (inlinePlaying) { InlinePlayer( streamUrl = streamUrl, title = d.title, uploader = d.uploader, thumbnail = d.thumbnail, - onFullscreen = { onPlay(inlinePositionMs) }, - onPositionChanged = { inlinePositionMs = it }, + onFullscreen = onPlay, modifier = Modifier .fillMaxWidth() .aspectRatio(16f / 9f) @@ -154,8 +148,8 @@ fun VideoDetailScreen( Box( modifier = Modifier .size(64.dp) - .clip(androidx.compose.foundation.shape.CircleShape) - .background(Color(0xCC000000)), + .clip(CircleShape) + .background(OverlayDimColor), contentAlignment = Alignment.Center, ) { Icon( @@ -169,25 +163,24 @@ fun VideoDetailScreen( } Spacer(modifier = Modifier.height(12.dp)) - // ── Title + uploader ───────────────────────────────────── Text( text = d.title, style = MaterialTheme.typography.titleLarge, fontWeight = FontWeight.SemiBold, ) Spacer(modifier = Modifier.height(4.dp)) - val uploaderClickable = d.uploaderUrl != null + val uploaderUrl = d.uploaderUrl Text( text = d.uploader, style = MaterialTheme.typography.bodyMedium, - color = if (uploaderClickable) MaterialTheme.colorScheme.primary else MaterialTheme.colorScheme.onSurfaceVariant, - modifier = if (uploaderClickable) Modifier.clickable { - onOpenChannel(d.uploaderUrl!!, d.uploader) + color = if (uploaderUrl != null) MaterialTheme.colorScheme.primary + else MaterialTheme.colorScheme.onSurfaceVariant, + modifier = if (uploaderUrl != null) Modifier.clickable { + onOpenChannel(uploaderUrl, d.uploader) } else Modifier, ) Spacer(modifier = Modifier.height(12.dp)) - // ── Engagement row: views + RYD likes/dislikes ─────────── Row( horizontalArrangement = Arrangement.spacedBy(8.dp), verticalAlignment = Alignment.CenterVertically, @@ -225,7 +218,7 @@ fun VideoDetailScreen( horizontalArrangement = Arrangement.spacedBy(12.dp), verticalArrangement = Arrangement.spacedBy(8.dp), ) { - Button(onClick = { onPlay(inlinePositionMs) }) { Text("Play") } + Button(onClick = onPlay) { Text("Play") } OutlinedButton(onClick = { val send = Intent(Intent.ACTION_SEND).apply { type = "text/plain" @@ -243,17 +236,15 @@ fun VideoDetailScreen( } Spacer(modifier = Modifier.height(20.dp)) - // ── Description ────────────────────────────────────────── Text("Description", style = MaterialTheme.typography.titleSmall, fontWeight = FontWeight.SemiBold) Spacer(modifier = Modifier.height(8.dp)) - // AUD-MED: cap input length before regex passes — defends - // against ANR on multi-MB descriptions. + // Cap input length before regex passes — defends against + // ANR on multi-MB descriptions. Text( text = stripHtml(d.description.take(20_000)).take(2000), style = MaterialTheme.typography.bodySmall, ) - // ── Recommended ────────────────────────────────────────── if (d.related.isNotEmpty()) { Spacer(modifier = Modifier.height(24.dp)) Text( @@ -264,11 +255,10 @@ fun VideoDetailScreen( Spacer(modifier = Modifier.height(8.dp)) d.related.take(20).forEach { rel -> RelatedRow(rel) { onOpenVideo(rel.url, rel.title) } - androidx.compose.material3.HorizontalDivider() + HorizontalDivider() } } - // ── More from ───────────────────────────────── if (d.moreFromChannel.isNotEmpty()) { Spacer(modifier = Modifier.height(24.dp)) Text( @@ -280,7 +270,7 @@ fun VideoDetailScreen( Spacer(modifier = Modifier.height(8.dp)) d.moreFromChannel.take(20).forEach { item -> RelatedRow(item) { onOpenVideo(item.url, item.title) } - androidx.compose.material3.HorizontalDivider() + HorizontalDivider() } } @@ -315,7 +305,6 @@ fun VideoDetailScreen( confirmButton = { Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) { Button(onClick = { - // info is now uniffi.strawcore.StreamInfo (Path C-4). val audio = info?.audioOnly?.maxByOrNull { it.bitrate }?.url if (audio != null) { val id = Downloader.enqueue(context, audio, d.title, DownloadKind.Audio) @@ -341,13 +330,12 @@ fun VideoDetailScreen( } }, dismissButton = { - androidx.compose.material3.TextButton(onClick = { showDownloadDialog = false }) { + TextButton(onClick = { showDownloadDialog = false }) { Text("Cancel") } }, ) } - } } } @@ -355,7 +343,7 @@ fun VideoDetailScreen( @Composable private fun RelatedRow( - item: com.sulkta.straw.feature.search.StreamItem, + item: StreamItem, onClick: () -> Unit, ) { Row( @@ -380,7 +368,7 @@ private fun RelatedRow( style = MaterialTheme.typography.bodyMedium, fontWeight = FontWeight.SemiBold, maxLines = 2, - overflow = androidx.compose.ui.text.style.TextOverflow.Ellipsis, + overflow = TextOverflow.Ellipsis, ) Spacer(modifier = Modifier.height(2.dp)) Text( @@ -394,7 +382,7 @@ private fun RelatedRow( style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.onSurfaceVariant, maxLines = 1, - overflow = androidx.compose.ui.text.style.TextOverflow.Ellipsis, + overflow = TextOverflow.Ellipsis, ) } } @@ -479,18 +467,18 @@ private fun SaveToPlaylistDialog( } }, confirmButton = { - androidx.compose.material3.TextButton(onClick = onDismiss) { Text("Close") } + TextButton(onClick = onDismiss) { Text("Close") } }, ) } /** - * Inline player embedded in the 16:9 thumbnail box on VideoDetailScreen. - * Uses its own ExoPlayer + PlayerView (with the built-in controller for - * play/pause/seek). A small fullscreen pill in the top-right hops the user - * to the fullscreen PlayerScreen for the full toolset (speed picker, audio- - * only, share, PiP, background). Player is released when the composable - * leaves composition (navigate back or away from VideoDetail). + * Inline player surface inside VideoDetail's 16:9 thumbnail box. Renders + * a PlayerView bound to the shared LocalStrawController — the same + * player as the fullscreen PlayerScreen and the minibar overlay. The ⛶ + * pill hops to fullscreen; playback continues unchanged. There is + * nothing to release here: the controller is process-wide, and the + * PlayerView's surface is detached on dispose via onRelease. */ @OptIn(UnstableApi::class) @Composable @@ -500,42 +488,27 @@ private fun InlinePlayer( uploader: String, thumbnail: String?, onFullscreen: () -> Unit, - onPositionChanged: (Long) -> Unit, modifier: Modifier = Modifier, ) { val controller = LocalStrawController.current - val playerVm: PlayerViewModel = viewModel() - val state by playerVm.ui.collectAsStateWithLifecycle() - LaunchedEffect(streamUrl) { playerVm.resolve(streamUrl) } + val vm: VideoDetailViewModel = viewModel() + val state by vm.ui.collectAsStateWithLifecycle() - // As soon as we have a resolved stream AND the active video isn't - // already this URL, push it into the shared controller. The controller - // is the same one driving the fullscreen Player + the minibar overlay, - // so playback survives any nav transition unchanged. + // 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). val resolved = state.resolved LaunchedEffect(controller, resolved, streamUrl) { val c = controller ?: return@LaunchedEffect val r = resolved ?: return@LaunchedEffect - val activeUrl = NowPlaying.current.value?.streamUrl - if (activeUrl != streamUrl) { - c.setPlayingFrom( - streamUrl = streamUrl, - title = title, - uploader = uploader, - thumbnail = thumbnail, - resolved = r, - ) - } - } - - // Report position to the parent on every tick so a Play / ⛶ tap picks - // up at the right spot if the active video is somehow desynced. - LaunchedEffect(controller) { - val c = controller ?: return@LaunchedEffect - while (true) { - onPositionChanged(c.currentPosition.coerceAtLeast(0L)) - delay(500) - } + if (NowPlaying.current.value?.streamUrl == streamUrl) return@LaunchedEffect + c.setPlayingFrom( + streamUrl = streamUrl, + title = title, + uploader = uploader, + thumbnail = thumbnail, + resolved = r, + ) } var playbackError by remember { mutableStateOf(null) } @@ -577,17 +550,16 @@ private fun InlinePlayer( } }, update = { it.player = controller }, + onRelease = { it.player = null }, modifier = Modifier.fillMaxSize(), ) - // Top-right fullscreen pill — hops to the fullscreen - // PlayerScreen which has speed/audio-only/share/PiP/background. Box( modifier = Modifier .align(Alignment.TopEnd) .padding(8.dp) .size(36.dp) .clip(RoundedCornerShape(6.dp)) - .background(Color(0xCC222222)) + .background(OverlayChromeColor) .clickable(onClick = onFullscreen), contentAlignment = Alignment.Center, ) { @@ -597,4 +569,3 @@ private fun InlinePlayer( } } } - diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailViewModel.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailViewModel.kt index 63dea79c3..476615c25 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailViewModel.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/detail/VideoDetailViewModel.kt @@ -2,11 +2,16 @@ * SPDX-FileCopyrightText: 2026 Sulkta-Coop * SPDX-License-Identifier: GPL-3.0-or-later * - * Phase U-3 / Path C-4: streamInfo() moves from NewPipeExtractor (Java) to - * strawcore (Rust + rustypipe via UniFFI). Channel fetch for - * `moreFromChannel` stays on NPE until C-5. + * One VM per video URL — drives VideoDetail, the fullscreen Player, and + * the inline player on detail (all live in the same activity-scoped VM + * store, so `viewModel()` from each composable returns this instance). + * + * `load(url)` fetches strawcore.streamInfo once, derives both `detail` + * (title, uploader, view count, RYD, related, more-from-channel) and + * `resolved` (the picked stream URLs the player needs), and records the + * video to watch history. Subsequent `load(url)` calls for the same URL + * are a no-op so the spinner only fires on a real navigation change. */ - package com.sulkta.straw.feature.detail import androidx.lifecycle.ViewModel @@ -16,7 +21,9 @@ import com.sulkta.straw.data.Settings import com.sulkta.straw.data.WatchHistoryItem import com.sulkta.straw.net.RydClient import com.sulkta.straw.net.RydVotes +import com.sulkta.straw.net.SbSegment import com.sulkta.straw.net.SponsorBlockClient +import com.sulkta.straw.feature.search.StreamItem import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -34,17 +41,41 @@ data class VideoDetail( val thumbnail: String?, val ryd: RydVotes? = null, val sbSegmentCount: Int = 0, - val related: List = emptyList(), - /** Other videos from the same channel — separate from related (which is YT's - * algo). Anchored to the uploader the user chose; matches the sub-feed ethos. */ - val moreFromChannel: List = emptyList(), + val related: List = emptyList(), + /** + * Other videos from the same channel — separate from `related` + * (which is YT's algo). Anchored to the uploader the user chose; + * matches the sub-feed ethos. + */ + val moreFromChannel: List = emptyList(), ) +/** + * Stream URLs picked from `streamInfo` for the player. The picker prefers + * DASH (whole-quality + adaptive) → HLS → combined progressive → merged + * video+audio progressive → video-only. Carries SB segments for the + * activity-level skip loop. + */ +data class ResolvedPlayback( + val title: String, + val videoUrl: String?, + val audioUrl: String?, + val combinedUrl: String?, + val dashMpdUrl: String?, + val hlsUrl: String?, + val segments: List = emptyList(), +) { + val isPlayable: Boolean + get() = !combinedUrl.isNullOrBlank() || !videoUrl.isNullOrBlank() || + !dashMpdUrl.isNullOrBlank() || !hlsUrl.isNullOrBlank() +} + data class VideoDetailUiState( val loading: Boolean = true, val detail: VideoDetail? = null, + val resolved: ResolvedPlayback? = null, val error: String? = null, - // Stored on success for handoff to the player + Download dialog. Not in UI. + /** Raw extractor result — kept around for the Download dialog. */ val streamInfo: uniffi.strawcore.StreamInfo? = null, ) @@ -55,8 +86,9 @@ class VideoDetailViewModel : ViewModel() { private var loadedUrl: String? = null fun load(streamUrl: String) { - // viewModel() is Activity-scoped, so the same VM is reused across - // navigations. Compare the requested URL with what we last loaded. + // viewModel() is activity-scoped, so the same VM is reused across + // navigations. Skip the refetch if the requested URL already has + // a resolved state. if (loadedUrl == streamUrl && _ui.value.detail != null) return loadedUrl = streamUrl _ui.value = VideoDetailUiState(loading = true) @@ -86,14 +118,13 @@ class VideoDetailViewModel : ViewModel() { runCatching { RydClient.fetch(videoId) }.getOrNull() } val sbCats = Settings.get().sbCategories.value.map { it.key } - val sbCount = if (sbCats.isEmpty()) 0 else withContext(Dispatchers.IO) { - runCatching { SponsorBlockClient.fetch(videoId, sbCats).size }.getOrDefault(0) + val segments = if (sbCats.isEmpty()) emptyList() else withContext(Dispatchers.IO) { + runCatching { SponsorBlockClient.fetch(videoId, sbCats) } + .getOrDefault(emptyList()) } - // strawcore returns `related` as List. Map to the - // Kotlin StreamItem shape used elsewhere. val related = info.related.map { r -> - com.sulkta.straw.feature.search.StreamItem( + StreamItem( url = r.url, title = r.title.ifBlank { "(no title)" }, uploader = r.uploader, @@ -107,7 +138,7 @@ class VideoDetailViewModel : ViewModel() { // More from this channel via strawcore.channelInfo — one // Rust round-trip returns the channel's Videos tab pre-mapped. val uploaderUrl = info.uploaderUrl - val moreFromChannel: List = + val moreFromChannel: List = if (uploaderUrl.isNullOrBlank()) emptyList() else runCatching { val ch = uniffi.strawcore.channelInfo(uploaderUrl) @@ -115,7 +146,7 @@ class VideoDetailViewModel : ViewModel() { .filter { it.url != streamUrl } .take(20) .map { v -> - com.sulkta.straw.feature.search.StreamItem( + StreamItem( url = v.url, title = v.title.ifBlank { "(no title)" }, uploader = v.uploader.ifBlank { uploader }, @@ -127,6 +158,8 @@ class VideoDetailViewModel : ViewModel() { } }.getOrDefault(emptyList()) + val resolved = resolvePlayback(info, segments) + _ui.value = VideoDetailUiState( loading = false, detail = VideoDetail( @@ -138,10 +171,11 @@ class VideoDetailViewModel : ViewModel() { description = info.description, thumbnail = thumb, ryd = ryd, - sbSegmentCount = sbCount, + sbSegmentCount = segments.size, related = related, moreFromChannel = moreFromChannel, ), + resolved = resolved, streamInfo = info, ) } catch (t: Throwable) { @@ -152,4 +186,28 @@ class VideoDetailViewModel : ViewModel() { } } } + + private fun resolvePlayback( + info: uniffi.strawcore.StreamInfo, + segments: List, + ): ResolvedPlayback { + val maxRes = Settings.get().maxResolution.value.ceiling + // Filter by max-resolution ceiling but fall back to the lowest + // available if the ceiling excludes everything (e.g. a 360p-only + // upload with the user on a 480p cap). + fun pickVideo(streams: List): String? { + if (streams.isEmpty()) return null + val pool = streams.filter { it.height <= maxRes }.ifEmpty { streams } + return pool.maxByOrNull { it.bitrate }?.url + } + return ResolvedPlayback( + title = info.title, + videoUrl = pickVideo(info.videoOnly), + audioUrl = info.audioOnly.maxByOrNull { it.bitrate }?.url, + combinedUrl = pickVideo(info.combined), + dashMpdUrl = info.dashMpdUrl?.takeIf { it.isNotBlank() }, + hlsUrl = info.hlsUrl?.takeIf { it.isNotBlank() }, + segments = segments, + ) + } } diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/download/Downloader.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/download/Downloader.kt index 4bb361506..bdb7dcce3 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/download/Downloader.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/download/Downloader.kt @@ -2,18 +2,18 @@ * SPDX-FileCopyrightText: 2026 Sulkta-Coop * SPDX-License-Identifier: GPL-3.0-or-later * - * Phase R: minimal download via Android's DownloadManager. Saves to the + * Minimal download via Android's DownloadManager. Saves to the * app-private external files dir so we don't need WRITE_EXTERNAL_STORAGE * on older Android. The user can pull files out via a file manager * (under Android/data/com.sulkta.straw.debug/files/...). * - * Audit fixes (2026-05-24 pass #2): - * HIGH-4: scheme + host validation on the URL before handing it to - * DownloadManager — extractor output is not trusted root-of-truth. - * HIGH-5: harder filename sanitization — control chars, bidi overrides, - * leading dots, trailing whitespace. - * MED-6: catch IllegalArgumentException from enqueue so a malformed URI - * doesn't crash the click handler. + * Hardening: + * - scheme + host validation on the URL before enqueueing (extractor + * output is not trusted root-of-truth) + * - filename sanitization for control chars, bidi overrides, leading + * dots, and trailing whitespace + * - catches IllegalArgumentException from enqueue so a malformed URI + * doesn't crash the click handler */ package com.sulkta.straw.feature.download @@ -88,8 +88,9 @@ object Downloader { val uri = runCatching { Uri.parse(url) }.getOrNull() ?: return false if (!uri.scheme.equals("https", ignoreCase = true)) return false val host = uri.host?.lowercase() ?: return false - return host.endsWith(".googlevideo.com") || - host.endsWith(".youtube.com") || - host == "youtube.com" + // strawcore returns video/audio stream URLs from googlevideo CDN + // exclusively — youtube.com URLs aren't direct streams and have + // no business going to DownloadManager. + return host.endsWith(".googlevideo.com") } } 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 d7f27b11c..f69be037f 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 @@ -2,26 +2,26 @@ * SPDX-FileCopyrightText: 2026 Sulkta-Coop * SPDX-License-Identifier: GPL-3.0-or-later * - * Phase Q: aggregate latest videos across all subscribed channels into a - * single feed. Fans out per-channel channelInfo() fetches in parallel, - * merges by view count desc, caps at 200 items. + * Aggregate latest videos across all subscribed channels into a single + * feed. Fans out per-channel channelInfo() fetches in parallel, caches + * each channel's videos independently, merges by view-count desc, caps + * at 200 items. * - * Path C-5: each per-channel fetch is now ONE strawcore.channelInfo() - * call instead of two NewPipeExtractor round-trips (ChannelInfo.getInfo + - * ChannelTabInfo.getInfo). Halves the network work for the feed. + * Each per-channel cache entry has its own TTL so adding one new + * subscription doesn't invalidate the other 49 — only the new one + * actually goes to the network on the next refresh. * - * Audit fixes (2026-05-24 pass #2): - * HIGH-6: cancel any prior in-flight refresh when a new one starts, cap - * concurrency with a Semaphore, time-bound each per-channel fetch so - * one hung channel can't stall the whole feed. - * MED-7: use `update { }` for atomic UI-state writes (matches the - * convention applied to the data stores in audit pass #1). + * Concurrency hardening: cancel any in-flight refresh when a new one + * starts, cap parallelism with a Semaphore so 100+ subs don't slam YT, + * time-bound each per-channel fetch so one hung channel can't stall the + * whole batch. */ package com.sulkta.straw.feature.feed import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import com.sulkta.straw.data.ChannelRef import com.sulkta.straw.data.Subscriptions import com.sulkta.straw.feature.search.StreamItem import com.sulkta.straw.util.strawLogW @@ -37,6 +37,7 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Semaphore import kotlinx.coroutines.sync.withPermit import kotlinx.coroutines.withTimeoutOrNull +import java.util.concurrent.ConcurrentHashMap data class SubscriptionFeedUiState( val loading: Boolean = false, @@ -49,10 +50,14 @@ class SubscriptionFeedViewModel : ViewModel() { private val _ui = MutableStateFlow(SubscriptionFeedUiState()) val ui: StateFlow = _ui.asStateFlow() - /** Cache feed for 10 min to avoid hammering YT on tab re-entry. */ - private val cacheTtlMs = 10L * 60 * 1000 + /** Per-channel cache: each entry refreshes independently. */ + private data class ChannelCacheEntry(val fetchedAt: Long, val items: List) + private val channelCache = ConcurrentHashMap() - /** Per-channel fetch timeout — slowest channel can't stall the whole batch. */ + /** Per-channel TTL — Refresh just re-fetches stale entries. */ + private val perChannelTtlMs = 10L * 60 * 1000 + + /** Per-channel fetch timeout — slowest channel can't stall the batch. */ private val perChannelTimeoutMs = 15_000L /** Cap parallel network fetches even with 100+ subs. */ @@ -63,64 +68,39 @@ class SubscriptionFeedViewModel : ViewModel() { fun refreshIfStale() { val now = System.currentTimeMillis() - if (_ui.value.items.isNotEmpty() && now - _ui.value.lastFetchedAt < cacheTtlMs) return - refresh() + val anyStale = Subscriptions.get().subs.value.any { ch -> + val entry = channelCache[ch.url] + entry == null || now - entry.fetchedAt >= perChannelTtlMs + } + if (anyStale || _ui.value.items.isEmpty()) refresh() } fun refresh() { val channels = Subscriptions.get().subs.value if (channels.isEmpty()) { _ui.update { SubscriptionFeedUiState(loading = false, items = emptyList()) } + channelCache.clear() return } inFlight?.cancel() _ui.update { it.copy(loading = true, error = null) } inFlight = viewModelScope.launch { try { - val perChannelMax = 5 val gate = Semaphore(parallelism) - val items = coroutineScope { - val deferreds = channels.map { ch -> - async { - gate.withPermit { - withTimeoutOrNull(perChannelTimeoutMs) { - runCatching { - val info = uniffi.strawcore.channelInfo(ch.url) - info.videos - .take(perChannelMax) - .map { v -> - StreamItem( - url = v.url, - title = v.title.ifBlank { "(no title)" }, - uploader = v.uploader.ifBlank { ch.name }, - uploaderUrl = v.uploaderUrl ?: ch.url, - thumbnail = v.thumbnail, - durationSeconds = v.durationSeconds, - viewCount = v.viewCount, - ) - } - }.onFailure { - strawLogW("StrawFeed") { "channel fetch failed for ${ch.url}: ${it.message}" } - }.getOrDefault(emptyList()) - } ?: run { - strawLogW("StrawFeed") { "channel fetch timed out: ${ch.url}" } - emptyList() - } - } + val now = System.currentTimeMillis() + coroutineScope { + channels + .filter { ch -> + val entry = channelCache[ch.url] + entry == null || now - entry.fetchedAt >= perChannelTtlMs } - } - deferreds.awaitAll() + .map { ch -> async { gate.withPermit { fetchChannelInto(ch) } } } + .awaitAll() } - .flatten() - // No reliable upload-timestamp on the search-item shape — sort - // by view count desc as a soft proxy for recency-popularity - // within the recent window. - .sortedByDescending { it.viewCount } - .take(200) _ui.update { SubscriptionFeedUiState( loading = false, - items = items, + items = mergeFromCache(channels), lastFetchedAt = System.currentTimeMillis(), ) } @@ -134,4 +114,45 @@ class SubscriptionFeedViewModel : ViewModel() { } } } + + private suspend fun fetchChannelInto(ch: ChannelRef) { + val perChannelMax = 5 + val fetched = withTimeoutOrNull(perChannelTimeoutMs) { + runCatching { + val info = uniffi.strawcore.channelInfo(ch.url) + info.videos.take(perChannelMax).map { v -> + StreamItem( + url = v.url, + title = v.title.ifBlank { "(no title)" }, + uploader = v.uploader.ifBlank { ch.name }, + uploaderUrl = v.uploaderUrl ?: ch.url, + thumbnail = v.thumbnail, + durationSeconds = v.durationSeconds, + viewCount = v.viewCount, + ) + } + }.onFailure { + strawLogW("StrawFeed") { "channel fetch failed for ${ch.url}: ${it.message}" } + }.getOrDefault(emptyList()) + } ?: run { + strawLogW("StrawFeed") { "channel fetch timed out: ${ch.url}" } + emptyList() + } + // Only update the cache on a successful fetch. A timeout/error + // leaves any prior cache entry intact, so a glitchy channel + // doesn't blank your feed for that channel. + if (fetched.isNotEmpty()) { + channelCache[ch.url] = ChannelCacheEntry(System.currentTimeMillis(), fetched) + } + } + + 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. + channelCache.keys.toList().forEach { if (it !in subUrls) channelCache.remove(it) } + return channels.flatMap { ch -> channelCache[ch.url]?.items.orEmpty() } + .sortedByDescending { it.viewCount } + .take(200) + } } diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerLeaveHandler.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerLeaveHandler.kt deleted file mode 100644 index e7f57495a..000000000 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerLeaveHandler.kt +++ /dev/null @@ -1,17 +0,0 @@ -/* - * SPDX-FileCopyrightText: 2026 Sulkta-Coop - * SPDX-License-Identifier: GPL-3.0-or-later - * - * Bridge between StrawActivity.onUserLeaveHint() (HOME / recents button) - * and the active PlayerScreen. When the user leaves the player by pressing - * HOME, we want a seamless hand-off to the background-audio foreground - * service — not Picture-in-Picture. PlayerScreen registers a handler on - * compose, clears it on dispose; the activity calls it from the OS hook. - */ - -package com.sulkta.straw.feature.player - -object PlayerLeaveHandler { - @Volatile - var handler: (() -> Unit)? = null -} diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerScreen.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerScreen.kt index 173718f7a..0f4339d6f 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerScreen.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerScreen.kt @@ -2,19 +2,12 @@ * SPDX-FileCopyrightText: 2026 Sulkta-Coop * SPDX-License-Identifier: GPL-3.0-or-later * - * Fullscreen player surface. After the V-2 unification, the player - * itself lives in PlaybackService (one ExoPlayer for the whole app). - * This composable is a thin shell that: - * 1. Asks the PlayerViewModel to resolve the stream URL - * 2. Pushes the resolved MediaItem into the shared MediaController - * 3. Renders PlayerView bound to that controller - * 4. Runs the SponsorBlock skip loop against the controller - * 5. Lets the user drag-down to dismiss into the minibar - * - * Audio-only toggle, speed picker, share, manual PiP, and the - * background-audio button stay as overlays. Audio-only flips the - * controller's track-selection params; nothing more to do because - * playback is one player. + * Fullscreen player surface. The player itself lives in PlaybackService + * (one ExoPlayer for the whole app); this composable is a thin shell that + * renders a PlayerView bound to the shared MediaController, lets the user + * drag down to dismiss into the minibar, and overlays speed / audio-only + * / share / PiP / minimize controls. SponsorBlock auto-skip lives at the + * activity root in [SponsorBlockSkipLoop]. */ package com.sulkta.straw.feature.player @@ -35,12 +28,10 @@ import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row -import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxSize -import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.offset import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size -import androidx.compose.foundation.layout.width import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material3.AlertDialog import androidx.compose.material3.CircularProgressIndicator @@ -65,7 +56,6 @@ import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.unit.IntOffset import androidx.compose.ui.unit.dp import androidx.compose.ui.viewinterop.AndroidView -import androidx.compose.foundation.layout.offset import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.lifecycle.viewmodel.compose.viewModel import androidx.media3.common.C @@ -74,6 +64,8 @@ import androidx.media3.common.Player import androidx.media3.common.TrackSelectionParameters import androidx.media3.common.util.UnstableApi import androidx.media3.ui.PlayerView +import com.sulkta.straw.OverlayChromeColor +import com.sulkta.straw.feature.detail.VideoDetailViewModel import com.sulkta.straw.net.SbSegment import com.sulkta.straw.util.strawLogI import kotlin.math.roundToInt @@ -85,67 +77,46 @@ import kotlinx.coroutines.launch fun PlayerScreen( streamUrl: String, title: String, - startPositionMs: Long = 0L, onMinimize: () -> Unit = {}, - vm: PlayerViewModel = viewModel(), + vm: VideoDetailViewModel = viewModel(), ) { val context = LocalContext.current val controller = LocalStrawController.current val state by vm.ui.collectAsStateWithLifecycle() - LaunchedEffect(streamUrl) { vm.resolve(streamUrl) } + LaunchedEffect(streamUrl) { vm.load(streamUrl) } var playbackSpeed by remember { mutableStateOf(1.0f) } var audioOnly by remember { mutableStateOf(false) } var showSpeedDialog by remember { mutableStateOf(false) } // Drag-to-minimize: vertical offset accumulated during the gesture. - // On release, if past threshold we dismiss into the minibar. + // On release past the threshold we dismiss into the minibar. val density = LocalDensity.current val dismissThresholdPx = with(density) { 200.dp.toPx() } val dragY = remember { Animatable(0f) } val scope = rememberCoroutineScope() - // Push the resolved video into the shared controller as soon as we - // have stream URLs. If something else is already playing the same - // streamUrl, just seek instead of re-loading. - // For metadata that vm.resolve doesn't return (uploader / thumbnail) we - // try to lift them from the matching VideoDetail item if it's open in - // the same nav stack; otherwise fall back to whatever NowPlaying - // already has. Either way the minibar gets enough to render. - val detailVm: com.sulkta.straw.feature.detail.VideoDetailViewModel = viewModel() - LaunchedEffect(streamUrl) { detailVm.load(streamUrl) } - val detailState by detailVm.ui.collectAsStateWithLifecycle() + // When the resolved playback for this URL is ready, push it into the + // shared controller — unless it's already playing this exact URL, in + // which case do nothing: the player is already where we want it. The + // previous "seek-to-self" path here was always a few ms backwards and + // produced a jerk on every entry; the controller's currentPosition is + // its own source of truth. val resolved = state.resolved - LaunchedEffect(controller, resolved, detailState.detail) { + val detail = state.detail + LaunchedEffect(controller, resolved, detail) { val c = controller ?: return@LaunchedEffect val r = resolved ?: return@LaunchedEffect - val d = detailState.detail - val uploader = d?.uploader ?: NowPlaying.current.value?.uploader.orEmpty() - val thumbnail = d?.thumbnail ?: NowPlaying.current.value?.thumbnail - val sameVideo = NowPlaying.current.value?.streamUrl == streamUrl - val currentTitle = c.mediaMetadata.title?.toString() - if (sameVideo && currentTitle == title) { - if (startPositionMs > 0) c.seekTo(startPositionMs) - if (!c.isPlaying) c.play() - NowPlaying.set( - NowPlayingItem( - streamUrl = streamUrl, - title = title, - uploader = uploader, - thumbnail = thumbnail, - segments = r.segments, - ), - ) - } else { - c.setPlayingFrom( - streamUrl = streamUrl, - title = title, - uploader = uploader, - thumbnail = thumbnail, - resolved = r, - startPositionMs = startPositionMs, - ) - } + val uploader = detail?.uploader.orEmpty() + val thumbnail = detail?.thumbnail + if (NowPlaying.current.value?.streamUrl == streamUrl) return@LaunchedEffect + c.setPlayingFrom( + streamUrl = streamUrl, + title = title, + uploader = uploader, + thumbnail = thumbnail, + resolved = r, + ) } // Surface ExoPlayer failures from the service into the UI. @@ -161,9 +132,6 @@ fun PlayerScreen( onDispose { c?.removeListener(listener) } } - // Manual-PiP wiring (the ⊟ overlay button). The activity is the PiP - // host; we just feed it the right params. Auto-enter-on-home stays - // disabled — HOME triggers seamless minibar/background per #255. val activity = context as? Activity Box( @@ -174,7 +142,6 @@ fun PlayerScreen( detectVerticalDragGestures( onDragEnd = { if (dragY.value > dismissThresholdPx) { - // Snap to dismiss + pop into minibar. onMinimize() } else { scope.launch { dragY.animateTo(0f, tween(180)) } @@ -185,7 +152,6 @@ fun PlayerScreen( }, onVerticalDrag = { _, dy -> scope.launch { - // Clamp to non-negative — upward drag has no effect. dragY.snapTo((dragY.value + dy).coerceAtLeast(0f)) } }, @@ -222,26 +188,23 @@ fun PlayerScreen( } }, update = { it.player = controller }, + onRelease = { it.player = null }, modifier = Modifier.fillMaxSize(), ) - // SponsorBlock segment count badge — small overlay top-left. - resolved.let { r -> - Box( - modifier = Modifier - .align(Alignment.TopStart) - .padding(12.dp) - .clip(RoundedCornerShape(6.dp)) - .background(Color(0xCC222222)) - .padding(horizontal = 8.dp, vertical = 4.dp), - ) { - Text( - text = "SB: ${r.segments.size} segment${if (r.segments.size == 1) "" else "s"}", - color = Color.White, - style = MaterialTheme.typography.labelSmall, - ) - } + Box( + modifier = Modifier + .align(Alignment.TopStart) + .padding(12.dp) + .clip(RoundedCornerShape(6.dp)) + .background(OverlayChromeColor) + .padding(horizontal = 8.dp, vertical = 4.dp), + ) { + Text( + text = "SB: ${resolved.segments.size} segment${if (resolved.segments.size == 1) "" else "s"}", + color = Color.White, + style = MaterialTheme.typography.labelSmall, + ) } - // Top-right overlay — speed / audio-only / share / PiP / minimize. Row( modifier = Modifier.align(Alignment.TopEnd).padding(12.dp), horizontalArrangement = Arrangement.spacedBy(8.dp), @@ -288,7 +251,6 @@ fun PlayerScreen( Toast.makeText(context, "PiP failed: ${t.message}", Toast.LENGTH_LONG).show() } } - // Explicit minimize button — same effect as drag-down. OverlayButton(label = "⌄") { onMinimize() } } @@ -314,7 +276,7 @@ private fun OverlayButton(label: String, onClick: () -> Unit) { modifier = Modifier .size(36.dp) .clip(RoundedCornerShape(6.dp)) - .background(Color(0xCC222222)) + .background(OverlayChromeColor) .clickable(onClick = onClick), contentAlignment = Alignment.Center, ) { @@ -360,9 +322,12 @@ private fun SpeedPickerDialog( /** * SponsorBlock skip loop driven by the controller's currentPosition. - * Runs at the activity composition root (not per-screen) so it skips - * segments whether the user is fullscreen, in the minibar, or away from - * the player surface. + * Lives at the activity composition root so it skips segments whether + * the user is fullscreen, in the minibar, or away from the player + * surface. + * + * The `skipped` set is only mutated from this single coroutine — safe + * without synchronization while that invariant holds. */ @Composable @OptIn(UnstableApi::class) diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerViewModel.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerViewModel.kt deleted file mode 100644 index 5dd9eeaa3..000000000 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerViewModel.kt +++ /dev/null @@ -1,103 +0,0 @@ -/* - * SPDX-FileCopyrightText: 2026 Sulkta-Coop - * SPDX-License-Identifier: GPL-3.0-or-later - * - * Phase U-3 / Path C-4: extractor moved from NewPipeExtractor (Java) to - * strawcore (Rust + rustypipe via UniFFI). PlayerScreen still calls - * vm.resolve(url) the same way — the engine underneath flipped. - */ - -package com.sulkta.straw.feature.player - -import androidx.lifecycle.ViewModel -import androidx.lifecycle.viewModelScope -import com.sulkta.straw.data.Settings -import com.sulkta.straw.net.SbSegment -import com.sulkta.straw.net.SponsorBlockClient -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.launch -import kotlinx.coroutines.withContext - -data class ResolvedPlayback( - val title: String, - val videoUrl: String?, - val audioUrl: String?, - val combinedUrl: String?, - val dashMpdUrl: String?, - val hlsUrl: String?, - val segments: List = emptyList(), -) { - /** Have anything playable? */ - val isPlayable: Boolean - get() = !combinedUrl.isNullOrBlank() || !videoUrl.isNullOrBlank() || - !dashMpdUrl.isNullOrBlank() || !hlsUrl.isNullOrBlank() -} - -data class PlayerUiState( - val loading: Boolean = true, - val resolved: ResolvedPlayback? = null, - val error: String? = null, -) - -class PlayerViewModel : ViewModel() { - private val _ui = MutableStateFlow(PlayerUiState()) - val ui: StateFlow = _ui.asStateFlow() - - fun resolve(streamUrl: String) { - _ui.value = PlayerUiState(loading = true) - viewModelScope.launch { - try { - // strawcore.streamInfo is a suspend fun running on the tokio - // runtime baked into libstrawcore.so — no Dispatchers.IO needed. - val info = uniffi.strawcore.streamInfo(streamUrl) - val videoId = info.id - - val sbCategories = Settings.get().sbCategories.value.map { it.key } - val segments = if (sbCategories.isEmpty()) { - emptyList() - } else { - withContext(Dispatchers.IO) { - runCatching { SponsorBlockClient.fetch(videoId, sbCategories) } - .getOrDefault(emptyList()) - } - } - - val maxRes = Settings.get().maxResolution.value.ceiling - - // Audit HIGH-8 carry-over: filter by max resolution but fall - // back to lowest available if the ceiling excludes everything. - fun pickVideo(streams: List): String? { - if (streams.isEmpty()) return null - val filtered = streams.filter { it.height <= maxRes } - val pool = filtered.ifEmpty { streams } - return pool.maxByOrNull { it.bitrate }?.url - } - - val combined = pickVideo(info.combined) - val videoOnly = pickVideo(info.videoOnly) - val audioOnly = info.audioOnly.maxByOrNull { it.bitrate }?.url - - _ui.value = PlayerUiState( - loading = false, - resolved = ResolvedPlayback( - title = info.title, - videoUrl = videoOnly, - audioUrl = audioOnly, - combinedUrl = combined, - dashMpdUrl = info.dashMpdUrl?.takeIf { it.isNotBlank() }, - hlsUrl = info.hlsUrl?.takeIf { it.isNotBlank() }, - segments = segments, - ), - ) - } catch (t: Throwable) { - _ui.value = PlayerUiState( - loading = false, - error = t.message ?: t.javaClass.simpleName, - ) - } - } - } -} 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 6ad3d376f..498dc96a9 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 @@ -41,6 +41,7 @@ import androidx.media3.common.util.UnstableApi import androidx.media3.session.MediaController import androidx.media3.session.SessionToken import com.google.common.util.concurrent.MoreExecutors +import com.sulkta.straw.feature.detail.ResolvedPlayback val LocalStrawController = compositionLocalOf { null } diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/search/SearchViewModel.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/search/SearchViewModel.kt index 338388af9..e48aecd83 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/search/SearchViewModel.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/search/SearchViewModel.kt @@ -41,15 +41,11 @@ class SearchViewModel : ViewModel() { fun submit() { val q = _ui.value.query.trim() if (q.isEmpty()) return - runCatching { History.get().recordSearch(q) } _ui.value = _ui.value.copy(loading = true, error = null, results = emptyList()) viewModelScope.launch { try { - // Phase U-2 / Path C-3: rustypipe via UniFFI. The bindgen-generated - // `search()` is already a suspend fun running on the tokio runtime - // baked into libstrawcore.so — no Dispatchers.IO wrapper needed, - // the JNI call returns to us on the caller dispatcher when the - // future completes. + // strawcore.search() is suspend on the tokio runtime baked + // into libstrawcore.so — no Dispatchers.IO wrap needed. val rustItems = uniffi.strawcore.search(q) val items = rustItems.map { r -> StreamItem( @@ -63,6 +59,9 @@ class SearchViewModel : ViewModel() { ) } _ui.value = _ui.value.copy(loading = false, results = items) + // Record AFTER the search succeeds so mistyped queries + // that error out don't pollute the recent-searches list. + runCatching { History.get().recordSearch(q) } } catch (t: Throwable) { _ui.value = _ui.value.copy( loading = false, diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/util/HtmlText.kt b/strawApp/src/main/kotlin/com/sulkta/straw/util/HtmlText.kt index f228b23d2..b4933d38a 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/util/HtmlText.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/util/HtmlText.kt @@ -2,9 +2,9 @@ * SPDX-FileCopyrightText: 2026 Sulkta-Coop * SPDX-License-Identifier: GPL-3.0-or-later * - * Strip HTML tags from NewPipeExtractor's description.content for plain-text - * rendering. Day-3 polish replaces this with a real Markwon/Compose annotated - * renderer; for now we just want readable text. + * Strip HTML tags from video descriptions for plain-text rendering. + * Replace with a real annotated renderer (Markwon, Compose annotated + * strings) when the description UI needs richer formatting. */ package com.sulkta.straw.util