diff options
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) |