diff options
Diffstat (limited to 'app/src')
7 files changed, 85 insertions, 22 deletions
diff --git a/app/src/main/java/org/terst/listen/playback/AdJump.kt b/app/src/main/java/org/terst/listen/playback/AdJump.kt index 99622c1..baf09ed 100644 --- a/app/src/main/java/org/terst/listen/playback/AdJump.kt +++ b/app/src/main/java/org/terst/listen/playback/AdJump.kt @@ -7,19 +7,18 @@ import org.terst.listen.data.EpisodeMidrollEntity * How far "this is an ad" jumps. * * The press means "get me past this advertisement", so the jump is the best estimate available - * at that moment rather than a constant. If detection already found a break here, its end is a - * measured answer and is used. Otherwise the default stands in. + * at that moment: if detection already found a break here, its end is a measured answer and is + * used. * - * The size of that default is the trade between too small -- several presses to clear one pod -- - * and too large, which lands him in the programme and makes him correct constantly. Advertising - * is sold in roughly 15-second units grouped into pods, and he presses partway through one, so - * the typical *remaining* duration is what this estimates. Erring small is deliberate: a second - * press costs one tap and improves the label, while over-shooting costs him content. + * With nothing measured it falls back to the forward button's own amount, and deliberately + * introduces no timing of its own. Skipping forward is already what the listener does to an + * advertisement; this button does the same distance and additionally says why. One number to + * know, and pressing again goes the same distance again -- which also extends the label. */ object AdJump { - /** Three 15-second units. See the class comment for why it is not larger. */ - const val DEFAULT_JUMP_MS = 45_000L + /** The forward button's amount. Not a constant of this feature's own; see [SeekAmounts]. */ + const val DEFAULT_JUMP_MS = SeekAmounts.FORWARD_MS fun targetFor( ranges: List<EpisodeMidrollEntity>, diff --git a/app/src/main/java/org/terst/listen/playback/NotificationControls.kt b/app/src/main/java/org/terst/listen/playback/NotificationControls.kt index 21f3959..1dba3df 100644 --- a/app/src/main/java/org/terst/listen/playback/NotificationControls.kt +++ b/app/src/main/java/org/terst/listen/playback/NotificationControls.kt @@ -41,17 +41,18 @@ object NotificationControls { * Media3 ships numbered icons for 5, 10, 15 and 30 seconds only. * * Back is 15, so it gets the numbered icon. Forward is 25 and there is no 25 icon, so it - * gets the plain arrow rather than one captioned with a number the button doesn't do. + * gets the plain arrow rather than one captioned with a number the button doesn't do. The + * captions come from [SeekAmounts] for the same reason. */ fun mediaButtonPreferences(): List<CommandButton> = listOf( CommandButton.Builder() - .setDisplayName("Back 15 seconds") + .setDisplayName(SeekAmounts.backLabel) .setIconResId(androidx.media3.session.R.drawable.media3_icon_skip_back_15) .setSessionCommand(seekBackCommand) .setSlots(CommandButton.SLOT_BACK) .build(), CommandButton.Builder() - .setDisplayName("Forward 25 seconds") + .setDisplayName(SeekAmounts.forwardLabel) .setIconResId(androidx.media3.session.R.drawable.media3_icon_skip_forward) .setSessionCommand(seekForwardCommand) .setSlots(CommandButton.SLOT_FORWARD) 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 3a2543b..7af4e0a 100644 --- a/app/src/main/java/org/terst/listen/playback/PlaybackService.kt +++ b/app/src/main/java/org/terst/listen/playback/PlaybackService.kt @@ -66,10 +66,10 @@ class PlaybackService : MediaLibraryService() { /* handleAudioFocus = */ true, ) .setHandleAudioBecomingNoisy(true) - // These drive the notification, lock screen, Bluetooth remote and car controls. - // Asymmetric on purpose: back catches a missed sentence, forward steps over an ad. - .setSeekBackIncrementMs(SEEK_BACK_MS) - .setSeekForwardIncrementMs(SEEK_FORWARD_MS) + // These drive the notification, lock screen, Bluetooth remote and car controls -- + // and, through SeekAmounts, the in-app transport and the button captions too. + .setSeekBackIncrementMs(SeekAmounts.BACK_MS) + .setSeekForwardIncrementMs(SeekAmounts.FORWARD_MS) .build() player = exo @@ -345,8 +345,6 @@ class PlaybackService : MediaLibraryService() { private companion object { const val TAG = "ListenPlayback" const val POSITION_TICK_MS = 2_000L - const val SEEK_BACK_MS = 15_000L - const val SEEK_FORWARD_MS = 25_000L } } diff --git a/app/src/main/java/org/terst/listen/playback/SeekAmounts.kt b/app/src/main/java/org/terst/listen/playback/SeekAmounts.kt new file mode 100644 index 0000000..a02a56b --- /dev/null +++ b/app/src/main/java/org/terst/listen/playback/SeekAmounts.kt @@ -0,0 +1,24 @@ +package org.terst.listen.playback + +/** + * How far a skip goes. One definition, because there were three. + * + * The player's increments, the episode screen's transport buttons and the notification button's + * caption each carried their own copy of 15 and 25 seconds, and the "this is an ad" jump nearly + * became a fourth with a different number again. A listener has one idea of how far a skip goes; + * every control that skips uses this. + * + * Asymmetric on purpose: back catches a missed sentence, forward steps over an advertisement. + */ +object SeekAmounts { + + const val BACK_MS = 15_000L + const val FORWARD_MS = 25_000L + + /** + * Captions, derived rather than written out, so a caption cannot name a number the button + * does not do. + */ + val backLabel: String = "Back ${BACK_MS / 1_000} seconds" + val forwardLabel: String = "Forward ${FORWARD_MS / 1_000} seconds" +} diff --git a/app/src/main/java/org/terst/listen/ui/Transport.kt b/app/src/main/java/org/terst/listen/ui/Transport.kt index 80c1018..506770d 100644 --- a/app/src/main/java/org/terst/listen/ui/Transport.kt +++ b/app/src/main/java/org/terst/listen/ui/Transport.kt @@ -1,10 +1,14 @@ package org.terst.listen.ui +import org.terst.listen.playback.SeekAmounts + /** The episode screen's transport arithmetic, kept out of the composable so it can be tested. */ object Transport { - const val BACK_MS = 15_000L - const val FORWARD_MS = 25_000L + // The player's own increments, not a second opinion about them: the screen's buttons and + // the notification's have to move the playhead by the same amount. + const val BACK_MS = SeekAmounts.BACK_MS + const val FORWARD_MS = SeekAmounts.FORWARD_MS /** Where a skip lands, kept inside the episode. */ fun stepped(positionMs: Long, deltaMs: Long, durationMs: Long?): Long { diff --git a/app/src/test/java/org/terst/listen/playback/AdJumpTest.kt b/app/src/test/java/org/terst/listen/playback/AdJumpTest.kt index 3421aa2..d2b3138 100644 --- a/app/src/test/java/org/terst/listen/playback/AdJumpTest.kt +++ b/app/src/test/java/org/terst/listen/playback/AdJumpTest.kt @@ -87,7 +87,7 @@ class AdJumpTest { } @Test - fun `the default is a plausible remaining advertisement, in whole ad units`() { - assertEquals(0L, AdJump.DEFAULT_JUMP_MS % 15_000L) + fun `the default jump is the forward button's amount, not a number of its own`() { + assertEquals(SeekAmounts.FORWARD_MS, AdJump.DEFAULT_JUMP_MS) } } diff --git a/app/src/test/java/org/terst/listen/playback/SeekAmountsTest.kt b/app/src/test/java/org/terst/listen/playback/SeekAmountsTest.kt new file mode 100644 index 0000000..8d0865c --- /dev/null +++ b/app/src/test/java/org/terst/listen/playback/SeekAmountsTest.kt @@ -0,0 +1,37 @@ +package org.terst.listen.playback + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * One definition of how far a skip goes. + * + * These were duplicated in three places -- the player's increments, the episode screen's + * transport, and the notification button's caption -- and a fourth nearly appeared. + */ +class SeekAmountsTest { + + @Test + fun `the amounts are what they have always been`() { + assertEquals(15_000L, SeekAmounts.BACK_MS) + assertEquals(25_000L, SeekAmounts.FORWARD_MS) + } + + @Test + fun `the episode screen skips by the same amounts as the player`() { + assertEquals(SeekAmounts.BACK_MS, org.terst.listen.ui.Transport.BACK_MS) + assertEquals(SeekAmounts.FORWARD_MS, org.terst.listen.ui.Transport.FORWARD_MS) + } + + @Test + fun `a notification caption states the amount it actually seeks`() { + assertEquals("Back 15 seconds", SeekAmounts.backLabel) + assertEquals("Forward 25 seconds", SeekAmounts.forwardLabel) + } + + @Test + fun `back is shorter than forward -- back catches a missed sentence, forward steps over an ad`() { + assertTrue(SeekAmounts.BACK_MS < SeekAmounts.FORWARD_MS) + } +} |