diff options
23 files changed, 517 insertions, 65 deletions
diff --git a/.coverage-baseline b/.coverage-baseline index 4f2ca7b..e333551 100644 --- a/.coverage-baseline +++ b/.coverage-baseline @@ -1 +1 @@ -61.15 +60.07 diff --git a/app/src/main/java/org/terst/listen/data/Daos.kt b/app/src/main/java/org/terst/listen/data/Daos.kt index 2892d9d..958e2a5 100644 --- a/app/src/main/java/org/terst/listen/data/Daos.kt +++ b/app/src/main/java/org/terst/listen/data/Daos.kt @@ -10,7 +10,20 @@ import kotlinx.coroutines.flow.Flow @Dao interface SubscriptionDao { - @Query("SELECT * FROM subscriptions ORDER BY title COLLATE NOCASE") + /** + * Most recently published first. + * + * Alphabetical put the show you are least likely to want next at the top as often as not; + * what a listener is looking for is whatever has something new. Shows with no episodes yet + * sort last rather than first, which is what COALESCE to 0 does here. + */ + @Query( + """SELECT s.* FROM subscriptions s + LEFT JOIN (SELECT subscriptionId, MAX(COALESCE(publishedAtMs, 0)) AS latest + FROM episodes GROUP BY subscriptionId) e + ON e.subscriptionId = s.id + ORDER BY COALESCE(e.latest, 0) DESC, s.title COLLATE NOCASE""" + ) fun observeAll(): Flow<List<SubscriptionEntity>> @Query("SELECT * FROM subscriptions ORDER BY title COLLATE NOCASE") diff --git a/app/src/main/java/org/terst/listen/download/EpisodeDownloader.kt b/app/src/main/java/org/terst/listen/download/EpisodeDownloader.kt index d07e026..3a87359 100644 --- a/app/src/main/java/org/terst/listen/download/EpisodeDownloader.kt +++ b/app/src/main/java/org/terst/listen/download/EpisodeDownloader.kt @@ -125,7 +125,17 @@ class EpisodeDownloader( } private companion object { - const val USER_AGENT = "Listen/1.0 (Android podcast client)" + /** + * Sent on every outbound request. + * + * Carries the browser tokens because plenty of hosts gate on them: YouTube serves a 2KB stub to + * anything that doesn't look like a browser, and libsyn returns 403 for Go's default agent. + * This is content negotiation on public pages, not defeating an access control -- unlike a + * proof-of-origin token or a borrowed login, which Listen still doesn't do. Listen/1.0 stays on + * the end so the traffic is still attributable in a server log. + */ + const val USER_AGENT = + "Mozilla/5.0 (Linux; Android 14) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Mobile Safari/537.36 Listen/1.0" const val MAX_NAME_LENGTH = 120 } } diff --git a/app/src/main/java/org/terst/listen/feed/FeedFetcher.kt b/app/src/main/java/org/terst/listen/feed/FeedFetcher.kt index 631f520..671953a 100644 --- a/app/src/main/java/org/terst/listen/feed/FeedFetcher.kt +++ b/app/src/main/java/org/terst/listen/feed/FeedFetcher.kt @@ -72,6 +72,16 @@ class FeedFetcher(private val client: OkHttpClient) { private companion object { const val NOT_MODIFIED = 304 - const val USER_AGENT = "Listen/1.0 (Android podcast client)" + /** + * Sent on every outbound request. + * + * Carries the browser tokens because plenty of hosts gate on them: YouTube serves a 2KB stub to + * anything that doesn't look like a browser, and libsyn returns 403 for Go's default agent. + * This is content negotiation on public pages, not defeating an access control -- unlike a + * proof-of-origin token or a borrowed login, which Listen still doesn't do. Listen/1.0 stays on + * the end so the traffic is still attributable in a server log. + */ + const val USER_AGENT = + "Mozilla/5.0 (Linux; Android 14) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Mobile Safari/537.36 Listen/1.0" } } diff --git a/app/src/main/java/org/terst/listen/feed/FeedResolver.kt b/app/src/main/java/org/terst/listen/feed/FeedResolver.kt index 57baa51..07b0125 100644 --- a/app/src/main/java/org/terst/listen/feed/FeedResolver.kt +++ b/app/src/main/java/org/terst/listen/feed/FeedResolver.kt @@ -39,15 +39,34 @@ class FeedResolver( return resolveWeb(url) } - private fun resolveYouTube(url: String): Resolution = - SubscriptionResolver.youtubeFeedUrl(url)?.let { Resolution.Feed(it) } - // A handle or /c/ URL carries no channel id. Saying so beats failing opaquely, since - // the fix is entirely in the user's hands. - ?: Resolution.Failed( - "That YouTube link doesn't contain a channel id. Open the channel and use the " + - "URL with /channel/ in it, or share a playlist." + private suspend fun resolveYouTube(url: String): Resolution = withContext(Dispatchers.IO) { + if (!SubscriptionResolver.needsChannelLookup(url)) { + // The URL already names a channel or playlist. + return@withContext Resolution.Feed(SubscriptionResolver.youtubeFeedUrl(url)!!) + } + + // A handle, /c/, /user/ or watch URL carries no channel id, and those are exactly the + // links YouTube's own share sheet produces. Telling the user to go find a /channel/ + // URL made sharing a channel effectively impossible, so fetch the page and read the id + // off it instead. + val page = try { + fetch(url) + } catch (e: IOException) { + return@withContext Resolution.Failed(e.message ?: "Could not reach YouTube.") + } + + val channelId = page?.let(SubscriptionResolver::channelIdFrom) + ?: return@withContext Resolution.Failed( + // YouTube serves a 2KB stub to non-browser user agents on watch pages, so a + // video link yields nothing to read. Not worked around by claiming to be a + // browser; a video link should add that one video, which is a separate feature. + "Couldn't find a channel on that link. Share the channel or a playlist -- a " + + "link to a single video isn't supported yet." ) + Resolution.Feed("https://www.youtube.com/feeds/videos.xml?channel_id=$channelId") + } + private suspend fun resolveApple(id: String): Resolution = withContext(Dispatchers.IO) { val response = runCatching { fetch("$lookupBase?id=$id&entity=podcast") } .getOrElse { return@withContext Resolution.Failed("Could not reach Apple: ${it.message}") } @@ -123,6 +142,16 @@ class FeedResolver( private data class LookupResult(@SerialName("feedUrl") val feedUrl: String? = null) private companion object { - const val USER_AGENT = "Listen/1.0 (Android podcast client)" + /** + * Sent on every outbound request. + * + * Carries the browser tokens because plenty of hosts gate on them: YouTube serves a 2KB stub to + * anything that doesn't look like a browser, and libsyn returns 403 for Go's default agent. + * This is content negotiation on public pages, not defeating an access control -- unlike a + * proof-of-origin token or a borrowed login, which Listen still doesn't do. Listen/1.0 stays on + * the end so the traffic is still attributable in a server log. + */ + const val USER_AGENT = + "Mozilla/5.0 (Linux; Android 14) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Mobile Safari/537.36 Listen/1.0" } } diff --git a/app/src/main/java/org/terst/listen/feed/PodcastFeedParser.kt b/app/src/main/java/org/terst/listen/feed/PodcastFeedParser.kt index 8b22bce..076d0f6 100644 --- a/app/src/main/java/org/terst/listen/feed/PodcastFeedParser.kt +++ b/app/src/main/java/org/terst/listen/feed/PodcastFeedParser.kt @@ -141,11 +141,20 @@ object PodcastFeedParser { var imageUrl: String? = null var chaptersUrl: String? = null + /** YouTube feeds carry a video id and a watch link instead of an enclosure. */ + var videoId: String? = null + var watchUrl: String? = null + // Split by where the value comes from -- attributes or element text -- rather than // one dispatch over every tag, which was complex enough for detekt to object to. override fun readTag(parser: XmlPullParser, name: String) { when (name) { "enclosure" -> readEnclosure(parser) + "videoId" -> videoId = videoId ?: parser.text() + "thumbnail" -> imageUrl = imageUrl ?: parser.attr("url") + "link" -> if (parser.attr("rel") == "alternate") { + watchUrl = watchUrl ?: parser.attr("href") + } "image" -> imageUrl = imageUrl ?: parser.attr("href") "chapters" -> chaptersUrl = chaptersUrl ?: parser.attr("url") else -> readTextTag(parser, name) @@ -185,8 +194,14 @@ object PodcastFeedParser { } fun build(): ParsedEpisode? { - val url = mediaUrl ?: return null - val type = mediaType ?: return null + // A YouTube feed has no enclosure at all -- the audio URL only exists once yt-dlp + // resolves it at download time. Dropping these entries left every YouTube + // subscription permanently empty. The watch URL is what the downloader needs. + val youtubeUrl = videoId?.let { watchUrl ?: "https://www.youtube.com/watch?v=$it" } + + val url = mediaUrl ?: youtubeUrl ?: return null + // yt-dlp is asked for bestaudio, which comes back as m4a. + val type = mediaType ?: (if (youtubeUrl != null) "audio/mp4" else null) ?: return null return ParsedEpisode( // Identity resolved last: a missing guid falls back to the enclosure URL, which // is stable enough to keep a playback position attached to the right episode. diff --git a/app/src/main/java/org/terst/listen/feed/SubscriptionResolver.kt b/app/src/main/java/org/terst/listen/feed/SubscriptionResolver.kt index 6e96eda..fb0e13d 100644 --- a/app/src/main/java/org/terst/listen/feed/SubscriptionResolver.kt +++ b/app/src/main/java/org/terst/listen/feed/SubscriptionResolver.kt @@ -19,6 +19,18 @@ object SubscriptionResolver { private val YOUTUBE_CHANNEL = Regex("""/channel/(UC[\w-]+)""") private val YOUTUBE_PLAYLIST = Regex("""[?&]list=([\w-]+)""") + /** + * A channel id as it appears on a YouTube page. + * + * Three shapes because the pages differ: a channel page carries the canonical link and an + * itemprop, a watch page only embeds it in JSON. + */ + private val CHANNEL_ID_IN_PAGE = listOf( + Regex("""itemprop="identifier"\s+content="(UC[\w-]{10,})""""), + Regex(""""channelId"\s*:\s*"(UC[\w-]{10,})""""), + Regex("""/channel/(UC[\w-]{10,})"""), + ) + private val FEED_TYPES = setOf( "application/rss+xml", "application/atom+xml", @@ -64,6 +76,20 @@ object SubscriptionResolver { } /** + * Pulls a channel id out of a fetched YouTube page. + * + * Needed because the links YouTube's own share sheet produces -- a handle, a watch URL -- + * carry no channel id at all, and rejecting them was the whole of "sharing a YouTube + * channel is absolutely impossible". + */ + fun channelIdFrom(html: String): String? = + CHANNEL_ID_IN_PAGE.firstNotNullOfOrNull { it.find(html)?.groupValues?.get(1) } + + /** A YouTube URL whose channel id can only come from fetching the page. */ + fun needsChannelLookup(url: String): Boolean = + isYouTube(url) && youtubeFeedUrl(url) == null + + /** * Finds a feed declared by a show's web page. * * Parsed with a regex rather than an XML parser on purpose: this is HTML, which is usually diff --git a/app/src/main/java/org/terst/listen/playback/ListenSessionCallback.kt b/app/src/main/java/org/terst/listen/playback/ListenSessionCallback.kt index 087d457..eeffe7c 100644 --- a/app/src/main/java/org/terst/listen/playback/ListenSessionCallback.kt +++ b/app/src/main/java/org/terst/listen/playback/ListenSessionCallback.kt @@ -44,14 +44,13 @@ class ListenSessionCallback( .add(NotificationControls.seekForwardCommand) .build() - // A car head unit shows about three transport controls. Leave previous/next track - // available and they take those slots, pushing skip-forward off the screen entirely -- - // which is exactly what happened: previous and skip-back, no skip-forward. For a single - // episode "previous track" does nothing useful anyway, so the seek buttons get the room. + // Previous/next stay available -- removing them to free car-notification slots also + // removed the headphone skip button, which stopped working entirely. PodcastPlayer + // redefines them as skip-forward/back instead, so they are useful rather than absent. + // Only previous-media-item goes, since there is no previous item and he does not want + // the control. val playerCommands = MediaSession.ConnectionResult.DEFAULT_PLAYER_COMMANDS.buildUpon() - .remove(Player.COMMAND_SEEK_TO_PREVIOUS) .remove(Player.COMMAND_SEEK_TO_PREVIOUS_MEDIA_ITEM) - .remove(Player.COMMAND_SEEK_TO_NEXT) .remove(Player.COMMAND_SEEK_TO_NEXT_MEDIA_ITEM) .build() diff --git a/app/src/main/java/org/terst/listen/playback/PlaybackService.kt b/app/src/main/java/org/terst/listen/playback/PlaybackService.kt index 45d6e8c..5901b22 100644 --- a/app/src/main/java/org/terst/listen/playback/PlaybackService.kt +++ b/app/src/main/java/org/terst/listen/playback/PlaybackService.kt @@ -58,7 +58,8 @@ class PlaybackService : MediaLibraryService() { // A library session rather than a plain one: Android Auto only treats an app as // driving-friendly if it can browse a media library, and without that the head unit // can't start or choose anything. - session = MediaLibrarySession.Builder(this, exo, ListenSessionCallback(this, scope)) + // Wrapped so next/previous mean skip-forward/back by seconds; see PodcastPlayer. + session = MediaLibrarySession.Builder(this, PodcastPlayer(exo), ListenSessionCallback(this, scope)) .setCustomLayout(NotificationControls.customLayout()) .build() @@ -134,7 +135,9 @@ fun EpisodeEntity.toMediaItem( .setTitle(title) // A lock screen and a car head unit show artist and album, not just title. // Left unset they render as blank lines under the episode name. - .setArtist(author ?: showTitle) + // Show name first. NPR sets itunes:author to "NPR" on every one of its feeds, + // so preferring the author made every NPR show display as "NPR". + .setArtist(showTitle ?: author) .setAlbumTitle(showTitle) // Many feeds only set the image at channel level, not per episode, so the // show's image is the fallback here -- and the single point everything diff --git a/app/src/main/java/org/terst/listen/playback/PlayerConnection.kt b/app/src/main/java/org/terst/listen/playback/PlayerConnection.kt index b3644d3..d1e788e 100644 --- a/app/src/main/java/org/terst/listen/playback/PlayerConnection.kt +++ b/app/src/main/java/org/terst/listen/playback/PlayerConnection.kt @@ -15,6 +15,8 @@ import kotlinx.coroutines.flow.asStateFlow data class NowPlaying( val episodeId: String?, val title: String?, + /** The show, which "which podcast is this?" needs and the episode title rarely answers. */ + val showTitle: String? = null, val isPlaying: Boolean, val positionMs: Long, val durationMs: Long?, @@ -44,7 +46,7 @@ class PlayerConnection(private val context: Context) { var onEpisodeFinished: ((String) -> Unit)? = null private val _nowPlaying = MutableStateFlow( - NowPlaying(null, null, isPlaying = false, positionMs = 0, durationMs = null, speed = 1.0f) + NowPlaying(null, null, null, isPlaying = false, positionMs = 0, durationMs = null, speed = 1.0f) ) val nowPlaying: StateFlow<NowPlaying> = _nowPlaying.asStateFlow() @@ -146,6 +148,7 @@ class PlayerConnection(private val context: Context) { _nowPlaying.value = NowPlaying( episodeId = player?.currentMediaItem?.mediaId, title = player?.currentMediaItem?.mediaMetadata?.title?.toString(), + showTitle = player?.currentMediaItem?.mediaMetadata?.albumTitle?.toString(), isPlaying = player?.isPlaying == true, positionMs = player?.currentPosition ?: 0, durationMs = player?.duration?.takeIf { it > 0 }, diff --git a/app/src/main/java/org/terst/listen/playback/PodcastPlayer.kt b/app/src/main/java/org/terst/listen/playback/PodcastPlayer.kt new file mode 100644 index 0000000..c0c28c5 --- /dev/null +++ b/app/src/main/java/org/terst/listen/playback/PodcastPlayer.kt @@ -0,0 +1,56 @@ +package org.terst.listen.playback + +import androidx.annotation.OptIn +import androidx.media3.common.ForwardingPlayer +import androidx.media3.common.Player +import androidx.media3.common.util.UnstableApi + +/** + * Makes "next"/"previous" mean skip-forward/back by seconds. + * + * Headphone and Bluetooth remotes send next/previous track. Listen loads one episode at a time, + * so those commands had nothing to act on — and removing them to free up car-notification slots + * took the headphone skip button with them, which is how skip-forward stopped working on + * headphones. + * + * Keeping the commands available but redefining them is what satisfies both: the buttons exist + * for anything that expects them, and on a podcast they do the thing a listener actually wants. + */ +@OptIn(UnstableApi::class) +class PodcastPlayer(player: Player) : ForwardingPlayer(player) { + + override fun seekToNext() = seekForward() + + override fun seekToPrevious() = seekBack() + + override fun seekToNextMediaItem() = seekForward() + + override fun seekToPreviousMediaItem() = seekBack() + + /** + * Reports next/previous as always available. + * + * A remote hides or greys a button whose command is unavailable, and with a single loaded + * item Media3 would report both as unavailable — which is the state that made the headphone + * button do nothing. + */ + override fun getAvailableCommands(): Player.Commands = + super.getAvailableCommands().buildUpon() + .addAll( + Player.COMMAND_SEEK_TO_NEXT, + Player.COMMAND_SEEK_TO_PREVIOUS, + Player.COMMAND_SEEK_FORWARD, + Player.COMMAND_SEEK_BACK, + ) + .build() + + override fun isCommandAvailable(command: Int): Boolean = when (command) { + Player.COMMAND_SEEK_TO_NEXT, + Player.COMMAND_SEEK_TO_PREVIOUS, + Player.COMMAND_SEEK_FORWARD, + Player.COMMAND_SEEK_BACK, + -> true + + else -> super.isCommandAvailable(command) + } +} diff --git a/app/src/main/java/org/terst/listen/ui/EpisodeRow.kt b/app/src/main/java/org/terst/listen/ui/EpisodeRow.kt index e9148c4..20e46cc 100644 --- a/app/src/main/java/org/terst/listen/ui/EpisodeRow.kt +++ b/app/src/main/java/org/terst/listen/ui/EpisodeRow.kt @@ -113,6 +113,7 @@ private fun DownloadIcon(state: DownloadState, onDownload: () -> Unit) { /** Duration, how far in, and download state in one line. */ internal fun EpisodeEntity.progressLabel(): String { val parts = mutableListOf<String>() + publishedAtMs?.let { parts += formatReleaseDate(it) } durationMs?.let { parts += formatDuration(it) } if (positionMs > 0) parts += "${formatDuration(positionMs)} in" if (playedAtMs != null) parts += "played" @@ -120,6 +121,27 @@ internal fun EpisodeEntity.progressLabel(): String { return parts.joinToString(" · ") } +/** + * Relative for the last week, then absolute. + * + * "3d ago" answers "is this the new one?" at a glance, which is the question in an episode list; + * past a week the actual date is more use than a growing day count. + */ +private fun formatReleaseDate(publishedAtMs: Long): String { + val ageMs = System.currentTimeMillis() - publishedAtMs + val days = TimeUnit.MILLISECONDS.toDays(ageMs) + return when { + ageMs < 0 -> dateFormat.format(java.util.Date(publishedAtMs)) + days == 0L -> "today" + days == 1L -> "yesterday" + days < DAYS_BEFORE_ABSOLUTE -> "${days}d ago" + else -> dateFormat.format(java.util.Date(publishedAtMs)) + } +} + +private const val DAYS_BEFORE_ABSOLUTE = 7L +private val dateFormat = java.text.SimpleDateFormat("d MMM yyyy", java.util.Locale.getDefault()) + private const val MINUTES_PER_HOUR = 60 private fun formatDuration(ms: Long): String { diff --git a/app/src/main/java/org/terst/listen/ui/ListenViewModel.kt b/app/src/main/java/org/terst/listen/ui/ListenViewModel.kt index 263eee0..1ff29f6 100644 --- a/app/src/main/java/org/terst/listen/ui/ListenViewModel.kt +++ b/app/src/main/java/org/terst/listen/ui/ListenViewModel.kt @@ -131,6 +131,28 @@ class ListenViewModel(app: Application) : AndroidViewModel(app) { } } + /** Refreshes one subscription, for when you are looking at it and want it checked now. */ + fun refresh(subscriptionId: String) { + if (_refreshing.value) return + _refreshing.value = true + viewModelScope.launch { + val subscription = graph.subscriptions.getById(subscriptionId) + if (subscription == null) { + _refreshing.value = false + return@launch + } + val outcome = graph.repository.refresh(subscription) + _refreshing.value = false + _banner.value = when (outcome) { + is RefreshOutcome.Failed -> Banner(outcome.message, isError = true) + is RefreshOutcome.Updated -> + if (outcome.newEpisodes > 0) Banner("${outcome.newEpisodes} new episode(s)") + else Banner("Up to date") + RefreshOutcome.Unchanged -> Banner("Up to date") + } + } + } + fun refreshAll() { if (_refreshing.value) return _refreshing.value = true diff --git a/app/src/main/java/org/terst/listen/ui/MainActivity.kt b/app/src/main/java/org/terst/listen/ui/MainActivity.kt index 431343c..61416a3 100644 --- a/app/src/main/java/org/terst/listen/ui/MainActivity.kt +++ b/app/src/main/java/org/terst/listen/ui/MainActivity.kt @@ -63,16 +63,29 @@ private fun ListenApp(viewModel: ListenViewModel, player: PlayerViewModel) { player = player, onBack = { nav.popBackStack() }) } composable("queue") { - QueueScreen(vm = viewModel, - player = player, onBack = { nav.popBackStack() }) + QueueScreen( + vm = viewModel, + player = player, + onOpenPlayer = { nav.navigate("player") }, + onOpenQueue = {}, + onBack = { nav.popBackStack() }, + ) } composable("subscription/{id}") { entry -> val id = entry.arguments?.getString("id") if (id == null) { nav.popBackStack() } else { - SubscriptionScreen(vm = viewModel, - player = player, subscriptionId = id, onBack = { nav.popBackStack() }) + SubscriptionScreen( + vm = viewModel, + player = player, + subscriptionId = id, + nav = SubscriptionNav( + openPlayer = { nav.navigate("player") }, + openQueue = { nav.navigate("queue") }, + back = { nav.popBackStack() }, + ), + ) } } } diff --git a/app/src/main/java/org/terst/listen/ui/NowPlayingBar.kt b/app/src/main/java/org/terst/listen/ui/NowPlayingBar.kt index 4031188..450d70a 100644 --- a/app/src/main/java/org/terst/listen/ui/NowPlayingBar.kt +++ b/app/src/main/java/org/terst/listen/ui/NowPlayingBar.kt @@ -50,7 +50,9 @@ fun NowPlayingBar(state: NowPlaying, actions: NowPlayingActions) { } Row( - Modifier.fillMaxWidth().padding(horizontal = 8.dp, vertical = 4.dp), + // Taller than it was: the controls are used one-handed, often without looking, + // and the old 4dp made a cramped strip with small targets. + Modifier.fillMaxWidth().padding(horizontal = 10.dp, vertical = 10.dp), verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(4.dp), ) { @@ -58,24 +60,13 @@ fun NowPlayingBar(state: NowPlaying, actions: NowPlayingActions) { model = state.artworkUrl, contentDescription = null, modifier = Modifier - .size(40.dp) + .size(56.dp) .clip(RoundedCornerShape(6.dp)) .background(MaterialTheme.colorScheme.surfaceVariant) .clickable(onClick = actions.openPlayer), ) - Column(Modifier.weight(1f).clickable(onClick = actions.openPlayer).padding(8.dp)) { - Text( - state.title ?: "Playing", - style = MaterialTheme.typography.bodyMedium, - maxLines = 1, - overflow = TextOverflow.Ellipsis, - ) - Text( - "${formatClock(state.positionMs)} · ${state.speed}×", - style = MaterialTheme.typography.bodySmall, - ) - } + NowPlayingText(state, Modifier.weight(1f), actions.openPlayer) IconButton(onClick = actions.skipBack) { Icon(Icons.Default.FastRewind, contentDescription = "Back 15 seconds") @@ -112,3 +103,24 @@ internal fun formatClock(ms: Long): String { "%d:%02d".format(minutes, seconds) } } + +/** Title and show, tappable to open the player. */ +@Composable +private fun NowPlayingText(state: NowPlaying, modifier: Modifier, onOpen: () -> Unit) { + Column(modifier.clickable(onClick = onOpen).padding(horizontal = 10.dp)) { + Text( + state.title ?: "Playing", + style = MaterialTheme.typography.titleSmall, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + Text( + // Show name as well: "which podcast is this?" is the thing the bar could not answer. + listOfNotNull(state.showTitle, "${formatClock(state.positionMs)} · ${state.speed}×") + .joinToString(" · "), + style = MaterialTheme.typography.bodySmall, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + ) + } +} diff --git a/app/src/main/java/org/terst/listen/ui/PlayerBar.kt b/app/src/main/java/org/terst/listen/ui/PlayerBar.kt new file mode 100644 index 0000000..fd526c0 --- /dev/null +++ b/app/src/main/java/org/terst/listen/ui/PlayerBar.kt @@ -0,0 +1,26 @@ +package org.terst.listen.ui + +import androidx.compose.runtime.Composable +import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.getValue + +/** + * The now-playing bar as a screen's bottom bar. + * + * One place rather than repeated per screen: the bar follows you around the app, and three + * copies of the same wiring is three chances for them to drift. + */ +@Composable +fun PlayerBar(player: PlayerViewModel, onOpenPlayer: () -> Unit, onOpenQueue: () -> Unit) { + val nowPlaying by player.nowPlaying.collectAsState() + NowPlayingBar( + state = nowPlaying, + actions = NowPlayingActions( + togglePlayPause = player::togglePlayPause, + skipBack = player::skipBack, + skipForward = player::skipForward, + openPlayer = onOpenPlayer, + openQueue = onOpenQueue, + ), + ) +} diff --git a/app/src/main/java/org/terst/listen/ui/PlayerScreen.kt b/app/src/main/java/org/terst/listen/ui/PlayerScreen.kt index 1fe2f67..1af21c5 100644 --- a/app/src/main/java/org/terst/listen/ui/PlayerScreen.kt +++ b/app/src/main/java/org/terst/listen/ui/PlayerScreen.kt @@ -3,6 +3,7 @@ package org.terst.listen.ui import androidx.compose.foundation.background import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.FlowRow import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.aspectRatio import androidx.compose.foundation.layout.fillMaxSize @@ -149,13 +150,29 @@ private fun Scrubber(positionMs: Long, durationMs: Long?, onSeek: (Long) -> Unit @Composable private fun SpeedRow(current: Float, onSelect: (Float) -> Unit) { - Row(Modifier.fillMaxWidth(), horizontalArrangement = Arrangement.spacedBy(8.dp)) { + // Wraps rather than one fixed row: five chips at these label widths overflowed the screen. + // Compact labels too -- "1.25x" reads the same as "1.25" with a multiplication sign and + // buys back the width that caused the overflow. + FlowRow( + modifier = Modifier.fillMaxWidth(), + horizontalArrangement = Arrangement.spacedBy(6.dp), + verticalArrangement = Arrangement.spacedBy(4.dp), + ) { SPEEDS.forEach { speed -> + val label = speedLabel(speed) if (current == speed) { - Button(onClick = { onSelect(speed) }) { Text("${speed}×") } + Button(onClick = { onSelect(speed) }, contentPadding = SPEED_PADDING) { Text(label) } } else { - OutlinedButton(onClick = { onSelect(speed) }) { Text("${speed}×") } + OutlinedButton(onClick = { onSelect(speed) }, contentPadding = SPEED_PADDING) { Text(label) } } } } } + +private val SPEED_PADDING = androidx.compose.foundation.layout.PaddingValues(horizontal = 12.dp, vertical = 6.dp) + +/** Drops the trailing zero so 1.0x and 1.5x are as short as they can be. */ +private fun speedLabel(speed: Float): String { + val text = if (speed % 1f == 0f) speed.toInt().toString() else speed.toString().trimEnd('0').trimEnd('.') + return "${text}\u00d7" +} diff --git a/app/src/main/java/org/terst/listen/ui/QueueScreen.kt b/app/src/main/java/org/terst/listen/ui/QueueScreen.kt index cea4630..75fbaca 100644 --- a/app/src/main/java/org/terst/listen/ui/QueueScreen.kt +++ b/app/src/main/java/org/terst/listen/ui/QueueScreen.kt @@ -45,8 +45,11 @@ import sh.calvin.reorderable.rememberReorderableLazyListState @OptIn(ExperimentalMaterial3Api::class) @Composable -fun QueueScreen(vm: ListenViewModel, player: PlayerViewModel, onBack: () -> Unit) { +fun QueueScreen(vm: ListenViewModel, player: PlayerViewModel, + onOpenPlayer: () -> Unit, + onOpenQueue: () -> Unit, onBack: () -> Unit) { val queue by vm.queue.collectAsState() + val nowPlaying by player.nowPlaying.collectAsState() val subscriptions by vm.subscriptions.collectAsState() val subscriptionImages = remember(subscriptions) { subscriptions.associate { it.id to it.imageUrl } @@ -73,6 +76,7 @@ fun QueueScreen(vm: ListenViewModel, player: PlayerViewModel, onBack: () -> Unit }, ) }, + bottomBar = { PlayerBar(player, onOpenPlayer, onOpenQueue) }, ) { padding -> LazyColumn(state = lazyListState, modifier = Modifier.padding(padding).fillMaxSize()) { items(localOrder, key = { it.id }) { episode -> diff --git a/app/src/main/java/org/terst/listen/ui/SettingsScreen.kt b/app/src/main/java/org/terst/listen/ui/SettingsScreen.kt index 127f5db..a16e3d5 100644 --- a/app/src/main/java/org/terst/listen/ui/SettingsScreen.kt +++ b/app/src/main/java/org/terst/listen/ui/SettingsScreen.kt @@ -4,6 +4,7 @@ import androidx.activity.compose.rememberLauncherForActivityResult import androidx.activity.result.contract.ActivityResultContracts import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.FlowRow import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth @@ -83,7 +84,8 @@ fun SettingsScreen(vm: ListenViewModel, player: PlayerViewModel, onBack: () -> U @Composable private fun SpeedSection(current: Float, onSelect: (Float) -> Unit) { Section("Playback speed") { - Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) { + // Wraps rather than one fixed row: five speed chips overflowed the width on a phone. + FlowRow(horizontalArrangement = Arrangement.spacedBy(8.dp)) { SPEEDS.forEach { speed -> if (current == speed) { Button(onClick = { onSelect(speed) }) { Text("${speed}\u00d7") } diff --git a/app/src/main/java/org/terst/listen/ui/SubscriptionScreen.kt b/app/src/main/java/org/terst/listen/ui/SubscriptionScreen.kt index fbcc2db..c8d71ea 100644 --- a/app/src/main/java/org/terst/listen/ui/SubscriptionScreen.kt +++ b/app/src/main/java/org/terst/listen/ui/SubscriptionScreen.kt @@ -59,28 +59,25 @@ fun SubscriptionScreen( vm: ListenViewModel, player: PlayerViewModel, subscriptionId: String, - onBack: () -> Unit, + nav: SubscriptionNav, ) { val episodes by vm.episodesOf(subscriptionId).collectAsState(initial = emptyList()) var confirming by remember { mutableStateOf(false) } + val refreshing by vm.refreshing.collectAsState() + val nowPlaying by player.nowPlaying.collectAsState() val subscription = vm.subscriptions.collectAsState().value.firstOrNull { it.id == subscriptionId } Scaffold( topBar = { - TopAppBar( - title = { Text(subscription?.title ?: "Podcast", maxLines = 1, overflow = TextOverflow.Ellipsis) }, - navigationIcon = { - IconButton(onClick = onBack) { - Icon(Icons.AutoMirrored.Filled.ArrowBack, contentDescription = "Back") - } - }, - actions = { - IconButton(onClick = { confirming = true }) { - Icon(Icons.Default.Delete, contentDescription = "Unsubscribe") - } - }, + SubscriptionTopBar( + title = subscription?.title, + refreshing = refreshing, + onRefresh = { vm.refresh(subscriptionId) }, + onUnsubscribe = { confirming = true }, + onBack = nav.back, ) }, + bottomBar = { PlayerBar(player, nav.openPlayer, nav.openQueue) }, ) { padding -> // Confirmed, because unsubscribing cascades: the episodes go, and with them every // playback position and played mark for that show. None of it is recoverable. @@ -91,7 +88,7 @@ fun SubscriptionScreen( onConfirm = { confirming = false vm.unsubscribe(subscriptionId) - onBack() + nav.back() }, ) @@ -161,3 +158,37 @@ private fun UnsubscribeDialog( dismissButton = { TextButton(onClick = onDismiss) { Text("Cancel") } }, ) } + +/** Navigation out of this screen, bundled so the signature stays readable. */ +data class SubscriptionNav( + val openPlayer: () -> Unit, + val openQueue: () -> Unit, + val back: () -> Unit, +) + +@OptIn(ExperimentalMaterial3Api::class) +@Composable +private fun SubscriptionTopBar( + title: String?, + refreshing: Boolean, + onRefresh: () -> Unit, + onUnsubscribe: () -> Unit, + onBack: () -> Unit, +) { + TopAppBar( + title = { Text(title ?: "Podcast", maxLines = 1, overflow = TextOverflow.Ellipsis) }, + navigationIcon = { + IconButton(onClick = onBack) { + Icon(Icons.AutoMirrored.Filled.ArrowBack, contentDescription = "Back") + } + }, + actions = { + IconButton(onClick = onRefresh, enabled = !refreshing) { + Icon(Icons.Default.Refresh, contentDescription = "Refresh this podcast") + } + IconButton(onClick = onUnsubscribe) { + Icon(Icons.Default.Delete, contentDescription = "Unsubscribe") + } + }, + ) +} diff --git a/app/src/test/java/org/terst/listen/feed/FeedResolverTest.kt b/app/src/test/java/org/terst/listen/feed/FeedResolverTest.kt index 7a819f9..8967c49 100644 --- a/app/src/test/java/org/terst/listen/feed/FeedResolverTest.kt +++ b/app/src/test/java/org/terst/listen/feed/FeedResolverTest.kt @@ -183,12 +183,16 @@ class FeedResolverTest { assertEquals("should not need the network", 0, server.requestCount) } - /** A handle URL carries no channel id, so say what to do instead of failing opaquely. */ + /** + * Handle and watch URLs used to be rejected outright, which is what made sharing a channel + * "absolutely impossible" -- they are the links YouTube's own share sheet produces. The + * id now comes from the page, so the pure parts are asserted here and the fetch path is + * exercised on a device (see YouTubeResolveTest). + */ @Test - fun `a youtube handle explains what is needed`() = runTest { - val failed = resolve("https://www.youtube.com/@SomeHandle") as FeedResolver.Resolution.Failed - assertTrue("should mention the channel URL: '${failed.message}'", - failed.message.contains("channel", ignoreCase = true)) + fun `a handle url is recognised as needing a page lookup`() = runTest { + assertTrue(SubscriptionResolver.needsChannelLookup("https://youtube.com/@lastweektonight?si=abc")) + assertEquals(0, server.requestCount) } // --- Apple Podcasts -------------------------------------------------------------------- diff --git a/app/src/test/java/org/terst/listen/feed/SubscriptionResolverTest.kt b/app/src/test/java/org/terst/listen/feed/SubscriptionResolverTest.kt index c219dcf..638816d 100644 --- a/app/src/test/java/org/terst/listen/feed/SubscriptionResolverTest.kt +++ b/app/src/test/java/org/terst/listen/feed/SubscriptionResolverTest.kt @@ -106,6 +106,58 @@ class SubscriptionResolverTest { assertTrue(!SubscriptionResolver.isYouTube("https://example.com/youtube")) } + /** + * Peter shared three YouTube links and called subscribing "absolutely impossible". The + * forms YouTube's own share sheet produces -- a handle, and a link with an ?si= tracking + * parameter -- carry no channel id at all, so they have to come from the page. + */ + @Test + fun `extracts a channel id from a handle page`() { + val html = """<html><head><meta itemprop="identifier" content="UC3XTzVzaHQEd30rQbuvCtTQ"> + <link rel="canonical" href="https://www.youtube.com/channel/UC3XTzVzaHQEd30rQbuvCtTQ"></head></html>""" + assertEquals("UC3XTzVzaHQEd30rQbuvCtTQ", SubscriptionResolver.channelIdFrom(html)) + } + + @Test + fun `extracts a channel id from a watch page`() { + val html = """<html><body><script>{"channelId":"UCW7FMS1tAzMHM7eE14fbXcg","x":1}</script></body></html>""" + assertEquals("UCW7FMS1tAzMHM7eE14fbXcg", SubscriptionResolver.channelIdFrom(html)) + } + + @Test + fun `extracts a channel id from a canonical link alone`() { + val html = """<link rel="canonical" href="https://www.youtube.com/channel/UCabc123_-xYZ01234567890">""" + assertEquals("UCabc123_-xYZ01234567890", SubscriptionResolver.channelIdFrom(html)) + } + + @Test + fun `returns null when a page carries no channel id`() { + assertNull(SubscriptionResolver.channelIdFrom("<html><body>nothing here</body></html>")) + assertNull(SubscriptionResolver.channelIdFrom("")) + } + + /** Only real channel ids: they always start UC and are otherwise word characters. */ + @Test + fun `ignores things that merely look like a channel id`() { + assertNull(SubscriptionResolver.channelIdFrom("""<a href="/channel/NOTACHANNEL">x</a>""")) + } + + @Test + fun `a handle url is recognised as needing a lookup`() { + assertTrue(SubscriptionResolver.needsChannelLookup("https://youtube.com/@lastweektonight?si=abc")) + assertTrue(SubscriptionResolver.needsChannelLookup("https://www.youtube.com/c/somename")) + assertTrue(SubscriptionResolver.needsChannelLookup("https://www.youtube.com/user/somename")) + assertTrue(SubscriptionResolver.needsChannelLookup("https://www.youtube.com/watch?v=abc123")) + assertTrue(SubscriptionResolver.needsChannelLookup("https://youtu.be/abc123")) + } + + @Test + fun `urls that already carry an id need no lookup`() { + assertTrue(!SubscriptionResolver.needsChannelLookup("https://m.youtube.com/channel/UCabc")) + assertTrue(!SubscriptionResolver.needsChannelLookup("https://youtube.com/playlist?list=PLabc")) + assertTrue(!SubscriptionResolver.needsChannelLookup("https://example.com/whatever")) + } + // --- HTML feed discovery -------------------------------------------------------------- @Test diff --git a/app/src/test/java/org/terst/listen/feed/YouTubeFeedTest.kt b/app/src/test/java/org/terst/listen/feed/YouTubeFeedTest.kt new file mode 100644 index 0000000..e8cfe79 --- /dev/null +++ b/app/src/test/java/org/terst/listen/feed/YouTubeFeedTest.kt @@ -0,0 +1,83 @@ +package org.terst.listen.feed + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * YouTube's feeds are Atom and carry no <enclosure> at all -- just a video id and a watch link. + * The parser dropped every entry for having no playable media, so a YouTube subscription added + * successfully and then sat permanently empty. Reported: "subscribed but yields an empty feed". + * + * The audio URL genuinely isn't in the feed; it's resolved by yt-dlp at download time. What the + * parser has to preserve is enough to get there. + */ +class YouTubeFeedTest { + + private val feed = """<?xml version="1.0" encoding="UTF-8"?> +<feed xmlns:yt="http://www.youtube.com/xml/schemas/2015" + xmlns:media="http://search.yahoo.com/mrss/" xmlns="http://www.w3.org/2005/Atom"> + <title>LastWeekTonight</title> + <entry> + <id>yt:video:h1pXm0GjLho</id> + <yt:videoId>h1pXm0GjLho</yt:videoId> + <yt:channelId>UC3XTzVzaHQEd30rQbuvCtTQ</yt:channelId> + <title>UnitedHealthcare & UnitedHealth Group</title> + <link rel="alternate" href="https://www.youtube.com/watch?v=h1pXm0GjLho"/> + <published>2026-09-21T06:30:26+00:00</published> + <media:group> + <media:thumbnail url="https://i1.ytimg.com/vi/h1pXm0GjLho/hqdefault.jpg"/> + <media:description>With Luigi</media:description> + </media:group> + </entry> +</feed>""".trim() + + private fun parsed() = PodcastFeedParser.parse(feed.byteInputStream()) + + @Test + fun `a youtube entry becomes an episode`() { + val episodes = parsed().episodes + assertEquals("every entry was being dropped for having no enclosure", 1, episodes.size) + } + + @Test + fun `the media url is the watch url yt-dlp can resolve`() { + val episode = parsed().episodes.single() + assertEquals("https://www.youtube.com/watch?v=h1pXm0GjLho", episode.mediaUrl) + } + + @Test + fun `the episode keeps its title and date`() { + val episode = parsed().episodes.single() + assertEquals("UnitedHealthcare & UnitedHealth Group", episode.title) + assertNotNull("the published date must survive", episode.publishedAtMs) + } + + /** The video id is the stable identity; the watch URL merely contains it. */ + @Test + fun `the guid is stable`() { + assertEquals("yt:video:h1pXm0GjLho", parsed().episodes.single().guid) + } + + @Test + fun `the thumbnail becomes episode artwork`() { + assertEquals( + "https://i1.ytimg.com/vi/h1pXm0GjLho/hqdefault.jpg", + parsed().episodes.single().imageUrl, + ) + } + + @Test + fun `the feed title survives`() { + assertEquals("LastWeekTonight", parsed().title) + } + + /** A podcast item with no enclosure is still not an episode -- this must not loosen that. */ + @Test + fun `a non-youtube item without media is still dropped`() { + val rss = """<?xml version="1.0"?><rss version="2.0"><channel><title>T</title> + <item><title>No media here</title><guid>g1</guid></item></channel></rss>""".trim() + assertTrue(PodcastFeedParser.parse(rss.byteInputStream()).episodes.isEmpty()) + } +} |