Skip to content

Fix announcements on a speaker group playing out of sync - #6392

Merged
marcelveldt merged 4 commits into
devfrom
marcelveldt/synced-announcements-groups-f9c20b
Sep 19, 2026
Merged

marcelveldt merged 4 commits into
devfrom
marcelveldt/synced-announcements-groups-f9c20b

Conversation

@marcelveldt

@marcelveldt marcelveldt commented Sep 18, 2026

Copy link
Copy Markdown
Member

What does this implement/fix?

Announcements sent to a group of speakers that announce natively (for example a sync group of Home Assistant Voice speakers) were handed to each speaker separately. Those speakers have no way to line up their start, so the rooms played the announcement noticeably out of step, which makes it hard to understand.

A group now only hands the announcement to the individual speakers when all of them can start it together. Otherwise the group plays the clip through its own synchronized stream, as it did before these speakers gained native announcements.

  • New coordinates_announcement_start hook on the player model, off by default
  • AirPlay and Sonos players report it, so their groups keep the native per-speaker announcement
  • Speakers that announce through Home Assistant (ESPHome Sendspin players, HA media players), Snapcast, Bose and Samsung do not, so a group of those plays the announcement in sync through the group stream (music stops and resumes)
  • A group mixing speakers from different providers also uses the group stream, since only speakers of one provider start together

Related issue (if applicable):

Types of changes

  • Bugfix (non-breaking change which fixes an issue) — bugfix
  • New feature (non-breaking change which adds functionality) — new-feature
  • Enhancement to an existing feature — enhancement
  • New music/player/metadata/plugin provider — new-provider
  • Breaking change (fix or feature that would cause existing functionality to not work as expected) — breaking-change
  • Refactor (no behaviour change) — refactor
  • Documentation only — documentation
  • Maintenance / chore — maintenance
  • CI / workflow change — ci
  • Dependencies bump — dependencies

Checklist

  • The code change is tested and works locally.
  • pre-commit run --all-files passes.
  • pytest passes, and tests have been added/updated under tests/ where applicable.
  • For changes to shared models, the companion PR in music-assistant/models is linked.
  • For changes affecting the UI, the companion PR in music-assistant/frontend is linked.
  • I have read and complied with the project's AI Policy for any AI-assisted contributions.
  • I have raised a PR against the documentation repository targeting the main or beta branch as appropriate.

A group only hands an announcement to each speaker when those speakers can
start it together. Otherwise the group plays the clip through its own
synchronized stream, so all rooms are heard as one. AirPlay and Sonos speakers
start together; speakers that announce through Home Assistant do not.
Copilot AI balanced review requested due to automatic review settings September 18, 2026 22:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Mixed-provider and unavailable-member groups can still select an unsafe fan-out path.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity

Open (1)
What changed in this PR

Fixes out-of-sync group announcements by limiting native fan-out to players reporting coordinated starts.

Changes:

  • Adds a default-off coordination capability.
  • Enables it for AirPlay and Sonos.
  • Adds routing regression tests.
File Description
tests/​providers/​sonos/​test_player.py Tests Sonos coordination support.
tests/​providers/​airplay/​test_player.py Tests AirPlay coordination support.
tests/​controllers/​players/​test_player_controller.py Tests group announcement routing.
music_assistant/​providers/​sonos/​player.py Enables coordination for Sonos.
music_assistant/​providers/​airplay/​player.py Enables coordination for AirPlay.
music_assistant/​providers/​_demo_player_provider/​player.py Documents the provider capability.
music_assistant/​models/​player.py Defines the default capability.
music_assistant/​controllers/​players/​announcements.py Gates native group fan-out.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread music_assistant/models/player.py Outdated
…uncement

A member that announces through a linked protocol (e.g. an AirPlay child) is
judged by that output, so such groups keep the in-sync overlay.
@github-actions

Copy link
Copy Markdown
Contributor

