diff options
| author | Peter Stone <thepeterstone@gmail.com> | 2026-09-26 10:13:04 +0000 |
|---|---|---|
| committer | Peter Stone <thepeterstone@gmail.com> | 2026-09-26 10:13:04 +0000 |
| commit | 4488a3f49cadcdcd37ce29bedf49d0369983bfb1 (patch) | |
| tree | 71180ffbf868bd9bdbec5f07d1c98ba1d1822c64 /app/src | |
| parent | 315fee32e64c75f641b7673092d1f3f42ea27e57 (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
Diffstat (limited to 'app/src')
6 files changed, 89 insertions, 5 deletions
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)) |
