Name each break instead of calling all of it advertising

111 episodes reported 'Ads inserted' about a show's own opening theme and
closing credits.

3 files changed, 116 insertions, 21 deletions

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)
+    }
 }