diff options
| author | Peter Stone <thepeterstone@gmail.com> | 2026-09-29 06:26:00 +0000 |
|---|---|---|
| committer | Peter Stone <thepeterstone@gmail.com> | 2026-09-29 06:26:00 +0000 |
| commit | 16a47088a315786a940590794d7ce4003447cf7c (patch) | |
| tree | 968816a3175164f8f01e69180bf4e18c2e8f9c8d | |
| parent | c496ce8d8fb9d91d41cd544800a1f7fabb5d3ca1 (diff) | |
Start shared timestamps playing, and stop the feed clobbering real lengths
**The hand-off.** Sharing a timestamped YouTube link now opens Listen on the
player, already playing at that point. Two things were missing. The
timestamp was only honoured on first insert -- the insert ignores a clashing
guid, so every share after the first silently did nothing and playback
resumed wherever Listen had last left it, which is exactly the going-back-
and-forth case. And nothing started playing; the video landed in the library
to be found. Re-sharing now moves the position and unplays the episode,
because sharing a moment in something you finished is how you go back to it.
A share with no timestamp still just adds it quietly -- interrupting
whatever is playing would be rude.
**A regression in yesterday's duration fix, found by following Peter's own
observation.** He noted the 22:05-vs-29:34 gap was probably content versus
ads. It is -- and that has two consequences I had missed.
The first is that `EpisodeMatcher.mergedWith` rewrites `durationMs` from the
feed on every refresh, correctly, because it is feed metadata. Writing the
measured length into that column meant the correction survived until the
next refresh and then silently reverted. So there are two columns now:
`durationMs` stays the feed's claim, `actualDurationMs` is what the player
measured, and `playbackDurationMs` prefers the latter.
The second is that the wrong number was never only cosmetic. It sizes the
decode buffer a full-episode midroll scan allocates -- truncating the scan
by minutes on precisely the shows that have midrolls -- places the outro
offset, and drives the finished check, which would mark a 29-minute episode
played at 21 and advance the queue. Those all read `playbackDurationMs` now.
And the third thing, which is Peter's idea and free: the difference between
the two numbers is the advertising in the episode, with no audio analysis at
all. `insertedAdMs`, shown on the episode screen, and a check on the midroll
detector -- whatever it eventually finds should add up to about that much.
Measuring it turned up something worth knowing: the same Grass Apes came
back 7:29 longer than declared on one fetch and 5:34 on another. Insertion
happens per request, so this is a property of a file, not of an episode.
Verified on the emulator. Hand-off: cold start into the player at 12:39 from
a stored 12:30, and the warm onNewIntent path at 25:14 from 25:00. The
YouTube resolver itself cannot be exercised here -- this host gets a 302 bot
page for every watch URL, as CLAUDE.md already records -- so the new code was
driven through the intent it receives, with the resolver's own behaviour
unchanged. Durations: list showed 22m on a fresh subscribe, 27m after
playing, with 1325000 and 1659454 stored separately, and the episode screen
reading "Ads inserted ~5:34". 550 unit tests, 12 migration tests on device,
detekt and lint clean. Coverage 92.96% -> 93.02%.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B
24 files changed, 1211 insertions, 39 deletions
diff --git a/.coverage-baseline b/.coverage-baseline index d7655a6..d294bec 100644 --- a/.coverage-baseline +++ b/.coverage-baseline @@ -1 +1 @@ -92.96 +93.02 @@ -211,13 +211,38 @@ by default and correct; it is simply waiting for a publisher who marks breaks. ## Domain notes worth keeping - **A feed's declared duration is often wrong, and not by rounding.** Radiolab publishes Grass - Apes as `00:22:05`; the file ExoPlayer opens is 29:34. The seven and a half minutes between - them are dynamically inserted ads -- the feed states the ad-free content length and the CDN - stitches in the rest -- so the error is large, systematic, and worst on exactly the shows the - midroll detector cares about. `PositionWriter` replaces the stored value with the player's - once per episode, on the position tick, via the narrow `PlaybackStateDao`. Once per episode, - not once per tick: a lookup on every tick for the length of an episode is the per-episode - work the feed-scale note below warns about. + Apes as `00:22:05`; the file served is 27-30 minutes. The gap is dynamically inserted ads -- + the feed states the ad-free content length and the CDN stitches in the rest -- so the error + is large, systematic, and worst on exactly the shows the midroll detector cares about. + + **Two columns, not one.** `durationMs` is the feed's claim and stays the feed's: + `EpisodeMatcher.mergedWith` rewrites it on every refresh, correctly, which is why writing the + measured length there lasted exactly until the next refresh. `actualDurationMs` is what the + player measured and nothing else touches. **Read `playbackDurationMs`**, which prefers the + measured one -- the feed's figure otherwise sizes the decode buffer a full-episode scan + allocates, places the outro offset, and drives the finished check that would mark a + 29-minute episode played at 21. + + `PositionWriter` records it once per episode on the position tick, via the narrow + `PlaybackStateDao` -- not once per tick, which would be the per-episode work the feed-scale + note below warns about. + +- **The difference between the two durations is the advertising**, free and with no audio + analysis: `insertedAdMs`, shown on the episode screen. Peter's observation. It is also a + check on the midroll detector, which should eventually account for roughly that much. + **It is per *file*, not per episode** -- the same Grass Apes measured 7:29 of ads on one + fetch and 5:34 on another, because insertion happens per request. Do not treat it as a + stable property of an episode, and do not be surprised when a re-download changes the + length. + +- **A timestamped link shared in starts playing there.** `WatchLink` parses the `t`, the + repository stores it as the position, and the share activity hands the episode to + `MainActivity.playIntent` so the app opens on the player already playing -- the point being + to move between YouTube and Listen without hunting for your place. Re-sharing a video + already in the library updates its position and unplays it, which is the case that was + broken: the insert ignores a clashing guid, so every share after the first silently did + nothing. A share carrying no timestamp still just adds it quietly, since interrupting + whatever is playing would be rude. - **The media notification opens the app**, via `setSessionActivity` on the session. Without it the transport buttons work and tapping the body does nothing, which reads as breakage rather diff --git a/app/schemas/org.terst.listen.data.AppDatabase/12.json b/app/schemas/org.terst.listen.data.AppDatabase/12.json new file mode 100644 index 0000000..81389bb --- /dev/null +++ b/app/schemas/org.terst.listen.data.AppDatabase/12.json @@ -0,0 +1,752 @@ +{ + "formatVersion": 1, + "database": { + "version": 12, + "identityHash": "de82aefa2e5ee08d1c216ce30ad03d4f", + "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, `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": "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, 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 + } + ], + "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" + ] + } + ] + } + ], + "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, 'de82aefa2e5ee08d1c216ce30ad03d4f')" + ] + } +}
\ No newline at end of file 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 2c7440d..98f5ad1 100644 --- a/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt +++ b/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt @@ -479,4 +479,42 @@ class MigrationTest { assertEquals("/data/midrolls/s1/c2.pcm", cursor.getString(0)) } } + + /** + * Splitting the feed's claimed length from the measured one. + * + * The new column arrives null -- nothing has been measured for an existing episode -- and + * the feed's figure is left exactly where it was, because it is still the feed's to own + * and refreshes still rewrite it. + */ + @Test + fun migrates11To12AddingMeasuredDurationWithoutTouchingTheFeeds() { + helper.createDatabase(name, 11).use { db -> + db.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)""" + ) + db.execSQL( + """INSERT INTO episodes + (id, subscriptionId, guid, title, mediaUrl, mediaType, discoveredAtMs, + positionMs, downloadState, durationMs) + VALUES ('e1', 's1', 'g1', 'Grass Apes', 'https://cdn/1.mp3', 'audio/mpeg', + 1000, 616161, 'NONE', 1325000)""" + ) + } + + val db = helper.runMigrationsAndValidate(name, 12, true, AppDatabase.MIGRATION_11_12) + + db.query("SELECT durationMs, actualDurationMs, positionMs FROM episodes WHERE id = 'e1'") + .use { cursor -> + assertEquals(1, cursor.count) + cursor.moveToFirst() + assertEquals("the feed's claim is untouched", 1325000L, cursor.getLong(0)) + assertEquals("nothing measured yet", true, cursor.isNull(1)) + assertEquals("the crown jewel must survive", 616161L, cursor.getLong(2)) + } + } } 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 fe6451c..443a23d 100644 --- a/app/src/main/java/org/terst/listen/data/AppDatabase.kt +++ b/app/src/main/java/org/terst/listen/data/AppDatabase.kt @@ -30,7 +30,7 @@ class Converters { ShowMidrollEntity::class, EpisodeMidrollEntity::class, ], - version = 11, + version = 12, exportSchema = true, ) @TypeConverters(Converters::class) @@ -323,6 +323,29 @@ abstract class AppDatabase : RoomDatabase() { } } + /** + * Separates the length a feed *claims* from the length the file actually is. + * + * They are not the same number and the difference is not rounding: Radiolab declares + * Grass Apes at 22:05 and serves 29:34, the gap being ads the CDN inserts. Overwriting + * the feed's value with the real one -- the first attempt -- lost twice over. A feed + * refresh puts the claim straight back (`mergedWith` treats duration as feed metadata, + * correctly), and the claim is worth keeping anyway: the difference between the two is + * a free measure of how much advertising an episode carries, with no audio analysis at + * all. Peter's observation. + * + * So `durationMs` stays the feed's, refreshable, and this holds what the player + * measured. Everything that needs a real length reads + * [EpisodeEntity.playbackDurationMs], which prefers this one -- and several things do + * need it: the outro offset, the decode buffer a full-episode scan allocates, and the + * "finished" check that would otherwise mark a 29-minute episode played at 21. + */ + val MIGRATION_11_12 = object : Migration(11, 12) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL("ALTER TABLE `episodes` ADD COLUMN `actualDurationMs` INTEGER") + } + } + fun get(context: Context): AppDatabase = instance ?: synchronized(this) { instance ?: build(context.applicationContext).also { instance = it } } @@ -334,7 +357,7 @@ abstract class AppDatabase : RoomDatabase() { .addMigrations( 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_10_11, MIGRATION_11_12, ) .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 efb39b9..dec1ba0 100644 --- a/app/src/main/java/org/terst/listen/data/Daos.kt +++ b/app/src/main/java/org/terst/listen/data/Daos.kt @@ -221,6 +221,10 @@ interface EpisodeDao { @Query("SELECT * FROM episodes WHERE id = :id") suspend fun getById(id: String): EpisodeEntity? + /** How a re-shared video is recognised as one already in the library. */ + @Query("SELECT * FROM episodes WHERE subscriptionId = :subscriptionId AND guid = :guid") + suspend fun getByGuid(subscriptionId: String, guid: String): EpisodeEntity? + @Insert(onConflict = OnConflictStrategy.IGNORE) suspend fun insertAll(episodes: List<EpisodeEntity>) @@ -413,11 +417,11 @@ interface PlaybackStateDao { * CDN stitches in and the feed knows nothing about. The player is the first thing in the * app to see the truth. */ - @Query("UPDATE episodes SET durationMs = :durationMs WHERE id = :id") + @Query("UPDATE episodes SET actualDurationMs = :durationMs WHERE id = :id") suspend fun updateDuration(id: String, durationMs: Long) /** Just the column, rather than a whole row, because this runs on the position tick. */ - @Query("SELECT durationMs FROM episodes WHERE id = :id") + @Query("SELECT actualDurationMs FROM episodes WHERE id = :id") suspend fun storedDurationMs(id: String): 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 6f868b1..b14ed2b 100644 --- a/app/src/main/java/org/terst/listen/data/Entities.kt +++ b/app/src/main/java/org/terst/listen/data/Entities.kt @@ -123,6 +123,25 @@ data class SubscriptionEntity( @ColumnInfo(defaultValue = "0") val bedtime: Boolean = false, ) +/** + * The episode's real length where it is known, falling back to what the feed claimed. + * + * Read this, not [EpisodeEntity.durationMs], anywhere the number has consequences. The feed's + * figure drives the decode buffer a full-episode scan allocates, the outro offset, and the + * check that decides an episode is finished -- and on a show using dynamic ad insertion it is + * minutes short, which truncates the first, misplaces the second and fires the third early. + */ +val EpisodeEntity.playbackDurationMs: Long? + get() = actualDurationMs ?: durationMs + +/** How much longer the served file is than the feed says, which is the advertising in it. */ +val EpisodeEntity.insertedAdMs: Long? + get() { + val claimed = durationMs ?: return null + val actual = actualDurationMs ?: return null + return (actual - claimed).takeIf { it > 0 } + } + /** One a day, the cadence a drip subscription is for. */ const val DEFAULT_DRIP_INTERVAL_MS: Long = 24 * 60 * 60 * 1000L @@ -165,6 +184,17 @@ data class EpisodeEntity( /** Null while unplayed. Set when finished, and what retention counts its grace period from. */ val playedAtMs: Long? = null, + /** + * The length the player measured, as opposed to the length [durationMs] claims. + * + * Feeds are wrong about this routinely and substantially -- see + * [AppDatabase.MIGRATION_11_12]. Kept apart from the feed's figure rather than replacing + * it, because a refresh legitimately overwrites feed metadata and would undo the + * correction, and because the gap between the two is worth having on its own. Read + * [playbackDurationMs] rather than either field directly. + */ + val actualDurationMs: Long? = null, + val downloadState: DownloadState = DownloadState.NONE, val localPath: String? = null, val downloadedAtMs: Long? = null, diff --git a/app/src/main/java/org/terst/listen/data/EpisodeMatcher.kt b/app/src/main/java/org/terst/listen/data/EpisodeMatcher.kt index f11b602..7ea9477 100644 --- a/app/src/main/java/org/terst/listen/data/EpisodeMatcher.kt +++ b/app/src/main/java/org/terst/listen/data/EpisodeMatcher.kt @@ -71,6 +71,11 @@ object EpisodeMatcher { * Feed metadata is overwritten; listener state is carried across untouched. The stored id and * discovery date are kept too -- the id because the queue and downloads reference it, the * date because it records when the episode first appeared, not when it was last seen. + * + * `actualDurationMs` is carried across for the same reason, and it matters: `durationMs` is + * feed metadata and is rewritten here every refresh, so a measured length stored in that + * column would be quietly replaced by the feed's claim again. That is what happened when + * the correction was first written to `durationMs` directly. */ private fun EpisodeEntity.mergedWith(item: ParsedEpisode) = copy( guid = item.guid, diff --git a/app/src/main/java/org/terst/listen/data/SubscriptionRepository.kt b/app/src/main/java/org/terst/listen/data/SubscriptionRepository.kt index 1afb861..9b94c75 100644 --- a/app/src/main/java/org/terst/listen/data/SubscriptionRepository.kt +++ b/app/src/main/java/org/terst/listen/data/SubscriptionRepository.kt @@ -169,7 +169,7 @@ class SubscriptionRepository( * sentinel because there is no feed to refresh. There is one container per source, because * the kind lives on the container and has to be honest about where its episodes come from. */ - suspend fun addSingleEpisode(video: FeedResolver.Resolution.SingleEpisode): SubscriptionEntity { + suspend fun addSingleEpisode(video: FeedResolver.Resolution.SingleEpisode): EpisodeEntity? { // One container per source rather than one shared one: a container carries the kind // that decides how its episodes are fetched, and a single row cannot be honest about // two sources at once. @@ -194,14 +194,16 @@ class SubscriptionRepository( // and this costs a network round trip. val probed = if (video.title.isNullOrBlank()) probe(video.watchUrl) else null - // insertAll ignores a clashing (subscriptionId, guid), so re-adding the same video is a - // no-op rather than a duplicate. + val guid = if (facebook) "fb:video:${video.videoId}" else "yt:video:${video.videoId}" + val existing = episodes.getByGuid(container.id, guid) + if (existing != null) return reshare(existing, video) + episodes.insertAll( listOf( EpisodeEntity( id = UUID.randomUUID().toString(), subscriptionId = container.id, - guid = if (facebook) "fb:video:${video.videoId}" else "yt:video:${video.videoId}", + guid = guid, title = video.title?.takeIf { it.isNotBlank() } ?: probed?.title?.takeIf { it.isNotBlank() } ?: "Video ${video.videoId}", @@ -218,7 +220,32 @@ class SubscriptionRepository( ) ) ) - return container + return episodes.getByGuid(container.id, guid) + } + + /** + * A video shared again, which is the ordinary case once you are going back and forth. + * + * The insert ignores a clashing guid, so this used to be silently nothing: after the first + * hand-off, sharing the same video from a later point left Listen resuming wherever it had + * got to on its own. A timestamp shared now is an instruction about where to be, so it + * wins -- and it unplays the episode, because sharing a moment in something you finished + * is how you go back to it. + * + * A share carrying no timestamp says nothing about position and changes nothing. + */ + private suspend fun reshare( + existing: EpisodeEntity, + video: FeedResolver.Resolution.SingleEpisode, + ): EpisodeEntity { + val startAt = video.startPositionMs ?: return existing + + // One whole-row write rather than a narrow position update: the position column + // belongs to PlaybackStateDao, which is playback's to write, and this is not playback + // reporting where it got to -- it is the listener saying where to go. + val moved = existing.copy(positionMs = startAt, playedAtMs = null) + episodes.updateAll(listOf(moved)) + return moved } private fun ParsedFeed.toSubscription(feedUrl: String, kind: SubscriptionKind, nowMs: Long) = diff --git a/app/src/main/java/org/terst/listen/fingerprint/FingerprintUpdater.kt b/app/src/main/java/org/terst/listen/fingerprint/FingerprintUpdater.kt index eff48ae..bf73e9d 100644 --- a/app/src/main/java/org/terst/listen/fingerprint/FingerprintUpdater.kt +++ b/app/src/main/java/org/terst/listen/fingerprint/FingerprintUpdater.kt @@ -5,6 +5,7 @@ import org.terst.listen.data.EpisodeEntity import org.terst.listen.data.FingerprintKind import org.terst.listen.data.ShowFingerprintDao import org.terst.listen.data.ShowFingerprintEntity +import org.terst.listen.data.playbackDurationMs /** A short, decoded window of an episode's audio, and where it sits in the episode's own timeline. */ data class DecodedClip(val samples: DoubleArray, val sampleRate: Int, val startMs: Long) @@ -80,7 +81,7 @@ class FingerprintUpdater( } FingerprintKind.OUTRO -> { - val durationMs = episode.durationMs ?: return + val durationMs = episode.playbackDurationMs ?: return fingerprints.setDetectedOutroMs(episode.id, (durationMs - matchedStartMs).coerceAtLeast(0)) } } diff --git a/app/src/main/java/org/terst/listen/fingerprint/MediaCodecClipSource.kt b/app/src/main/java/org/terst/listen/fingerprint/MediaCodecClipSource.kt index 20f4417..4daed7d 100644 --- a/app/src/main/java/org/terst/listen/fingerprint/MediaCodecClipSource.kt +++ b/app/src/main/java/org/terst/listen/fingerprint/MediaCodecClipSource.kt @@ -7,6 +7,7 @@ import java.nio.ByteOrder import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import org.terst.listen.data.EpisodeEntity +import org.terst.listen.data.playbackDurationMs /** * The real clip decoder, backed by Android's `MediaExtractor`/`MediaCodec`. @@ -30,7 +31,7 @@ class MediaCodecClipSource : AudioClipSource { override suspend fun decodeTail(episode: EpisodeEntity): DecodedClip? { val path = episode.localPath ?: return null - val totalMs = episode.durationMs ?: return null + val totalMs = episode.playbackDurationMs ?: return null val startMs = (totalMs - CLIP_SECONDS * MS_PER_SECOND).coerceAtLeast(0) return decode(path, startMs, CLIP_SECONDS) } @@ -43,7 +44,7 @@ class MediaCodecClipSource : AudioClipSource { */ override suspend fun decodeFull(episode: EpisodeEntity): DecodedClip? { val path = episode.localPath ?: return null - val totalMs = episode.durationMs ?: return null + val totalMs = episode.playbackDurationMs ?: return null val clipSeconds = (totalMs / MS_PER_SECOND + FULL_DECODE_MARGIN_SECONDS).toInt() return decode(path, startMs = 0, clipSeconds) } diff --git a/app/src/main/java/org/terst/listen/fingerprint/SkipSummary.kt b/app/src/main/java/org/terst/listen/fingerprint/SkipSummary.kt index 3718f58..fedd3e4 100644 --- a/app/src/main/java/org/terst/listen/fingerprint/SkipSummary.kt +++ b/app/src/main/java/org/terst/listen/fingerprint/SkipSummary.kt @@ -2,12 +2,21 @@ package org.terst.listen.fingerprint import org.terst.listen.data.EpisodeEntity import org.terst.listen.data.EpisodeMidrollEntity +import org.terst.listen.data.insertedAdMs import org.terst.listen.data.ShowFingerprintEntity import org.terst.listen.data.ShowMidrollEntity import org.terst.listen.data.SubscriptionEntity -/** The three lines the episode screen shows about skipping. [breaks] is never empty. */ -data class EpisodeSkips(val intro: String, val outro: String, val breaks: List<String>) +/** + * What the episode screen shows about skipping. [breaks] is never empty; [insertedAds] is null + * when there is nothing to compare, which is any episode the player has not opened yet. + */ +data class EpisodeSkips( + val intro: String, + val outro: String, + val breaks: List<String>, + val insertedAds: String?, +) /** * Says in words what the skip detection has worked out, so it stops being invisible. @@ -19,6 +28,12 @@ data class EpisodeSkips(val intro: String, val outro: String, val breaks: List<S * * Pure, and the wording is pinned by tests: this is the only explanation of the feature the app * ever gives. + * + * It also reports how much advertising an episode carries, which costs nothing to know: a feed + * declares the ad-free length and the CDN serves something longer, so the difference is the + * ads. Peter's observation, and it is the one number here that needs no audio analysis -- + * useful as a check on the midroll detector, which should eventually account for roughly that + * much. */ object SkipSummary { @@ -82,6 +97,7 @@ object SkipSummary { ): EpisodeSkips { val breaks = ranges(ranges) return EpisodeSkips( + insertedAds = episode.insertedAdMs?.let { clock(it) }, intro = offset(episode.detectedIntroMs, show?.skipIntroMs ?: 0), outro = offset(episode.detectedOutroMs, show?.skipOutroMs ?: 0), breaks = when { diff --git a/app/src/main/java/org/terst/listen/playback/PlaybackAdvancer.kt b/app/src/main/java/org/terst/listen/playback/PlaybackAdvancer.kt index 51bef62..0e76075 100644 --- a/app/src/main/java/org/terst/listen/playback/PlaybackAdvancer.kt +++ b/app/src/main/java/org/terst/listen/playback/PlaybackAdvancer.kt @@ -7,6 +7,7 @@ import org.terst.listen.data.QueueRepository import org.terst.listen.data.SubscriptionDao import org.terst.listen.data.SubscriptionEntity import org.terst.listen.data.SubscriptionKind +import org.terst.listen.data.playbackDurationMs import org.terst.listen.feed.Chapter /** What the ticker needs to know about whatever is playing, read once per episode. */ @@ -153,7 +154,7 @@ class PlaybackAdvancer( show = show, startPositionMs = progress.resumePosition( episode.positionMs, - episode.durationMs, + episode.playbackDurationMs, effectiveIntroMs(episode, show), ), speed = PlaybackSpeed.forEpisode(episode.speedOverride, show?.playbackSpeed), diff --git a/app/src/main/java/org/terst/listen/queue/RuleEngine.kt b/app/src/main/java/org/terst/listen/queue/RuleEngine.kt index c668ff9..05e7227 100644 --- a/app/src/main/java/org/terst/listen/queue/RuleEngine.kt +++ b/app/src/main/java/org/terst/listen/queue/RuleEngine.kt @@ -2,6 +2,7 @@ package org.terst.listen.queue import org.terst.listen.data.EpisodeEntity import org.terst.listen.data.RuleEntity +import org.terst.listen.data.playbackDurationMs /** The rule that caught an episode, and the plain-words account of why. */ data class RuleMatch(val rule: RuleEntity, val reason: String) @@ -59,12 +60,12 @@ object RuleEngine { add(!episode.title.contains(it, ignoreCase = true) to """title has no "$it"""") } rule.minDurationMs?.let { limit -> - val actual = episode.durationMs + val actual = episode.playbackDurationMs add((actual != null && actual >= limit) to "duration ${HumanTime.duration(actual)} is at least ${HumanTime.duration(limit)}") } rule.maxDurationMs?.let { limit -> - val actual = episode.durationMs + val actual = episode.playbackDurationMs add((actual != null && actual <= limit) to "duration ${HumanTime.duration(actual)} is under ${HumanTime.duration(limit)}") } diff --git a/app/src/main/java/org/terst/listen/ui/EpisodeRow.kt b/app/src/main/java/org/terst/listen/ui/EpisodeRow.kt index 80c282c..3574d25 100644 --- a/app/src/main/java/org/terst/listen/ui/EpisodeRow.kt +++ b/app/src/main/java/org/terst/listen/ui/EpisodeRow.kt @@ -31,6 +31,7 @@ import androidx.compose.ui.unit.dp import coil3.compose.AsyncImage import org.terst.listen.data.DownloadState import org.terst.listen.data.EpisodeEntity +import org.terst.listen.data.playbackDurationMs import java.util.concurrent.TimeUnit /** @@ -125,7 +126,9 @@ private fun DownloadIcon(state: DownloadState, onDownload: () -> Unit) { internal fun EpisodeEntity.progressLabel(): String { val parts = mutableListOf<String>() publishedAtMs?.let { parts += formatReleaseDate(it) } - durationMs?.let { parts += formatDuration(it) } + // The measured length where it is known: the feed's is minutes short on any show using + // dynamic ad insertion, which is how Peter spotted the list and the player disagreeing. + playbackDurationMs?.let { parts += formatDuration(it) } if (positionMs > 0) parts += "${formatDuration(positionMs)} in" if (playedAtMs != null) parts += "played" if (downloadState == DownloadState.FAILED) parts += "download failed" diff --git a/app/src/main/java/org/terst/listen/ui/EpisodeScreen.kt b/app/src/main/java/org/terst/listen/ui/EpisodeScreen.kt index 09356f1..de828e2 100644 --- a/app/src/main/java/org/terst/listen/ui/EpisodeScreen.kt +++ b/app/src/main/java/org/terst/listen/ui/EpisodeScreen.kt @@ -180,6 +180,9 @@ private fun EpisodeDetail( private fun SkipReadout(skips: EpisodeSkips) { Column(verticalArrangement = Arrangement.spacedBy(2.dp)) { Text("Skips", style = MaterialTheme.typography.titleSmall) + // Free, and a check on the detector: whatever it finds should eventually add up to + // about this much. + skips.insertedAds?.let { ReadoutRow("Ads inserted", "~$it") } ReadoutRow("Intro", skips.intro) ReadoutRow("Outro", skips.outro) skips.breaks.forEachIndexed { index, line -> diff --git a/app/src/main/java/org/terst/listen/ui/MainActivity.kt b/app/src/main/java/org/terst/listen/ui/MainActivity.kt index a63082b..fedc34d 100644 --- a/app/src/main/java/org/terst/listen/ui/MainActivity.kt +++ b/app/src/main/java/org/terst/listen/ui/MainActivity.kt @@ -1,5 +1,7 @@ package org.terst.listen.ui +import android.content.Context +import android.content.Intent import android.os.Bundle import androidx.activity.ComponentActivity import androidx.activity.compose.setContent @@ -9,6 +11,9 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.material3.darkColorScheme import androidx.compose.material3.Surface import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.getValue import androidx.compose.ui.Modifier import androidx.navigation.compose.NavHost import androidx.navigation.compose.composable @@ -24,8 +29,41 @@ class MainActivity : ComponentActivity() { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) + playShared(intent) setContent { ListenTheme { ListenApp(viewModel, playerViewModel, rulesViewModel, opmlViewModel, showSettingsViewModel) } } } + + /** + * A second share while the app is already open reuses this instance, so the new intent + * arrives here rather than through onCreate -- which is the normal case once you are going + * back and forth with YouTube. + */ + override fun onNewIntent(intent: Intent) { + super.onNewIntent(intent) + setIntent(intent) + playShared(intent) + } + + private fun playShared(intent: Intent?) { + val episodeId = intent?.getStringExtra(EXTRA_PLAY_EPISODE_ID) ?: return + // Cleared so a rotation or a return to the app does not start it over again. + intent.removeExtra(EXTRA_PLAY_EPISODE_ID) + playerViewModel.playShared(episodeId) + } + + companion object { + private const val EXTRA_PLAY_EPISODE_ID = "org.terst.listen.PLAY_EPISODE_ID" + + /** Hands an episode to the app to start playing, used by the share sheet. */ + fun playIntent(context: Context, episodeId: String): Intent = + Intent(context, MainActivity::class.java) + .putExtra(EXTRA_PLAY_EPISODE_ID, episodeId) + .addFlags( + Intent.FLAG_ACTIVITY_NEW_TASK or + Intent.FLAG_ACTIVITY_CLEAR_TOP or + Intent.FLAG_ACTIVITY_SINGLE_TOP + ) + } } /** @@ -51,6 +89,8 @@ private fun ListenApp( ) { val nav = rememberNavController() + ShowPlayerWhenShared(player, nav) + NavHost(navController = nav, startDestination = "library") { composable("library") { LibraryScreen( @@ -105,6 +145,23 @@ private fun ListenApp( } /** + * Opens the player when playback was started from a share. + * + * The share activity has no UI and no navigation controller, and the app may not have been + * running at all when the link arrived, so it cannot navigate. It starts playback and this + * reacts once there is something to navigate. + */ +@Composable +private fun ShowPlayerWhenShared(player: PlayerViewModel, nav: androidx.navigation.NavHostController) { + val openPlayer by player.openPlayer.collectAsState() + LaunchedEffect(openPlayer) { + if (!openPlayer) return@LaunchedEffect + player.openPlayerHandled() + nav.navigate("player") + } +} + +/** * The episode detail route. * * Extracted because the nav graph grew past the length limit as screens were added; each route diff --git a/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt b/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt index fed4960..81d2ecc 100644 --- a/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt +++ b/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt @@ -105,6 +105,36 @@ class PlayerViewModel(app: Application) : AndroidViewModel(app) { super.onCleared() } + private val _openPlayer = MutableStateFlow(false) + + /** + * Set when playback was started from outside the UI, so the app can show the player. + * + * A share arrives at an activity that may not be running; there is no screen to navigate + * from. This lets the nav graph react once the UI does exist, rather than the share + * activity trying to drive navigation it knows nothing about. + */ + val openPlayer: StateFlow<Boolean> = _openPlayer.asStateFlow() + + fun openPlayerHandled() { + _openPlayer.value = false + } + + /** + * Starts an episode handed over from a share, at wherever the share said. + * + * The position is already on the row -- [org.terst.listen.data.SubscriptionRepository] + * writes the shared timestamp before this is called -- so this is the ordinary play path, + * including the yt-dlp fetch a YouTube episode needs before it has any audio at all. + */ + fun playShared(episodeId: String) { + viewModelScope.launch { + val episode = graph.episodes.getById(episodeId) ?: return@launch + _openPlayer.value = true + play(episode) + } + } + /** * Starts an episode the listener picked. * diff --git a/app/src/main/java/org/terst/listen/ui/ShareReceiverActivity.kt b/app/src/main/java/org/terst/listen/ui/ShareReceiverActivity.kt index 4c6c637..62ff95e 100644 --- a/app/src/main/java/org/terst/listen/ui/ShareReceiverActivity.kt +++ b/app/src/main/java/org/terst/listen/ui/ShareReceiverActivity.kt @@ -41,8 +41,17 @@ class ShareReceiverActivity : ComponentActivity() { is FeedResolver.Resolution.Failed -> reportAndFinish(resolved.message) is FeedResolver.Resolution.SingleEpisode -> { - graph.repository.addSingleEpisode(resolved) - reportAndFinish("Added ${resolved.title ?: "video"}") + val episode = graph.repository.addSingleEpisode(resolved) + // A link shared with a timestamp was shared at that moment on purpose, so + // going back to Listen should pick the audio up there rather than leave it + // somewhere in the library to be found. Without a timestamp the share is + // just "keep this", and interrupting whatever is playing would be rude. + if (episode != null && resolved.startPositionMs != null) { + startActivity(MainActivity.playIntent(this@ShareReceiverActivity, episode.id)) + finish() + } else { + reportAndFinish("Added ${resolved.title ?: "video"}") + } } is FeedResolver.Resolution.Feed -> { diff --git a/app/src/test/java/org/terst/listen/data/EpisodeMatcherTest.kt b/app/src/test/java/org/terst/listen/data/EpisodeMatcherTest.kt index 7012c30..82e7574 100644 --- a/app/src/test/java/org/terst/listen/data/EpisodeMatcherTest.kt +++ b/app/src/test/java/org/terst/listen/data/EpisodeMatcherTest.kt @@ -204,4 +204,29 @@ class EpisodeMatcherTest { ) assertEquals(2, result.inserts.map { it.id }.distinct().size) } + + /** + * The regression that motivated splitting the two duration columns. + * + * `durationMs` is feed metadata and is rewritten on every refresh, correctly. A measured + * length written into that same column therefore survived only until the next refresh put + * the feed's claim back -- silently, and on a show using dynamic ad insertion the claim is + * minutes short. + */ + @Test + fun `a refresh overwrites the feed's duration but not the measured one`() { + val existing = stored("a").copy(durationMs = 1_325_000, actualDurationMs = 1_774_000) + + val result = EpisodeMatcher.match( + subId, + listOf(existing), + listOf(parsed("a", title = "Renamed", durationMs = 1_325_000)), + nowMs = 5, + ) + + val merged = result.updates.single() + assertEquals("the feed's claim is still the feed's to set", 1_325_000L, merged.durationMs) + assertEquals("what the player measured survives", 1_774_000L, merged.actualDurationMs) + assertEquals(1_774_000L, merged.playbackDurationMs) + } } diff --git a/app/src/test/java/org/terst/listen/data/FakeDaos.kt b/app/src/test/java/org/terst/listen/data/FakeDaos.kt index 1e265f5..8b00c3e 100644 --- a/app/src/test/java/org/terst/listen/data/FakeDaos.kt +++ b/app/src/test/java/org/terst/listen/data/FakeDaos.kt @@ -106,12 +106,12 @@ class FakePlaybackStateDao(private val rows: MutableList<EpisodeEntity>) : Playb } override suspend fun updateDuration(id: String, durationMs: Long) { - replace(id) { it.copy(durationMs = durationMs) } + replace(id) { it.copy(actualDurationMs = durationMs) } } override suspend fun storedDurationMs(id: String): Long? { reads++ - return rows.firstOrNull { it.id == id }?.durationMs + return rows.firstOrNull { it.id == id }?.actualDurationMs } private fun replace(id: String, transform: (EpisodeEntity) -> EpisodeEntity) { @@ -178,6 +178,9 @@ class FakeEpisodeDao : EpisodeDao { override suspend fun getById(id: String) = rows.firstOrNull { it.id == id } + override suspend fun getByGuid(subscriptionId: String, guid: String) = + rows.firstOrNull { it.subscriptionId == subscriptionId && it.guid == guid } + override suspend fun insertAll(episodes: List<EpisodeEntity>) { episodes.forEach { candidate -> val clash = rows.any { diff --git a/app/src/test/java/org/terst/listen/data/SubscriptionRepositoryTest.kt b/app/src/test/java/org/terst/listen/data/SubscriptionRepositoryTest.kt index 3508fa3..ccede65 100644 --- a/app/src/test/java/org/terst/listen/data/SubscriptionRepositoryTest.kt +++ b/app/src/test/java/org/terst/listen/data/SubscriptionRepositoryTest.kt @@ -207,6 +207,79 @@ class SubscriptionRepositoryTest { assertEquals("https://www.youtube.com/watch?v=aaa", episode.mediaUrl) } + /** + * The point of the hand-off: a link shared from YouTube at 12:30 starts the audio at 12:30. + */ + @Test + fun `a shared timestamp becomes the starting position`() = runTest { + repo.addSingleEpisode(aVideo.copy(startPositionMs = 750_000)) + + assertEquals(750_000L, eps.rows.single().positionMs) + } + + /** + * The going-back-and-forth case, and the one that was broken. + * + * After the first hand-off the video is already in the library, and the insert ignores a + * clashing guid -- so sharing the same video again from a later point silently did nothing + * and playback resumed wherever Listen had last left it. Re-sharing is an explicit + * instruction about where to be. + */ + @Test + fun `re-sharing the same video at a new timestamp moves the position`() = runTest { + repo.addSingleEpisode(aVideo.copy(startPositionMs = 60_000)) + + repo.addSingleEpisode(aVideo.copy(startPositionMs = 900_000)) + + assertEquals("still one episode", 1, eps.rows.size) + assertEquals(900_000L, eps.rows.single().positionMs) + } + + /** Re-sharing without one is not an instruction to go back to the beginning. */ + @Test + fun `re-sharing with no timestamp leaves the position alone`() = runTest { + repo.addSingleEpisode(aVideo.copy(startPositionMs = 900_000)) + + repo.addSingleEpisode(aVideo.copy(startPositionMs = null)) + + assertEquals(900_000L, eps.rows.single().positionMs) + } + + /** + * Sharing a point you had already passed is still where you want to be -- it is how you go + * back to something, and an episode marked played must not stay played. + */ + @Test + fun `re-sharing an episode already played unplays it`() = runTest { + repo.addSingleEpisode(aVideo) + eps.markPlayed(eps.rows.single().id, 5_000) + + repo.addSingleEpisode(aVideo.copy(startPositionMs = 300_000)) + + val episode = eps.rows.single() + assertEquals(300_000L, episode.positionMs) + assertEquals(null, episode.playedAtMs) + } + + /** The caller needs it to start playing, which is the whole point of the hand-off. */ + @Test + fun `adding returns the episode`() = runTest { + val added = repo.addSingleEpisode(aVideo.copy(startPositionMs = 42_000)) + + assertEquals(eps.rows.single().id, added?.id) + assertEquals(42_000L, added?.positionMs) + } + + @Test + fun `re-sharing returns the episode that already existed`() = runTest { + val first = repo.addSingleEpisode(aVideo) + + val second = repo.addSingleEpisode(aVideo.copy(startPositionMs = 99_000)) + + assertEquals(first?.id, second?.id) + assertEquals(99_000L, second?.positionMs) + } + /** The container downloads through yt-dlp, so it has to be marked as a YouTube source. */ @Test fun `the container is a youtube subscription`() = runTest { diff --git a/app/src/test/java/org/terst/listen/fingerprint/SkipSummaryTest.kt b/app/src/test/java/org/terst/listen/fingerprint/SkipSummaryTest.kt index 1fa3b24..bd10ece 100644 --- a/app/src/test/java/org/terst/listen/fingerprint/SkipSummaryTest.kt +++ b/app/src/test/java/org/terst/listen/fingerprint/SkipSummaryTest.kt @@ -37,6 +37,8 @@ class SkipSummaryTest { detectedIntroMs: Long? = null, detectedOutroMs: Long? = null, midrollCheckedAtMs: Long? = null, + durationMs: Long? = null, + actualDurationMs: Long? = null, ) = EpisodeEntity( id = "e1", subscriptionId = "s1", @@ -48,6 +50,8 @@ class SkipSummaryTest { detectedIntroMs = detectedIntroMs, detectedOutroMs = detectedOutroMs, midrollCheckedAtMs = midrollCheckedAtMs, + durationMs = durationMs, + actualDurationMs = actualDurationMs, ) private fun showRow(skipIntroMs: Long = 0, skipOutroMs: Long = 0) = SubscriptionEntity( @@ -206,4 +210,41 @@ class SkipSummaryTest { SkipSummary.episode(episodeRow(), showRow(), ranges).breaks, ) } + + /** + * How much advertising is in an episode, for free. + * + * The feed declares the ad-free length and the CDN serves something longer; the gap is the + * ads. Radiolab's Grass Apes: 22:05 declared, 29:34 served. + */ + @Test + fun `the gap between declared and measured length is the advertising`() { + val skips = SkipSummary.episode( + episodeRow(durationMs = 1_325_000, actualDurationMs = 1_774_000), + showRow(), + emptyList(), + ) + + assertEquals("7:29", skips.insertedAds) + } + + /** Nothing to compare until the player has actually opened the file. */ + @Test + fun `an episode never played reports no advertising figure`() { + val skips = SkipSummary.episode(episodeRow(durationMs = 1_325_000), showRow(), emptyList()) + + assertEquals(null, skips.insertedAds) + } + + /** A file shorter than claimed is a wrong feed, not negative advertising. */ + @Test + fun `a file shorter than the feed claims reports nothing`() { + val skips = SkipSummary.episode( + episodeRow(durationMs = 1_774_000, actualDurationMs = 1_325_000), + showRow(), + emptyList(), + ) + + assertEquals(null, skips.insertedAds) + } } diff --git a/app/src/test/java/org/terst/listen/playback/PositionWriterTest.kt b/app/src/test/java/org/terst/listen/playback/PositionWriterTest.kt index 1af5279..db6e523 100644 --- a/app/src/test/java/org/terst/listen/playback/PositionWriterTest.kt +++ b/app/src/test/java/org/terst/listen/playback/PositionWriterTest.kt @@ -158,7 +158,7 @@ class PositionWriterTest { // both large and systematic. The player knows the truth as soon as it prepares, and that // is the only place the truth is available without downloading the file. - private fun withFeedDuration(ms: Long?) = FakeEpisodeDao().apply { + private fun withFeedDuration(ms: Long?, measured: Long? = null) = FakeEpisodeDao().apply { rows += EpisodeEntity( id = "ep-1", subscriptionId = "s1", @@ -168,6 +168,7 @@ class PositionWriterTest { mediaType = "audio/mpeg", discoveredAtMs = 0, durationMs = ms, + actualDurationMs = measured, ) } @@ -178,7 +179,9 @@ class PositionWriterTest { PositionWriter(playhead, playbackOf(dao)).capture() - assertEquals(29 * 60_000L + 34_000L, dao.rows.single().durationMs) + val row = dao.rows.single() + assertEquals("measured length recorded", 29 * 60_000L + 34_000L, row.actualDurationMs) + assertEquals("the feed's claim is left for the feed to own", 22 * 60_000L + 5_000L, row.durationMs) } @Test @@ -188,18 +191,18 @@ class PositionWriterTest { PositionWriter(playhead, playbackOf(dao)).capture() - assertEquals(1_774_000L, dao.rows.single().durationMs) + assertEquals(1_774_000L, dao.rows.single().actualDurationMs) } /** Encoders and containers disagree by a second all the time; that is not worth a write. */ @Test fun `a trivial disagreement is left alone`() = runTest { - val dao = withFeedDuration(1_774_000) + val dao = withFeedDuration(1_325_000, measured = 1_774_000) val playhead = FakePlayhead(positionMs = 30_000, durationMs = 1_775_500) PositionWriter(playhead, playbackOf(dao)).capture() - assertEquals("unchanged", 1_774_000L, dao.rows.single().durationMs) + assertEquals("unchanged", 1_774_000L, dao.rows.single().actualDurationMs) } /** Before the player has prepared there is no duration to believe. */ @@ -210,7 +213,8 @@ class PositionWriterTest { PositionWriter(playhead, playbackOf(dao)).capture() - assertEquals(1_325_000L, dao.rows.single().durationMs) + assertEquals("nothing measured", null, dao.rows.single().actualDurationMs) + assertEquals("and the feed's claim untouched", 1_325_000L, dao.rows.single().durationMs) } /** @@ -247,7 +251,7 @@ class PositionWriterTest { playhead.positionMs = 60_000 writer.capture() - assertEquals(1_774_000L, dao.rows.single().durationMs) + assertEquals(1_774_000L, dao.rows.single().actualDurationMs) } @Test @@ -256,7 +260,7 @@ class PositionWriterTest { rows += EpisodeEntity( id = "ep-2", subscriptionId = "s1", guid = "g2", title = "Two", mediaUrl = "https://cdn/2.mp3", mediaType = "audio/mpeg", discoveredAtMs = 0, - durationMs = 600_000, + durationMs = 600_000, actualDurationMs = 600_000, ) } val playhead = FakePlayhead(positionMs = 30_000, durationMs = 1_774_000) @@ -268,6 +272,6 @@ class PositionWriterTest { playhead.durationMs = 999_000 writer.capture() - assertEquals(999_000L, dao.rows.first { it.id == "ep-2" }.durationMs) + assertEquals(999_000L, dao.rows.first { it.id == "ep-2" }.actualDurationMs) } } |