Conversation
e56c496 to
cb01719
Compare
In DeviceWhilePlaying mode the device volume is driven by the player volume. The server re-sends its volume on each stream start, and applying it again overwrote volume changes the user made with a car head unit, which sets the device volume directly using Bluetooth absolute volume. Fix this by adopting external device volume changes (that were not caused by ourselves) as the new desired volume, and by not applying the volume the server had already announced before.
The server ramps the volume when pausing or resuming playback, and the device (e.g. a car head unit using Bluetooth absolute volume) also reports our own volume changes back, rounded to its own volume steps. The previous code only ignored changes which were exactly the volume we applied ourselves, so each step of such a ramp was adopted as the volume the user wanted. As the adopted volume is applied again on subsequent playback state changes, the volume changed (and drifted) on every pause/resume cycle. Ignore changes which arrive shortly after we changed the volume ourselves instead, which covers both cases.
The server ramps the volume when pausing and resuming playback. Applying the ramp to the device volume made connected devices (e.g. car head units following the Bluetooth volume) show every step and pop up their volume slider, and the saved system volume was restored while playback was only paused. - Remember the time of a pause toggle and treat server volume changes arriving around it as the pause/resume fade: they are applied as a factor on the player volume, which connected devices don't see. - While paused, only ever apply server volume changes to the player volume. - Don't write the device volume when the value doesn't change: devices connected via Bluetooth still report such a write as a volume change. - Restore the saved device volume only after playback really stopped (no new stream follows within 3s), not on pause or track change.
cb01719 to
be5c786
Compare
The server ramps the volume when pausing, resuming or stopping a stream and re-announces its volume when a new stream starts. Those values are now recognised as fades for the whole burst (the last step of a ramp can arrive after the fade window has passed) and are kept out of both the device volume and lastServerVolume: applying them made the device volume jump on every track change and pause toggle, and remembering them made the next announcement of the real server volume overwrite the volume the user had set. The check for volume changes we caused ourselves happened after lastAppliedDeviceVolume had been updated to the reported volume, which made it always match: externally set volumes were never adopted, so the device volume was reset to the server volume on every playback state change. Check before updating the tracking again. Ramps are now applied relative to the volume the server ramps from (and to the player volume only), so playback isn't attenuated once the ramp is over.
The server ramps its volume around pause toggles, track changes and stream stops, and it announces its volume when a stream is set up. Such values still reached the device volume in some orderings (e.g. a ramp that started while playback was still running and only settled after the pause), which muted or reset the volume the user had set on the device. - Re-check the fade condition when a pending volume change is applied, not only when it arrives. - Re-adopt the volume the device is set to on pause toggles and around stream starts/stops, so playback continues at the volume the user set there instead of a fade value the server sent along. - While playback isn't running, server volume changes count as fades in the modes driving the device volume (PlayerOnly mode is unaffected).
Explainer why so many changesBehaviour now (Device / DeviceWhilePlaying)
Why 5 commits
Protocol reality (why it is easy to get wrong)
Verified on device (SM-F956U1, LMS 9.1.2; adb + LMS CLI)
Limits
|
The complication (and I think the proposed change is way too complicated for my taste) comes from this very problem: it's trying to fight the system. |
|
My idea is something like this: diff --git a/app/src/main/java/de/maniac103/squeezeclient/service/localplayer/LocalPlaybackService.kt b/app/src/main/java/de/maniac103/squeezeclient/service/localplayer/LocalPlaybackService.kt
index b87dbcc..ec6be8c 100644
--- a/app/src/main/java/de/maniac103/squeezeclient/service/localplayer/LocalPlaybackService.kt
+++ b/app/src/main/java/de/maniac103/squeezeclient/service/localplayer/LocalPlaybackService.kt
@@ -42,6 +42,8 @@ import androidx.work.NetworkType
import androidx.work.OneTimeWorkRequestBuilder
import androidx.work.OutOfQuotaPolicy
import de.maniac103.squeezeclient.R
+import de.maniac103.squeezeclient.cometd.ConnectionState
+import de.maniac103.squeezeclient.extfuncs.connectionHelper
import de.maniac103.squeezeclient.extfuncs.getOrCreateNotificationChannel
import de.maniac103.squeezeclient.extfuncs.localPlayerEnabled
import de.maniac103.squeezeclient.extfuncs.localPlayerName
@@ -51,6 +53,7 @@ import de.maniac103.squeezeclient.extfuncs.workManager
import de.maniac103.squeezeclient.service.NotificationIds
import de.maniac103.squeezeclient.ui.MainActivity
import de.maniac103.squeezeclient.ui.prefs.SettingsActivity
+import kotlin.math.roundToInt
import kotlin.time.Duration.Companion.milliseconds
import kotlin.time.Duration.Companion.seconds
import kotlin.time.DurationUnit
@@ -64,8 +67,11 @@ import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.collectLatest
import kotlinx.coroutines.flow.debounce
+import kotlinx.coroutines.flow.filterIsInstance
+import kotlinx.coroutines.flow.first
import kotlinx.coroutines.isActive
import kotlinx.coroutines.launch
+import kotlinx.coroutines.withTimeoutOrNull
import okhttp3.Response
@OptIn(ExperimentalTime::class)
@@ -103,7 +109,8 @@ class LocalPlaybackService :
onDecoderLoadFinished = { onDecoderLoadFinished() },
onDecodingFinished = { onDecodingFinished() },
onHeadersReceived = { resp -> onHeadersReceived(resp) },
- onMetadataReceived = { title, artworkUri -> onMetadataReceived(title, artworkUri) }
+ onMetadataReceived = { title, artworkUri -> onMetadataReceived(title, artworkUri) },
+ onVolumeChanged = { volume -> onVolumeChanged(volume) }
)
startupTimestampNanos = System.nanoTime()
}
@@ -157,6 +164,16 @@ class LocalPlaybackService :
}
}
+ private fun onVolumeChanged(volume: Float) = lifecycleScope.launch {
+ connectionHelper.connect()
+ val connectedState = withTimeoutOrNull(2.seconds) {
+ connectionHelper.state.filterIsInstance<ConnectionState.Connected>().first()
+ }
+ if (connectedState != null) {
+ connectionHelper.setVolume(slimproto.playerId, (volume * 100).roundToInt()) // playerId depends on #62
+ }
+ }
+
private fun onPlaybackReady(buffering: Boolean) = lifecycleScope.launch {
slimprotoStateFlow.emit(
SlimprotoState.PlayingOrPaused(player.playingTitle, player.paused)
diff --git a/app/src/main/java/de/maniac103/squeezeclient/service/localplayer/LocalPlayer.kt b/app/src/main/java/de/maniac103/squeezeclient/service/localplayer/LocalPlayer.kt
index e4f78cf..f71fdd5 100644
--- a/app/src/main/java/de/maniac103/squeezeclient/service/localplayer/LocalPlayer.kt
+++ b/app/src/main/java/de/maniac103/squeezeclient/service/localplayer/LocalPlayer.kt
@@ -70,7 +70,8 @@ class LocalPlayer(
onDecodingFinished: () -> Unit = {},
onAudioStreamFlushed: () -> Unit = {},
private val onHeadersReceived: (response: Response) -> Unit = {},
- private val onMetadataReceived: (title: CharSequence, artworkUri: Uri?) -> Unit = { _, _ -> }
+ private val onMetadataReceived: (title: CharSequence, artworkUri: Uri?) -> Unit = { _, _ -> },
+ private val onVolumeChanged: (volume: Float) -> Unit = {}
) : Player.Listener {
private val prefs = context.prefs
private val dataSourceFactory: HttpDataSource.Factory
@@ -109,6 +110,7 @@ class LocalPlayer(
private var playerInternalVolume = 1F
private var currentReplayGain = 1F
private var lastSavedDeviceVolume: Int? = null
+ private var lastSetDeviceVolume: Int? = null
@UnstableApi
private val audioProcessor = LocalPlayerAudioProcessor(
@@ -317,6 +319,16 @@ class LocalPlayer(
mediaMetadata.title?.let { onMetadataReceived(it, mediaMetadata.artworkUri) }
}
·
+ override fun onDeviceVolumeChanged(volume: Int, muted: Boolean) {
+ super.onDeviceVolumeChanged(volume, muted)
+ if (lastSetDeviceVolume != null && volume != lastSetDeviceVolume) {
+ val maxVolume = player.deviceInfo.maxVolume.toFloat()
+ if (maxVolume > 0) {
+ onVolumeChanged(volume.toFloat() / maxVolume)
+ }
+ }
+ }
+
private fun updatePlayerVolume(isSetVolume: Boolean) {
val volume = lastSetVolume ?: return
// Playback is considered ongoing while playing or buffering to continue playing
@@ -347,6 +359,7 @@ class LocalPlayer(
!playbackOngoing && lastSavedDeviceVolume != null -> {
player.setDeviceVolume(lastSavedDeviceVolume!!, 0)
lastSavedDeviceVolume = null
+ lastSetDeviceVolume = null
}
}
}
@@ -355,6 +368,7 @@ class LocalPlayer(
val maxVolume = player.deviceInfo.maxVolume.takeIf { it > 0 } ?: return
val volumeAsInt = (volume * maxVolume).roundToInt()
player.setDeviceVolume(volumeAsInt, 0)
+ lastSetDeviceVolume = volumeAsInt
}
@OptIn(UnstableApi::class)That way the server knows about the volume change and we don't need to fight its logic. |
|
Mirroring the volume to the server instead of adopting it locally is the better direction — but two things still stand in the way: 1. To push back losslessly the app has to invert the table: device fraction 0.333 is 2. The fades are still there On your diff
If you want to go that way, I can rework the branch: push-back with the inverted gain mapping plus a shrunken fade guard. Or take it from here, whichever you prefer. |
|
I think working off my diff + the missing fixes is the better idea. Let's see what the outcome of that one looks like. |
|
On another thought, I wonder whether setting the volume to fixed 100% for the local player on the server side wouldn't be the better solution for this specific use case. That way the in-app volume control is disabled and it's only the device volume that controls the playback volume (outside of the app's control). |
Problem
When using the local player with a Bluetooth car head unit, volume set with the car's volume buttons was reset on every new track, back to the value that had last been set from the app/server side.
Root cause
Two separate places re-applied the server's volume:
audg) on every stream start, andLocalPlayerapplied it again, overwriting the car's change.updatePlayerVolume(), which inDevice while playingmode applies the app's volume to the device volume.A device volume change done by the head unit (AVRCP absolute volume) doesn't come from the app, so it was neither tracked nor adopted - it was simply lost at the next re-application.
Fix
LocalPlayer.onDeviceVolumeChanged()now adopts external device volume changes as the new desired volume. Changes caused by the app itself are filtered out via the trackedlastAppliedDeviceVolume;lastSavedDeviceVolumeis updated as well, so the "restore volume when playback ends" path (device-while-playing mode) does not undo the change.LocalPlayer.volumesetter: a server volume identical to the last announced one is not applied again - that's exactly LMS's per-stream re-send. Tolerance is 1/1000; real LMS steps are >= ~0.009 gain (1%) and the unmute ramp is ~0.02, so no legitimate change is skipped.Coverage of the volume controls (all paths audited)
VolumeFragment->ConnectionHelper.setVolume->audgMainActivity.onKeyDown->VolumeFragment.handleKeyDown-> serverMediaService.handleSetDeviceVolume/ `handleIncreasePlayerManagementActivity-> serversetMuteState-> LMS sends gain 0 / restores on unmuteAudioGain(0.0), then ramp back up)onDeviceVolumeChangedDevice while playing)In-app onlymodeUse device volumemodeKnown limitation
A car-side change is not reported back to the server (the local player has no Cometd connection to publish a volume request), so the app's volume slider keeps showing the previous server value until volume is changed there; a change made there then takes precedence. Reporting it back would require inverting LMS's volume->gain curve (measured: server 40 -> device 4, 50 -> 5, 80 -> 11); a naive percent round-trip would snap the device volume onto LMS's curve on every car adjustment.
Testing
Debug build on a Samsung device (Android 16), local player,
Device while playingmode:In-app onlymode unchangedNo unit test added: the logic lives in the media3 player wrapper (device volume callback + playback state machine) and was verified on-device instead.