diff options
Diffstat (limited to 'app/src')
16 files changed, 910 insertions, 16 deletions
diff --git a/app/src/androidTest/java/org/terst/listen/data/AdReportsDbTest.kt b/app/src/androidTest/java/org/terst/listen/data/AdReportsDbTest.kt new file mode 100644 index 0000000..e6274f8 --- /dev/null +++ b/app/src/androidTest/java/org/terst/listen/data/AdReportsDbTest.kt @@ -0,0 +1,99 @@ +package org.terst.listen.data + +import androidx.room.Room +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.terst.listen.playback.AdReportGesture +import org.terst.listen.playback.AdReportStore + +/** + * The whole gesture against the real table: press, press again, correct, withdraw. + * + * Driven through [AdReportGesture] rather than by writing rows by hand, because the thing worth + * proving is that one claim stays one row however many times he presses. + */ +@RunWith(AndroidJUnit4::class) +class AdReportsDbTest { + + private lateinit var db: AppDatabase + private lateinit var gesture: AdReportGesture + private lateinit var store: AdReportStore + + @Before + fun setUp() { + db = Room.inMemoryDatabaseBuilder( + InstrumentationRegistry.getInstrumentation().targetContext, + AppDatabase::class.java, + ).build() + gesture = AdReportGesture(correctionWindowMs = 20_000) + store = AdReportStore(db.adReports()) + } + + @After + fun tearDown() = db.close() + + @Test + fun pressingTwiceLeavesOneClaimCoveringBoth() = runBlocking { + store.apply(gesture.press("e1", positionMs = 600_000, jumpToMs = 645_000, nowMs = 1_000)) + store.apply(gesture.press("e1", positionMs = 645_000, jumpToMs = 690_000, nowMs = 6_000)) + + val rows = db.adReports().getForEpisode("e1") + assertEquals("two presses, one pod, one row", 1, rows.size) + assertEquals(600_000L, rows.single().startMs) + assertEquals(690_000L, rows.single().endMs) + assertFalse("nothing has corrected the end yet", rows.single().endCorrected) + } + + @Test + fun comingBackTrimsTheClaimAndSaysSoWasMeasured() = runBlocking { + store.apply(gesture.press("e1", positionMs = 600_000, jumpToMs = 645_000, nowMs = 1_000)) + store.apply(gesture.seekedBack("e1", landedAtMs = 622_000, nowMs = 6_000)) + + val row = db.adReports().getForEpisode("e1").single() + assertEquals(600_000L, row.startMs) + assertEquals(622_000L, row.endMs) + assertTrue("he heard the programme resume here, so the end is measured", row.endCorrected) + } + + @Test + fun comingBackPastThePressRemovesTheClaimEntirely() = runBlocking { + store.apply(gesture.press("e1", positionMs = 600_000, jumpToMs = 645_000, nowMs = 1_000)) + store.apply(gesture.seekedBack("e1", landedAtMs = 598_000, nowMs = 6_000)) + + assertEquals(emptyList<AdReportEntity>(), db.adReports().getForEpisode("e1")) + } + + @Test + fun ordinaryNavigationWritesNothing() = runBlocking { + store.apply(gesture.seekedBack("e1", landedAtMs = 400_000, nowMs = 1_000)) + + assertEquals(emptyList<AdReportEntity>(), db.adReports().getForEpisode("e1")) + } + + @Test + fun separatePodsAreSeparateClaims() = runBlocking { + store.apply(gesture.press("e1", positionMs = 600_000, jumpToMs = 645_000, nowMs = 1_000)) + store.apply(gesture.press("e1", positionMs = 1_200_000, jumpToMs = 1_245_000, nowMs = 600_000)) + + assertEquals(2, db.adReports().getForEpisode("e1").size) + assertEquals(listOf(600_000L, 1_200_000L), db.adReports().getAll().map { it.startMs }) + } + + @Test + fun aClaimNeedsNoEpisodeRow() = runBlocking { + // Nothing has inserted an episode here, and the insert still succeeds. That is the + // no-foreign-key decision holding: the corpus should outlive whichever episode happened + // to be on the phone when the label was given. + store.apply(gesture.press("gone", positionMs = 600_000, jumpToMs = 645_000, nowMs = 1_000)) + + assertEquals(1, db.adReports().getAll().size) + } +} 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 0124562..836ec3d 100644 --- a/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt +++ b/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt @@ -653,4 +653,49 @@ class MigrationTest { assertEquals(0, it.getInt(1)) } } + + /** + * The manual-report table arrives empty and accepts a row. Nothing else moves: a report is + * new evidence, not a change to anything already stored. + */ + @Test + fun migrate15To16AddsAdReportsAndLeavesEverythingElseAlone() { + helper.createDatabase(name, 15).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, + 141421, 'NONE')""" + ) + close() + } + + val db = helper.runMigrationsAndValidate(name, 16, true, AppDatabase.MIGRATION_15_16) + + db.query("SELECT COUNT(*) FROM ad_reports").use { + it.moveToFirst() + assertEquals(0, it.getInt(0)) + } + db.execSQL( + "INSERT INTO ad_reports (episodeId, startMs, endMs, reportedAtMs, correctionWindowMs) " + + "VALUES ('e1', 600000, 645000, 99, 20000)" + ) + db.query("SELECT endMs, endCorrected FROM ad_reports WHERE episodeId = 'e1'").use { + it.moveToFirst() + assertEquals(645000L, it.getLong(0)) + assertEquals("an uncorrected end is where the jump landed, which is a guess", 0, it.getInt(1)) + } + db.query("SELECT positionMs FROM episodes WHERE id = 'e1'").use { + it.moveToFirst() + assertEquals("the crown jewel must survive", 141421L, 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 3d5d54e..636dd9f 100644 --- a/app/src/main/java/org/terst/listen/data/AppDatabase.kt +++ b/app/src/main/java/org/terst/listen/data/AppDatabase.kt @@ -30,8 +30,9 @@ class Converters { ShowMidrollEntity::class, EpisodeMidrollEntity::class, BreakVerdictEntity::class, + AdReportEntity::class, ], - version = 15, + version = 16, exportSchema = true, ) @TypeConverters(Converters::class) @@ -44,6 +45,7 @@ abstract class AppDatabase : RoomDatabase() { abstract fun fingerprints(): ShowFingerprintDao abstract fun midrolls(): MidrollDao abstract fun breakVerdicts(): BreakVerdictDao + abstract fun adReports(): AdReportDao companion object { @Volatile private var instance: AppDatabase? = null @@ -403,6 +405,28 @@ abstract class AppDatabase : RoomDatabase() { } } + /** + * Advertising the listener reported himself, which detection missed. + * + * A new table only. Nothing already stored changes meaning: a report is evidence added + * beside the detector's, not a correction to it. + */ + val MIGRATION_15_16 = object : Migration(15, 16) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL( + """CREATE TABLE IF NOT EXISTS `ad_reports` ( + `episodeId` TEXT NOT NULL, `startMs` INTEGER NOT NULL, + `endMs` INTEGER NOT NULL, `reportedAtMs` INTEGER NOT NULL, + `correctionWindowMs` INTEGER NOT NULL, + `endCorrected` INTEGER NOT NULL DEFAULT 0, + PRIMARY KEY(`episodeId`, `startMs`))""" + ) + db.execSQL( + "CREATE INDEX IF NOT EXISTS `index_ad_reports_episodeId` ON `ad_reports` (`episodeId`)" + ) + } + } + fun get(context: Context): AppDatabase = instance ?: synchronized(this) { instance ?: build(context.applicationContext).also { instance = it } } @@ -415,7 +439,7 @@ abstract class AppDatabase : RoomDatabase() { MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5, MIGRATION_5_6, 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_14_15, MIGRATION_15_16, ) .addCallback(SEED_DEFAULT_FILTERS) // No fallbackToDestructiveMigration. A listener's playback positions and play 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 bdbc28e..08f8c45 100644 --- a/app/src/main/java/org/terst/listen/data/Daos.kt +++ b/app/src/main/java/org/terst/listen/data/Daos.kt @@ -534,3 +534,20 @@ interface BreakVerdictDao { @Query("DELETE FROM break_verdicts WHERE episodeId = :episodeId AND startMs = :startMs AND endMs = :endMs") suspend fun delete(episodeId: String, startMs: Long, endMs: Long) } + +/** Advertising the listener pointed at himself; see [AdReportEntity]. */ +@Dao +interface AdReportDao { + @Query("SELECT * FROM ad_reports WHERE episodeId = :episodeId ORDER BY startMs") + suspend fun getForEpisode(episodeId: String): List<AdReportEntity> + + /** Oldest first, which is the order they should be pushed to the server in. */ + @Query("SELECT * FROM ad_reports ORDER BY reportedAtMs") + suspend fun getAll(): List<AdReportEntity> + + @Insert(onConflict = OnConflictStrategy.REPLACE) + suspend fun upsert(report: AdReportEntity) + + @Query("DELETE FROM ad_reports WHERE episodeId = :episodeId AND startMs = :startMs") + suspend fun delete(episodeId: String, startMs: Long) +} 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 4ae9faa..6deaef3 100644 --- a/app/src/main/java/org/terst/listen/data/Entities.kt +++ b/app/src/main/java/org/terst/listen/data/Entities.kt @@ -508,6 +508,40 @@ data class BreakVerdictEntity( ) /** + * A stretch of audio the listener said was advertising, by pressing the notification button. + * + * The opposite evidence to [BreakVerdictEntity]: that one corrects a break detection found, this + * one reports advertising detection missed entirely. Detection accounts for roughly 38% of the + * advertising the feeds imply, so most of what he hears is undetected and only he can point at + * it. + * + * No foreign key, for the same reason as [BreakVerdictEntity]: the label should outlive the + * episode row. Keyed by start rather than by the whole range, because a second press extends the + * same claim and must replace it rather than add a row beside it. + */ +@Entity( + tableName = "ad_reports", + primaryKeys = ["episodeId", "startMs"], + indices = [Index(value = ["episodeId"])], +) +data class AdReportEntity( + val episodeId: String, + val startMs: Long, + val endMs: Long, + val reportedAtMs: Long, + /** The window the gesture was judged by; see [org.terst.listen.playback.AdReport]. */ + val correctionWindowMs: Long, + /** + * Whether he came back and trimmed the end. + * + * An uncorrected end is wherever the jump landed, which is a guess. A corrected one is where + * he heard the programme resume, which is measured. A seeded search should not treat the two + * as equally good. + */ + @ColumnInfo(defaultValue = "0") val endCorrected: Boolean = false, +) + +/** * One entry in the single Up Next queue. * * Position is a plain ordinal rewritten on reorder. A personal queue is short enough that the 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 0cee350..2b972b8 100644 --- a/app/src/main/java/org/terst/listen/data/Repositories.kt +++ b/app/src/main/java/org/terst/listen/data/Repositories.kt @@ -19,6 +19,7 @@ import org.terst.listen.feed.FeedFetcher import org.terst.listen.feed.FeedResolver import org.terst.listen.fingerprint.FileMidrollClipStore import org.terst.listen.fingerprint.MidrollClipStore +import org.terst.listen.playback.AdReportStore import org.terst.listen.playback.PlaybackAdvancer import org.terst.listen.playback.PlaybackSources import org.terst.listen.playback.VerdictRecorder @@ -68,6 +69,7 @@ class AppGraph(private val context: Context) { val fingerprints: ShowFingerprintDao get() = db.fingerprints() val midrolls: MidrollDao get() = db.midrolls() val breakVerdicts: BreakVerdictDao get() = db.breakVerdicts() + val adReports: AdReportDao get() = db.adReports() val libraryDir: File by lazy { File(context.filesDir, "audio").also { it.mkdirs() } @@ -149,6 +151,8 @@ class AppGraph(private val context: Context) { val verdictRecorder: VerdictRecorder by lazy { VerdictRecorder(breakVerdicts) } + val adReportStore: AdReportStore by lazy { AdReportStore(adReports) } + val repository: SubscriptionRepository by lazy { SubscriptionRepository( subscriptions, diff --git a/app/src/main/java/org/terst/listen/playback/AdJump.kt b/app/src/main/java/org/terst/listen/playback/AdJump.kt new file mode 100644 index 0000000..99622c1 --- /dev/null +++ b/app/src/main/java/org/terst/listen/playback/AdJump.kt @@ -0,0 +1,36 @@ +package org.terst.listen.playback + +import org.terst.listen.data.BreakVerdictEntity +import org.terst.listen.data.EpisodeMidrollEntity + +/** + * How far "this is an ad" jumps. + * + * The press means "get me past this advertisement", so the jump is the best estimate available + * at that moment rather than a constant. If detection already found a break here, its end is a + * measured answer and is used. Otherwise the default stands in. + * + * The size of that default is the trade between too small -- several presses to clear one pod -- + * and too large, which lands him in the programme and makes him correct constantly. Advertising + * is sold in roughly 15-second units grouped into pods, and he presses partway through one, so + * the typical *remaining* duration is what this estimates. Erring small is deliberate: a second + * press costs one tap and improves the label, while over-shooting costs him content. + */ +object AdJump { + + /** Three 15-second units. See the class comment for why it is not larger. */ + const val DEFAULT_JUMP_MS = 45_000L + + fun targetFor( + ranges: List<EpisodeMidrollEntity>, + positionMs: Long, + durationMs: Long?, + verdicts: List<BreakVerdictEntity> = emptyList(), + ): Long { + val detected = BreakVerdicts.withoutInvalidated(ranges, verdicts) + .firstOrNull { positionMs >= it.startMs && positionMs < it.endMs } + ?.endMs + val target = detected ?: (positionMs + DEFAULT_JUMP_MS) + return if (durationMs != null) target.coerceAtMost(durationMs) else target + } +} diff --git a/app/src/main/java/org/terst/listen/playback/AdReportGesture.kt b/app/src/main/java/org/terst/listen/playback/AdReportGesture.kt new file mode 100644 index 0000000..e30abf8 --- /dev/null +++ b/app/src/main/java/org/terst/listen/playback/AdReportGesture.kt @@ -0,0 +1,119 @@ +package org.terst.listen.playback + +/** + * One stretch of audio the listener said was advertising. + * + * A press is a timestamp, not a range, so [endMs] starts as wherever the jump took him and is + * corrected downwards if he comes back. [correctionWindowMs] is stored rather than read from + * configuration so that changing the window later cannot retroactively reinterpret old labels. + */ +data class AdReport( + val episodeId: String, + val startMs: Long, + val endMs: Long, + val reportedAtMs: Long, + val correctionWindowMs: Long, +) + +/** What a press or a seek-back meant. */ +sealed interface ReportMove { + /** Advertising claimed from here; seek to [toMs]. */ + data class Jump(val toMs: Long, val report: AdReport) : ReportMove + + /** He came back, so the claim ends where he landed. */ + data class Corrected(val report: AdReport) : ReportMove + + /** He came back past the press: there was no advertising there. */ + data class Withdrawn(val report: AdReport) : ReportMove + + /** Ordinary navigation. Says nothing about advertising. */ + data object NotALabel : ReportMove +} + +/** + * The "this is an ad" button, which is one button and four meanings. + * + * Detection finds roughly 38% of the advertising the feeds imply, so most of what he hears is + * undetected and the popup on a mark can only ever correct what was already found. This is the + * other direction: a positive label, given at the one moment he actually notices, without + * unlocking anything. + * + * | gesture | means | + * |---------------------------------------|----------------------------------------| + * | press | advertising starts at or before here | + * | press again | still advertising -- extend | + * | back, landing inside the jumped span | over-shot -- the end is where I landed | + * | back, landing at or before the press | that was not advertising | + * + * The two back cases are told apart by where he lands against a span recorded exactly, not by + * guessing, and the jump size never enters into it. Outside the correction window a seek is + * ordinary navigation and must not be read as a label at all. + * + * Holds the one open claim. Not thread-safe: the service drives it from the main thread. + */ +class AdReportGesture(private val correctionWindowMs: Long = DEFAULT_WINDOW_MS) { + + private var open: AdReport? = null + + /** + * Claims advertising from [positionMs], extending the open claim when this press continues + * the last one. Returns where to seek to. + */ + fun press(episodeId: String, positionMs: Long, jumpToMs: Long, nowMs: Long): ReportMove.Jump { + val continuing = current(episodeId, nowMs) + val report = AdReport( + episodeId = episodeId, + // The start of the claim survives every later press: he is labelling one pod, and + // the first press is the earliest point he can vouch for. + startMs = continuing?.startMs ?: positionMs, + endMs = jumpToMs, + reportedAtMs = nowMs, + correctionWindowMs = correctionWindowMs, + ) + open = report + return ReportMove.Jump(toMs = jumpToMs, report = report) + } + + /** + * What landing at [landedAtMs] after seeking backwards meant. + * + * Every backwards seek arrives here, whatever pressed it -- notification, headphone remote, + * the in-app bar, a dragged slider. That is the point: he corrects with whatever is to hand, + * and only the landing position carries the meaning. + */ + fun seekedBack(episodeId: String, landedAtMs: Long, nowMs: Long): ReportMove { + val report = current(episodeId, nowMs) ?: return ReportMove.NotALabel + // Landing beyond the span is not a correction to it; he jumped somewhere else entirely. + if (landedAtMs >= report.endMs) return ReportMove.NotALabel + + if (landedAtMs <= report.startMs) { + open = null + return ReportMove.Withdrawn(report) + } + + // Stays open, and the window is refreshed: hunting back for where the programme resumes + // takes several presses, and the last landing is the one he settled on. Settling short + // of the true end under-claims, which is the safe direction -- it never cuts programme. + val corrected = report.copy(endMs = landedAtMs, reportedAtMs = nowMs) + open = corrected + return ReportMove.Corrected(corrected) + } + + /** Drops the open claim. Called when the playing episode changes. */ + fun forget() { + open = null + } + + private fun current(episodeId: String, nowMs: Long): AdReport? = + open?.takeIf { it.episodeId == episodeId && nowMs - it.reportedAtMs <= correctionWindowMs } + + companion object { + /** + * How long after a press a backwards seek still counts as a correction. + * + * Long enough to get the phone out of a pocket, short enough that it is unmistakably + * connected to the press. Later than this and a seek is just a seek. + */ + const val DEFAULT_WINDOW_MS = 20_000L + } +} diff --git a/app/src/main/java/org/terst/listen/playback/AdReportStore.kt b/app/src/main/java/org/terst/listen/playback/AdReportStore.kt new file mode 100644 index 0000000..2843f67 --- /dev/null +++ b/app/src/main/java/org/terst/listen/playback/AdReportStore.kt @@ -0,0 +1,36 @@ +package org.terst.listen.playback + +import org.terst.listen.data.AdReportDao +import org.terst.listen.data.AdReportEntity + +/** + * Writes down what the gesture decided. + * + * Every move is persisted as it happens rather than when the claim looks finished, because + * "finished" is only knowable in hindsight -- the correction window expiring is not an event, and + * the media process can be killed at any point in the middle of one. + */ +class AdReportStore(private val dao: AdReportDao) { + + suspend fun apply(move: ReportMove) { + when (move) { + // Keyed on the start, which every press of one pod shares, so extending replaces the + // row rather than adding a second claim beside it. + is ReportMove.Jump -> dao.upsert(move.report.row(endCorrected = false)) + is ReportMove.Corrected -> dao.upsert(move.report.row(endCorrected = true)) + // He took it back. Gone entirely rather than stored as a negative: the detector found + // nothing here, so there is nothing for a "no" to be about. + is ReportMove.Withdrawn -> dao.delete(move.report.episodeId, move.report.startMs) + ReportMove.NotALabel -> Unit + } + } + + private fun AdReport.row(endCorrected: Boolean) = AdReportEntity( + episodeId = episodeId, + startMs = startMs, + endMs = endMs, + reportedAtMs = reportedAtMs, + correctionWindowMs = correctionWindowMs, + endCorrected = endCorrected, + ) +} diff --git a/app/src/main/java/org/terst/listen/playback/AdReporter.kt b/app/src/main/java/org/terst/listen/playback/AdReporter.kt new file mode 100644 index 0000000..4912a74 --- /dev/null +++ b/app/src/main/java/org/terst/listen/playback/AdReporter.kt @@ -0,0 +1,42 @@ +package org.terst.listen.playback + +/** + * The "this is an ad" button, joined up: the gesture that reads the presses, the estimate that + * decides how far to jump, and the table the claim lands in. + * + * One object so the service has one call per event and the three parts cannot be wired up + * inconsistently -- a press that jumps without recording, or records without jumping, is worse + * than either on its own. + */ +class AdReporter( + private val gesture: AdReportGesture, + private val store: AdReportStore, + private val now: () -> Long = System::currentTimeMillis, +) { + + /** Records the claim and returns where to seek to. */ + suspend fun pressed( + episodeId: String, + positionMs: Long, + durationMs: Long?, + context: PlayingContext, + ): Long { + val target = AdJump.targetFor(context.midrolls, positionMs, durationMs, context.verdicts) + val move = gesture.press(episodeId, positionMs, jumpToMs = target, nowMs = now()) + store.apply(move) + return move.toMs + } + + /** + * Tells the gesture about a backwards seek, whatever caused it. + * + * Most are ordinary navigation and change nothing; the gesture decides which. Calling this + * for every backwards seek is deliberate -- he corrects with whatever control is to hand. + */ + suspend fun seekedBack(episodeId: String, landedAtMs: Long) { + store.apply(gesture.seekedBack(episodeId, landedAtMs, nowMs = now())) + } + + /** The playing episode changed, so no claim is open any more. */ + fun forget() = gesture.forget() +} diff --git a/app/src/main/java/org/terst/listen/playback/ListenSessionCallback.kt b/app/src/main/java/org/terst/listen/playback/ListenSessionCallback.kt index 72fe5de..f0c9ae3 100644 --- a/app/src/main/java/org/terst/listen/playback/ListenSessionCallback.kt +++ b/app/src/main/java/org/terst/listen/playback/ListenSessionCallback.kt @@ -28,6 +28,12 @@ import org.terst.listen.data.Graph class ListenSessionCallback( private val context: Context, private val scope: CoroutineScope = CoroutineScope(Dispatchers.Main), + /** + * What a "this is an ad" press does. Supplied by [PlaybackService] rather than built here, + * because the claim has to be judged against the episode's detected breaks and its length, + * which only the component holding the player knows. + */ + private val onReportAd: () -> Unit = {}, ) : MediaLibraryService.MediaLibrarySession.Callback { private val graph by lazy { Graph.get(context) } @@ -42,13 +48,20 @@ class ListenSessionCallback( val sessionCommands = MediaSession.ConnectionResult.DEFAULT_SESSION_AND_LIBRARY_COMMANDS.buildUpon() .add(NotificationControls.seekBackCommand) .add(NotificationControls.seekForwardCommand) + .add(NotificationControls.reportAdCommand) .build() // Previous/next stay available -- removing them to free car-notification slots also // removed the headphone skip button, which stopped working entirely. PodcastPlayer // redefines them as skip-forward/back instead, so they are useful rather than absent. - // Only previous-media-item goes, since there is no previous item and he does not want - // the control. + // + // The two *media-item* commands go, and only those: there is one item loaded, so moving + // between items is meaningless, and a control that does nothing is worse than no control. + // COMMAND_SEEK_TO_PREVIOUS and COMMAND_SEEK_TO_NEXT -- the ones a headphone remote and a + // Bluetooth stack actually send -- are untouched and must stay that way. + // + // Nothing here is removed to make room in the notification any more; the slots on + // NotificationControls' buttons do that without touching commands at all. val playerCommands = MediaSession.ConnectionResult.DEFAULT_PLAYER_COMMANDS.buildUpon() .remove(Player.COMMAND_SEEK_TO_PREVIOUS_MEDIA_ITEM) .remove(Player.COMMAND_SEEK_TO_NEXT_MEDIA_ITEM) @@ -57,7 +70,7 @@ class ListenSessionCallback( return MediaSession.ConnectionResult.AcceptedResultBuilder(session) .setAvailableSessionCommands(sessionCommands) .setAvailablePlayerCommands(playerCommands) - .setCustomLayout(ImmutableList.copyOf(NotificationControls.customLayout())) + .setMediaButtonPreferences(ImmutableList.copyOf(NotificationControls.mediaButtonPreferences())) .build() } @@ -70,6 +83,7 @@ class ListenSessionCallback( when (customCommand.customAction) { NotificationControls.ACTION_SEEK_BACK -> session.player.seekBack() NotificationControls.ACTION_SEEK_FORWARD -> session.player.seekForward() + NotificationControls.ACTION_REPORT_AD -> onReportAd() else -> return Futures.immediateFuture(SessionResult(SessionError.ERROR_NOT_SUPPORTED)) } return Futures.immediateFuture(SessionResult(SessionResult.RESULT_SUCCESS)) diff --git a/app/src/main/java/org/terst/listen/playback/NotificationControls.kt b/app/src/main/java/org/terst/listen/playback/NotificationControls.kt index 5c8d7c5..21f3959 100644 --- a/app/src/main/java/org/terst/listen/playback/NotificationControls.kt +++ b/app/src/main/java/org/terst/listen/playback/NotificationControls.kt @@ -7,13 +7,21 @@ import androidx.media3.session.CommandButton import androidx.media3.session.SessionCommand /** - * Seek buttons for the notification, lock screen and Bluetooth controls. + * What the notification, lock screen, Bluetooth controls and car head unit show. * - * Media3's default layout offers previous/next *track*, which is right for an album and wrong - * for a podcast: with a single item loaded there is no next track, so the notification showed a - * previous button and nothing else. What a listener actually wants there is skip-back and - * skip-forward by seconds, and those are not default buttons -- they have to be published as - * custom commands. + * Media3's defaults offer previous/next *track*, which is right for an album and wrong for a + * podcast: with a single item loaded there is no next track. [PodcastPlayer] redefines those two + * as skip-back and skip-forward by seconds so a headphone remote does something useful, and this + * published skip-back-15 / forward-25 pair beside them -- which left the notification carrying + * four buttons doing two jobs. + * + * These are media button *preferences*, not a custom layout, so the duplication goes: a button + * claiming [CommandButton.SLOT_BACK] or [CommandButton.SLOT_FORWARD] takes that slot instead of + * the default previous/next button, and nothing is removed from the player's available commands + * to achieve it. That matters -- removing commands to free notification slots once silently + * killed the headphone skip button, and this route never touches them. + * + * The reclaimed slot carries "this is an ad". See [AdReportGesture] for what a press means. * * The seek amounts themselves live on the player ([PlaybackService] sets them), so these buttons * and the in-app bar cannot drift apart. @@ -23,9 +31,11 @@ object NotificationControls { const val ACTION_SEEK_BACK = "org.terst.listen.SEEK_BACK" const val ACTION_SEEK_FORWARD = "org.terst.listen.SEEK_FORWARD" + const val ACTION_REPORT_AD = "org.terst.listen.REPORT_AD" val seekBackCommand = SessionCommand(ACTION_SEEK_BACK, Bundle.EMPTY) val seekForwardCommand = SessionCommand(ACTION_SEEK_FORWARD, Bundle.EMPTY) + val reportAdCommand = SessionCommand(ACTION_REPORT_AD, Bundle.EMPTY) /** * Media3 ships numbered icons for 5, 10, 15 and 30 seconds only. @@ -33,17 +43,28 @@ object NotificationControls { * Back is 15, so it gets the numbered icon. Forward is 25 and there is no 25 icon, so it * gets the plain arrow rather than one captioned with a number the button doesn't do. */ - fun customLayout(): List<CommandButton> = listOf( + fun mediaButtonPreferences(): List<CommandButton> = listOf( CommandButton.Builder() .setDisplayName("Back 15 seconds") .setIconResId(androidx.media3.session.R.drawable.media3_icon_skip_back_15) .setSessionCommand(seekBackCommand) + .setSlots(CommandButton.SLOT_BACK) .build(), CommandButton.Builder() .setDisplayName("Forward 25 seconds") .setIconResId(androidx.media3.session.R.drawable.media3_icon_skip_forward) .setSessionCommand(seekForwardCommand) + .setSlots(CommandButton.SLOT_FORWARD) + .build(), + // A circle-and-slash rather than another arrow: it must not be mistakable for the + // forward button in a pocket, which is the one real risk of putting it next to one. + CommandButton.Builder() + .setDisplayName("This is an ad") + .setIconResId(androidx.media3.session.R.drawable.media3_icon_block) + .setSessionCommand(reportAdCommand) + // Beside forward where there is room, in the overflow where there is not. Never in + // a seek slot: it would then replace a control he uses constantly. + .setSlots(CommandButton.SLOT_FORWARD_SECONDARY, CommandButton.SLOT_OVERFLOW) .build(), ) - } diff --git a/app/src/main/java/org/terst/listen/playback/PlaybackService.kt b/app/src/main/java/org/terst/listen/playback/PlaybackService.kt index 603ec5b..3a2543b 100644 --- a/app/src/main/java/org/terst/listen/playback/PlaybackService.kt +++ b/app/src/main/java/org/terst/listen/playback/PlaybackService.kt @@ -37,6 +37,9 @@ class PlaybackService : MediaLibraryService() { private var session: MediaLibrarySession? = null private var writer: PositionWriter? = null private var advancer: PlaybackAdvancer? = null + + /** The "this is an ad" button, which lives on the notification. See [AdReportGesture]. */ + private var reporter: AdReporter? = null private val progress = PlaybackProgress() /** @@ -70,19 +73,28 @@ class PlaybackService : MediaLibraryService() { .build() player = exo + + // Read before the session is built: the session's callback needs the reporter, because + // a "this is an ad" press is answered from the player's position. + val graph = Graph.get(this) + reporter = AdReporter(AdReportGesture(), graph.adReportStore) + // A library session rather than a plain one: Android Auto only treats an app as // driving-friendly if it can browse a media library, and without that the head unit // can't start or choose anything. // Wrapped so next/previous mean skip-forward/back by seconds; see PodcastPlayer. - session = MediaLibrarySession.Builder(this, PodcastPlayer(exo), ListenSessionCallback(this, scope)) - .setCustomLayout(NotificationControls.customLayout()) + session = MediaLibrarySession.Builder( + this, + PodcastPlayer(exo), + ListenSessionCallback(this, scope, onReportAd = ::reportAd), + ) + .setMediaButtonPreferences(NotificationControls.mediaButtonPreferences()) // Without this the notification is inert to a tap -- the transport buttons work and // the body does nothing, which reads as the app being broken rather than as a // deliberate omission. .setSessionActivity(openAppIntent()) .build() - val graph = Graph.get(this) writer = PositionWriter(ExoPlayhead(exo), graph.playbackState) advancer = graph.advancer @@ -136,6 +148,28 @@ class PlaybackService : MediaLibraryService() { finish(finished) } + /** + * Every seek, whatever asked for it -- the notification, a headphone remote, the in-app + * bar, a dragged slider. + * + * Only backwards ones mean anything here, and most of those are ordinary navigation; + * [AdReportGesture] decides which few are corrections to a claim he just made. Hooking + * the discontinuity rather than each control is the point: he corrects with whatever is + * to hand, and adding a control later must not quietly stop being heard. + */ + override fun onPositionDiscontinuity( + oldPosition: Player.PositionInfo, + newPosition: Player.PositionInfo, + reason: Int, + ) { + if (reason != Player.DISCONTINUITY_REASON_SEEK) return + if (newPosition.positionMs >= oldPosition.positionMs) return + val episodeId = newPosition.mediaItem?.mediaId ?: return + val current = reporter ?: return + val landedAtMs = newPosition.positionMs + scope.launch { current.seekedBack(episodeId, landedAtMs) } + } + override fun onMediaItemTransition(mediaItem: MediaItem?, reason: Int) { // Where the prepare clock starts. This is the one hook both play paths reach: the // service's own queue advance, and a tap in the UI, which sets the item through a @@ -144,6 +178,9 @@ class PlaybackService : MediaLibraryService() { val uri = mediaItem.localConfiguration?.uri?.scheme prepareTrace.start(id, durationMs = null, local = uri == null || uri == "file") } + // Whatever he was labelling, he has left it. A claim open across an episode change + // would read the next episode's first seek-back as a correction to the last one's. + reporter?.forget() // The cached show and chapters belong to whatever is playing; refresh them whenever // that changes, including when something else in the app starts an episode. loadContextFor(mediaItem?.mediaId) @@ -171,6 +208,7 @@ class PlaybackService : MediaLibraryService() { player = null writer = null advancer = null + reporter = null super.onDestroy() } @@ -224,6 +262,29 @@ class PlaybackService : MediaLibraryService() { } } + /** + * "This is an ad": records the claim, then jumps past it. + * + * Recorded before the seek rather than after, so the span he is about to jump over is what + * gets stored. The jump is answered from this episode's detected breaks when there is one + * here -- see [AdJump] -- which is why this lives with the player rather than in the session + * callback. + */ + private fun reportAd() { + val exo = player ?: return + val current = reporter ?: return + val episodeId = exo.currentMediaItem?.mediaId ?: return + val positionMs = exo.currentPosition + val durationMs = exo.duration.takeIf { it > 0 } + val playing = context + scope.launch { + val target = current.pressed(episodeId, positionMs, durationMs, playing) + // Re-checked because the write suspended: a queue advance in that window would + // otherwise move the next episode's playhead by this episode's press. + if (player?.currentMediaItem?.mediaId == episodeId) player?.seekTo(target) + } + } + /** Marks the episode played, moves the queue on, and starts what follows or stops. */ private fun finish(episodeId: String) { val current = advancer ?: return diff --git a/app/src/test/java/org/terst/listen/playback/AdJumpTest.kt b/app/src/test/java/org/terst/listen/playback/AdJumpTest.kt new file mode 100644 index 0000000..3421aa2 --- /dev/null +++ b/app/src/test/java/org/terst/listen/playback/AdJumpTest.kt @@ -0,0 +1,93 @@ +package org.terst.listen.playback + +import org.junit.Assert.assertEquals +import org.junit.Test +import org.terst.listen.data.BreakVerdictEntity +import org.terst.listen.data.EpisodeMidrollEntity + +/** + * How far "this is an ad" jumps. + * + * A press means "get me past this advertisement", so the jump is the best estimate available at + * that moment rather than a constant. + */ +class AdJumpTest { + + private fun range(startMs: Long, endMs: Long, kind: String = "ad") = + EpisodeMidrollEntity( + episodeId = "ep", + candidateId = "c", + startMs = startMs, + endMs = endMs, + kind = kind, + ) + + @Test + fun `with nothing known it jumps the default`() { + assertEquals( + 600_000 + AdJump.DEFAULT_JUMP_MS, + AdJump.targetFor(emptyList(), positionMs = 600_000, durationMs = 3_600_000), + ) + } + + @Test + fun `inside a detected break it jumps to that break's end, which is already known`() { + val ranges = listOf(range(590_000, 700_000)) + + assertEquals(700_000L, AdJump.targetFor(ranges, positionMs = 600_000, durationMs = 3_600_000)) + } + + @Test + fun `a detected break that ends sooner than the default still wins`() { + val ranges = listOf(range(595_000, 620_000)) + + assertEquals(620_000L, AdJump.targetFor(ranges, positionMs = 600_000, durationMs = 3_600_000)) + } + + @Test + fun `a break he has marked wrong is not used as the estimate`() { + val ranges = listOf(range(590_000, 700_000)) + val verdicts = listOf( + BreakVerdictEntity( + episodeId = "ep", + startMs = 590_000, + endMs = 700_000, + verdict = BreakVerdicts.INVALID, + decidedAtMs = 0, + ) + ) + + assertEquals( + 600_000 + AdJump.DEFAULT_JUMP_MS, + AdJump.targetFor(ranges, positionMs = 600_000, durationMs = 3_600_000, verdicts = verdicts), + ) + } + + @Test + fun `a break starting ahead of the playhead is not what he is hearing`() { + val ranges = listOf(range(700_000, 800_000)) + + assertEquals( + 600_000 + AdJump.DEFAULT_JUMP_MS, + AdJump.targetFor(ranges, positionMs = 600_000, durationMs = 3_600_000), + ) + } + + @Test + fun `the jump never runs off the end of the episode`() { + assertEquals(600_000L, AdJump.targetFor(emptyList(), positionMs = 580_000, durationMs = 600_000)) + } + + @Test + fun `an unknown length is no reason not to jump`() { + assertEquals( + 600_000 + AdJump.DEFAULT_JUMP_MS, + AdJump.targetFor(emptyList(), positionMs = 600_000, durationMs = null), + ) + } + + @Test + fun `the default is a plausible remaining advertisement, in whole ad units`() { + assertEquals(0L, AdJump.DEFAULT_JUMP_MS % 15_000L) + } +} diff --git a/app/src/test/java/org/terst/listen/playback/AdReportGestureTest.kt b/app/src/test/java/org/terst/listen/playback/AdReportGestureTest.kt new file mode 100644 index 0000000..2f25391 --- /dev/null +++ b/app/src/test/java/org/terst/listen/playback/AdReportGestureTest.kt @@ -0,0 +1,150 @@ +package org.terst.listen.playback + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The "this is an ad" gesture, which is one button and four meanings. + */ +class AdReportGestureTest { + + private val gesture = AdReportGesture(correctionWindowMs = 20_000L) + + @Test + fun `a press claims advertising from here and jumps`() { + val move = gesture.press("ep", positionMs = 600_000, jumpToMs = 645_000, nowMs = 1_000) + + assertEquals(645_000L, move.toMs) + assertEquals(600_000L, move.report.startMs) + assertEquals(645_000L, move.report.endMs) + } + + @Test + fun `pressing again extends the same claim rather than starting a second`() { + gesture.press("ep", positionMs = 600_000, jumpToMs = 645_000, nowMs = 1_000) + + val again = gesture.press("ep", positionMs = 645_000, jumpToMs = 690_000, nowMs = 6_000) + + assertEquals(600_000L, again.report.startMs) + assertEquals(690_000L, again.report.endMs) + } + + @Test + fun `a press long after the last one is a separate break`() { + gesture.press("ep", positionMs = 600_000, jumpToMs = 645_000, nowMs = 1_000) + + val later = gesture.press("ep", positionMs = 1_200_000, jumpToMs = 1_245_000, nowMs = 600_000) + + assertEquals(1_200_000L, later.report.startMs) + } + + @Test + fun `a press on a different episode is a separate break`() { + gesture.press("ep", positionMs = 600_000, jumpToMs = 645_000, nowMs = 1_000) + + val other = gesture.press("other", positionMs = 10_000, jumpToMs = 55_000, nowMs = 6_000) + + assertEquals(10_000L, other.report.startMs) + assertEquals("other", other.report.episodeId) + } + + @Test + fun `seeking back into the jumped span moves the end to where he landed`() { + gesture.press("ep", positionMs = 600_000, jumpToMs = 645_000, nowMs = 1_000) + + val move = gesture.seekedBack("ep", landedAtMs = 630_000, nowMs = 6_000) + + assertTrue("got $move", move is ReportMove.Corrected) + assertEquals(600_000L, (move as ReportMove.Corrected).report.startMs) + assertEquals(630_000L, move.report.endMs) + } + + @Test + fun `a second seek back corrects the end again`() { + gesture.press("ep", positionMs = 600_000, jumpToMs = 645_000, nowMs = 1_000) + gesture.seekedBack("ep", landedAtMs = 630_000, nowMs = 6_000) + + val move = gesture.seekedBack("ep", landedAtMs = 615_000, nowMs = 11_000) + + assertEquals(615_000L, (move as ReportMove.Corrected).report.endMs) + } + + @Test + fun `seeking back to before the press withdraws the claim`() { + gesture.press("ep", positionMs = 600_000, jumpToMs = 645_000, nowMs = 1_000) + + val move = gesture.seekedBack("ep", landedAtMs = 598_000, nowMs = 6_000) + + assertTrue("got $move", move is ReportMove.Withdrawn) + assertEquals(600_000L, (move as ReportMove.Withdrawn).report.startMs) + } + + @Test + fun `landing exactly back at the press withdraws it -- nothing was there`() { + gesture.press("ep", positionMs = 600_000, jumpToMs = 645_000, nowMs = 1_000) + + val move = gesture.seekedBack("ep", landedAtMs = 600_000, nowMs = 6_000) + + assertTrue("got $move", move is ReportMove.Withdrawn) + } + + @Test + fun `a withdrawn claim is gone, so the next seek back is ordinary navigation`() { + gesture.press("ep", positionMs = 600_000, jumpToMs = 645_000, nowMs = 1_000) + gesture.seekedBack("ep", landedAtMs = 598_000, nowMs = 6_000) + + assertEquals(ReportMove.NotALabel, gesture.seekedBack("ep", landedAtMs = 583_000, nowMs = 11_000)) + } + + @Test + fun `seeking back long after the press is ordinary navigation`() { + gesture.press("ep", positionMs = 600_000, jumpToMs = 645_000, nowMs = 1_000) + + assertEquals(ReportMove.NotALabel, gesture.seekedBack("ep", landedAtMs = 630_000, nowMs = 600_000)) + } + + @Test + fun `seeking back with no claim open is ordinary navigation`() { + assertEquals(ReportMove.NotALabel, gesture.seekedBack("ep", landedAtMs = 630_000, nowMs = 1_000)) + } + + @Test + fun `seeking back on a different episode is ordinary navigation`() { + gesture.press("ep", positionMs = 600_000, jumpToMs = 645_000, nowMs = 1_000) + + assertEquals(ReportMove.NotALabel, gesture.seekedBack("other", landedAtMs = 630_000, nowMs = 6_000)) + } + + @Test + fun `landing past the end of the jump is not a correction`() { + gesture.press("ep", positionMs = 600_000, jumpToMs = 645_000, nowMs = 1_000) + + assertEquals(ReportMove.NotALabel, gesture.seekedBack("ep", landedAtMs = 660_000, nowMs = 6_000)) + } + + @Test + fun `each correction refreshes the window, so a hunt for the content start holds together`() { + gesture.press("ep", positionMs = 600_000, jumpToMs = 645_000, nowMs = 1_000) + gesture.seekedBack("ep", landedAtMs = 630_000, nowMs = 15_000) + + val move = gesture.seekedBack("ep", landedAtMs = 615_000, nowMs = 30_000) + + assertEquals(615_000L, (move as ReportMove.Corrected).report.endMs) + } + + @Test + fun `the window the label was judged by is stored with it`() { + val move = gesture.press("ep", positionMs = 600_000, jumpToMs = 645_000, nowMs = 1_000) + + assertEquals(20_000L, move.report.correctionWindowMs) + } + + @Test + fun `changing episode closes the claim`() { + gesture.press("ep", positionMs = 600_000, jumpToMs = 645_000, nowMs = 1_000) + gesture.forget() + + assertEquals(ReportMove.NotALabel, gesture.seekedBack("ep", landedAtMs = 630_000, nowMs = 6_000)) + } +} diff --git a/app/src/test/java/org/terst/listen/playback/AdReporterTest.kt b/app/src/test/java/org/terst/listen/playback/AdReporterTest.kt new file mode 100644 index 0000000..6ead0de --- /dev/null +++ b/app/src/test/java/org/terst/listen/playback/AdReporterTest.kt @@ -0,0 +1,99 @@ +package org.terst.listen.playback + +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import org.terst.listen.data.AdReportDao +import org.terst.listen.data.AdReportEntity +import org.terst.listen.data.EpisodeMidrollEntity + +/** The press, the jump and the row, joined up. */ +class AdReporterTest { + + private val dao = FakeAdReportDao() + private var clock = 1_000L + private val reporter = AdReporter( + gesture = AdReportGesture(correctionWindowMs = 20_000), + store = AdReportStore(dao), + now = { clock }, + ) + + private fun range(startMs: Long, endMs: Long) = + EpisodeMidrollEntity(episodeId = "e1", candidateId = "c", startMs = startMs, endMs = endMs) + + @Test + fun `a press jumps the default and records the claim`() = runBlocking { + val target = reporter.pressed("e1", positionMs = 600_000, durationMs = 3_600_000, context = PlayingContext()) + + assertEquals(600_000 + AdJump.DEFAULT_JUMP_MS, target) + assertEquals(600_000L, dao.rows.single().startMs) + assertEquals(600_000 + AdJump.DEFAULT_JUMP_MS, dao.rows.single().endMs) + } + + @Test + fun `inside a detected break the press jumps to its measured end`() = runBlocking { + val context = PlayingContext(midrolls = listOf(range(590_000, 712_000))) + + val target = reporter.pressed("e1", positionMs = 600_000, durationMs = 3_600_000, context = context) + + assertEquals(712_000L, target) + } + + @Test + fun `coming back inside the jump trims the claim`() = runBlocking { + reporter.pressed("e1", positionMs = 600_000, durationMs = 3_600_000, context = PlayingContext()) + clock = 6_000 + + reporter.seekedBack("e1", landedAtMs = 620_000) + + assertEquals(620_000L, dao.rows.single().endMs) + assertTrue(dao.rows.single().endCorrected) + } + + @Test + fun `coming back past the press removes the claim`() = runBlocking { + reporter.pressed("e1", positionMs = 600_000, durationMs = 3_600_000, context = PlayingContext()) + clock = 6_000 + + reporter.seekedBack("e1", landedAtMs = 599_000) + + assertEquals(emptyList<AdReportEntity>(), dao.rows) + } + + @Test + fun `a seek back with nothing claimed records nothing`() = runBlocking { + reporter.seekedBack("e1", landedAtMs = 100_000) + + assertEquals(emptyList<AdReportEntity>(), dao.rows) + } + + @Test + fun `changing episode ends the gesture, so the next seek back is navigation`() = runBlocking { + reporter.pressed("e1", positionMs = 600_000, durationMs = 3_600_000, context = PlayingContext()) + reporter.forget() + clock = 6_000 + + reporter.seekedBack("e1", landedAtMs = 620_000) + + assertEquals("the claim stands as pressed", 600_000 + AdJump.DEFAULT_JUMP_MS, dao.rows.single().endMs) + } +} + +private class FakeAdReportDao : AdReportDao { + val rows = mutableListOf<AdReportEntity>() + + override suspend fun getForEpisode(episodeId: String) = + rows.filter { it.episodeId == episodeId }.sortedBy { it.startMs } + + override suspend fun getAll() = rows.sortedBy { it.reportedAtMs } + + override suspend fun upsert(report: AdReportEntity) { + rows.removeAll { it.episodeId == report.episodeId && it.startMs == report.startMs } + rows += report + } + + override suspend fun delete(episodeId: String, startMs: Long) { + rows.removeAll { it.episodeId == episodeId && it.startMs == startMs } + } +} |