[REVIEW] fix(audio): keep stream open + harden stale-format races

.github/PULL_REQUEST_TEMPLATE.md

@@ -0,0 +1,24 @@
+# What does this implement/fix?
+
+<!-- Quick description and explanation of changes. -->
+
+**Related issue (if applicable):**
+
+- <link>
+
+## Types of changes
+
+<!--
+Tick exactly one box. CI (.github/workflows/pr-labels.yaml) applies the
+matching label, and the release notes use it to sort this change.
+-->
+
+- [ ] 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`
+- [ ] Breaking change (changes CLI options, settings, hooks or the protocol in a way that is not backwards compatible): `breaking-change`
+- [ ] Refactor (no behavior change): `refactor`
+- [ ] Documentation only: `documentation`
+- [ ] Maintenance / chore: `maintenance`
+- [ ] CI / workflow change: `ci`
+- [ ] Dependencies bump: `dependencies`

.github/release-drafter.yml

@@ -12,19 +12,20 @@ categories:
 
   - title: "🚀 Features"
     labels:
-      - "feature"
+      - "new-feature"
       - "enhancement"
 
   - title: "🧰 Maintenance"
     labels:
+      - "refactor"
+      - "documentation"
       - "ci"
       - "maintenance"
 
-  - title: "⬆️ Dependencies and CI"
+  - title: "⬆️ Dependencies"
     collapse-after: 1
     labels:
       - "dependencies"
-      - "ci"
 template: |
   ## What’s Changed
 

.github/workflows/pr-labels.yaml

@@ -0,0 +1,95 @@
+---
+name: PR Labels
+
+# Each PR must carry exactly one of the labels the release notes sort by.
+# The label comes from the ticked "Types of changes" checkbox in the PR
+# template, since external contributors can't label their own PRs.
+
+# yamllint disable-line rule:truthy
+on:
+  pull_request_target:
+    types:
+      - opened
+      - edited
+      - synchronize
+      - labeled
+      - unlabeled
+      - reopened
+      - ready_for_review
+    branches:
+      - main
+
+permissions:
+  pull-requests: write
+
+jobs:
+  pr_labels:
+    name: Verify
+    runs-on: ubuntu-latest
+    # Bots like Dependabot don't use the template and label their own PRs.
+    if: github.event.pull_request.user.type != 'Bot'
+    steps:
+      - name: 🏷 Apply label from PR description checkbox
+        uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3  # v9.0.0
+        with:
+          script: |
+            const known = [
+              'breaking-change', 'bugfix', 'new-feature', 'enhancement',
+              'refactor', 'documentation', 'maintenance', 'ci', 'dependencies',
+            ];
+
+            const { owner, repo } = context.repo;
+            const issue_number = context.payload.pull_request.number;
+            const current = (context.payload.pull_request.labels || [])
+              .map(l => l.name);
+
+            // Match a ticked bullet whose first backticked word is a label,
+            // e.g.  - [x] CI / workflow change: `ci`
+            const body = context.payload.pull_request.body || '';
+            const re = /^\s*-\s*\[x\][^`\n]*`([^`]+)`/gim;
+            const ticked = [];
+            for (const m of body.matchAll(re)) {
+              const name = m[1].trim();
+              if (known.includes(name) && !ticked.includes(name)) {
+                ticked.push(name);
+              }
+            }
+
+            if (ticked.length > 1) {
+              core.setFailed(
+                `Multiple "Types of changes" checkboxes are ticked ` +
+                `(${ticked.join(', ')}). Tick exactly one.`
+              );
+              return;
+            }
+
+            // A ticked box wins. Without one, accept a single known label
+            // already on the PR, for PRs opened without the template.
+            if (ticked.length === 0) {
+              const existing = current.filter(n => known.includes(n));
+              if (existing.length === 1) {
+                core.info(`Skipping: PR already labelled '${existing[0]}'.`);
+                return;
+              }
+              core.setFailed(
+                'No "Types of changes" checkbox is ticked in the PR ' +
+                'description. Edit the description and tick exactly ' +
+                'one box so this PR can be release-noted.'
+              );
+              return;
+            }
+
+            const desired = ticked[0];
+
+            for (const name of current) {
+              if (known.includes(name) && name !== desired) {
+                await github.rest.issues.removeLabel({
+                  owner, repo, issue_number, name,
+                }).catch(() => {});
+              }
+            }
+            if (!current.includes(desired)) {
+              await github.rest.issues.addLabels({
+                owner, repo, issue_number, labels: [desired],
+              });
+            }

README.md

@@ -85,6 +85,13 @@ uv tool upgrade sendspin
 sudo systemctl restart sendspin
 ```
 
