diff options
Diffstat (limited to 'app/src')
7 files changed, 252 insertions, 1 deletions
diff --git a/app/src/main/java/org/terst/listen/data/Daos.kt b/app/src/main/java/org/terst/listen/data/Daos.kt index 2ca227c..8dbaa32 100644 --- a/app/src/main/java/org/terst/listen/data/Daos.kt +++ b/app/src/main/java/org/terst/listen/data/Daos.kt @@ -386,6 +386,17 @@ interface MidrollDao { @Insert(onConflict = OnConflictStrategy.REPLACE) suspend fun upsertRange(range: EpisodeMidrollEntity) + /** + * Drops every detected range for one episode, so a fresh answer replaces the previous one + * rather than adding to it. + * + * The skip server is the authority on what an episode's ad breaks are, and advertising + * rotates between fetches: left to accumulate, the phone would go on skipping stretches + * that are no longer advertisements. + */ + @Query("DELETE FROM episode_midrolls WHERE episodeId = :episodeId") + suspend fun deleteRanges(episodeId: String) + /** Downloaded episodes of this show [MidrollDetector][org.terst.listen.fingerprint.MidrollDetector] * has never looked at, oldest download first -- so a backlog of downloads is worked through * in the order they actually landed, not newest-first forever leaving the rest unchecked. */ 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 e425ccf..07ffab2 100644 --- a/app/src/main/java/org/terst/listen/data/Repositories.kt +++ b/app/src/main/java/org/terst/listen/data/Repositories.kt @@ -134,7 +134,7 @@ class AppGraph(private val context: Context) { episodes, libraryDir, ChaquopyAudioSource(context), - serverAudio = ServerSkips(skipServer, subscriptions, fingerprints, playbackState), + serverAudio = ServerSkips(skipServer, subscriptions, fingerprints, playbackState, midrolls), ) } 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 edff77b..ed70d07 100644 --- a/app/src/main/java/org/terst/listen/server/ServerSkips.kt +++ b/app/src/main/java/org/terst/listen/server/ServerSkips.kt @@ -1,6 +1,8 @@ package org.terst.listen.server import org.terst.listen.data.EpisodeEntity +import org.terst.listen.data.EpisodeMidrollEntity +import org.terst.listen.data.MidrollDao import org.terst.listen.data.PlaybackStateDao import org.terst.listen.data.ShowFingerprintDao import org.terst.listen.data.SubscriptionDao @@ -18,6 +20,7 @@ class ServerSkips( private val subscriptions: SubscriptionDao, private val fingerprints: ShowFingerprintDao, private val playbackState: PlaybackStateDao, + private val midrolls: MidrollDao, ) : ServerAudio { /** @@ -52,5 +55,36 @@ class ServerSkips( // dynamic ad insertion, and this is the number that places the outro offset and decides // when an episode counts as finished. info.durationMs?.let { playbackState.updateDuration(episode.id, it) } + + recordBreaks(episode, info) + } + + /** + * Stores the ad breaks as skip ranges, replacing whatever was there. + * + * Into `episode_midrolls`, which [org.terst.listen.playback.MidrollSkip] and + * [org.terst.listen.playback.PlayheadPolicy] have read since the on-device detector existed + * -- so playback needs no changes at all and there is no migration. Only the origin of the + * ranges has moved. + * + * Replaced rather than merged because the server is the authority and advertising rotates + * between fetches: accumulating them would leave the phone skipping stretches that are no + * longer advertisements. + */ + private suspend fun recordBreaks(episode: EpisodeEntity, info: ServerEpisode) { + midrolls.deleteRanges(episode.id) + info.breaks.forEachIndexed { index, range -> + midrolls.upsertRange( + EpisodeMidrollEntity( + episodeId = episode.id, + // The server does not name its breaks, and nothing on the phone needs it + // to: the id only has to be stable within an episode so a replacement + // overwrites rather than duplicates. + candidateId = "server-$index", + startMs = range.startMs, + endMs = range.endMs, + ) + ) + } } } 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 a480fb5..2e013de 100644 --- a/app/src/main/java/org/terst/listen/server/SkipServer.kt +++ b/app/src/main/java/org/terst/listen/server/SkipServer.kt @@ -23,6 +23,14 @@ interface SkipTransport { suspend fun post(url: String, token: String, body: String): Boolean } +/** One stretch of an episode to skip. Deserialised straight off the wire -- the shape the + * server sends and the shape the app wants are the same two numbers. */ +@Serializable +data class ServerBreak( + @SerialName("startMs") val startMs: Long = 0, + @SerialName("endMs") val endMs: Long = 0, +) + /** What the server knows about one episode. */ data class ServerEpisode( /** Whether the audio can be played from the server now. */ @@ -39,6 +47,32 @@ data class ServerEpisode( * found yet" is the mistake the app's own readout used to make. */ val detected: Boolean, + /** + * The advertising this episode shares with the others mirrored alongside it. + * + * The thing the server detects reliably, and the reason intro offsets are no longer + * published: a segment "confirmed" on 8 of 8 episodes turned out to be an advertisement, + * proved by searching for it in an ad-free copy of the same episode, where it scored + * r = 0.0621. Mirroring eight episodes in one minute makes them share whatever campaign is + * running, which destroys the independence intro detection needs -- and makes ad detection + * work. + */ + val breaks: List<ServerBreak> = emptyList(), + /** + * What the scan accounted for, against what the feed's declared length implies is there. + * + * The second figure comes from the feed and the file rather than from the detector, so it + * is the only number able to contradict a scan. Measured on Radiolab: 53:29 found against + * 59:39 implied. + */ + val foundAdMs: Long? = null, + val insertedAdMs: Long? = null, + /** + * Matches were found but were too extensive to be advertising, so none were published. + * Distinct from nothing having matched, and kept distinct because collapsing those two is + * what made the app's skip readout useless. + */ + val adsRefused: Boolean = false, ) /** @@ -111,6 +145,13 @@ class SkipServer( introMs = parsed.introMs, outroMs = parsed.outroMs, detected = parsed.detected, + // A zero-length or inverted range would be a skip to nowhere, or backwards: + // playback seeks to a break's end, so an inverted one would move the playhead back + // and the episode would never finish. + breaks = parsed.breaks.filter { it.endMs > it.startMs }, + foundAdMs = parsed.foundAdMs, + insertedAdMs = parsed.insertedAdMs, + adsRefused = parsed.adsRefused, ) } @@ -131,8 +172,13 @@ class SkipServer( @SerialName("introMs") val introMs: Long? = null, @SerialName("outroMs") val outroMs: Long? = null, val detected: Boolean = false, + val breaks: List<ServerBreak> = emptyList(), + @SerialName("foundAdMs") val foundAdMs: Long? = null, + @SerialName("insertedAdMs") val insertedAdMs: Long? = null, + @SerialName("adsRefused") val adsRefused: Boolean = false, ) + private companion object { /** Tolerant of fields this app does not know about, so the server can grow. */ val lenient = Json { ignoreUnknownKeys = true; isLenient = true } diff --git a/app/src/test/java/org/terst/listen/data/FakeDaos.kt b/app/src/test/java/org/terst/listen/data/FakeDaos.kt index 14d1394..a1dab05 100644 --- a/app/src/test/java/org/terst/listen/data/FakeDaos.kt +++ b/app/src/test/java/org/terst/listen/data/FakeDaos.kt @@ -345,6 +345,10 @@ class FakeMidrollDao : MidrollDao { rangeRows += range } + override suspend fun deleteRanges(episodeId: String) { + rangeRows.removeAll { it.episodeId == episodeId } + } + override suspend fun getUncheckedDownloaded(subscriptionId: String, downloaded: DownloadState) = episodeRows.filter { it.subscriptionId == subscriptionId && it.downloadState == downloaded && it.midrollCheckedAtMs == null 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 57c03f3..11bba35 100644 --- a/app/src/test/java/org/terst/listen/server/ServerSkipsTest.kt +++ b/app/src/test/java/org/terst/listen/server/ServerSkipsTest.kt @@ -7,6 +7,7 @@ import org.junit.Assert.assertTrue import org.junit.Test import org.terst.listen.data.EpisodeEntity import org.terst.listen.data.FakeEpisodeDao +import org.terst.listen.data.FakeMidrollDao import org.terst.listen.data.FakePlaybackStateDao import org.terst.listen.data.FakeShowFingerprintDao import org.terst.listen.data.FakeSubscriptionDao @@ -53,6 +54,7 @@ class ServerSkipsTest { subscriptions: FakeSubscriptionDao = FakeSubscriptionDao().apply { rows += show }, episodes: FakeEpisodeDao = FakeEpisodeDao().apply { rows += episode }, fingerprints: FakeShowFingerprintDao = FakeShowFingerprintDao(), + midrolls: FakeMidrollDao = FakeMidrollDao(), ): ServerSkips { // Both fakes write through to the same episode rows, the way the real foreign table // does, so a test sees the value where production would put it. @@ -62,6 +64,7 @@ class ServerSkipsTest { subscriptions, fingerprints, FakePlaybackStateDao(episodes.rows), + midrolls, ) } @@ -152,4 +155,78 @@ class ServerSkipsTest { assertEquals(0L, episodes.rows.single { it.id == "e1" }.detectedIntroMs) } + + /* + * Ad breaks go into the table playback already reads. + * + * MidrollSkip and PlayheadPolicy have consumed episode_midrolls since the on-device + * detector existed, so nothing in playback changes -- only where the ranges come from. That + * is also why there is no migration: the columns and the table were already there. + */ + @Test + fun `records the ad breaks as skip ranges`() = runTest { + val midrolls = FakeMidrollDao() + + subject(StubTransport(null), midrolls = midrolls).record( + episode, + ServerEpisode( + ready = true, audioUrl = "x", durationMs = null, introMs = null, outroMs = null, + detected = true, + breaks = listOf(ServerBreak(0, 142_000), ServerBreak(900_000, 1_020_000)), + ), + ) + + val ranges = midrolls.rangeRows.filter { it.episodeId == "e1" }.sortedBy { it.startMs } + assertEquals(2, ranges.size) + assertEquals(0L, ranges[0].startMs) + assertEquals(142_000L, ranges[0].endMs) + assertEquals(900_000L, ranges[1].startMs) + assertEquals(1_020_000L, ranges[1].endMs) + } + + /** + * The server is the authority on an episode's breaks, so a fresh answer replaces the old + * one. Left to accumulate, a campaign rotating between fetches would leave the phone + * skipping stretches that are no longer advertising. + */ + @Test + fun `replaces the previous breaks rather than adding to them`() = runTest { + val midrolls = FakeMidrollDao() + val skips = subject(StubTransport(null), midrolls = midrolls) + + skips.record( + episode, + ServerEpisode(ready = true, audioUrl = "x", durationMs = null, introMs = null, + outroMs = null, detected = true, breaks = listOf(ServerBreak(0, 60_000))), + ) + skips.record( + episode, + ServerEpisode(ready = true, audioUrl = "x", durationMs = null, introMs = null, + outroMs = null, detected = true, breaks = listOf(ServerBreak(10_000, 70_000))), + ) + + val ranges = midrolls.rangeRows.filter { it.episodeId == "e1" } + assertEquals(1, ranges.size) + assertEquals(10_000L, ranges.single().startMs) + } + + /** An episode the server published no breaks for must not keep stale ones. */ + @Test + fun `clears the breaks when the server published none`() = runTest { + val midrolls = FakeMidrollDao() + val skips = subject(StubTransport(null), midrolls = midrolls) + + skips.record( + episode, + ServerEpisode(ready = true, audioUrl = "x", durationMs = null, introMs = null, + outroMs = null, detected = true, breaks = listOf(ServerBreak(0, 60_000))), + ) + skips.record( + episode, + ServerEpisode(ready = true, audioUrl = "x", durationMs = null, introMs = null, + outroMs = null, detected = true, breaks = emptyList()), + ) + + assertTrue(midrolls.rangeRows.none { it.episodeId == "e1" }) + } } diff --git a/app/src/test/java/org/terst/listen/server/SkipServerTest.kt b/app/src/test/java/org/terst/listen/server/SkipServerTest.kt index bdf9d6a..320306a 100644 --- a/app/src/test/java/org/terst/listen/server/SkipServerTest.kt +++ b/app/src/test/java/org/terst/listen/server/SkipServerTest.kt @@ -191,4 +191,83 @@ class SkipServerTest { assertEquals("https://listen.terst.org/api/subscriptions", transport.posts.single().first) } + + /* + * Ad breaks, which are what the server actually detects reliably. + * + * Intro detection on the server reported a segment confirmed on 8 of 8 Radiolab episodes + * and was wrong -- the segment was an advertisement, proved by searching for it in an + * ad-free copy of the same episode, where it scored r = 0.0621. Mirroring eight episodes in + * one minute makes them share whatever ad campaign is running, which destroys the + * independence the confirmation threshold needs. + * + * The same property makes ad detection reliable, and unlike intro detection it can be + * checked against something the detector did not produce: the feed declares the ad-free + * length, the file is longer, and the difference is the advertising. Measured on Radiolab, + * the scan accounted for 53:29 of the 59:39 that gap implies. + */ + @Test + fun `reads the ad breaks the server found`() = runTest { + val transport = FakeTransport().apply { + response = """ + {"status":"ready","audioUrl":"/audio/abc","detected":true, + "breaks":[{"startMs":0,"endMs":142000},{"startMs":900000,"endMs":1020000}], + "foundAdMs":391000,"insertedAdMs":373000} + """.trimIndent() + } + + val got = server(transport).episode("https://f", "g1", "", "") + + assertEquals(2, got?.breaks?.size) + assertEquals(0L, got?.breaks?.get(0)?.startMs) + assertEquals(142000L, got?.breaks?.get(0)?.endMs) + assertEquals(900000L, got?.breaks?.get(1)?.startMs) + assertEquals(391000L, got?.foundAdMs) + assertEquals(373000L, got?.insertedAdMs) + } + + @Test + fun `an episode with no breaks reports an empty list rather than null`() = runTest { + val transport = FakeTransport().apply { response = """{"status":"ready","audioUrl":"/audio/abc"}""" } + + assertEquals(emptyList<ServerBreak>(), server(transport).episode("https://f", "g1", "", "")?.breaks) + } + + /** + * Matches were found but were too extensive to be advertising, so the server published none. + * A different state from nothing having matched, and the app must be able to tell them + * apart -- collapsing the two is exactly what made the skip readout useless before. + */ + @Test + fun `a refusal is reported distinctly from having found nothing`() = runTest { + val transport = FakeTransport().apply { + response = """{"status":"ready","audioUrl":"/audio/abc","detected":true,"adsRefused":true}""" + } + + val got = server(transport).episode("https://f", "g1", "", "") + + assertTrue("refusal lost", got?.adsRefused == true) + assertEquals(emptyList<ServerBreak>(), got?.breaks) + } + + /** + * A zero-length or inverted range would be a skip to nowhere, or backwards. Dropped here + * rather than trusted: playback seeks to a break's end, so an inverted one would move the + * playhead backwards and the episode would never finish. + */ + @Test + fun `discards a break that does not describe a real stretch`() = runTest { + val transport = FakeTransport().apply { + response = """ + {"status":"ready","audioUrl":"/audio/abc", + "breaks":[{"startMs":1000,"endMs":1000},{"startMs":5000,"endMs":2000}, + {"startMs":7000,"endMs":9000}]} + """.trimIndent() + } + + val got = server(transport).episode("https://f", "g1", "", "") + + assertEquals(1, got?.breaks?.size) + assertEquals(7000L, got?.breaks?.single()?.startMs) + } } |