Repository navigation
Add --audio-format: pin a preferred format, refuse a device that cannot take it - #31
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds preferred audio-format pinning, alongside stacked stream-hook and client-identity changes.
Changes:
- Parses, validates, and prioritizes
--audio-format. - Adds stream lifecycle hooks and configurable client identity.
- Updates tests, documentation, configuration examples, and build targets.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/supported_formats.{h,cpp} |
Parses and prioritizes preferred formats. |
src/cli.{h,cpp} |
Adds new CLI/config options. |
src/hooks.{h,cpp} |
Implements asynchronous stream hooks. |
src/player_listener.{h,cpp} |
Emits stream lifecycle events. |
src/main.cpp |
Wires formats, identity, and hooks. |
src/log.h |
Adds the hook log tag. |
tests/supported_formats_test.cpp |
Tests format parsing and pinning. |
tests/hooks_test.cpp |
Tests hook options and execution. |
tests/config_file_test.cpp |
Tests identity configuration. |
tests/cli_test.cpp |
Tests identity and format flags. |
CMakeLists.txt |
Builds hook implementation and tests. |
README.md |
Documents the new features. |
packaging/sendspin-cli.conf.example |
Adds example configuration. |
docs/wiki/Controlling-the-Player.md |
Documents stream hooks. |
docs/wiki/Configuration.md |
Lists new configuration keys. |
docs/ROADMAP.md |
Marks stacked features as shipped. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+931
to
+932
| if (outbound) { | ||
| context.server_url = outbound->dialed_url(); |
Comment on lines
+101
to
+102
| dup2(STDERR_FILENO, STDOUT_FILENO); | ||
| execve("/bin/sh", argv, envp.data()); |
|
|
||
| cli_log(LogLevel::DEBUG, "Running %s hook [%d]: %s", event, static_cast<int>(pid), | ||
| command.c_str()); | ||
| this->running_.push_back({pid, event, command}); |
Comment on lines
+216
to
+219
| out.sample_rate = static_cast<uint32_t>(rate); | ||
| out.bit_depth = static_cast<uint8_t>(depth); | ||
| out.channels = static_cast<uint8_t>(channels); | ||
| return true; |
Comment on lines
+108
to
+109
| cli_log(LogLevel::DEBUG, "Running %s hook [%d]: %s", event, static_cast<int>(pid), | ||
| command.c_str()); |
Comment on lines
+124
to
+129
| if (reaped == hook.pid && WIFEXITED(status) && WEXITSTATUS(status) != 0) { | ||
| cli_log(LogLevel::WARN, "The %s hook exited %d: %s", hook.event.c_str(), | ||
| WEXITSTATUS(status), hook.command.c_str()); | ||
| } else if (reaped == hook.pid && WIFSIGNALED(status)) { | ||
| cli_log(LogLevel::WARN, "The %s hook was killed by signal %d: %s", | ||
| hook.event.c_str(), WTERMSIG(status), hook.command.c_str()); |
…ot take it The device-derived, ranked advertisement (item 4) decides for the operator, and the case it cannot cover is the fussy DAC: a device that opens at many formats and is only actually happy in one. The Python CLI's --audio-format holds the player at that shape and exits at startup when the device cannot satisfy it; this is that flag, grammar included (codec:rate:depth:channels), extended with opus because this player decodes it. The pin is a reorder, not a narrowing: pin_preferred_format() moves the entry to the front of the derived advertisement -- the protocol has supported_formats in priority order, so the front is the whole of what "preferred" means on the wire -- and everything else the device takes is still offered behind it, so a server that cannot encode the pin has the rest of the list to fall back on. A pin the device cannot take refuses to start, naming the device, the format, and -l as the way to see what would be accepted. It is checked against the derived list -- or the permissive fallback when the device reported nothing, since that is what actually goes out -- so the refusal describes the real advertisement. The shape is settled at parse time in parse_format_spec() -- grammar, codec names, and the four bit depths any sink can emit -- so a config file is validated without opening a device; whether the device takes the format is answered at startup, where the sink is real. An opus spec is held to 48 kHz / 16-bit / at most two channels on that same side, and for a stronger reason than the depth check: those are constants at supported_formats()' push site rather than anything read off the device, so opus at any other shape is unreachable rather than merely unadvertised. Left to the device match, opus:44100:24:2 would parse, miss every entry in the derived list, and refuse to start naming the output device -- blaming it for a format no device could have been offered. Verified against a real aiosendspin server: with pcm:44100:16:2 pinned on a device whose ranked head is FLAC 48 kHz, the server encoded and streamed PCM at 44100 Hz.
chrisuthe
force-pushed
the
task/audio-format-pin
branch
from
August 28, 2026 02:41
9c94802 to
ac4eee3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the
--audio-formatgap from the daemon-parity audit: the device-derived, ranked advertisement decides for the operator, and the case it cannot cover is the fussy DAC — a device that opens at many formats and is only actually happy in one. The Python CLI holds the player at that shape and exits at startup when the device cannot satisfy it; this is that flag.What it adds
--audio-format <codec:rate:depth:channels>(config keyaudio-format), e.g.flac:48000:24:2. The grammar is the Python CLI's, extended withopusbecause this player decodes it.pin_preferred_format()moves the pinned entry to the front of the derived advertisement — the protocol hassupported_formatsin priority order, so the front is the whole of what "preferred" means on the wire — and everything else the device takes is still offered behind it, so a server that cannot encode the pin has the rest of the list to fall back on. (This matches the Python daemon, which inserts the preferred format at index 0 rather than dropping the rest.)-las the way to see what would be accepted. Checked against the derived list — or the permissive fallback when the device reported nothing, since that is what actually goes out.parse_format_spec()when the flag is read — so a config file is validated without opening a device — and whether the device takes the format is answered where the sink is real.Testing
aiosendspinserver: withpcm:44100:16:2pinned on a device whose ranked head is FLAC 48 kHz, the server encoded and streamed PCM at 44100 Hz — and a pin the device cannot take exits 1 with a singleE audio:line.Docs updated:
--help, README (flag list + a paragraph in "Buffering, and what gets advertised"), wikiConfiguration.mdkey table, the annotated example config, anddocs/ROADMAP.md(item 24).Review notes
Copilot raised six comments. One is this PR's and is fixed here:
parse_format_spec()accepted codec-impossible shapes, soopus:44100:24:2parsed and then failed the device match — blaming the device for a formatsupported_formats()can never emit on any device. It is now refused at parse time with the actual Opus constraint.The other five are against
src/hooks.cppandsrc/main.cpp, which merged via #28 and are not this PR's to change:SENDSPIN_SERVER_URLon an inbound connection during an-srun — already tracked in SENDSPIN_SERVER_URL can export a stale dialled URL for an inbound connection on an -s run #34, and documented indialed_url()as an upstream-library limitation: the library exposes neither a connect callback nor a connection's direction.execve()— stale againstmain, which already closes every descriptor above stderr in the child before the exec.