From 7847e13c3fab23edfda096396e4dbe01df3471c3 Mon Sep 17 00:00:00 2001 From: Peter Stone Date: Tue, 6 Oct 2026 07:31:53 +0000 Subject: Follow the data: detect the advertising, not the intro The property that broke intro detection is the property that makes this work. Eight episodes mirrored within one minute are maximally dependent, because the ad server inserts whatever campaign is running into all of them identically. For intro detection that destroys the independence the confirmation threshold needs -- which is how it came to confirm an advertisement on 8 of 8 episodes. For ad detection it is the signal. And unlike intro detection it is checkable against a number the detector does not produce: the feed declares the ad-free length, the file served is longer, and the difference is the advertising. **Measured on the live deployment, Radiolab's eight mirrored episodes: the scan accounted for 53:29 of the 59:39 the length gap implies, with every individual episode between 71% and 105%.** That is the first time detection in this project has been checked against external ground truth and agreed with it. The old sliding search could never have done this -- one full-length correlation per window position, 199 seconds for a 240-second clip. BestLags computes the cross-correlation over every lag in a single FFT pair, which says where two recordings align; MatchedRuns then walks the extent of the agreement directly, window by window, with no transform. Sized against measurements rather than guesses: 2^22 is 1.69s, so a 70-minute episode decimated to 250Hz is about five seconds a pair. Silence is explicitly not agreement, or separate advertisements weld together across the pauses between them. Breaks publish as ranges into episode_midrolls, which MidrollSkip and PlayheadPolicy already read -- no playback changes and no migration. Intro and outro offsets are still computed and logged but no longer published, and the ad-derived ones already in the server's state were cleared. Two guards publish nothing rather than something wrong: an episode whose matches cover over 35% of it, and any single match over four minutes. A re-aired episode or a two-parter would otherwise be eaten. Both record the refusal, because "found nothing" and "refused what it found" are different states and conflating them is the readout mistake over again. Also three data-loss bugs, found by reading the journal after a restart rather than by any test. The subscription list -- the one thing the server cannot rediscover -- was persisted only when a whole poll cycle finished, which for 61 feeds is hours. A cycle re-applied its stale snapshot at the end, which would undo a push that arrived during it and then delete the audio of every show the snapshot omitted. And detection ignored cancellation, so systemd's 90-second stop SIGKILLed the process before the save-on-exit could run. All three fixed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B --- app/src/main/java/org/terst/listen/data/Daos.kt | 11 +++ .../java/org/terst/listen/data/Repositories.kt | 2 +- .../java/org/terst/listen/server/ServerSkips.kt | 34 ++++++++++ .../java/org/terst/listen/server/SkipServer.kt | 46 +++++++++++++ .../test/java/org/terst/listen/data/FakeDaos.kt | 4 ++ .../org/terst/listen/server/ServerSkipsTest.kt | 77 +++++++++++++++++++++ .../java/org/terst/listen/server/SkipServerTest.kt | 79 ++++++++++++++++++++++ 7 files changed, 252 insertions(+), 1 deletion(-) (limited to 'app/src') 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 = 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 = 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(), 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(), 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) + } } -- cgit v1.2.3