summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPeter Stone <thepeterstone@gmail.com>2026-09-26 10:13:04 +0000
committerPeter Stone <thepeterstone@gmail.com>2026-09-26 10:13:04 +0000
commit4488a3f49cadcdcd37ce29bedf49d0369983bfb1 (patch)
tree71180ffbf868bd9bdbec5f07d1c98ba1d1822c64
parent315fee32e64c75f641b7673092d1f3f42ea27e57 (diff)
Name a shared video from the extractor when its page will not
Facebook serves no og: tags to a non-browser at all, so a shared video was filed as "Video 10153231379946729". yt-dlp knows perfectly well what it is called, and is going to be run against that URL anyway. The probe only happens when the page gave no title: a title that was shared is the one that was meant, and this costs a network round trip. It is allowed to fail, because it buys a nicer name rather than anything the episode needs -- a video that cannot be probed still downloads and plays. Duration comes along with it, which Facebook episodes previously had to do without entirely. This retires the last entry but one from the reachability allowlist: YouTubeAudio.probe has been sitting there since phase 1 described as "the resolver will want it for episode durations", which turned out to be exactly right. Verified on a device: the same Facebook video that arrived as "Video 10153231379946729" this morning now arrives as "How to Share With Just Friends", 74 seconds. No permission changes, no schema change. 428 unit tests green, detekt and lint clean, coverage 89.93 -> 89.97. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AwnALZL6bD4VBtATDVv3B
-rw-r--r--.coverage-baseline2
-rw-r--r--CLAUDE.md8
-rw-r--r--app/src/main/java/org/terst/listen/data/Repositories.kt1
-rw-r--r--app/src/main/java/org/terst/listen/data/SubscriptionRepository.kt22
-rw-r--r--app/src/main/java/org/terst/listen/download/ChaquopyAudioSource.kt12
-rw-r--r--app/src/main/java/org/terst/listen/download/EpisodeDownloader.kt4
-rw-r--r--app/src/test/java/org/terst/listen/ReachabilityTest.kt3
-rw-r--r--app/src/test/java/org/terst/listen/data/SubscriptionRepositoryTest.kt52
8 files changed, 94 insertions, 10 deletions
diff --git a/.coverage-baseline b/.coverage-baseline
index 33e28ab..c289320 100644
--- a/.coverage-baseline
+++ b/.coverage-baseline
@@ -1 +1 @@
-89.93
+89.97
diff --git a/CLAUDE.md b/CLAUDE.md
index 3abde17..84a7fd5 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -227,10 +227,10 @@ out of a possibly-partial MP3, and the JSON path covers the shows that mark chap
episodes are fetched and one row cannot be honest about two sources.
- **Facebook tells a non-browser nothing.** It serves no `og:` tags at all -- verified with the
- app's own user agent -- so a shared video gets a fallback title of "Video <id>". yt-dlp knows
- the real title (`probe` in `ytaudio.py` returns it, and `YouTubeAudio.probe` is still sitting
- in the reachability allowlist waiting for a caller). Backfilling from it is the obvious fix
- and is not done yet.
+ app's own user agent -- so the page cannot name a shared video. `SubscriptionRepository` asks
+ the extractor instead when the page gave no title, which is where the title and duration now
+ come from. Only then: a title that was shared is the one that was meant, and the probe costs
+ a network round trip.
- **Anything entering the queue is downloaded immediately, on any network.** The hook is
`QueueRepository`'s `onQueued`, fired with newly-present ids only, because the queue is the
diff --git a/app/src/main/java/org/terst/listen/data/Repositories.kt b/app/src/main/java/org/terst/listen/data/Repositories.kt
index 6604a10..8c994a1 100644
--- a/app/src/main/java/org/terst/listen/data/Repositories.kt
+++ b/app/src/main/java/org/terst/listen/data/Repositories.kt
@@ -79,6 +79,7 @@ class AppGraph(private val context: Context) {
episodes,
FeedFetcher(client),
onDiscovered = { ruleApplier.apply(it) },
+ probe = { url -> ChaquopyAudioSource(context).probe(url) },
)
}
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 5cdc9d4..0e104d4 100644
--- a/app/src/main/java/org/terst/listen/data/SubscriptionRepository.kt
+++ b/app/src/main/java/org/terst/listen/data/SubscriptionRepository.kt
@@ -14,6 +14,9 @@ sealed interface RefreshOutcome {
data class Failed(val message: String) : RefreshOutcome
}
+/** What the extractor knows about a video that its page would not say. */
+data class ProbedVideo(val title: String?, val durationMs: Long?)
+
/** Outcome of walking a feed's RFC 5005 archive pages. */
sealed interface ArchiveOutcome {
/** [stoppedEarly] is null when the walk reached the end of the chain. */
@@ -38,6 +41,15 @@ class SubscriptionRepository(
* is. Deliberately not invoked by [subscribe] or [addSingleEpisode]: see their comments.
*/
private val onDiscovered: suspend (List<EpisodeEntity>) -> Unit = {},
+ /**
+ * Asks the extractor what a video is, for the cases where its page will not say.
+ *
+ * Facebook serves no `og:` tags to a non-browser at all, so a shared video would otherwise
+ * be filed as "Video 10153231379946729". yt-dlp knows, and is going to be run against the
+ * URL anyway. A collaborator rather than a direct call because it reaches the embedded
+ * Python, which cannot run in a unit test.
+ */
+ private val probe: suspend (String) -> ProbedVideo? = { null },
private val now: () -> Long = System::currentTimeMillis,
) {
@@ -178,6 +190,10 @@ class SubscriptionRepository(
subscriptions.getByFeedUrl(feedUrl) ?: entity
}
+ // Only when the page gave nothing: a title that was shared is the one that was meant,
+ // 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.
episodes.insertAll(
@@ -186,11 +202,13 @@ class SubscriptionRepository(
id = UUID.randomUUID().toString(),
subscriptionId = container.id,
guid = if (facebook) "fb:video:${video.videoId}" else "yt:video:${video.videoId}",
- title = video.title?.takeIf { it.isNotBlank() } ?: "Video ${video.videoId}",
+ title = video.title?.takeIf { it.isNotBlank() }
+ ?: probed?.title?.takeIf { it.isNotBlank() }
+ ?: "Video ${video.videoId}",
mediaUrl = video.watchUrl,
// yt-dlp is asked for bestaudio, which comes back as m4a.
mediaType = "audio/mp4",
- durationMs = video.durationMs,
+ durationMs = video.durationMs ?: probed?.durationMs,
publishedAtMs = now(),
imageUrl = video.imageUrl,
discoveredAtMs = now(),
diff --git a/app/src/main/java/org/terst/listen/download/ChaquopyAudioSource.kt b/app/src/main/java/org/terst/listen/download/ChaquopyAudioSource.kt
index 88038f6..de6d2b7 100644
--- a/app/src/main/java/org/terst/listen/download/ChaquopyAudioSource.kt
+++ b/app/src/main/java/org/terst/listen/download/ChaquopyAudioSource.kt
@@ -3,6 +3,7 @@ package org.terst.listen.download
import android.content.Context
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext
+import org.terst.listen.data.ProbedVideo
import org.terst.listen.youtube.YouTubeAudio
import java.io.File
@@ -21,4 +22,15 @@ class ChaquopyAudioSource(private val context: Context) : ExtractedAudioSource {
YouTubeAudio.start(context)
YouTubeAudio.download(url, targetDir)
}
+
+ /**
+ * Best effort, and allowed to fail: this only supplies a nicer title than "Video <id>",
+ * and a video that cannot be probed may still download perfectly well.
+ */
+ override suspend fun probe(url: String): ProbedVideo? = withContext(Dispatchers.IO) {
+ YouTubeAudio.start(context)
+ runCatching { YouTubeAudio.probe(url) }
+ .map { ProbedVideo(title = it.title, durationMs = it.durationMs) }
+ .getOrNull()
+ }
}
diff --git a/app/src/main/java/org/terst/listen/download/EpisodeDownloader.kt b/app/src/main/java/org/terst/listen/download/EpisodeDownloader.kt
index e671aa8..831708f 100644
--- a/app/src/main/java/org/terst/listen/download/EpisodeDownloader.kt
+++ b/app/src/main/java/org/terst/listen/download/EpisodeDownloader.kt
@@ -7,6 +7,7 @@ import okhttp3.Request
import org.terst.listen.data.DownloadState
import org.terst.listen.data.EpisodeDao
import org.terst.listen.data.EpisodeEntity
+import org.terst.listen.data.ProbedVideo
import org.terst.listen.data.SubscriptionKind
import java.io.File
import java.io.IOException
@@ -148,4 +149,7 @@ class EpisodeDownloader(
/** Indirection so the Chaquopy-backed downloader can be swapped out in tests. */
interface ExtractedAudioSource {
suspend fun download(url: String, targetDir: File): File
+
+ /** What the extractor knows about a video without downloading it. Null if it cannot say. */
+ suspend fun probe(url: String): ProbedVideo?
}
diff --git a/app/src/test/java/org/terst/listen/ReachabilityTest.kt b/app/src/test/java/org/terst/listen/ReachabilityTest.kt
index 6a33380..da7d9a3 100644
--- a/app/src/test/java/org/terst/listen/ReachabilityTest.kt
+++ b/app/src/test/java/org/terst/listen/ReachabilityTest.kt
@@ -27,9 +27,6 @@ class ReachabilityTest {
"YouTubeAudio.ytDlpVersion" to
"Called from YouTubeAudioTest, which fails on a stale bundled yt-dlp. That guard " +
"is the whole point; it has no job in production.",
- "YouTubeAudio.probe" to
- "Metadata without downloading. Unused while the only YouTube path is download, but " +
- "the resolver will want it for episode durations.",
)
private val mainDir = File("src/main/java/org/terst/listen")
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 a5544e7..3508fa3 100644
--- a/app/src/test/java/org/terst/listen/data/SubscriptionRepositoryTest.kt
+++ b/app/src/test/java/org/terst/listen/data/SubscriptionRepositoryTest.kt
@@ -265,6 +265,58 @@ class SubscriptionRepositoryTest {
assertEquals(1, eps.rows.size)
}
+ /**
+ * Facebook serves no og: tags to a non-browser at all, so a shared video arrived as
+ * "Video 10153231379946729". yt-dlp knows what it is really called, and is going to be run
+ * against the URL anyway.
+ */
+ @Test
+ fun `a video the page could not name is named by the extractor`() = runTest {
+ val probing = SubscriptionRepository(
+ subs,
+ eps,
+ FeedFetcher(OkHttpClient()),
+ probe = { ProbedVideo(title = "Sheep dog at work", durationMs = 92_000) },
+ ) { clock }
+
+ probing.addSingleEpisode(video("111", null, kind = SubscriptionKind.FACEBOOK_VIDEO))
+
+ val episode = eps.rows.single()
+ assertEquals("Sheep dog at work", episode.title)
+ assertEquals(92_000L, episode.durationMs)
+ }
+
+ /** A title the page did give is the one that was shared; the extractor does not overrule it. */
+ @Test
+ fun `a video the page named keeps that name`() = runTest {
+ val probing = SubscriptionRepository(
+ subs,
+ eps,
+ FeedFetcher(OkHttpClient()),
+ probe = { ProbedVideo(title = "Something else", durationMs = 1) },
+ ) { clock }
+
+ probing.addSingleEpisode(video("aaa", "From the page"))
+
+ assertEquals("From the page", eps.rows.single().title)
+ }
+
+ /** The extractor is allowed to fail -- it is a network call for a nicety. */
+ @Test
+ fun `a video still gets added when the extractor cannot name it`() = runTest {
+ val probing = SubscriptionRepository(
+ subs,
+ eps,
+ FeedFetcher(OkHttpClient()),
+ probe = { null },
+ ) { clock }
+
+ probing.addSingleEpisode(video("111", null, kind = SubscriptionKind.FACEBOOK_VIDEO))
+
+ assertEquals(1, eps.rows.size)
+ assertTrue("a blank title would render as an empty row", eps.rows.single().title.isNotBlank())
+ }
+
@Test
fun `a video with no title still gets one`() = runTest {
repo.addSingleEpisode(video("aaa", null))