From c26988f39969fc9d6255b6bd1320823ef20d0822 Mon Sep 17 00:00:00 2001 From: davotoula Date: Wed, 29 Jul 2026 13:56:15 +0200 Subject: [PATCH] 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()))) } /**