From c2d4728352d540af4c0863e11ad74a180f793c78 Mon Sep 17 00:00:00 2001 From: theov Date: Sat, 16 May 2026 13:06:28 -0300 Subject: [PATCH] fix: prevent audio HAL-reset heuristic from misinterpreting user seeks Prevents the `DualPlayerEngine` from incorrectly identifying a manual seek as a HAL offload underflow. By tracking seek timing, the engine now distinguishes between legitimate audio-sink resets and standard buffering caused by position changes, avoiding unnecessary player rebuilds that could drop pending commands. - Added `lastSeekAtMs` to track the timestamp of the most recent seek operation. - Implemented `notifyExternalSeekInitiated()` to allow the UI to flag seeks synchronously before they are dispatched to the `MediaController`. - Updated `onPlaybackStateChanged` logic to ignore `STATE_BUFFERING` events if they occur within a 1.5-second window of a seek. - Added `onPositionDiscontinuity` listener to capture and record seek events within the player engine. - Integrated the seek notification call in `PlaybackStateHolder` immediately prior to executing `seekTo`. --- .../data/service/player/DualPlayerEngine.kt | 41 ++++++++++++++++++- .../viewmodel/PlaybackStateHolder.kt | 4 ++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/theveloper/pixelplay/data/service/player/DualPlayerEngine.kt b/app/src/main/java/com/theveloper/pixelplay/data/service/player/DualPlayerEngine.kt index 9ee5fce9d..aa282d2ac 100644 --- a/app/src/main/java/com/theveloper/pixelplay/data/service/player/DualPlayerEngine.kt +++ b/app/src/main/java/com/theveloper/pixelplay/data/service/player/DualPlayerEngine.kt @@ -132,6 +132,12 @@ class DualPlayerEngine @Inject constructor( private var isFocusLossPause = false private var lastPlayWhenReadyAtMs: Long = 0L private var lastPlayingAtMs: Long = 0L + // Used to distinguish a STATE_BUFFERING caused by a user seek from a real HAL offload + // reset (where audio underflows mid-playback). Without this, seeking shortly after + // playback starts re-enters BUFFERING within the HAL-reset window and triggers a full + // player rebuild, which leaves the MediaSession briefly pointing at the released player + // and silently drops any subsequent seeks. + private var lastSeekAtMs: Long = 0L /** * Set by MusicService once ReplayGain for the incoming track is known. @@ -272,9 +278,14 @@ class DualPlayerEngine @Inject constructor( override fun onPlaybackStateChanged(playbackState: Int) { when (playbackState) { Player.STATE_BUFFERING -> { - val timeSincePlayingMs = SystemClock.elapsedRealtime() - lastPlayingAtMs + val now = SystemClock.elapsedRealtime() + val timeSincePlayingMs = now - lastPlayingAtMs + val timeSinceSeekMs = now - lastSeekAtMs + val isPostSeekBuffering = lastSeekAtMs > 0L && timeSinceSeekMs < 1_500L if (audioOffloadEnabled && !transitionRunning && - lastPlayingAtMs > 0L && timeSincePlayingMs < 500L) { + lastPlayingAtMs > 0L && timeSincePlayingMs < 500L && + !isPostSeekBuffering + ) { disableAudioOffloadForSession( reason = "HAL offload reset detected: STATE_BUFFERING after ${timeSincePlayingMs}ms of playback" ) @@ -285,6 +296,18 @@ class DualPlayerEngine @Inject constructor( Player.STATE_READY, Player.STATE_IDLE, Player.STATE_ENDED -> cancelAudioOffloadFallback() } } + + override fun onPositionDiscontinuity( + oldPosition: Player.PositionInfo, + newPosition: Player.PositionInfo, + reason: Int + ) { + if (reason == Player.DISCONTINUITY_REASON_SEEK || + reason == Player.DISCONTINUITY_REASON_SEEK_ADJUSTMENT + ) { + lastSeekAtMs = SystemClock.elapsedRealtime() + } + } } fun addPlayerSwapListener(listener: (Player) -> Unit) { @@ -307,6 +330,20 @@ class DualPlayerEngine @Inject constructor( onTransitionFinishedListeners.add(listener) } + /** + * Notifies the engine that an external caller (UI seek, etc.) is about to issue a + * seek through the MediaController. Used to mark the upcoming STATE_BUFFERING as + * seek-driven so the HAL-reset heuristic does not trigger a player rebuild that + * would race with the in-flight seek command. + * + * Setting this here (synchronously, before the seek dispatches) is more reliable + * than waiting for onPositionDiscontinuity, which is delivered on the next event + * batch and can race with onPlaybackStateChanged on some Media3 versions. + */ + fun notifyExternalSeekInitiated() { + lastSeekAtMs = SystemClock.elapsedRealtime() + } + fun removeTransitionFinishedListener(listener: () -> Unit) { onTransitionFinishedListeners.remove(listener) } diff --git a/app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/PlaybackStateHolder.kt b/app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/PlaybackStateHolder.kt index fdb0e0a62..7df212488 100644 --- a/app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/PlaybackStateHolder.kt +++ b/app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/PlaybackStateHolder.kt @@ -384,6 +384,10 @@ class PlaybackStateHolder @Inject constructor( val targetPosition = position.coerceAtLeast(0L) val currentMediaId = mediaController?.currentMediaItem?.mediaId rememberPausedPositionOverride(currentMediaId, targetPosition) + // Mark the seek before dispatching so the engine's HAL-reset heuristic does + // not misinterpret the resulting STATE_BUFFERING as an audio HAL underflow and + // rebuild the players (which would race with the in-flight seek command). + dualPlayerEngine.notifyExternalSeekInitiated() mediaController?.seekTo(targetPosition) } }