summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPeter Stone <thepeterstone@gmail.com>2026-09-26 08:37:53 +0000
committerPeter Stone <thepeterstone@gmail.com>2026-09-26 08:37:53 +0000
commit8a4ae0293cc7870fe5c88034067c619071810440 (patch)
tree1f86e145d001b015c061c242ca848fde9a51378b
parent4965d5971c1443ee69af42b9f86920fd6121fea5 (diff)
Fetch yt-dlp sources before playing them, and stop losing errors
Peter reported that YouTube subscribing and individual videos work but nothing plays. Reproduced and root-caused on a device rather than guessed at: UnrecognizedInputFormatException: None of the available extractors (FlvExtractor, FlacExtractor, ... Mp3Extractor, ...) could read the stream A YouTube episode's mediaUrl is its watch page -- the feed carries no enclosure, and the audio URL only exists once yt-dlp has resolved it. SubscriptionKind was consulted in exactly one place, EpisodeDownloader, which routes downloads. So playback handed ExoPlayer an HTML page. SubscriptionKind now says whether a source needs extracting, and PlaybackAdvancer.start reports NeedsFetch instead of a NextUp that cannot work. fetchAndStart runs the extraction and re-resolves, so the two places that start episodes -- the service on queue advance, the view model on a tap -- share one implementation rather than each growing their own. Both had to handle it: the queue moves on without the UI, which was the lesson of the previous commit. The second half of this is that none of it was visible. Nothing in the app observed onPlayerError, and nothing rendered PlayerViewModel.banner at all -- so playback failures, fetch failures and even "sleep timer finished" were set on a flow no screen collected. That is why a broken feature looked like a dead button. Errors now surface in the player bar and on the player screen, and a failed fetch carries yt-dlp's own words: "Sign in to confirm you're not a bot" and "video unavailable" are very different problems and identical without them. A fetch in progress says so rather than reading as "Nothing playing" for thirty seconds. Verified on a device, and one part could not be: The mechanism works end to end. A YOUTUBE_CHANNEL episode pointed at a real audio URL went NONE -> DOWNLOADING -> DOWNLOADED with a file on disk, and then played -- yt-dlp doing the extraction and the download for real. YouTube's own extractor could not be verified here, and I am not claiming it was. This host's IP is bot-challenged for every video tried, directly from the command line as well as from the app -- the same block CLAUDE.md recorded as transient in July. Peter's phone is on a residential connection and is the only place that can confirm the reported bug is actually gone. Correcting something I said earlier in the session: YouTubeAudioTest passing does not prove extraction works. It only starts Python and checks yt-dlp's version; it never extracts anything. No permission changes, no schema change. 392 unit tests green, detekt and lint clean. Coverage 65.33 -> 65.16, exception recorded. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B
-rw-r--r--.coverage-baseline2
-rw-r--r--app/src/main/java/org/terst/listen/data/Entities.kt15
-rw-r--r--app/src/main/java/org/terst/listen/data/Repositories.kt7
-rw-r--r--app/src/main/java/org/terst/listen/playback/PlaybackAdvancer.kt105
-rw-r--r--app/src/main/java/org/terst/listen/playback/PlaybackService.kt37
-rw-r--r--app/src/main/java/org/terst/listen/playback/PlayerConnection.kt14
-rw-r--r--app/src/main/java/org/terst/listen/ui/PlayerBar.kt75
-rw-r--r--app/src/main/java/org/terst/listen/ui/PlayerMessage.kt35
-rw-r--r--app/src/main/java/org/terst/listen/ui/PlayerScreen.kt8
-rw-r--r--app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt48
-rw-r--r--app/src/test/java/org/terst/listen/playback/PlaybackAdvancerTest.kt166
11 files changed, 450 insertions, 62 deletions
diff --git a/.coverage-baseline b/.coverage-baseline
index 5d8929f..e81d199 100644
--- a/.coverage-baseline
+++ b/.coverage-baseline
@@ -1 +1 @@
-65.33
+65.16
diff --git a/app/src/main/java/org/terst/listen/data/Entities.kt b/app/src/main/java/org/terst/listen/data/Entities.kt
index 359a626..2d0298a 100644
--- a/app/src/main/java/org/terst/listen/data/Entities.kt
+++ b/app/src/main/java/org/terst/listen/data/Entities.kt
@@ -7,7 +7,20 @@ import androidx.room.Index
import androidx.room.PrimaryKey
/** Where a subscription's episodes come from, which decides how their media is fetched. */
-enum class SubscriptionKind { PODCAST, YOUTUBE_CHANNEL }
+enum class SubscriptionKind {
+ PODCAST,
+ YOUTUBE_CHANNEL,
+ ;
+
+ /**
+ * Whether an episode of this kind needs yt-dlp before it can be played at all.
+ *
+ * A podcast enclosure is a real audio URL and streams as-is. A YouTube episode's mediaUrl is
+ * its watch page -- the feed carries no enclosure, and the audio URL only exists once yt-dlp
+ * has resolved it. Handing that page to ExoPlayer is what made these episodes fail to play.
+ */
+ val needsExtraction: Boolean get() = this != PODCAST
+}
enum class DownloadState { NONE, QUEUED, DOWNLOADING, DOWNLOADED, FAILED }
diff --git a/app/src/main/java/org/terst/listen/data/Repositories.kt b/app/src/main/java/org/terst/listen/data/Repositories.kt
index bcd132a..2a2d74e 100644
--- a/app/src/main/java/org/terst/listen/data/Repositories.kt
+++ b/app/src/main/java/org/terst/listen/data/Repositories.kt
@@ -10,6 +10,7 @@ import org.terst.listen.feed.ChapterFetcher
import org.terst.listen.feed.FeedFetcher
import org.terst.listen.feed.FeedResolver
import org.terst.listen.playback.PlaybackAdvancer
+import org.terst.listen.playback.PlaybackSources
import org.terst.listen.queue.DripReleaser
import org.terst.listen.queue.QueueOrdering
import org.terst.listen.queue.RetentionPolicy
@@ -63,7 +64,11 @@ class AppGraph(private val context: Context) {
// Shared with PlaybackService, so a user-started episode and one the queue moved on to are
// worked out the same way -- same resume position, same intro offset, same speed.
val advancer: PlaybackAdvancer by lazy {
- PlaybackAdvancer(episodes, subscriptions, queue, chaptersFor = { chapters.chapters(it) })
+ PlaybackAdvancer(episodes, subscriptions, queue, sources = object : PlaybackSources {
+ override suspend fun chapters(episode: EpisodeEntity) = chapters.chapters(episode)
+ override suspend fun fetchAudio(episode: EpisodeEntity, kind: SubscriptionKind) =
+ downloader.download(episode, kind).map { }
+ })
}
val repository: SubscriptionRepository by lazy {
diff --git a/app/src/main/java/org/terst/listen/playback/PlaybackAdvancer.kt b/app/src/main/java/org/terst/listen/playback/PlaybackAdvancer.kt
index 6d529e8..92d2147 100644
--- a/app/src/main/java/org/terst/listen/playback/PlaybackAdvancer.kt
+++ b/app/src/main/java/org/terst/listen/playback/PlaybackAdvancer.kt
@@ -5,6 +5,7 @@ import org.terst.listen.data.EpisodeEntity
import org.terst.listen.data.QueueRepository
import org.terst.listen.data.SubscriptionDao
import org.terst.listen.data.SubscriptionEntity
+import org.terst.listen.data.SubscriptionKind
import org.terst.listen.feed.Chapter
/** What the ticker needs to know about whatever is playing, read once per episode. */
@@ -13,6 +14,50 @@ data class PlayingContext(
val chapters: List<Chapter> = emptyList(),
)
+/**
+ * The parts of starting an episode that reach the network: its chapter file, and yt-dlp.
+ *
+ * One seam rather than two lambdas on the constructor, so a test stands in for both at once and
+ * the class keeps a readable signature.
+ */
+interface PlaybackSources {
+ suspend fun chapters(episode: EpisodeEntity): List<Chapter>
+
+ suspend fun fetchAudio(episode: EpisodeEntity, kind: SubscriptionKind): Result<Unit>
+
+ /** For the cases that never reach the network: no chapters, and nothing to extract with. */
+ object None : PlaybackSources {
+ override suspend fun chapters(episode: EpisodeEntity) = emptyList<Chapter>()
+ override suspend fun fetchAudio(episode: EpisodeEntity, kind: SubscriptionKind) =
+ Result.failure<Unit>(IllegalStateException("no audio source configured"))
+ }
+}
+
+/** Whether an episode can be started, and what is in the way if it cannot. */
+sealed interface StartOutcome {
+ data class Ready(val next: NextUp) : StartOutcome
+
+ /**
+ * yt-dlp has to produce a file before this will play; the caller fetches, then asks again.
+ *
+ * Reported rather than handled here so the decision stays testable off-device and the
+ * fetching stays with whoever can show that it is happening.
+ */
+ data class NeedsFetch(val episode: EpisodeEntity, val kind: SubscriptionKind) : StartOutcome
+
+ /**
+ * yt-dlp could not produce the audio, with whatever it said about why.
+ *
+ * The reason is carried rather than collapsed to a boolean because it is the only place the
+ * real cause shows up: "Sign in to confirm you're not a bot" and "video unavailable" are
+ * very different problems and look identical without it.
+ */
+ data class FetchFailed(val reason: String) : StartOutcome
+
+ /** Nothing to play: the episode has gone, or the queue is empty. */
+ data object Gone : StartOutcome
+}
+
/** An episode ready to begin: which one, where from, and how fast. */
data class NextUp(
val episode: EpisodeEntity,
@@ -37,11 +82,8 @@ class PlaybackAdvancer(
private val episodes: EpisodeDao,
private val subscriptions: SubscriptionDao,
private val queue: QueueRepository,
- /**
- * How chapters are obtained. A collaborator rather than a ChapterFetcher so the decision to
- * fetch at all -- only for a show that opted in -- stays here and stays testable.
- */
- private val chaptersFor: suspend (EpisodeEntity) -> List<Chapter> = { emptyList() },
+ /** Everything here that needs the network, behind one seam so tests can stand it in. */
+ private val sources: PlaybackSources = PlaybackSources.None,
private val progress: PlaybackProgress = PlaybackProgress(),
private val now: () -> Long = System::currentTimeMillis,
) {
@@ -56,7 +98,7 @@ class PlaybackAdvancer(
val episode = episodeId?.let { episodes.getById(it) } ?: return PlayingContext()
val show = subscriptions.getById(episode.subscriptionId)
// Only worth fetching for a show that asked for it; most publish no chapters at all.
- val chapters = if (show?.skipAdChapters == true) chaptersFor(episode) else emptyList()
+ val chapters = if (show?.skipAdChapters == true) sources.chapters(episode) else emptyList()
return PlayingContext(show, chapters)
}
@@ -66,24 +108,47 @@ class PlaybackAdvancer(
* Marking played happens before advancing: advance rewrites the queue, and the other order
* loses the id.
*/
- suspend fun finish(episodeId: String): NextUp? {
+ suspend fun finish(episodeId: String): StartOutcome {
episodes.markPlayed(episodeId, now())
- return queue.advance()?.let { start(it) }
+ return queue.advance()?.let { start(it) } ?: StartOutcome.Gone
}
- /** Everything needed to begin [episodeId], or null if it has gone. */
- suspend fun start(episodeId: String): NextUp? {
- val episode = episodes.getById(episodeId) ?: return null
+ /**
+ * Fetches the audio an episode is missing and then resolves it, so callers never have to
+ * implement the fetch-then-retry sequence themselves.
+ *
+ * A failed fetch reports [StartOutcome.FetchFailed] rather than retrying: the episode's
+ * FAILED download state is visible in the list, and looping on a video YouTube will not
+ * serve gets nowhere.
+ */
+ suspend fun fetchAndStart(episode: EpisodeEntity, kind: SubscriptionKind): StartOutcome =
+ sources.fetchAudio(episode, kind).fold(
+ onSuccess = { start(episode.id) },
+ onFailure = { StartOutcome.FetchFailed(it.message ?: it.javaClass.simpleName) },
+ )
+
+ /** What it would take to begin [episodeId]. */
+ suspend fun start(episodeId: String): StartOutcome {
+ val episode = episodes.getById(episodeId) ?: return StartOutcome.Gone
val show = subscriptions.getById(episode.subscriptionId)
- return NextUp(
- episode = episode,
- show = show,
- startPositionMs = progress.resumePosition(
- episode.positionMs,
- episode.durationMs,
- show?.skipIntroMs ?: 0,
- ),
- speed = PlaybackSpeed.forEpisode(episode.speedOverride, show?.playbackSpeed),
+
+ // Nothing to play until yt-dlp has written a file. The download is the caller's job.
+ val kind = show?.kind
+ if (kind?.needsExtraction == true && episode.localPath == null) {
+ return StartOutcome.NeedsFetch(episode, kind)
+ }
+
+ return StartOutcome.Ready(
+ NextUp(
+ episode = episode,
+ show = show,
+ startPositionMs = progress.resumePosition(
+ episode.positionMs,
+ episode.durationMs,
+ show?.skipIntroMs ?: 0,
+ ),
+ speed = PlaybackSpeed.forEpisode(episode.speedOverride, show?.playbackSpeed),
+ )
)
}
}
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 0135cdc..bdb0a38 100644
--- a/app/src/main/java/org/terst/listen/playback/PlaybackService.kt
+++ b/app/src/main/java/org/terst/listen/playback/PlaybackService.kt
@@ -163,20 +163,35 @@ class PlaybackService : MediaLibraryService() {
/** Marks the episode played, moves the queue on, and starts what follows or stops. */
private fun finish(episodeId: String) {
val current = advancer ?: return
- scope.launch {
- val next = current.finish(episodeId)
- if (next == null) {
- // Stopping matters because an episode can now finish before its file does. Left
- // running, an outro offset marked the episode played and played the credits
- // anyway -- the exact thing the offset exists to avoid.
- player?.pause()
- loadContextFor(null)
- return@launch
- }
- play(next)
+ scope.launch { begin(current.finish(episodeId)) }
+ }
+
+ /**
+ * Acts on what the advancer said, fetching first when the next episode has no audio yet.
+ *
+ * The fetch happens here as well as in the view model because the queue moves on without
+ * the UI: a YouTube episode reached by auto-advance would otherwise be handed its watch
+ * page, which is the bug this fixes.
+ */
+ private suspend fun begin(outcome: StartOutcome) {
+ when (outcome) {
+ is StartOutcome.Ready -> play(outcome.next)
+
+ is StartOutcome.NeedsFetch ->
+ begin(advancer?.fetchAndStart(outcome.episode, outcome.kind) ?: StartOutcome.Gone)
+
+ // Stopping matters because an episode can now finish before its file does. Left
+ // running, an outro offset marked the episode played and played the credits
+ // anyway -- the exact thing the offset exists to avoid.
+ is StartOutcome.FetchFailed, StartOutcome.Gone -> stop()
}
}
+ private fun stop() {
+ player?.pause()
+ loadContextFor(null)
+ }
+
/** Starts an episode on this service's own player, settings and all. */
private fun play(next: NextUp) {
val exo = player ?: return
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 2296109..d88414f 100644
--- a/app/src/main/java/org/terst/listen/playback/PlayerConnection.kt
+++ b/app/src/main/java/org/terst/listen/playback/PlayerConnection.kt
@@ -36,6 +36,15 @@ class PlayerConnection(private val context: Context) {
private var controller: MediaController? = null
+ /**
+ * Reported so playback failures are never silent again.
+ *
+ * A YouTube episode used to be handed its watch page and die with
+ * UnrecognizedInputFormatException, with nothing in the app listening -- the screen simply
+ * sat there. Whatever the next failure of this class is, it should say so.
+ */
+ var onError: ((String) -> Unit)? = null
+
private val _nowPlaying = MutableStateFlow(
NowPlaying(null, null, null, isPlaying = false, positionMs = 0, durationMs = null, speed = 1.0f)
)
@@ -68,6 +77,11 @@ class PlayerConnection(private val context: Context) {
// Completion is PlaybackService's job, not this connection's: it has to keep happening
// when the activity is gone and only the service is left playing.
override fun onPlaybackStateChanged(playbackState: Int) = publish()
+
+ override fun onPlayerError(error: androidx.media3.common.PlaybackException) {
+ onError?.invoke(error.errorCodeName)
+ publish()
+ }
}
/**
diff --git a/app/src/main/java/org/terst/listen/ui/PlayerBar.kt b/app/src/main/java/org/terst/listen/ui/PlayerBar.kt
index fd526c0..d9b8652 100644
--- a/app/src/main/java/org/terst/listen/ui/PlayerBar.kt
+++ b/app/src/main/java/org/terst/listen/ui/PlayerBar.kt
@@ -1,8 +1,22 @@
package org.terst.listen.ui
+import androidx.compose.foundation.clickable
+import androidx.compose.foundation.layout.Column
+import androidx.compose.foundation.layout.Row
+import androidx.compose.foundation.layout.fillMaxWidth
+import androidx.compose.foundation.layout.padding
+import androidx.compose.foundation.layout.size
+import androidx.compose.material3.CircularProgressIndicator
+import androidx.compose.material3.MaterialTheme
+import androidx.compose.material3.Surface
+import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.getValue
+import androidx.compose.ui.Alignment
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.text.style.TextOverflow
+import androidx.compose.ui.unit.dp
/**
* The now-playing bar as a screen's bottom bar.
@@ -13,14 +27,55 @@ import androidx.compose.runtime.getValue
@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,
- ),
- )
+ val preparing by player.preparing.collectAsState()
+ val banner by player.banner.collectAsState()
+
+ // Rendered here rather than through a screen's snackbar because nothing rendered it at all:
+ // playback failures, fetch failures and even "sleep timer finished" were set on a flow no
+ // screen collected. The bar is on every screen that plays anything, so it is the one place
+ // that always exists.
+ // A Column because these stack: a message can be showing while something plays underneath.
+ Column {
+ PlayerMessage(banner, onDismiss = player::dismissBanner)
+
+ // A YouTube or Facebook episode has to be fetched by yt-dlp before anything can play,
+ // which takes real seconds and leaves nothing playing meanwhile -- so the normal bar
+ // hides itself and the tap looks like it did nothing. Saying so is this branch's point.
+ val fetching = preparing
+ if (fetching != null) {
+ PreparingBar(fetching)
+ } else {
+ NowPlayingBar(
+ state = nowPlaying,
+ actions = NowPlayingActions(
+ togglePlayPause = player::togglePlayPause,
+ skipBack = player::skipBack,
+ skipForward = player::skipForward,
+ openPlayer = onOpenPlayer,
+ openQueue = onOpenQueue,
+ ),
+ )
+ }
+ }
+}
+
+@Composable
+private fun PreparingBar(title: String) {
+ Surface(tonalElevation = 3.dp, modifier = Modifier.fillMaxWidth()) {
+ Row(
+ Modifier.fillMaxWidth().padding(horizontal = 16.dp, vertical = 12.dp),
+ verticalAlignment = Alignment.CenterVertically,
+ ) {
+ CircularProgressIndicator(Modifier.size(20.dp), strokeWidth = 2.dp)
+ Column(Modifier.padding(start = 12.dp)) {
+ Text("Fetching audio", style = MaterialTheme.typography.titleSmall)
+ Text(
+ title,
+ style = MaterialTheme.typography.bodySmall,
+ maxLines = 1,
+ overflow = TextOverflow.Ellipsis,
+ )
+ }
+ }
+ }
}
diff --git a/app/src/main/java/org/terst/listen/ui/PlayerMessage.kt b/app/src/main/java/org/terst/listen/ui/PlayerMessage.kt
new file mode 100644
index 0000000..11f6218
--- /dev/null
+++ b/app/src/main/java/org/terst/listen/ui/PlayerMessage.kt
@@ -0,0 +1,35 @@
+package org.terst.listen.ui
+
+import androidx.compose.foundation.clickable
+import androidx.compose.foundation.layout.fillMaxWidth
+import androidx.compose.foundation.layout.padding
+import androidx.compose.material3.MaterialTheme
+import androidx.compose.material3.Surface
+import androidx.compose.material3.Text
+import androidx.compose.runtime.Composable
+import androidx.compose.ui.Modifier
+import androidx.compose.ui.unit.dp
+
+/**
+ * Whatever the player has to say, shown wherever the player is.
+ *
+ * Nothing rendered [PlayerViewModel.banner] at all until now: playback failures, fetch failures
+ * and even "sleep timer finished" were set on a flow no screen collected. That is why a YouTube
+ * episode failing to play looked like nothing happening. Tapping it dismisses it.
+ */
+@Composable
+fun PlayerMessage(banner: Banner?, onDismiss: () -> Unit) {
+ if (banner == null) return
+ val colour = if (banner.isError) {
+ MaterialTheme.colorScheme.errorContainer
+ } else {
+ MaterialTheme.colorScheme.surfaceVariant
+ }
+ Surface(color = colour, modifier = Modifier.fillMaxWidth().clickable(onClick = onDismiss)) {
+ Text(
+ banner.text,
+ style = MaterialTheme.typography.bodySmall,
+ modifier = Modifier.padding(horizontal = 16.dp, vertical = 10.dp),
+ )
+ }
+}
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 e012834..b802825 100644
--- a/app/src/main/java/org/terst/listen/ui/PlayerScreen.kt
+++ b/app/src/main/java/org/terst/listen/ui/PlayerScreen.kt
@@ -53,6 +53,8 @@ fun PlayerScreen(player: PlayerViewModel, onBack: () -> Unit) {
val nowPlaying by player.nowPlaying.collectAsState()
val show by player.currentShow.collectAsState()
val chapters by player.chapters.collectAsState()
+ val banner by player.banner.collectAsState()
+ val preparing by player.preparing.collectAsState()
Scaffold(
topBar = {
@@ -81,8 +83,12 @@ fun PlayerScreen(player: PlayerViewModel, onBack: () -> Unit) {
.background(MaterialTheme.colorScheme.surfaceVariant),
)
+ PlayerMessage(banner, onDismiss = player::dismissBanner)
+
+ // Tapping Play navigates straight here, so this is the screen that has to say a
+ // fetch is running -- otherwise a yt-dlp extraction reads as "Nothing playing".
Text(
- nowPlaying.title ?: "Nothing playing",
+ preparing?.let { "Fetching audio — $it" } ?: nowPlaying.title ?: "Nothing playing",
style = MaterialTheme.typography.titleLarge,
maxLines = 2,
overflow = TextOverflow.Ellipsis,
diff --git a/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt b/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt
index 1d9aa5d..418a0ff 100644
--- a/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt
+++ b/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt
@@ -16,6 +16,7 @@ import org.terst.listen.data.SubscriptionEntity
import org.terst.listen.feed.Chapter
import org.terst.listen.playback.NowPlaying
import org.terst.listen.playback.PlayerConnection
+import org.terst.listen.playback.StartOutcome
import org.terst.listen.playback.SleepTimer
import org.terst.listen.playback.toMediaItem
@@ -36,9 +37,16 @@ class PlayerViewModel(app: Application) : AndroidViewModel(app) {
_banner.value = null
}
- private val connection = PlayerConnection(app).also { it.connect() }
+ private val connection = PlayerConnection(app).also { connection ->
+ connection.connect()
+ connection.onError = { code -> _banner.value = Banner("Playback failed: $code", isError = true) }
+ }
val nowPlaying: StateFlow<NowPlaying> = connection.nowPlaying
+ /** The episode whose audio is being fetched before it can play, or null. */
+ private val _preparing = MutableStateFlow<String?>(null)
+ val preparing: StateFlow<String?> = _preparing.asStateFlow()
+
/** Chapters for whatever is playing, empty when the show publishes none. */
private val _chapters = MutableStateFlow<List<Chapter>>(emptyList())
val chapters: StateFlow<List<Chapter>> = _chapters.asStateFlow()
@@ -99,12 +107,40 @@ class PlayerViewModel(app: Application) : AndroidViewModel(app) {
// whatever was actually queued.
graph.queue.playNow(episode.id)
- val next = graph.advancer.start(episode.id) ?: return@launch
- connection.play(
- item = next.episode.toMediaItem(next.show?.title, next.show?.author, next.show?.imageUrl),
- startPositionMs = next.startPositionMs,
- speed = next.speed,
+ begin(graph.advancer.start(episode.id))
+ }
+ }
+
+ /**
+ * Starts what the advancer resolved, fetching the audio first when there is none yet.
+ *
+ * A YouTube or Facebook episode has no playable URL until yt-dlp has written a file -- its
+ * mediaUrl is a watch page. That fetch takes real time, so [preparing] drives a label
+ * rather than leaving the UI looking broken, which is how this bug presented.
+ */
+ private suspend fun begin(outcome: StartOutcome) {
+ when (outcome) {
+ is StartOutcome.Ready -> connection.play(
+ item = outcome.next.episode.toMediaItem(
+ outcome.next.show?.title,
+ outcome.next.show?.author,
+ outcome.next.show?.imageUrl,
+ ),
+ startPositionMs = outcome.next.startPositionMs,
+ speed = outcome.next.speed,
)
+
+ is StartOutcome.NeedsFetch -> {
+ _preparing.value = outcome.episode.title
+ val started = graph.advancer.fetchAndStart(outcome.episode, outcome.kind)
+ _preparing.value = null
+ begin(started)
+ }
+
+ is StartOutcome.FetchFailed ->
+ _banner.value = Banner("Could not fetch audio: ${outcome.reason}", isError = true)
+
+ StartOutcome.Gone -> Unit
}
}
diff --git a/app/src/test/java/org/terst/listen/playback/PlaybackAdvancerTest.kt b/app/src/test/java/org/terst/listen/playback/PlaybackAdvancerTest.kt
index e94bd06..e22be1e 100644
--- a/app/src/test/java/org/terst/listen/playback/PlaybackAdvancerTest.kt
+++ b/app/src/test/java/org/terst/listen/playback/PlaybackAdvancerTest.kt
@@ -11,7 +11,9 @@ import org.terst.listen.data.FakeEpisodeDao
import org.terst.listen.data.FakeQueueDao
import org.terst.listen.data.FakeSubscriptionDao
import org.terst.listen.data.QueueRepository
+import org.terst.listen.data.DownloadState
import org.terst.listen.data.SubscriptionEntity
+import org.terst.listen.data.SubscriptionKind
import org.terst.listen.feed.Chapter
/**
@@ -46,11 +48,13 @@ class PlaybackAdvancerTest {
id: String = "s1",
skipIntroMs: Long = 0,
playbackSpeed: Float? = null,
+ kind: SubscriptionKind = SubscriptionKind.PODCAST,
): SubscriptionEntity {
val entity = SubscriptionEntity(
id = id,
feedUrl = "https://example.com/$id.xml",
title = "Show $id",
+ kind = kind,
addedAtMs = 0,
skipIntroMs = skipIntroMs,
playbackSpeed = playbackSpeed,
@@ -64,6 +68,7 @@ class PlaybackAdvancerTest {
subscriptionId: String = "s1",
positionMs: Long = 0,
speedOverride: Float? = null,
+ localPath: String? = null,
): EpisodeEntity {
val entity = EpisodeEntity(
id = id,
@@ -76,6 +81,8 @@ class PlaybackAdvancerTest {
discoveredAtMs = 0,
positionMs = positionMs,
speedOverride = speedOverride,
+ localPath = localPath,
+ downloadState = if (localPath != null) DownloadState.DOWNLOADED else DownloadState.NONE,
)
episodes.rows += entity
return entity
@@ -83,6 +90,24 @@ class PlaybackAdvancerTest {
private suspend fun queued() = queueDao.getEntries().map { it.episodeId }
+ /** Stands in for the network: yt-dlp and chapter files. */
+ private class FakeSources(
+ val onFetch: suspend (EpisodeEntity) -> Result<Unit> = { Result.success(Unit) },
+ val chapters: List<Chapter> = emptyList(),
+ val onChapters: () -> Unit = {},
+ ) : PlaybackSources {
+ override suspend fun chapters(episode: EpisodeEntity): List<Chapter> {
+ onChapters()
+ return chapters
+ }
+
+ override suspend fun fetchAudio(episode: EpisodeEntity, kind: SubscriptionKind) =
+ onFetch(episode)
+ }
+
+ /** Unwraps the common case so the assertions stay about the thing under test. */
+ private fun StartOutcome.ready() = (this as? StartOutcome.Ready)?.next
+
// --- finishing --------------------------------------------------------------------------
@Test
@@ -104,7 +129,7 @@ class PlaybackAdvancerTest {
queue.append("a")
queue.append("b")
- val next = advancer.finish("a")
+ val next = advancer.finish("a").ready()
assertEquals("b", next?.episode?.id)
assertEquals(listOf("b"), queued())
@@ -117,7 +142,7 @@ class PlaybackAdvancerTest {
episode("a")
queue.append("a")
- assertNull(advancer.finish("a"))
+ assertEquals(StartOutcome.Gone, advancer.finish("a"))
assertEquals(emptyList<String>(), queued())
}
@@ -130,7 +155,7 @@ class PlaybackAdvancerTest {
show()
episode("a")
- assertNull(advancer.finish("a"))
+ assertEquals(StartOutcome.Gone, advancer.finish("a"))
assertEquals(now, episodes.rows.single().playedAtMs)
}
@@ -141,7 +166,7 @@ class PlaybackAdvancerTest {
show(skipIntroMs = 47_000)
episode("a")
- assertEquals(47_000L, advancer.start("a")?.startPositionMs)
+ assertEquals(47_000L, advancer.start("a").ready()?.startPositionMs)
}
@Test
@@ -149,7 +174,7 @@ class PlaybackAdvancerTest {
show(skipIntroMs = 47_000)
episode("a", positionMs = 12 * minute)
- assertEquals(12 * minute, advancer.start("a")?.startPositionMs)
+ assertEquals(12 * minute, advancer.start("a").ready()?.startPositionMs)
}
@Test
@@ -158,8 +183,8 @@ class PlaybackAdvancerTest {
episode("a")
episode("b", speedOverride = 1.0f)
- assertEquals(1.5f, advancer.start("a")?.speed)
- assertEquals(1.0f, advancer.start("b")?.speed)
+ assertEquals(1.5f, advancer.start("a").ready()?.speed)
+ assertEquals(1.0f, advancer.start("b").ready()?.speed)
}
@Test
@@ -171,7 +196,7 @@ class PlaybackAdvancerTest {
queue.append("a")
queue.append("b")
- val next = advancer.finish("a")
+ val next = advancer.finish("a").ready()
assertEquals(30_000L, next?.startPositionMs)
assertEquals(2.0f, next?.speed)
@@ -180,7 +205,123 @@ class PlaybackAdvancerTest {
@Test
fun `an episode that no longer exists starts nothing`() = runTest {
- assertNull(advancer.start("gone"))
+ assertEquals(StartOutcome.Gone, advancer.start("gone"))
+ }
+
+ // --- sources that need extracting first ---------------------------------------------------
+ //
+ // The reported bug. A YouTube episode's mediaUrl is its watch page, because the feed carries
+ // no enclosure -- the audio URL only exists once yt-dlp has resolved it. Handing that page to
+ // ExoPlayer produced UnrecognizedInputFormatException, silently.
+
+ @Test
+ fun `a youtube episode with nothing downloaded cannot start yet`() = runTest {
+ show(kind = SubscriptionKind.YOUTUBE_CHANNEL)
+ episode("a")
+
+ val outcome = advancer.start("a")
+
+ assertEquals(
+ StartOutcome.NeedsFetch(episodes.rows.single(), SubscriptionKind.YOUTUBE_CHANNEL),
+ outcome,
+ )
+ }
+
+ @Test
+ fun `a youtube episode already downloaded starts from its file`() = runTest {
+ show(kind = SubscriptionKind.YOUTUBE_CHANNEL)
+ episode("a", localPath = "/audio/a.m4a")
+
+ assertNotNull(advancer.start("a").ready())
+ }
+
+ /** An ordinary podcast enclosure is a real audio URL and streams without any of this. */
+ @Test
+ fun `a podcast episode starts without being downloaded`() = runTest {
+ show()
+ episode("a")
+
+ assertNotNull(advancer.start("a").ready())
+ }
+
+ @Test
+ fun `fetching then starting plays from the file yt-dlp wrote`() = runTest {
+ show(kind = SubscriptionKind.YOUTUBE_CHANNEL)
+ val episode = episode("a")
+ val fetching = PlaybackAdvancer(
+ episodes,
+ subscriptions,
+ queue,
+ // Stands in for yt-dlp: writing a path is what makes the episode playable.
+ sources = FakeSources(onFetch = { target ->
+ episodes.updateDownload(target.id, DownloadState.DOWNLOADED, "/audio/a.m4a", now)
+ Result.success(Unit)
+ }),
+ ) { now }
+
+ val outcome = fetching.fetchAndStart(episode, SubscriptionKind.YOUTUBE_CHANNEL)
+
+ assertEquals("a", (outcome as? StartOutcome.Ready)?.next?.episode?.id)
+ }
+
+ /**
+ * The reason is carried, not collapsed to a failure.
+ *
+ * "Sign in to confirm you're not a bot" and "video unavailable" are very different problems
+ * and indistinguishable without it -- as a real debugging session proved.
+ */
+ @Test
+ fun `a failed fetch reports what yt-dlp said`() = runTest {
+ show(kind = SubscriptionKind.YOUTUBE_CHANNEL)
+ val episode = episode("a")
+ val failing = PlaybackAdvancer(
+ episodes,
+ subscriptions,
+ queue,
+ sources = FakeSources(onFetch = {
+ Result.failure(IllegalStateException("Sign in to confirm you're not a bot"))
+ }),
+ ) { now }
+
+ val outcome = failing.fetchAndStart(episode, SubscriptionKind.YOUTUBE_CHANNEL)
+
+ assertEquals(
+ StartOutcome.FetchFailed("Sign in to confirm you're not a bot"),
+ outcome,
+ )
+ }
+
+ /**
+ * The default has no extractor, and says so rather than pretending the fetch worked.
+ *
+ * This is what a unit test or a half-wired graph gets; silently reporting success would
+ * send it round the loop again with the episode still unplayable.
+ */
+ @Test
+ fun `with no audio source configured the fetch reports that`() = runTest {
+ show(kind = SubscriptionKind.YOUTUBE_CHANNEL)
+ val episode = episode("a")
+
+ val outcome = advancer.fetchAndStart(episode, SubscriptionKind.YOUTUBE_CHANNEL)
+
+ assertEquals(StartOutcome.FetchFailed("no audio source configured"), outcome)
+ assertEquals(emptyList<Chapter>(), advancer.contextFor("a").chapters)
+ }
+
+ /** The queue advancing onto a YouTube episode hits the same wall, and must report it. */
+ @Test
+ fun `advancing onto a youtube episode reports that it needs fetching`() = runTest {
+ show(id = "s1")
+ show(id = "yt", kind = SubscriptionKind.YOUTUBE_CHANNEL)
+ episode("a", subscriptionId = "s1")
+ episode("b", subscriptionId = "yt")
+ queue.append("a")
+ queue.append("b")
+
+ val outcome = advancer.finish("a")
+
+ assertEquals(SubscriptionKind.YOUTUBE_CHANNEL, (outcome as? StartOutcome.NeedsFetch)?.kind)
+ assertEquals("b", (outcome as? StartOutcome.NeedsFetch)?.episode?.id)
}
// --- the context the ticker runs against --------------------------------------------------
@@ -209,7 +350,10 @@ class PlaybackAdvancerTest {
episodes,
subscriptions,
queue,
- chaptersFor = { asked++; listOf(Chapter(startMs = 0, title = "One")) },
+ sources = FakeSources(
+ chapters = listOf(Chapter(startMs = 0, title = "One")),
+ onChapters = { asked++ },
+ ),
) { now }
show(id = "s1")
@@ -227,7 +371,7 @@ class PlaybackAdvancerTest {
fun `an episode with no surviving show still starts`() = runTest {
episode("a", subscriptionId = "gone")
- val start = advancer.start("a")
+ val start = advancer.start("a").ready()
assertNotNull(start)
assertEquals(1.0f, start?.speed)