From 1f53fbd27144193f0bc7f9790b83cc971d029394 Mon Sep 17 00:00:00 2001 From: Kevin Ahrendt Date: Tue, 13 Oct 2026 10:56:49 -0400 Subject: [PATCH 1/2] Let the player withdraw the server's volume and mute commands Adds PlayerRole::set_volume_adjustable() and set_mute_adjustable(), enabled by default. While one is disabled, its command leaves client/state supported_commands, its value is omitted, and a server command for it is ignored; the stored value is kept. The spec lets a player change supported_commands mid-session, so no reconnect is needed. The basic and TUI examples gain -F (the server cannot set the volume, which starts at full scale) and -M (the server cannot set mute). Closes #82. --- docs/integration-guide.md | 7 +- examples/basic_client/README.md | 1 + examples/basic_client/main.cpp | 19 ++++- examples/tui_client/README.md | 1 + examples/tui_client/main.cpp | 19 ++++- include/sendspin/player_role.h | 14 ++++ src/player_role.cpp | 37 +++++++--- src/player_role_impl.h | 2 + src/protocol.cpp | 8 ++- src/protocol_messages.h | 4 +- tests/test_player_role.cpp | 121 +++++++++++++++++++++++++------- tests/test_protocol.cpp | 28 ++++++++ 12 files changed, 220 insertions(+), 41 deletions(-) diff --git a/docs/integration-guide.md b/docs/integration-guide.md index 8fa384a7..45cb0b76 100644 --- a/docs/integration-guide.md +++ b/docs/integration-guide.md @@ -1094,6 +1094,11 @@ player.update_output_delay(50); // User-adjustable delay in ms // Enable/disable output delay adjustment by the server. When disabled, the stored delay // is not applied to sync timing and is reported as 0 in client state. player.set_output_delay_adjustable(true); + +// Volume and mute are server-adjustable by default. Disable either for a fixed-level output +// (e.g. line out to an amplifier): client state omits its value and the server's command is ignored. +player.set_volume_adjustable(false); +player.set_mute_adjustable(false); ``` ## External Sources @@ -1568,7 +1573,7 @@ format arrives as the `format` argument of `on_display_pairing_code`. | `MUTE` | Mute state change from the server | | `SET_OUTPUT_DELAY` | Output delay adjustment from the server | -These represent commands the server can send to the player. The player advertises which commands it supports. Enable `SET_OUTPUT_DELAY` with `player.set_output_delay_adjustable(true)`. +These represent commands the server can send to the player. The player advertises which commands it supports. `VOLUME` and `MUTE` are advertised unless disabled with `player.set_volume_adjustable(false)` / `player.set_mute_adjustable(false)`; enable `SET_OUTPUT_DELAY` with `player.set_output_delay_adjustable(true)`. ### SendspinGoodbyeReason diff --git a/examples/basic_client/README.md b/examples/basic_client/README.md index e5a77915..ae6f733c 100644 --- a/examples/basic_client/README.md +++ b/examples/basic_client/README.md @@ -31,6 +31,7 @@ macOS has mDNS support built in; no extra dependencies needed. ./build/examples/basic_client/basic_client -p 8930 # listen on a custom port ./build/examples/basic_client/basic_client -s 12345678 # offer a static pairing code ./build/examples/basic_client/basic_client -a # allow unpaired access +./build/examples/basic_client/basic_client -F -M # server cannot set volume or mute ``` The client listens on port 8928 by default. When mDNS is enabled it advertises `_sendspin._tcp` with the configured port so Sendspin servers on the local network discover and connect automatically; otherwise tell the server to connect with `ws://:8928/sendspin`, replacing `8928` if you passed `-p`. diff --git a/examples/basic_client/main.cpp b/examples/basic_client/main.cpp index e94f65be..c255d1dd 100644 --- a/examples/basic_client/main.cpp +++ b/examples/basic_client/main.cpp @@ -28,6 +28,8 @@ /// -p PORT Listen on PORT (default: 8928) /// -s CODE Offer the static pairing code CODE (8 digits) instead of the dynamic one /// -a Enable unpaired access, so a server can stream without pairing first +/// -F The server cannot set the volume, which starts at full scale +/// -M The server cannot set the mute state /// -l LEVEL Set log level: none, error, warn, info (default), debug, verbose /// -v Verbose logging (same as -l verbose) /// -q Quiet logging (same as -l error) @@ -169,6 +171,8 @@ static void print_usage(const char* prog) { fprintf(stderr, " SIGUSR2 cancels it\n"); fprintf(stderr, " -a Enable unpaired access, so a server can stream without\n"); fprintf(stderr, " pairing first\n"); + fprintf(stderr, " -F The server cannot set the volume, which starts at full scale\n"); + fprintf(stderr, " -M The server cannot set the mute state\n"); fprintf(stderr, " -l LEVEL Log level: none, error, warn, info (default), debug, verbose\n"); fprintf(stderr, " -v Verbose logging (same as -l verbose)\n"); fprintf(stderr, " -q Quiet logging (same as -l error)\n"); @@ -207,9 +211,11 @@ int main(int argc, char* argv[]) { std::string connect_url; std::string static_pairing_code; bool unpaired_access = false; + bool server_volume = true; + bool server_mute = true; uint16_t server_port = DEFAULT_SENDSPIN_PORT; int opt; - while ((opt = getopt(argc, argv, "u:p:s:al:vqh")) != -1) { + while ((opt = getopt(argc, argv, "u:p:s:aFMl:vqh")) != -1) { switch (opt) { case 'u': connect_url = optarg; @@ -226,6 +232,12 @@ int main(int argc, char* argv[]) { case 'a': unpaired_access = true; break; + case 'F': + server_volume = false; + break; + case 'M': + server_mute = false; + break; case 'p': if (!parse_port(optarg, server_port)) { fprintf(stderr, "Invalid port: %s\n", optarg); @@ -311,6 +323,11 @@ int main(int argc, char* argv[]) { }; auto& player = client.add_player(std::move(player_config)); player.set_output_delay_adjustable(true); + player.set_volume_adjustable(server_volume); + player.set_mute_adjustable(server_mute); + if (!server_volume) { + player.update_volume(100); + } #endif #ifdef SENDSPIN_ENABLE_CONTROLLER // Added for its side effect: the client offers the role and accepts server/state for it. diff --git a/examples/tui_client/README.md b/examples/tui_client/README.md index 2c1a71ee..86fd0a01 100644 --- a/examples/tui_client/README.md +++ b/examples/tui_client/README.md @@ -45,6 +45,7 @@ brew install portaudio ./build/examples/tui_client/tui_client -u ws://192.168.1.10:8928/sendspin # connect to a specific server ./build/examples/tui_client/tui_client -p 8930 # listen on a custom port ./build/examples/tui_client/tui_client -a # allow unpaired access +./build/examples/tui_client/tui_client -F -M # server cannot set volume or mute ./build/examples/tui_client/tui_client -V # disable visualizer ``` diff --git a/examples/tui_client/main.cpp b/examples/tui_client/main.cpp index 23be919e..c28ffd63 100644 --- a/examples/tui_client/main.cpp +++ b/examples/tui_client/main.cpp @@ -25,6 +25,8 @@ /// -p PORT Listen on PORT (default: 8928) /// -f FORMAT Audio format as codec:rate:bits:channels (repeatable) /// -a Enable unpaired access, so a server can stream without pairing first +/// -F The server cannot set the volume, which starts at full scale +/// -M The server cannot set the mute state /// -V Disable visualizer /// -h Show usage @@ -442,6 +444,8 @@ static void print_usage(const char* prog) { fprintf(stderr, " At least one flac or pcm format is required\n"); fprintf(stderr, " -a Enable unpaired access, so a server can stream without\n"); fprintf(stderr, " pairing first\n"); + fprintf(stderr, " -F The server cannot set the volume, which starts at full scale\n"); + fprintf(stderr, " -M The server cannot set the mute state\n"); fprintf(stderr, " -V Disable visualizer\n"); fprintf(stderr, " -h Show this help\n"); } @@ -450,11 +454,13 @@ int main(int argc, char* argv[]) { // Parse command line options bool enable_visualizer = true; bool unpaired_access = false; + bool server_volume = true; + bool server_mute = true; std::string connect_url; uint16_t server_port = DEFAULT_SENDSPIN_PORT; std::vector audio_formats; int opt; - while ((opt = getopt(argc, argv, "u:p:f:aVh")) != -1) { + while ((opt = getopt(argc, argv, "u:p:f:aFMVh")) != -1) { switch (opt) { case 'u': connect_url = optarg; @@ -477,6 +483,12 @@ int main(int argc, char* argv[]) { case 'a': unpaired_access = true; break; + case 'F': + server_volume = false; + break; + case 'M': + server_mute = false; + break; case 'V': enable_visualizer = false; break; @@ -593,6 +605,11 @@ int main(int argc, char* argv[]) { player_config.audio_formats = std::move(audio_formats); auto& player = client.add_player(std::move(player_config)); player.set_output_delay_adjustable(true); + player.set_volume_adjustable(server_volume); + player.set_mute_adjustable(server_mute); + if (!server_volume) { + player.update_volume(100); + } #endif #ifdef SENDSPIN_ENABLE_CONTROLLER // Added for its side effect: the client offers the role and accepts server/state for it. diff --git a/include/sendspin/player_role.h b/include/sendspin/player_role.h index 3119b7ab..9298498c 100644 --- a/include/sendspin/player_role.h +++ b/include/sendspin/player_role.h @@ -188,6 +188,20 @@ class PlayerRole { /// @param adjustable true if the delay can be adjusted at runtime void set_output_delay_adjustable(bool adjustable); + /// @brief Enables or disables the server's volume command. Main loop only. + /// + /// Enabled by default. While disabled, volume is omitted from client state and a server + /// volume command is ignored; the stored volume is kept. + /// @param adjustable true if the server may set the volume + void set_volume_adjustable(bool adjustable); + + /// @brief Enables or disables the server's mute command. Main loop only. + /// + /// Enabled by default. While disabled, the mute state is omitted from client state and a + /// server mute command is ignored; the stored state is kept. + /// @param adjustable true if the server may set the mute state + void set_mute_adjustable(bool adjustable); + // ======================================== // Queries // ======================================== diff --git a/src/player_role.cpp b/src/player_role.cpp index 9663c90a..d943b562 100644 --- a/src/player_role.cpp +++ b/src/player_role.cpp @@ -177,6 +177,16 @@ void PlayerRole::set_output_delay_adjustable(bool adjustable) { this->impl_->client->publish_state(); } +void PlayerRole::set_volume_adjustable(bool adjustable) { + this->impl_->volume_adjustable = adjustable; + this->impl_->client->publish_state(); +} + +void PlayerRole::set_mute_adjustable(bool adjustable) { + this->impl_->mute_adjustable = adjustable; + this->impl_->client->publish_state(); +} + const ServerPlayerStreamObject& PlayerRole::get_current_stream_params() const { return this->impl_->current_stream_params; } @@ -293,8 +303,6 @@ size_t PlayerRole::Impl::advertised_buffer_capacity() const { void PlayerRole::Impl::build_state_fields(ClientStateMessage& msg) const { ClientPlayerStateObject player_state{}; - player_state.volume = this->volume; - player_state.muted = this->muted; bool adjustable = this->output_delay_adjustable.load(std::memory_order_relaxed); player_state.output_delay_ms = adjustable ? this->output_delay_ms.load(std::memory_order_relaxed) : 0; @@ -305,8 +313,16 @@ void PlayerRole::Impl::build_state_fields(ClientStateMessage& msg) const { std::max(this->config.required_lead_time_ms.value_or(0), PlayerRoleConfig::pipeline_lead_time_ms(this->config.extra_startup_silence_ms)); player_state.min_buffer_ms = this->config.min_buffer_ms; - // roles/player/v1.md "client/state player object": the commands the server may send. - player_state.supported_commands = {SendspinPlayerCommand::VOLUME, SendspinPlayerCommand::MUTE}; + // roles/player/v1.md "client/state player object": the commands the server may send. volume + // and muted are required only alongside their command, so a withheld command omits its value. + if (this->volume_adjustable) { + player_state.volume = this->volume; + player_state.supported_commands.push_back(SendspinPlayerCommand::VOLUME); + } + if (this->mute_adjustable) { + player_state.muted = this->muted; + player_state.supported_commands.push_back(SendspinPlayerCommand::MUTE); + } if (adjustable) { player_state.supported_commands.push_back(SendspinPlayerCommand::SET_OUTPUT_DELAY); } @@ -504,26 +520,29 @@ void PlayerRole::Impl::drain_events() { // Check each field independently since multiple command types may have been merged into one // inbox slot between drain ticks. Each callback may re-enter teardown (a listener calling // stop()), whose own drain already reset this role, so the rest is abandoned when the - // generation moved on. + // generation moved on. roles/player/v1.md "server/command player object": a command absent + // from the current supported_commands is ignored. if (have_command && cmd_msg.player.has_value()) { const ServerPlayerCommandObject& player_cmd = cmd_msg.player.value(); - if (player_cmd.volume.has_value()) { + if (player_cmd.volume.has_value() && !this->volume_adjustable) { + SS_LOGD(TAG, "Ignoring volume: not in supported_commands"); + } else if (player_cmd.volume.has_value()) { this->update_volume(player_cmd.volume.value()); if (this->listener) { this->listener->on_volume_changed(player_cmd.volume.value()); } } - if (player_cmd.mute.has_value() && this->accepts(generation)) { + if (player_cmd.mute.has_value() && !this->mute_adjustable) { + SS_LOGD(TAG, "Ignoring mute: not in supported_commands"); + } else if (player_cmd.mute.has_value() && this->accepts(generation)) { this->update_muted(player_cmd.mute.value()); if (this->listener) { this->listener->on_mute_changed(player_cmd.mute.value()); } } - // roles/player/v1.md "server/command player object": a command absent from the current - // supported_commands is ignored. const bool delay_advertised = this->output_delay_adjustable.load(std::memory_order_relaxed); if (player_cmd.output_delay_ms.has_value() && !delay_advertised) { SS_LOGD(TAG, "Ignoring set_output_delay: not in supported_commands"); diff --git a/src/player_role_impl.h b/src/player_role_impl.h index b97f0661..70302ee1 100644 --- a/src/player_role_impl.h +++ b/src/player_role_impl.h @@ -216,6 +216,7 @@ struct PlayerRole::Impl : RoleTeardown { // to go idle; the drain then runs again when sync_idle_slot is written. Main loop only. bool awaiting_sync_idle{false}; bool high_performance_requested_for_playback{false}; + bool mute_adjustable{true}; bool muted{false}; // True between the drained STREAM_START and STREAM_END callbacks (main-thread only); keeps // on_stream_end() from firing without a matching on_stream_start() @@ -224,6 +225,7 @@ struct PlayerRole::Impl : RoleTeardown { /// and the sync task. std::atomic output_delay_adjustable{false}; uint8_t volume{0}; + bool volume_adjustable{true}; }; } // namespace sendspin diff --git a/src/protocol.cpp b/src/protocol.cpp index b436621a..ba19b11d 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -949,8 +949,12 @@ std::string format_client_state_message(const ClientStateMessage* msg, if (msg->player.has_value()) { const ClientPlayerStateObject& player_state = msg->player.value(); - root["payload"]["player"]["volume"] = player_state.volume; - root["payload"]["player"]["muted"] = player_state.muted; + if (player_state.volume.has_value()) { + root["payload"]["player"]["volume"] = player_state.volume.value(); + } + if (player_state.muted.has_value()) { + root["payload"]["player"]["muted"] = player_state.muted.value(); + } root["payload"]["player"]["output_delay_ms"] = player_state.output_delay_ms; root["payload"]["player"]["required_lead_time_ms"] = player_state.required_lead_time_ms; root["payload"]["player"]["min_buffer_ms"] = player_state.min_buffer_ms; diff --git a/src/protocol_messages.h b/src/protocol_messages.h index 52f45bd6..fc4f1570 100644 --- a/src/protocol_messages.h +++ b/src/protocol_messages.h @@ -495,8 +495,8 @@ static constexpr uint8_t VOLUME_MAX = 100; /// @brief Player state reported by the client to the server in client/state messages struct ClientPlayerStateObject { - uint8_t volume{}; - bool muted{}; + std::optional volume; + std::optional muted; uint16_t output_delay_ms{}; uint16_t required_lead_time_ms{}; uint16_t min_buffer_ms{}; diff --git a/tests/test_player_role.cpp b/tests/test_player_role.cpp index 6154e462..eed740b6 100644 --- a/tests/test_player_role.cpp +++ b/tests/test_player_role.cpp @@ -184,32 +184,52 @@ TEST(PlayerRoleTimingParameters, ReportedWhileOutputDelayIsNotAdjustable) { // Supported commands in client/state // ============================================================================ -// roles/player/v1.md "client/state player object": supported_commands lists what the server may -// send. The role applies a volume or mute command on any output, so both are reported whether or -// not the output delay can be changed. -TEST(PlayerRoleSupportedCommands, VolumeAndMuteAreAlwaysReported) { - SendspinClient client(make_client_config("player-commands-fixed-delay")); - auto& player = client.add_player(make_player_config()); - - ClientPlayerStateObject state = build_player_state(player); - EXPECT_EQ(state.supported_commands, (std::vector{ - SendspinPlayerCommand::VOLUME, - SendspinPlayerCommand::MUTE, - })); -} - -// An adjustable output delay adds set_output_delay to the same list rather than replacing it. -TEST(PlayerRoleSupportedCommands, AdjustableOutputDelayAddsItsCommand) { - SendspinClient client(make_client_config("player-commands-adjustable-delay")); - auto& player = client.add_player(make_player_config()); - player.set_output_delay_adjustable(true); +// volume and muted are reported only alongside their command. Rows leaving a knob unset check +// its default; the toggled row checks that re-enabling reports the stored value. +TEST(PlayerRoleSupportedCommands, EachAdjustableControlAddsItsCommandAndField) { + using Cmd = SendspinPlayerCommand; + struct Row { + const char* name; + std::optional volume; + std::optional mute; + bool delay; + bool toggled; + std::vector commands; + }; + const Row rows[] = { + {"Control: defaults", std::nullopt, std::nullopt, false, false, {Cmd::VOLUME, Cmd::MUTE}}, + {"adjustable delay", std::nullopt, std::nullopt, true, false, + {Cmd::VOLUME, Cmd::MUTE, Cmd::SET_OUTPUT_DELAY}}, + {"fixed volume", false, std::nullopt, false, false, {Cmd::MUTE}}, + {"fixed mute", std::nullopt, false, false, false, {Cmd::VOLUME}}, + {"fixed volume and mute", false, false, true, false, {Cmd::SET_OUTPUT_DELAY}}, + {"volume and mute re-enabled", true, true, false, true, {Cmd::VOLUME, Cmd::MUTE}}, + }; + for (const Row& row : rows) { + SCOPED_TRACE(row.name); + SendspinClient client(make_client_config("player-commands")); + auto& player = client.add_player(make_player_config()); + player.update_volume(40); + player.update_muted(true); + if (row.toggled) { + player.set_volume_adjustable(false); + player.set_mute_adjustable(false); + } + if (row.volume.has_value()) { + player.set_volume_adjustable(row.volume.value()); + } + if (row.mute.has_value()) { + player.set_mute_adjustable(row.mute.value()); + } + player.set_output_delay_adjustable(row.delay); - ClientPlayerStateObject state = build_player_state(player); - EXPECT_EQ(state.supported_commands, (std::vector{ - SendspinPlayerCommand::VOLUME, - SendspinPlayerCommand::MUTE, - SendspinPlayerCommand::SET_OUTPUT_DELAY, - })); + const bool volume_reported = row.volume.value_or(true); + const bool mute_reported = row.mute.value_or(true); + ClientPlayerStateObject state = build_player_state(player); + EXPECT_EQ(state.supported_commands, row.commands); + EXPECT_EQ(state.volume, volume_reported ? std::optional(40) : std::nullopt); + EXPECT_EQ(state.muted, mute_reported ? std::optional(true) : std::nullopt); + } } // ============================================================================ @@ -851,6 +871,57 @@ TEST(PlayerRoleVolume, ValueOverTheSpecMaximumIsClamped) { } } +// roles/player/v1.md "server/command player object": an unadvertised volume or mute command +// reaches neither the listener nor the stored value. +TEST(PlayerRoleSupportedCommands, VolumeAndMuteCommandsAreIgnoredUnlessAdvertised) { + class LevelListener : public PlayerRoleListener { + public: + size_t on_audio_write(uint8_t* /*data*/, size_t length, uint32_t /*timeout_ms*/) override { + return length; + } + void on_volume_changed(uint8_t volume) override { + this->volumes.push_back(volume); + } + void on_mute_changed(bool muted) override { + this->mutes.push_back(muted); + } + std::vector volumes; + std::vector mutes; + }; + struct Row { + const char* name; + bool volume; + bool mute; + }; + const Row rows[] = { + {"Control: both advertised", true, true}, + {"volume not advertised", false, true}, + {"mute not advertised", true, false}, + }; + for (const Row& row : rows) { + SCOPED_TRACE(row.name); + LevelListener listener; // Outlives the role that holds it + auto impl = make_impl(); + impl->listener = &listener; + impl->volume_adjustable = row.volume; + impl->mute_adjustable = row.mute; + + ServerPlayerCommandObject player_cmd; + player_cmd.command = SendspinPlayerCommand::VOLUME; + player_cmd.volume = 70; + player_cmd.mute = true; + ServerCommandMessage cmd; + cmd.player = player_cmd; + impl->handle_server_command(cmd); + impl->drain_events(); + + EXPECT_EQ(listener.volumes, row.volume ? std::vector{70} : std::vector{}); + EXPECT_EQ(listener.mutes, row.mute ? std::vector{true} : std::vector{}); + EXPECT_EQ(impl->volume, row.volume ? 70 : 0); + EXPECT_EQ(impl->muted, row.mute); + } +} + // roles/player/v1.md "server/command player object": set_output_delay is only honored while the // player advertises it in supported_commands, which the consumer sees as on_output_delay_changed(). TEST(PlayerRoleOutputDelay, SetOutputDelayCommandIsIgnoredUnlessAdvertised) { diff --git a/tests/test_protocol.cpp b/tests/test_protocol.cpp index 512e8206..121a4977 100644 --- a/tests/test_protocol.cpp +++ b/tests/test_protocol.cpp @@ -1131,6 +1131,34 @@ TEST(Protocol, FormatClientStatePlayerEmptySupportedCommandsIsStillEmitted) { EXPECT_EQ(doc["payload"]["player"]["supported_commands"].as().size(), 0u); } +// roles/player/v1.md "client/state player object": volume and muted are optional keys, written +// only when set. The control row's falsy values are still written. +TEST(Protocol, FormatClientStatePlayerOmitsUnsetVolumeAndMuted) { + struct Row { + const char* name; + std::optional volume; + std::optional muted; + }; + const Row rows[] = { + {"Control: both set to falsy values", 0, false}, + {"volume unset", std::nullopt, false}, + {"muted unset", 0, std::nullopt}, + }; + for (const Row& row : rows) { + SCOPED_TRACE(row.name); + ClientStateMessage msg; + ClientPlayerStateObject player{}; + player.volume = row.volume; + player.muted = row.muted; + msg.player = player; + + JsonDocument doc; + ASSERT_FALSE(deserializeJson(doc, format_client_state_message(&msg, TestArena()))); + EXPECT_EQ(doc["payload"]["player"]["volume"].is(), row.volume.has_value()); + EXPECT_EQ(doc["payload"]["player"]["muted"].is(), row.muted.has_value()); + } +} + // Control: zero is a value the server must see reported, not a reason to omit the key. A // serializer that skipped falsy timing fields would pass the test above and fail this one. TEST(Protocol, FormatClientStatePlayerReportsZeroTimingFields) { From 5e52f8645722c74bb7b663478ea17e7ed6cd7848 Mon Sep 17 00:00:00 2001 From: Kevin Ahrendt Date: Tue, 13 Oct 2026 11:13:41 -0400 Subject: [PATCH 2/2] Test the player knobs through the public API and over a live connection The command-gate test now builds the role with add_player() and sets and reads it through the public setters and getters. A new loopback test checks that withdrawing and restoring volume and mute republishes client/state while connected; dropping either setter's publish hangs it. The player test's access comment now names PlayerRole::impl_. --- tests/CMakeLists.txt | 26 ++++++------- tests/test_encrypted_lifecycle.cpp | 59 ++++++++++++++++++++++++++++++ tests/test_player_role.cpp | 17 +++++---- 3 files changed, 81 insertions(+), 21 deletions(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 4291dccf..7a8fd8aa 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -158,20 +158,20 @@ set_source_files_properties(test_client_lifecycle.cpp PROPERTIES COMPILE_OPTIONS # role inbox's topic bits). set_source_files_properties(test_role_deactivation.cpp PROPERTIES COMPILE_OPTIONS -fno-access-control) -# test_player_role.cpp drives PlayerRole::Impl directly - no connected server, no sync-task thread - +# test_player_role.cpp drives PlayerRole::Impl directly (no connected server, no sync-task thread), # which is the only way to see the client/state player object and the only way to stage a slot -# payload stamped before a teardown. Reached: Impl's handle_stream_start(), handle_binary(), -# handle_stream_clear(), handle_stream_end(), handle_server_command(), cleanup(), attach_inbox(), -# drain_events(), volume, cleanup_generation (moved on directly to stage a take between a teardown's -# generation bump and its recall) and event_state's generation-stamped slots (one written with a -# stamp from before a teardown, the payload a drain on the far side of it would hold); SyncTask's -# inbound_ (and take_item()), bound to a test ring the way start() binds it but with no thread, so -# what a handler handed over (or dropped) stays on the list to be read, and -# discard_to_clear_marker(), the sync thread's stream/clear step; and SyncTask's -# handle_synchronize_audio(), handle_transfer_audio() and starved_playtime_floor(), the inner -# loop's hard-sync steps driven without the thread. The inbound tests also bind a -# VisualizerRole::Impl to the same ring, to show one holder's quota does not stop the other. The -# availability table reaches SendspinClient's protocol_task_, build_client_state(), +# payload stamped before a teardown. Reached: PlayerRole's impl_ (a role add_player() built), Impl's +# handle_stream_start(), handle_binary(), handle_stream_clear(), handle_stream_end(), +# handle_server_command(), cleanup(), attach_inbox(), drain_events(), volume, cleanup_generation +# (moved on directly to stage a take between a teardown's generation bump and its recall) and +# event_state's generation-stamped slots (one written with a stamp from before a teardown, the +# payload a drain on the far side of it would hold); SyncTask's inbound_ (and take_item()), bound to +# a test ring the way start() binds it but with no thread, so what a handler handed over (or +# dropped) stays on the list to be read, and discard_to_clear_marker(), the sync thread's +# stream/clear step; and SyncTask's handle_synchronize_audio(), handle_transfer_audio() and +# starved_playtime_floor(), the inner loop's hard-sync steps driven without the thread. The inbound +# tests also bind a VisualizerRole::Impl to the same ring, to show one holder's quota does not stop +# the other. The availability table reaches SendspinClient's protocol_task_, build_client_state(), # adopt_client_state(), connection_manager_ (its install_admitted()) and process_binary_message(): # the client is never started, so the test publishes and adopts each snapshot and dispatches the # chunk itself. diff --git a/tests/test_encrypted_lifecycle.cpp b/tests/test_encrypted_lifecycle.cpp index 83b1f01a..8f7e874d 100644 --- a/tests/test_encrypted_lifecycle.cpp +++ b/tests/test_encrypted_lifecycle.cpp @@ -102,6 +102,7 @@ constexpr uint16_t RECENCY_IDLE_TEST_PORT = 19021; constexpr uint16_t RECENCY_PLAYBACK_TEST_PORT = 19022; constexpr uint16_t RECENCY_LATER_PLAYBACK_TEST_PORT = 19023; constexpr uint16_t LIVENESS_STAMP_TEST_PORT = 19024; +constexpr uint16_t PLAYER_KNOBS_TEST_PORT = 19025; constexpr uint16_t AVAILABILITY_TEST_PORT = 19079; constexpr uint16_t CLOCK_GATE_SYNCED_TEST_PORT = 19080; constexpr uint16_t CLOCK_GATE_UNSYNCED_TEST_PORT = 19081; @@ -1421,6 +1422,64 @@ TEST(EncryptedLifecycle, ActivateThatAddsARoleSendsItsClientState) { pump_for(client, 100); } +// roles/player/v1.md "client/state player object": a player MAY update supported_commands and +// MUST send a state update whenever a field changes, so withdrawing or restoring volume and mute +// republishes client/state. The two steps call the setters in opposite orders, so each setter's +// publish is the last one in some step. +TEST(EncryptedLifecycle, PlayerVolumeAndMuteKnobsRepublishClientState) { + SendspinClientConfig config; + config.name = "Player Knobs Test Client"; + config.server_port = PLAYER_KNOBS_TEST_PORT; + + PairedClientBundle bundle(config); + SendspinClient& client = bundle.client(); + add_state_object_roles(client); + PlayerRole& player = *client.player(); + ASSERT_TRUE(bundle.start()); + + FakeEncryptedServerOptions options; + options.answer_time = true; // An active player's state waits for clock sync. + options.first_roles_json = R"(["player@v1"])"; + auto server = std::make_unique( + server_url(PLAYER_KNOBS_TEST_PORT), std::string(NOISE_SUITE_CHACHAPOLY), + bundle.peer.server_identity, bundle.peer.record.psk_id, bundle.peer.psk, + std::move(options)); + pump_until(client, [&] { return server->client_state_count() > 0; }); + + // True when the last client/state carries volume and mute, each with its command, exactly when + // `reported`. + auto last_state_reports = [&](bool reported) { + JsonDocument doc; + if (!parse_last_client_state(*server, doc)) { + return false; + } + JsonObjectConst state = doc["payload"]["player"].as(); + if (state.isNull()) { + return false; + } + bool volume_command = false; + bool mute_command = false; + for (JsonVariantConst command : state["supported_commands"].as()) { + volume_command |= command == "volume"; + mute_command |= command == "mute"; + } + return volume_command == reported && mute_command == reported && + state["volume"].is() == reported && state["muted"].is() == reported; + }; + ASSERT_TRUE(last_state_reports(true)) << "Control: both are reported by default"; + + player.set_volume_adjustable(false); + player.set_mute_adjustable(false); + pump_until(client, [&] { return last_state_reports(false); }); + + player.set_mute_adjustable(true); + player.set_volume_adjustable(true); + pump_until(client, [&] { return last_state_reports(true); }); + + client.disconnect(SendspinGoodbyeReason::SHUTDOWN); + pump_for(client, 100); +} + // connection.md "Re-handshake": once the client has received Noise message 1 it sends nothing but // the handshake until the new server/activate arrives. Role-originated traffic waits with // everything else, even though the connection keeps its admitted slot and its active roles diff --git a/tests/test_player_role.cpp b/tests/test_player_role.cpp index eed740b6..94dcdaa6 100644 --- a/tests/test_player_role.cpp +++ b/tests/test_player_role.cpp @@ -901,10 +901,11 @@ TEST(PlayerRoleSupportedCommands, VolumeAndMuteCommandsAreIgnoredUnlessAdvertise for (const Row& row : rows) { SCOPED_TRACE(row.name); LevelListener listener; // Outlives the role that holds it - auto impl = make_impl(); - impl->listener = &listener; - impl->volume_adjustable = row.volume; - impl->mute_adjustable = row.mute; + SendspinClient client(make_client_config("player-command-gate")); + auto& player = client.add_player(make_player_config()); + player.set_listener(&listener); + player.set_volume_adjustable(row.volume); + player.set_mute_adjustable(row.mute); ServerPlayerCommandObject player_cmd; player_cmd.command = SendspinPlayerCommand::VOLUME; @@ -912,13 +913,13 @@ TEST(PlayerRoleSupportedCommands, VolumeAndMuteCommandsAreIgnoredUnlessAdvertise player_cmd.mute = true; ServerCommandMessage cmd; cmd.player = player_cmd; - impl->handle_server_command(cmd); - impl->drain_events(); + player.impl_->handle_server_command(cmd); + player.impl_->drain_events(); EXPECT_EQ(listener.volumes, row.volume ? std::vector{70} : std::vector{}); EXPECT_EQ(listener.mutes, row.mute ? std::vector{true} : std::vector{}); - EXPECT_EQ(impl->volume, row.volume ? 70 : 0); - EXPECT_EQ(impl->muted, row.mute); + EXPECT_EQ(player.get_volume(), row.volume ? 70 : 0); + EXPECT_EQ(player.get_muted(), row.mute); } }