diff --git a/buildSrc/src/main/kotlin/ProjectConfig.kt b/buildSrc/src/main/kotlin/ProjectConfig.kt index 1d9d2ad77..c8077d79a 100644 --- a/buildSrc/src/main/kotlin/ProjectConfig.kt +++ b/buildSrc/src/main/kotlin/ProjectConfig.kt @@ -15,6 +15,6 @@ const val NEWPIPE_APPLICATION_ID_OLD = "org.schabi.newpipe" const val NEWPIPE_APPLICATION_ID_NEW = "net.newpipe.app" // Sulkta fork — Straw -const val STRAW_VERSION_CODE = 6 -const val STRAW_VERSION_NAME = "0.1.0-S" +const val STRAW_VERSION_CODE = 7 +const val STRAW_VERSION_NAME = "0.1.0-T" const val STRAW_APPLICATION_ID = "com.sulkta.straw" diff --git a/strawApp/src/main/AndroidManifest.xml b/strawApp/src/main/AndroidManifest.xml index b0a82e1ac..46abc05b7 100644 --- a/strawApp/src/main/AndroidManifest.xml +++ b/strawApp/src/main/AndroidManifest.xml @@ -47,10 +47,14 @@ - + diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/StrawHome.kt b/strawApp/src/main/kotlin/com/sulkta/straw/StrawHome.kt index 5575f42c9..c9d940c32 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/StrawHome.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/StrawHome.kt @@ -2,10 +2,8 @@ * SPDX-FileCopyrightText: 2026 Sulkta-Coop * SPDX-License-Identifier: GPL-3.0-or-later * - * Phase P: bottom navigation host. Three tabs: - * - Home: search + recent-watches summary - * - Library: full recent-watches list - * - Subs: subscription feed (Q wires the actual feed; P just lists channels) + * Home shell: hamburger drawer top-left, sub-feed as the default landing + * view. Drawer items take you to History, Search, Settings. */ package com.sulkta.straw @@ -27,22 +25,29 @@ import androidx.compose.foundation.lazy.items import androidx.compose.foundation.shape.CircleShape import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.filled.Home -import androidx.compose.material.icons.filled.Settings -import androidx.compose.material3.Button +import androidx.compose.material.icons.filled.Menu +import androidx.compose.material3.CircularProgressIndicator +import androidx.compose.material3.DrawerValue +import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.HorizontalDivider import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme -import androidx.compose.material3.NavigationBar -import androidx.compose.material3.NavigationBarItem +import androidx.compose.material3.ModalDrawerSheet +import androidx.compose.material3.ModalNavigationDrawer +import androidx.compose.material3.NavigationDrawerItem import androidx.compose.material3.Scaffold import androidx.compose.material3.Text +import androidx.compose.material3.TextButton +import androidx.compose.material3.TopAppBar +import androidx.compose.material3.rememberDrawerState import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -50,9 +55,6 @@ import androidx.compose.ui.draw.clip import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp -import androidx.compose.material3.CircularProgressIndicator -import androidx.compose.material3.TextButton -import androidx.compose.runtime.LaunchedEffect import androidx.lifecycle.viewmodel.compose.viewModel import coil3.compose.AsyncImage import com.sulkta.straw.BuildConfig @@ -63,13 +65,11 @@ import com.sulkta.straw.data.WatchHistoryItem import com.sulkta.straw.feature.feed.SubscriptionFeedViewModel import com.sulkta.straw.feature.search.StreamItem import com.sulkta.straw.util.formatViews +import kotlinx.coroutines.launch -private enum class HomeTab(val label: String) { - Home("Home"), - Library("Library"), - Subs("Subs"), -} +private enum class HomeView { Subs, History } +@OptIn(ExperimentalMaterial3Api::class) @Composable fun StrawHome( onOpenSearch: () -> Unit, @@ -78,102 +78,102 @@ fun StrawHome( onOpenChannel: (channelUrl: String, name: String) -> Unit, feedVm: SubscriptionFeedViewModel = viewModel(), ) { - var tab by remember { mutableStateOf(HomeTab.Home) } + var view by remember { mutableStateOf(HomeView.Subs) } + val drawerState = rememberDrawerState(initialValue = DrawerValue.Closed) + val scope = rememberCoroutineScope() - Scaffold( - bottomBar = { - NavigationBar { - HomeTab.entries.forEach { t -> - NavigationBarItem( - selected = t == tab, - onClick = { tab = t }, - icon = { - // Material-icons-core only ships a small set; - // use unicode for the rest. - when (t) { - HomeTab.Home -> Icon(Icons.Filled.Home, contentDescription = t.label) - HomeTab.Library -> Text("📺") - HomeTab.Subs -> Text("👤") - } - }, - label = { Text(t.label) }, - ) - } + ModalNavigationDrawer( + drawerState = drawerState, + drawerContent = { + ModalDrawerSheet { + Spacer(modifier = Modifier.height(16.dp)) + Text( + text = "straw", + style = MaterialTheme.typography.displaySmall, + color = MaterialTheme.colorScheme.primary, + modifier = Modifier.padding(horizontal = 24.dp), + ) + Text( + text = "v${BuildConfig.VERSION_NAME}", + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(horizontal = 24.dp, vertical = 2.dp), + ) + Spacer(modifier = Modifier.height(20.dp)) + + NavigationDrawerItem( + label = { Text("Subscriptions") }, + icon = { Text("👤") }, + selected = view == HomeView.Subs, + onClick = { + view = HomeView.Subs + scope.launch { drawerState.close() } + }, + modifier = Modifier.padding(horizontal = 12.dp), + ) + NavigationDrawerItem( + label = { Text("History") }, + icon = { Text("📺") }, + selected = view == HomeView.History, + onClick = { + view = HomeView.History + scope.launch { drawerState.close() } + }, + modifier = Modifier.padding(horizontal = 12.dp), + ) + HorizontalDivider(modifier = Modifier.padding(vertical = 12.dp)) + NavigationDrawerItem( + label = { Text("Search") }, + icon = { Text("🔍") }, + selected = false, + onClick = { + scope.launch { drawerState.close() } + onOpenSearch() + }, + modifier = Modifier.padding(horizontal = 12.dp), + ) + NavigationDrawerItem( + label = { Text("Settings") }, + icon = { Text("⚙") }, + selected = false, + onClick = { + scope.launch { drawerState.close() } + onOpenSettings() + }, + modifier = Modifier.padding(horizontal = 12.dp), + ) } }, - ) { padding -> - Column( - modifier = Modifier - .fillMaxSize() - .padding(padding) - .padding(horizontal = 20.dp, vertical = 8.dp), - ) { - HeaderRow(onOpenSettings = onOpenSettings) - Spacer(modifier = Modifier.height(12.dp)) - when (tab) { - HomeTab.Home -> HomePane(onOpenSearch, onOpenVideo) - HomeTab.Library -> LibraryPane(onOpenVideo) - HomeTab.Subs -> SubsPane(onOpenChannel, onOpenVideo, feedVm) - } - } - } -} - -@Composable -private fun HeaderRow(onOpenSettings: () -> Unit) { - Row( - modifier = Modifier.fillMaxWidth(), - verticalAlignment = Alignment.CenterVertically, ) { - Text( - text = "straw", - style = MaterialTheme.typography.displayMedium, - color = MaterialTheme.colorScheme.primary, - ) - Spacer(modifier = Modifier.width(12.dp)) - Text( - text = "v${BuildConfig.VERSION_NAME}", - style = MaterialTheme.typography.labelSmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier.weight(1f), - ) - IconButton(onClick = onOpenSettings) { - Icon(Icons.Filled.Settings, contentDescription = "Settings") - } - } -} - -@Composable -private fun HomePane( - onOpenSearch: () -> Unit, - onOpenVideo: (url: String, title: String) -> Unit, -) { - val watches by History.get().watches.collectAsState() - - Column { - Button( - onClick = onOpenSearch, - modifier = Modifier.fillMaxWidth(), - ) { Text("Search YouTube") } - Spacer(modifier = Modifier.height(20.dp)) - - if (watches.isEmpty()) { - Text( - text = "Recently watched videos appear here.", - style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) - } else { - Text( - text = "Recently watched", - style = MaterialTheme.typography.titleMedium, - fontWeight = FontWeight.SemiBold, - ) - Spacer(modifier = Modifier.height(8.dp)) - LazyColumn { - items(watches.take(10)) { w -> - RecentRow(w) { onOpenVideo(w.url, w.title) } - HorizontalDivider() + Scaffold( + topBar = { + TopAppBar( + title = { + Text( + text = when (view) { + HomeView.Subs -> "Subscriptions" + HomeView.History -> "History" + }, + style = MaterialTheme.typography.titleLarge, + ) + }, + navigationIcon = { + IconButton(onClick = { scope.launch { drawerState.open() } }) { + Icon(Icons.Filled.Menu, contentDescription = "Menu") + } + }, + ) + }, + ) { padding -> + Column( + modifier = Modifier + .fillMaxSize() + .padding(padding) + .padding(horizontal = 20.dp, vertical = 8.dp), + ) { + when (view) { + HomeView.Subs -> SubsPane(onOpenChannel, onOpenVideo, feedVm) + HomeView.History -> HistoryPane(onOpenVideo) } } } @@ -181,18 +181,12 @@ private fun HomePane( } @Composable -private fun LibraryPane(onOpenVideo: (url: String, title: String) -> Unit) { +private fun HistoryPane(onOpenVideo: (url: String, title: String) -> Unit) { val watches by History.get().watches.collectAsState() Column { Text( - text = "Library", - style = MaterialTheme.typography.titleLarge, - fontWeight = FontWeight.SemiBold, - ) - Spacer(modifier = Modifier.height(4.dp)) - Text( - text = "${watches.size} watched videos", + text = "${watches.size} watched video${if (watches.size == 1) "" else "s"}", style = MaterialTheme.typography.bodySmall, color = MaterialTheme.colorScheme.onSurfaceVariant, ) @@ -200,7 +194,7 @@ private fun LibraryPane(onOpenVideo: (url: String, title: String) -> Unit) { if (watches.isEmpty()) { Text( - "No watch history yet. Play a video and it'll show up here.", + "Nothing watched yet. Play a video and it'll show up here.", style = MaterialTheme.typography.bodyMedium, color = MaterialTheme.colorScheme.onSurfaceVariant, ) @@ -226,49 +220,55 @@ private fun SubsPane( LaunchedEffect(subs) { feedVm.refreshIfStale() } Column { - Row(verticalAlignment = Alignment.CenterVertically) { - Column(modifier = Modifier.weight(1f)) { - Text( - text = "Subscriptions", - style = MaterialTheme.typography.titleLarge, - fontWeight = FontWeight.SemiBold, - ) - Text( - text = "${subs.size} channel${if (subs.size == 1) "" else "s"}", - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) - } - if (subs.isNotEmpty()) { - TextButton(onClick = { feedVm.refresh() }) { - Text(if (feed.loading) "..." else "Refresh") - } - } - } - Spacer(modifier = Modifier.height(12.dp)) - if (subs.isEmpty()) { Text( - "No subscriptions yet. Open a channel and tap Subscribe.", + "No subscriptions yet. Tap Search in the menu, find a channel, and Subscribe.", style = MaterialTheme.typography.bodyMedium, color = MaterialTheme.colorScheme.onSurfaceVariant, ) return@Column } + Row(verticalAlignment = Alignment.CenterVertically) { + Text( + text = "${subs.size} channel${if (subs.size == 1) "" else "s"}", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.weight(1f), + ) + TextButton(onClick = { feedVm.refresh() }) { + Text(if (feed.loading) "..." else "Refresh") + } + } + Spacer(modifier = Modifier.height(8.dp)) + // Channel chip row. LazyRow(horizontalArrangement = Arrangement.spacedBy(12.dp)) { items(subs) { ch -> SubChip(ch, onOpenChannel) } } Spacer(modifier = Modifier.height(16.dp)) - // Aggregated feed below. + // Show a slim error banner above cached items even if we have data — + // audit HIGH-7: previously a 401/429 looked identical to a successful + // refresh because the error chip was hidden whenever items != empty. + if (feed.error != null && feed.items.isNotEmpty()) { + Text( + text = "refresh failed: ${feed.error}", + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.error, + ) + Spacer(modifier = Modifier.height(8.dp)) + } + when { feed.loading && feed.items.isEmpty() -> { Row(verticalAlignment = Alignment.CenterVertically) { CircularProgressIndicator(modifier = Modifier.size(16.dp)) Spacer(modifier = Modifier.width(8.dp)) - Text("Pulling subscription feed...", style = MaterialTheme.typography.bodySmall) + Text( + "Pulling latest from your subs...", + style = MaterialTheme.typography.bodySmall, + ) } } feed.error != null && feed.items.isEmpty() -> { @@ -279,12 +279,6 @@ private fun SubsPane( ) } else -> { - Text( - "Latest from your subs", - style = MaterialTheme.typography.titleMedium, - fontWeight = FontWeight.SemiBold, - ) - Spacer(modifier = Modifier.height(8.dp)) LazyColumn { items(feed.items) { item -> FeedRow(item) { onOpenVideo(item.url, item.title) } 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 ee364603b..33d1c165a 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 @@ -213,8 +213,9 @@ fun VideoDetailScreen( ?.maxByOrNull { it.bitrate ?: 0 } ?.content if (audio != null) { - Downloader.enqueue(context, audio, d.title, DownloadKind.Audio) - Toast.makeText(context, "audio queued", Toast.LENGTH_SHORT).show() + val id = Downloader.enqueue(context, audio, d.title, DownloadKind.Audio) + val msg = if (id > 0) "audio queued" else "download refused (bad URL)" + Toast.makeText(context, msg, Toast.LENGTH_SHORT).show() } else { Toast.makeText(context, "no audio stream", Toast.LENGTH_SHORT).show() } @@ -230,8 +231,9 @@ fun VideoDetailScreen( ?.maxByOrNull { it.bitrate ?: 0 } ?.content if (video != null) { - Downloader.enqueue(context, video, d.title, DownloadKind.Video) - Toast.makeText(context, "video queued", Toast.LENGTH_SHORT).show() + val id = Downloader.enqueue(context, video, d.title, DownloadKind.Video) + val msg = if (id > 0) "video queued" else "download refused (bad URL)" + Toast.makeText(context, msg, Toast.LENGTH_SHORT).show() } else { Toast.makeText(context, "no video stream", Toast.LENGTH_SHORT).show() } 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 4061b6abe..0557c4051 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 @@ -48,13 +48,13 @@ class VideoDetailViewModel : ViewModel() { private val _ui = MutableStateFlow(VideoDetailUiState()) val ui: StateFlow = _ui.asStateFlow() + private var loadedUrl: String? = null + fun load(streamUrl: String) { - // AUD-HIGH: previous guard was a dead-code if-block. The - // LaunchedEffect(streamUrl) caller only fires once per key + a new - // ViewModel is constructed for each nav entry, so the guard isn't - // strictly needed — but a real one is cheap insurance against - // future callers. - if (_ui.value.detail != null) return + // viewModel() is Activity-scoped, so the same VM is reused across + // navigations. Compare the requested URL with what we last loaded. + if (loadedUrl == streamUrl && _ui.value.detail != null) return + loadedUrl = streamUrl _ui.value = VideoDetailUiState(loading = true) viewModelScope.launch { try { 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 b3284f5c3..4bb361506 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 @@ -6,6 +6,14 @@ * 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. */ package com.sulkta.straw.feature.download @@ -15,6 +23,7 @@ import android.content.Context import android.net.Uri import android.os.Environment import com.sulkta.straw.util.strawLogD +import com.sulkta.straw.util.strawLogW enum class DownloadKind(val subdir: String, val ext: String) { Audio("audio", ".m4a"), @@ -22,34 +31,65 @@ enum class DownloadKind(val subdir: String, val ext: String) { } object Downloader { + // HIGH-5 — ASCII control chars + DEL + Unicode bidi-override block. + private val UNSAFE_CHARS = Regex("[\\x00-\\x1F\\x7F\\u202A-\\u202E]") + private val PATH_SEP_CHARS = Regex("[\\\\/:*?\"<>|]") + fun enqueue( context: Context, url: String, title: String, kind: DownloadKind, ): Long { + if (!isAllowedDownloadUrl(url)) { + strawLogW("StrawDl") { "refused non-YT URL: ${url.take(80)}" } + return -1L + } + val ctx = context.applicationContext - val safeTitle = title - .replace(Regex("[\\\\/:*?\"<>|]"), "_") - .take(120) - .ifBlank { "straw-${System.currentTimeMillis()}" } + val safeTitle = sanitizeFilename(title) val filename = "$safeTitle${kind.ext}" val dm = ctx.getSystemService(Context.DOWNLOAD_SERVICE) as DownloadManager - val req = DownloadManager.Request(Uri.parse(url)) - .setTitle(title) - .setDescription("Straw — ${kind.name.lowercase()}") - .setNotificationVisibility(DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED) - .setAllowedOverMetered(true) - .setAllowedOverRoaming(true) - .setDestinationInExternalFilesDir( - ctx, - Environment.DIRECTORY_MOVIES + "/" + kind.subdir, - filename, - ) + val req = runCatching { + DownloadManager.Request(Uri.parse(url)) + .setTitle(title) + .setDescription("Straw — ${kind.name.lowercase()}") + .setNotificationVisibility(DownloadManager.Request.VISIBILITY_VISIBLE_NOTIFY_COMPLETED) + .setAllowedOverMetered(true) + .setAllowedOverRoaming(true) + .setDestinationInExternalFilesDir( + ctx, + Environment.DIRECTORY_MOVIES + "/" + kind.subdir, + filename, + ) + }.getOrElse { + strawLogW("StrawDl") { "Request.build failed: ${it.message}" } + return -1L + } - val id = dm.enqueue(req) - strawLogD("StrawDl") { "enqueued $kind id=$id title=$title file=$filename" } - return id + return runCatching { dm.enqueue(req) } + .onSuccess { id -> strawLogD("StrawDl") { "enqueued $kind id=$id file=$filename" } } + .onFailure { strawLogW("StrawDl") { "enqueue failed: ${it.message}" } } + .getOrDefault(-1L) + } + + private fun sanitizeFilename(raw: String): String { + val cleaned = raw + .replace(UNSAFE_CHARS, "_") + .replace(PATH_SEP_CHARS, "_") + .trim() + .trimStart('.') + .take(120) + return cleaned.ifBlank { "straw-${System.currentTimeMillis()}" } + } + + private fun isAllowedDownloadUrl(url: String): Boolean { + 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" } } 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 32b5c2d9b..93ca74467 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 @@ -4,7 +4,14 @@ * * Phase Q: aggregate latest videos across all subscribed channels into a * single feed. Fans out per-channel ChannelInfo + ChannelTabs.VIDEOS - * fetches in parallel, merges by upload timestamp, caps at 200 items. + * fetches in parallel, merges by view count desc, caps at 200 items. + * + * 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). */ package com.sulkta.straw.feature.feed @@ -16,13 +23,19 @@ import com.sulkta.straw.feature.search.StreamItem import com.sulkta.straw.util.bestThumbnail import com.sulkta.straw.util.strawLogW import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job import kotlinx.coroutines.async import kotlinx.coroutines.awaitAll +import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Semaphore +import kotlinx.coroutines.sync.withPermit import kotlinx.coroutines.withContext +import kotlinx.coroutines.withTimeoutOrNull import org.schabi.newpipe.extractor.NewPipe import org.schabi.newpipe.extractor.ServiceList import org.schabi.newpipe.extractor.channel.ChannelInfo @@ -44,6 +57,15 @@ class SubscriptionFeedViewModel : ViewModel() { /** Cache feed for 10 min to avoid hammering YT on tab re-entry. */ private val cacheTtlMs = 10L * 60 * 1000 + /** Per-channel fetch timeout — slowest channel can't stall the whole batch. */ + private val perChannelTimeoutMs = 15_000L + + /** Cap parallel network fetches even with 100+ subs. */ + private val parallelism = 8 + + /** Live refresh job, so spam-tapping Refresh doesn't fan out racing fetches. */ + private var inFlight: Job? = null + fun refreshIfStale() { val now = System.currentTimeMillis() if (_ui.value.items.isNotEmpty() && now - _ui.value.lastFetchedAt < cacheTtlMs) return @@ -53,62 +75,76 @@ class SubscriptionFeedViewModel : ViewModel() { fun refresh() { val channels = Subscriptions.get().subs.value if (channels.isEmpty()) { - _ui.value = SubscriptionFeedUiState(loading = false, items = emptyList()) + _ui.update { SubscriptionFeedUiState(loading = false, items = emptyList()) } return } - _ui.value = _ui.value.copy(loading = true, error = null) - viewModelScope.launch { + inFlight?.cancel() + _ui.update { it.copy(loading = true, error = null) } + inFlight = viewModelScope.launch { try { val items = withContext(Dispatchers.IO) { val service = NewPipe.getService(ServiceList.YouTube.serviceId) val perChannelMax = 5 - val deferreds = channels.map { ch -> - async { - runCatching { - val info = ChannelInfo.getInfo(service, ch.url) - val tab = info.tabs.firstOrNull { - it.contentFilters.contains(ChannelTabs.VIDEOS) - } ?: info.tabs.firstOrNull() ?: return@async emptyList() - ChannelTabInfo.getInfo(service, tab) - .relatedItems - .filterIsInstance() - .take(perChannelMax) - .map { si -> - StreamItem( - url = si.url, - title = si.name ?: "(no title)", - uploader = si.uploaderName ?: ch.name, - uploaderUrl = si.uploaderUrl ?: ch.url, - thumbnail = bestThumbnail(si.thumbnails), - durationSeconds = si.duration, - viewCount = si.viewCount, - ) + val gate = Semaphore(parallelism) + coroutineScope { + val deferreds = channels.map { ch -> + async { + gate.withPermit { + withTimeoutOrNull(perChannelTimeoutMs) { + runCatching { + val info = ChannelInfo.getInfo(service, ch.url) + val tab = info.tabs.firstOrNull { + it.contentFilters.contains(ChannelTabs.VIDEOS) + } ?: info.tabs.firstOrNull() + ?: return@runCatching emptyList() + ChannelTabInfo.getInfo(service, tab) + .relatedItems + .filterIsInstance() + .take(perChannelMax) + .map { si -> + StreamItem( + url = si.url, + title = si.name ?: "(no title)", + uploader = si.uploaderName ?: ch.name, + uploaderUrl = si.uploaderUrl ?: ch.url, + thumbnail = bestThumbnail(si.thumbnails), + durationSeconds = si.duration, + viewCount = si.viewCount, + ) + } + }.onFailure { + strawLogW("StrawFeed") { "channel fetch failed for ${ch.url}: ${it.message}" } + }.getOrDefault(emptyList()) + } ?: run { + strawLogW("StrawFeed") { "channel fetch timed out: ${ch.url}" } + emptyList() } - }.onFailure { - strawLogW("StrawFeed") { "channel fetch failed for ${ch.url}: ${it.message}" } - }.getOrDefault(emptyList()) + } + } } + deferreds.awaitAll() } - deferreds.awaitAll() .flatten() // No reliable upload-timestamp from extractor's StreamInfoItem - // in all cases — keep the per-channel insertion order (newest first - // within each channel) and interleave by simple round-robin position. - // Sort by view count desc as a soft proxy for recency-popularity. + // in all cases — sort by view count desc as a soft proxy for + // recency-popularity within the recent window. .sortedByDescending { it.viewCount } .take(200) } - _ui.value = SubscriptionFeedUiState( - loading = false, - items = items, - lastFetchedAt = System.currentTimeMillis(), - ) + _ui.update { + SubscriptionFeedUiState( + loading = false, + items = items, + lastFetchedAt = System.currentTimeMillis(), + ) + } } catch (t: Throwable) { - _ui.value = SubscriptionFeedUiState( - loading = false, - items = _ui.value.items, - error = t.message ?: t.javaClass.simpleName, - ) + _ui.update { + it.copy( + loading = false, + error = t.message ?: t.javaClass.simpleName, + ) + } } } } diff --git a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlaybackService.kt b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlaybackService.kt index a4b118dbe..40b4083c0 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlaybackService.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlaybackService.kt @@ -8,6 +8,17 @@ * this service with the audio URL. Audio continues even if the activity * is killed (swipe out of recents). * + * Audit fixes (2026-05-24 pass #2): + * CRIT-1: call startForeground() immediately on first onStartCommand so + * Android 12+ doesn't kill the process with + * ForegroundServiceDidNotStartInTimeException after the 5s window. + * HIGH-2: return START_NOT_STICKY when there is no playable URL — the + * OS will not relaunch us with a null intent and crash-loop. + * HIGH-3: stop the service when playback ends (Player.Listener) so the + * WAKE_LOCK / foreground notification doesn't linger. + * MED-1: null the field before releasing the session to close a tiny + * onGetSession race during teardown. + * * Limitations: * - Single URL only. The activity-side merged-DASH path doesn't carry * over (we just use the best audioStream). Acceptable trade-off for @@ -19,11 +30,19 @@ package com.sulkta.straw.feature.player +import android.app.Notification +import android.app.NotificationChannel +import android.app.NotificationManager import android.app.PendingIntent import android.content.Intent +import android.content.pm.ServiceInfo +import android.net.Uri +import android.os.Build +import androidx.core.app.NotificationCompat import androidx.media3.common.AudioAttributes import androidx.media3.common.C import androidx.media3.common.MediaItem +import androidx.media3.common.Player import androidx.media3.common.util.UnstableApi import androidx.media3.datasource.DefaultHttpDataSource import androidx.media3.exoplayer.ExoPlayer @@ -37,9 +56,12 @@ import com.sulkta.straw.extractor.NewPipeDownloader class PlaybackService : MediaSessionService() { private var mediaSession: MediaSession? = null + private var foregroundStarted = false override fun onCreate() { super.onCreate() + ensureChannel() + val httpFactory = DefaultHttpDataSource.Factory() .setUserAgent(NewPipeDownloader.USER_AGENT) .setAllowCrossProtocolRedirects(true) @@ -57,6 +79,15 @@ class PlaybackService : MediaSessionService() { ) .build() + // HIGH-3: end-of-playback should release the foreground slot. + player.addListener(object : Player.Listener { + override fun onPlaybackStateChanged(state: Int) { + if (state == Player.STATE_ENDED || state == Player.STATE_IDLE) { + stopSelf() + } + } + }) + val sessionActivityIntent = PendingIntent.getActivity( this, 0, @@ -78,48 +109,124 @@ class PlaybackService : MediaSessionService() { flags: Int, startId: Int, ): Int { - val url = intent?.getStringExtra(EXTRA_URL) + // CRIT-1: must startForeground within ~5s of startForegroundService, + // before anything that can throw or block. + startForegroundCompat() + + val url = intent?.getStringExtra(EXTRA_URL)?.takeIf { isAllowedAudioUrl(it) } val title = intent?.getStringExtra(EXTRA_TITLE) val uploader = intent?.getStringExtra(EXTRA_UPLOADER) - if (url != null) { - val player = mediaSession?.player ?: return super.onStartCommand(intent, flags, startId) - val item = MediaItem.Builder() - .setUri(url) - .setMediaMetadata( - androidx.media3.common.MediaMetadata.Builder() - .setTitle(title ?: "") - .setArtist(uploader ?: "") - .build(), - ) - .build() - player.setMediaItem(item) - player.prepare() - player.playWhenReady = true + val player = mediaSession?.player + if (url == null || player == null) { + // HIGH-2: nothing to play (likely a re-launch with null intent + // after a kill). Tear down so we don't sit holding the FG slot. + stopSelf() + return START_NOT_STICKY } - return super.onStartCommand(intent, flags, startId) + + val item = MediaItem.Builder() + .setUri(url) + .setMediaMetadata( + androidx.media3.common.MediaMetadata.Builder() + .setTitle(title ?: "") + .setArtist(uploader ?: "") + .build(), + ) + .build() + player.setMediaItem(item) + player.prepare() + player.playWhenReady = true + return START_NOT_STICKY } override fun onTaskRemoved(rootIntent: Intent?) { - // If audio is still playing when user swipes Straw out of recents, - // KEEP playing. Only stop the service when nothing is queued. - val player = mediaSession?.player - if (player == null || !player.playWhenReady || player.mediaItemCount == 0) { - stopSelf() - } + // HIGH-3: keep service alive ONLY while playback is genuinely in + // progress. After STATE_ENDED, playWhenReady stays true but state + // is ENDED — old check missed that and held WAKE_LOCK forever. + val p = mediaSession?.player + val keep = p != null && + p.playWhenReady && + p.mediaItemCount > 0 && + p.playbackState != Player.STATE_IDLE && + p.playbackState != Player.STATE_ENDED + if (!keep) stopSelf() } override fun onDestroy() { - mediaSession?.run { - player.release() - release() - mediaSession = null - } + // MED-1: null the field first so a late onGetSession from the + // controller-binding teardown gets null instead of a released session. + val s = mediaSession + mediaSession = null + s?.player?.release() + s?.release() super.onDestroy() } + private fun startForegroundCompat() { + if (foregroundStarted) return + val tap = PendingIntent.getActivity( + this, + 0, + Intent(this, StrawActivity::class.java), + PendingIntent.FLAG_IMMUTABLE, + ) + val notification: Notification = NotificationCompat.Builder(this, NOTIF_CHANNEL_ID) + .setSmallIcon(android.R.drawable.ic_media_play) + .setContentTitle("Straw") + .setContentText("Background audio") + .setContentIntent(tap) + .setOngoing(true) + .setCategory(Notification.CATEGORY_TRANSPORT) + .setPriority(NotificationCompat.PRIORITY_LOW) + .build() + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + startForeground( + NOTIF_ID, + notification, + ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PLAYBACK, + ) + } else { + startForeground(NOTIF_ID, notification) + } + foregroundStarted = true + } + + private fun ensureChannel() { + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) return + val nm = getSystemService(NotificationManager::class.java) ?: return + if (nm.getNotificationChannel(NOTIF_CHANNEL_ID) != null) return + val ch = NotificationChannel( + NOTIF_CHANNEL_ID, + "Background audio", + NotificationManager.IMPORTANCE_LOW, + ).apply { + description = "Straw audio playback while the app is in background" + setShowBadge(false) + } + nm.createNotificationChannel(ch) + } + + /** + * HIGH-4 mirror on the service side: the URL in EXTRA_URL came from + * NewPipeExtractor's audioStream.content. Re-validate host + scheme + * before handing it to ExoPlayer's HTTP source. Only YT googlevideo + * hosts allowed; HTTPS only. + */ + private fun isAllowedAudioUrl(url: String): Boolean { + 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" + } + companion object { const val EXTRA_URL = "com.sulkta.straw.extra.URL" const val EXTRA_TITLE = "com.sulkta.straw.extra.TITLE" const val EXTRA_UPLOADER = "com.sulkta.straw.extra.UPLOADER" + + private const val NOTIF_CHANNEL_ID = "straw.playback" + private const val NOTIF_ID = 4242 } } 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 f9fb84830..1478ca857 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 @@ -121,6 +121,31 @@ fun PlayerScreen( } } + // PiP setup: on Android 12+ tell the OS this activity can auto-enter + // PiP, so when the user presses Home or swipes away the video shrinks + // into a floating window instead of pausing/exiting. Aspect ratio is + // set eagerly so the system can sample it before the first transition. + val activity = context as? Activity + DisposableEffect(activity) { + if (activity != null && Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { + val params = PictureInPictureParams.Builder() + .setAspectRatio(Rational(16, 9)) + .setAutoEnterEnabled(true) + .build() + runCatching { activity.setPictureInPictureParams(params) } + } + onDispose { + // Disable auto-enter when leaving the player so the rest of the + // app doesn't accidentally PiP on background. + if (activity != null && Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { + val off = PictureInPictureParams.Builder() + .setAutoEnterEnabled(false) + .build() + runCatching { activity.setPictureInPictureParams(off) } + } + } + } + // AUD-MED: pause playback when app goes to background. Without this, // ExoPlayer keeps playing audio with no MediaSession — user can't pause // from the notification shade. EXCEPTION: don't pause when entering @@ -289,17 +314,43 @@ fun PlayerScreen( } context.startActivity(Intent.createChooser(send, "Share video")) } - // PiP + // PiP — manual entry (auto-enter on home gesture is wired + // up via the DisposableEffect above on Android 12+). OverlayButton(label = "⊟") { - val activity = (context as? Activity) ?: return@OverlayButton - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - val params = PictureInPictureParams.Builder() - .setAspectRatio(Rational(16, 9)) - .build() - runCatching { activity.enterPictureInPictureMode(params) } + val act = (context as? Activity) + if (act == null) { + Toast.makeText(context, "PiP: no activity", Toast.LENGTH_SHORT).show() + return@OverlayButton + } + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) { + Toast.makeText(context, "PiP needs Android 8+", Toast.LENGTH_SHORT).show() + return@OverlayButton + } + val params = PictureInPictureParams.Builder() + .setAspectRatio(Rational(16, 9)) + .build() + val result = runCatching { act.enterPictureInPictureMode(params) } + result.onSuccess { ok -> + if (!ok) { + Toast.makeText( + context, + "PiP refused — check Settings > Apps > Straw > PiP", + Toast.LENGTH_LONG, + ).show() + } + } + result.onFailure { t -> + Toast.makeText( + context, + "PiP failed: ${t.message ?: t.javaClass.simpleName}", + Toast.LENGTH_LONG, + ).show() } } - // Background audio (phase S) — independent foreground-service playback + // Background audio (phase S) — independent foreground-service playback. + // Audit HIGH-1: handing off, not dual-hosting. Stop activity's player + // first so the OS sees a single MediaSession (cleaner lockscreen + + // audio focus) and we don't leak two active ExoPlayers. OverlayButton(label = "🎧") { val r = resolved ?: return@OverlayButton val audio = r.audioUrl ?: r.combinedUrl @@ -307,7 +358,8 @@ fun PlayerScreen( Toast.makeText(context, "no audio stream", Toast.LENGTH_SHORT).show() return@OverlayButton } - exoPlayer.pause() + runCatching { exoPlayer.stop() } + runCatching { exoPlayer.clearMediaItems() } val intent = Intent(context, PlaybackService::class.java).apply { component = ComponentName(context, PlaybackService::class.java) putExtra(PlaybackService.EXTRA_URL, audio) 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 index c6bf0ba22..ee523f634 100644 --- a/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerViewModel.kt +++ b/strawApp/src/main/kotlin/com/sulkta/straw/feature/player/PlayerViewModel.kt @@ -64,14 +64,20 @@ class PlayerViewModel : ViewModel() { fun heightOf(q: String?): Int = q?.removeSuffix("p")?.takeWhile { it.isDigit() }?.toIntOrNull() ?: 0 - val combined = info.videoStreams - ?.filter { it.content?.isNotBlank() == true && heightOf(it.getResolution()) <= maxRes } - ?.maxByOrNull { it.bitrate ?: 0 } - ?.content - val videoOnly = info.videoOnlyStreams - ?.filter { it.content?.isNotBlank() == true && heightOf(it.getResolution()) <= maxRes } - ?.maxByOrNull { it.bitrate ?: 0 } - ?.content + // Audit HIGH-8: when no stream is under the resolution ceiling + // (e.g. user picked 144p but the video only has 360p+), fall + // back to the lowest-resolution available instead of returning + // null and showing a black-screen player. + fun pickVideo(streams: List?): String? { + if (streams.isNullOrEmpty()) return null + val withContent = streams.filter { it.content?.isNotBlank() == true } + val filtered = withContent.filter { heightOf(it.getResolution()) <= maxRes } + val pool = filtered.ifEmpty { withContent } + return pool.maxByOrNull { it.bitrate ?: 0 }?.content + } + + val combined = pickVideo(info.videoStreams) + val videoOnly = pickVideo(info.videoOnlyStreams) val audioOnly = info.audioStreams ?.filter { it.content?.isNotBlank() == true } ?.maxByOrNull { it.bitrate ?: 0 } diff --git a/strawApp/src/main/res/xml/network_security_config.xml b/strawApp/src/main/res/xml/network_security_config.xml index 4ba64bd35..5e3b6a15d 100644 --- a/strawApp/src/main/res/xml/network_security_config.xml +++ b/strawApp/src/main/res/xml/network_security_config.xml @@ -15,7 +15,9 @@ - + + returnyoutubedislike.com returnyoutubedislikeapi.com