From 6f5e0c6a6bbe0b6d6720d4a6f485d874eb37ce0d Mon Sep 17 00:00:00 2001 From: davotoula Date: Tue, 28 Jul 2026 11:54:23 +0200 Subject: [PATCH 1/4] fix(playback): strip Low-Latency HLS tags before media3 parses the playlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit media3 crashes fatally on an LL-HLS playlist whose parts are byte ranges — what zap-stream-core emits (`#EXT-X-PART:...,BYTERANGE="359712@0"`). Reproduced on media3 1.10.1, Pixel 9a / Android 17, ~0.6s after ExoPlayerImpl.Init: IllegalArgumentException at DataSpec. // checkArgument(length > 0 || length == LENGTH_UNSET) at DataSpec.subrange at HlsMediaChunk.feedDataToExtractor at HlsMediaChunk.loadMedia --- .../playerPool/CustomMediaSourceFactory.kt | 50 ++++-- .../playerPool/LowLatencyHlsStripper.kt | 149 ++++++++++++++++ .../playerPool/LowLatencyHlsStripperTest.kt | 163 ++++++++++++++++++ 3 files changed, 350 insertions(+), 12 deletions(-) create mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripper.kt create mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripperTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/CustomMediaSourceFactory.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/CustomMediaSourceFactory.kt index c38b614e45..692263214d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/CustomMediaSourceFactory.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/CustomMediaSourceFactory.kt @@ -20,10 +20,13 @@ */ package com.vitorpamplona.amethyst.service.playback.playerPool +import androidx.media3.common.C import androidx.media3.common.MediaItem import androidx.media3.common.util.UnstableApi +import androidx.media3.common.util.Util import androidx.media3.datasource.DataSource import androidx.media3.exoplayer.drm.DrmSessionManagerProvider +import androidx.media3.exoplayer.hls.HlsMediaSource import androidx.media3.exoplayer.source.DefaultMediaSourceFactory import androidx.media3.exoplayer.source.MediaSource import androidx.media3.exoplayer.upstream.LoadErrorHandlingPolicy @@ -81,20 +84,33 @@ class CustomMediaSourceFactory( videoCache: VideoCache, dataSourceFactory: DataSource.Factory, ) : MediaSource.Factory { + private val cachingDataSource: DataSource.Factory = videoCache.get(dataSourceFactory) + private var cachingFactory: MediaSource.Factory = - DefaultMediaSourceFactory(videoCache.get(dataSourceFactory)) + DefaultMediaSourceFactory(cachingDataSource) private var nonCachingFactory: MediaSource.Factory = DefaultMediaSourceFactory(dataSourceFactory) + // HLS is built explicitly rather than through DefaultMediaSourceFactory, which exposes no hook + // for a playlist parser factory. See LowLatencyStrippingHlsPlaylistParserFactory for why we need + // one. Everything else still routes through the Default factories above. + private val cachingHlsFactory: MediaSource.Factory = hlsFactory(cachingDataSource) + private val nonCachingHlsFactory: MediaSource.Factory = hlsFactory(dataSourceFactory) + + private fun hlsFactory(dataSource: DataSource.Factory): MediaSource.Factory = + HlsMediaSource + .Factory(dataSource) + .setPlaylistParserFactory(LowLatencyStrippingHlsPlaylistParserFactory()) + + private fun allFactories() = listOf(cachingFactory, nonCachingFactory, cachingHlsFactory, nonCachingHlsFactory) + override fun setDrmSessionManagerProvider(drmSessionManagerProvider: DrmSessionManagerProvider): MediaSource.Factory { - cachingFactory.setDrmSessionManagerProvider(drmSessionManagerProvider) - nonCachingFactory.setDrmSessionManagerProvider(drmSessionManagerProvider) + allFactories().forEach { it.setDrmSessionManagerProvider(drmSessionManagerProvider) } return this } override fun setLoadErrorHandlingPolicy(loadErrorHandlingPolicy: LoadErrorHandlingPolicy): MediaSource.Factory { - cachingFactory.setLoadErrorHandlingPolicy(loadErrorHandlingPolicy) - nonCachingFactory.setLoadErrorHandlingPolicy(loadErrorHandlingPolicy) + allFactories().forEach { it.setLoadErrorHandlingPolicy(loadErrorHandlingPolicy) } return this } @@ -107,17 +123,27 @@ class CustomMediaSourceFactory( val knownOnDemand = HlsLivenessCache.isKnownOnDemand(id) val bypassCache = shouldBypassCache(flaggedLive, hls, knownOnDemand) + // Reuse media3's own inference rather than the looser `.m3u8`-substring check that drives + // cache routing, so the HLS branch here always agrees with what DefaultMediaSourceFactory + // would have picked. `isLiveStreaming` matches a `.m3u8` anywhere in the URL — including a + // query string on a progressive file — which must not select an HlsMediaSource. + val config = mediaItem.localConfiguration + val isHlsContent = + config != null && + Util.inferContentTypeForUriAndMimeType(config.uri, config.mimeType) == C.CONTENT_TYPE_HLS + val source = - if (bypassCache) { - nonCachingFactory.createMediaSource(mediaItem) - } else { - cachingFactory.createMediaSource(mediaItem) + when { + isHlsContent && bypassCache -> nonCachingHlsFactory.createMediaSource(mediaItem) + isHlsContent -> cachingHlsFactory.createMediaSource(mediaItem) + bypassCache -> nonCachingFactory.createMediaSource(mediaItem) + else -> cachingFactory.createMediaSource(mediaItem) } - // Logs the three routing inputs directly rather than a re-derived label, so it can't drift - // from shouldBypassCache. + // Logs the routing inputs directly rather than a re-derived label, so it can't drift from + // shouldBypassCache. Log.d(PLAYBACK_DIAG_TAG) { "SOURCE ${if (bypassCache) "BYPASS" else "CACHE"} flaggedLive=$flaggedLive hls=$hls knownOnDemand=$knownOnDemand " + - "mime=${mediaItem.localConfiguration?.mimeType} -> ${source::class.java.simpleName} id=$id" + "hlsContent=$isHlsContent mime=${config?.mimeType} -> ${source::class.java.simpleName} id=$id" } return source } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripper.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripper.kt new file mode 100644 index 0000000000..c803437304 --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripper.kt @@ -0,0 +1,149 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service.playback.playerPool + +import android.net.Uri +import androidx.media3.common.util.UnstableApi +import androidx.media3.exoplayer.hls.playlist.DefaultHlsPlaylistParserFactory +import androidx.media3.exoplayer.hls.playlist.HlsMediaPlaylist +import androidx.media3.exoplayer.hls.playlist.HlsMultivariantPlaylist +import androidx.media3.exoplayer.hls.playlist.HlsPlaylist +import androidx.media3.exoplayer.hls.playlist.HlsPlaylistParserFactory +import androidx.media3.exoplayer.upstream.ParsingLoadable +import java.io.ByteArrayInputStream +import java.io.InputStream + +/** + * Low-Latency HLS tags that we remove before media3's playlist parser sees them. + * + * `EXT-X-PART` and `EXT-X-PRELOAD-HINT` are the ones that matter: they are the only things in these + * playlists that produce a **byte-range-bounded** chunk, which is what triggers the crash documented + * on [LowLatencyStrippingHlsPlaylistParserFactory]. `EXT-X-PART-INF` and `EXT-X-SERVER-CONTROL` go + * with them — leaving those behind advertises a low-latency contract (PART-TARGET, PART-HOLD-BACK, + * CAN-BLOCK-RELOAD) that the stripped playlist can no longer honour. + * + * Deliberately **not** stripped: + * - `EXT-X-SKIP` — marks a delta playlist whose segments were legitimately omitted. Removing the tag + * while the segments stay missing would corrupt the playlist. Dropping `EXT-X-SERVER-CONTROL` + * already stops media3 requesting deltas (`_HLS_skip=YES`), so this should never appear anyway. + * - `EXT-X-RENDITION-REPORT` — inert once the parts are gone. + */ +private val LOW_LATENCY_TAGS = + listOf( + "#EXT-X-PART:", + "#EXT-X-PART-INF:", + "#EXT-X-PRELOAD-HINT:", + "#EXT-X-SERVER-CONTROL:", + ) + +/** + * Removes the Low-Latency HLS tags from a playlist, leaving every other byte untouched. + * + * Line separators are preserved exactly: the split/join round-trips `\n`, keeps the `\r` of a CRLF + * playlist as trailing content, and keeps a trailing newline (which `split` surfaces as a final + * empty element). Blank lines are never dropped. + */ +internal fun stripLowLatencyTags(playlist: String): String { + // Cheap pre-check: the overwhelming majority of playlists carry no LL tags at all, and this + // runs on every playlist reload of every live stream. + if (LOW_LATENCY_TAGS.none { playlist.contains(it) }) return playlist + + return playlist + .split("\n") + .filterNot { line -> + val trimmed = line.trimStart() + LOW_LATENCY_TAGS.any { trimmed.startsWith(it) } + }.joinToString("\n") +} + +/** + * Wraps a media3 playlist parser and strips the Low-Latency tags before delegating. + * + * Playlists are a few KB, so reading the stream fully into memory is cheaper than the alternative of + * a streaming line filter and keeps the transform a pure, testable [stripLowLatencyTags] call. + */ +@UnstableApi +internal class LowLatencyStrippingParser( + private val delegate: ParsingLoadable.Parser, +) : ParsingLoadable.Parser { + override fun parse( + uri: Uri, + inputStream: InputStream, + ): HlsPlaylist { + val original = inputStream.readBytes().toString(Charsets.UTF_8) + val stripped = stripLowLatencyTags(original) + return delegate.parse(uri, ByteArrayInputStream(stripped.toByteArray(Charsets.UTF_8))) + } +} + +/** + * Serves media3 a de-low-latency-ed view of every HLS playlist. + * + * ## Why + * + * media3 crashes fatally on a Low-Latency HLS playlist whose parts are byte ranges — which is what + * zap-stream-core emits (`#EXT-X-PART:URI="…",DURATION=…,BYTERANGE="359712@0"`). Reproduced on + * media3 1.10.1, Pixel 9a / Android 17, ~0.6s after `ExoPlayerImpl.Init`: + * + * ``` + * IllegalArgumentException + * at androidx.media3.datasource.DataSpec. // checkArgument(length > 0 || length == LENGTH_UNSET) + * at androidx.media3.datasource.DataSpec.subrange + * at androidx.media3.exoplayer.hls.HlsMediaChunk.feedDataToExtractor + * at androidx.media3.exoplayer.hls.HlsMediaChunk.loadMedia + * ``` + * + * `HlsMediaChunk.feedDataToExtractor` re-enters as `dataSpec.subrange(nextLoadPosition)`. Once the + * whole bounded range has been fed to the extractor, `nextLoadPosition == length`, so `subrange` + * asks for a zero-length `DataSpec` and the constructor's `length > 0` precondition throws. The + * early-return guard in `subrange` only covers `offset == 0`, so a fully-consumed chunk falls + * straight through. Unbounded chunks are safe — `length == C.LENGTH_UNSET` short-circuits — so this + * is reachable only via a byte-range part. + * + * The failure is unrecoverable rather than merely retried: `Loader` wraps it as + * `UnexpectedLoaderException`, which `DefaultLoadErrorHandlingPolicy` lists as non-retriable, so it + * becomes a fatal `ExoPlaybackException: Source error`. Forcing a retry would not help either — the + * `HlsMediaChunk` instance keeps its `nextLoadPosition`, so it would throw identically forever. + * + * Still present verbatim in media3 1.11.0-rc01, and unreported upstream at the time of writing, so + * there is no version to upgrade to. + * + * ## Trade-off + * + * We lose low latency on LL-HLS streams: playback falls back to whole segments, roughly one + * `TARGETDURATION` further behind the live edge. LL playlists still list their complete segments + * below the part tags, so they play normally otherwise. Given the alternative is a hard failure + * within a second, and that media3 offers no per-stream way to decline just the parts, disabling it + * globally is the conservative trade. + * + * Remove this once media3 fixes `HlsMediaChunk.feedDataToExtractor`. + */ +@UnstableApi +class LowLatencyStrippingHlsPlaylistParserFactory( + private val delegate: HlsPlaylistParserFactory = DefaultHlsPlaylistParserFactory(), +) : HlsPlaylistParserFactory { + override fun createPlaylistParser(): ParsingLoadable.Parser = LowLatencyStrippingParser(delegate.createPlaylistParser()) + + override fun createPlaylistParser( + multivariantPlaylist: HlsMultivariantPlaylist, + previousMediaPlaylist: HlsMediaPlaylist?, + ): ParsingLoadable.Parser = LowLatencyStrippingParser(delegate.createPlaylistParser(multivariantPlaylist, previousMediaPlaylist)) +} diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripperTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripperTest.kt new file mode 100644 index 0000000000..9735598411 --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripperTest.kt @@ -0,0 +1,163 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service.playback.playerPool + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Verbatim excerpt of a zap-stream-core LL-HLS media playlist (api-uk.zap.stream, 2026-07-26) — the + * playlist that crashes media3 1.10.1 in `HlsMediaChunk.feedDataToExtractor`. The byte-range parts + * are what produce the bounded chunk behind that crash. + */ +private val ZAP_STREAM_LL_PLAYLIST = + """ + #EXTM3U + #EXT-X-VERSION:6 + #EXT-X-PART-INF:PART-TARGET=0.49007290601730347 + #EXT-X-TARGETDURATION:2 + #EXT-X-MEDIA-SEQUENCE:191184 + #EXT-X-MAP:URI="init.mp4" + #EXT-X-SERVER-CONTROL:PART-HOLD-BACK=1.715,CAN-BLOCK-RELOAD=YES + #EXT-X-PROGRAM-DATE-TIME:2026-07-26T20:18:32.611Z + #EXTINF:1.961, + 191198.m4s + #EXT-X-PART:URI="191199.m4s",DURATION=0.5009999999892898,INDEPENDENT=YES,BYTERANGE="359712@0" + #EXT-X-PART:URI="191199.m4s",DURATION=0.5,BYTERANGE="153946@359712" + #EXT-X-PROGRAM-DATE-TIME:2026-07-26T20:18:34.572Z + #EXTINF:1.96, + 191199.m4s + """.trimIndent() + +class LowLatencyHlsStripperTest { + @Test + fun removesEveryLowLatencyTagFromARealPlaylist() { + val result = stripLowLatencyTags(ZAP_STREAM_LL_PLAYLIST) + + assertFalse("byte-range parts are the crash trigger", result.contains("#EXT-X-PART:")) + assertFalse(result.contains("#EXT-X-PART-INF:")) + assertFalse(result.contains("#EXT-X-SERVER-CONTROL:")) + assertFalse("no BYTERANGE should survive", result.contains("BYTERANGE")) + } + + @Test + fun keepsThePlaylistPlayable() { + val result = stripLowLatencyTags(ZAP_STREAM_LL_PLAYLIST) + + // Everything a plain HLS player needs must survive untouched. + assertTrue(result.startsWith("#EXTM3U")) + assertTrue(result.contains("#EXT-X-VERSION:6")) + assertTrue(result.contains("#EXT-X-TARGETDURATION:2")) + assertTrue(result.contains("#EXT-X-MEDIA-SEQUENCE:191184")) + assertTrue(result.contains("""#EXT-X-MAP:URI="init.mp4"""")) + assertTrue(result.contains("#EXT-X-PROGRAM-DATE-TIME:2026-07-26T20:18:34.572Z")) + assertTrue(result.contains("#EXTINF:1.961,")) + assertTrue(result.contains("191198.m4s")) + assertTrue(result.contains("191199.m4s")) + } + + @Test + fun keepsSegmentAndTagOrdering() { + val result = stripLowLatencyTags(ZAP_STREAM_LL_PLAYLIST) + + assertEquals( + listOf( + "#EXTM3U", + "#EXT-X-VERSION:6", + "#EXT-X-TARGETDURATION:2", + "#EXT-X-MEDIA-SEQUENCE:191184", + """#EXT-X-MAP:URI="init.mp4"""", + "#EXT-X-PROGRAM-DATE-TIME:2026-07-26T20:18:32.611Z", + "#EXTINF:1.961,", + "191198.m4s", + "#EXT-X-PROGRAM-DATE-TIME:2026-07-26T20:18:34.572Z", + "#EXTINF:1.96,", + "191199.m4s", + ), + result.split("\n"), + ) + } + + @Test + fun leavesAPlainPlaylistByteIdentical() { + // The common case, on every playlist reload of every live stream: nothing to do. + val plain = + """ + #EXTM3U + #EXT-X-VERSION:6 + #EXT-X-TARGETDURATION:2 + #EXT-X-MEDIA-SEQUENCE:4230 + #EXT-X-MAP:URI="init.mp4" + #EXTINF:2, + 4230.m4s + """.trimIndent() + + assertSame("unchanged playlists should not be rebuilt", plain, stripLowLatencyTags(plain)) + } + + @Test + fun preservesTrailingNewlineAndBlankLines() { + val input = "#EXTM3U\n#EXT-X-PART:URI=\"a.m4s\",BYTERANGE=\"1@0\"\n\n#EXTINF:2,\na.m4s\n" + + assertEquals("#EXTM3U\n\n#EXTINF:2,\na.m4s\n", stripLowLatencyTags(input)) + } + + @Test + fun preservesCrLfLineEndings() { + val input = "#EXTM3U\r\n#EXT-X-PART:URI=\"a.m4s\",BYTERANGE=\"1@0\"\r\n#EXTINF:2,\r\na.m4s\r\n" + + assertEquals("#EXTM3U\r\n#EXTINF:2,\r\na.m4s\r\n", stripLowLatencyTags(input)) + } + + @Test + fun keepsDeltaPlaylistAndRenditionReportTags() { + // EXT-X-SKIP marks legitimately omitted segments; removing it would corrupt the playlist. + // EXT-X-RENDITION-REPORT is inert once the parts are gone. + val input = + """ + #EXTM3U + #EXT-X-SKIP:SKIPPED-SEGMENTS=10 + #EXT-X-PART:URI="a.m4s",BYTERANGE="1@0" + #EXT-X-RENDITION-REPORT:URI="../b/live.m3u8",LAST-MSN=42 + """.trimIndent() + + val result = stripLowLatencyTags(input) + + assertTrue(result.contains("#EXT-X-SKIP:SKIPPED-SEGMENTS=10")) + assertTrue(result.contains("#EXT-X-RENDITION-REPORT:")) + assertFalse(result.contains("#EXT-X-PART:")) + } + + @Test + fun doesNotMatchTagsBySubstring() { + // EXT-X-PARTY-TIME is fictional, but the point is that the match must be anchored: a bare + // `contains("#EXT-X-PART")` without the colon would eat unrelated tags. + val input = "#EXTM3U\n#EXT-X-PARTY-TIME:1\n#EXT-X-PART:URI=\"a.m4s\"" + + val result = stripLowLatencyTags(input) + + assertTrue(result.contains("#EXT-X-PARTY-TIME:1")) + assertFalse(result.contains("#EXT-X-PART:")) + } +} From cabe539e8eb946f4a99ab6360b5f7b6b3325058f Mon Sep 17 00:00:00 2001 From: davotoula Date: Tue, 28 Jul 2026 19:46:18 +0200 Subject: [PATCH 2/4] Code review: - retire the `.m3u8` substring predicate across the UI - cover the preload hint and the byte/charset layer - share one HLS predicate with the liveness recorder - unify the HLS predicate and track the media3 bug upstream --- .../playback/composable/RenderVideoPlayer.kt | 4 +- .../composable/controls/RenderTopButtons.kt | 4 +- .../composable/mediaitem/IsHlsMedia.kt | 51 +++++++++++++ .../playback/diskCache/IsLiveStreaming.kt | 23 ------ .../playerPool/CustomMediaSourceFactory.kt | 72 ++++++++++++------ .../playerPool/HlsLivenessRecorder.kt | 10 ++- .../playerPool/LowLatencyHlsStripper.kt | 36 ++++++--- .../positions/CurrentPlayPositionCacher.kt | 7 +- .../ui/components/ZoomableContentDialog.kt | 4 +- .../amethyst/ui/note/types/VoiceTrack.kt | 4 +- .../loggedIn/profile/gallery/GalleryThumb.kt | 4 +- .../composable/mediaitem/IsHlsMediaTest.kt | 73 +++++++++++++++++++ .../playerPool/LowLatencyHlsStripperTest.kt | 70 ++++++++++++++---- 13 files changed, 273 insertions(+), 89 deletions(-) create mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/mediaitem/IsHlsMedia.kt delete mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/diskCache/IsLiveStreaming.kt create mode 100644 amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/composable/mediaitem/IsHlsMediaTest.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/RenderVideoPlayer.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/RenderVideoPlayer.kt index 30896bee0c..894948a953 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/RenderVideoPlayer.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/RenderVideoPlayer.kt @@ -53,9 +53,9 @@ import com.vitorpamplona.amethyst.service.playback.composable.controls.RenderTop import com.vitorpamplona.amethyst.service.playback.composable.controls.TopGradientOverlay import com.vitorpamplona.amethyst.service.playback.composable.controls.fullscreenSwipeControls import com.vitorpamplona.amethyst.service.playback.composable.mediaitem.LoadedMediaItem +import com.vitorpamplona.amethyst.service.playback.composable.mediaitem.isHlsMedia import com.vitorpamplona.amethyst.service.playback.composable.wavefront.AudioPlayingAnimation import com.vitorpamplona.amethyst.service.playback.composable.wavefront.rememberIsAudioTrack -import com.vitorpamplona.amethyst.service.playback.diskCache.isLiveStreaming import com.vitorpamplona.amethyst.ui.components.getDialogWindow import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel @@ -105,7 +105,7 @@ fun RenderVideoPlayer( // unnecessary recomposition of the whole player tree just to update a value that is only // ever read inside the onDoubleTap callback below. val containerWidth = remember { intArrayOf(0) } - val isLive = remember(mediaItem.src.videoUri) { isLiveStreaming(mediaItem.src.videoUri) } + val isLive = remember(mediaItem.src.videoUri, mediaItem.src.mimeType) { isHlsMedia(mediaItem.src.videoUri, mediaItem.src.mimeType) } val swipeState = remember { FullscreenSwipeControlsState() } val context = LocalContext.current diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/RenderTopButtons.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/RenderTopButtons.kt index 843b83f01e..b6eac316f5 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/RenderTopButtons.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/controls/RenderTopButtons.kt @@ -63,7 +63,7 @@ import com.vitorpamplona.amethyst.service.cast.CastSessionState import com.vitorpamplona.amethyst.service.playback.composable.DEFAULT_MUTED_SETTING import com.vitorpamplona.amethyst.service.playback.composable.MediaControllerState import com.vitorpamplona.amethyst.service.playback.composable.mediaitem.MediaItemData -import com.vitorpamplona.amethyst.service.playback.diskCache.isLiveStreaming +import com.vitorpamplona.amethyst.service.playback.composable.mediaitem.isHlsMedia import com.vitorpamplona.amethyst.service.playback.pip.PipVideoActivity import com.vitorpamplona.amethyst.ui.cast.CastDevicePickerDialog import com.vitorpamplona.amethyst.ui.components.ShareMediaAction @@ -113,7 +113,7 @@ fun RenderTopButtons( accountViewModel: AccountViewModel, ) { val context = LocalContext.current - val isLive = remember(mediaData.videoUri) { isLiveStreaming(mediaData.videoUri) } + val isLive = remember(mediaData.videoUri, mediaData.mimeType) { isHlsMedia(mediaData.videoUri, mediaData.mimeType) } val pipSupported = remember { context.packageManager.hasSystemFeature(PackageManager.FEATURE_PICTURE_IN_PICTURE) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/mediaitem/IsHlsMedia.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/mediaitem/IsHlsMedia.kt new file mode 100644 index 0000000000..464553f440 --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/composable/mediaitem/IsHlsMedia.kt @@ -0,0 +1,51 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service.playback.composable.mediaitem + +import androidx.media3.common.MimeTypes + +/** + * Whether a URL plus its imeta mime identifies HLS. + * + * This is the caller-side form for code that holds a URL and a mime but no `MediaItem` yet — UI that + * has a [MediaItemData] or a `MediaUrlContent`. Once a `MediaItem` exists, + * `isHlsMediaItem` is the equivalent, and both must answer the same way: the UI decides what to + * render, the factory decides how to load it, and a disagreement shows up as a player that streams + * something the surrounding chrome says is a still image. + * + * Defined in terms of [MediaItemCache.toExoPlayerMimeType] rather than re-deriving the rules, so it + * cannot drift from the mime that actually reaches ExoPlayer. That normalizer prefers an explicit + * mime (mapping the four HLS aliases onto [MimeTypes.APPLICATION_M3U8]) and otherwise falls back to a + * **path-anchored** `.m3u8` test. + * + * Both halves matter. A BUD-10 blossom playlist is `https://host/` with no extension at all, + * so only the mime identifies it; and anchoring to the path stops `video.mp4?ref=a.m3u8` counting as + * HLS on the strength of its query string. + * + * Note this answers *is it HLS*, which callers use as a proxy for *is it live*. The proxy is + * imprecise in the same way for every HLS URL — an on-demand HLS playlist also answers true — and + * that imprecision is older than this function. Liveness is only truly knowable from + * `#EXT-X-ENDLIST` once the playlist is loaded, which is what `HlsLivenessCache` records. + */ +fun isHlsMedia( + url: String, + mimeType: String?, +): Boolean = MediaItemCache.toExoPlayerMimeType(mimeType, url) == MimeTypes.APPLICATION_M3U8 diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/diskCache/IsLiveStreaming.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/diskCache/IsLiveStreaming.kt deleted file mode 100644 index 0bc15b4868..0000000000 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/diskCache/IsLiveStreaming.kt +++ /dev/null @@ -1,23 +0,0 @@ -/* - * Copyright (c) 2025 Vitor Pamplona - * - * Permission is hereby granted, free of charge, to any person obtaining a copy of - * this software and associated documentation files (the "Software"), to deal in - * the Software without restriction, including without limitation the rights to use, - * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the - * Software, and to permit persons to whom the Software is furnished to do so, - * subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all - * copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS - * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR - * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN - * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION - * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - */ -package com.vitorpamplona.amethyst.service.playback.diskCache - -fun isLiveStreaming(url: String) = url.contains(".m3u8", true) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/CustomMediaSourceFactory.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/CustomMediaSourceFactory.kt index 692263214d..d9ec09d6e9 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/CustomMediaSourceFactory.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/CustomMediaSourceFactory.kt @@ -34,7 +34,6 @@ import com.vitorpamplona.amethyst.service.playback.PLAYBACK_DIAG_TAG import com.vitorpamplona.amethyst.service.playback.composable.mediaitem.MediaItemCache import com.vitorpamplona.amethyst.service.playback.diskCache.HlsLivenessCache import com.vitorpamplona.amethyst.service.playback.diskCache.VideoCache -import com.vitorpamplona.amethyst.service.playback.diskCache.isLiveStreaming import com.vitorpamplona.quartz.utils.Log /** @@ -61,6 +60,26 @@ internal fun shouldBypassCache( else -> true } +/** + * Whether a [MediaItem] is HLS, via media3's own content-type inference. + * + * This is the one "is this HLS?" predicate for playback routing. [CustomMediaSourceFactory] uses it + * to pick both the cache route and the media-source factory, and [HlsLivenessRecorder] uses it to + * decide which items are worth learning a liveness verdict for. Those two **must** agree: if the + * recorder tested more narrowly than the factory, an item the factory treats as HLS would never be + * classified, [HlsLivenessCache] would answer `isKnownOnDemand = false` for it forever, and + * [shouldBypassCache] would keep it out of the disk cache permanently. + * + * It reads back the mimeType [MediaItemCache] already normalised, which is what makes it correct for + * BUD-10 blossom URIs — `https://host/`, no extension, mime as the only signal. A `.m3u8` + * substring test on the URL misses those, and separately gives a false positive on a query string + * over progressive media (`video.mp4?ref=a.m3u8`). + */ +internal fun isHlsMediaItem(mediaItem: MediaItem?): Boolean { + val config = mediaItem?.localConfiguration ?: return false + return Util.inferContentTypeForUriAndMimeType(config.uri, config.mimeType) == C.CONTENT_TYPE_HLS +} + /** * Decides whether a [MediaItem] plays through the caching data source or bypasses it. * @@ -86,31 +105,40 @@ class CustomMediaSourceFactory( ) : MediaSource.Factory { private val cachingDataSource: DataSource.Factory = videoCache.get(dataSourceFactory) - private var cachingFactory: MediaSource.Factory = + private val cachingFactory: MediaSource.Factory = DefaultMediaSourceFactory(cachingDataSource) - private var nonCachingFactory: MediaSource.Factory = + private val nonCachingFactory: MediaSource.Factory = DefaultMediaSourceFactory(dataSourceFactory) + // Stateless, so one instance serves both HLS factories. + private val playlistParserFactory = LowLatencyStrippingHlsPlaylistParserFactory() + // HLS is built explicitly rather than through DefaultMediaSourceFactory, which exposes no hook // for a playlist parser factory. See LowLatencyStrippingHlsPlaylistParserFactory for why we need // one. Everything else still routes through the Default factories above. + // + // The cost of bypassing it: HLS items skip what DefaultMediaSourceFactory wraps around the + // source — side-loaded subtitleConfigurations (MergingMediaSource), clipping, ad insertion, and + // live target-offset defaults. None are reachable today (MediaItemCache sets none of them, and + // the live setters aren't on the MediaSource.Factory interface), but anything added later must + // be mirrored here. private val cachingHlsFactory: MediaSource.Factory = hlsFactory(cachingDataSource) private val nonCachingHlsFactory: MediaSource.Factory = hlsFactory(dataSourceFactory) private fun hlsFactory(dataSource: DataSource.Factory): MediaSource.Factory = HlsMediaSource .Factory(dataSource) - .setPlaylistParserFactory(LowLatencyStrippingHlsPlaylistParserFactory()) + .setPlaylistParserFactory(playlistParserFactory) - private fun allFactories() = listOf(cachingFactory, nonCachingFactory, cachingHlsFactory, nonCachingHlsFactory) + private val allFactories = listOf(cachingFactory, nonCachingFactory, cachingHlsFactory, nonCachingHlsFactory) override fun setDrmSessionManagerProvider(drmSessionManagerProvider: DrmSessionManagerProvider): MediaSource.Factory { - allFactories().forEach { it.setDrmSessionManagerProvider(drmSessionManagerProvider) } + allFactories.forEach { it.setDrmSessionManagerProvider(drmSessionManagerProvider) } return this } override fun setLoadErrorHandlingPolicy(loadErrorHandlingPolicy: LoadErrorHandlingPolicy): MediaSource.Factory { - allFactories().forEach { it.setLoadErrorHandlingPolicy(loadErrorHandlingPolicy) } + allFactories.forEach { it.setLoadErrorHandlingPolicy(loadErrorHandlingPolicy) } return this } @@ -119,31 +147,27 @@ class CustomMediaSourceFactory( override fun createMediaSource(mediaItem: MediaItem): MediaSource { val id = mediaItem.mediaId val flaggedLive = isFlaggedLive(mediaItem) - val hls = isLiveStreaming(id) + + // One predicate governs both the cache routing below and which factory builds the source, so + // the two can never disagree. See isHlsMediaItem. + val hls = isHlsMediaItem(mediaItem) + val knownOnDemand = HlsLivenessCache.isKnownOnDemand(id) val bypassCache = shouldBypassCache(flaggedLive, hls, knownOnDemand) - // Reuse media3's own inference rather than the looser `.m3u8`-substring check that drives - // cache routing, so the HLS branch here always agrees with what DefaultMediaSourceFactory - // would have picked. `isLiveStreaming` matches a `.m3u8` anywhere in the URL — including a - // query string on a progressive file — which must not select an HlsMediaSource. - val config = mediaItem.localConfiguration - val isHlsContent = - config != null && - Util.inferContentTypeForUriAndMimeType(config.uri, config.mimeType) == C.CONTENT_TYPE_HLS - - val source = - when { - isHlsContent && bypassCache -> nonCachingHlsFactory.createMediaSource(mediaItem) - isHlsContent -> cachingHlsFactory.createMediaSource(mediaItem) - bypassCache -> nonCachingFactory.createMediaSource(mediaItem) - else -> cachingFactory.createMediaSource(mediaItem) + val factory = + if (hls) { + if (bypassCache) nonCachingHlsFactory else cachingHlsFactory + } else { + if (bypassCache) nonCachingFactory else cachingFactory } + val source = factory.createMediaSource(mediaItem) + // Logs the routing inputs directly rather than a re-derived label, so it can't drift from // shouldBypassCache. Log.d(PLAYBACK_DIAG_TAG) { "SOURCE ${if (bypassCache) "BYPASS" else "CACHE"} flaggedLive=$flaggedLive hls=$hls knownOnDemand=$knownOnDemand " + - "hlsContent=$isHlsContent mime=${config?.mimeType} -> ${source::class.java.simpleName} id=$id" + "mime=${mediaItem.localConfiguration?.mimeType} -> ${source::class.java.simpleName} id=$id" } return source } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/HlsLivenessRecorder.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/HlsLivenessRecorder.kt index 4da281e78a..9b95564fed 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/HlsLivenessRecorder.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/HlsLivenessRecorder.kt @@ -25,7 +25,6 @@ import androidx.media3.common.Player import androidx.media3.common.Timeline import com.vitorpamplona.amethyst.service.playback.PLAYBACK_DIAG_TAG import com.vitorpamplona.amethyst.service.playback.diskCache.HlsLivenessCache -import com.vitorpamplona.amethyst.service.playback.diskCache.isLiveStreaming import com.vitorpamplona.quartz.utils.Log /** @@ -66,7 +65,7 @@ internal fun livenessVerdictToRecord( * reliable live/on-demand discriminator (`#EXT-X-ENDLIST`) is inside the playlist, so it is knowable * only once ExoPlayer has loaded it — hence learning it here rather than from the URL. * - * Only `.m3u8` items are considered; progressive media is unambiguous and never routed by liveness. + * Only HLS items are considered; progressive media is unambiguous and never routed by liveness. */ class HlsLivenessRecorder( private val player: Player, @@ -85,8 +84,11 @@ class HlsLivenessRecorder( private fun maybeRecord(allowOnDemand: Boolean) { if (player.currentTimeline.isEmpty) return - val url = player.currentMediaItem?.mediaId ?: return - if (!isLiveStreaming(url)) return + val mediaItem = player.currentMediaItem ?: return + // Must be the same predicate CustomMediaSourceFactory routes on — see isHlsMediaItem for + // what goes wrong when the two disagree. + if (!isHlsMediaItem(mediaItem)) return + val url = mediaItem.mediaId val known = HlsLivenessCache.verdict(url) val toRecord = diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripper.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripper.kt index c803437304..45ee84bc05 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripper.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripper.kt @@ -74,6 +74,23 @@ internal fun stripLowLatencyTags(playlist: String): String { }.joinToString("\n") } +/** + * The byte-level form: decode as UTF-8, strip, re-encode. + * + * Split out from [LowLatencyStrippingParser] so the charset round-trip is reachable from a plain JVM + * unit test — `parse` takes an `android.net.Uri`, which stubs to null under + * `unitTests.isReturnDefaultValues`, so nothing that goes through it is testable without Robolectric. + * + * A UTF-8 BOM survives: decoding leaves U+FEFF in the string, `trimStart` does not treat it as + * whitespace, and re-encoding reproduces the same three bytes. When there is nothing to strip the + * *original array* is returned, so the common path neither re-encodes nor copies. + */ +internal fun stripLowLatencyTags(playlist: ByteArray): ByteArray { + val original = playlist.toString(Charsets.UTF_8) + val stripped = stripLowLatencyTags(original) + return if (stripped === original) playlist else stripped.toByteArray(Charsets.UTF_8) +} + /** * Wraps a media3 playlist parser and strips the Low-Latency tags before delegating. * @@ -87,11 +104,7 @@ internal class LowLatencyStrippingParser( override fun parse( uri: Uri, inputStream: InputStream, - ): HlsPlaylist { - val original = inputStream.readBytes().toString(Charsets.UTF_8) - val stripped = stripLowLatencyTags(original) - return delegate.parse(uri, ByteArrayInputStream(stripped.toByteArray(Charsets.UTF_8))) - } + ): HlsPlaylist = delegate.parse(uri, ByteArrayInputStream(stripLowLatencyTags(inputStream.readBytes()))) } /** @@ -123,8 +136,13 @@ internal class LowLatencyStrippingParser( * becomes a fatal `ExoPlaybackException: Source error`. Forcing a retry would not help either — the * `HlsMediaChunk` instance keeps its `nextLoadPosition`, so it would throw identically forever. * - * Still present verbatim in media3 1.11.0-rc01, and unreported upstream at the time of writing, so - * there is no version to upgrade to. + * Still present verbatim in media3 1.11.0-rc01, so there is no version to upgrade to. + * + * **Tracking: https://github.com/androidx/media/issues/3350** — delete this whole file and its test + * once that is fixed and we are on a media3 release carrying the fix, then drop the explicit + * `HlsMediaSource.Factory` in [CustomMediaSourceFactory] and let `DefaultMediaSourceFactory` build + * HLS again. That also restores low latency, and removes the caveat about the wrapping + * `DefaultMediaSourceFactory` features documented there. * * ## Trade-off * @@ -133,11 +151,9 @@ internal class LowLatencyStrippingParser( * below the part tags, so they play normally otherwise. Given the alternative is a hard failure * within a second, and that media3 offers no per-stream way to decline just the parts, disabling it * globally is the conservative trade. - * - * Remove this once media3 fixes `HlsMediaChunk.feedDataToExtractor`. */ @UnstableApi -class LowLatencyStrippingHlsPlaylistParserFactory( +internal class LowLatencyStrippingHlsPlaylistParserFactory( private val delegate: HlsPlaylistParserFactory = DefaultHlsPlaylistParserFactory(), ) : HlsPlaylistParserFactory { override fun createPlaylistParser(): ParsingLoadable.Parser = LowLatencyStrippingParser(delegate.createPlaylistParser()) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/positions/CurrentPlayPositionCacher.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/positions/CurrentPlayPositionCacher.kt index c5d43c0954..3e6048c510 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/positions/CurrentPlayPositionCacher.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/positions/CurrentPlayPositionCacher.kt @@ -22,7 +22,7 @@ package com.vitorpamplona.amethyst.service.playback.playerPool.positions import androidx.media3.common.MediaItem import androidx.media3.common.Player -import com.vitorpamplona.amethyst.service.playback.diskCache.isLiveStreaming +import com.vitorpamplona.amethyst.service.playback.playerPool.isHlsMediaItem import kotlin.math.abs class CurrentPlayPositionCacher( @@ -41,7 +41,10 @@ class CurrentPlayPositionCacher( isLiveStreaming = false } else { currentUrl = mediaItem.mediaId - isLiveStreaming = isLiveStreaming(mediaItem.mediaId) + // Same predicate the source factory routes on, so a blossom-hosted playlist — which has + // no `.m3u8` in its URL — is recognised here too and doesn't get a resume position + // persisted against a stream that has no stable position to return to. + isLiveStreaming = isHlsMediaItem(mediaItem) } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentDialog.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentDialog.kt index defca41a12..f211bbfa4f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentDialog.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentDialog.kt @@ -98,7 +98,7 @@ import com.vitorpamplona.amethyst.commons.richtext.MediaUrlVideo import com.vitorpamplona.amethyst.commons.richtext.toCoilModel import com.vitorpamplona.amethyst.model.MediaAspectRatioCache import com.vitorpamplona.amethyst.service.playback.composable.VideoViewInner -import com.vitorpamplona.amethyst.service.playback.diskCache.isLiveStreaming +import com.vitorpamplona.amethyst.service.playback.composable.mediaitem.isHlsMedia import com.vitorpamplona.amethyst.ui.actions.MediaSaverToDisk import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel import com.vitorpamplona.amethyst.ui.stringRes @@ -431,7 +431,7 @@ private fun DialogContent( } val isPdfOrStaticImage = myContent is MediaUrlImage || myContent is MediaLocalImage || myContent is MediaUrlPdf - val isNotLiveStream = myContent !is MediaUrlContent || !isLiveStreaming(myContent.url) + val isNotLiveStream = myContent !is MediaUrlContent || !isHlsMedia(myContent.url, myContent.mimeType) if (isPdfOrStaticImage && isNotLiveStream) { val localContext = LocalContext.current diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/VoiceTrack.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/VoiceTrack.kt index 52e76860cd..f11f31d5e6 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/VoiceTrack.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/note/types/VoiceTrack.kt @@ -63,8 +63,8 @@ import com.vitorpamplona.amethyst.service.playback.composable.controls.PictureIn import com.vitorpamplona.amethyst.service.playback.composable.mediaitem.GetMediaItem import com.vitorpamplona.amethyst.service.playback.composable.mediaitem.LoadedMediaItem import com.vitorpamplona.amethyst.service.playback.composable.mediaitem.MediaItemData +import com.vitorpamplona.amethyst.service.playback.composable.mediaitem.isHlsMedia import com.vitorpamplona.amethyst.service.playback.composable.wavefront.Waveform -import com.vitorpamplona.amethyst.service.playback.diskCache.isLiveStreaming import com.vitorpamplona.amethyst.service.playback.pip.PipVideoActivity import com.vitorpamplona.amethyst.ui.components.ShareMediaAction import com.vitorpamplona.amethyst.ui.components.getActivity @@ -349,7 +349,7 @@ fun RenderTopButtonsForVoice( accountViewModel: AccountViewModel, ) { Row(modifier) { - if (!isLiveStreaming(mediaData.videoUri)) { + if (!isHlsMedia(mediaData.videoUri, mediaData.mimeType)) { AnimatedShareButton(controllerVisible) { popupExpanded, toggle -> ShareMediaAction( popupExpanded = popupExpanded, diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/gallery/GalleryThumb.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/gallery/GalleryThumb.kt index 6bc0a4d96d..009e8194d9 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/gallery/GalleryThumb.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/profile/gallery/GalleryThumb.kt @@ -51,7 +51,7 @@ import com.vitorpamplona.amethyst.commons.richtext.RichTextParser.Companion.isVi import com.vitorpamplona.amethyst.commons.richtext.toCoilModel import com.vitorpamplona.amethyst.commons.ui.components.LoadingAnimation import com.vitorpamplona.amethyst.model.Note -import com.vitorpamplona.amethyst.service.playback.diskCache.isLiveStreaming +import com.vitorpamplona.amethyst.service.playback.composable.mediaitem.isHlsMedia import com.vitorpamplona.amethyst.service.relayClient.reqCommand.event.observeNote import com.vitorpamplona.amethyst.ui.actions.CrossfadeIfEnabled import com.vitorpamplona.amethyst.ui.components.AutoNonlazyGrid @@ -265,7 +265,7 @@ fun UrlImageView( content.toCoilModel(useLocalBlossomBridge) } val imageModelUrl = artworkUri ?: bridgedUrl - val canLoadAsImage = !isVideo || artworkUri != null || !isLiveStreaming(content.url) + val canLoadAsImage = !isVideo || artworkUri != null || !isHlsMedia(content.url, content.mimeType) CrossfadeIfEnabled(targetState = showImage.value, contentAlignment = Alignment.Center, accountViewModel = accountViewModel) { if (it && canLoadAsImage) { diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/composable/mediaitem/IsHlsMediaTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/composable/mediaitem/IsHlsMediaTest.kt new file mode 100644 index 0000000000..9b34587518 --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/composable/mediaitem/IsHlsMediaTest.kt @@ -0,0 +1,73 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service.playback.composable.mediaitem + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The two cases that motivated replacing the old `.m3u8`-substring predicate are + * [recognisesAnExtensionlessBlossomPlaylistByMime] and [ignoresM3u8InAQueryString]; the rest pin the + * behaviour that must not regress while fixing them. + */ +class IsHlsMediaTest { + @Test + fun recognisesAnExtensionlessBlossomPlaylistByMime() { + // BUD-10: the URL is a bare sha256 with no extension, so the mime is the only signal. The + // old substring predicate answered false here and the item was routed to the disk cache — + // fatal for a live playlist. + val blossom = "https://blossom.example.com/b1674191a88ec5cdd733e4240a81803105dc412d6c6708d53ab94fc248f4f553" + + assertTrue(isHlsMedia(blossom, "application/x-mpegurl")) + assertTrue(isHlsMedia(blossom, "application/vnd.apple.mpegurl")) + assertFalse("no mime and no extension leaves nothing to go on", isHlsMedia(blossom, null)) + } + + @Test + fun ignoresM3u8InAQueryString() { + // The other direction: progressive media permanently excluded from the cache because its + // query string mentioned a playlist. + assertFalse(isHlsMedia("https://host/video.mp4?ref=a.m3u8", null)) + assertFalse(isHlsMedia("https://host/video.mp4#a.m3u8", null)) + } + + @Test + fun recognisesAPlainM3u8Path() { + assertTrue(isHlsMedia("https://host/live.m3u8", null)) + assertTrue(isHlsMedia("https://host/live.M3U8", null)) + assertTrue("query strings don't hide the path", isHlsMedia("https://host/live.m3u8?vt=abc", null)) + } + + @Test + fun anExplicitMimeWins() { + // A non-HLS mime is respected even on an .m3u8 path — the mime is the more specific signal, + // and toExoPlayerMimeType only consults the path when no mime was supplied. + assertFalse(isHlsMedia("https://host/odd.m3u8", "video/mp4")) + } + + @Test + fun progressiveMediaIsNotHls() { + assertFalse(isHlsMedia("https://host/video.mp4", null)) + assertFalse(isHlsMedia("https://host/video.mp4", "video/mp4")) + assertFalse(isHlsMedia("https://host/audio.m4a", "audio/mp4")) + } +} diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripperTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripperTest.kt index 9735598411..406274260c 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripperTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripperTest.kt @@ -20,6 +20,7 @@ */ package com.vitorpamplona.amethyst.service.playback.playerPool +import org.junit.Assert.assertArrayEquals import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertSame @@ -61,26 +62,12 @@ class LowLatencyHlsStripperTest { assertFalse("no BYTERANGE should survive", result.contains("BYTERANGE")) } - @Test - fun keepsThePlaylistPlayable() { - val result = stripLowLatencyTags(ZAP_STREAM_LL_PLAYLIST) - - // Everything a plain HLS player needs must survive untouched. - assertTrue(result.startsWith("#EXTM3U")) - assertTrue(result.contains("#EXT-X-VERSION:6")) - assertTrue(result.contains("#EXT-X-TARGETDURATION:2")) - assertTrue(result.contains("#EXT-X-MEDIA-SEQUENCE:191184")) - assertTrue(result.contains("""#EXT-X-MAP:URI="init.mp4"""")) - assertTrue(result.contains("#EXT-X-PROGRAM-DATE-TIME:2026-07-26T20:18:34.572Z")) - assertTrue(result.contains("#EXTINF:1.961,")) - assertTrue(result.contains("191198.m4s")) - assertTrue(result.contains("191199.m4s")) - } - @Test fun keepsSegmentAndTagOrdering() { val result = stripLowLatencyTags(ZAP_STREAM_LL_PLAYLIST) + // Exact equality, so this also pins that everything a plain HLS player needs survives + // untouched and in order: the header, MAP, PROGRAM-DATE-TIME, EXTINF and both segments. assertEquals( listOf( "#EXTM3U", @@ -149,6 +136,57 @@ class LowLatencyHlsStripperTest { assertFalse(result.contains("#EXT-X-PART:")) } + @Test + fun stripsPreloadHint() { + // The *other* tag that yields a byte-range chunk, so it matters as much as EXT-X-PART. + // Synthetic rather than folded into the capture above, which is labelled verbatim and did + // not carry a hint; shaped per RFC 8216 §4.4.5.3. + val input = + """ + #EXTM3U + #EXTINF:1.96, + 191199.m4s + #EXT-X-PRELOAD-HINT:TYPE=PART,URI="191200.m4s",BYTERANGE-START=402501 + """.trimIndent() + + val result = stripLowLatencyTags(input) + + assertFalse(result.contains("#EXT-X-PRELOAD-HINT")) + assertFalse("no byte range should survive", result.contains("BYTERANGE-START")) + assertEquals("#EXTM3U\n#EXTINF:1.96,\n191199.m4s", result) + } + + @Test + fun byteFormPreservesBomAndMultibyteWhenStripping() { + // Written as raw bytes rather than a "" literal so the fixture states the on-the-wire + // encoding directly. 🎵 is a 4-byte sequence / surrogate pair, so it also covers the + // decode-modify-re-encode round trip beyond the BMP. + val bom = byteArrayOf(0xEF.toByte(), 0xBB.toByte(), 0xBF.toByte()) + val input = + bom + + ( + "#EXTM3U\n" + + "#EXT-X-PART:URI=\"a.m4s\",BYTERANGE=\"1@0\"\n" + + "#EXTINF:2,caffè 🎵\n" + + "a.m4s\n" + ).toByteArray(Charsets.UTF_8) + + val result = stripLowLatencyTags(input) + + assertArrayEquals( + bom + "#EXTM3U\n#EXTINF:2,caffè 🎵\na.m4s\n".toByteArray(Charsets.UTF_8), + result, + ) + } + + @Test + fun byteFormForwardsTheOriginalArrayWhenNothingToStrip() { + val input = "#EXTM3U\n#EXTINF:2,\na.m4s\n".toByteArray(Charsets.UTF_8) + + // Same array, not an equal copy: the common path must not re-encode. + assertSame(input, stripLowLatencyTags(input)) + } + @Test fun doesNotMatchTagsBySubstring() { // EXT-X-PARTY-TIME is fictional, but the point is that the match must be anchored: a bare From 76b33235837350c17a03993f31ff51a6c71f7488 Mon Sep 17 00:00:00 2001 From: davotoula Date: Wed, 29 Jul 2026 13:37:16 +0200 Subject: [PATCH 3/4] TEMP: HlsVerify error-level probes for on-device verification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DO NOT MERGE — revert this commit before the branch goes anywhere. PLAYBACK_DIAG_TAG logs at DEBUG and Amethyst.onCreate sets `Log.minLevel = if (BuildConfig.DEBUG) DEBUG else ERROR`, so nothing on that tag survives in the benchmark build. Three logcat captures during this work had zero PlaybackDiag lines for exactly that reason, which left the routing decision unverifiable — the fix was only ever confirmed indirectly, by the crash no longer happening. Adds one ERROR-level tag, HlsVerify, on the three paths unit tests cannot reach: - CustomMediaSourceFactory.createMediaSource — the routing decision, so isHlsMediaItem's verdict and the chosen factory are visible. This is the predicate with no test coverage, since android.net.Uri stubs to null under unitTests.isReturnDefaultValues. - LowLatencyStrippingParser.parse — whether the stripper actually engaged and how many bytes it removed, so a silent stop-stripping regression is visible rather than inferred. - HlsLivenessRecorder.maybeRecord — the other consumer of the shared predicate, unreachable without a Player. The pure predicates underneath are already unit-tested and remain the durable regression guard; this is only for confirming the wiring on a device. Capture with `adb logcat -s HlsVerify:E`. Remove with `grep -rn HlsVerify` or by reverting this commit. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018uE2Db4pzmhi5cFKKkyq2m --- .../amethyst/service/playback/HlsVerifyLog.kt | 44 +++++++++++++++++++ .../playerPool/CustomMediaSourceFactory.kt | 6 +++ .../playerPool/HlsLivenessRecorder.kt | 3 ++ .../playerPool/LowLatencyHlsStripper.kt | 16 ++++++- 4 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/HlsVerifyLog.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/HlsVerifyLog.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/HlsVerifyLog.kt new file mode 100644 index 0000000000..9f89e66b1c --- /dev/null +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/HlsVerifyLog.kt @@ -0,0 +1,44 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.service.playback + +/** + * TEMPORARY — delete this file and its three call sites before merging. + * `grep -rn HlsVerify` finds all of them; reverting the commit that added it does the same job. + * + * [PLAYBACK_DIAG_TAG] logs at DEBUG, and `Amethyst.onCreate` sets + * `Log.minLevel = if (BuildConfig.DEBUG) DEBUG else ERROR` — so nothing on that tag survives in a + * benchmark or release build. On-device verification of the HLS work therefore has to log at ERROR + * to be visible at all. + * + * Deliberately placed only on the paths that unit tests *cannot* reach: + * - `isHlsMediaItem` and the factory choice — needs `android.net.Uri`, which stubs to null under + * `unitTests.isReturnDefaultValues`. + * - `LowLatencyStrippingParser.parse` — needs a `Uri` for the same reason. + * - `HlsLivenessRecorder.maybeRecord` — needs a `Player`. + * + * The pure predicates underneath (`stripLowLatencyTags`, `isHlsMedia`, `shouldBypassCache`, + * `livenessVerdictToRecord`) already have unit tests, which are the durable regression guard. This + * tag is only for confirming the wiring on a real device. + * + * Capture with: `adb logcat -s HlsVerify:E` + */ +const val HLS_VERIFY_TAG = "HlsVerify" diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/CustomMediaSourceFactory.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/CustomMediaSourceFactory.kt index d9ec09d6e9..bfabbd885d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/CustomMediaSourceFactory.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/CustomMediaSourceFactory.kt @@ -30,6 +30,7 @@ import androidx.media3.exoplayer.hls.HlsMediaSource import androidx.media3.exoplayer.source.DefaultMediaSourceFactory import androidx.media3.exoplayer.source.MediaSource import androidx.media3.exoplayer.upstream.LoadErrorHandlingPolicy +import com.vitorpamplona.amethyst.service.playback.HLS_VERIFY_TAG import com.vitorpamplona.amethyst.service.playback.PLAYBACK_DIAG_TAG import com.vitorpamplona.amethyst.service.playback.composable.mediaitem.MediaItemCache import com.vitorpamplona.amethyst.service.playback.diskCache.HlsLivenessCache @@ -165,6 +166,11 @@ class CustomMediaSourceFactory( // Logs the routing inputs directly rather than a re-derived label, so it can't drift from // shouldBypassCache. + // TEMPORARY HlsVerify — see HlsVerifyLog.kt. Remove before merge. + Log.e(HLS_VERIFY_TAG) { + "ROUTE hls=$hls bypassCache=$bypassCache flaggedLive=$flaggedLive knownOnDemand=$knownOnDemand " + + "mime=${mediaItem.localConfiguration?.mimeType} -> ${source::class.java.simpleName} id=$id" + } Log.d(PLAYBACK_DIAG_TAG) { "SOURCE ${if (bypassCache) "BYPASS" else "CACHE"} flaggedLive=$flaggedLive hls=$hls knownOnDemand=$knownOnDemand " + "mime=${mediaItem.localConfiguration?.mimeType} -> ${source::class.java.simpleName} id=$id" diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/HlsLivenessRecorder.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/HlsLivenessRecorder.kt index 9b95564fed..7ff88465db 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/HlsLivenessRecorder.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/HlsLivenessRecorder.kt @@ -23,6 +23,7 @@ package com.vitorpamplona.amethyst.service.playback.playerPool import androidx.media3.common.C import androidx.media3.common.Player import androidx.media3.common.Timeline +import com.vitorpamplona.amethyst.service.playback.HLS_VERIFY_TAG import com.vitorpamplona.amethyst.service.playback.PLAYBACK_DIAG_TAG import com.vitorpamplona.amethyst.service.playback.diskCache.HlsLivenessCache import com.vitorpamplona.quartz.utils.Log @@ -105,6 +106,8 @@ class HlsLivenessRecorder( // refresh of a live stream, and the verdict is stable once learned, so re-putting the same // value would take a ConcurrentHashMap bin lock on every callback for nothing. if (known != toRecord) { + // TEMPORARY HlsVerify — see HlsVerifyLog.kt. Remove before merge. + Log.e(HLS_VERIFY_TAG) { "LIVENESS ${if (toRecord) "LIVE" else "ON-DEMAND"} learned for $url" } Log.d(PLAYBACK_DIAG_TAG) { "LIVENESS ${if (toRecord) "LIVE" else "ON-DEMAND"} learned for $url" } HlsLivenessCache.record(url, toRecord) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripper.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripper.kt index 45ee84bc05..50d61e1af1 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripper.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripper.kt @@ -28,6 +28,8 @@ import androidx.media3.exoplayer.hls.playlist.HlsMultivariantPlaylist import androidx.media3.exoplayer.hls.playlist.HlsPlaylist import androidx.media3.exoplayer.hls.playlist.HlsPlaylistParserFactory import androidx.media3.exoplayer.upstream.ParsingLoadable +import com.vitorpamplona.amethyst.service.playback.HLS_VERIFY_TAG +import com.vitorpamplona.quartz.utils.Log import java.io.ByteArrayInputStream import java.io.InputStream @@ -104,7 +106,19 @@ internal class LowLatencyStrippingParser( override fun parse( uri: Uri, inputStream: InputStream, - ): HlsPlaylist = delegate.parse(uri, ByteArrayInputStream(stripLowLatencyTags(inputStream.readBytes()))) + ): HlsPlaylist { + val original = inputStream.readBytes() + val stripped = stripLowLatencyTags(original) + // TEMPORARY HlsVerify — see HlsVerifyLog.kt. Remove before merge. + Log.e(HLS_VERIFY_TAG) { + if (stripped === original) { + "PARSE no LL tags (${original.size}B) $uri" + } else { + "PARSE STRIPPED ${original.size - stripped.size}B of ${original.size}B $uri" + } + } + return delegate.parse(uri, ByteArrayInputStream(stripped)) + } } /** From c26988f39969fc9d6255b6bd1320823ef20d0822 Mon Sep 17 00:00:00 2001 From: davotoula Date: Wed, 29 Jul 2026 13:56:15 +0200 Subject: [PATCH 4/4] Revert "TEMP: HlsVerify error-level probes for on-device verification" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 76b3323583. The probes did their job. Verified on a Pixel 9a / Android 17 against beam.mapboss.co.th — a third-party LL-HLS packager, unrelated to zap-stream-core, so this is not tuned to one vendor's output: PARSE STRIPPED 2360B of 3426B chunklist_0_video_..._llhls.m3u8 PARSE STRIPPED 3002B of 4068B chunklist_2_audio_..._llhls.m3u8 26 reloads of each rendition over 50s of continuous playback, ~70% of every playlist removed, and zero DataSpec.subrange occurrences — the crash that androidx/media#3350 tracks. Both branches confirmed. Eight ordinary HLS streams logged "no LL tags" and took the identity path untouched, so the stripper is inert on normal playlists. Routing was correct on ~15 distinct real URLs: `hls=true -> HlsMediaSource` for application/x-mpegURL, and `hls=false -> ProgressiveMediaSource` for video/mp4 — which is the isHlsMediaItem coverage that unit tests cannot provide. The 18 ERROR states in the same capture were all unrelated: dead URLs (403/404/502/504), four UnknownHostException from a network drop, and two UnrecognizedInputFormatException from a youtu.be link on the progressive path, which ExoPlayer has never been able to play directly. The pure predicates keep their unit tests, which are the durable guard. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018uE2Db4pzmhi5cFKKkyq2m --- .../amethyst/service/playback/HlsVerifyLog.kt | 44 ------------------- .../playerPool/CustomMediaSourceFactory.kt | 6 --- .../playerPool/HlsLivenessRecorder.kt | 3 -- .../playerPool/LowLatencyHlsStripper.kt | 16 +------ 4 files changed, 1 insertion(+), 68 deletions(-) delete mode 100644 amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/HlsVerifyLog.kt diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/HlsVerifyLog.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/HlsVerifyLog.kt deleted file mode 100644 index 9f89e66b1c..0000000000 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/HlsVerifyLog.kt +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright (c) 2025 Vitor Pamplona - * - * Permission is hereby granted, free of charge, to any person obtaining a copy of - * this software and associated documentation files (the "Software"), to deal in - * the Software without restriction, including without limitation the rights to use, - * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the - * Software, and to permit persons to whom the Software is furnished to do so, - * subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all - * copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS - * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR - * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN - * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION - * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - */ -package com.vitorpamplona.amethyst.service.playback - -/** - * TEMPORARY — delete this file and its three call sites before merging. - * `grep -rn HlsVerify` finds all of them; reverting the commit that added it does the same job. - * - * [PLAYBACK_DIAG_TAG] logs at DEBUG, and `Amethyst.onCreate` sets - * `Log.minLevel = if (BuildConfig.DEBUG) DEBUG else ERROR` — so nothing on that tag survives in a - * benchmark or release build. On-device verification of the HLS work therefore has to log at ERROR - * to be visible at all. - * - * Deliberately placed only on the paths that unit tests *cannot* reach: - * - `isHlsMediaItem` and the factory choice — needs `android.net.Uri`, which stubs to null under - * `unitTests.isReturnDefaultValues`. - * - `LowLatencyStrippingParser.parse` — needs a `Uri` for the same reason. - * - `HlsLivenessRecorder.maybeRecord` — needs a `Player`. - * - * The pure predicates underneath (`stripLowLatencyTags`, `isHlsMedia`, `shouldBypassCache`, - * `livenessVerdictToRecord`) already have unit tests, which are the durable regression guard. This - * tag is only for confirming the wiring on a real device. - * - * Capture with: `adb logcat -s HlsVerify:E` - */ -const val HLS_VERIFY_TAG = "HlsVerify" diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/CustomMediaSourceFactory.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/CustomMediaSourceFactory.kt index bfabbd885d..d9ec09d6e9 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/CustomMediaSourceFactory.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/CustomMediaSourceFactory.kt @@ -30,7 +30,6 @@ import androidx.media3.exoplayer.hls.HlsMediaSource import androidx.media3.exoplayer.source.DefaultMediaSourceFactory import androidx.media3.exoplayer.source.MediaSource import androidx.media3.exoplayer.upstream.LoadErrorHandlingPolicy -import com.vitorpamplona.amethyst.service.playback.HLS_VERIFY_TAG import com.vitorpamplona.amethyst.service.playback.PLAYBACK_DIAG_TAG import com.vitorpamplona.amethyst.service.playback.composable.mediaitem.MediaItemCache import com.vitorpamplona.amethyst.service.playback.diskCache.HlsLivenessCache @@ -166,11 +165,6 @@ class CustomMediaSourceFactory( // Logs the routing inputs directly rather than a re-derived label, so it can't drift from // shouldBypassCache. - // TEMPORARY HlsVerify — see HlsVerifyLog.kt. Remove before merge. - Log.e(HLS_VERIFY_TAG) { - "ROUTE hls=$hls bypassCache=$bypassCache flaggedLive=$flaggedLive knownOnDemand=$knownOnDemand " + - "mime=${mediaItem.localConfiguration?.mimeType} -> ${source::class.java.simpleName} id=$id" - } Log.d(PLAYBACK_DIAG_TAG) { "SOURCE ${if (bypassCache) "BYPASS" else "CACHE"} flaggedLive=$flaggedLive hls=$hls knownOnDemand=$knownOnDemand " + "mime=${mediaItem.localConfiguration?.mimeType} -> ${source::class.java.simpleName} id=$id" diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/HlsLivenessRecorder.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/HlsLivenessRecorder.kt index 7ff88465db..9b95564fed 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/HlsLivenessRecorder.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/HlsLivenessRecorder.kt @@ -23,7 +23,6 @@ package com.vitorpamplona.amethyst.service.playback.playerPool import androidx.media3.common.C import androidx.media3.common.Player import androidx.media3.common.Timeline -import com.vitorpamplona.amethyst.service.playback.HLS_VERIFY_TAG import com.vitorpamplona.amethyst.service.playback.PLAYBACK_DIAG_TAG import com.vitorpamplona.amethyst.service.playback.diskCache.HlsLivenessCache import com.vitorpamplona.quartz.utils.Log @@ -106,8 +105,6 @@ class HlsLivenessRecorder( // refresh of a live stream, and the verdict is stable once learned, so re-putting the same // value would take a ConcurrentHashMap bin lock on every callback for nothing. if (known != toRecord) { - // TEMPORARY HlsVerify — see HlsVerifyLog.kt. Remove before merge. - Log.e(HLS_VERIFY_TAG) { "LIVENESS ${if (toRecord) "LIVE" else "ON-DEMAND"} learned for $url" } Log.d(PLAYBACK_DIAG_TAG) { "LIVENESS ${if (toRecord) "LIVE" else "ON-DEMAND"} learned for $url" } HlsLivenessCache.record(url, toRecord) } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripper.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripper.kt index 50d61e1af1..45ee84bc05 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripper.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/service/playback/playerPool/LowLatencyHlsStripper.kt @@ -28,8 +28,6 @@ import androidx.media3.exoplayer.hls.playlist.HlsMultivariantPlaylist import androidx.media3.exoplayer.hls.playlist.HlsPlaylist import androidx.media3.exoplayer.hls.playlist.HlsPlaylistParserFactory import androidx.media3.exoplayer.upstream.ParsingLoadable -import com.vitorpamplona.amethyst.service.playback.HLS_VERIFY_TAG -import com.vitorpamplona.quartz.utils.Log import java.io.ByteArrayInputStream import java.io.InputStream @@ -106,19 +104,7 @@ internal class LowLatencyStrippingParser( override fun parse( uri: Uri, inputStream: InputStream, - ): HlsPlaylist { - val original = inputStream.readBytes() - val stripped = stripLowLatencyTags(original) - // TEMPORARY HlsVerify — see HlsVerifyLog.kt. Remove before merge. - Log.e(HLS_VERIFY_TAG) { - if (stripped === original) { - "PARSE no LL tags (${original.size}B) $uri" - } else { - "PARSE STRIPPED ${original.size - stripped.size}B of ${original.size}B $uri" - } - } - return delegate.parse(uri, ByteArrayInputStream(stripped)) - } + ): HlsPlaylist = delegate.parse(uri, ByteArrayInputStream(stripLowLatencyTags(inputStream.readBytes()))) } /**