+The installer may create a dedicated `sendspin` service account. In that case, the package is installed in that user's `uv` environment, not in your own shell. Upgrade the package as that user instead:
+
+```bash
+sudo -u sendspin bash -l -c "uv tool upgrade sendspin"
+sudo systemctl restart sendspin
+```
+
 Your client name, audio device selection, and other settings in `~/.config/sendspin/settings-daemon.json` are preserved during the update.
 
 > **Note:** You do **not** need to uninstall and reinstall when updating. Your configuration (client name, audio device, delay settings) is stored separately and will be preserved.
@@ -114,6 +121,7 @@ Settings are stored in `~/.config/sendspin/`:
   "listen_port": 8927,
   "use_mpris": true,
   "use_hardware_volume": true,
+  "unpaired_access": true,
   "hook_set_volume": "/usr/local/bin/set-avr-volume",
   "manufacturer": "Acme Corp",
   "product_name": "Living Room Speaker",
@@ -147,6 +155,7 @@ Settings are stored in `~/.config/sendspin/`:
 | `listen_port` | integer | daemon/serve | Listen port (`--port`, default: 8927) |
 | `use_mpris` | boolean | TUI/daemon | Enable MPRIS integration (default: true) |
 | `use_hardware_volume` | boolean | TUI/daemon | Control hardware/system output volume instead of software volume (`--hardware-volume true/false`). Default: on for daemon (if available), off for TUI |
