diff options
Diffstat (limited to 'app/src')
9 files changed, 250 insertions, 2 deletions
diff --git a/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt b/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt index 49d3406..a261149 100644 --- a/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt +++ b/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt @@ -856,4 +856,54 @@ class MigrationTest { assertEquals("the crown jewel must survive", 161803L, it.getLong(0)) } } + + /** + * A break gains the name of the detector that found it, and existing rows default to the + * audio scan -- which is where every break in the database came from before the words + * could produce one. + */ + @Test + fun migrate18To19NamesWhereABreakCameFrom() { + helper.createDatabase(name, 18).apply { + execSQL( + """INSERT INTO subscriptions + (id, feedUrl, kind, title, addedAtMs, skipIntroMs, skipOutroMs, dripEnabled, + dripIntervalMs, dripCount, dripOldestFirst, skipAdChapters, bedtime) + VALUES ('s1', 'https://example.com/feed.xml', 'PODCAST', 'A Show', 1000, 0, 0, + 0, 86400000, 1, 1, 1, 0)""" + ) + execSQL( + """INSERT INTO episodes + (id, subscriptionId, guid, title, mediaUrl, mediaType, discoveredAtMs, + positionMs, downloadState) + VALUES ('e1', 's1', 'g1', 'One', 'https://cdn/1.mp3', 'audio/mpeg', 1000, + 314159, 'NONE')""" + ) + execSQL( + """INSERT INTO episode_midrolls (episodeId, candidateId, startMs, endMs, kind, partners) + VALUES ('e1', 'server-0', 60000, 90000, 'ad', 3)""" + ) + close() + } + + val db = helper.runMigrationsAndValidate(name, 19, true, AppDatabase.MIGRATION_18_19) + + db.query("SELECT origin, startMs FROM episode_midrolls WHERE episodeId = 'e1'").use { + it.moveToFirst() + assertEquals("a break that predates the words came from the audio", "audio", it.getString(0)) + assertEquals(60000L, it.getLong(1)) + } + db.execSQL( + "INSERT INTO episode_midrolls (episodeId, candidateId, startMs, endMs, kind, origin) " + + "VALUES ('e1', 'server-1', 600000, 660000, 'ad', 'words')" + ) + db.query("SELECT origin FROM episode_midrolls WHERE candidateId = 'server-1'").use { + it.moveToFirst() + assertEquals("words", it.getString(0)) + } + db.query("SELECT positionMs FROM episodes WHERE id = 'e1'").use { + it.moveToFirst() + assertEquals("the crown jewel must survive", 314159L, it.getLong(0)) + } + } } diff --git a/app/src/main/java/org/terst/listen/data/AppDatabase.kt b/app/src/main/java/org/terst/listen/data/AppDatabase.kt index 5b9219d..dc9c094 100644 --- a/app/src/main/java/org/terst/listen/data/AppDatabase.kt +++ b/app/src/main/java/org/terst/listen/data/AppDatabase.kt @@ -34,7 +34,7 @@ class Converters { EpisodeTranscriptEntity::class, TranscriptCueEntity::class, ], - version = 18, + version = 19, exportSchema = true, ) @TypeConverters(Converters::class) @@ -484,6 +484,21 @@ abstract class AppDatabase : RoomDatabase() { } } + /** + * A break gains the name of the detector that found it. + * + * Existing rows default to the audio scan, because that is where every break in the + * database came from: the words could not produce one until now. Defaulting to + * "unknown" would have been honest about the column and useless about the rows. + */ + val MIGRATION_18_19 = object : Migration(18, 19) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL( + "ALTER TABLE `episode_midrolls` ADD COLUMN `origin` TEXT NOT NULL DEFAULT 'audio'" + ) + } + } + fun get(context: Context): AppDatabase = instance ?: synchronized(this) { instance ?: build(context.applicationContext).also { instance = it } } @@ -497,7 +512,7 @@ abstract class AppDatabase : RoomDatabase() { MIGRATION_6_7, MIGRATION_7_8, MIGRATION_8_9, MIGRATION_9_10, MIGRATION_10_11, MIGRATION_11_12, MIGRATION_12_13, MIGRATION_13_14, MIGRATION_14_15, MIGRATION_15_16, MIGRATION_16_17, - MIGRATION_17_18, + MIGRATION_17_18, MIGRATION_18_19, ) .addCallback(SEED_DEFAULT_FILTERS) // No fallbackToDestructiveMigration. A listener's playback positions and play 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 5a81e5d..1ef3bde 100644 --- a/app/src/main/java/org/terst/listen/data/Entities.kt +++ b/app/src/main/java/org/terst/listen/data/Entities.kt @@ -476,6 +476,16 @@ data class EpisodeMidrollEntity( * before this column existed, and a server that did not say, both read as 0. */ @ColumnInfo(defaultValue = "0") val partners: Int = 0, + /** + * Which detector found this: "audio" for the cross-correlation scan over several episodes' + * waveforms, "words" for subtraction or recurrence over their transcripts, or both names + * where the two agreed. + * + * Shown when a break is judged. The listener deciding whether a skip was right wants to + * know what decided it, and the two are not equally sure of themselves. Rows written before + * the words could find anything default to "audio", which is where they all came from. + */ + @ColumnInfo(defaultValue = "audio") val origin: String = "audio", ) /** 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 3f5d04b..66b5fcc 100644 --- a/app/src/main/java/org/terst/listen/fingerprint/SkipSummary.kt +++ b/app/src/main/java/org/terst/listen/fingerprint/SkipSummary.kt @@ -27,6 +27,14 @@ data class EpisodeSkips( * answers and must stay different. */ val status: String?, + /** + * What decided the breaks -- the audio scan, the words, or both. + * + * Null when nothing is cut. It is here because the listener judging a skip wants to know + * what judged it first, and the two detectors are not equally sure of themselves: one + * compares waveforms across a batch of episodes, the other reads what was said. + */ + val foundBy: String? = null, ) { /** The three numbers on one line, known ones only; null when none is known. */ val numbers: String? = listOfNotNull( @@ -162,8 +170,29 @@ object SkipSummary { .takeIf { ranges.isNotEmpty() } ?.let { clock(it) }, status = if (ranges.isEmpty()) statusWithoutBreaks(episode) else null, + foundBy = foundBy(ranges), ) + /** + * Names the detectors behind an episode's breaks, in a fixed order so the line reads the + * same every time. + * + * A break the two agreed on arrives already saying so, and counts for both. + */ + internal fun foundBy(ranges: List<EpisodeMidrollEntity>): String? { + if (ranges.isEmpty()) return null + val origins = ranges.flatMap { it.origin.split("+") }.map { it.trim() }.toSet() + val named = listOfNotNull( + "the audio".takeIf { "audio" in origins }, + "the words".takeIf { "words" in origins }, + ) + return when (named.size) { + 0 -> null + 1 -> "Found by ${named.single()}" + else -> "Found by ${named.joinToString(" and ")}" + } + } + private fun statusWithoutBreaks(episode: EpisodeEntity): String = when { // Nothing has been downloaded yet, so the skips cannot have been checked either; say what // is actually happening instead of "Not checked yet". diff --git a/app/src/main/java/org/terst/listen/server/ServerSkips.kt b/app/src/main/java/org/terst/listen/server/ServerSkips.kt index 8dcd115..87aa104 100644 --- a/app/src/main/java/org/terst/listen/server/ServerSkips.kt +++ b/app/src/main/java/org/terst/listen/server/ServerSkips.kt @@ -151,6 +151,9 @@ class ServerSkips( endMs = range.endMs, kind = range.kind, partners = range.partners, + // An older server says nothing, and everything it ever found came from + // the audio scan, so that is the honest default rather than "unknown". + origin = range.origin.ifBlank { "audio" }, ) ) } diff --git a/app/src/main/java/org/terst/listen/server/SkipServer.kt b/app/src/main/java/org/terst/listen/server/SkipServer.kt index a05eaf7..68a04ed 100644 --- a/app/src/main/java/org/terst/listen/server/SkipServer.kt +++ b/app/src/main/java/org/terst/listen/server/SkipServer.kt @@ -62,6 +62,11 @@ data class ServerBreak( val kind: String = "ad", /** How many other episodes carried this audio; 0 when the server does not say. */ val partners: Int = 0, + /** + * Which detector found it: "audio", "words", or both names where they agreed. Empty when + * the server did not say, which is what an older server sends. + */ + val origin: String = "", ) /** diff --git a/app/src/main/java/org/terst/listen/ui/EpisodeTimeline.kt b/app/src/main/java/org/terst/listen/ui/EpisodeTimeline.kt index cd8c4b6..6fe77e5 100644 --- a/app/src/main/java/org/terst/listen/ui/EpisodeTimeline.kt +++ b/app/src/main/java/org/terst/listen/ui/EpisodeTimeline.kt @@ -149,6 +149,15 @@ fun SkipReadout(skips: EpisodeSkips, breaks: List<SkipSummary.BreakMarker>) { Column(verticalArrangement = Arrangement.spacedBy(2.dp)) { skips.numbers?.let { Text(it, style = MaterialTheme.typography.bodyMedium) } skips.status?.let { Text(it, style = MaterialTheme.typography.bodySmall) } + // What decided the cut, which is the question anyone has about a skip they disagree + // with. Quieter than the numbers: it explains them rather than competing with them. + skips.foundBy?.let { + Text( + it, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } if (breaks.any { SkipSummary.isAdvertising(it.kind) }) { LegendEntry(MaterialTheme.colorScheme.tertiary, SkipSummary.LEGEND_ADVERTISING) } diff --git a/app/src/test/java/org/terst/listen/fingerprint/FoundBySummaryTest.kt b/app/src/test/java/org/terst/listen/fingerprint/FoundBySummaryTest.kt new file mode 100644 index 0000000..819d2a9 --- /dev/null +++ b/app/src/test/java/org/terst/listen/fingerprint/FoundBySummaryTest.kt @@ -0,0 +1,75 @@ +package org.terst.listen.fingerprint + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test +import org.terst.listen.data.EpisodeMidrollEntity + +/** + * What the episode screen says about which detector decided a skip. + * + * The listener judging a skip wants to know what judged it, and the two are not equally sure + * of themselves: one compares waveforms across a batch of episodes, the other reads what was + * said. A break the two agreed on counts for both, because agreement by entirely different + * means is the strongest thing either can offer. + */ +class FoundBySummaryTest { + + private fun range(origin: String, startMs: Long = 0) = EpisodeMidrollEntity( + episodeId = "e1", candidateId = "c$startMs", startMs = startMs, endMs = startMs + 1000, + origin = origin, + ) + + @Test + fun `breaks from the audio scan alone say so`() { + assertEquals("Found by the audio", SkipSummary.foundBy(listOf(range("audio")))) + } + + @Test + fun `breaks from the words alone say so`() { + assertEquals("Found by the words", SkipSummary.foundBy(listOf(range("words")))) + } + + @Test + fun `an episode with some of each names both`() { + val got = SkipSummary.foundBy(listOf(range("audio"), range("words", 5000))) + assertEquals("Found by the audio and the words", got) + } + + /** A break both detectors found arrives already saying so, and counts for both. */ + @Test + fun `a break the two agreed on names both`() { + assertEquals("Found by the audio and the words", SkipSummary.foundBy(listOf(range("audio+words")))) + } + + /** The order is fixed, so the line does not rearrange itself between episodes. */ + @Test + fun `the order does not depend on which break came first`() { + val one = SkipSummary.foundBy(listOf(range("words"), range("audio", 5000))) + val other = SkipSummary.foundBy(listOf(range("audio"), range("words", 5000))) + assertEquals(one, other) + } + + @Test + fun `nothing cut says nothing`() { + assertNull(SkipSummary.foundBy(emptyList())) + } + + /** A row written by a version that did not record it is not worth a sentence. */ + @Test + fun `an unrecognised origin says nothing`() { + assertNull(SkipSummary.foundBy(listOf(range("")))) + } + + /** + * A readout built without naming a detector explains nothing, which is right: the default + * belongs to the case where nothing has been cut and there is nothing to account for. + */ + @Test + fun `a readout that names no detector says nothing about one`() { + val skips = EpisodeSkips(served = "15:16", declared = "13:08", cut = null, status = "None found") + + assertNull(skips.foundBy) + assertEquals("Served 15:16 \u00b7 Declared 13:08", skips.numbers) + } +} diff --git a/app/src/test/java/org/terst/listen/server/ServerSkipsTest.kt b/app/src/test/java/org/terst/listen/server/ServerSkipsTest.kt index 3b0d671..0d5f686 100644 --- a/app/src/test/java/org/terst/listen/server/ServerSkipsTest.kt +++ b/app/src/test/java/org/terst/listen/server/ServerSkipsTest.kt @@ -516,4 +516,56 @@ class ServerSkipsTest { assertEquals(emptyList<Long>(), transcripts.cueRows.filter { it.advertising }.map { it.startMs }) } + + /* + * A break records which detector found it. + * + * The listener judging a skip wants to know what decided it, and the two detectors are not + * equally sure of themselves: one compares waveforms across a batch of episodes, the other + * reads what was said. + */ + + @Test + fun `records which detector found each break`() = runTest { + val midrolls = FakeMidrollDao() + val transport = StubTransport("""{"status":"ready","audioUrl":"/audio/a?token=t"}""") + + subject(transport, Fakes(midrolls = midrolls)).record( + episode, + ServerEpisode( + ready = true, audioUrl = "x", durationMs = null, introMs = null, outroMs = null, + detected = true, + breaks = listOf( + ServerBreak(60_000, 90_000, "ad", partners = 3, origin = "audio"), + ServerBreak(600_000, 660_000, "ad", origin = "words"), + ), + ), + ) + + assertEquals( + listOf("audio", "words"), + midrolls.rangeRows.sortedBy { it.startMs }.map { it.origin }, + ) + } + + /** + * A server that says nothing found it by the audio scan: that is where every break came + * from before the words could produce one, so "unknown" would be honest about the field + * and useless about the break. + */ + @Test + fun `a break from a server that does not say came from the audio`() = runTest { + val midrolls = FakeMidrollDao() + + subject(StubTransport(null), Fakes(midrolls = midrolls)).record( + episode, + ServerEpisode( + ready = true, audioUrl = "x", durationMs = null, introMs = null, outroMs = null, + detected = true, + breaks = listOf(ServerBreak(60_000, 90_000, "ad")), + ), + ) + + assertEquals("audio", midrolls.rangeRows.single().origin) + } } |