diff options
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)) |