Merge pull request #38 from Sendspin/chrisuthe/task/reword-the-audio-format-refusal-to-name-the

Say the --audio-format pin missed the advertisement, not the device

Author: chrisuthe

README.md

@@ -641,15 +641,19 @@ device, the format, and the fact that the stream's audio is being discarded —
 rather than leaving a player that looks healthy and plays nothing.
 
 `--audio-format <codec:rate:depth:channels>` (e.g. `flac:48000:24:2`) pins a
-preferred format on top of that derived list — the way to hold a fussy DAC at the
-one shape it is happy in. It is a *reorder*, not a narrowing: the pinned entry
-moves to the front, which is where a spec-following server picks, and everything
-else the device takes is still offered behind it. A pin the device cannot take
-**refuses to start** — playing something else instead is the failure the flag
-exists to prevent — and `-l` shows what would be accepted. Codecs are `flac`,
-`opus` and `pcm`; the grammar is the Python `sendspin-cli`'s, plus `opus`. An
-`opus` pin is refused outright at anything but 48 kHz / 16-bit / at most two
-channels, since that is the only shape it is ever advertised in.
+preferred format on top of that derived list — the way to hold a fussy DAC at
+the one shape it is happy in. It is a *reorder*, not a narrowing: the pinned
+entry moves to the front, which is where a spec-following server picks, and
+everything else the device takes is still offered behind it. A pin that
+derived list does not carry **refuses to start** — playing something else
+instead is the failure the flag exists to prevent — and `-l` shows what the
+device itself reports, which is not the same set. The advertisement carries a
+single channel count — stereo where the device takes it, its narrowest count
+otherwise — so a mono or multichannel pin is refused on any device that also
+takes stereo. Codecs are `flac`, `opus` and `pcm`; the grammar is the Python
+`sendspin-cli`'s, plus `opus`. An `opus` pin is refused outright at anything
+but 48 kHz / 16-bit / at most two channels, since that is the only shape it is
+ever advertised in.
 
 ### Running as a daemon
 

docs/ROADMAP.md

@@ -2191,14 +2191,15 @@ exit on failure.
   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. 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.
-- **Parse-time shape validation, startup-time device validation.** The grammar, the
+- **A pin the advertisement does not carry refuses to start**, naming the format, the
+  device, and `-l` as the way to see what the device itself reports — not the same set as
+  what is advertised, which carries a single channel count. 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.
+- **Parse-time shape validation, startup-time advertisement validation.** The grammar, the
   codec names, the four emittable bit depths and the one shape Opus is ever advertised in
   are settled in `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
+  validated without opening a device — and whether the advertisement carries the format is
   answered where the sink is real.
 
 Verified against a real `aiosendspin` server: with `pcm:44100:16:2` pinned on a device

docs/wiki/Configuration.md

@@ -55,7 +55,7 @@ already does that.
 | `server` | `-s`, `--server` | `<host>[:<port>]`, a `ws://` URL, or `mdns:[<name>]` | none — wait to be discovered |
 | `port` | `--port` | the port this player's own WebSocket server listens on | `8928` |
 | `buffer-ms` | `--buffer-ms` | audio the output backend keeps queued, 10–2000 | `100` |
-| `audio-format` | `--audio-format` | pin a preferred format to the front of the advertised list: `codec:rate:depth:channels`, e.g. `flac:48000:24:2`; refuses to start if the device cannot take it, and an `opus` pin at anything but 48000/16 and at most 2 channels is refused outright | none — device-derived order |
+| `audio-format` | `--audio-format` | pin a preferred format to the front of the advertised list: `codec:rate:depth:channels`, e.g. `flac:48000:24:2`; refuses to start if the advertised list does not carry it — it carries a single channel count — and an `opus` pin at anything but 48000/16 and at most 2 channels is refused outright | none — device-derived order |
 | `static-delay` | `--static-delay` | latency this endpoint's hardware adds after the audio port, 0–5000 | `0` |
 | `no-mdns` | `--no-mdns` | `true`/`false` — do not advertise `_sendspin._tcp` | `false` |
 | `mdns-name` | `--mdns-name` | the instance label to advertise, when it should differ from `name` | `name` |

packaging/sendspin-cli.conf.example

@@ -51,7 +51,8 @@
 # Pin a preferred audio format -- codec:rate:depth:channels, codecs flac/opus/pcm -- for
 # the DAC that is only happy in one shape. Moves that entry to the front of the advertised
 # list, where a server picks first; the rest of what the device takes is still offered.
-# A format the device cannot take refuses to start; `sendspin-cli -l` shows what it accepts.
+# A format the advertised list does not carry refuses to start; `sendspin-cli -l` shows what
+# the device itself reports -- not the same set as what goes out.
 #audio-format = flac:48000:24:2
 
 # The port this player's own WebSocket server listens on, for a server that connects in.

src/cli.cpp

