diff --git a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/AudioOutputCachePolicy.kt b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/AudioOutputCachePolicy.kt new file mode 100644 index 00000000..dd834531 --- /dev/null +++ b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/AudioOutputCachePolicy.kt @@ -0,0 +1,36 @@ +package com.edde746.plezy.exoplayer + +/** + * Decides which audio outputs `RawPositionOutputProvider` may park in its reuse cache instead of + * releasing on a sink flush. + * + * The cache exists so a seek does not tear down and rebuild the hardware audio pipeline, which + * costs 7-10s of silence on Android TV boxes that reinitialize tunneled output slowly (Sony + * Bravia class). That win is real for decoded PCM, but parking a *bitstream* output is a + * different trade: a direct/passthrough `AudioTrack` occupies the platform's direct output for as + * long as it is held, and several HDMI HALs expose only one. A parked AC3 track therefore blocks + * the next AC3 track from being created at all, and the failure surfaces as + * `UnsupportedOperationException: Cannot create AudioTrack` rather than as a slow seek (#1790). + * + * Parking also has to stay honest about media3's process-wide pending-release accounting: a + * cached output is *not* releasing, so `RawPositionAudioOutput` signals `onReleased` immediately + * for it. That is only true because nothing scarce is being held, which is exactly what this + * policy guarantees. + * + * Offload outputs are excluded for the same reason plus their separate gapless/end-of-stream + * state machine, which the cache does not model. + */ +internal object AudioOutputCachePolicy { + + /** + * `AudioTrack.flush()` only reliably resets a stopped track from N MR1 onwards; below that the + * cache would hand back a track carrying the previous playback's state. + */ + const val MIN_SDK_INT = 25 + + fun mayCache(encoding: Int, isOffload: Boolean, sdkInt: Int): Boolean { + if (sdkInt < MIN_SDK_INT) return false + if (isOffload) return false + return isPcmEncoding(encoding) + } +} diff --git a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/AudioOutputPolicy.kt b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/AudioOutputPolicy.kt index 2154ac7b..32f571b3 100644 --- a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/AudioOutputPolicy.kt +++ b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/AudioOutputPolicy.kt @@ -1,6 +1,7 @@ package com.edde746.plezy.exoplayer import android.content.Context +import android.media.AudioFormat import android.util.Log import androidx.annotation.OptIn import androidx.media3.common.AudioAttributes @@ -25,6 +26,19 @@ internal fun isPassthroughAudioMimeType(mimeType: String): Boolean = when (mimeT internal fun shouldBlockDirectOutputForPassthrough(mimeType: String, audioPassthroughEnabled: Boolean): Boolean = !audioPassthroughEnabled && isPassthroughAudioMimeType(mimeType) +/** + * Linear PCM output encodings, i.e. the sink decoded the bitstream instead of + * passing it through. Mirrors the platform's `AudioFormat.ENCODING_PCM_*` set. + */ +internal fun isPcmEncoding(encoding: Int): Boolean = when (encoding) { + AudioFormat.ENCODING_PCM_8BIT, + AudioFormat.ENCODING_PCM_16BIT, + AudioFormat.ENCODING_PCM_FLOAT, + AudioFormat.ENCODING_PCM_24BIT_PACKED, + AudioFormat.ENCODING_PCM_32BIT -> true + else -> false +} + /** * mpv `audio-spdif` codec names and the exact platform encoding a route must * advertise to carry that bitstream. diff --git a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/BufferingStallPolicy.kt b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/BufferingStallPolicy.kt new file mode 100644 index 00000000..78aa430b --- /dev/null +++ b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/BufferingStallPolicy.kt @@ -0,0 +1,82 @@ +package com.edde746.plezy.exoplayer + +/** + * Decision logic for the buffering stall watchdog (#1790). + * + * A renderer that can never become ready pins ExoPlayer in `STATE_BUFFERING` with a full buffer + * and a frozen clock. The reported case is an `AudioTrack` that fails to initialize: media3 logs a + * sink error, keeps retrying, and — when its process-wide pending-release accounting has been + * knocked out of balance — never converts the failure into a `PlaybackException`. Nothing else + * notices. The frame watchdog wants `STATE_READY` and zero rendered frames, the decoder-hang check + * is cancelled by the first frame, [ResumeStallPolicy] needs `isPlaying` and deliberately treats a + * frozen clock as "not a decoder stall", [EndOfStreamPolicy] needs the position past the duration, + * and media3's own stuck-buffering detector needs an *empty* buffer. So the user sits on a spinner + * with no error and no way out. + * + * This watchdog covers exactly that hole: buffering, intending to play, holding data, and not + * moving. A starved buffer is left alone — that is an ordinary network stall and the loader is the + * right thing to wait for. + */ +internal object BufferingStallPolicy { + + /** Poll interval while buffering. */ + const val CHECK_INTERVAL_MS = 2_000L + + /** + * How long a buffering player may hold enough data to start and still not move before it is + * called stalled. Long enough to sit out a slow decoder handover or a track reselection, short + * enough that a user has not yet given up and force-quit. + * + * Only time spent above [MIN_BUFFER_AHEAD_MS] counts: a rebuffer that takes a minute to refill + * is the loader doing its job, and the clock restarts once it is done. + */ + const val STALL_TIMEOUT_MS = 12_000L + + /** Position movement that counts as progress rather than clock jitter. */ + const val PROGRESS_EPSILON_MS = 250L + + /** + * Media buffered ahead of the position before a motionless player is anomalous at all. + * + * This has to clear `DefaultLoadControl`'s own play-start bar with room to spare, or the + * watchdog would indict the player for obeying it: below + * [LoadControlPolicy.BUFFER_FOR_PLAYBACK_AFTER_REBUFFER_MS] the load control deliberately keeps + * playback in `STATE_BUFFERING`, and the two evaluate on independent schedules. + */ + const val MIN_BUFFER_AHEAD_MS = LoadControlPolicy.BUFFER_FOR_PLAYBACK_AFTER_REBUFFER_MS + 2_000L + + enum class Verdict { + /** The position moved; re-baseline and keep watching. */ + HEALTHY, + + /** Enough data to start, not moving, but not for long enough yet. */ + WAITING, + + /** Not moving because there is not yet enough to play. An ordinary rebuffer; not ours. */ + STARVED, + + /** Enough data to start, playback is wanted, and nothing has moved. */ + STALLED + } + + fun evaluate( + elapsedMs: Long, + baselinePositionMs: Long, + currentPositionMs: Long, + bufferedPositionMs: Long + ): Verdict = when { + currentPositionMs - baselinePositionMs >= PROGRESS_EPSILON_MS -> Verdict.HEALTHY + bufferedPositionMs - currentPositionMs < MIN_BUFFER_AHEAD_MS -> Verdict.STARVED + elapsedMs < STALL_TIMEOUT_MS -> Verdict.WAITING + else -> Verdict.STALLED + } + + /** + * Whether [verdict] means the elapsed stall clock was measuring something the watchdog is not + * responsible for, and must restart. + * + * Without this a long rebuffer accrues the whole timeout while starved, and the first poll after + * the buffer refills reports a stall that never happened. + */ + fun resetsStallClock(verdict: Verdict): Boolean = verdict == Verdict.HEALTHY || verdict == Verdict.STARVED +} diff --git a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/ExoPlayerCore.kt b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/ExoPlayerCore.kt index bea1d10f..7f328d52 100644 --- a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/ExoPlayerCore.kt +++ b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/ExoPlayerCore.kt @@ -275,6 +275,14 @@ class ExoPlayerCore(private val activity: Activity) : private var resumeStallRecoveryCount = 0 private var loggedResumeStallCap = false + // Buffering stall watchdog (#1790): a renderer that never becomes ready pins the player in + // STATE_BUFFERING with a full buffer and a frozen clock, and — when media3 absorbs the + // underlying failure instead of raising it — nothing else ever notices. Armed on every + // transition into buffering; see BufferingStallPolicy. + private var bufferingStallRunnable: Runnable? = null + private var bufferingStallSinceMs = 0L + private var bufferingStallBaselinePositionMs = 0L + // Decoder hang detection: tracks gap between decoder init and first rendered frame private var decoderHangRunnable: Runnable? = null private var decoderInitName: String? = null @@ -323,8 +331,17 @@ class ExoPlayerCore(private val activity: Activity) : // Track state for event emission private var lastPosition: Long = 0 - /** Position to use for fallback: max of current position and pending start position. */ - private val effectivePosition: Long get() = maxOf(lastPosition, pendingStartPositionMs) + /** + * Highest position actually reached in the current media generation, or the last explicit seek + * target. [lastPosition] tracks the emitted timeline and follows the player down as well as up, + * so a renderer that reports 0 while its clock is dead would otherwise hand recovery a 0ms + * resume point — which is how an audio recovery restarted a resumed episode from the top + * (#1790). Recovery reads this instead. + */ + private var lastKnownGoodPositionMs: Long = 0L + + /** Position to use for fallback: the furthest of the tracked positions. */ + private val effectivePosition: Long get() = maxOf(lastPosition, pendingStartPositionMs, lastKnownGoodPositionMs) private var lastDuration: Long = 0 private var lastBufferedPosition: Long = 0 private var positionUpdateRunnable: Runnable? = null @@ -752,9 +769,19 @@ class ExoPlayerCore(private val activity: Activity) : setTargetBufferBytes(targetBufferBytes) setPrioritizeTimeOverSizeThresholds(false) if (availableMB <= 2048) { - setBufferDurationsMs(15_000, 50_000, 1_000, 5_000) + setBufferDurationsMs( + 15_000, + 50_000, + LoadControlPolicy.BUFFER_FOR_PLAYBACK_MS, + LoadControlPolicy.BUFFER_FOR_PLAYBACK_AFTER_REBUFFER_MS + ) } else { - setBufferDurationsMs(30_000, 60_000, 1_000, 5_000) + setBufferDurationsMs( + 30_000, + 60_000, + LoadControlPolicy.BUFFER_FOR_PLAYBACK_MS, + LoadControlPolicy.BUFFER_FOR_PLAYBACK_AFTER_REBUFFER_MS + ) } }.build() emitLog( @@ -963,6 +990,7 @@ class ExoPlayerCore(private val activity: Activity) : // classifies an EOF by position against duration. if (currentPosition != lastPosition && terminalErrorGeneration != currentMediaGeneration) { lastPosition = currentPosition + if (currentPosition > lastKnownGoodPositionMs) lastKnownGoodPositionMs = currentPosition delegate?.onPropertyChange("time-pos", currentPosition / 1000.0) } @@ -1004,6 +1032,7 @@ class ExoPlayerCore(private val activity: Activity) : private fun resetPlaybackProgress(startPositionMs: Long) { lastPosition = startPositionMs + lastKnownGoodPositionMs = startPositionMs lastDuration = 0L lastBufferedPosition = 0L // Dart already seeds the visible timeline before open. Emitting native @@ -1083,8 +1112,10 @@ class ExoPlayerCore(private val activity: Activity) : when (state) { Player.STATE_BUFFERING -> { delegate?.onPropertyChange("paused-for-cache", true) + armBufferingStallWatchdog() } Player.STATE_READY -> { + cancelBufferingStallWatchdog() // Restore start position if it was lost during track reselection // (e.g. tunneling state change in onTracksChanged triggers renderer teardown) if (pendingStartPositionMs > 0L) { @@ -1120,6 +1151,7 @@ class ExoPlayerCore(private val activity: Activity) : } Player.STATE_ENDED -> { stopFrameWatchdog() + cancelBufferingStallWatchdog() delegate?.onPropertyChange("eof-reached", true) delegate?.onEvent("end-file", mapOf("reason" to "eof")) } @@ -1206,6 +1238,7 @@ class ExoPlayerCore(private val activity: Activity) : stopFrameWatchdog() cancelDecoderHangCheck() cancelResumeStallWatchdog() + cancelBufferingStallWatchdog() emitSeekable(false, force = true) // If native DV7 failed, retry with conversion before falling to MPV @@ -1346,7 +1379,7 @@ class ExoPlayerCore(private val activity: Activity) : val player = exoPlayer ?: return false val uri = currentMediaUri ?: return false - val savedPosition = maxOf(player.currentPosition, lastPosition, pendingStartPositionMs) + val savedPosition = maxOf(player.currentPosition, effectivePosition) val savedPlayWhenReady = player.playWhenReady val previousDecoder = decoderInitName pendingTrackRestore = pendingTrackRestore ?: captureTrackRestore()?.also { restore -> @@ -1368,6 +1401,7 @@ class ExoPlayerCore(private val activity: Activity) : stopFrameWatchdog() cancelDecoderHangCheck() cancelResumeStallWatchdog() + cancelBufferingStallWatchdog() applyTrackSelectorPolicy( reason = "video decoder recovery", forceSelector = true, @@ -1413,33 +1447,51 @@ class ExoPlayerCore(private val activity: Activity) : private fun retryAfterAudioTrackError(error: PlaybackException, causeChain: String): Boolean { if (!isAudioTrackError(error.errorCode)) return false + val errorFormat = (error as? ExoPlaybackException)?.rendererFormat?.takeIf { format -> + format.sampleMimeType?.startsWith("audio/") == true + } + return recoverAudioOutputInPlace( + label = PlaybackException.getErrorCodeName(error.errorCode), + reason = "${PlaybackException.getErrorCodeName(error.errorCode)}: ${error.message ?: causeChain.ifEmpty { "unknown" }}", + fallbackFormat = errorFormat, + blockDirectOutput = true + ) + } + /** + * Re-prepares the current media with the audio output forced onto a path that has not just + * failed. Shared by the [PlaybackException] route and by the buffering stall watchdog, which + * reaches the same failure without an exception ever being raised (#1790). + */ + private fun recoverAudioOutputInPlace( + label: String, + reason: String, + fallbackFormat: Format?, + blockDirectOutput: Boolean + ): Boolean { val player = exoPlayer ?: return false val uri = currentMediaUri ?: return false if (audioRecoveryAttempts >= MAX_AUDIO_RECOVERY_ATTEMPTS) { emitLog( "warn", "audio-recovery", - "ExoPlayer audio recovery exhausted after $audioRecoveryAttempts attempts for ${PlaybackException.getErrorCodeName(error.errorCode)}" + "ExoPlayer audio recovery exhausted after $audioRecoveryAttempts attempts for $label" ) return false } - val selectedFormat = selectedAudioFormat() - val errorFormat = (error as? ExoPlaybackException)?.rendererFormat?.takeIf { format -> - format.sampleMimeType?.startsWith("audio/") == true - } - val recoveryFormat = selectedFormat ?: errorFormat + val recoveryFormat = selectedAudioFormat() ?: fallbackFormat val actions = mutableListOf() - recoveryFormat?.sampleMimeType - ?.takeIf { isEncodedAudioMimeType(it) } - ?.let { mimeType -> - if (directAudioOutputBlockedAfterFailure.add(mimeType)) { - actions.add("force-decoded-pcm($mimeType)") + if (blockDirectOutput) { + recoveryFormat?.sampleMimeType + ?.takeIf { isEncodedAudioMimeType(it) } + ?.let { mimeType -> + if (directAudioOutputBlockedAfterFailure.add(mimeType)) { + actions.add("force-decoded-pcm($mimeType)") + } } - } - + } if (!tunnelingDisabledForAudioRecovery) { tunnelingDisabledForAudioRecovery = true actions.add("disable-tunneling") @@ -1447,7 +1499,7 @@ class ExoPlayerCore(private val activity: Activity) : if (actions.isEmpty()) actions.add("reload") audioRecoveryAttempts++ - val savedPosition = maxOf(player.currentPosition, lastPosition, pendingStartPositionMs) + val savedPosition = maxOf(player.currentPosition, effectivePosition) val savedPlayWhenReady = player.playWhenReady val previousAudioTrackConfig = lastAudioTrackConfig pendingStartPositionMs = savedPosition @@ -1455,11 +1507,12 @@ class ExoPlayerCore(private val activity: Activity) : audioDecoderInitName = null lastAudioTrackConfig = null lastAudioRecoveryAction = actions.joinToString(",") - lastAudioRecoveryReason = "${PlaybackException.getErrorCodeName(error.errorCode)}: ${error.message ?: causeChain.ifEmpty { "unknown" }}" + lastAudioRecoveryReason = reason stopFrameWatchdog() cancelDecoderHangCheck() cancelResumeStallWatchdog() + cancelBufferingStallWatchdog() applyTrackSelectorPolicy(reason = "audio recovery", forceSelector = true) emitLog( @@ -2632,15 +2685,6 @@ class ExoPlayerCore(private val activity: Activity) : return selectedAudioGroup.mediaTrackGroup.getFormat(0) } - private fun isPcmEncoding(encoding: Int): Boolean = when (encoding) { - AudioFormat.ENCODING_PCM_8BIT, - AudioFormat.ENCODING_PCM_16BIT, - AudioFormat.ENCODING_PCM_FLOAT, - AudioFormat.ENCODING_PCM_24BIT_PACKED, - AudioFormat.ENCODING_PCM_32BIT -> true - else -> false - } - private fun formatAudioSummary(format: Format): String { val parts = mutableListOf() parts.add("mime=${format.sampleMimeType ?: "unknown"}") @@ -3124,6 +3168,99 @@ class ExoPlayerCore(private val activity: Activity) : } } + // Buffering stall watchdog (#1790) — see BufferingStallPolicy for the coverage argument. + // Main-thread only, like the watchdogs above. + + private fun armBufferingStallWatchdog() { + cancelBufferingStallWatchdog() + val player = exoPlayer ?: return + bufferingStallSinceMs = System.currentTimeMillis() + bufferingStallBaselinePositionMs = player.currentPosition + val mediaGeneration = currentMediaGeneration + bufferingStallRunnable = object : Runnable { + override fun run() { + if (mediaGeneration != currentMediaGeneration) return + if (disposing || !isInitialized) return + val current = exoPlayer ?: return + if (current.playbackState != Player.STATE_BUFFERING) { + cancelBufferingStallWatchdog() + return + } + + val now = System.currentTimeMillis() + // Paused mid-buffer: nothing is meant to progress, so the clock does not run. + if (!current.playWhenReady) { + bufferingStallSinceMs = now + bufferingStallBaselinePositionMs = current.currentPosition + handler.postDelayed(this, BufferingStallPolicy.CHECK_INTERVAL_MS) + return + } + + val elapsedMs = now - bufferingStallSinceMs + val verdict = BufferingStallPolicy.evaluate( + elapsedMs = elapsedMs, + baselinePositionMs = bufferingStallBaselinePositionMs, + currentPositionMs = current.currentPosition, + bufferedPositionMs = current.bufferedPosition + ) + if (verdict == BufferingStallPolicy.Verdict.STALLED) { + cancelBufferingStallWatchdog() + recoverFromBufferingStall(current, mediaGeneration, elapsedMs) + return + } + // Moving again, or still short of the load control's play-start threshold: either way the + // clock was not measuring a stall this watchdog owns, so it restarts. + if (BufferingStallPolicy.resetsStallClock(verdict)) { + bufferingStallSinceMs = now + bufferingStallBaselinePositionMs = current.currentPosition + } + handler.postDelayed(this, BufferingStallPolicy.CHECK_INTERVAL_MS) + } + } + handler.postDelayed(bufferingStallRunnable!!, BufferingStallPolicy.CHECK_INTERVAL_MS) + } + + private fun cancelBufferingStallWatchdog() { + bufferingStallRunnable?.let { handler.removeCallbacks(it) } + bufferingStallRunnable = null + } + + private fun recoverFromBufferingStall(player: ExoPlayer, mediaGeneration: Int, stalledMs: Long) { + val positionMs = player.currentPosition + val playWhenReady = player.playWhenReady + val sinkError = lastAudioSinkError + emitLog( + "warn", + "buffering-stall", + "No progress for ${stalledMs}ms at ${positionMs}ms while buffering " + + "(buffered=${player.bufferedPosition}ms, audio=${selectedAudioFormat()?.let { formatAudioSummary(it) } ?: "unknown"}, " + + "tunneling=$currentTunneledPlayback, lastSinkError=${sinkError ?: "none"})" + ) + + // A sink error with no exception behind it is the shape media3 absorbs, so take the audio + // path off bitstream. Without one, the reload alone is the unstick. + if (recoverAudioOutputInPlace( + label = "buffering stall", + reason = "buffering stall after ${stalledMs}ms: ${sinkError ?: "no sink error reported"}", + fallbackFormat = null, + blockDirectOutput = sinkError != null + ) + ) { + return + } + + // In-place recovery is spent. Hand over to the MPV backend, which has its own audio path, + // rather than leaving the user on a spinner. + val uri = currentMediaUri ?: return + requestFormatFallback( + mediaGeneration = mediaGeneration, + uri = uri, + positionMs = positionMs, + playWhenReady = playWhenReady, + errorMessage = "Playback stalled while buffering for ${stalledMs}ms" + ) + } + // Public API fun open( @@ -3140,6 +3277,7 @@ class ExoPlayerCore(private val activity: Activity) : stopFrameWatchdog() cancelDecoderHangCheck() cancelResumeStallWatchdog() + cancelBufferingStallWatchdog() resumeStallRecoveryCount = 0 loggedResumeStallCap = false @@ -3451,7 +3589,7 @@ class ExoPlayerCore(private val activity: Activity) : val uri = currentMediaUri ?: return false if (currentMediaIsLive) return false - val savedPosition = maxOf(player.currentPosition, lastPosition, pendingStartPositionMs) + val savedPosition = maxOf(player.currentPosition, effectivePosition) val savedPlayWhenReady = player.playWhenReady pendingTrackRestore = captureTrackRestore()?.also { restore -> emitLog( @@ -3479,6 +3617,7 @@ class ExoPlayerCore(private val activity: Activity) : stopFrameWatchdog() cancelDecoderHangCheck() cancelResumeStallWatchdog() + cancelBufferingStallWatchdog() applyTrackSelectorPolicy( reason = "DV reload", @@ -3508,6 +3647,7 @@ class ExoPlayerCore(private val activity: Activity) : stopFrameWatchdog() cancelDecoderHangCheck() cancelResumeStallWatchdog() + cancelBufferingStallWatchdog() exoPlayer?.stop() emitSeekable(false, force = true) setVisible(false) @@ -3527,6 +3667,7 @@ class ExoPlayerCore(private val activity: Activity) : pendingStartPositionMs = 0L player.seekTo(clampedPositionMs) lastPosition = clampedPositionMs + lastKnownGoodPositionMs = clampedPositionMs delegate?.onPropertyChange("time-pos", clampedPositionMs / 1000.0) } @@ -4012,6 +4153,7 @@ class ExoPlayerCore(private val activity: Activity) : stopFrameWatchdog() cancelDecoderHangCheck() cancelResumeStallWatchdog() + cancelBufferingStallWatchdog() stopPositionUpdates() handler.removeCallbacksAndMessages(null) // releasePending (not clearVideoFrameRate): on the ExoPlayer→MPV fallback diff --git a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/ExoPlayerPlugin.kt b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/ExoPlayerPlugin.kt index cd14fb6c..47777a34 100644 --- a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/ExoPlayerPlugin.kt +++ b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/ExoPlayerPlugin.kt @@ -319,6 +319,14 @@ class ExoPlayerPlugin : fallbackInProgress = false } + // An initialize without an intervening dispose would otherwise orphan the previous core + // along with its ExoPlayer, audio sink, codecs and surface views — nothing else holds a + // reference, so they would never be released. + playerCore?.let { stale -> + playerCore = null + stale.dispose() + } + try { val core = ExoPlayerCore(currentActivity).apply { delegate = this@ExoPlayerPlugin diff --git a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/LoadControlPolicy.kt b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/LoadControlPolicy.kt index d4fb68c6..9b76c503 100644 --- a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/LoadControlPolicy.kt +++ b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/LoadControlPolicy.kt @@ -31,6 +31,16 @@ internal object LoadControlPolicy { */ const val MIN_TARGET_BYTES = 32 * MIB + /** `DefaultLoadControl` play-start threshold on a first buffer. */ + const val BUFFER_FOR_PLAYBACK_MS = 1_000 + + /** + * `DefaultLoadControl` play-start threshold after a rebuffer. Below this the player is + * *supposed* to stay in `STATE_BUFFERING`, so anything judging a buffering player has to clear + * this bar before it can call the wait anomalous — see [BufferingStallPolicy]. + */ + const val BUFFER_FOR_PLAYBACK_AFTER_REBUFFER_MS = 5_000 + /** * Fraction of a memory budget the allocator may claim. Matches the threshold the Buffer * Size setting already warns at (`value > heapMB / 4`). diff --git a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/PlezyRenderersFactory.kt b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/PlezyRenderersFactory.kt index 0c08ca11..a40a6457 100644 --- a/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/PlezyRenderersFactory.kt +++ b/android/app/src/main/kotlin/com/edde746/plezy/exoplayer/PlezyRenderersFactory.kt @@ -161,6 +161,11 @@ class PlezyRenderersFactory(context: Context) : DefaultRenderersFactory(context) .setMinPcmBufferDurationUs(500_000) .setMaxPcmBufferDurationUs(1_000_000) .setPcmBufferMultiplicationFactor(4) + // media3 defaults passthrough to 250ms, which the AC3 factor doubles to 500ms — 40000 + // bytes at 640 kbps. Some HDMI routes reject a buffer that short outright and the only + // retry media3 1.10.1 has is to keep halving it (#1790). Ask for a second up front; + // upstream adopted the same 1s floor as its last-resort retry in #3207. + .setPassthroughBufferDurationUs(500_000) .build() val realProvider = AudioTrackAudioOutputProvider.Builder(context) @@ -514,17 +519,46 @@ internal class DvSanitizingVideoRenderer( // AudioTrack and creates a new one. On Android TV with tunneled playback, this causes // 7-10s audio dropout while the hardware pipeline reinitializes (Sony Bravia, etc). // By flushing instead of releasing and caching the output, we skip the teardown cycle. +// +// Two invariants keep that safe (#1790): +// +// 1. Only outputs [AudioOutputCachePolicy] admits are parked, so the cache never holds a +// scarce direct/passthrough route hostage while the next one is being created. +// 2. Every flush is answered by exactly one onReleased, promptly. +// +// DefaultAudioSink increments a *static, process-wide* counter on every flush and decrements it +// only from `Listener::onReleased`, which media3 delivers by posting to the playback looper — a +// looper that is already dead by the time a player-release actually completes. A dropped callback +// pins the counter above zero, and any nonzero value disables media3's escalation of *both* init +// and write failures: `PendingExceptionHolder` refuses to arm its throw deadline and every retry +// short-circuits, so a sink error never becomes a PlaybackException and playback hangs in +// STATE_BUFFERING with no recovery. The wrapper therefore owns the listener set and answers the +// flush itself when it parks the track, because a parked track is never going to release. +// +// Deferring that answer until the parked track is really evicted is tempting — it would let +// media3 stay patient through the eviction, whose replacement is built while the old AudioTrack +// is still going away — but it holds the counter above zero for the whole live track after the +// first seek, which is the very hang above. So the eviction overlap is tolerated instead, as +// upstream tolerates it: if the replacement does fail to allocate, media3 escalates on its own +// 200ms deadline into the audio recovery ladder, and the buffering stall watchdog backs that up. +// +// Everything below is confined to the ExoPlayer playback thread: sink flush/release, provider +// lookups, and media3's own onReleased delivery all run there. @OptIn(UnstableApi::class) -private class RawPositionOutputProvider( +internal class RawPositionOutputProvider( private val delegate: AudioOutputProvider, private val rawPositionUs: AtomicLong, - private val log: ((String, String, String) -> Unit)? + private val log: ((String, String, String) -> Unit)?, + private val sdkInt: Int = Build.VERSION.SDK_INT ) : AudioOutputProvider { private var cachedOutput: RawPositionAudioOutput? = null private var cachedConfig: AudioOutputProvider.OutputConfig? = null + /** Outputs whose real release was started but whose completion has not been observed yet. */ + private val unsettledReleases = LinkedHashSet() + override fun getFormatSupport(config: AudioOutputProvider.FormatConfig) = delegate.getFormatSupport(config) override fun getOutputConfig(config: AudioOutputProvider.FormatConfig) = delegate.getOutputConfig(config) @@ -533,14 +567,24 @@ private class RawPositionOutputProvider( val cached = cachedOutput if (cached != null && cachedConfig == config) { cachedOutput = null + cached.markReacquired() return cached } cached?.forceRelease() cachedOutput = null + cachedConfig = null + // The replacement is built while the evicted track is still going away. Upstream does the + // same; the alternatives are worse (see the note above this class). val realOutput = delegate.getAudioOutput(config) cachedConfig = config - return RawPositionAudioOutput(realOutput, rawPositionUs, this, log) + return RawPositionAudioOutput( + delegate = realOutput, + rawPositionUs = rawPositionUs, + provider = this, + mayCache = AudioOutputCachePolicy.mayCache(config.encoding, config.isOffload, sdkInt), + log = log + ) } fun returnToCache(output: RawPositionAudioOutput) { @@ -551,6 +595,14 @@ private class RawPositionOutputProvider( cachedOutput = output } + fun onRealReleaseStarted(output: RawPositionAudioOutput) { + unsettledReleases.add(output) + } + + fun onReleaseSettled(output: RawPositionAudioOutput) { + unsettledReleases.remove(output) + } + override fun addListener(listener: AudioOutputProvider.Listener) = delegate.addListener(listener) override fun removeListener(listener: AudioOutputProvider.Listener) = delegate.removeListener(listener) @@ -561,15 +613,21 @@ private class RawPositionOutputProvider( cachedOutput?.forceRelease() cachedOutput = null cachedConfig = null + // Player teardown: media3 schedules the real AudioTrack release with a delay and posts the + // completion back to the playback looper this call is in the middle of quitting, so nothing + // will ever deliver it. Settle here rather than leak the accounting for the whole process. + for (output in unsettledReleases.toList()) output.settleReleaseNow() + unsettledReleases.clear() delegate.release() } } @OptIn(UnstableApi::class) -private class RawPositionAudioOutput( +internal class RawPositionAudioOutput( private val delegate: AudioOutput, private val rawPositionUs: AtomicLong, private val provider: RawPositionOutputProvider, + private val mayCache: Boolean, private val log: ((String, String, String) -> Unit)? ) : AudioOutput { @@ -578,6 +636,46 @@ private class RawPositionAudioOutput( private var writtenBytes = 0L private var failed = false + /** + * DefaultAudioSink registers here instead of on the real output, so the wrapper can report the + * releases media3 will not: a parked output never really releases, and a released output's + * completion callback is posted to a playback looper that may already be gone. The set is + * cleared on every release report and repopulated by the sink on the next acquisition, which + * also stops one stale listener per reuse from piling up on the real output. + */ + private val listeners = mutableListOf() + private var releaseSignalled = false + + private val currentListener: AudioOutput.Listener? + get() = listeners.lastOrNull() + + private val forwarder = object : AudioOutput.Listener { + override fun onPositionAdvancing(playoutStartSystemTimeMs: Long) { + currentListener?.onPositionAdvancing(playoutStartSystemTimeMs) + } + + override fun onOffloadDataRequest() { + currentListener?.onOffloadDataRequest() + } + + override fun onOffloadPresentationEnded() { + currentListener?.onOffloadPresentationEnded() + } + + override fun onUnderrun() { + currentListener?.onUnderrun() + } + + override fun onReleased() { + provider.onReleaseSettled(this@RawPositionAudioOutput) + signalReleased() + } + } + + init { + delegate.addListener(forwarder) + } + override fun getPositionUs(): Long { val pos = delegate.getPositionUs() rawPositionUs.set(pos) @@ -626,24 +724,50 @@ private class RawPositionAudioOutput( override fun release() { rawPositionUs.set(Long.MIN_VALUE) - if (failed) { - delegate.release() - return - } - if (Build.VERSION.SDK_INT >= 25) { + if (!failed && mayCache) { delegate.stop() delegate.flush() provider.returnToCache(this) - } else { - delegate.release() + // A parked track is never going to release, so answer the flush now. Holding the sink's + // pending-release count open for it would disable media3's escalation of every later sink + // error, for as long as the track stays parked or live — the #1790 hang, re-armed by an + // ordinary seek. + signalReleased() + return } + startRealRelease() } + /** Releases for real even when the output would otherwise be cacheable. */ fun forceRelease() { rawPositionUs.set(Long.MIN_VALUE) + startRealRelease() + } + + /** Reports a release whose completion callback can no longer be delivered. */ + fun settleReleaseNow() { + provider.onReleaseSettled(this) + signalReleased() + } + + /** Re-arms the wrapper for a fresh acquisition out of the provider's cache. */ + fun markReacquired() { + releaseSignalled = false + } + + private fun startRealRelease() { + provider.onRealReleaseStarted(this) delegate.release() } + private fun signalReleased() { + if (releaseSignalled) return + releaseSignalled = true + val notified = listeners.toList() + listeners.clear() + for (listener in notified) listener.onReleased() + } + override fun setVolume(volume: Float) = delegate.setVolume(volume) override fun isOffloadedPlayback() = delegate.isOffloadedPlayback() override fun getAudioSessionId() = delegate.getAudioSessionId() @@ -651,8 +775,12 @@ private class RawPositionAudioOutput( override fun getBufferSizeInFrames() = delegate.getBufferSizeInFrames() override fun getPlaybackParameters() = delegate.getPlaybackParameters() override fun isStalled() = delegate.isStalled() - override fun addListener(listener: AudioOutput.Listener) = delegate.addListener(listener) - override fun removeListener(listener: AudioOutput.Listener) = delegate.removeListener(listener) + override fun addListener(listener: AudioOutput.Listener) { + listeners.add(listener) + } + override fun removeListener(listener: AudioOutput.Listener) { + listeners.remove(listener) + } override fun setPlaybackParameters(playbackParameters: PlaybackParameters) = delegate.setPlaybackParameters(playbackParameters) override fun setOffloadDelayPadding(delayInFrames: Int, paddingInFrames: Int) = delegate.setOffloadDelayPadding(delayInFrames, paddingInFrames) override fun setOffloadEndOfStream() = delegate.setOffloadEndOfStream() diff --git a/android/app/src/test/kotlin/com/edde746/plezy/exoplayer/AudioOutputCachePolicyTest.kt b/android/app/src/test/kotlin/com/edde746/plezy/exoplayer/AudioOutputCachePolicyTest.kt new file mode 100644 index 00000000..66ab82c8 --- /dev/null +++ b/android/app/src/test/kotlin/com/edde746/plezy/exoplayer/AudioOutputCachePolicyTest.kt @@ -0,0 +1,85 @@ +package com.edde746.plezy.exoplayer + +import android.media.AudioFormat +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class AudioOutputCachePolicyTest { + + @Test + fun decodedPcmIsCacheable() { + assertTrue( + AudioOutputCachePolicy.mayCache( + encoding = AudioFormat.ENCODING_PCM_16BIT, + isOffload = false, + sdkInt = 34 + ) + ) + } + + @Test + fun everyPcmWidthIsCacheable() { + val pcmEncodings = listOf( + AudioFormat.ENCODING_PCM_8BIT, + AudioFormat.ENCODING_PCM_16BIT, + AudioFormat.ENCODING_PCM_24BIT_PACKED, + AudioFormat.ENCODING_PCM_32BIT, + AudioFormat.ENCODING_PCM_FLOAT + ) + for (encoding in pcmEncodings) { + assertTrue( + "expected $encoding to be cacheable", + AudioOutputCachePolicy.mayCache(encoding = encoding, isOffload = false, sdkInt = 34) + ) + } + } + + /** Parking a bitstream track holds the platform's direct output, which is often single-instance. */ + @Test + fun bitstreamOutputIsNeverCacheable() { + val passthroughEncodings = listOf( + AudioFormat.ENCODING_AC3, + AudioFormat.ENCODING_E_AC3, + AudioFormat.ENCODING_E_AC3_JOC, + AudioFormat.ENCODING_DTS, + AudioFormat.ENCODING_DTS_HD, + AudioFormat.ENCODING_DOLBY_TRUEHD + ) + for (encoding in passthroughEncodings) { + assertFalse( + "expected $encoding to be excluded from the cache", + AudioOutputCachePolicy.mayCache(encoding = encoding, isOffload = false, sdkInt = 34) + ) + } + } + + @Test + fun offloadOutputIsNeverCacheable() { + assertFalse( + AudioOutputCachePolicy.mayCache( + encoding = AudioFormat.ENCODING_PCM_16BIT, + isOffload = true, + sdkInt = 34 + ) + ) + } + + @Test + fun cachingIsOffBelowTheFlushableApiLevel() { + assertFalse( + AudioOutputCachePolicy.mayCache( + encoding = AudioFormat.ENCODING_PCM_16BIT, + isOffload = false, + sdkInt = AudioOutputCachePolicy.MIN_SDK_INT - 1 + ) + ) + assertTrue( + AudioOutputCachePolicy.mayCache( + encoding = AudioFormat.ENCODING_PCM_16BIT, + isOffload = false, + sdkInt = AudioOutputCachePolicy.MIN_SDK_INT + ) + ) + } +} diff --git a/android/app/src/test/kotlin/com/edde746/plezy/exoplayer/BufferingStallPolicyTest.kt b/android/app/src/test/kotlin/com/edde746/plezy/exoplayer/BufferingStallPolicyTest.kt new file mode 100644 index 00000000..55187b89 --- /dev/null +++ b/android/app/src/test/kotlin/com/edde746/plezy/exoplayer/BufferingStallPolicyTest.kt @@ -0,0 +1,156 @@ +package com.edde746.plezy.exoplayer + +import com.edde746.plezy.exoplayer.BufferingStallPolicy.Verdict +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class BufferingStallPolicyTest { + + private val position = 85_206L + + private fun evaluate( + elapsedMs: Long = BufferingStallPolicy.STALL_TIMEOUT_MS, + baselinePositionMs: Long = position, + currentPositionMs: Long = position, + bufferedAheadMs: Long = 30_000L + ) = BufferingStallPolicy.evaluate( + elapsedMs = elapsedMs, + baselinePositionMs = baselinePositionMs, + currentPositionMs = currentPositionMs, + bufferedPositionMs = currentPositionMs + bufferedAheadMs + ) + + // Progress + + @Test + fun advancingPositionIsHealthy() { + assertEquals( + Verdict.HEALTHY, + evaluate(currentPositionMs = position + BufferingStallPolicy.PROGRESS_EPSILON_MS) + ) + } + + @Test + fun advancingPositionOutranksAnExpiredTimeout() { + assertEquals( + Verdict.HEALTHY, + evaluate(elapsedMs = 10 * BufferingStallPolicy.STALL_TIMEOUT_MS, currentPositionMs = position + 5_000) + ) + } + + @Test + fun clockJitterBelowTheEpsilonIsNotProgress() { + assertEquals( + Verdict.STALLED, + evaluate(currentPositionMs = position + BufferingStallPolicy.PROGRESS_EPSILON_MS - 1) + ) + } + + @Test + fun positionMovingBackwardsIsNotProgress() { + assertEquals(Verdict.STALLED, evaluate(currentPositionMs = position - 5_000)) + } + + // Timeout + + @Test + fun frozenPositionWaitsOutTheTimeout() { + assertEquals(Verdict.WAITING, evaluate(elapsedMs = BufferingStallPolicy.STALL_TIMEOUT_MS - 1)) + } + + /** The #1790 shape: data available, nothing playing, no error raised. */ + @Test + fun bufferedButFrozenIsStalled() { + assertEquals(Verdict.STALLED, evaluate()) + } + + // Starvation — the loader's problem, not the renderer's + + @Test + fun emptyBufferIsStarved() { + assertEquals(Verdict.STARVED, evaluate(bufferedAheadMs = 0)) + } + + @Test + fun starvationOutranksAnExpiredTimeout() { + assertEquals( + Verdict.STARVED, + evaluate(elapsedMs = 10 * BufferingStallPolicy.STALL_TIMEOUT_MS, bufferedAheadMs = 0) + ) + } + + /** + * `DefaultLoadControl` deliberately holds playback in `STATE_BUFFERING` until it has + * [LoadControlPolicy.BUFFER_FOR_PLAYBACK_AFTER_REBUFFER_MS] buffered. A watchdog that indicts + * the player inside that window is indicting it for obeying its own load control. + */ + @Test + fun aBufferBelowTheLoadControlPlayStartThresholdIsStarved() { + assertEquals( + Verdict.STARVED, + evaluate( + elapsedMs = 10 * BufferingStallPolicy.STALL_TIMEOUT_MS, + bufferedAheadMs = LoadControlPolicy.BUFFER_FOR_PLAYBACK_AFTER_REBUFFER_MS.toLong() + ) + ) + } + + @Test + fun theStallThresholdClearsTheLoadControlPlayStartThreshold() { + assertTrue( + "MIN_BUFFER_AHEAD_MS (${BufferingStallPolicy.MIN_BUFFER_AHEAD_MS}) must exceed the load " + + "control's post-rebuffer play-start threshold " + + "(${LoadControlPolicy.BUFFER_FOR_PLAYBACK_AFTER_REBUFFER_MS})", + BufferingStallPolicy.MIN_BUFFER_AHEAD_MS > LoadControlPolicy.BUFFER_FOR_PLAYBACK_AFTER_REBUFFER_MS + ) + } + + @Test + fun starvedIsStickyWhileTheBufferStaysThin() { + assertEquals( + Verdict.STARVED, + evaluate( + elapsedMs = 10 * BufferingStallPolicy.STALL_TIMEOUT_MS, + bufferedAheadMs = BufferingStallPolicy.MIN_BUFFER_AHEAD_MS - 1 + ) + ) + } + + // Stall clock ownership + + @Test + fun starvationAndProgressBothRestartTheStallClock() { + assertTrue(BufferingStallPolicy.resetsStallClock(Verdict.STARVED)) + assertTrue(BufferingStallPolicy.resetsStallClock(Verdict.HEALTHY)) + } + + @Test + fun waitingKeepsTheStallClockRunning() { + assertFalse(BufferingStallPolicy.resetsStallClock(Verdict.WAITING)) + } + + /** + * Regression: a long network stall used to accrue the whole timeout while starved, so the very + * first poll after the buffer refilled reported a stall that never happened and force-reloaded + * a perfectly healthy rebuffer. Starvation restarts the clock, so the refilled buffer gets the + * full timeout to start playing. + */ + @Test + fun aRecoveringBufferIsNotStalledByTimeSpentStarved() { + val starvedFor = 60_000L + val starved = evaluate(elapsedMs = starvedFor, bufferedAheadMs = 0) + assertEquals(Verdict.STARVED, starved) + assertTrue(BufferingStallPolicy.resetsStallClock(starved)) + + // The watchdog re-baselines on that verdict, so the next poll starts from zero elapsed. + assertEquals( + Verdict.WAITING, + evaluate( + elapsedMs = BufferingStallPolicy.CHECK_INTERVAL_MS, + bufferedAheadMs = BufferingStallPolicy.MIN_BUFFER_AHEAD_MS + 1 + ) + ) + } +} diff --git a/android/app/src/test/kotlin/com/edde746/plezy/exoplayer/RawPositionAudioOutputTest.kt b/android/app/src/test/kotlin/com/edde746/plezy/exoplayer/RawPositionAudioOutputTest.kt new file mode 100644 index 00000000..101dfddf --- /dev/null +++ b/android/app/src/test/kotlin/com/edde746/plezy/exoplayer/RawPositionAudioOutputTest.kt @@ -0,0 +1,302 @@ +package com.edde746.plezy.exoplayer + +import android.media.AudioDeviceInfo +import android.media.AudioFormat +import androidx.annotation.OptIn +import androidx.media3.common.PlaybackParameters +import androidx.media3.common.util.UnstableApi +import androidx.media3.exoplayer.audio.AudioOutput +import androidx.media3.exoplayer.audio.AudioOutputProvider +import java.nio.ByteBuffer +import java.util.concurrent.atomic.AtomicLong +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertSame +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * `DefaultAudioSink` increments a static, process-wide pending-release counter on every flush and + * decrements it only when the output reports `onReleased`. A dropped report pins that counter above + * zero for the life of the process, which silently disables media3's own AudioTrack-init retry and + * turns any later init failure into a permanent buffering hang (#1790). These tests pin the + * wrapper's side of that contract: exactly one report per release, from every path. + */ +@OptIn(UnstableApi::class) +class RawPositionAudioOutputTest { + + private val rawPositionUs = AtomicLong(Long.MIN_VALUE) + + private fun config( + encoding: Int = AudioFormat.ENCODING_PCM_16BIT, + isOffload: Boolean = false, + isTunneling: Boolean = false + ): AudioOutputProvider.OutputConfig = AudioOutputProvider.OutputConfig.Builder() + .setEncoding(encoding) + .setSampleRate(48_000) + .setChannelMask(AudioFormat.CHANNEL_OUT_STEREO) + .setBufferSize(40_000) + .setIsOffload(isOffload) + .setIsTunneling(isTunneling) + .build() + + private fun provider(delegate: FakeOutputProvider) = RawPositionOutputProvider(delegate, rawPositionUs, log = null, sdkInt = 34) + + private fun acquire( + provider: RawPositionOutputProvider, + config: AudioOutputProvider.OutputConfig + ): Pair { + val output = provider.getAudioOutput(config) as RawPositionAudioOutput + val listener = RecordingListener() + output.addListener(listener) + return output to listener + } + + // Release reporting + + /** + * A parked track is never going to release, so its flush has to be answered at once. Deferring + * until the eviction would hold `DefaultAudioSink`'s process-wide pending-release count above + * zero for the whole live track after the first seek, and any nonzero value stops media3 + * escalating *both* init and write failures — the #1790 hang, re-armed by an ordinary seek. + */ + @Test + fun aParkedOutputAnswersItsFlushImmediately() { + val delegate = FakeOutputProvider() + val provider = provider(delegate) + val (output, listener) = acquire(provider, config()) + + output.release() + + assertEquals("the flush must be answered while the track is parked", 1, listener.releasedCount) + assertFalse("a parked output must keep its AudioTrack", delegate.outputs.single().released) + } + + @Test + fun realReleaseReportsOnlyOnceTheDelegateConfirms() { + val delegate = FakeOutputProvider() + val provider = provider(delegate) + val (output, listener) = acquire(provider, config(encoding = AudioFormat.ENCODING_AC3)) + + output.release() + + val real = delegate.outputs.single() + assertTrue("a bitstream output must really release", real.released) + assertEquals("release must not be reported before the AudioTrack is gone", 0, listener.releasedCount) + + real.confirmReleased() + + assertEquals(1, listener.releasedCount) + } + + @Test + fun releaseIsReportedOnceWhenTheDelegateNeverConfirms() { + val delegate = FakeOutputProvider() + val provider = provider(delegate) + val (output, listener) = acquire(provider, config(encoding = AudioFormat.ENCODING_AC3)) + + // Player teardown: the sink flushes, then media3 posts the release completion to a playback + // looper that is already quitting, so the confirmation never arrives. + output.release() + provider.release() + + assertEquals(1, listener.releasedCount) + } + + @Test + fun aLateDelegateConfirmationDoesNotReportTwice() { + val delegate = FakeOutputProvider() + val provider = provider(delegate) + val (output, listener) = acquire(provider, config(encoding = AudioFormat.ENCODING_AC3)) + + output.release() + output.settleReleaseNow() + delegate.outputs.single().confirmReleased() + + assertEquals(1, listener.releasedCount) + } + + // Reuse + + @Test + fun aParkedOutputIsHandedBackForAnIdenticalConfig() { + val delegate = FakeOutputProvider() + val provider = provider(delegate) + val (first, _) = acquire(provider, config()) + + first.release() + val (second, _) = acquire(provider, config()) + + assertSame(first, second) + assertEquals("the delegate must not build a second AudioTrack", 1, delegate.outputs.size) + } + + /** + * The sink charges the pending-release counter once per flush and builds a fresh listener per + * acquisition, so every reuse cycle has to settle its own flush. One unanswered cycle is enough + * to pin the counter above zero for the rest of the process. + */ + @Test + fun everyReuseCycleAnswersItsOwnFlush() { + val delegate = FakeOutputProvider() + val provider = provider(delegate) + + repeat(5) { + val (output, listener) = acquire(provider, config()) + output.release() + assertEquals("cycle $it must report exactly one release", 1, listener.releasedCount) + } + + assertEquals("the delegate must not build a second AudioTrack", 1, delegate.outputs.size) + } + + /** A stale listener per cycle on the real output is how the accounting drifted in the first place. */ + @Test + fun reuseDoesNotAccumulateListenersOnTheRealOutput() { + val delegate = FakeOutputProvider() + val provider = provider(delegate) + + repeat(5) { + val (output, _) = acquire(provider, config()) + output.release() + } + + assertEquals(1, delegate.outputs.single().listeners.size) + } + + @Test + fun forwardedEventsReachTheCurrentListenerOnly() { + val delegate = FakeOutputProvider() + val provider = provider(delegate) + val (first, firstListener) = acquire(provider, config()) + first.release() + val (_, secondListener) = acquire(provider, config()) + + delegate.outputs.single().emitUnderrun() + + assertEquals(0, firstListener.underrunCount) + assertEquals(1, secondListener.underrunCount) + } + + // Eviction — the overlap media3 itself tolerates, kept tolerable + + /** + * The replacement is deliberately built while the evicted track is still going away. Refusing + * until it confirms looks safer, but the refusal reaches media3 as an init failure with no + * pending release to excuse it, so its 200ms deadline starts immediately — and on the TVs this + * cache exists for a teardown can outlast that, turning an ordinary config change into a + * playback error. Upstream tolerates the same overlap. + */ + @Test + fun anEvictedParkedOutputIsReplacedWithoutWaitingForItsRelease() { + val delegate = FakeOutputProvider() + val provider = provider(delegate) + val (output, _) = acquire(provider, config()) + + output.release() + provider.getAudioOutput(config(isTunneling = true)) + + assertTrue("the parked output must have been released", delegate.outputs.first().released) + assertEquals("the replacement must not wait for the release", 2, delegate.outputs.size) + } + + /** + * The evicted track's flush was already answered when it parked, so its late confirmation — + * however late — must not report a second release and drive the process-wide count negative. + */ + @Test + fun aSlowEvictionReleaseDoesNotReportASecondTime() { + val delegate = FakeOutputProvider() + val provider = provider(delegate) + val (output, listener) = acquire(provider, config()) + + output.release() + val evicted = delegate.outputs.first() + provider.getAudioOutput(config(isTunneling = true)) + assertEquals(1, listener.releasedCount) + + evicted.confirmReleased() + evicted.confirmReleased() + + assertEquals(1, listener.releasedCount) + } + + // Fakes + + private class RecordingListener : AudioOutput.Listener { + var releasedCount = 0 + var underrunCount = 0 + + override fun onPositionAdvancing(playoutStartSystemTimeMs: Long) = Unit + override fun onOffloadDataRequest() = Unit + override fun onOffloadPresentationEnded() = Unit + override fun onUnderrun() { + underrunCount++ + } + override fun onReleased() { + releasedCount++ + } + } + + private class FakeOutputProvider : AudioOutputProvider { + val outputs = mutableListOf() + + override fun getFormatSupport(formatConfig: AudioOutputProvider.FormatConfig) = throw UnsupportedOperationException() + + override fun getOutputConfig(formatConfig: AudioOutputProvider.FormatConfig) = throw UnsupportedOperationException() + + override fun getAudioOutput(config: AudioOutputProvider.OutputConfig): AudioOutput = FakeAudioOutput().also { outputs.add(it) } + + override fun addListener(listener: AudioOutputProvider.Listener) = Unit + override fun removeListener(listener: AudioOutputProvider.Listener) = Unit + override fun release() = Unit + } + + private class FakeAudioOutput : AudioOutput { + val listeners = mutableListOf() + var released = false + var stopped = false + var flushed = false + + fun confirmReleased() { + for (listener in listeners.toList()) listener.onReleased() + } + + fun emitUnderrun() { + for (listener in listeners.toList()) listener.onUnderrun() + } + + override fun play() = Unit + override fun pause() = Unit + override fun flush() { + flushed = true + } + override fun stop() { + stopped = true + } + override fun release() { + released = true + } + override fun write(buffer: ByteBuffer, encodedAccessUnitCount: Int, presentationTimeUs: Long) = true + override fun setVolume(volume: Float) = Unit + override fun isOffloadedPlayback() = false + override fun getAudioSessionId() = 1 + override fun getSampleRate() = 48_000 + override fun getBufferSizeInFrames() = 0L + override fun getPositionUs() = 0L + override fun getPlaybackParameters(): PlaybackParameters = PlaybackParameters.DEFAULT + override fun isStalled() = false + override fun addListener(listener: AudioOutput.Listener) { + listeners.add(listener) + } + override fun removeListener(listener: AudioOutput.Listener) { + listeners.remove(listener) + } + override fun setPlaybackParameters(playbackParams: PlaybackParameters) = Unit + override fun setOffloadDelayPadding(delayInFrames: Int, paddingInFrames: Int) = Unit + override fun setOffloadEndOfStream() = Unit + override fun attachAuxEffect(effectId: Int) = Unit + override fun setAuxEffectSendLevel(level: Float) = Unit + override fun setPreferredDevice(preferredDevice: AudioDeviceInfo?) = Unit + } +} diff --git a/lib/screens/video_player/parts/build.dart b/lib/screens/video_player/parts/build.dart index 184050c2..67cafb64 100644 --- a/lib/screens/video_player/parts/build.dart +++ b/lib/screens/video_player/parts/build.dart @@ -258,7 +258,11 @@ extension _VideoPlayerBuildMethods on VideoPlayerScreenState { if (widget.isLive) { onNext = _hasNextChannel ? () => _switchLiveChannel(1) : null; } else { - onNext = (_nextEpisode != null && authority.canNavigateMediaItems) ? _playNext : null; + // _playNext no-ops while a navigation is in flight; matching that here + // keeps the control from looking live while it does nothing. + onNext = (_nextEpisode != null && !_isLoadingNext && authority.canNavigateMediaItems) + ? _playNext + : null; } VoidCallback? onPrevious; diff --git a/lib/screens/video_player/parts/episode_navigation.dart b/lib/screens/video_player/parts/episode_navigation.dart index 2845292c..f123d234 100644 --- a/lib/screens/video_player/parts/episode_navigation.dart +++ b/lib/screens/video_player/parts/episode_navigation.dart @@ -139,52 +139,61 @@ extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { return; } - // Carry the playing version to the next episode by signature — its Media - // list may order versions differently, so the bare index is a guess and - // the source id is per-episode. - final currentVersionSignature = - _effectiveSelectedMediaIndex >= 0 && _effectiveSelectedMediaIndex < _availableVersions.length - ? _availableVersions[_effectiveSelectedMediaIndex].signature - : null; - // Users who curate per-episode selections server-side (e.g. via Plex Auto - // Languages) opt out of carrying tracks across episodes entirely: with no - // preferences, both audio and subtitles start at the server-selected - // priority (#1717). - final settingsService = await SettingsService.getInstance(); - final followServerSelections = settingsService.read(SettingsService.followServerTrackSelections); - final committedSubtitleSelection = _playbackSession?.subtitleSelection; - final primarySubtitlePreference = followServerSelections - ? null - : subtitlePreferenceForItemChange( - hasCommittedSelection: committedSubtitleSelection != null, - committedTrack: committedSubtitleSelection?.primaryTrack, - nativeTrack: currentPlayer.state.track.subtitle, - declinedPreference: committedSubtitleSelection?.declinedPreference, - sessionPreference: _sessionSubtitlePreference, - ); - final secondarySubtitlePreference = followServerSelections - ? null - : subtitlePreferenceForItemChange( - hasCommittedSelection: committedSubtitleSelection != null, - committedTrack: committedSubtitleSelection?.secondaryTrack, - nativeTrack: currentPlayer.state.track.secondarySubtitle, - sessionPreference: _sessionSecondarySubtitlePreference, - ); - await _reloadMediaInPlace( - metadata: episodeMetadata, - selectedMediaIndex: _effectiveSelectedMediaIndex, - selectedMediaSourceId: null, - preferredVersionSignature: currentVersionSignature, - qualityPreset: _selectedQualityPreset, - // Stream ids are per-part: the previous episode's audio id is - // meaningless on the new item, so let preferences pick the track. - useCurrentAudioStreamSelection: false, - preserveCurrentTrackSelection: !followServerSelections, - preservedAudioTrack: _sessionAudioPreference, - preservedSubtitleTrack: primarySubtitlePreference, - preservedSecondarySubtitleTrack: secondarySubtitlePreference, - reason: 'episode navigation', - ); + // Callers fire this without awaiting (auto-play countdown, PiP, the prompt), so an escaping + // throw would be an unhandled async error that also strands the loading flag set by + // _playNext — which then silently disables item navigation for the rest of the session. + try { + // Carry the playing version to the next episode by signature — its Media + // list may order versions differently, so the bare index is a guess and + // the source id is per-episode. + final currentVersionSignature = + _effectiveSelectedMediaIndex >= 0 && _effectiveSelectedMediaIndex < _availableVersions.length + ? _availableVersions[_effectiveSelectedMediaIndex].signature + : null; + // Users who curate per-episode selections server-side (e.g. via Plex Auto + // Languages) opt out of carrying tracks across episodes entirely: with no + // preferences, both audio and subtitles start at the server-selected + // priority (#1717). + final settingsService = await SettingsService.getInstance(); + final followServerSelections = settingsService.read(SettingsService.followServerTrackSelections); + final committedSubtitleSelection = _playbackSession?.subtitleSelection; + final primarySubtitlePreference = followServerSelections + ? null + : subtitlePreferenceForItemChange( + hasCommittedSelection: committedSubtitleSelection != null, + committedTrack: committedSubtitleSelection?.primaryTrack, + nativeTrack: currentPlayer.state.track.subtitle, + declinedPreference: committedSubtitleSelection?.declinedPreference, + sessionPreference: _sessionSubtitlePreference, + ); + final secondarySubtitlePreference = followServerSelections + ? null + : subtitlePreferenceForItemChange( + hasCommittedSelection: committedSubtitleSelection != null, + committedTrack: committedSubtitleSelection?.secondaryTrack, + nativeTrack: currentPlayer.state.track.secondarySubtitle, + sessionPreference: _sessionSecondarySubtitlePreference, + ); + await _reloadMediaInPlace( + metadata: episodeMetadata, + selectedMediaIndex: _effectiveSelectedMediaIndex, + selectedMediaSourceId: null, + preferredVersionSignature: currentVersionSignature, + qualityPreset: _selectedQualityPreset, + // Stream ids are per-part: the previous episode's audio id is + // meaningless on the new item, so let preferences pick the track. + useCurrentAudioStreamSelection: false, + preserveCurrentTrackSelection: !followServerSelections, + preservedAudioTrack: _sessionAudioPreference, + preservedSubtitleTrack: primarySubtitlePreference, + preservedSecondarySubtitleTrack: secondarySubtitlePreference, + reason: 'episode navigation', + ); + } catch (e, stackTrace) { + appLogger.e('Failed to navigate to the next item', error: e, stackTrace: stackTrace); + _clearEpisodeLoadingFlags(); + if (mounted) showErrorSnackBar(context, t.messages.errorLoading(error: e.toString())); + } } Future _switchPlaybackSource({ @@ -970,6 +979,12 @@ extension _VideoPlayerEpisodeNavigationMethods on VideoPlayerScreenState { // throw before the operational try/catch is entered. Identity ownership // prevents this continuation from releasing a newer transition. _releasePlaybackTransition(reloadLease); + // Every superseded return leaves the episode loading flags set otherwise, and + // _playNext/_playPrevious treat them as a re-entrancy guard — a stuck flag turns the + // Next button into a no-op for the rest of the session. Idempotent: the success and + // rollback paths above have already cleared them, and this skips the rebuild when + // neither flag is set. + _clearEpisodeLoadingFlags(); } } }