diff options
| -rw-r--r-- | .coverage-baseline | 2 | ||||
| -rw-r--r-- | app/src/main/java/org/terst/listen/fingerprint/SkipSummary.kt | 49 | ||||
| -rw-r--r-- | app/src/main/java/org/terst/listen/ui/EpisodeScreen.kt | 17 | ||||
| -rw-r--r-- | app/src/test/java/org/terst/listen/fingerprint/SkipSummaryTest.kt | 71 |
4 files changed, 117 insertions, 22 deletions
diff --git a/.coverage-baseline b/.coverage-baseline index d6985ea..f5a155b 100644 --- a/.coverage-baseline +++ b/.coverage-baseline @@ -1 +1 @@ -93.77 +93.80 diff --git a/app/src/main/java/org/terst/listen/fingerprint/SkipSummary.kt b/app/src/main/java/org/terst/listen/fingerprint/SkipSummary.kt index 63b3ec5..47bb248 100644 --- a/app/src/main/java/org/terst/listen/fingerprint/SkipSummary.kt +++ b/app/src/main/java/org/terst/listen/fingerprint/SkipSummary.kt @@ -14,7 +14,8 @@ import org.terst.listen.data.SubscriptionEntity data class EpisodeSkips( val intro: String, val outro: String, - val breaks: List<String>, + /** One line per detected break, or a single status line when none is detected yet. */ + val breaks: List<SkipSummary.BreakLine>, val insertedAds: String?, /** What the feed says the episode is, null if it does not say. */ val declaredLength: String? = null, @@ -53,8 +54,14 @@ object SkipSummary { private const val SECONDS_PER_MINUTE = 60 private const val MINUTES_PER_HOUR = 60 - /** Where one detected break sits in an episode, as fractions of its length. */ - data class BreakMarker(val start: Float, val end: Float) + /** Where one detected break sits in an episode, as fractions of its length, and what it is. */ + data class BreakMarker(val start: Float, val end: Float, val kind: String) + + /** + * One line of the break list on the episode screen: what to call it, from [describeKind], + * and when it falls (a clock range, or a status message when nothing has a range yet). + */ + data class BreakLine(val label: String, val range: String) /** * The detected breaks as positions on a 0..1 timeline, for the episode screen's scrubber. @@ -71,10 +78,24 @@ object SkipSummary { BreakMarker( start = (it.startMs.toFloat() / durationMs).coerceIn(0f, 1f), end = (it.endMs.toFloat() / durationMs).coerceIn(0f, 1f), + kind = it.kind, ) } } + /** + * What to call a break on screen. + * + * Anything unrecognised is an ad break, matching what playback does with it: a kind added by + * a newer server is still skipped, so it must not appear nameless. + */ + fun describeKind(kind: String): String = when (kind) { + "intro" -> "Show intro" + "cue" -> "Break cue" + "outro" -> "Credits" + else -> "Ad break" + } + /** One end of a show's episodes -- its intro or its outro. */ fun candidateState(state: ShowFingerprintEntity?, threshold: Int): String { // No reference clip means no candidate: either nothing has been processed, or a single @@ -116,9 +137,15 @@ object SkipSummary { else -> "None" } - /** Detected ad breaks in one episode, earliest first, as clock times you can scrub to. */ - private fun ranges(ranges: List<EpisodeMidrollEntity>): List<String> = - ranges.sortedBy { it.startMs }.map { "${clock(it.startMs)} - ${clock(it.endMs)}" } + /** + * Detected breaks in one episode, earliest first, each named by [describeKind] rather than + * lumped under one "Ad breaks" header -- the whole point of this is that they are not all + * advertising. + */ + private fun breakLines(ranges: List<EpisodeMidrollEntity>): List<BreakLine> = + ranges.sortedBy { it.startMs }.map { + BreakLine(describeKind(it.kind), "${clock(it.startMs)} - ${clock(it.endMs)}") + } /** * Everything the episode screen says about skipping, decided here rather than in the @@ -129,7 +156,7 @@ object SkipSummary { show: SubscriptionEntity?, ranges: List<EpisodeMidrollEntity>, ): EpisodeSkips { - val breaks = ranges(ranges) + val breaks = breakLines(ranges) return EpisodeSkips( insertedAds = episode.insertedAdMs?.let { clock(it) }, declaredLength = episode.durationMs?.let { clock(it) }, @@ -142,9 +169,11 @@ object SkipSummary { breaks = when { breaks.isNotEmpty() -> breaks // Distinct from "none found": scanning runs nightly while charging, so a - // freshly downloaded episode has genuinely not been looked at yet. - episode.midrollCheckedAtMs == null -> listOf("Not checked yet") - else -> listOf("None found") + // freshly downloaded episode has genuinely not been looked at yet. Neither + // status line names a kind, so both use a generic label rather than one of + // describeKind's four. + episode.midrollCheckedAtMs == null -> listOf(BreakLine("Breaks", "Not checked yet")) + else -> listOf(BreakLine("Breaks", "None found")) }, ) } diff --git a/app/src/main/java/org/terst/listen/ui/EpisodeScreen.kt b/app/src/main/java/org/terst/listen/ui/EpisodeScreen.kt index 649d24f..9baff97 100644 --- a/app/src/main/java/org/terst/listen/ui/EpisodeScreen.kt +++ b/app/src/main/java/org/terst/listen/ui/EpisodeScreen.kt @@ -293,11 +293,17 @@ private fun EpisodeScrubber(playback: EpisodePlayback, actions: EpisodeDetailAct } } -/** The detected ad breaks as bands across the track. */ +/** + * The detected breaks as bands across the track, advertising in one colour and the show's own + * audio (intro, cue, outro) in another -- the same distinction [SkipSummary.describeKind] names + * in words, so a kind this version does not recognise draws as advertising too, matching what + * playback does with it. + */ @Composable private fun BreakMarks(breaks: List<SkipSummary.BreakMarker>, modifier: Modifier) { if (breaks.isEmpty()) return - val colour = MaterialTheme.colorScheme.tertiary + val adColour = MaterialTheme.colorScheme.tertiary + val furnitureColour = MaterialTheme.colorScheme.secondary Canvas(modifier.fillMaxWidth().height(BREAK_MARK_HEIGHT)) { // Inset by the thumb's radius at each end: the slider's track is laid out inside it, so // marks drawn edge to edge would sit a thumb-width off the positions they describe. @@ -306,8 +312,9 @@ private fun BreakMarks(breaks: List<SkipSummary.BreakMarker>, modifier: Modifier breaks.forEach { mark -> val left = inset + usable * mark.start val right = inset + usable * mark.end + val isAd = SkipSummary.describeKind(mark.kind) == "Ad break" drawRect( - color = colour, + color = if (isAd) adColour else furnitureColour, topLeft = Offset(left, 0f), size = Size((right - left).coerceAtLeast(MIN_BREAK_MARK_PX), size.height), ) @@ -342,9 +349,7 @@ private fun SkipReadout(skips: EpisodeSkips) { skips.insertedAds?.let { ReadoutRow("Ads inserted", "~$it") } ReadoutRow("Intro", skips.intro) ReadoutRow("Outro", skips.outro) - skips.breaks.forEachIndexed { index, line -> - ReadoutRow(if (index == 0) "Ad breaks" else "", line) - } + skips.breaks.forEach { line -> ReadoutRow(line.label, line.range) } skips.breaksTotal?.let { ReadoutRow("Breaks found", it) } } } diff --git a/app/src/test/java/org/terst/listen/fingerprint/SkipSummaryTest.kt b/app/src/test/java/org/terst/listen/fingerprint/SkipSummaryTest.kt index efb01ae..b1decbd 100644 --- a/app/src/test/java/org/terst/listen/fingerprint/SkipSummaryTest.kt +++ b/app/src/test/java/org/terst/listen/fingerprint/SkipSummaryTest.kt @@ -154,7 +154,10 @@ class SkipSummaryTest { ) assertEquals( - listOf("1:01 - 1:31", "12:30 - 13:45"), + listOf( + SkipSummary.BreakLine("Ad break", "1:01 - 1:31"), + SkipSummary.BreakLine("Ad break", "12:30 - 13:45"), + ), SkipSummary.episode(episodeRow(), showRow(), ranges).breaks, ) } @@ -170,7 +173,7 @@ class SkipSummaryTest { fun anUncheckedEpisodeSaysSoRatherThanClaimingNoBreaks() { val skips = SkipSummary.episode(episodeRow(), showRow(), ranges = emptyList()) - assertEquals(listOf("Not checked yet"), skips.breaks) + assertEquals(listOf(SkipSummary.BreakLine("Breaks", "Not checked yet")), skips.breaks) } @Test @@ -181,7 +184,7 @@ class SkipSummaryTest { ranges = emptyList(), ) - assertEquals(listOf("None found"), skips.breaks) + assertEquals(listOf(SkipSummary.BreakLine("Breaks", "None found")), skips.breaks) } @Test @@ -196,7 +199,33 @@ class SkipSummaryTest { assertEquals("47s, detected", skips.intro) assertEquals("20s, set by hand", skips.outro) - assertEquals(listOf("1:01 - 1:31"), skips.breaks) + assertEquals(listOf(SkipSummary.BreakLine("Ad break", "1:01 - 1:31")), skips.breaks) + } + + /** + * The point of this whole task: a break list with a mix of kinds names each one by what it + * actually is, instead of calling all of it "Ad breaks". + */ + @Test + fun `breaks of different kinds are each named for what they are`() { + val skips = SkipSummary.episode( + episodeRow(), + showRow(), + ranges = listOf( + EpisodeMidrollEntity("e1", "a", startMs = 0, endMs = 20_000, kind = "intro"), + EpisodeMidrollEntity("e1", "b", startMs = 300_000, endMs = 420_000, kind = "ad"), + EpisodeMidrollEntity("e1", "c", startMs = 500_000, endMs = 520_000, kind = "outro"), + ), + ) + + assertEquals( + listOf( + SkipSummary.BreakLine("Show intro", "0:00 - 0:20"), + SkipSummary.BreakLine("Ad break", "5:00 - 7:00"), + SkipSummary.BreakLine("Credits", "8:20 - 8:40"), + ), + skips.breaks, + ) } @Test @@ -206,7 +235,7 @@ class SkipSummaryTest { ) assertEquals( - listOf("1:02:03 - 1:03:00"), + listOf(SkipSummary.BreakLine("Ad break", "1:02:03 - 1:03:00")), SkipSummary.episode(episodeRow(), showRow(), ranges).breaks, ) } @@ -341,4 +370,36 @@ class SkipSummaryTest { assertEquals(emptyList<SkipSummary.BreakMarker>(), markers) } + + /** + * The readout is the only explanation of this feature the app ever gives, so the wording is + * pinned here. Calling a show's own opening "Ads inserted" is what this whole change is for + * -- 111 episodes said exactly that about audio no advertiser paid for. + */ + @Test + fun `names each kind of break in words`() { + assertEquals("Ad break", SkipSummary.describeKind("ad")) + assertEquals("Show intro", SkipSummary.describeKind("intro")) + assertEquals("Break cue", SkipSummary.describeKind("cue")) + assertEquals("Credits", SkipSummary.describeKind("outro")) + } + + /** A kind this version does not know is still skipped, so it must still have a name. */ + @Test + fun `an unrecognised kind is described as an ad break`() { + assertEquals("Ad break", SkipSummary.describeKind("sponsorship")) + } + + @Test + fun `break markers carry the kind so the scrubber can distinguish them`() { + val ranges = listOf( + EpisodeMidrollEntity("e1", "a", 0, 98_000, "intro"), + EpisodeMidrollEntity("e1", "b", 300_000, 420_000, "ad"), + ) + + val got = SkipSummary.breakMarkers(ranges, 600_000) + + assertEquals("intro", got[0].kind) + assertEquals("ad", got[1].kind) + } } |