diff options
| author | Peter Stone <thepeterstone@gmail.com> | 2026-09-25 17:43:33 +0000 |
|---|---|---|
| committer | Peter Stone <thepeterstone@gmail.com> | 2026-09-25 17:43:33 +0000 |
| commit | 3f650131627f6737e01585b97519e03413119b44 (patch) | |
| tree | 04d3f82015887c35047a74d539afa1023e8eb929 /app/src | |
| parent | 593ee7740de24aae22ddec1dc5a2e083fff06bd3 (diff) | |
Simple skip: manual offsets and chapter-marked breaks
The last of Phase 2's three tracks, and the part Peter asked for in place of the
audio fingerprinting, which stays deferred.
Two tiers. Manual per-show offsets start each fresh episode past the intro and
treat it as finished before the outro; resuming something part-heard still lands
where it was left, and an offset longer than the episode is dropped rather than
obeyed, because someone typing minutes where seconds were meant should not have
a show start at the end. The fields have been on SubscriptionEntity since phase
1 with nothing reading them.
Chapters are the exact tier: where a show publishes a Podcasting 2.0 chapter
file, the player lists it and taps seek. Breaks the publisher marked `toc:
false` can be skipped, opt-in per show. Nothing is guessed from chapter titles
-- a show discussing its own sponsorship deal has a chapter called "Sponsors"
that is the actual programme, and a skip feature that eats an episode is worse
than none. A break whose end cannot be worked out plays rather than throwing the
playhead somewhere arbitrary.
Two real bugs surfaced while verifying this on a device, both older than this
work:
A start position handed to `seekTo` after `prepare()` is silently dropped across
the MediaController-to-service boundary. The intro offset began at zero, and so,
all along, did every resume -- an episode left half-heard restarted from the
top. Nothing failed and nothing logged. It now goes to `setMediaItem`, verified
on a device both ways: a fresh episode starts at the offset, a stored position
resumes at 150s and plays on from there.
Finishing an episode did not stop playback when the queue was empty, because
until now the file had always ended anyway. With an outro offset the episode
finishes while audio is still playing, so it marked the episode played and then
played the credits regardless.
Schema v3 -> v4: the chapter file cached per episode, and the per-show opt-in.
The migration test now covers 1 -> 4 straight through and asserts the opt-in
arrives off -- defaulting it on would have started removing audio from every
show at once.
Per-show settings moved to their own view model. Drip, the archive fetch and
both skip settings are one concern used by one screen, and ListenViewModel had
become the entry point for the library, the queue, episode actions and these as
well. It also gets its own busy flag: fetching an archive and refreshing a feed
are different operations, and one spinner for both had the top bar claiming a
refresh was running when it was not.
Verified on a device against a real feed throughout: a 60-second intro offset
starts the episode at 60s, an outro offset finishes it early and stops the
audio, and a seeded chapter file with a five-second marked break plays 0-5s then
jumps to 45s with the chapter list rendering alongside.
No permission changes. 364 unit tests green, detekt and lint clean, coverage
65.30 -> 66.23.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B
Diffstat (limited to 'app/src')
22 files changed, 1105 insertions, 129 deletions
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 6c40126..0eeab50 100644 --- a/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt +++ b/app/src/androidTest/java/org/terst/listen/data/MigrationTest.kt @@ -110,4 +110,42 @@ class MigrationTest { assertEquals(true, cursor.isNull(1)) } } + + /** The whole chain, which is the path a phone that skipped two builds actually takes. */ + @Test + fun migrates1To4KeepingEpisodesAndDefaultingChapterSkip() { + helper.createDatabase(name, 1).use { db -> + db.execSQL( + """INSERT INTO subscriptions + (id, feedUrl, kind, title, addedAtMs, skipIntroMs, skipOutroMs) + VALUES ('s1', 'https://example.com/feed.xml', 'PODCAST', 'A Show', 1000, 47000, 0)""" + ) + db.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, + 555555, 'NONE')""" + ) + } + + val db = helper.runMigrationsAndValidate( + name, 4, true, + AppDatabase.MIGRATION_1_2, AppDatabase.MIGRATION_2_3, AppDatabase.MIGRATION_3_4, + ) + + // Ad-chapter skipping must arrive off. It is the one part of the feature that can remove + // audio nobody has heard, and defaulting it on would do that to every show at once. + db.query("SELECT skipIntroMs, skipAdChapters FROM subscriptions WHERE id = 's1'").use { cursor -> + cursor.moveToFirst() + assertEquals("a manual offset set before the upgrade must survive", 47000L, cursor.getLong(0)) + assertEquals(0, cursor.getInt(1)) + } + + db.query("SELECT positionMs, chaptersJson FROM episodes WHERE id = 'e1'").use { cursor -> + cursor.moveToFirst() + assertEquals(555555L, cursor.getLong(0)) + assertEquals(true, cursor.isNull(1)) + } + } } 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 f779b06..7897678 100644 --- a/app/src/main/java/org/terst/listen/data/AppDatabase.kt +++ b/app/src/main/java/org/terst/listen/data/AppDatabase.kt @@ -25,7 +25,7 @@ class Converters { QueueEntity::class, RuleEntity::class, ], - version = 3, + version = 4, exportSchema = true, ) @TypeConverters(Converters::class) @@ -82,6 +82,22 @@ abstract class AppDatabase : RoomDatabase() { } } + /** + * Chapters: the fetched Podcasting 2.0 file per episode, and the per-show opt-in to + * skipping the breaks it marks. + * + * The chapter list is a JSON blob on the episode rather than a table of its own. It is + * only ever read whole, for one episode, and a table would buy joins nothing needs. + */ + val MIGRATION_3_4 = object : Migration(3, 4) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL("ALTER TABLE `episodes` ADD COLUMN `chaptersJson` TEXT") + db.execSQL( + "ALTER TABLE `subscriptions` ADD COLUMN `skipAdChapters` INTEGER NOT NULL DEFAULT 0" + ) + } + } + fun get(context: Context): AppDatabase = instance ?: synchronized(this) { instance ?: build(context.applicationContext).also { instance = it } } @@ -90,7 +106,7 @@ abstract class AppDatabase : RoomDatabase() { Room.databaseBuilder(context, AppDatabase::class.java, "listen.db") // Listed one by one rather than spread from an array: every migration here is a // deliberate addition, and the list is the record of them. - .addMigrations(MIGRATION_1_2, MIGRATION_2_3) + .addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4) // No fallbackToDestructiveMigration. A listener's playback positions and play // history are not reproducible from anywhere, so a missing migration must fail // loudly rather than silently wipe the library. 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 6b2d334..608cf77 100644 --- a/app/src/main/java/org/terst/listen/data/Daos.kt +++ b/app/src/main/java/org/terst/listen/data/Daos.kt @@ -59,6 +59,13 @@ interface SubscriptionDao { @Query("UPDATE subscriptions SET playbackSpeed = :speed WHERE id = :id") suspend fun updateSpeed(id: String, speed: Float) + @Query("UPDATE subscriptions SET skipAdChapters = :enabled WHERE id = :id") + suspend fun setSkipAdChapters(id: String, enabled: Boolean) + + /** Tier 2 of the skip feature: how much of the top and tail of every episode to pass over. */ + @Query("UPDATE subscriptions SET skipIntroMs = :introMs, skipOutroMs = :outroMs WHERE id = :id") + suspend fun updateSkips(id: String, introMs: Long, outroMs: Long) + @Query("SELECT * FROM subscriptions WHERE dripEnabled = 1") suspend fun dripEnabled(): List<SubscriptionEntity> @@ -188,6 +195,9 @@ interface EpisodeDao { ) suspend fun markPlayedUpTo(subscriptionId: String, publishedAtMs: Long, atMs: Long) + @Query("UPDATE episodes SET chaptersJson = :json WHERE id = :id") + suspend fun setChapters(id: String, json: String?) + /** Records which auto-queue rule claimed an episode, why, and any speed it asked for. */ @Query( """UPDATE episodes SET appliedRuleId = :ruleId, appliedRuleReason = :reason, 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 f366aab..359a626 100644 --- a/app/src/main/java/org/terst/listen/data/Entities.kt +++ b/app/src/main/java/org/terst/listen/data/Entities.kt @@ -62,6 +62,15 @@ data class SubscriptionEntity( @ColumnInfo(defaultValue = "1") val dripOldestFirst: Boolean = true, /** When the last batch was released. Null means drip has never paid out for this show. */ val dripLastReleaseAtMs: Long? = null, + + /** + * Whether to pass over chapters the publisher marked `toc: false`. + * + * Off by default and opt-in per show. Acting on someone else's metadata to remove audio a + * listener has not heard is the one part of this feature that can eat an episode, so it is + * never on unless asked for. + */ + @ColumnInfo(defaultValue = "0") val skipAdChapters: Boolean = false, ) /** One a day, the cadence a drip subscription is for. */ @@ -123,6 +132,15 @@ data class EpisodeEntity( val appliedRuleReason: String? = null, /** A speed set for this episode alone by a rule, taking precedence over the show's. */ val speedOverride: Float? = null, + + /** + * The fetched Podcasting 2.0 chapter file, verbatim, or null if there is none yet. + * + * Stored as the raw document rather than parsed rows: it is only ever read whole, for one + * episode, and keeping the original means a parser improvement re-reads real data rather + * than whatever an earlier version managed to understand. + */ + val chaptersJson: String? = null, ) /** What an auto-queue rule does to an episode that matches it. */ 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 4f39c13..773aa96 100644 --- a/app/src/main/java/org/terst/listen/data/Repositories.kt +++ b/app/src/main/java/org/terst/listen/data/Repositories.kt @@ -6,6 +6,7 @@ import coil3.network.okhttp.OkHttpNetworkFetcherFactory import okhttp3.OkHttpClient import org.terst.listen.download.ChaquopyYouTubeSource import org.terst.listen.download.EpisodeDownloader +import org.terst.listen.feed.ChapterFetcher import org.terst.listen.feed.FeedFetcher import org.terst.listen.feed.FeedResolver import org.terst.listen.queue.DripReleaser @@ -56,6 +57,8 @@ class AppGraph(private val context: Context) { val drip: DripReleaser by lazy { DripReleaser(subscriptions, episodes, queue) } + val chapters: ChapterFetcher by lazy { ChapterFetcher(client, episodes) } + val repository: SubscriptionRepository by lazy { SubscriptionRepository( subscriptions, diff --git a/app/src/main/java/org/terst/listen/feed/ChapterFetcher.kt b/app/src/main/java/org/terst/listen/feed/ChapterFetcher.kt new file mode 100644 index 0000000..018cec4 --- /dev/null +++ b/app/src/main/java/org/terst/listen/feed/ChapterFetcher.kt @@ -0,0 +1,57 @@ +package org.terst.listen.feed + +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import okhttp3.OkHttpClient +import okhttp3.Request +import org.terst.listen.data.EpisodeDao +import org.terst.listen.data.EpisodeEntity +import java.io.IOException + +/** + * Fetches an episode's Podcasting 2.0 chapter file, once, and caches it on the episode. + * + * Failure is silent by design, unlike a feed refresh. A show with no chapters is the normal + * case, not a fault, and a banner every time a publisher's chapter URL 404s would be noise on + * something the listener never asked for. + */ +class ChapterFetcher( + private val client: OkHttpClient, + private val episodes: EpisodeDao, +) { + + /** Returns the chapters for an episode, fetching them the first time they are wanted. */ + suspend fun chapters(episode: EpisodeEntity): List<Chapter> { + episode.chaptersJson?.let { return ChapterParser.parse(it) } + val url = episode.chaptersUrl ?: return emptyList() + + // The reason a chapter file could not be fetched is deliberately not surfaced: a show + // with no usable chapters is the normal case, not a fault, and a banner every time a + // publisher's chapter URL 404s would be noise on something nobody asked for. + val body = fetch(url).getOrNull() ?: return emptyList() + val parsed = ChapterParser.parse(body) + // Cached even when it parses to nothing, so an episode whose chapter file is junk is not + // re-fetched on every play. + episodes.setChapters(episode.id, body) + return parsed + } + + private suspend fun fetch(url: String): Result<String> = withContext(Dispatchers.IO) { + val request = Request.Builder().url(url).build() + try { + client.newCall(request).execute().use { response -> + val body = response.body?.string() + when { + !response.isSuccessful -> Result.failure(IOException("HTTP ${response.code}")) + body == null -> Result.failure(IOException("empty chapter file")) + else -> Result.success(body) + } + } + } catch (e: IOException) { + Result.failure(e) + } catch (e: IllegalArgumentException) { + // A chapter URL that is not a URL at all; the feed said so, not us. + Result.failure(e) + } + } +} diff --git a/app/src/main/java/org/terst/listen/feed/ChapterParser.kt b/app/src/main/java/org/terst/listen/feed/ChapterParser.kt new file mode 100644 index 0000000..e695436 --- /dev/null +++ b/app/src/main/java/org/terst/listen/feed/ChapterParser.kt @@ -0,0 +1,62 @@ +package org.terst.listen.feed + +import kotlinx.serialization.Serializable +import kotlinx.serialization.json.Json + +/** One chapter of an episode, as a listener would navigate it. */ +data class Chapter( + val startMs: Long, + val endMs: Long? = null, + val title: String? = null, + /** + * Whether the publisher counts this as part of the episode. + * + * `toc: false` is how Podcasting 2.0 marks a section that is not really content -- in + * practice, almost always an ad read. + */ + val inToc: Boolean = true, +) + +/** + * Parses a Podcasting 2.0 chapters file. + * + * Tier 1 of the skip feature, and the only exact one: where a show marks its own breaks, + * detection is just reading what it said. Everything here degrades to an empty list rather than + * throwing, because the input is a JSON file fetched from a URL in a stranger's feed and a + * malformed one must not take playback down with it. + */ +object ChapterParser { + + private const val MS_PER_SECOND = 1000.0 + + private val json = Json { ignoreUnknownKeys = true } + + fun parse(raw: String): List<Chapter> { + val dto = runCatching { json.decodeFromString<Document>(raw) }.getOrNull() ?: return emptyList() + return dto.chapters.orEmpty() + // A chapter with no start time has nowhere to sit on the timeline. + .mapNotNull { chapter -> + chapter.startTime?.let { + Chapter( + startMs = (it * MS_PER_SECOND).toLong(), + endMs = chapter.endTime?.let { end -> (end * MS_PER_SECOND).toLong() }, + title = chapter.title?.takeIf(String::isNotBlank), + inToc = chapter.toc ?: true, + ) + } + } + // Publishers do emit them out of order, and everything downstream assumes otherwise. + .sortedBy { it.startMs } + } + + @Serializable + private data class Document(val chapters: List<Entry>? = null) + + @Serializable + private data class Entry( + val startTime: Double? = null, + val endTime: Double? = null, + val title: String? = null, + val toc: Boolean? = null, + ) +} diff --git a/app/src/main/java/org/terst/listen/playback/AdChapters.kt b/app/src/main/java/org/terst/listen/playback/AdChapters.kt new file mode 100644 index 0000000..bf1a2b1 --- /dev/null +++ b/app/src/main/java/org/terst/listen/playback/AdChapters.kt @@ -0,0 +1,31 @@ +package org.terst.listen.playback + +import org.terst.listen.feed.Chapter + +/** + * Whether the playhead is sitting in a section the publisher marked as not-content, and where + * to jump to if it is. + * + * Conservative by design. The design plan's warning stands: a skip feature that silently eats + * content is worse than none. So the only signal trusted here is `toc: false` -- the publisher + * saying outright that a section is not part of the episode -- and never the chapter title. A + * show discussing its own sponsorship deal has a chapter called "Sponsors" that is the actual + * programme, and guessing from titles is how a skip feature starts deleting episodes. + */ +object AdChapters { + + /** Where to jump to if [positionMs] is inside a marked break, or null to carry on playing. */ + fun skipTarget(chapters: List<Chapter>, positionMs: Long, durationMs: Long?): Long? { + val index = chapters.indexOfLast { it.startMs <= positionMs } + if (index < 0) return null + + val chapter = chapters[index] + if (chapter.inToc) return null + + // Its end, the next chapter's start, or the end of the episode -- in that order. With + // none of the three there is nowhere to jump to, so the break plays rather than the + // playhead being thrown somewhere arbitrary. + val end = chapter.endMs ?: chapters.getOrNull(index + 1)?.startMs ?: durationMs ?: return null + return end.takeIf { it > positionMs } + } +} diff --git a/app/src/main/java/org/terst/listen/playback/PlaybackProgress.kt b/app/src/main/java/org/terst/listen/playback/PlaybackProgress.kt index ab34c28..bdec56d 100644 --- a/app/src/main/java/org/terst/listen/playback/PlaybackProgress.kt +++ b/app/src/main/java/org/terst/listen/playback/PlaybackProgress.kt @@ -22,30 +22,56 @@ class PlaybackProgress( private val resumeFloorMs: Long = 5_000, ) { - fun isComplete(positionMs: Long, durationMs: Long?): Boolean { + /** + * [skipOutroMs] moves the finish line earlier, for a show whose last minutes are always the + * same credits. An offset that would swallow the whole episode is ignored rather than + * obeyed: someone typing minutes where seconds were meant should not mark a show played the + * moment it starts. + */ + fun isComplete(positionMs: Long, durationMs: Long?, skipOutroMs: Long = 0): Boolean { if (durationMs == null || durationMs <= 0) return false - val remaining = durationMs - positionMs + val end = effectiveEnd(durationMs, skipOutroMs) + val remaining = end - positionMs // Effectively at the end. if (remaining <= minRemainingMs) return true // Otherwise past the threshold, but only if what's left is plausibly credits rather // than content. A percentage on its own scales wrongly with episode length. - return positionMs >= durationMs * completionThreshold && remaining <= maxCreditsMs + return positionMs >= end * completionThreshold && remaining <= maxCreditsMs } /** + * Where an episode ends as far as this app is concerned. + * + * An outro offset at or past the duration is nonsense and is dropped; anything shorter + * brings the end forward. + */ + private fun effectiveEnd(durationMs: Long, skipOutroMs: Long): Long = + if (skipOutroMs <= 0 || skipOutroMs >= durationMs) durationMs else durationMs - skipOutroMs + + /** * Where to start playing. * * A stored position past the end means the media changed under us -- a re-encode, or a feed * whose declared duration was wrong. Restarting beats seeking past the end and stalling. + * + * [skipIntroMs] applies only to a start from the beginning, never to a resume: coming back + * to something half-heard must land where it was left, not at the show's theme-tune offset. + * An offset longer than the episode is dropped, for the same reason the outro one is. */ - fun resumePosition(storedMs: Long, durationMs: Long?): Long = when { - storedMs < resumeFloorMs -> 0 + fun resumePosition(storedMs: Long, durationMs: Long?, skipIntroMs: Long = 0): Long = when { + storedMs < resumeFloorMs -> start(durationMs, skipIntroMs) durationMs == null -> storedMs - storedMs >= durationMs -> 0 - isComplete(storedMs, durationMs) -> 0 + storedMs >= durationMs -> start(durationMs, skipIntroMs) + isComplete(storedMs, durationMs) -> start(durationMs, skipIntroMs) else -> storedMs } + private fun start(durationMs: Long?, skipIntroMs: Long): Long = when { + skipIntroMs <= 0 -> 0 + durationMs != null && skipIntroMs >= durationMs -> 0 + else -> skipIntroMs + } + /** * Whether the position has moved enough to write. * diff --git a/app/src/main/java/org/terst/listen/playback/PlayerConnection.kt b/app/src/main/java/org/terst/listen/playback/PlayerConnection.kt index d1e788e..e44ff35 100644 --- a/app/src/main/java/org/terst/listen/playback/PlayerConnection.kt +++ b/app/src/main/java/org/terst/listen/playback/PlayerConnection.kt @@ -36,6 +36,9 @@ class PlayerConnection(private val context: Context) { private var controller: MediaController? = null + /** The resume rules. A field rather than a parameter: nothing ever passed a different one. */ + private val progress = PlaybackProgress() + /** * Fired when an episode plays to its end, with that episode's id. * @@ -87,22 +90,27 @@ class PlayerConnection(private val context: Context) { } /** - * Starts an episode, seeking to where the listener got to. + * Starts an episode where the listener should hear it. + * + * The decision belongs to PlaybackProgress, not here: a stored position past the end has to + * restart rather than seek past the end and stall, and a fresh start honours the show's + * intro offset. * - * The resume decision belongs to PlaybackProgress, not here: a stored position past the end - * has to restart rather than seek past the end and stall. + * The start position goes to `setMediaItem`, not to a `seekTo` after `prepare()`. Seeking + * afterwards is silently dropped across the controller-to-service boundary -- verified on a + * device: an episode set to start sixty seconds in began at zero, and so, all along, did + * every resume. Nothing failed and nothing logged; playback simply started at the top. */ fun play( item: MediaItem, storedPositionMs: Long, durationMs: Long?, speed: Float, - progress: PlaybackProgress = PlaybackProgress(), + skipIntroMs: Long = 0, ) { val player = controller ?: return - player.setMediaItem(item) + player.setMediaItem(item, progress.resumePosition(storedPositionMs, durationMs, skipIntroMs)) player.prepare() - player.seekTo(progress.resumePosition(storedPositionMs, durationMs)) player.applySpeed(speed, speed) player.play() publish() diff --git a/app/src/main/java/org/terst/listen/ui/ListenViewModel.kt b/app/src/main/java/org/terst/listen/ui/ListenViewModel.kt index 23ba532..553f376 100644 --- a/app/src/main/java/org/terst/listen/ui/ListenViewModel.kt +++ b/app/src/main/java/org/terst/listen/ui/ListenViewModel.kt @@ -9,7 +9,6 @@ import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch -import org.terst.listen.data.ArchiveOutcome import org.terst.listen.data.EpisodeEntity import org.terst.listen.data.Graph import org.terst.listen.data.RefreshOutcome @@ -23,20 +22,6 @@ import org.terst.listen.feed.YouTubeUrls /** Transient message for the UI. Errors are surfaced, never only logged. */ data class Banner(val text: String, val isError: Boolean = false) -/** - * An archive walk, reported in full. - * - * A partial walk is flagged as an error even though episodes did arrive: a chain that stopped - * early means the rest of the catalogue is missing, and a plain success message would leave the - * show looking permanently short with no indication why. - */ -private fun ArchiveOutcome.Fetched.toBanner(): Banner { - val found = "$newEpisodes older episode(s) from $pages page(s)" - return stoppedEarly - ?.let { Banner("$found, then stopped: $it", isError = true) } - ?: Banner(found) -} - class ListenViewModel(app: Application) : AndroidViewModel(app) { private val graph = Graph.get(app) @@ -149,59 +134,6 @@ class ListenViewModel(app: Application) : AndroidViewModel(app) { } } - // --- slow drip ------------------------------------------------------------------------- - - /** - * Turns slow-drip on for a show, or adjusts a running one, and pays out anything owed. - * - * Releasing straight away is deliberate: flipping the switch and seeing nothing happen for - * a day reads as the feature not working. - */ - fun enableDrip(subscriptionId: String, intervalMs: Long, count: Int, oldestFirst: Boolean) { - viewModelScope.launch { - graph.drip.enable(subscriptionId, intervalMs, count, oldestFirst) - val released = graph.subscriptions.getById(subscriptionId)?.let { graph.drip.release(it) } ?: 0 - _banner.value = if (released > 0) { - Banner("Dripping — $released episode(s) queued") - } else { - Banner("Dripping — nothing to release yet") - } - } - } - - fun disableDrip(subscriptionId: String) { - viewModelScope.launch { - graph.drip.disable(subscriptionId) - _banner.value = Banner("Drip off — the whole catalogue is back") - } - } - - /** - * Pulls a show's back catalogue by walking its RFC 5005 archive pages. - * - * Reported in detail rather than as a bare success: plenty of publishers do not implement - * paging at all, and "nothing happened" needs to be distinguishable from "it failed". - */ - fun fetchArchive(subscriptionId: String) { - if (_refreshing.value) return - _refreshing.value = true - viewModelScope.launch { - val subscription = graph.subscriptions.getById(subscriptionId) - if (subscription == null) { - _refreshing.value = false - return@launch - } - val outcome = graph.repository.fetchArchive(subscription) - _refreshing.value = false - _banner.value = when (outcome) { - ArchiveOutcome.NoArchive -> - Banner("This feed publishes no archive pages, so there is nothing older to fetch") - is ArchiveOutcome.Failed -> Banner(outcome.message, isError = true) - is ArchiveOutcome.Fetched -> outcome.toBanner() - } - } - } - fun unsubscribe(id: String) { viewModelScope.launch { graph.subscriptions.delete(id) } } 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 9632b02..a63082b 100644 --- a/app/src/main/java/org/terst/listen/ui/MainActivity.kt +++ b/app/src/main/java/org/terst/listen/ui/MainActivity.kt @@ -20,10 +20,11 @@ class MainActivity : ComponentActivity() { private val playerViewModel: PlayerViewModel by viewModels() private val rulesViewModel: RulesViewModel by viewModels() private val opmlViewModel: OpmlViewModel by viewModels() + private val showSettingsViewModel: ShowSettingsViewModel by viewModels() override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) - setContent { ListenTheme { ListenApp(viewModel, playerViewModel, rulesViewModel, opmlViewModel) } } + setContent { ListenTheme { ListenApp(viewModel, playerViewModel, rulesViewModel, opmlViewModel, showSettingsViewModel) } } } } @@ -46,6 +47,7 @@ private fun ListenApp( player: PlayerViewModel, rulesViewModel: RulesViewModel, opmlViewModel: OpmlViewModel, + showSettings: ShowSettingsViewModel, ) { val nav = rememberNavController() @@ -88,6 +90,7 @@ private fun ListenApp( SubscriptionScreen( vm = viewModel, player = player, + settings = showSettings, subscriptionId = id, nav = SubscriptionNav( openEpisode = { nav.navigate("episode/$it") }, diff --git a/app/src/main/java/org/terst/listen/ui/PlayerScreen.kt b/app/src/main/java/org/terst/listen/ui/PlayerScreen.kt index 1af21c5..e012834 100644 --- a/app/src/main/java/org/terst/listen/ui/PlayerScreen.kt +++ b/app/src/main/java/org/terst/listen/ui/PlayerScreen.kt @@ -26,18 +26,23 @@ import androidx.compose.material3.Scaffold import androidx.compose.material3.Slider import androidx.compose.material3.Text import androidx.compose.material3.TopAppBar +import androidx.compose.foundation.clickable +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.verticalScroll import androidx.compose.runtime.Composable import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableFloatStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue +import androidx.compose.foundation.layout.ColumnScope import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp import coil3.compose.AsyncImage +import org.terst.listen.feed.Chapter /** Offered speeds. Beyond 2x speech becomes hard to follow, so the list stops there. */ private val SPEEDS = listOf(1.0f, 1.25f, 1.5f, 1.75f, 2.0f) @@ -47,6 +52,7 @@ private val SPEEDS = listOf(1.0f, 1.25f, 1.5f, 1.75f, 2.0f) fun PlayerScreen(player: PlayerViewModel, onBack: () -> Unit) { val nowPlaying by player.nowPlaying.collectAsState() val show by player.currentShow.collectAsState() + val chapters by player.chapters.collectAsState() Scaffold( topBar = { @@ -93,6 +99,56 @@ fun PlayerScreen(player: PlayerViewModel, onBack: () -> Unit) { if (show != null) { SpeedRow(current = show?.playbackSpeed ?: nowPlaying.speed, onSelect = player::setShowSpeed) } + + ChapterList( + chapters = chapters, + positionMs = nowPlaying.positionMs, + onSeek = player::seekTo, + ) + } + } +} + +/** + * The episode's chapters, if it has any. + * + * Renders nothing at all when it does not, which is most shows -- an empty "Chapters" heading on + * every episode would be a permanent reminder of a feature that rarely applies. + */ +@Composable +private fun ColumnScope.ChapterList(chapters: List<Chapter>, positionMs: Long, onSeek: (Long) -> Unit) { + if (chapters.isEmpty()) return + + val currentIndex = chapters.indexOfLast { it.startMs <= positionMs } + + Column(Modifier.fillMaxWidth().weight(1f, fill = false).verticalScroll(rememberScrollState())) { + Text("Chapters", style = MaterialTheme.typography.titleSmall) + chapters.forEachIndexed { index, chapter -> + Row( + Modifier.fillMaxWidth().clickable { onSeek(chapter.startMs) }.padding(vertical = 6.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Text( + formatClock(chapter.startMs), + style = MaterialTheme.typography.bodySmall, + modifier = Modifier.padding(end = 12.dp), + ) + Text( + chapter.title ?: "Chapter ${index + 1}", + style = if (index == currentIndex) { + MaterialTheme.typography.bodyMedium + } else { + MaterialTheme.typography.bodySmall + }, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.weight(1f), + ) + // Marked so a skipped section is explained rather than just vanishing. + if (!chapter.inToc) { + Text("break", style = MaterialTheme.typography.bodySmall) + } + } } } } 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 106bae4..c91a52f 100644 --- a/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt +++ b/app/src/main/java/org/terst/listen/ui/PlayerViewModel.kt @@ -11,7 +11,10 @@ import kotlinx.coroutines.launch import org.terst.listen.data.EpisodeEntity import org.terst.listen.data.Graph import org.terst.listen.data.SubscriptionEntity +import org.terst.listen.feed.Chapter +import org.terst.listen.playback.AdChapters import org.terst.listen.playback.NowPlaying +import org.terst.listen.playback.PlaybackProgress import org.terst.listen.playback.PlaybackSpeed import org.terst.listen.playback.PlayerConnection import org.terst.listen.playback.SleepTimer @@ -42,6 +45,15 @@ class PlayerViewModel(app: Application) : AndroidViewModel(app) { } val nowPlaying: StateFlow<NowPlaying> = connection.nowPlaying + private val progress = PlaybackProgress() + + /** The episode whose outro has already been acted on, so the tick fires once per episode. */ + private var outroHandledFor: String? = null + + /** Chapters for whatever is playing, empty when the show publishes none. */ + private val _chapters = MutableStateFlow<List<Chapter>>(emptyList()) + val chapters: StateFlow<List<Chapter>> = _chapters.asStateFlow() + /** The show behind whatever is currently playing, for the player screen's speed control. */ private val _currentShow = MutableStateFlow<SubscriptionEntity?>(null) val currentShow: StateFlow<SubscriptionEntity?> = _currentShow.asStateFlow() @@ -52,10 +64,48 @@ class PlayerViewModel(app: Application) : AndroidViewModel(app) { while (true) { delay(PROGRESS_TICK_MS) connection.publish() + finishIfPastTheOutro() + skipPastAnAdChapter() } } } + /** + * Ends an episode early when its show has an outro offset set. + * + * Media3 reports STATE_ENDED at the real end of the file and nowhere else, so an offset that + * moves the finish line needs the tick to notice it. Guarded by episode id because the tick + * keeps running after the queue advances, and firing twice would mark the next episode + * played before a note of it had been played. + */ + private fun finishIfPastTheOutro() { + val outroMs = _currentShow.value?.skipOutroMs ?: return + if (outroMs <= 0) return + val playing = connection.nowPlaying.value + val episodeId = playing.episodeId ?: return + if (episodeId == outroHandledFor) return + if (!progress.isComplete(playing.positionMs, playing.durationMs, outroMs)) return + + outroHandledFor = episodeId + onFinished(episodeId) + } + + /** + * Jumps over a chapter the publisher marked as not-content, when the show opts in. + * + * A tick means up to a second of the break plays before the jump. Living with that beats + * pre-empting it: seeking early on a chapter boundary that turns out to be wrong removes + * audio the listener wanted, and this feature only earns its place by never doing that. + */ + private fun skipPastAnAdChapter() { + if (_currentShow.value?.skipAdChapters != true) return + val playing = connection.nowPlaying.value + if (!playing.isPlaying) return + val target = AdChapters.skipTarget(_chapters.value, playing.positionMs, playing.durationMs) + ?: return + connection.seekTo(target) + } + override fun onCleared() { connection.release() super.onCleared() @@ -72,12 +122,18 @@ class PlayerViewModel(app: Application) : AndroidViewModel(app) { val show = graph.subscriptions.getById(episode.subscriptionId) _currentShow.value = show val speed = PlaybackSpeed.forEpisode(episode.speedOverride, show?.playbackSpeed) + outroHandledFor = null + _chapters.value = emptyList() connection.play( item = episode.toMediaItem(showTitle = show?.title, author = show?.author, showImageUrl = show?.imageUrl), storedPositionMs = episode.positionMs, durationMs = episode.durationMs, speed = speed, + skipIntroMs = show?.skipIntroMs ?: 0, ) + // After playback has started, never before: a chapter file is a second network + // round trip and nothing should wait on it to hear the episode. + _chapters.value = graph.chapters.chapters(episode) } } @@ -91,7 +147,16 @@ class PlayerViewModel(app: Application) : AndroidViewModel(app) { viewModelScope.launch { graph.episodes.markPlayed(episodeId, System.currentTimeMillis()) val nextId = graph.queue.advance() - val next = nextId?.let { graph.episodes.getById(it) } ?: return@launch + val next = nextId?.let { graph.episodes.getById(it) } + if (next == null) { + // Stopping matters now that an episode can finish before its file does. Left + // running, an outro offset marked the episode played and then played the + // credits anyway, which is the exact thing the offset exists to avoid. When + // there is a next episode, starting it replaces the media item and has the + // same effect. + connection.pause() + return@launch + } play(next) } } diff --git a/app/src/main/java/org/terst/listen/ui/ShowSettingsViewModel.kt b/app/src/main/java/org/terst/listen/ui/ShowSettingsViewModel.kt new file mode 100644 index 0000000..1e972f9 --- /dev/null +++ b/app/src/main/java/org/terst/listen/ui/ShowSettingsViewModel.kt @@ -0,0 +1,112 @@ +package org.terst.listen.ui + +import android.app.Application +import androidx.lifecycle.AndroidViewModel +import androidx.lifecycle.viewModelScope +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.launch +import org.terst.listen.data.ArchiveOutcome +import org.terst.listen.data.Graph + +/** + * An archive walk, reported in full. + * + * A partial walk is flagged as an error even though episodes did arrive: a chain that stopped + * early means the rest of the catalogue is missing, and a plain success message would leave the + * show looking permanently short with no indication why. + */ +private fun ArchiveOutcome.Fetched.toBanner(): Banner { + val found = "$newEpisodes older episode(s) from $pages page(s)" + return stoppedEarly + ?.let { Banner("$found, then stopped: $it", isError = true) } + ?: Banner(found) +} + +/** + * The per-show settings: slow drip, the archive fetch, and the skip offsets. + * + * Split out of [ListenViewModel], which had become the entry point for the library, the queue, + * episode actions and these as well. They belong together and nowhere else: one screen uses + * them, they all act on a single subscription, and none of them touches the library as a whole. + * + * It carries its own busy flag rather than sharing the library's refreshing one. Fetching an + * archive and refreshing a feed are different operations of very different lengths, and one + * spinner covering both made the top bar claim a refresh was running when it was not. + */ +class ShowSettingsViewModel(app: Application) : AndroidViewModel(app) { + + private val graph = Graph.get(app) + + private val _banner = MutableStateFlow<Banner?>(null) + val banner: StateFlow<Banner?> = _banner.asStateFlow() + + private val _busy = MutableStateFlow(false) + val busy: StateFlow<Boolean> = _busy.asStateFlow() + + fun dismissBanner() { + _banner.value = null + } + + /** + * Turns slow-drip on for a show, or adjusts a running one, and pays out anything owed. + * + * Releasing straight away is deliberate: flipping the switch and seeing nothing happen for + * a day reads as the feature not working. + */ + fun enableDrip(subscriptionId: String, intervalMs: Long, count: Int, oldestFirst: Boolean) { + viewModelScope.launch { + graph.drip.enable(subscriptionId, intervalMs, count, oldestFirst) + val released = graph.subscriptions.getById(subscriptionId)?.let { graph.drip.release(it) } ?: 0 + _banner.value = if (released > 0) { + Banner("Dripping — $released episode(s) queued") + } else { + Banner("Dripping — nothing to release yet") + } + } + } + + fun disableDrip(subscriptionId: String) { + viewModelScope.launch { + graph.drip.disable(subscriptionId) + _banner.value = Banner("Drip off — the whole catalogue is back") + } + } + + /** + * Pulls a show's back catalogue by walking its RFC 5005 archive pages. + * + * Reported in detail rather than as a bare success: plenty of publishers do not implement + * paging at all, and "nothing happened" needs to be distinguishable from "it failed". + */ + fun fetchArchive(subscriptionId: String) { + if (_busy.value) return + _busy.value = true + viewModelScope.launch { + val subscription = graph.subscriptions.getById(subscriptionId) + if (subscription == null) { + _busy.value = false + return@launch + } + val outcome = graph.repository.fetchArchive(subscription) + _busy.value = false + _banner.value = when (outcome) { + ArchiveOutcome.NoArchive -> + Banner("This feed publishes no archive pages, so there is nothing older to fetch") + is ArchiveOutcome.Failed -> Banner(outcome.message, isError = true) + is ArchiveOutcome.Fetched -> outcome.toBanner() + } + } + } + + /** Tier 2 skip offsets for one show; see SkipSettings for what they mean. */ + fun setSkips(subscriptionId: String, introMs: Long, outroMs: Long) { + viewModelScope.launch { graph.subscriptions.updateSkips(subscriptionId, introMs, outroMs) } + } + + /** Tier 1 skip: act on the breaks a show marks in its own chapter file. Opt-in per show. */ + fun setSkipAdChapters(subscriptionId: String, enabled: Boolean) { + viewModelScope.launch { graph.subscriptions.setSkipAdChapters(subscriptionId, enabled) } + } +} diff --git a/app/src/main/java/org/terst/listen/ui/SkipSettings.kt b/app/src/main/java/org/terst/listen/ui/SkipSettings.kt new file mode 100644 index 0000000..7c6146c --- /dev/null +++ b/app/src/main/java/org/terst/listen/ui/SkipSettings.kt @@ -0,0 +1,96 @@ +package org.terst.listen.ui + +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.text.KeyboardOptions +import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.OutlinedTextField +import androidx.compose.material3.Switch +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.semantics.contentDescription +import androidx.compose.ui.semantics.semantics +import androidx.compose.ui.text.input.KeyboardType +import androidx.compose.ui.unit.dp +import org.terst.listen.data.SubscriptionEntity + +private const val MS_PER_SECOND = 1000L + +/** + * Per-show intro and outro offsets. + * + * The cheap tier of the skip feature, and the one that always works: a show whose theme runs + * forty-seven seconds gets forty-seven seconds skipped, with no detection involved. It is also + * the override for when detection eventually arrives and gets a show wrong. + */ +@Composable +fun SkipSettings( + subscription: SubscriptionEntity, + onChange: (introMs: Long, outroMs: Long) -> Unit, + onSkipAdChapters: (Boolean) -> Unit, +) { + Column( + Modifier.fillMaxWidth().padding(horizontal = 16.dp), + verticalArrangement = Arrangement.spacedBy(8.dp), + ) { + Text("Skip", style = MaterialTheme.typography.titleSmall) + Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) { + SecondsField( + label = "Intro (sec)", + valueMs = subscription.skipIntroMs, + modifier = Modifier.weight(1f), + ) { onChange(it, subscription.skipOutroMs) } + SecondsField( + label = "Outro (sec)", + valueMs = subscription.skipOutroMs, + modifier = Modifier.weight(1f), + ) { onChange(subscription.skipIntroMs, it) } + } + Text( + "Starts each new episode past the intro and treats it as finished before the outro. " + + "Resuming something part-heard still lands where you left it.", + style = MaterialTheme.typography.bodySmall, + ) + + Row(verticalAlignment = Alignment.CenterVertically) { + Switch( + modifier = Modifier.semantics { contentDescription = "Skip marked ad chapters" }, + checked = subscription.skipAdChapters, + onCheckedChange = onSkipAdChapters, + ) + Text("Skip marked breaks", Modifier.padding(start = 12.dp)) + } + Text( + "Only where the show publishes chapters and marks a section as not part of the " + + "episode. Nothing is guessed from chapter titles, so a show discussing its own " + + "sponsors keeps playing.", + style = MaterialTheme.typography.bodySmall, + ) + } +} + +@Composable +private fun SecondsField( + label: String, + valueMs: Long, + modifier: Modifier, + onChange: (Long) -> Unit, +) { + OutlinedTextField( + // Blank rather than "0" for an unset offset: a zero in the box reads as a value someone + // chose, and invites wondering what it does. + value = if (valueMs > 0) (valueMs / MS_PER_SECOND).toString() else "", + onValueChange = { entered -> + onChange(entered.filter(Char::isDigit).toLongOrNull()?.times(MS_PER_SECOND) ?: 0) + }, + label = { Text(label) }, + singleLine = true, + keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Number), + modifier = modifier, + ) +} diff --git a/app/src/main/java/org/terst/listen/ui/SubscriptionScreen.kt b/app/src/main/java/org/terst/listen/ui/SubscriptionScreen.kt index 42e9b1d..3e10466 100644 --- a/app/src/main/java/org/terst/listen/ui/SubscriptionScreen.kt +++ b/app/src/main/java/org/terst/listen/ui/SubscriptionScreen.kt @@ -58,25 +58,21 @@ import java.util.concurrent.TimeUnit fun SubscriptionScreen( vm: ListenViewModel, player: PlayerViewModel, + settings: ShowSettingsViewModel, subscriptionId: String, nav: SubscriptionNav, ) { var confirming by remember { mutableStateOf(false) } val refreshing by vm.refreshing.collectAsState() - val banner by vm.banner.collectAsState() val nowPlaying by player.nowPlaying.collectAsState() val subscription = vm.subscriptions.collectAsState().value.firstOrNull { it.id == subscriptionId } val snackbar = remember { SnackbarHostState() } + // Which set the show lists is the show's own business: the list and the drip switch above + // it read the same flag, so they can never disagree. + val episodes by vm.episodesOf(subscriptionId, subscription?.dripEnabled == true) + .collectAsState(initial = emptyList()) - // This screen had no snackbar until slow-drip needed one, so a refresh started from here - // reported nothing. Fetching an archive is slower still and can half-succeed, which is - // exactly the case that must not be silent. - LaunchedEffect(banner) { - banner?.let { - snackbar.showSnackbar(it.text) - vm.dismissBanner() - } - } + DrainBanners(vm, settings, snackbar) Scaffold( topBar = { @@ -105,67 +101,115 @@ fun SubscriptionScreen( ) SubscriptionBody( - vm = vm, - player = player, subscription = subscription, - nav = nav, + episodes = episodes, + settings = settings, + actionsFor = { episode -> episodeActions(vm, player, nav, episode) }, modifier = Modifier.padding(padding), ) } } -/** The scrolling part: the show, its drip settings, and the episodes it currently lists. */ +/** + * Drains both view models' messages into the one snackbar. + * + * This screen had no snackbar at all until slow-drip needed one, so a refresh started from here + * reported nothing. Fetching an archive is slower still and can half-succeed, which is exactly + * the case that must not be silent -- and the per-show settings report through their own view + * model, so both have to be drained or one of them goes unseen. + */ @Composable -private fun SubscriptionBody( +private fun DrainBanners( + vm: ListenViewModel, + settings: ShowSettingsViewModel, + snackbar: SnackbarHostState, +) { + val banner by vm.banner.collectAsState() + val settingsBanner by settings.banner.collectAsState() + + LaunchedEffect(banner) { + banner?.let { + snackbar.showSnackbar(it.text) + vm.dismissBanner() + } + } + LaunchedEffect(settingsBanner) { + settingsBanner?.let { + snackbar.showSnackbar(it.text) + settings.dismissBanner() + } + } +} + +/** Everything one episode row can do, gathered so the list does not need three view models. */ +private fun episodeActions( vm: ListenViewModel, player: PlayerViewModel, - subscription: SubscriptionEntity?, nav: SubscriptionNav, + episode: EpisodeEntity, +) = EpisodeActions( + play = { player.play(episode) }, + enqueue = { vm.enqueue(episode.id) }, + playNext = { vm.playNext(episode.id) }, + download = { vm.download(episode) }, + openDetails = { nav.openEpisode(episode.id) }, + togglePlayed = { + if (episode.playedAtMs == null) vm.markPlayed(episode.id) else vm.markUnplayed(episode.id) + }, +) + +/** The scrolling part: the show, its per-show settings, and the episodes it currently lists. */ +@Composable +private fun SubscriptionBody( + subscription: SubscriptionEntity?, + episodes: List<EpisodeEntity>, + settings: ShowSettingsViewModel, + actionsFor: (EpisodeEntity) -> EpisodeActions, modifier: Modifier, ) { - val busy by vm.refreshing.collectAsState() - // Which set the show lists is the show's own business, so it is read here rather than - // passed in: the list and the drip switch above it can never disagree. - val dripEnabled = subscription?.dripEnabled == true - val episodes by vm.episodesOf(subscription?.id.orEmpty(), dripEnabled) - .collectAsState(initial = emptyList()) + val busy by settings.busy.collectAsState() LazyColumn(modifier.fillMaxSize()) { item { SubscriptionHeader(subscription) } subscription?.let { show -> - item { - DripSettings( - subscription = show, - busy = busy, - onEnable = { intervalMs, count, oldestFirst -> - vm.enableDrip(show.id, intervalMs, count, oldestFirst) - }, - onDisable = { vm.disableDrip(show.id) }, - onFetchArchive = { vm.fetchArchive(show.id) }, - ) - HorizontalDivider(Modifier.padding(top = 12.dp)) - } + item { ShowSettings(show, settings, busy) } } items(episodes, key = { it.id }) { episode -> EpisodeRow( episode = episode, artworkUrl = Artwork.preferredUrl(episode.imageUrl, subscription?.imageUrl), - actions = EpisodeActions( - play = { player.play(episode) }, - enqueue = { vm.enqueue(episode.id) }, - playNext = { vm.playNext(episode.id) }, - download = { vm.download(episode) }, - openDetails = { nav.openEpisode(episode.id) }, - togglePlayed = { - if (episode.playedAtMs == null) vm.markPlayed(episode.id) else vm.markUnplayed(episode.id) - }, - ), + actions = actionsFor(episode), ) HorizontalDivider() } } } +/** Slow drip and the skip offsets, which are settings for this show rather than for the app. */ +@Composable +private fun ShowSettings( + show: SubscriptionEntity, + settings: ShowSettingsViewModel, + busy: Boolean, +) { + DripSettings( + subscription = show, + busy = busy, + onEnable = { intervalMs, count, oldestFirst -> + settings.enableDrip(show.id, intervalMs, count, oldestFirst) + }, + onDisable = { settings.disableDrip(show.id) }, + onFetchArchive = { settings.fetchArchive(show.id) }, + ) + HorizontalDivider(Modifier.padding(vertical = 12.dp)) + SkipSettings( + subscription = show, + onChange = { introMs, outroMs -> settings.setSkips(show.id, introMs, outroMs) }, + onSkipAdChapters = { settings.setSkipAdChapters(show.id, it) }, + ) + HorizontalDivider(Modifier.padding(top = 12.dp)) +} + @Composable private fun SubscriptionHeader(subscription: SubscriptionEntity?) { if (subscription == null) return 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 d5bc993..c1f7bd2 100644 --- a/app/src/test/java/org/terst/listen/data/FakeDaos.kt +++ b/app/src/test/java/org/terst/listen/data/FakeDaos.kt @@ -44,6 +44,14 @@ class FakeSubscriptionDao : SubscriptionDao { replace(id) { it.copy(playbackSpeed = speed) } } + override suspend fun setSkipAdChapters(id: String, enabled: Boolean) { + replace(id) { it.copy(skipAdChapters = enabled) } + } + + override suspend fun updateSkips(id: String, introMs: Long, outroMs: Long) { + replace(id) { it.copy(skipIntroMs = introMs, skipOutroMs = outroMs) } + } + override suspend fun dripEnabled() = rows.filter { it.dripEnabled } override suspend fun updateDrip( @@ -173,6 +181,10 @@ class FakeEpisodeDao : EpisodeDao { replace(id) { it.copy(downloadState = state, localPath = localPath, downloadedAtMs = downloadedAtMs) } } + override suspend fun setChapters(id: String, json: String?) { + replace(id) { it.copy(chaptersJson = json) } + } + override suspend fun recordRule(id: String, ruleId: String, reason: String, speed: Float?) { replace(id) { it.copy(appliedRuleId = ruleId, appliedRuleReason = reason, speedOverride = speed) } } diff --git a/app/src/test/java/org/terst/listen/feed/ChapterFetcherTest.kt b/app/src/test/java/org/terst/listen/feed/ChapterFetcherTest.kt new file mode 100644 index 0000000..439485c --- /dev/null +++ b/app/src/test/java/org/terst/listen/feed/ChapterFetcherTest.kt @@ -0,0 +1,104 @@ +package org.terst.listen.feed + +import kotlinx.coroutines.test.runTest +import okhttp3.OkHttpClient +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Before +import org.junit.Test +import org.terst.listen.data.EpisodeEntity +import org.terst.listen.data.FakeEpisodeDao + +/** Fetching an episode's chapter file once and caching it on the episode. */ +class ChapterFetcherTest { + + private lateinit var server: MockWebServer + private lateinit var episodes: FakeEpisodeDao + private lateinit var fetcher: ChapterFetcher + + private val document = """{"chapters":[ + {"startTime":0,"title":"Part one"}, + {"startTime":60,"title":"Sponsor","toc":false} + ]}""" + + @Before + fun setUp() { + server = MockWebServer() + server.start() + episodes = FakeEpisodeDao() + fetcher = ChapterFetcher(OkHttpClient(), episodes) + } + + @After + fun tearDown() = server.shutdown() + + private fun episode(chaptersUrl: String? = null, chaptersJson: String? = null): EpisodeEntity { + val entity = EpisodeEntity( + id = "e1", + subscriptionId = "s1", + guid = "g1", + title = "One", + mediaUrl = "https://cdn/1.mp3", + mediaType = "audio/mpeg", + chaptersUrl = chaptersUrl, + chaptersJson = chaptersJson, + discoveredAtMs = 0, + ) + episodes.rows += entity + return entity + } + + @Test + fun `an episode with no chapter url has no chapters`() = runTest { + assertEquals(emptyList<Chapter>(), fetcher.chapters(episode())) + } + + @Test + fun `a chapter file is fetched and cached on the episode`() = runTest { + server.enqueue(MockResponse().setBody(document)) + + val chapters = fetcher.chapters(episode(chaptersUrl = server.url("/ch.json").toString())) + + assertEquals(listOf("Part one", "Sponsor"), chapters.map { it.title }) + assertNotNull("the file must be cached, not re-fetched every play", episodes.rows.single().chaptersJson) + } + + /** Already cached means no network at all; the server would fail the request if asked. */ + @Test + fun `a cached file is parsed without fetching again`() = runTest { + val chapters = fetcher.chapters(episode(chaptersUrl = "https://unused", chaptersJson = document)) + + assertEquals(2, chapters.size) + assertEquals(0, server.requestCount) + } + + @Test + fun `a failed fetch yields no chapters and caches nothing`() = runTest { + server.enqueue(MockResponse().setResponseCode(404)) + + val chapters = fetcher.chapters(episode(chaptersUrl = server.url("/ch.json").toString())) + + assertEquals(emptyList<Chapter>(), chapters) + assertNull("a 404 must not be cached as an answer", episodes.rows.single().chaptersJson) + } + + /** + * A file that fetches but parses to nothing is still cached. + * + * Otherwise every play of an episode whose publisher serves junk re-downloads it, forever, + * for a feature nobody asked for. + */ + @Test + fun `an unparseable file is cached so it is not fetched again`() = runTest { + server.enqueue(MockResponse().setBody("this is not json")) + + val chapters = fetcher.chapters(episode(chaptersUrl = server.url("/ch.json").toString())) + + assertEquals(emptyList<Chapter>(), chapters) + assertNotNull(episodes.rows.single().chaptersJson) + } +} diff --git a/app/src/test/java/org/terst/listen/feed/ChapterParserTest.kt b/app/src/test/java/org/terst/listen/feed/ChapterParserTest.kt new file mode 100644 index 0000000..42aa52a --- /dev/null +++ b/app/src/test/java/org/terst/listen/feed/ChapterParserTest.kt @@ -0,0 +1,98 @@ +package org.terst.listen.feed + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Podcasting 2.0 chapter files. + * + * Tier 1 of the skip feature, and the only exact one: where a show marks its own ad breaks, + * detection is just parsing. This runs on a file fetched from a URL in a stranger's feed, so + * every malformed shape has to degrade to "no chapters" rather than take playback down. + */ +class ChapterParserTest { + + @Test + fun `start times in seconds become milliseconds`() { + val chapters = ChapterParser.parse( + """{"version":"1.2.0","chapters":[ + {"startTime":0,"title":"Cold open"}, + {"startTime":47.5,"title":"Theme"} + ]}""" + ) + assertEquals(listOf(0L, 47_500L), chapters.map { it.startMs }) + assertEquals(listOf("Cold open", "Theme"), chapters.map { it.title }) + } + + @Test + fun `an end time is kept when the publisher gives one`() { + val chapters = ChapterParser.parse( + """{"chapters":[{"startTime":10,"endTime":75.25,"title":"Sponsor"}]}""" + ) + assertEquals(10_000L, chapters.single().startMs) + assertEquals(75_250L, chapters.single().endMs) + } + + /** `toc` is how a publisher says "this is not really part of the episode". */ + @Test + fun `toc defaults to true and false is captured`() { + val chapters = ChapterParser.parse( + """{"chapters":[ + {"startTime":0,"title":"Part one"}, + {"startTime":60,"title":"Sponsor","toc":false} + ]}""" + ) + assertEquals(listOf(true, false), chapters.map { it.inToc }) + } + + /** Publishers do emit them out of order, and everything downstream assumes they are not. */ + @Test + fun `chapters come back in start order`() { + val chapters = ChapterParser.parse( + """{"chapters":[ + {"startTime":120,"title":"Third"}, + {"startTime":0,"title":"First"}, + {"startTime":60,"title":"Second"} + ]}""" + ) + assertEquals(listOf("First", "Second", "Third"), chapters.map { it.title }) + } + + @Test + fun `an empty chapter list is not an error`() { + assertEquals(emptyList<Chapter>(), ChapterParser.parse("""{"chapters":[]}""")) + } + + @Test + fun `malformed json yields no chapters rather than throwing`() { + assertTrue(ChapterParser.parse("not json at all").isEmpty()) + assertTrue(ChapterParser.parse("").isEmpty()) + assertTrue(ChapterParser.parse("""{"chapters":"nonsense"}""").isEmpty()) + } + + /** A file with no chapters key at all is a valid JSON document and still means nothing. */ + @Test + fun `json without a chapters key yields none`() { + assertTrue(ChapterParser.parse("""{"version":"1.2.0"}""").isEmpty()) + } + + /** Unknown keys are the norm: the spec has img, url, location and more. */ + @Test + fun `unfamiliar fields are ignored rather than rejected`() { + val chapters = ChapterParser.parse( + """{"chapters":[{"startTime":5,"title":"One","img":"https://x/y.jpg", + "url":"https://x","location":{"name":"somewhere"}}]}""" + ) + assertEquals(1, chapters.size) + assertEquals("One", chapters.single().title) + } + + @Test + fun `a chapter with no start time is dropped`() { + val chapters = ChapterParser.parse( + """{"chapters":[{"title":"No start"},{"startTime":30,"title":"Fine"}]}""" + ) + assertEquals(listOf("Fine"), chapters.map { it.title }) + } +} diff --git a/app/src/test/java/org/terst/listen/playback/AdChaptersTest.kt b/app/src/test/java/org/terst/listen/playback/AdChaptersTest.kt new file mode 100644 index 0000000..b74e09c --- /dev/null +++ b/app/src/test/java/org/terst/listen/playback/AdChaptersTest.kt @@ -0,0 +1,97 @@ +package org.terst.listen.playback + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test +import org.terst.listen.feed.Chapter + +/** + * Deciding whether the playhead is sitting in something the publisher marked as not-content. + * + * Conservative on purpose. A skip feature that silently eats an episode is worse than none, so + * the only signal trusted here is `toc: false` -- the publisher's own statement -- and a break + * whose end cannot be worked out is left alone rather than guessed at. + */ +class AdChaptersTest { + + private val minute = 60_000L + + private val chapters = listOf( + Chapter(startMs = 0, title = "Cold open"), + Chapter(startMs = 2 * minute, title = "Sponsor", inToc = false), + Chapter(startMs = 3 * minute, title = "The interview"), + ) + + @Test + fun `inside a marked break, it reports where to jump to`() { + assertEquals(3 * minute, AdChapters.skipTarget(chapters, 2 * minute + 5_000, 30 * minute)) + } + + @Test + fun `inside real content it reports nothing`() { + assertNull(AdChapters.skipTarget(chapters, minute, 30 * minute)) + assertNull(AdChapters.skipTarget(chapters, 10 * minute, 30 * minute)) + } + + /** A boundary belongs to the chapter starting there, or a skip would fire again on arrival. */ + @Test + fun `the first instant of the next chapter is not part of the break`() { + assertNull(AdChapters.skipTarget(chapters, 3 * minute, 30 * minute)) + } + + @Test + fun `an explicit end time is preferred over the next chapter`() { + val withEnd = listOf( + Chapter(startMs = 0, endMs = 90_000, title = "Sponsor", inToc = false), + Chapter(startMs = 5 * minute, title = "Content"), + ) + assertEquals(90_000L, AdChapters.skipTarget(withEnd, 10_000, 30 * minute)) + } + + @Test + fun `a break that runs to the end of the episode jumps to the duration`() { + val trailing = listOf( + Chapter(startMs = 0, title = "Content"), + Chapter(startMs = 28 * minute, title = "Outro ad", inToc = false), + ) + assertEquals(30 * minute, AdChapters.skipTarget(trailing, 29 * minute, 30 * minute)) + } + + /** With no end time, no following chapter and no duration, there is nowhere to jump to. */ + @Test + fun `a break with no determinable end is left alone`() { + val trailing = listOf( + Chapter(startMs = 0, title = "Content"), + Chapter(startMs = 28 * minute, title = "Outro ad", inToc = false), + ) + assertNull(AdChapters.skipTarget(trailing, 29 * minute, durationMs = null)) + } + + /** + * A chapter the publisher left in the table of contents is content, whatever it is called. + * + * Guessing from titles is how a skip feature starts eating the episode: a show discussing + * its own sponsorship deal has a chapter called "Sponsors" that is the actual programme. + */ + @Test + fun `a chapter in the table of contents is never skipped, whatever its title`() { + val titled = listOf( + Chapter(startMs = 0, title = "Content"), + Chapter(startMs = 2 * minute, title = "Sponsors and advertising"), + Chapter(startMs = 4 * minute, title = "More content"), + ) + assertNull(AdChapters.skipTarget(titled, 3 * minute, 30 * minute)) + } + + @Test + fun `no chapters means nothing to skip`() { + assertNull(AdChapters.skipTarget(emptyList(), minute, 30 * minute)) + } + + /** A position before the first chapter starts is not inside anything. */ + @Test + fun `a position ahead of the first chapter is left alone`() { + val late = listOf(Chapter(startMs = 5 * minute, title = "Ad", inToc = false)) + assertNull(AdChapters.skipTarget(late, minute, 30 * minute)) + } +} diff --git a/app/src/test/java/org/terst/listen/playback/PlaybackProgressTest.kt b/app/src/test/java/org/terst/listen/playback/PlaybackProgressTest.kt index 94749a0..d209fb6 100644 --- a/app/src/test/java/org/terst/listen/playback/PlaybackProgressTest.kt +++ b/app/src/test/java/org/terst/listen/playback/PlaybackProgressTest.kt @@ -112,4 +112,92 @@ class PlaybackProgressTest { fun `the first write always happens`() { assertTrue(progress.shouldPersist(lastWrittenMs = null, currentMs = 1_000)) } + + // --- manual skip offsets ----------------------------------------------------------------- + // + // Tier 2 of the skip feature. The fields have been on SubscriptionEntity since phase 1 and + // nothing read them; these are the rules that make them mean something. + + @Test + fun `a fresh episode starts past the intro`() { + assertEquals( + 47_000L, + progress.resumePosition(storedMs = 0, durationMs = 60 * minute, skipIntroMs = 47_000), + ) + } + + /** Coming back to something half-heard must land where you left it, not at the offset. */ + @Test + fun `a resumed episode ignores the intro offset`() { + assertEquals( + 30 * minute, + progress.resumePosition(storedMs = 30 * minute, durationMs = 60 * minute, skipIntroMs = 47_000), + ) + } + + /** A stored position past the end restarts, and restarting means past the intro too. */ + @Test + fun `a restart also starts past the intro`() { + assertEquals( + 47_000L, + progress.resumePosition(storedMs = 90 * minute, durationMs = 60 * minute, skipIntroMs = 47_000), + ) + } + + /** + * A mistyped offset must not swallow the episode. + * + * Someone entering minutes where seconds were meant would otherwise start a 40-minute show + * at the end, which looks exactly like a broken download. + */ + @Test + fun `an intro offset longer than the episode is ignored`() { + assertEquals( + 0L, + progress.resumePosition(storedMs = 0, durationMs = 40 * minute, skipIntroMs = 90 * minute), + ) + } + + @Test + fun `an unknown duration still honours the intro offset`() { + assertEquals( + 47_000L, + progress.resumePosition(storedMs = 0, durationMs = null, skipIntroMs = 47_000), + ) + } + + @Test + fun `the outro offset finishes an episode early`() { + assertFalse(progress.isComplete(positionMs = 50 * minute, durationMs = 60 * minute)) + assertTrue( + progress.isComplete( + positionMs = 50 * minute, + durationMs = 60 * minute, + skipOutroMs = 10 * minute, + ) + ) + } + + @Test + fun `an episode short of the outro offset is not complete`() { + assertFalse( + progress.isComplete( + positionMs = 40 * minute, + durationMs = 60 * minute, + skipOutroMs = 10 * minute, + ) + ) + } + + /** Same reasoning as the intro: a nonsense offset must not mark everything played at once. */ + @Test + fun `an outro offset as long as the episode is ignored`() { + assertFalse( + progress.isComplete( + positionMs = 1_000, + durationMs = 60 * minute, + skipOutroMs = 60 * minute, + ) + ) + } } |
