diff options
12 files changed, 298 insertions, 33 deletions
diff --git a/.coverage-baseline b/.coverage-baseline index f5a155b..cad862e 100644 --- a/.coverage-baseline +++ b/.coverage-baseline @@ -1 +1 @@ -93.80 +93.85 diff --git a/app/src/main/java/org/terst/listen/data/Preferences.kt b/app/src/main/java/org/terst/listen/data/Preferences.kt index e63b898..8cbc583 100644 --- a/app/src/main/java/org/terst/listen/data/Preferences.kt +++ b/app/src/main/java/org/terst/listen/data/Preferences.kt @@ -46,6 +46,22 @@ class Preferences(private val context: Context) { } /** + * Whether a break that is due to be acted on is played at high speed instead of being + * skipped. + * + * Orthogonal to [skipShowFurniture]: that preference decides *which* breaks get acted on + * (advertising always; the show's own intro, cue and credits only when asked for); this one + * decides *what the action is* once a break is already due to be acted on. Off by default -- + * skipping is what every existing user already hears, and changing that without being asked + * is not wanted. + */ + val fastPlayBreaks: Flow<Boolean> = context.store.data.map { it[FAST_PLAY_BREAKS] ?: false } + + suspend fun setFastPlayBreaks(value: Boolean) { + context.store.edit { it[FAST_PLAY_BREAKS] = value } + } + + /** * Where the skip server is, and the token for it. Blank means not configured, which is a * working state: the app downloads from publishers and plays without detected skips, * exactly as it did before the server existed. @@ -65,5 +81,6 @@ class Preferences(private val context: Context) { val SERVER_URL = stringPreferencesKey("server_url") val SERVER_TOKEN = stringPreferencesKey("server_token") val SKIP_SHOW_FURNITURE = booleanPreferencesKey("skip_show_furniture") + val FAST_PLAY_BREAKS = booleanPreferencesKey("fast_play_breaks") } } 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 8a2acb2..d064bb9 100644 --- a/app/src/main/java/org/terst/listen/data/Repositories.kt +++ b/app/src/main/java/org/terst/listen/data/Repositories.kt @@ -124,6 +124,21 @@ class AppGraph(private val context: Context) { android.util.Log.w("AppGraph", "skipShowFurniture unreadable, defaulting to skip", e) true } + + // Same shape as skipFurniture() just above, and for the same reason: this also runs + // inside PlaybackService's bare scope.launch, so CancellationException must be + // rethrown rather than swallowed by a bare runCatching, or a fast episode switch + // overwrites the new episode's context with the previous one's. Defaults to false + // (skip) on a read failure -- that is today's behaviour, and the safe direction: a + // break that silently stops being skipped is the failure mode that matters here. + override suspend fun fastPlayBreaks() = try { + preferences.fastPlayBreaks.first() + } catch (e: CancellationException) { + throw e + } catch (e: IOException) { + android.util.Log.w("AppGraph", "fastPlayBreaks unreadable, defaulting to skip", e) + false + } }) } diff --git a/app/src/main/java/org/terst/listen/playback/MidrollSkip.kt b/app/src/main/java/org/terst/listen/playback/MidrollSkip.kt index c13656b..32b2144 100644 --- a/app/src/main/java/org/terst/listen/playback/MidrollSkip.kt +++ b/app/src/main/java/org/terst/listen/playback/MidrollSkip.kt @@ -29,12 +29,30 @@ object MidrollSkip { ranges: List<EpisodeMidrollEntity>, positionMs: Long, skipFurniture: Boolean, - ): Long? = + ): Long? = activeBreak(ranges, positionMs, skipFurniture)?.endMs?.takeIf { it > positionMs } + + /** + * Whether the playhead is inside a break that should be acted on right now, without working + * out where a seek would land -- what fast-play mode needs instead of [skipTarget]. + * + * Same filter as [skipTarget], kept in [activeBreak] rather than duplicated here: a kind + * added by a newer server must keep being acted on by both callers, not just the one that + * happened to get updated. + */ + fun isActedOnBreak( + ranges: List<EpisodeMidrollEntity>, + positionMs: Long, + skipFurniture: Boolean, + ): Boolean = activeBreak(ranges, positionMs, skipFurniture) != null + + private fun activeBreak( + ranges: List<EpisodeMidrollEntity>, + positionMs: Long, + skipFurniture: Boolean, + ): EpisodeMidrollEntity? = ranges.asSequence() .filter { skipFurniture || !isShowsOwnAudio(it.kind) } .firstOrNull { positionMs >= it.startMs && positionMs < it.endMs } - ?.endMs - ?.takeIf { it > positionMs } private fun isShowsOwnAudio(kind: String) = kind in SHOW_FURNITURE } 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 254b636..50f172c 100644 --- a/app/src/main/java/org/terst/listen/playback/PlaybackAdvancer.kt +++ b/app/src/main/java/org/terst/listen/playback/PlaybackAdvancer.kt @@ -19,7 +19,12 @@ data class PlayingContext( val midrolls: List<EpisodeMidrollEntity> = emptyList(), /** Whether a show's own intro, cue and credits are skipped as well as the advertising. */ val skipFurniture: Boolean = true, -) + /** Whether a break that is due to be acted on is played fast instead of being skipped. */ + val fastPlayBreaks: Boolean = false, +) { + /** The episode's normal speed: a rule's choice, else the show's, else the global default. */ + val speed: Float get() = PlaybackSpeed.forEpisode(episode?.speedOverride, show?.playbackSpeed) +} /** * Everything about starting or scoring an episode that lives outside [PlaybackAdvancer]'s own @@ -38,6 +43,9 @@ interface PlaybackSources { /** Whether a show's own intro, cue and credits are skipped as well as the advertising. */ suspend fun skipFurniture(): Boolean + /** Whether a break that is due to be acted on is played fast instead of being skipped. */ + suspend fun fastPlayBreaks(): Boolean + /** For the cases that never reach outside: no chapters, no midrolls, nothing to extract with. */ object None : PlaybackSources { override suspend fun chapters(episode: EpisodeEntity) = emptyList<Chapter>() @@ -45,6 +53,7 @@ interface PlaybackSources { override suspend fun fetchAudio(episode: EpisodeEntity, kind: SubscriptionKind) = Result.failure<Unit>(IllegalStateException("no audio source configured")) override suspend fun skipFurniture() = true + override suspend fun fastPlayBreaks() = false } } @@ -115,7 +124,14 @@ class PlaybackAdvancer( 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) sources.chapters(episode) else emptyList() - return PlayingContext(show, chapters, episode, sources.midrolls(episode), sources.skipFurniture()) + return PlayingContext( + show, + chapters, + episode, + sources.midrolls(episode), + sources.skipFurniture(), + sources.fastPlayBreaks(), + ) } /** 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 e2ca3a8..ef4be31 100644 --- a/app/src/main/java/org/terst/listen/playback/PlaybackService.kt +++ b/app/src/main/java/org/terst/listen/playback/PlaybackService.kt @@ -203,6 +203,11 @@ class PlaybackService : MediaLibraryService() { ) when (action) { is TickAction.SeekTo -> exo.seekTo(action.positionMs) + // Only reassigns playbackParameters when the speed actually changes: this tick runs + // every two seconds for hours at a stretch, and reassigning it unconditionally is + // wasteful and can audibly glitch. + is TickAction.SetSpeed -> + if (exo.playbackParameters.speed != action.speed) exo.applySpeed(action.speed, action.speed) TickAction.Finish -> exo.currentMediaItem?.mediaId?.let(::finish) TickAction.Nothing -> Unit } diff --git a/app/src/main/java/org/terst/listen/playback/PlayheadPolicy.kt b/app/src/main/java/org/terst/listen/playback/PlayheadPolicy.kt index 02b2768..f814034 100644 --- a/app/src/main/java/org/terst/listen/playback/PlayheadPolicy.kt +++ b/app/src/main/java/org/terst/listen/playback/PlayheadPolicy.kt @@ -1,8 +1,17 @@ package org.terst.listen.playback +import org.terst.listen.data.SubscriptionEntity + /** What the playback ticker should do about where the playhead currently is. */ sealed interface TickAction { data class SeekTo(val positionMs: Long) : TickAction + + /** + * Play at this speed right now -- fast-play mode's equivalent of [SeekTo]: raced through a + * break instead of jumped past it. Issued every tick as a pure function of where the + * playhead is, not as an edge transition, so it stays correct across a scrub. + */ + data class SetSpeed(val speed: Float) : TickAction data object Finish : TickAction data object Nothing : TickAction } @@ -16,6 +25,15 @@ sealed interface TickAction { */ object PlayheadPolicy { + /** + * How fast a break plays in fast-play mode, instead of being skipped. + * + * 3x, the low end of the range this was asked for: Media3's default audio processor + * pitch-corrects at this speed, so a break stays recognisable as speech instead of becoming + * a chirp. + */ + private const val FAST_BREAK_SPEED = 3.0f + fun decide( context: PlayingContext, positionMs: Long, @@ -23,25 +41,61 @@ object PlayheadPolicy { progress: PlaybackProgress, ): TickAction { val show = context.show ?: return TickAction.Nothing - - // Finishing beats skipping. A show with both an outro offset and a marked break at the - // end would otherwise seek into the credits it was told to skip, and sit there. val outroMs = effectiveOutroMs(context.episode, show) - if (outroMs > 0 && progress.isComplete(positionMs, durationMs, outroMs)) { - return TickAction.Finish - } - // Advertising is skipped unconditionally; the show's own audio (intro, cue, outro) is - // skipped only when context.skipFurniture says to -- the global preference read at - // episode start. There is still no *per-show* toggle: the confirmation threshold behind - // a detected midroll break is the safety mechanism for whether a break is trusted at - // all, and the furniture preference is the only switch on top of that. - MidrollSkip.skipTarget(context.midrolls, positionMs, context.skipFurniture) - ?.let { return TickAction.SeekTo(it) } - - if (!show.skipAdChapters) return TickAction.Nothing - return AdChapters.skipTarget(context.chapters, positionMs, durationMs) - ?.let(TickAction::SeekTo) - ?: TickAction.Nothing + // Each tier reports what it would do, or null to defer to the next. Finishing beats + // everything below it, skipping and fast-play alike: a show with both an outro offset + // and a marked break at the end would otherwise seek -- or speed up -- into the credits + // it was told to skip, and sit there. + return finishAction(outroMs, positionMs, durationMs, progress) + ?: breakAction(context, positionMs) + ?: chapterAction(show, context, positionMs, durationMs) + ?: fallbackSpeed(context) } + + private fun finishAction( + outroMs: Long, + positionMs: Long, + durationMs: Long?, + progress: PlaybackProgress, + ): TickAction? = + TickAction.Finish.takeIf { outroMs > 0 && progress.isComplete(positionMs, durationMs, outroMs) } + + /** + * Advertising is acted on unconditionally; the show's own audio (intro, cue, outro) is acted + * on only when [PlayingContext.skipFurniture] says to -- the global preference read at + * episode start. There is still no *per-show* toggle: the confirmation threshold behind a + * detected midroll break is the safety mechanism for whether a break is trusted at all, and + * the furniture preference is the only switch on top of that. [PlayingContext.fastPlayBreaks] + * is a second, orthogonal preference: it decides *what* the action is once a break is already + * due to be acted on, never *whether*. + */ + private fun breakAction(context: PlayingContext, positionMs: Long): TickAction? = + if (context.fastPlayBreaks) { + TickAction.SetSpeed(FAST_BREAK_SPEED) + .takeIf { MidrollSkip.isActedOnBreak(context.midrolls, positionMs, context.skipFurniture) } + } else { + MidrollSkip.skipTarget(context.midrolls, positionMs, context.skipFurniture)?.let(TickAction::SeekTo) + } + + private fun chapterAction( + show: SubscriptionEntity, + context: PlayingContext, + positionMs: Long, + durationMs: Long?, + ): TickAction? = + if (show.skipAdChapters) { + AdChapters.skipTarget(context.chapters, positionMs, durationMs)?.let(TickAction::SeekTo) + } else { + null + } + + /** + * Nothing above fired: in fast-play mode that still means asking for the episode's normal + * speed, so a break just left does not keep playing fast -- the service only reassigns + * playbackParameters when this differs from what is already set, so repeating it every tick + * is cheap. Skip mode has nothing left to do. + */ + private fun fallbackSpeed(context: PlayingContext): TickAction = + if (context.fastPlayBreaks) TickAction.SetSpeed(context.speed) else TickAction.Nothing } diff --git a/app/src/main/java/org/terst/listen/ui/FurnitureSkipSettingsState.kt b/app/src/main/java/org/terst/listen/ui/FurnitureSkipSettingsState.kt index ad0474b..fa14a2e 100644 --- a/app/src/main/java/org/terst/listen/ui/FurnitureSkipSettingsState.kt +++ b/app/src/main/java/org/terst/listen/ui/FurnitureSkipSettingsState.kt @@ -10,12 +10,18 @@ import kotlinx.coroutines.launch import org.terst.listen.data.Graph /** - * Reads and writes whether a show's own intro, break cue and credits are skipped. + * Reads and writes whether a show's own intro, break cue and credits are skipped, and whether a + * break that is due to be acted on is played fast instead. + * + * Bundled together rather than as two state holders: both describe what happens to a detected + * break and are rendered in the same section, right next to each other, so one `remember` call + * covers both -- and it keeps `SettingsScreen` at the same parameter count rather than growing it + * past detekt's limit for a second, closely related toggle. * * Trivial glue, kept out of the composable that renders it, same reason and same shape as * [rememberSkipServerSettings]: DataStore rather than Room, and a screen that stays a screen. - * Advertising has no toggle here on purpose -- it is always skipped and that is not meant to be - * configurable. + * Advertising has no toggle for *whether* it is acted on -- it is always acted on and that is + * not meant to be configurable; [fastPlayBreaks] only changes *how*. */ @Composable internal fun rememberFurnitureSkipSettings(): FurnitureSkipSettings { @@ -24,17 +30,27 @@ internal fun rememberFurnitureSkipSettings(): FurnitureSkipSettings { val scope = rememberCoroutineScope() val skipShowFurniture by preferences.skipShowFurniture.collectAsState(initial = true) + val fastPlayBreaks by preferences.fastPlayBreaks.collectAsState(initial = false) return FurnitureSkipSettings( skipShowFurniture = skipShowFurniture, onSkipShowFurnitureChange = { value -> scope.launch { preferences.setSkipShowFurniture(value) } }, + fastPlayBreaks = fastPlayBreaks, + onFastPlayBreaksChange = { value -> + scope.launch { preferences.setFastPlayBreaks(value) } + }, ) } -/** Whether the show's own furniture is skipped, as the settings screen reads and writes it. */ +/** + * Whether the show's own furniture is skipped, and whether an acted-on break is played fast + * instead of skipped, as the settings screen reads and writes them. + */ data class FurnitureSkipSettings( val skipShowFurniture: Boolean, val onSkipShowFurnitureChange: (Boolean) -> Unit, + val fastPlayBreaks: Boolean, + val onFastPlayBreaksChange: (Boolean) -> Unit, ) 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 5da9e85..361fcef 100644 --- a/app/src/main/java/org/terst/listen/ui/SettingsScreen.kt +++ b/app/src/main/java/org/terst/listen/ui/SettingsScreen.kt @@ -37,8 +37,8 @@ import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.unit.dp -/** Offered speeds. Beyond 2x speech becomes hard to follow, so the list stops there. */ -private val SPEEDS = listOf(1.0f, 1.25f, 1.5f, 1.75f, 2.0f) +/** Offered speeds. 1.0x stays in the list as the way back to normal. */ +private val SPEEDS = listOf(1.0f, 1.1f, 1.25f, 2.0f) /** Sleep durations in minutes. */ private val SLEEP_MINUTES = listOf(15L, 30L, 45L, 60L) @@ -170,6 +170,14 @@ private fun SkipServerSection(settings: SkipServerSettings, furniture: Furniture detail = "Advertising is always skipped. This covers a show's opening theme, " + "its break cue and its closing credits.", ) + SwitchRow( + label = "Play ads at 3x instead of skipping", + description = "Turn playing ads at 3x instead of skipping them on or off", + checked = furniture.fastPlayBreaks, + onCheckedChange = furniture.onFastPlayBreaksChange, + detail = "Hear a burst of each advertisement rather than jumping over it, so you " + + "can tell something was cut.", + ) } } diff --git a/app/src/test/java/org/terst/listen/playback/MidrollSkipTest.kt b/app/src/test/java/org/terst/listen/playback/MidrollSkipTest.kt index 617ab59..faafe44 100644 --- a/app/src/test/java/org/terst/listen/playback/MidrollSkipTest.kt +++ b/app/src/test/java/org/terst/listen/playback/MidrollSkipTest.kt @@ -1,7 +1,9 @@ package org.terst.listen.playback import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Test import org.terst.listen.data.EpisodeMidrollEntity @@ -95,4 +97,38 @@ class MidrollSkipTest { assertEquals(60_000L, MidrollSkip.skipTarget(ranges, 10_000, skipFurniture = false)) } + + // --- isActedOnBreak: same filter as skipTarget, without a seek target ------------------- + + @Test + fun `outside any range, nothing is acted on`() { + val ranges = listOf(range(60_000, 90_000)) + assertFalse(MidrollSkip.isActedOnBreak(ranges, positionMs = 10_000, skipFurniture = false)) + } + + @Test + fun `inside an ad range it is acted on regardless of furniture skipping`() { + val ranges = listOf(range(60_000, 90_000, "ad")) + assertTrue(MidrollSkip.isActedOnBreak(ranges, positionMs = 75_000, skipFurniture = false)) + } + + @Test + fun `inside the show's own audio it is acted on only when furniture skipping is on`() { + val ranges = listOf(range(0, 98_000, "intro")) + assertFalse(MidrollSkip.isActedOnBreak(ranges, positionMs = 10_000, skipFurniture = false)) + assertTrue(MidrollSkip.isActedOnBreak(ranges, positionMs = 10_000, skipFurniture = true)) + } + + /** An unknown kind must keep being acted on, same as skipTarget. */ + @Test + fun `isActedOnBreak treats an unrecognised kind as advertising`() { + val ranges = listOf(range(0, 60_000, "sponsorship")) + assertTrue(MidrollSkip.isActedOnBreak(ranges, positionMs = 10_000, skipFurniture = false)) + } + + @Test + fun `exactly at the end of a range it is no longer acted on`() { + val ranges = listOf(range(60_000, 90_000)) + assertFalse(MidrollSkip.isActedOnBreak(ranges, positionMs = 90_000, skipFurniture = false)) + } } 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 76d5302..4e3215e 100644 --- a/app/src/test/java/org/terst/listen/playback/PlaybackAdvancerTest.kt +++ b/app/src/test/java/org/terst/listen/playback/PlaybackAdvancerTest.kt @@ -110,6 +110,8 @@ class PlaybackAdvancerTest { onFetch(episode) override suspend fun skipFurniture() = true + + override suspend fun fastPlayBreaks() = false } /** Unwraps the common case so the assertions stay about the thing under test. */ diff --git a/app/src/test/java/org/terst/listen/playback/PlayheadPolicyTest.kt b/app/src/test/java/org/terst/listen/playback/PlayheadPolicyTest.kt index 516df2a..af4f3f8 100644 --- a/app/src/test/java/org/terst/listen/playback/PlayheadPolicyTest.kt +++ b/app/src/test/java/org/terst/listen/playback/PlayheadPolicyTest.kt @@ -30,8 +30,8 @@ class PlayheadPolicyTest { Chapter(startMs = 3 * minute, title = "The interview"), ) - // Six parameters because PlayingContext itself has that many fields worth varying per test; - // a data class wrapper here would just move the long list rather than shorten it. + // Seven parameters because PlayingContext itself has that many fields worth varying per + // test; a data class wrapper here would just move the long list rather than shorten it. @Suppress("LongParameterList") private fun decide( show: SubscriptionEntity? = this.show, @@ -40,8 +40,15 @@ class PlayheadPolicyTest { positionMs: Long, durationMs: Long? = 30 * minute, skipFurniture: Boolean = true, + fastPlayBreaks: Boolean = false, ) = PlayheadPolicy.decide( - PlayingContext(show, chapters, midrolls = midrolls, skipFurniture = skipFurniture), + PlayingContext( + show, + chapters, + midrolls = midrolls, + skipFurniture = skipFurniture, + fastPlayBreaks = fastPlayBreaks, + ), positionMs, durationMs, progress, @@ -176,4 +183,75 @@ class PlayheadPolicyTest { decide(midrolls = midrolls, positionMs = 10_000, skipFurniture = false), ) } + + // --- fast-play mode: a break is raced through at 3x rather than seeked past --------------- + + private val fastSpeed = 3.0f + + @Test + fun `inside an ad break, fast mode requests the fast speed`() { + val midrolls = listOf(EpisodeMidrollEntity("e1", "c1", 60_000, 90_000, "ad")) + + assertEquals( + TickAction.SetSpeed(fastSpeed), + decide(midrolls = midrolls, positionMs = 75_000, fastPlayBreaks = true), + ) + } + + @Test + fun `inside an ad break, skip mode still seeks as today`() { + val midrolls = listOf(EpisodeMidrollEntity("e1", "c1", 60_000, 90_000, "ad")) + + assertEquals( + TickAction.SeekTo(90_000), + decide(midrolls = midrolls, positionMs = 75_000, fastPlayBreaks = false), + ) + } + + /** The furniture preference still decides *whether* a break is acted on, even in fast mode. */ + @Test + fun `inside the show's own intro with furniture skipping off, fast mode does neither`() { + val midrolls = listOf(EpisodeMidrollEntity("e1", "c1", 0, 98_000, "intro")) + + assertEquals( + TickAction.SetSpeed(1.0f), + decide(midrolls = midrolls, positionMs = 10_000, skipFurniture = false, fastPlayBreaks = true), + ) + } + + @Test + fun `outside any break, fast mode restores normal speed rather than leaving it fast`() { + val midrolls = listOf(EpisodeMidrollEntity("e1", "c1", 60_000, 90_000, "ad")) + + assertEquals( + TickAction.SetSpeed(1.0f), + decide(midrolls = midrolls, positionMs = 10_000, fastPlayBreaks = true), + ) + } + + /** Finishing still beats speeding up, same precedence as it beats seeking. */ + @Test + fun `the outro wins over fast-play when a break sits on the finish line`() { + val midrolls = listOf(EpisodeMidrollEntity("e1", "c1", 18 * minute, 22 * minute)) + assertEquals( + TickAction.Finish, + decide( + show = show.copy(skipOutroMs = 10 * minute), + midrolls = midrolls, + positionMs = 19 * minute + 40_000, + fastPlayBreaks = true, + ), + ) + } + + /** An unknown kind must keep being acted on in fast mode too. */ + @Test + fun `an unrecognised kind is sped up in fast mode`() { + val midrolls = listOf(EpisodeMidrollEntity("e1", "c1", 0, 60_000, "sponsorship")) + + assertEquals( + TickAction.SetSpeed(fastSpeed), + decide(midrolls = midrolls, positionMs = 10_000, skipFurniture = false, fastPlayBreaks = true), + ) + } } |