🚧 Automated critical-issue gate — this PR has an unresolved [CRITICAL] finding from the automated review, so it is kept as a draft (a draft PR can't be merged) until that is resolved. Please address the [CRITICAL] thread(s), then mark it Ready for review.

Maintainers: add the override-critical label to bypass this.

Copilot AI review requested due to automatic review settings September 19, 2026 09:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Mixed providers and Sendspin-bridged AirPlay outputs can still take the unsynchronized fan-out path.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 3 High severity

Open (3)

Comment thread music_assistant/controllers/players/announcements.py Outdated
Comment thread music_assistant/providers/airplay/player.py
…rovider

Members line up their start with each other only within the same provider, so a
group mixing providers plays the clip through its own stream.
Copilot AI review requested due to automatic review settings September 19, 2026 09:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Same-provider independent sessions and capability changes during rendering can still produce unsynchronized playback.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 3 High severity

Open (3)
Resolved since last review (2)

Comment thread music_assistant/controllers/players/announcements.py Outdated
Comment thread music_assistant/controllers/players/announcements.py
The outputs that announce for the members are resolved on the same state the members act on, so a group never splits between native and fallback playback.
Copilot AI review requested due to automatic review settings September 19, 2026 10:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Readiness is awaited repeatedly, and single-member groups unnecessarily lose native announcement handling.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
Resolved since last review (3)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Allow single-output groups without requiring synchronized playback capability

music_assistant/​controllers/​players/​announcements.py:943

[PROBLEM] A one-member group whose only native output leaves this capability at the default now uses generic stop/play/resume, even though there is no second start to synchronize. Resolve the native outputs first and treat one output as coordinated automatically; require the capability and shared provider only when there are multiple outputs.

Comment thread music_assistant/controllers/players/announcements.py
@marcelveldt
marcelveldt marked this pull request as ready for review September 19, 2026 11:00
@marcelveldt
marcelveldt merged commit 23fa0d2 into dev Sep 19, 2026
18 checks passed
@marcelveldt
marcelveldt deleted the marcelveldt/synced-announcements-groups-f9c20b branch September 19, 2026 11:00
marcelveldt added a commit that referenced this pull request Sep 19, 2026
Announcements sent to a group of speakers that announce natively (for
example a sync group of Home Assistant Voice speakers) were handed to
each speaker separately. Those speakers have no way to line up their
start, so the rooms played the announcement noticeably out of step,
which makes it hard to understand.

A group now only hands the announcement to the individual speakers when
all of them can start it together. Otherwise the group plays the clip
through its own synchronized stream, as it did before these speakers
gained native announcements.

- New `coordinates_announcement_start` hook on the player model, off by
default
- AirPlay and Sonos players report it, so their groups keep the native
per-speaker announcement
- Speakers that announce through Home Assistant (ESPHome Sendspin
players, HA media players), Snapcast, Bose and Samsung do not, so a
group of those plays the announcement in sync through the group stream
(music stops and resumes)
- A group mixing speakers from different providers also uses the group
stream, since only speakers of one provider start together

**Related issue (if applicable):**

- related issue music-assistant/support#6273

<!--
Tick exactly one box. CI (.github/workflows/pr-labels.yaml) derives
the label from the ticked box and applies it automatically; the
release-notes generator uses that same label to slot this change
into the next release notes.
-->

- [x] Bugfix (non-breaking change which fixes an issue) — `bugfix`
- [ ] New feature (non-breaking change which adds functionality) —
`new-feature`
- [ ] Enhancement to an existing feature — `enhancement`
- [ ] New music/player/metadata/plugin provider — `new-provider`
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected) — `breaking-change`
- [ ] Refactor (no behaviour change) — `refactor`
- [ ] Documentation only — `documentation`
- [ ] Maintenance / chore — `maintenance`
- [ ] CI / workflow change — `ci`
- [ ] Dependencies bump — `dependencies`

- [x] The code change is tested and works locally.
- [x] `pre-commit run --all-files` passes.
- [x] `pytest` passes, and tests have been added/updated under `tests/`
where applicable.
- [ ] For changes to shared models, the companion PR in
`music-assistant/models` is linked.
- [ ] For changes affecting the UI, the companion PR in
`music-assistant/frontend` is linked.
- [x] I have read and complied with the project's [AI
Policy](https://github.com/music-assistant/.github/blob/main/AI_POLICY.md)
for any AI-assisted contributions.
- [ ] I have [raised a PR against the documentation
repository](https://github.com/music-assistant/music-assistant.io/blob/main/CONTRIBUTING.md)
targeting the main or beta branch as appropriate.
marcelveldt added a commit that referenced this pull request Sep 20, 2026
# What does this implement/fix?

Speakers whose native announcement is relayed through Home Assistant
(ESPHome Voice PE, and HA-relayed Sendspin) ignore the announcement
volume for a solo announcement. But when those same speakers are
announced to as a sync group, the announcement falls back to our own
synchronized stream, and each member got a hidden volume bump it
couldn't see or turn off.

This makes the behaviour consistent. The per-player announcement hints
(volume support, self-applied volume, coordinated start) now live in one
server-side `AnnouncementFeature` set on the player. A speaker whose
native route ignores the level defaults to no adjustment, so there's no
surprise bump. Choosing a volume strategy, or passing an explicit
volume, routes the announcement through the builtin path which applies
the level via the device volume, so it works the same solo and grouped.

**Related issue (if applicable):**

- Follow-up to #6392 (music-assistant/support#6273)

## Types of changes

- [ ] Bugfix (non-breaking change which fixes an issue) — `bugfix`
- [ ] New feature (non-breaking change which adds functionality) —
`new-feature`
- [x] Enhancement to an existing feature — `enhancement`
- [ ] New music/player/metadata/plugin provider — `new-provider`
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected) — `breaking-change`
- [ ] Refactor (no behaviour change) — `refactor`
- [ ] Documentation only — `documentation`
- [ ] Maintenance / chore — `maintenance`
- [ ] CI / workflow change — `ci`
- [ ] Dependencies bump — `dependencies`

## Checklist

- [x] The code change is tested and works locally.
- [x] `pre-commit run --all-files` passes.
- [x] `pytest` passes, and tests have been added/updated under `tests/`
where applicable.
- [ ] For changes to shared models, the companion PR in
`music-assistant/models` is linked.
- [ ] For changes affecting the UI, the companion PR in
`music-assistant/frontend` is linked.
- [x] I have read and complied with the project's [AI
Policy](https://github.com/music-assistant/.github/blob/main/AI_POLICY.md)
for any AI-assisted contributions.
- [ ] I have [raised a PR against the documentation
repository](https://github.com/music-assistant/music-assistant.io/blob/main/CONTRIBUTING.md)
targeting the main or beta branch as appropriate.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants