feat(android): adopt a running UPnP session + tighten cast recovery/anti-stickiness
android / Build + lint + test (push) Successful in 3m53s
android / Build + lint + test (push) Successful in 3m53s
Three related improvements to UPnP/Sonos session handling, on top of the WiFi-lock + drop-suppression fixes. 1. Adopt a running session instead of clear+reload (the headline). Selecting a renderer always did removeAllTracksFromQueue + full reload -- a jarring restart if the speaker was already playing our queue (e.g. after the phone got disconnected but the autonomous Sonos kept going). selectUpnp now probes the renderer first; if it's mid-playback on the same track id at the same queue index, we ATTACH in place: sync the local cursor to its position, wire ActiveUpnpHolder, start polling -- no clear, no reload, and skip/seek immediately drive its live queue. Falls through to clear+reload when it isn't our queue. New PlayerController.moveCursorTo aligns the local cursor without auto-playing. 2. Discovery expiry + selection revert (anti-stickiness). upsertRoute only ever added, so a powered-off renderer lingered in the picker forever and could pin a stale selection. Stamp lastSeen per route; after the picker's active M-SEARCH scan, prune routes that didn't re-announce. A collector reverts the selection to the phone when the selected route leaves discovery while we're not actively casting -- so a later play never targets a ghost. Pruning is tied to picker-open scans only (no background timer -> no row flicker). 3. Reconcile immediately on network recovery. When NetworkStatus flips back to Healthy while a route is active, nudge an immediate poll instead of waiting up to POLL_INTERVAL_MS -- the held session re-confirms the renderer in one round-trip. Verified (read-only): the tap-play-onto-dead-route fallback still fires when the phone's network is Healthy (the poll-loop drop path is unchanged for that case); the drop-suppression gate only holds during phone-side outages, where a local fallback couldn't play either. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -120,6 +120,20 @@ class MinstrelForwardingPlayer(
|
|||||||
scope.launch {
|
scope.launch {
|
||||||
holder.active.collect { active -> onActiveChanged(active) }
|
holder.active.collect { active -> onActiveChanged(active) }
|
||||||
}
|
}
|
||||||
|
scope.launch {
|
||||||
|
// When the phone's own network recovers, reconcile a held cast
|
||||||
|
// session immediately instead of waiting up to POLL_INTERVAL_MS:
|
||||||
|
// the renderer kept playing, so one round-trip re-confirms it and
|
||||||
|
// resyncs the cursor/position. Only a transition *into* Healthy
|
||||||
|
// while a route is active matters.
|
||||||
|
var prev = networkStatus.state.value
|
||||||
|
networkStatus.state.collect { next ->
|
||||||
|
if (next == ServerHealth.Healthy && prev != ServerHealth.Healthy && isRemote()) {
|
||||||
|
pollTrigger.trySend(Unit)
|
||||||
|
}
|
||||||
|
prev = next
|
||||||
|
}
|
||||||
|
}
|
||||||
// Process lifecycle is observed on the main thread; ProcessLifecycleOwner's
|
// Process lifecycle is observed on the main thread; ProcessLifecycleOwner's
|
||||||
// addObserver requires it. The observer just trySend's to the channel.
|
// addObserver requires it. The observer just trySend's to the channel.
|
||||||
handler.post {
|
handler.post {
|
||||||
|
|||||||
@@ -192,6 +192,20 @@ class PlayerController @Inject constructor(
|
|||||||
controller.play()
|
controller.play()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Align the local cursor to [index] + [positionMs] WITHOUT auto-playing.
|
||||||
|
* Used by the UPnP adopt path: when attaching to a renderer that's already
|
||||||
|
* playing our queue, we sync the local cursor to the renderer's current
|
||||||
|
* track/position so skip/seek and the UI map onto its live session — the
|
||||||
|
* renderer keeps the audio, the local player stays paused. Unlike
|
||||||
|
* [seekToIndex] this issues no play().
|
||||||
|
*/
|
||||||
|
fun moveCursorTo(index: Int, positionMs: Long) {
|
||||||
|
val controller = mediaController ?: return
|
||||||
|
if (index !in queueRefs.indices) return
|
||||||
|
runOnControllerThread(controller) { controller.seekTo(index, positionMs) }
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Replace the queue with [tracks] starting at [initialIndex].
|
* Replace the queue with [tracks] starting at [initialIndex].
|
||||||
* [source] tags the queue with its origin (e.g. "for_you") so the
|
* [source] tags the queue with its origin (e.g. "for_you") so the
|
||||||
|
|||||||
+130
@@ -15,6 +15,7 @@ import com.fabledsword.minstrel.player.output.upnp.AVTransportClient
|
|||||||
import com.fabledsword.minstrel.player.output.upnp.RenderingControlClient
|
import com.fabledsword.minstrel.player.output.upnp.RenderingControlClient
|
||||||
import com.fabledsword.minstrel.player.output.upnp.SoapClient
|
import com.fabledsword.minstrel.player.output.upnp.SoapClient
|
||||||
import com.fabledsword.minstrel.player.output.upnp.SoapFaultException
|
import com.fabledsword.minstrel.player.output.upnp.SoapFaultException
|
||||||
|
import com.fabledsword.minstrel.player.output.upnp.TransportState
|
||||||
import com.fabledsword.minstrel.player.output.upnp.UpnpDiscoveryController
|
import com.fabledsword.minstrel.player.output.upnp.UpnpDiscoveryController
|
||||||
import com.fabledsword.minstrel.player.output.upnp.bareUdn
|
import com.fabledsword.minstrel.player.output.upnp.bareUdn
|
||||||
import dagger.hilt.android.qualifiers.ApplicationContext
|
import dagger.hilt.android.qualifiers.ApplicationContext
|
||||||
@@ -29,6 +30,7 @@ import kotlinx.coroutines.flow.stateIn
|
|||||||
import kotlinx.coroutines.launch
|
import kotlinx.coroutines.launch
|
||||||
import kotlinx.coroutines.sync.Mutex
|
import kotlinx.coroutines.sync.Mutex
|
||||||
import kotlinx.coroutines.sync.withLock
|
import kotlinx.coroutines.sync.withLock
|
||||||
|
import okhttp3.HttpUrl.Companion.toHttpUrlOrNull
|
||||||
import okhttp3.OkHttpClient
|
import okhttp3.OkHttpClient
|
||||||
import timber.log.Timber
|
import timber.log.Timber
|
||||||
import javax.inject.Inject
|
import javax.inject.Inject
|
||||||
@@ -172,6 +174,31 @@ class OutputPickerController @Inject constructor(
|
|||||||
}
|
}
|
||||||
scope.launch { observeQueueChangesForSonosResync() }
|
scope.launch { observeQueueChangesForSonosResync() }
|
||||||
scope.launch { observeIdleRevertWhileUpnp() }
|
scope.launch { observeIdleRevertWhileUpnp() }
|
||||||
|
scope.launch { observeSelectedRouteDisappearance() }
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Revert the UPnP selection to the phone when the selected renderer leaves
|
||||||
|
* discovery (powered off / off the LAN) while we are NOT actively casting
|
||||||
|
* to it. Without this the selection could pin to a ghost route, so a later
|
||||||
|
* "play" would target a speaker that's gone. An *active* cast is left alone
|
||||||
|
* -- its own poll loop arbitrates liveness and won't drop on a mere
|
||||||
|
* discovery gap. Routes leave discovery via [UpnpDiscoveryController]'s
|
||||||
|
* post-picker-scan prune; this reacts to the resulting routes change.
|
||||||
|
*/
|
||||||
|
private suspend fun observeSelectedRouteDisappearance() {
|
||||||
|
upnpDiscovery.routes.collect { routes ->
|
||||||
|
val selected = selectedUpnpRouteIdInternal.value
|
||||||
|
val present = routes.any { it.id == selected }
|
||||||
|
val casting = activeUpnpHolder.active.value?.routeId == selected
|
||||||
|
if (selected != null && !present && !casting) {
|
||||||
|
Timber.w(
|
||||||
|
"Output: selected UPnP route %s left discovery; reverting to phone",
|
||||||
|
selected,
|
||||||
|
)
|
||||||
|
selectedUpnpRouteIdInternal.value = null
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -541,6 +568,13 @@ class OutputPickerController @Inject constructor(
|
|||||||
// local audio while we queue up Sonos.
|
// local audio while we queue up Sonos.
|
||||||
playerController.pause()
|
playerController.pause()
|
||||||
selectedUpnpRouteIdInternal.value = effectiveRoute.id
|
selectedUpnpRouteIdInternal.value = effectiveRoute.id
|
||||||
|
// If the renderer is already playing this exact queue (a "stray"
|
||||||
|
// session we got disconnected from), attach to it in place instead of
|
||||||
|
// wiping + reloading -- no jarring restart, and skip/seek land on its
|
||||||
|
// live queue. Falls through to the clear+reload path when it isn't ours.
|
||||||
|
if (tryAdoptRunningSession(transport, rendering, effectiveRoute, uiState.queue)) {
|
||||||
|
return@withLock
|
||||||
|
}
|
||||||
// Mark UPnP loading. ForwardingPlayer overrides drop transport commands
|
// Mark UPnP loading. ForwardingPlayer overrides drop transport commands
|
||||||
// silently while target is set but active is null -- the user's premature
|
// silently while target is set but active is null -- the user's premature
|
||||||
// taps don't hit Sonos's stale state from a prior session.
|
// taps don't hit Sonos's stale state from a prior session.
|
||||||
@@ -567,6 +601,102 @@ class OutputPickerController @Inject constructor(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The renderer's current position + transport state, validated as an
|
||||||
|
* adoptable continuation of our local queue: same track id at the same
|
||||||
|
* 1-based index. Null means "not ours / not playing" -> caller reloads.
|
||||||
|
*/
|
||||||
|
private data class AdoptPlan(
|
||||||
|
val localIndex: Int,
|
||||||
|
val positionMs: Long,
|
||||||
|
val durationMs: Long,
|
||||||
|
val trackUri: String,
|
||||||
|
val trackNumber: Int,
|
||||||
|
val playing: Boolean,
|
||||||
|
)
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Attach to a renderer that is *already* playing our queue (a session the
|
||||||
|
* phone got disconnected from but the speaker kept streaming) instead of
|
||||||
|
* clearing + reloading it. Probes the renderer's live state, and if it's
|
||||||
|
* mid-playback on the same track id at the same queue index, syncs the
|
||||||
|
* local cursor to match and wires [ActiveUpnpHolder] -- the speaker never
|
||||||
|
* skips a beat, and skip/seek now drive its live queue. Returns false (and
|
||||||
|
* the caller does the full clear+reload) when the renderer isn't playing
|
||||||
|
* our queue. The local player is already paused by the caller.
|
||||||
|
*/
|
||||||
|
private suspend fun tryAdoptRunningSession(
|
||||||
|
transport: AVTransportClient,
|
||||||
|
rendering: RenderingControlClient?,
|
||||||
|
route: OutputRoute,
|
||||||
|
localQueue: List<TrackRef>,
|
||||||
|
): Boolean {
|
||||||
|
val plan = probeAdoptable(transport, route, localQueue) ?: return false
|
||||||
|
Timber.w(
|
||||||
|
"UPnP adopt: attaching to live session on %s at track %d (%dms)",
|
||||||
|
route.name, plan.trackNumber, plan.positionMs,
|
||||||
|
)
|
||||||
|
// active is still null here, so this aligns the *local* cursor without
|
||||||
|
// emitting SOAP; the holder.set below then starts the poll loop, which
|
||||||
|
// reconciles to the renderer's advancing position from here.
|
||||||
|
playerController.moveCursorTo(plan.localIndex, plan.positionMs)
|
||||||
|
remoteState.applyPositionInfo(
|
||||||
|
positionMs = plan.positionMs,
|
||||||
|
durationMs = plan.durationMs,
|
||||||
|
trackUri = plan.trackUri,
|
||||||
|
trackNumber = plan.trackNumber,
|
||||||
|
)
|
||||||
|
remoteState.setPlayIntent(plan.playing)
|
||||||
|
activeUpnpHolder.set(
|
||||||
|
ActiveUpnp(
|
||||||
|
routeId = route.id,
|
||||||
|
routeName = route.name,
|
||||||
|
avTransport = transport,
|
||||||
|
rendering = rendering,
|
||||||
|
),
|
||||||
|
)
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
|
||||||
|
private suspend fun probeAdoptable(
|
||||||
|
transport: AVTransportClient,
|
||||||
|
route: OutputRoute,
|
||||||
|
localQueue: List<TrackRef>,
|
||||||
|
): AdoptPlan? {
|
||||||
|
val probe = runCatching {
|
||||||
|
transport.getTransportInfo() to transport.getPositionInfo()
|
||||||
|
}.getOrElse {
|
||||||
|
Timber.w(it, "UPnP adopt: probe failed for %s; will reload", route.name)
|
||||||
|
return null
|
||||||
|
}
|
||||||
|
val (info, pos) = probe
|
||||||
|
val idx0 = pos.track - 1
|
||||||
|
val playingId = trackIdFromStreamUrl(pos.trackUri)
|
||||||
|
val midPlayback =
|
||||||
|
info.state == TransportState.PLAYING || info.state == TransportState.PAUSED
|
||||||
|
val aligned = midPlayback && playingId != null &&
|
||||||
|
idx0 in localQueue.indices && localQueue[idx0].id == playingId
|
||||||
|
return if (aligned) {
|
||||||
|
AdoptPlan(
|
||||||
|
localIndex = idx0,
|
||||||
|
positionMs = pos.relTimeMs,
|
||||||
|
durationMs = pos.trackDurationMs,
|
||||||
|
trackUri = pos.trackUri,
|
||||||
|
trackNumber = pos.track,
|
||||||
|
playing = info.state == TransportState.PLAYING,
|
||||||
|
)
|
||||||
|
} else {
|
||||||
|
null
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Extract our track id from a stream URL `.../api/tracks/<id>/stream...`. */
|
||||||
|
private fun trackIdFromStreamUrl(url: String): String? {
|
||||||
|
val segments = url.toHttpUrlOrNull()?.pathSegments ?: return null
|
||||||
|
val i = segments.indexOf("tracks")
|
||||||
|
return if (i >= 0) segments.getOrNull(i + 1) else null
|
||||||
|
}
|
||||||
|
|
||||||
private suspend fun loadQueueOnSonos(
|
private suspend fun loadQueueOnSonos(
|
||||||
transport: AVTransportClient,
|
transport: AVTransportClient,
|
||||||
route: OutputRoute,
|
route: OutputRoute,
|
||||||
|
|||||||
+39
-1
@@ -8,6 +8,7 @@ import com.fabledsword.minstrel.player.output.upnp.sonos.ZoneGroupTopologyClient
|
|||||||
import dagger.hilt.android.qualifiers.ApplicationContext
|
import dagger.hilt.android.qualifiers.ApplicationContext
|
||||||
import kotlinx.coroutines.CoroutineScope
|
import kotlinx.coroutines.CoroutineScope
|
||||||
import kotlinx.coroutines.Dispatchers
|
import kotlinx.coroutines.Dispatchers
|
||||||
|
import kotlinx.coroutines.delay
|
||||||
import kotlinx.coroutines.flow.MutableStateFlow
|
import kotlinx.coroutines.flow.MutableStateFlow
|
||||||
import kotlinx.coroutines.flow.StateFlow
|
import kotlinx.coroutines.flow.StateFlow
|
||||||
import kotlinx.coroutines.flow.asStateFlow
|
import kotlinx.coroutines.flow.asStateFlow
|
||||||
@@ -17,6 +18,7 @@ import okhttp3.HttpUrl.Companion.toHttpUrlOrNull
|
|||||||
import okhttp3.OkHttpClient
|
import okhttp3.OkHttpClient
|
||||||
import okhttp3.Request
|
import okhttp3.Request
|
||||||
import timber.log.Timber
|
import timber.log.Timber
|
||||||
|
import java.util.concurrent.ConcurrentHashMap
|
||||||
import java.util.concurrent.TimeUnit
|
import java.util.concurrent.TimeUnit
|
||||||
import javax.inject.Inject
|
import javax.inject.Inject
|
||||||
import javax.inject.Singleton
|
import javax.inject.Singleton
|
||||||
@@ -58,6 +60,12 @@ class UpnpDiscoveryController @Inject constructor(
|
|||||||
private val routesInternal = MutableStateFlow<List<UpnpRoute>>(emptyList())
|
private val routesInternal = MutableStateFlow<List<UpnpRoute>>(emptyList())
|
||||||
val routes: StateFlow<List<UpnpRoute>> = routesInternal.asStateFlow()
|
val routes: StateFlow<List<UpnpRoute>> = routesInternal.asStateFlow()
|
||||||
|
|
||||||
|
// Wall-clock of the last NOTIFY / M-SEARCH response per route id. Drives
|
||||||
|
// post-active-scan pruning so a powered-off renderer leaves the picker
|
||||||
|
// (and can't pin a stale selection) instead of lingering for the process
|
||||||
|
// lifetime -- upsert only ever added before.
|
||||||
|
private val lastSeenMs = ConcurrentHashMap<String, Long>()
|
||||||
|
|
||||||
private val sonosTopologyInternal = MutableStateFlow<List<SonosZoneGroup>>(emptyList())
|
private val sonosTopologyInternal = MutableStateFlow<List<SonosZoneGroup>>(emptyList())
|
||||||
val sonosTopology: StateFlow<List<SonosZoneGroup>> = sonosTopologyInternal.asStateFlow()
|
val sonosTopology: StateFlow<List<SonosZoneGroup>> = sonosTopologyInternal.asStateFlow()
|
||||||
|
|
||||||
@@ -78,7 +86,31 @@ class UpnpDiscoveryController @Inject constructor(
|
|||||||
* that just opened).
|
* that just opened).
|
||||||
*/
|
*/
|
||||||
fun upgradeDiscovery() {
|
fun upgradeDiscovery() {
|
||||||
appScope.launch { ssdp.requestActiveScan() }
|
appScope.launch {
|
||||||
|
val scanStartMs = System.currentTimeMillis()
|
||||||
|
ssdp.requestActiveScan()
|
||||||
|
// Give present devices time to answer the M-SEARCH (+ our follow-up
|
||||||
|
// description fetch), then drop any route that didn't re-announce.
|
||||||
|
// Pruning is tied to picker-open scans only -- no background timer,
|
||||||
|
// so route rows never flicker while the user isn't looking.
|
||||||
|
delay(ACTIVE_SCAN_PRUNE_DELAY_MS)
|
||||||
|
pruneRoutesNotSeenSince(scanStartMs)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Drop routes whose last NOTIFY / M-SEARCH response predates [cutoffMs]
|
||||||
|
* -- i.e. they didn't answer the active scan that just ran, so they're
|
||||||
|
* gone from the LAN. Called only after [upgradeDiscovery]'s scan window.
|
||||||
|
*/
|
||||||
|
private fun pruneRoutesNotSeenSince(cutoffMs: Long) {
|
||||||
|
val before = routesInternal.value
|
||||||
|
val survivors = before.filter { (lastSeenMs[it.id] ?: 0L) >= cutoffMs }
|
||||||
|
if (survivors.size == before.size) return
|
||||||
|
val removed = before.filterNot { it in survivors }.map { it.id }
|
||||||
|
removed.forEach { lastSeenMs.remove(it) }
|
||||||
|
Timber.w("UPnP discovery: pruned %d stale route(s): %s", removed.size, removed)
|
||||||
|
routesInternal.value = survivors
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -115,6 +147,7 @@ class UpnpDiscoveryController @Inject constructor(
|
|||||||
}
|
}
|
||||||
|
|
||||||
private fun upsertRoute(route: UpnpRoute) {
|
private fun upsertRoute(route: UpnpRoute) {
|
||||||
|
lastSeenMs[route.id] = System.currentTimeMillis()
|
||||||
val current = routesInternal.value
|
val current = routesInternal.value
|
||||||
val idx = current.indexOfFirst { it.id == route.id }
|
val idx = current.indexOfFirst { it.id == route.id }
|
||||||
routesInternal.value = if (idx < 0) {
|
routesInternal.value = if (idx < 0) {
|
||||||
@@ -240,6 +273,11 @@ class UpnpDiscoveryController @Inject constructor(
|
|||||||
// trips the drop-recovery quickly. Conservative enough not to false-drop
|
// trips the drop-recovery quickly. Conservative enough not to false-drop
|
||||||
// a slow-but-alive LAN; read timeout still inherits the shared client.
|
// a slow-but-alive LAN; read timeout still inherits the shared client.
|
||||||
const val CONTROL_CONNECT_TIMEOUT_SECONDS = 2L
|
const val CONTROL_CONNECT_TIMEOUT_SECONDS = 2L
|
||||||
|
|
||||||
|
// Grace window after an M-SEARCH burst before pruning non-responders.
|
||||||
|
// Covers the SSDP round-trip plus our per-device description fetch;
|
||||||
|
// generous enough that a present-but-slow renderer isn't false-pruned.
|
||||||
|
const val ACTIVE_SCAN_PRUNE_DELAY_MS = 4_000L
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user