@@ -1133,8 +1133,9 @@ void print_usage(std::FILE* out, const char* prog) {
     std::fprintf(out, "                DAC that is only happy in one shape. Moves that entry to\n");
     std::fprintf(out, "                the front of the advertised list, where a server picks\n");
     std::fprintf(out, "                first; the rest of what the device takes is still\n");
-    std::fprintf(out, "                offered behind it. A format the device does not take\n");
-    std::fprintf(out, "                refuses to start -- run -l to see what it accepts\n");
+    std::fprintf(out, "                offered behind it. A format the advertised list does\n");
+    std::fprintf(out, "                not carry refuses to start -- run -l to see what the\n");
+    std::fprintf(out, "                device reports -- not the same set as what goes out\n");
     std::fprintf(out, "  --static-delay <ms>\n");
     std::fprintf(out, "                How much latency this endpoint's hardware adds AFTER the\n");
     std::fprintf(out, "                audio port -- an amplifier, an external speaker, a DSP.\n");

src/cli.h

@@ -214,8 +214,8 @@ struct Options {
     ///
     /// A *reorder*, not a narrowing: the pinned entry moves to the front of the advertised
     /// list, which is what "preferred" means on the wire, and everything the device takes is
-    /// still offered behind it. Parsing settles the shape here; whether the device really
-    /// takes it is answered at startup, where a pin the derived advertisement does not
+    /// still offered behind it. Parsing settles the shape here; whether the advertisement
+    /// carries it is answered at startup, where a pin the derived advertisement does not
     /// contain is a hard refusal to start -- playing something else instead is the failure
     /// this flag exists to prevent. Grammar and behaviour match the Python CLI's flag of the
     /// same name, extended with `opus`.

src/main.cpp

@@ -789,17 +789,20 @@ int main(int argc, char* argv[]) {
 
     std::vector<sendspin::AudioSupportedFormatObject> formats = advertised_formats(*sink);
     // The --audio-format pin, applied to the derived list because that is the promise being
-    // reordered: an entry in front of it is one the device really takes. A pin the list does
-    // not contain is a hard stop -- the operator asked for the one shape their DAC is happy
-    // in, and starting anyway would play everything except that. Against the *fallback* list
-    // when the device reported nothing, deliberately: that run advertises the permissive set,
-    // so the pin is checked against what actually goes out.
+    // reordered: an entry in front of it is one already going out. That list can be narrower
+    // than the device's own report -- advertised_channels() collapses the channels axis to one
+    // count -- so a pin missing from it is not necessarily one the device refuses. It is a
+    // hard stop either way: the operator asked for the one shape their DAC is happy in, and
+    // starting anyway would play everything except that. Against the *fallback* list when the
+    // device reported nothing, deliberately: that run advertises the permissive set, so the
+    // pin is checked against what actually goes out.
     if (opts.audio_format.has_value()) {
         if (!pin_preferred_format(formats, *opts.audio_format)) {
             log_fatal(LOG_TAG_AUDIO,
-                      "--audio-format asked for %s, which output device '%s' does not take -- "
-                      "refusing to start rather than play something else. Run with -l to see "
-                      "what the device accepts.",
+                      "--audio-format asked for %s, which is not among the formats advertised "
+                      "for output device '%s' -- refusing to start rather than play something "
+                      "else. Run with -l to see what the device itself reports -- not the "
+                      "same set as what gets advertised.",
                       describe_formats({*opts.audio_format}).c_str(), sink->name().c_str());
             return 1;
         }

src/supported_formats.h

@@ -67,7 +67,7 @@ std::vector<sendspin::AudioSupportedFormatObject> supported_formats(const SinkCa
 /// same reason and a stronger one: that is the only shape supported_formats() ever emits it in,
 /// whatever the device would take, so any other is unreachable rather than merely unadvertised.
 ///
-/// Only the *shape* is settled here. Whether the device actually takes the format is a
+/// Only the *shape* is settled here. Whether the advertisement carries the format is a
 /// property of the host, answered at startup by pin_preferred_format() against the derived
 /// advertisement -- the split that lets a config file be validated without opening a device.
 /// @param error Set to the reason when false comes back, without the flag's name -- the
@@ -84,9 +84,10 @@ bool parse_format_spec(const std::string& spec, sendspin::AudioSupportedFormatOb
 /// the front is the whole of what "preferred" means on the wire.
 ///
 /// Absence is the caller's to act on, and the intended action is to refuse to start: an
-/// operator who pinned a format their device cannot take asked for something this player
-/// cannot do, and playing something else instead is the failure --audio-format exists to
-/// prevent.
+/// operator who pinned a format the advertisement does not carry asked for something no
+/// conforming server will send, and playing something else instead is the failure
+/// --audio-format exists to prevent. Absence is not proof the device refuses the format --
+/// the advertisement is narrowed on the channels axis before it gets here.
 /// @return true if `preferred` was found (and is now first).
 bool pin_preferred_format(std::vector<sendspin::AudioSupportedFormatObject>& formats,
                           const sendspin::AudioSupportedFormatObject& preferred);