+| `unpaired_access` | boolean | TUI/daemon | Allow servers to play on this client without pairing (`--unpaired-access true/false`, default: true). See [Pairing](#pairing) |
 | `hook_set_volume` | string | TUI/daemon | Script to run for external volume control (`--hook-set-volume`). Receives the effective volume 0-100 as the last argument |
 | `hook_start` | string | TUI/daemon | Command to run when audio stream starts |
 | `hook_stop` | string | TUI/daemon | Command to run when audio stream stops |
@@ -185,13 +194,19 @@ same server don't require pairing again.
   pairing completes.
 - **Daemon**: the PIN is only logged (`Pairing required: enter PIN ...`) — there's no other
   surface for it. You need to be watching the daemon's output at the moment it first
-  connects to a new server. Under systemd, that means `journalctl -u sendspin -f`. There's
-  no unattended-pairing option today (no PIN written to a file, no QR code); for a headless
-  install, plan to watch the log during first setup for each new server.
+  connects to a new server. Under systemd, that means `journalctl -u sendspin -f`. For a
+  headless install with unpaired access turned off, plan to watch the log during first setup
+  for each new server.
 
 If a pairing attempt fails (e.g. a mismatched PIN), the client logs the reason and you
 can retry by reconnecting.
 
+Unpaired access is on by default, so servers can play on this client without a PIN once
+the server approves it (Music Assistant does this automatically). Unpaired connections are
+still encrypted, but another device on the network could impersonate either side. To
+require pairing, use `--unpaired-access false` (or `"unpaired_access": false` in the
+settings file).
+
 **Upgrading from a version before pairing support:** this client's protocol identity
 (`client_id`) used to be a stable, often user-chosen string; it's now derived from a
 generated cryptographic identity instead, so upgrading a previously-configured install

sendspin/audio.py

@@ -253,6 +253,26 @@ def set_format(self, audio_format: AudioFormat, device: AudioDevice) -> None:
             device: Audio device to use.
         """
         pcm_format = audio_format.pcm_format
+        if self._stream is not None and self._format == pcm_format:
+            logger.debug(
+                "Audio format unchanged (%dHz/%d-bit/%dch); keeping stream open",
+                pcm_format.sample_rate,
+                pcm_format.bit_depth,
+                pcm_format.channels,
+            )
+            return
+        # Stop the old stream synchronously BEFORE swapping the format. The
+        # close below is async, and an in-flight old-stream callback computing
+        # against the new format sizes its buffer wrong and crashes
+        # (ValueError: memoryview assignment, seen live on resume-after-
+        # announce). stop() blocks until in-flight callbacks finish, and this
+        # runs on the worker thread, never in a callback, so no deadlock.
+        # _call_stream logs internally. Also drop the last-frame cache: it
+        # carries the old frame size into the slow correction path.
+        old_stream = self._stream
+        if old_stream is not None:
+            self._call_stream(old_stream.stop)
+        self._last_output_frame = b""
         self._format = pcm_format
         self._close_stream()
 
@@ -340,17 +360,16 @@ def stop(self) -> None:
         self._stream_executor.shutdown(wait=True)
 
     def close_stream(self) -> None:
-        """Drop queued audio and fully close the stream to release the device.
+        """Drop queued audio but keep the stream open on silence.
 
-        Unlike clear(), which only stops the stream (leaving the device FD open),
-        this fully closes the PortAudio stream. Call when the server signals
-        end-of-stream; the stream will be recreated by set_format() when the
-        next track begins.
+        The callback renders silence while starved, so the audio device
+        stays open and DACs hold lock across pause/resume and across
+        stream end/start pairs. The device is released by stop()
+        (shutdown) and by set_format() when the format actually changes.
         """
         if self._closed:
             return
         self.clear()
-        self._close_stream()
 
     def clear(self) -> None:
         """Drop all queued audio chunks."""
@@ -359,13 +378,11 @@ def clear(self) -> None:
         # Clear deferred operation flag
         self._clear_requested = False
 
-        # Stop the audio stream (but don't close it) to release ALSA device
-        # This allows the device to transition to 'closed' state when paused.
-        # The stream restarts when new chunks arrive in submit().
-        self._stream_started = False
-        stream = self._stream
-        if stream is not None:
-            self._stream_executor.submit(self._call_stream, stream.stop)
+        # Keep the audio stream running: with the queue drained the callback
+        # renders silence, so the device stays open and DACs that click a
+        # muting relay on lock loss (e.g. XMOS USB) stay silent across
+        # pause/resume/seek. Chunks submitted after the clear play on the
+        # still-running stream; submit() restarts it only if never started.
 
         # Drain all queued chunks
         while True:
@@ -428,8 +445,14 @@ def _audio_callback(  # noqa: PLR0915
 
         assert self._format is not None
 
-        bytes_needed = frames * self._format.frame_size
         output_buffer = memoryview(outdata).cast("B")
+        # Trust the buffer, not the parameters: a stale callback racing a
+        # format switch can arrive with frames sized for another format.
+        # Every slice below derives from bytes_needed, so bounding it here
+        # bounds them all (bulk reads are capped separately to keep cursor
+        # accounting consistent).
+        got = frames * self._format.frame_size
+        bytes_needed = got if got < len(output_buffer) else len(output_buffer)
 
         if status:
             # Detect underflow and request re-anchor (processed by main thread)
@@ -480,6 +503,22 @@ def _audio_callback(  # noqa: PLR0915
             else:
                 frame_size = self._format.frame_size
 
+                # Hard guarantee: never read more frames than fit the buffer.
+                # A stale callback racing a format switch would otherwise size
+                # its read from the new format against the old buffer and crash
+                # (ValueError: memoryview assignment, seen live). Capped reads
+                # degrade to a short trailing silence fill instead.
+                max_frames = (
+                    (len(output_buffer) - bytes_written) // frame_size if frame_size > 0 else 0
+                )
+                if frames > max_frames:
+                    logger.debug(
+                        "Capping stale callback read: %d frames to %d",
+                        frames,
+                        max(0, max_frames),
+                    )
+                    frames = max(0, max_frames)
+
                 # Thread-safe snapshot of correction schedule (prevent mid-callback changes)
                 insert_every_n = self._insert_every_n_frames
                 drop_every_n = self._drop_every_n_frames
@@ -567,6 +606,12 @@ def _audio_callback(  # noqa: PLR0915
                     self._frames_until_next_insert = insert_counter
                     self._frames_until_next_drop = drop_counter
 
+            # Trailing guard: never hand PortAudio uninitialized bytes. A
+            # capped stale read above can come up short; pad the tail.
+            if bytes_written < bytes_needed:
+                self._fill_silence(output_buffer, bytes_written, bytes_needed - bytes_written)
+                bytes_written = bytes_needed
+
         except Exception:
             logger.exception("Error in audio callback")
             # Fill rest with silence on error
@@ -907,9 +952,13 @@ def _smooth_sync_error(self, error_us: int) -> None:
         self._sync_error_filtered_us = self._sync_error_filter.offset
 
     def _fill_silence(self, output_buffer: memoryview, offset: int, num_bytes: int) -> None:
-        """Fill output buffer range with silence."""
-        if num_bytes > 0:
-            output_buffer[offset : offset + num_bytes] = b"\x00" * num_bytes
+        """Fill output buffer range with silence, trimmed to the buffer."""
+        end = offset + num_bytes
+        buf = len(output_buffer)
+        if end > buf:
+            end = buf
+        if end > offset:
+            output_buffer[offset:end] = b"\x00" * (end - offset)
 
     def _apply_volume(self, output_buffer: memoryview) -> None:
         """

sendspin/audio_devices.py

@@ -90,7 +90,12 @@ def _check_format(device: AudioDevice, rate: int, channels: int, dtype: str) ->
             dtype=dtype,
         )
         return True
-    except sounddevice.PortAudioError:
+    except (sounddevice.PortAudioError, ValueError):
+        # PortAudio raises ValueError (not PortAudioError) for device names it
+        # does not recognise, e.g. raw ALSA names such as hw:CARD=...,DEV=... .
+        # The device itself is valid -- _try_alsa_device() already accepted it --
+        # so treat an unprobeable format as unsupported here and let the caller
+        # fall back to its safe defaults instead of failing at startup.
         return False