Add group mute support to volume controls#1404
Add group mute support to volume controls#1404scyto wants to merge 5 commits intomusic-assistant:mainfrom
Conversation
|
demo docker image |
4bf694e to
ad268db
Compare
|
backend PR now merged |
feff91a to
e7acd72
Compare
|
@marcelveldt updated to fix upstream bug and rebase this PR fork/branch with latest frontend changes as alwasy built and tested, dev image is at |
e7acd72 to
628b6eb
Compare
|
@stvncode i used your feedback to help me think about how to push claude to help me be better, i hope this addresses what you were getting at, this approach seems simpler to me and removed 15 lines of duplicative and custom code from my original PR. these commits have been tested on my live (patched) production system Commit 1: Deduplicate isGroupMuted and fix mute toggle logicProblem:isGroupMuted was copy-pasted in two components (VolumeControl.vue as a function, VolumeBtn.vue as a computed), and playerCommandGroupMuteToggle in the API class used !leader.volume_muted to decide mute/unmute — which could disagree with the icon that used the all-members isGroupMuted check. Fix:Extracted isGroupMuted to a single shared function in helpers.ts Commit 2: Make getVolumeIconComponent group-mute awareProblem:Every caller of getVolumeIconComponent had to know about group mute and either pass isGroupMuted(player) explicitly or duplicate the icon-selection logic entirely (as VolumeBtn did with 15 lines of hand-rolled code). Player.vue didn't pass it at all — a bug. Fix:Changed getVolumeIconComponent in utils.ts to fall back to isGroupMuted(player) instead of just player.volume_muted when no explicit isMuted override is passed. This is backward-compatible since isGroupMuted returns !!player.volume_muted for non-group players. |
|
@scyto Thanks for the changes, this looks better 😃 |
- Use server's group_volume_mute API (PR #3034) instead of looping through individual members - isGroupMuted() shows muted state only when ALL group members are muted - Group volume slider disabled only when entire group is muted - Child player sliders remain interactive when muted (server uses ATTR_MUTE_LOCK) - Bottom bar mute icon reflects aggregate group mute state - Fix main volume slider not calling API (upstream regression from bb0dbf9) Co-Authored-By: Claude Opus 4.6 <[email protected]>
Extract isGroupMuted to shared helper in helpers.ts to remove duplication between VolumeControl.vue and VolumeBtn.vue. Fix mute toggle using leader's volume_muted flag instead of the all-members isGroupMuted check, which caused the toggle action to disagree with the displayed icon state. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Move group mute check into getVolumeIconComponent so callers no longer need to pass isGroupMuted explicitly. This removes duplicated icon logic from VolumeBtn and fixes Player OSD not reflecting group mute. Co-Authored-By: Claude Opus 4.6 <[email protected]>
c6df3b9 to
13ec301
Compare
- Pass isGroupMuted(player) explicitly to getVolumeIconComponent so the icon reflects aggregate group mute state correctly - Pass individual child mute state (!!childPlayer.volume_muted) for child player icons instead of computing group state - Skip members with null volume_muted in isGroupMuted (players that don't support mute control) - Only disable mute button for non-group players when mute_control is PLAYER_CONTROL_NONE Co-Authored-By: Claude Opus 4.6 <[email protected]>
Status update — group mute now fully workingAfter rebasing onto the latest upstream, group mute broke for protocol-synced players (e.g., Sendspin). The root cause was on the server side: Frontend changes in this push (commit 5f04cd7):
Server fix required:
Both PRs are needed for full group mute functionality with protocol-synced players. |
|
@stvncode sorry about that churn some of the other work going on by marcel broke me when i rebased and it took a while for me to get claude to figure it out I built and tested a complete docker image This was the final analysis ME: was protocol synced players the new upstream feature that broke us? Yes, exactly. The upstream protocol linking refactor changed the contract for how group members are accessed: Before the refactor: player.group_members (raw) contained all members directly, so your original cmd_group_volume_mute code worked fine. After the refactor: For protocol-synced players (like your Sendspin setup), raw group_members can be empty. The real member list now lives in player.state.group_members, which runs through __final_group_members to translate protocol player IDs to visible player IDs. The upstream code was updated accordingly — cmd_group_volume already uses player.state.group_members. But since cmd_group_volume_mute was your new code in the PR, it didn't get that update during the refactor. After rebasing onto the refactored code, the mismatch surfaced. ME: will group mute still work for non-protocol synced players? Yes. The player.state.group_members property is a superset of player.group_members — it includes everything from the raw property plus protocol-translated members. For non-protocol-synced players, the __final_group_members logic in player.py essentially passes through the raw group_members unchanged (since there are no protocol IDs to translate). So player.state.group_members returns the same list as player.group_members. This is also confirmed by the fact that cmd_group_volume (the volume slider for groups) has been using player.state.group_members all along and works for all player types — our fix just makes cmd_group_volume_mute consistent with that existing pattern. |
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Summary
group_volume_muteserver API instead of looping through individual membersATTR_MUTE_LOCKto preserve mute state during group volume changes)playerCommandGroupVolumeMuteandplayerCommandGroupMuteToggleAPI methodsContext
This is the frontend counterpart to music-assistant/server#3034 which adds the
cmd_group_volume_muteAPI endpoint andATTR_MUTE_LOCKattribute.The maintainer's commit d160077 added a frontend-only workaround that loops through group members calling individual mute, with a TODO: "revisit this when api/server supports group mute toggle". This PR replaces that workaround with a single server API call.
Note: This PR also fixes an upstream regression where the main volume slider's
@update:model-valuehandler was reduced to justmainDisplayVolume = $event(no API call), making the volume slider a visual-only no-op. The fix restores the API call and makes it group-aware.Behavior changes
Files changed
isGroupMuted()helper, group-aware mute icon/slider logic, volume slider API fixplayerCommandGroupVolumeMuteandplayerCommandGroupMuteToggleAPI methodshandlePlayerMuteToggleuses single group API call instead of member loopgetVolumeIconComponentaccepts optionalisMutedoverride parameterDependencies
Requires music-assistant/server#3034 for the
group_volume_muteserver API.Test plan
🤖 Generated with Claude Code