From 673f98487f25833bb0bb2a584c222e4a0530a0eb Mon Sep 17 00:00:00 2001
From: Bryan Van Deusen <bryan.vandeusen@fabledsword.com>
Date: Wed, 3 Jun 2026 19:53:41 -0400
Subject: [PATCH] fix(android): UPnP local pause via delegate -- no SOAP race

Move local ExoPlayer pause from OutputPickerController.selectUpnp
into MinstrelForwardingPlayer.onActiveChanged (handler.post { delegate.pause() }).
This guarantees the pause hits ExoPlayer before the holder is live, eliminating
the async race that caused SOAP fault 701 on Sonos when pause() was dispatched
via playerController after holder.active was already set.
Also adds per-poll Timber.w before initialPreQueueDone for diagnostics.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 .../minstrel/player/MinstrelForwardingPlayer.kt   | 15 +++++++++++++++
 .../player/output/OutputPickerController.kt       | 13 +++++++------
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelForwardingPlayer.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelForwardingPlayer.kt
index 6099fb51..34cec946 100644
--- a/android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelForwardingPlayer.kt
+++ b/android/app/src/main/java/com/fabledsword/minstrel/player/MinstrelForwardingPlayer.kt
@@ -206,6 +206,13 @@ class MinstrelForwardingPlayer(
         pollJob?.cancel()
         if (active != null) {
             Timber.w("UPnP active: %s -- pollLoop starting", active.routeName)
+            // Pause the wrapped ExoPlayer so we are not playing local audio
+            // simultaneously with the remote renderer. handler.post targets the
+            // application looper, so this runs on the same thread that processes
+            // our override calls -- no race with the pause() override branching
+            // to SOAP (holder.active is already non-null by the time this post
+            // fires, but delegate.pause() bypasses the override entirely).
+            handler.post { delegate.pause() }
             pollJob = scope.launch { pollLoop(active) }
             // Do NOT pre-queue here: selectUpnp's SetAV+Play SOAP may not have
             // landed yet. Initial pre-queue fires from pollLoop once the first
@@ -225,6 +232,14 @@ class MinstrelForwardingPlayer(
                 // SetAV+Play from selectUpnp (trackUri lands in the first
                 // successful poll). Firing from onActiveChanged would race
                 // with selectUpnp's SOAP calls.
+                if (!initialPreQueueDone) {
+                    Timber.w(
+                        "UPnP poll OK: route=%s trackUri=%s posMs=%d",
+                        active.routeName,
+                        remoteState.currentTrackUri,
+                        remoteState.positionMs,
+                    )
+                }
                 if (!initialPreQueueDone && remoteState.currentTrackUri.isNotBlank()) {
                     Timber.w("UPnP initial pre-queue for %s", active.routeName)
                     preQueueNext(active)
diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerController.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerController.kt
index 096a3ef5..e72973fc 100644
--- a/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerController.kt
+++ b/android/app/src/main/java/com/fabledsword/minstrel/player/output/OutputPickerController.kt
@@ -258,12 +258,13 @@ class OutputPickerController @Inject constructor(
             return@withLock
         }
         val rendering = renderingClientFor(effectiveRoute.id)
-        // Pause local FIRST while holder.active is still null so the pause()
-        // call routes to ExoPlayer, not SOAP. After holder.set() below,
-        // ForwardingPlayer.pause() would translate this into AVTransport.Pause()
-        // -- the opposite of what we want, since we are about to tell Sonos
-        // to start playing.
-        playerController.pause()
+        // Local pause is handled by MinstrelForwardingPlayer.onActiveChanged
+        // via handler.post { delegate.pause() } -- called immediately after
+        // holder.set() fires. That path bypasses the SOAP-routing override, so
+        // there is no race between pausing ExoPlayer and starting the remote
+        // renderer. Do NOT call playerController.pause() here: it dispatches
+        // through Handler.post on the application looper, meaning it runs after
+        // holder.active is non-null and the override would route it to SOAP.
         selectedUpnpRouteIdInternal.value = effectiveRoute.id
         activeUpnpHolder.set(
             ActiveUpnp(