diff options
| author | Claude <noreply@anthropic.com> | 2026-09-24 09:25:29 +0000 |
|---|---|---|
| committer | Claude <noreply@anthropic.com> | 2026-09-24 09:25:29 +0000 |
| commit | 0b45924bfb78557ce207f10ffd3ceccd55df2485 (patch) | |
| tree | 5899d40a7fdd45916d3432cb6fadef72a6885f3d /app/src | |
| parent | 84ec886721f9f6052447a19e84d6c8e8d9f62ae6 (diff) | |
Fix YouTube subscriptions, headphone skip, and NPR show names
YouTube was broken in two separate ways and only the first was visible.
Resolution rejected anything without a channel id in the URL -- which is every
link YouTube's share sheet produces. The id now comes from the page, so
handles, /c/, /user/ and watch URLs all work.
Then the deeper one, from Peter testing the link that did resolve: it
"subscribed but yields an empty feed". YouTube's feeds are Atom and carry no
<enclosure> at all, just <yt:videoId> and a watch link, so the parser dropped
every single entry for having no playable media. Subscriptions were being
created permanently empty. The watch URL is now the media URL, which is
exactly what the yt-dlp path already expects; a podcast item with no
enclosure is still dropped.
Requests now send a browser user agent. I had refused this on the grounds it
was defeating an access control, which was wrong and Peter said so: fetching
a public page is content negotiation, not authentication. It is materially
different from a proof-of-origin token or a borrowed login, neither of which
Listen does. YouTube serves a 2KB stub to anything that doesn't look like a
browser, so this is also what makes video links resolve at all -- verified on
device, all three of his URLs now subscribe with 15, 15 and 13 episodes.
Headphone skip-forward: I broke this removing next/previous commands to free
car-notification slots -- headphone remotes send exactly those. Restored, and
PodcastPlayer redefines them as skip-forward/back, so they do the useful thing
on a podcast instead of nothing.
Every NPR show displayed as "NPR" because artist preferred itunes:author, and
NPR sets that identically on every feed. Show title comes first now.
Also: release dates on the episode list, podcasts sorted by most recent
episode rather than alphabetically, a per-feed refresh button, speed chips
that wrap instead of overflowing, and the now-playing bar taller, carrying the
show name, and present on the queue and podcast screens rather than only the
library.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B
Diffstat (limited to 'app/src')
22 files changed, 516 insertions, 64 deletions
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()) + } +} |
