diff options
| author | Peter Stone <thepeterstone@gmail.com> | 2026-10-20 08:04:06 +0000 |
|---|---|---|
| committer | Peter Stone <thepeterstone@gmail.com> | 2026-10-20 08:04:06 +0000 |
| commit | 37136b152e923f784ad4056f9deddfab1d7f1e8c (patch) | |
| tree | 4437ad2095d1de6242cbd2d89ec04e74fce7f3d1 | |
| parent | d53a8c54351ccf5e0ecfd890abe53e13c65c60fa (diff) | |
Add a "this is an ad" button to the notification
The notification carried four buttons doing two jobs: Media3's default
previous/next (which PodcastPlayer redefines as seek) plus our own
back-15/forward-25 pair. Media button preferences with explicit slots
let the custom pair claim the two seek slots, so the duplicates go and
the freed space carries "this is an ad". No player command is removed
to achieve it, which is the route that once silently killed the
headphone skip button.
One press means four things, told apart by where the playhead lands
rather than by guessing: press claims advertising from here and jumps
past it, pressing again extends the same claim, coming back inside the
jump moves its end to where you landed, and coming back past the press
withdraws it. Outside a 20-second window a seek is ordinary navigation
and says nothing. The window is stored with the label so changing it
later cannot reinterpret old data.
Every backwards seek feeds the gesture, whatever caused it --
notification, headphone remote, in-app bar, dragged slider -- because
you correct with whatever is to hand.
This is the opposite evidence to the tap-a-break popup. That one can
only correct breaks detection already found, and detection accounts for
about 38% of the advertising the feeds imply.
Verified on the emulator: the notification now shows exactly back-15,
pause, forward-25 and the new button; the lock screen shows it too and
pressing it there records a claim without unlocking; all four gesture
arms behave as specified; and the media transport keys still seek by
25s and 15s. Android Auto still needs the car.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B
19 files changed, 1837 insertions, 24 deletions
diff --git a/.coverage-baseline b/.coverage-baseline index afce29c..e4cbd3e 100644 --- a/.coverage-baseline +++ b/.coverage-baseline @@ -1 +1 @@ -94.34 +94.42 diff --git a/app/schemas/org.terst.listen.data.AppDatabase/16.json b/app/schemas/org.terst.listen.data.AppDatabase/16.json new file mode 100644 index 0000000..838fafc --- /dev/null +++ b/app/schemas/org.terst.listen.data.AppDatabase/16.json @@ -0,0 +1,903 @@ +{ + "formatVersion": 1, + "database": { + "version": 16, + "identityHash": "720a69af9ed1a99a3bc22b4272a799f1", + "entities": [ + { + "tableName": "subscriptions", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `feedUrl` TEXT NOT NULL, `kind` TEXT NOT NULL, `title` TEXT NOT NULL, `description` TEXT, `author` TEXT, `imageUrl` TEXT, `link` TEXT, `addedAtMs` INTEGER NOT NULL, `etag` TEXT, `lastModified` TEXT, `lastFetchedAtMs` INTEGER, `lastError` TEXT, `playbackSpeed` REAL, `skipIntroMs` INTEGER NOT NULL, `skipOutroMs` INTEGER NOT NULL, `dripEnabled` INTEGER NOT NULL DEFAULT 0, `dripIntervalMs` INTEGER NOT NULL DEFAULT 86400000, `dripCount` INTEGER NOT NULL DEFAULT 1, `dripOldestFirst` INTEGER NOT NULL DEFAULT 1, `dripLastReleaseAtMs` INTEGER, `skipAdChapters` INTEGER NOT NULL DEFAULT 0, `midrollPendingEpisodeId` TEXT, `bedtime` INTEGER NOT NULL DEFAULT 0, PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "feedUrl", + "columnName": "feedUrl", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "kind", + "columnName": "kind", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "title", + "columnName": "title", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "description", + "columnName": "description", + "affinity": "TEXT" + }, + { + "fieldPath": "author", + "columnName": "author", + "affinity": "TEXT" + }, + { + "fieldPath": "imageUrl", + "columnName": "imageUrl", + "affinity": "TEXT" + }, + { + "fieldPath": "link", + "columnName": "link", + "affinity": "TEXT" + }, + { + "fieldPath": "addedAtMs", + "columnName": "addedAtMs", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "etag", + "columnName": "etag", + "affinity": "TEXT" + }, + { + "fieldPath": "lastModified", + "columnName": "lastModified", + "affinity": "TEXT" + }, + { + "fieldPath": "lastFetchedAtMs", + "columnName": "lastFetchedAtMs", + "affinity": "INTEGER" + }, + { + "fieldPath": "lastError", + "columnName": "lastError", + "affinity": "TEXT" + }, + { + "fieldPath": "playbackSpeed", + "columnName": "playbackSpeed", + "affinity": "REAL" + }, + { + "fieldPath": "skipIntroMs", + "columnName": "skipIntroMs", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "skipOutroMs", + "columnName": "skipOutroMs", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "dripEnabled", + "columnName": "dripEnabled", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "dripIntervalMs", + "columnName": "dripIntervalMs", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "86400000" + }, + { + "fieldPath": "dripCount", + "columnName": "dripCount", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "1" + }, + { + "fieldPath": "dripOldestFirst", + "columnName": "dripOldestFirst", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "1" + }, + { + "fieldPath": "dripLastReleaseAtMs", + "columnName": "dripLastReleaseAtMs", + "affinity": "INTEGER" + }, + { + "fieldPath": "skipAdChapters", + "columnName": "skipAdChapters", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "midrollPendingEpisodeId", + "columnName": "midrollPendingEpisodeId", + "affinity": "TEXT" + }, + { + "fieldPath": "bedtime", + "columnName": "bedtime", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_subscriptions_feedUrl", + "unique": true, + "columnNames": [ + "feedUrl" + ], + "orders": [], + "createSql": "CREATE UNIQUE INDEX IF NOT EXISTS `index_subscriptions_feedUrl` ON `${TABLE_NAME}` (`feedUrl`)" + } + ] + }, + { + "tableName": "episodes", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `subscriptionId` TEXT NOT NULL, `guid` TEXT NOT NULL, `title` TEXT NOT NULL, `description` TEXT, `mediaUrl` TEXT NOT NULL, `mediaType` TEXT NOT NULL, `mediaLengthBytes` INTEGER, `durationMs` INTEGER, `publishedAtMs` INTEGER, `seasonNumber` INTEGER, `episodeNumber` INTEGER, `imageUrl` TEXT, `chaptersUrl` TEXT, `discoveredAtMs` INTEGER NOT NULL, `positionMs` INTEGER NOT NULL, `playedAtMs` INTEGER, `actualDurationMs` INTEGER, `downloadState` TEXT NOT NULL, `localPath` TEXT, `downloadedAtMs` INTEGER, `audioFromServer` INTEGER NOT NULL DEFAULT 0, `dripReleasedAtMs` INTEGER, `appliedRuleId` TEXT, `appliedRuleReason` TEXT, `speedOverride` REAL, `midrollCheckedAtMs` INTEGER, `chaptersJson` TEXT, `detectedIntroMs` INTEGER, `detectedOutroMs` INTEGER, PRIMARY KEY(`id`), FOREIGN KEY(`subscriptionId`) REFERENCES `subscriptions`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "subscriptionId", + "columnName": "subscriptionId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "guid", + "columnName": "guid", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "title", + "columnName": "title", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "description", + "columnName": "description", + "affinity": "TEXT" + }, + { + "fieldPath": "mediaUrl", + "columnName": "mediaUrl", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "mediaType", + "columnName": "mediaType", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "mediaLengthBytes", + "columnName": "mediaLengthBytes", + "affinity": "INTEGER" + }, + { + "fieldPath": "durationMs", + "columnName": "durationMs", + "affinity": "INTEGER" + }, + { + "fieldPath": "publishedAtMs", + "columnName": "publishedAtMs", + "affinity": "INTEGER" + }, + { + "fieldPath": "seasonNumber", + "columnName": "seasonNumber", + "affinity": "INTEGER" + }, + { + "fieldPath": "episodeNumber", + "columnName": "episodeNumber", + "affinity": "INTEGER" + }, + { + "fieldPath": "imageUrl", + "columnName": "imageUrl", + "affinity": "TEXT" + }, + { + "fieldPath": "chaptersUrl", + "columnName": "chaptersUrl", + "affinity": "TEXT" + }, + { + "fieldPath": "discoveredAtMs", + "columnName": "discoveredAtMs", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "positionMs", + "columnName": "positionMs", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "playedAtMs", + "columnName": "playedAtMs", + "affinity": "INTEGER" + }, + { + "fieldPath": "actualDurationMs", + "columnName": "actualDurationMs", + "affinity": "INTEGER" + }, + { + "fieldPath": "downloadState", + "columnName": "downloadState", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "localPath", + "columnName": "localPath", + "affinity": "TEXT" + }, + { + "fieldPath": "downloadedAtMs", + "columnName": "downloadedAtMs", + "affinity": "INTEGER" + }, + { + "fieldPath": "audioFromServer", + "columnName": "audioFromServer", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "dripReleasedAtMs", + "columnName": "dripReleasedAtMs", + "affinity": "INTEGER" + }, + { + "fieldPath": "appliedRuleId", + "columnName": "appliedRuleId", + "affinity": "TEXT" + }, + { + "fieldPath": "appliedRuleReason", + "columnName": "appliedRuleReason", + "affinity": "TEXT" + }, + { + "fieldPath": "speedOverride", + "columnName": "speedOverride", + "affinity": "REAL" + }, + { + "fieldPath": "midrollCheckedAtMs", + "columnName": "midrollCheckedAtMs", + "affinity": "INTEGER" + }, + { + "fieldPath": "chaptersJson", + "columnName": "chaptersJson", + "affinity": "TEXT" + }, + { + "fieldPath": "detectedIntroMs", + "columnName": "detectedIntroMs", + "affinity": "INTEGER" + }, + { + "fieldPath": "detectedOutroMs", + "columnName": "detectedOutroMs", + "affinity": "INTEGER" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_episodes_subscriptionId", + "unique": false, + "columnNames": [ + "subscriptionId" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_episodes_subscriptionId` ON `${TABLE_NAME}` (`subscriptionId`)" + }, + { + "name": "index_episodes_subscriptionId_guid", + "unique": true, + "columnNames": [ + "subscriptionId", + "guid" + ], + "orders": [], + "createSql": "CREATE UNIQUE INDEX IF NOT EXISTS `index_episodes_subscriptionId_guid` ON `${TABLE_NAME}` (`subscriptionId`, `guid`)" + }, + { + "name": "index_episodes_publishedAtMs", + "unique": false, + "columnNames": [ + "publishedAtMs" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_episodes_publishedAtMs` ON `${TABLE_NAME}` (`publishedAtMs`)" + } + ], + "foreignKeys": [ + { + "table": "subscriptions", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "subscriptionId" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "queue", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`episodeId` TEXT NOT NULL, `position` INTEGER NOT NULL, PRIMARY KEY(`episodeId`), FOREIGN KEY(`episodeId`) REFERENCES `episodes`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "episodeId", + "columnName": "episodeId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "position", + "columnName": "position", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "episodeId" + ] + }, + "indices": [ + { + "name": "index_queue_position", + "unique": false, + "columnNames": [ + "position" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_queue_position` ON `${TABLE_NAME}` (`position`)" + } + ], + "foreignKeys": [ + { + "table": "episodes", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "episodeId" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "rules", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `name` TEXT NOT NULL, `position` INTEGER NOT NULL, `enabled` INTEGER NOT NULL, `subscriptionId` TEXT, `titleContains` TEXT, `titleExcludes` TEXT, `minDurationMs` INTEGER, `maxDurationMs` INTEGER, `maxAgeMs` INTEGER, `seasonNumber` INTEGER, `action` TEXT NOT NULL, `autoDownload` INTEGER NOT NULL, `speed` REAL, PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "name", + "columnName": "name", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "position", + "columnName": "position", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "enabled", + "columnName": "enabled", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "subscriptionId", + "columnName": "subscriptionId", + "affinity": "TEXT" + }, + { + "fieldPath": "titleContains", + "columnName": "titleContains", + "affinity": "TEXT" + }, + { + "fieldPath": "titleExcludes", + "columnName": "titleExcludes", + "affinity": "TEXT" + }, + { + "fieldPath": "minDurationMs", + "columnName": "minDurationMs", + "affinity": "INTEGER" + }, + { + "fieldPath": "maxDurationMs", + "columnName": "maxDurationMs", + "affinity": "INTEGER" + }, + { + "fieldPath": "maxAgeMs", + "columnName": "maxAgeMs", + "affinity": "INTEGER" + }, + { + "fieldPath": "seasonNumber", + "columnName": "seasonNumber", + "affinity": "INTEGER" + }, + { + "fieldPath": "action", + "columnName": "action", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "autoDownload", + "columnName": "autoDownload", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "speed", + "columnName": "speed", + "affinity": "REAL" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_rules_position", + "unique": false, + "columnNames": [ + "position" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_rules_position` ON `${TABLE_NAME}` (`position`)" + } + ] + }, + { + "tableName": "show_fingerprints", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`subscriptionId` TEXT NOT NULL, `kind` TEXT NOT NULL, `referenceClipPcm` BLOB, `pendingEpisodeId` TEXT, `sampleRate` INTEGER NOT NULL, `confirmedCount` INTEGER NOT NULL, `lastConfirmedEpisodeId` TEXT, `updatedAtMs` INTEGER NOT NULL, PRIMARY KEY(`subscriptionId`, `kind`), FOREIGN KEY(`subscriptionId`) REFERENCES `subscriptions`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "subscriptionId", + "columnName": "subscriptionId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "kind", + "columnName": "kind", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "referenceClipPcm", + "columnName": "referenceClipPcm", + "affinity": "BLOB" + }, + { + "fieldPath": "pendingEpisodeId", + "columnName": "pendingEpisodeId", + "affinity": "TEXT" + }, + { + "fieldPath": "sampleRate", + "columnName": "sampleRate", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "confirmedCount", + "columnName": "confirmedCount", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "lastConfirmedEpisodeId", + "columnName": "lastConfirmedEpisodeId", + "affinity": "TEXT" + }, + { + "fieldPath": "updatedAtMs", + "columnName": "updatedAtMs", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "subscriptionId", + "kind" + ] + }, + "indices": [ + { + "name": "index_show_fingerprints_subscriptionId", + "unique": false, + "columnNames": [ + "subscriptionId" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_show_fingerprints_subscriptionId` ON `${TABLE_NAME}` (`subscriptionId`)" + } + ], + "foreignKeys": [ + { + "table": "subscriptions", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "subscriptionId" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "show_midrolls", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`subscriptionId` TEXT NOT NULL, `id` TEXT NOT NULL, `referenceClipPath` TEXT NOT NULL, `sampleRate` INTEGER NOT NULL, `confirmedCount` INTEGER NOT NULL, `lastConfirmedEpisodeId` TEXT, `updatedAtMs` INTEGER NOT NULL, PRIMARY KEY(`subscriptionId`, `id`), FOREIGN KEY(`subscriptionId`) REFERENCES `subscriptions`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "subscriptionId", + "columnName": "subscriptionId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "referenceClipPath", + "columnName": "referenceClipPath", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "sampleRate", + "columnName": "sampleRate", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "confirmedCount", + "columnName": "confirmedCount", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "lastConfirmedEpisodeId", + "columnName": "lastConfirmedEpisodeId", + "affinity": "TEXT" + }, + { + "fieldPath": "updatedAtMs", + "columnName": "updatedAtMs", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "subscriptionId", + "id" + ] + }, + "indices": [ + { + "name": "index_show_midrolls_subscriptionId", + "unique": false, + "columnNames": [ + "subscriptionId" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_show_midrolls_subscriptionId` ON `${TABLE_NAME}` (`subscriptionId`)" + } + ], + "foreignKeys": [ + { + "table": "subscriptions", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "subscriptionId" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "episode_midrolls", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`episodeId` TEXT NOT NULL, `candidateId` TEXT NOT NULL, `startMs` INTEGER NOT NULL, `endMs` INTEGER NOT NULL, `kind` TEXT NOT NULL DEFAULT 'ad', `partners` INTEGER NOT NULL DEFAULT 0, PRIMARY KEY(`episodeId`, `candidateId`), FOREIGN KEY(`episodeId`) REFERENCES `episodes`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "episodeId", + "columnName": "episodeId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "candidateId", + "columnName": "candidateId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "startMs", + "columnName": "startMs", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "endMs", + "columnName": "endMs", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "kind", + "columnName": "kind", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "'ad'" + }, + { + "fieldPath": "partners", + "columnName": "partners", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "episodeId", + "candidateId" + ] + }, + "indices": [ + { + "name": "index_episode_midrolls_episodeId", + "unique": false, + "columnNames": [ + "episodeId" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_episode_midrolls_episodeId` ON `${TABLE_NAME}` (`episodeId`)" + } + ], + "foreignKeys": [ + { + "table": "episodes", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "episodeId" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "break_verdicts", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`episodeId` TEXT NOT NULL, `startMs` INTEGER NOT NULL, `endMs` INTEGER NOT NULL, `verdict` TEXT NOT NULL, `decidedAtMs` INTEGER NOT NULL, `kind` TEXT NOT NULL DEFAULT 'ad', `partners` INTEGER NOT NULL DEFAULT 0, PRIMARY KEY(`episodeId`, `startMs`, `endMs`))", + "fields": [ + { + "fieldPath": "episodeId", + "columnName": "episodeId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "startMs", + "columnName": "startMs", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "endMs", + "columnName": "endMs", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "verdict", + "columnName": "verdict", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "decidedAtMs", + "columnName": "decidedAtMs", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "kind", + "columnName": "kind", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "'ad'" + }, + { + "fieldPath": "partners", + "columnName": "partners", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "episodeId", + "startMs", + "endMs" + ] + }, + "indices": [ + { + "name": "index_break_verdicts_episodeId", + "unique": false, + "columnNames": [ + "episodeId" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_break_verdicts_episodeId` ON `${TABLE_NAME}` (`episodeId`)" + } + ] + }, + { + "tableName": "ad_reports", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`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`))", + "fields": [ + { + "fieldPath": "episodeId", + "columnName": "episodeId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "startMs", + "columnName": "startMs", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "endMs", + "columnName": "endMs", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "reportedAtMs", + "columnName": "reportedAtMs", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "correctionWindowMs", + "columnName": "correctionWindowMs", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "endCorrected", + "columnName": "endCorrected", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "episodeId", + "startMs" + ] + }, + "indices": [ + { + "name": "index_ad_reports_episodeId", + "unique": false, + "columnNames": [ + "episodeId" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_ad_reports_episodeId` ON `${TABLE_NAME}` (`episodeId`)" + } + ] + } + ], + "setupQueries": [ + "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '720a69af9ed1a99a3bc22b4272a799f1')" + ] + } +}
\ No newline at end of file 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 } + } +} diff --git a/docs/superpowers/plans/2026-10-20-corrections-and-signature-library.md b/docs/superpowers/plans/2026-10-20-corrections-and-signature-library.md index 6dea531..cd79f1e 100644 --- a/docs/superpowers/plans/2026-10-20-corrections-and-signature-library.md +++ b/docs/superpowers/plans/2026-10-20-corrections-and-signature-library.md @@ -60,7 +60,7 @@ Tapping a break mark opens metadata and a verdict. Detail in the dispatch; the p - **Verdicts stay local for now.** No endpoint, no consumer. Storing them from today means a corpus exists by the time the library does. -### 1b. "This is an ad" in the notification — NOT STARTED +### 1b. "This is an ad" in the notification — DONE 2026-10-20 Peter, 2026-10-20: *"The notification currently has duplicate skip buttons (our custom ones plus forward/back) - there's UI space for explicit 'this is an ad' there."* @@ -71,12 +71,28 @@ forward/back) - there's UI space for explicit 'this is an ad' there."* `NotificationControls.customLayout()` publishes a second seek-back-15 / forward-25 pair on top. Reclaiming a slot means dropping one pair. -**Do this on a device, carefully.** `ListenSessionCallback` carries a comment recording that -removing commands to free notification slots once silently killed the headphone skip button. -Whichever pair goes, verify: the notification, the lock screen, Bluetooth/headphone transport, -and Android Auto. Note also that that comment currently contradicts its own code — it says "Only -previous-media-item goes" while the code removes both previous and next. Worth resolving while -in there. +**How the slot was reclaimed — no command was removed at all.** Media3 1.8 has media button +*preferences*: a `CommandButton` claiming `SLOT_BACK` or `SLOT_FORWARD` takes that slot instead +of the default previous/next button (`DefaultMediaNotificationProvider.getMediaButtons` checks +`CommandButton.containsButtonForSlot` before adding its own). So `setCustomLayout` became +`setMediaButtonPreferences`, the custom back-15 / forward-25 pair claimed the two seek slots, the +duplicate defaults disappeared, and "This is an ad" went in `SLOT_FORWARD_SECONDARY` falling back +to `SLOT_OVERFLOW`. Player commands are untouched, so the headphone regression that comment warns +about cannot recur by this route. The comment's contradiction was resolved while in there. + +**Verified on the emulator, 2026-10-20.** Notification actions are now exactly +`[Back 15 seconds] [Pause] [Forward 25 seconds] [This is an ad]` — four, not five. The platform +session carries all three custom actions, so the shade and lock-screen players show the +circle-and-slash button, and pressing it **from the lock screen without unlocking** recorded a +claim. All four gesture arms confirmed live, plus "nothing written" for a seek-back outside the +window. Headphone/Bluetooth transport keys still seek: `KEYCODE_MEDIA_NEXT` +25s, +`KEYCODE_MEDIA_PREVIOUS` -15s. + +**Android Auto is NOT verified** — it needs the real car. The risk is low (player commands, +browse tree and custom actions are all unchanged or strictly added to) but it is not zero. + +**What the default jump is.** 45s, three 15-second units, when nothing better is known; the end +of the detected break when the press lands inside one. See `AdJump`. **Why this button matters more than the popup.** They catch opposite errors: |
