From 52c0e8b670ea418eefe3bcfae77831af6279bba7 Mon Sep 17 00:00:00 2001 From: Kevin Ahrendt Date: Wed, 11 Nov 2026 09:44:11 -0500 Subject: [PATCH 1/2] Hand metadata and group text to consumers as string_view TrackMetadata and on_track_metadata() deliver a metadata state with its text as std::string_view into the role's current text block, and GroupState, on_group_state() and group_state() deliver the group state as views into an inline copy the main loop keeps. The library calls the new callback first and then the deprecated one, skipping the deprecated call after a re-entrant stop(), so existing consumers see no change. ServerMetadataStateObject, on_metadata(), GroupUpdateObject, on_group_update() and get_group_state() are deprecated and go in v0.11.0, together with the ReservedText strings that back them. The examples move to the new API, and the integration guide maps each old name to its replacement. --- docs/conventions.md | 12 +- docs/integration-guide.md | 49 ++- docs/internals.md | 9 +- examples/basic_client/main.cpp | 8 +- examples/tui_client/main.cpp | 12 +- examples/tui_client/tui.cpp | 2 +- include/sendspin/client.h | 31 +- include/sendspin/config.h | 20 +- include/sendspin/metadata_role.h | 43 ++- include/sendspin/types.h | 23 +- src/client.cpp | 98 ++++-- src/metadata_role.cpp | 68 +++- src/metadata_role_impl.h | 34 +- src/protocol_messages.h | 10 +- src/teardown_tracker.h | 5 + tests/CMakeLists.txt | 8 +- tests/lifecycle_test_fixtures.h | 2 +- tests/test_allocations.cpp | 84 +++-- tests/test_client_lifecycle.cpp | 42 ++- tests/test_encrypted_lifecycle.cpp | 486 ++++++++++++++++++++++++++--- 20 files changed, 827 insertions(+), 219 deletions(-) diff --git a/docs/conventions.md b/docs/conventions.md index 6ce5e4c3..9f5c2ca5 100644 --- a/docs/conventions.md +++ b/docs/conventions.md @@ -185,11 +185,13 @@ checklists in `.claude/skills/` apply these standards to a diff. is bounded where it is parsed: read in place, refused past its cap, or cut to its field's size. Peer text a log line prints that no field bounds (a type, a reason, an unrecognized value) is cut to `PEER_TEXT_LOG_BYTES` - (`peer_text_for_log()`). A public type keeps `std::string` and `std::vector`; where - its value has a bound, the library holds it inline and builds the public - type on the thread that hands it to the consumer. `docs/internals.md` - "Memory" lists the `operator new` allocations that remain after `start()`, - and `sendspin_alloc_tests` counts them on the paths it walks. + (`peer_text_for_log()`). Bounded text in a public type built for views + (`TrackMetadata`, `GroupState`) is a `std::string_view` into storage the + library keeps, valid for as long as its declaration says; a public type + still holding `std::string` or `std::vector` is built from the inline value + on the thread that hands it to the consumer. `docs/internals.md` "Memory" + lists the `operator new` allocations that remain after `start()`, and + `sendspin_alloc_tests` counts them on the paths it walks. - Buffer and queue capacities are justified numbers. Derive related constants from each other (`constexpr size_t X = Y / 4;`) so they cannot drift apart, and do not grow a budget without recording why. diff --git a/docs/integration-guide.md b/docs/integration-guide.md index e1695cd9..d84104d6 100644 --- a/docs/integration-guide.md +++ b/docs/integration-guide.md @@ -298,7 +298,7 @@ This method is thread-safe and is expected to be called from an audio callback t ```cpp struct MyMetadataListener : MetadataRoleListener { - void on_metadata(const ServerMetadataStateObject& md) override { + void on_track_metadata(const TrackMetadata& md) override { // Overwrite display state on every call so server clears (nullopt) propagate. display_title(md.title.value_or("")); display_artist(md.artist.value_or("")); @@ -312,25 +312,25 @@ struct MyMetadataListener : MetadataRoleListener { }; ``` -The `ServerMetadataStateObject` contains these fields (all optional except `timestamp`): +The `TrackMetadata` contains these fields (all optional except `timestamp`): | Field | Type | Description | |---|---|---| | `timestamp` | `int64_t` | Server clock µs at which this metadata becomes valid; delivery is held until the synced client clock reaches it | -| `title` | `std::optional` | Track title, at most 255 bytes | -| `artist` | `std::optional` | Track artist, at most 255 bytes | -| `album_artist` | `std::optional` | Album artist, at most 255 bytes | -| `album` | `std::optional` | Album name, at most 255 bytes | -| `artwork_url` | `std::optional` | Artwork URL, at most 2048 bytes | +| `title` | `std::optional` | Track title, at most 255 bytes | +| `artist` | `std::optional` | Track artist, at most 255 bytes | +| `album_artist` | `std::optional` | Album artist, at most 255 bytes | +| `album` | `std::optional` | Album name, at most 255 bytes | +| `artwork_url` | `std::optional` | Artwork URL, at most 2048 bytes | | `year` | `std::optional` | Release year | | `track` | `std::optional` | Track number | | `progress` | `std::optional` | Playback progress (see below) | `MetadataProgressObject` contains `track_progress` (ms), `track_duration` (ms), and `playback_speed`. -The text fields are bounded in UTF-8 bytes as roles/metadata/v1.md "server/state metadata object" caps them. A server that sends a longer one has it cut to the cap at a UTF-8 character boundary, with one warning per connection. A text field turned off in `MetadataRoleConfig` is always `nullopt`. +The text views are valid for the duration of the callback: copy a field the listener needs after it returns. The text fields are bounded in UTF-8 bytes as roles/metadata/v1.md "server/state metadata object" caps them. A server that sends a longer one has it cut to the cap at a UTF-8 character boundary, with one warning per connection. A text field turned off in `MetadataRoleConfig` is always `nullopt`. -Every `on_metadata()` call carries the full state of the track being described, not a set of changes: a field the server left out of that update is `nullopt`, whatever an earlier update reported for it, and a state without `progress` means there is no position to show. Listeners that mirror metadata into display state should therefore overwrite every displayed value on each call (using e.g. `value_or("")`) rather than merging into what they already show. +Every `on_track_metadata()` call carries the full state of the track being described, not a set of changes: a field the server left out of that update is `nullopt`, whatever an earlier update reported for it, and a state without `progress` means there is no position to show. Listeners that mirror metadata into display state should therefore overwrite every displayed value on each call (using e.g. `value_or("")`) rather than merging into what they already show. You can also poll track progress at any time: @@ -685,13 +685,15 @@ Receives client-level events. All callbacks fire on the main loop thread. ```cpp struct MyClientListener : SendspinClientListener { - // Called when group state changes (playback state, group name, etc.). An empty group_id - // means the group was cleared. A non-empty server group_id that is not 43 base64url + // Called when the server's group/update messages are applied, with only the fields changed + // by those since the last call (playback state, group id, group name); client.group_state() + // returns the merged state. The views are valid for the duration of the callback. An empty + // group_id means the group was cleared. A non-empty server group_id that is not 43 base64url // characters arrives as base64url of its SHA-256, which still names one group; a future // release closes the connection on such an id instead. - void on_group_update(const GroupUpdateObject& group) override { - if (group.playback_state) update_playback_indicator(*group.playback_state); - if (group.group_name) update_group_display(*group.group_name); + void on_group_state(const GroupState& changes) override { + if (changes.playback_state) update_playback_indicator(*changes.playback_state); + if (changes.group_name) update_group_display(*changes.group_name); } // Called after a time sync burst completes. @@ -1163,7 +1165,8 @@ The client and roles expose query methods for polling state in your main loop or bool paired = client.is_paired(); // Active connection is a paired session (read first) bool connected = client.is_connected(); // Active connection with completed handshake bool synced = client.is_time_synced(); // Time filter has received at least one measurement -const GroupUpdateObject& group = client.get_group_state(); // Group id, name, playback state (all optional) +GroupState group = client.group_state(); // Group id, name, playback state (all optional); + // its views hold until control returns to loop(), or stop() // Player state uint8_t vol = player.get_volume(); @@ -1182,6 +1185,20 @@ uint32_t duration = metadata.get_track_duration_ms(); int64_t client_ts = client.get_client_time(server_timestamp); ``` +## Deprecated String API + +The metadata and group text is now handed over as `std::string_view` into storage the library keeps, rather than copied into `std::string`. The `std::string` forms are deprecated in v0.10.0 and removed in v0.11.0; naming one of them draws a `-Wdeprecated-declarations` warning. + +| Deprecated | Replacement | +|---|---| +| `ServerMetadataStateObject` | `TrackMetadata` | +| `MetadataRoleListener::on_metadata()` | `MetadataRoleListener::on_track_metadata()` | +| `GroupUpdateObject` | `GroupState` | +| `SendspinClientListener::on_group_update()` | `SendspinClientListener::on_group_state()` | +| `SendspinClient::get_group_state()` | `SendspinClient::group_state()`, which returns by value | + +The fields keep their names and meaning; only the text type changes. A view handed to a callback is valid for the duration of the callback, and the views `group_state()` returns until control next returns to `loop()` (from inside a callback: until the callback returns) or until `stop()`: copy what is needed beyond that. Until the removal the library calls the new callback and then the deprecated one with the same content, so a listener overrides one of the two; one that overrides both gets both, the new one first. A `stop()` from the new callback ends the delivery there. + ## Thread Safety Summary The library runs its connection work on its own protocol task (`SsProto`): every handshake, @@ -1438,7 +1455,7 @@ Configuration passed to `client.add_metadata()`, which defaults it. | Field | Type | Default | Description | |---|---|---|---| -| `title`, `artist`, `album_artist`, `album`, `artwork_url` | `bool` | `true` | Whether to keep that text field of `ServerMetadataStateObject`. A field set `false` is dropped as each state is parsed and reaches the listener as `nullopt`; `year`, `track` and `progress` are always kept. Each kept field costs its cap (255 bytes, 2048 for `artwork_url`) in the public state's string, reserved by `add_metadata()` through `operator new` (internal RAM on ESPHome), and in each of the three text blocks that carry states from the protocol task to the main loop, allocated by `start()`. With every field kept that is 3,068 bytes of string capacity and 9,204 bytes of blocks; with `{.artwork_url = false}`, 1,020 and 3,060. `start()` fails, logged, when the blocks cannot be allocated. | +| `title`, `artist`, `album_artist`, `album`, `artwork_url` | `bool` | `true` | Whether to keep that text field of `TrackMetadata`. A field set `false` is dropped as each state is parsed and reaches the listener as `nullopt`; `year`, `track` and `progress` are always kept. Each kept field costs its cap (255 bytes, 2048 for `artwork_url`) in the string of the deprecated `ServerMetadataStateObject`, reserved by `add_metadata()` through `operator new` (internal RAM on ESPHome), and in each of the three text blocks that carry states from the protocol task to the main loop, allocated by `start()`. With every field kept that is 3,068 bytes of string capacity and 9,204 bytes of blocks; with `{.artwork_url = false}`, 1,020 and 3,060. `start()` fails, logged, when the blocks cannot be allocated. | | `buffer_location` | `MemoryLocation` | `PREFER_EXTERNAL` | Memory placement preference for the text blocks. `PREFER_EXTERNAL` tries SPIRAM first and falls back to internal RAM; `PREFER_INTERNAL` does the reverse. ESP-IDF only; ignored on host. | --- diff --git a/docs/internals.md b/docs/internals.md index a2790103..195a0ca4 100644 --- a/docs/internals.md +++ b/docs/internals.md @@ -156,7 +156,8 @@ While the client is stopped, `SendspinClient::loop()` only frees the outbound at 6. Dispatch the notes taken in step 5 in queue order, skipping a code or window dismissal, or a window prompt, that repeats the last note delivered for its prompt 7. Role drains: each role's impl_->drain_events(), gated on impl_->needs_drain(slot_bits) -8. Drain group_slot: apply deltas, fire on_group_update +8. Drain group_slot: merge the delta into the group state, fire on_group_state, then + on_group_update ``` Step 1 is what still runs a teardown's main-loop half, and delivers its clear, when the full event ring dropped its stamped `*_CLEARED`. It can only run a half earlier, never apply anything: a payload is applied only when its stamp is the current generation, and the events and role drains after it catch up again at the stamp they act on. The steps gate on two `poll()` snapshots: one before the ring drain (steps 2 to 4) and one after it (steps 6 to 8), which catches bits set while the ring was draining. Provider writes run after the notes are taken and before they are dispatched, so `on_pairing_succeeded` finds the record committed: the protocol task requests the write before it queues the note. Pairing notes follow the event ring, so a role's clear can be delivered ahead of a note queued before it: the two never describe the same listener state, and the one ordering between them that matters, the provider write ahead of `on_pairing_succeeded`, is step 5. Roles whose pending work waits on a deadline rather than on a new message (the metadata and color roles' pending state, artwork's held displays) add a carry-over term to `needs_drain()`, since no inbox bit tracks that wait; the metadata and color roles test their main loop's copy of the pending deadline (`ScheduledStateSlot::pending_at()`), so no tick takes the Inbox mutex while a state waits; the player's STREAM_END held for the sync task waits instead for `sync_idle_slot`, which the sync task writes each time it returns to idle from a stream. @@ -231,6 +232,8 @@ Requests a listener callback makes (`connect_to()`, `disconnect()`, ...) are que - `drain_generation` (on the client's `EventState`) is bumped by `stop()` before its teardown. The outer drain snapshots it once and stops as soon as it changes: the event ring between events, the pairing-note dispatch between notes, and the role drains and the group update before each starts. - Each role's `cleanup_generation` is bumped by its `cleanup()` and stamped onto every event and slot payload it queues afterwards; the ring drain drops stream events whose stamp no longer matches (`event_is_current()`). Inside a role drain the same counter catches a callback that re-entered teardown: the player checks it after each server-command callback and around each `on_stream_start()` and `on_stream_end()`, metadata and color by ending their drain once the teardown's catch-up has forgotten the pending deadline (`ScheduledStateSlot::pending_at()`), and artwork after each display or clear, and each abandons the rest of its work when it moved on. +A state handed to the listener in two forms, the view and the deprecated `std::string` copy, is not handed over in the second once the first re-entered teardown: the group update checks the drain generation between `on_group_state()` and `on_group_update()`, and the metadata role checks between `on_track_metadata()` and `on_metadata()` whether its `TeardownTracker` completed a teardown, which on the main loop only `stop()` can do inside a callback. + ## Message Flow ### Receive Path @@ -458,9 +461,9 @@ On host, IXWebSocket callbacks hold a `weak_ptr` to the connection the manager a The library allocates from heaps that fail differently. `operator new`, behind every `std::string`, `std::vector` and `std::make_shared` and any `std::function` whose capture outgrows its small buffer, draws on ESPHome only on internal RAM and aborts when it fails. The `platform_malloc` family (`src/platform/memory.h`, behind `PlatformBuffer` and the JSON arena's overflow) places each block by its `MemoryLocation`, and noise-c allocates its handshake, cipher and hash states with `calloc`; both report a failure, which the caller turns into a refused or closed connection or a dropped message. Normalizing a `group_id` that is not 43 base64url characters (`group_id_from_wire()`, while `GROUP_ID_STRICT` is off) has noise-c `calloc` a SHA-256 state on the protocol task for every `group/update` from a server that sends such ids; when that allocation fails, the connection is closed, as for a non-conforming id under `GROUP_ID_STRICT`, rather than applying the rest of the update under the previous group's id. The exceptions are hashing a PSK into its `psk_id`, which aborts when noise-c cannot allocate the hash state, but only while `start()` loads the record store (`psk_id_for()` on a `std::array`), and deriving `SENTINEL_PSK` the same way at static initialization; a pairing that cannot hash its new PSK fails instead. Construction, `add_*()` and `start()` use any of them. After `start()`, the per-connection and per-message buffers come from the `platform_malloc` family: the connection object of an ESP accept and of every `connect_to()`, with its `shared_ptr` control block (`make_connection_in_block()`: `std::allocate_shared()` into a block reserved first, so a failure refuses the session or the connect), the JSON arena's overflow, the handshake's prologue and msg1 buffers, the fallback buffer, the Noise transport's reassembly and send buffers, and on ESP a send that does not fit the `FixedBlockPool`. The sync task allocates on first use from the same family: its decode buffer's storage at the first codec header (the `TransferBuffer` itself sits by value in the task's `SyncContext`), where a failure drops the stream, and the FLAC decoder, constructed in a `PlatformBuffer` at the first FLAC codec header and reset for each later FLAC stream, where a failure fails the header and the stream's chunks are discarded unplayed. micro-flac allocates the rest of its state with `FLAC_MALLOC`, which fails the same way; its own `operator new` allocations, an Ogg demuxer and stored metadata blocks, never run on the `fLaC` and last-STREAMINFO `codec_header` the parser admits. The visualizer's spectrum bins are reserved by `start()` for the configured `n_disp_bins` when the configuration requests the spectrum type, and a stream serving more bins than that drops its spectrum frames rather than growing them on the drain thread. `connect_to()` takes its URL by value and moves it through the request slot into the transport, so the library copies none of it. The host transport, whose IXWebSocket API copies each frame into a `std::string` and which makes a host accept's connection object with `std::make_shared`, and the host-only message trace sit outside this model: ESP replaces the first with esp_http_server and esp_websocket_client and builds without the second. -Bounded values stay inline: the ids and names a connection holds (`IdText`, `ServerName`), activity, player and controller command, visualizer type and source codec sets (`EnumSet`), role sets, `stream/start`'s artwork channels (`InlineVector`) and its player's FLAC codec header, decoded as it is parsed (`FlacCodecHeader`), the protocol task's slot tables and the ESP server's session slots and pending upgrades (`InlineVector`, `std::array`), each connection's `NoiseSession`, which owns the noise-c handshake and cipher states, and `NoiseHandshake`, a pairing attempt's CPace state, its PRS and the code it emits (`InlineText`), and the pairing records (`PairingRecord`, its ids `IdText`): the one a pairing holds pending on its connection and those the record store holds, reserved for `max_pairing_records` when `start()` builds the store. The store's mutators mark the slots a flush owes in a bitset, and the flush encodes one write at a time into a buffer on the main loop's stack. CPace hashes each length-value piece of its transcript as it encodes it rather than building the transcript first. The pairing notes for the main loop trade two buffers reserved at construction between the slot and the drain (`InboxSlot::exchange()`). Role sets take two shapes: ownership, admission and `client/state` intersect and complement them, so `ActiveRoles` and the admitted slots hold `uint16_t` role masks (`role_mask_bit()`, `ALL_ROLES_MASK`), while the roles a `stream/start`, `stream/end` or `stream/clear` covers are only filled and looked up, so they are an `EnumSet`. `client/state`'s artwork channels and visualizer types view the role's configuration, fixed from `add_*()`, which may be called only while the client is stopped (`client.h` "Role registration"), and `client/hello`'s application roles view the client's configuration as `start()` normalized it. A getter that returns `std::string`, such as the server's id and name, builds it from the inline value on the caller's thread. The main loop lists the controller's `supported_commands` and a visualizer stream's `types` from their sets into public vectors each role reserves for every member when it is built, and copies `group/update`'s `group_id` and `group_name`, parsed into `GroupUpdatePayload` (`IdText`, `ServerName`), into the public strings of the client's group state and of the delta `on_group_update()` is handed, each reserved for its bound when the client is built and swapped aside rather than freed when its field is cleared (`ReservedText`, `src/reserved_text.h`). +Bounded values stay inline: the ids and names a connection holds (`IdText`, `ServerName`), activity, player and controller command, visualizer type and source codec sets (`EnumSet`), role sets, `stream/start`'s artwork channels (`InlineVector`) and its player's FLAC codec header, decoded as it is parsed (`FlacCodecHeader`), the protocol task's slot tables and the ESP server's session slots and pending upgrades (`InlineVector`, `std::array`), each connection's `NoiseSession`, which owns the noise-c handshake and cipher states, and `NoiseHandshake`, a pairing attempt's CPace state, its PRS and the code it emits (`InlineText`), and the pairing records (`PairingRecord`, its ids `IdText`): the one a pairing holds pending on its connection and those the record store holds, reserved for `max_pairing_records` when `start()` builds the store. The store's mutators mark the slots a flush owes in a bitset, and the flush encodes one write at a time into a buffer on the main loop's stack. CPace hashes each length-value piece of its transcript as it encodes it rather than building the transcript first. The pairing notes for the main loop trade two buffers reserved at construction between the slot and the drain (`InboxSlot::exchange()`). Role sets take two shapes: ownership, admission and `client/state` intersect and complement them, so `ActiveRoles` and the admitted slots hold `uint16_t` role masks (`role_mask_bit()`, `ALL_ROLES_MASK`), while the roles a `stream/start`, `stream/end` or `stream/clear` covers are only filled and looked up, so they are an `EnumSet`. `client/state`'s artwork channels and visualizer types view the role's configuration, fixed from `add_*()`, which may be called only while the client is stopped (`client.h` "Role registration"), and `client/hello`'s application roles view the client's configuration as `start()` normalized it. A getter that returns `std::string`, such as the server's id and name, builds it from the inline value on the caller's thread. The main loop lists the controller's `supported_commands` and a visualizer stream's `types` from their sets into public vectors each role reserves for every member when it is built. `group/update`'s `group_id` and `group_name` are parsed into `GroupUpdatePayload` (`IdText`, `ServerName`), and the main loop merges each payload into the client's group state, a `GroupUpdatePayload` it keeps, which `group_state()` views, as `on_group_state()` views the payload; it also copies both into the deprecated public strings of `get_group_state()` and of the delta `on_group_update()` is handed, each reserved for its bound when the client is built and swapped aside rather than freed when its field is cleared (`ReservedText`, `src/reserved_text.h`). -A metadata state's text, only the fields `MetadataRoleConfig` keeps, each cut to its cap as it is parsed (`MAX_METADATA_TEXT_BYTES`, `MAX_METADATA_URL_BYTES`), is copied twice: into a text block as it arrives and out of it into the public strings. `start()` allocates `METADATA_TEXT_BLOCKS` blocks from the `platform_malloc` family (`MetadataRoleConfig::buffer_location`; a failure fails the start, logged) and keeps them for the role's life. Each place a state can be names its block by index (`MetadataStateBuffer`): the slot's oldest and newest places (`ScheduledStateSlot`) and the main loop's current state, the one last delivered. A delivery swaps the delivered place with the current state under the Inbox mutex, so each block always has one place. `METADATA_TEXT_BLOCKS` is the fewest that keep every state the role must: the delivered state the main loop's callback is viewing, the pending one, and room for an arrival meanwhile, since the protocol task does not wait for the main loop. The protocol task copies the kept text from the parsed document into its place's block inside `ParsedJsonMessage::extract()`, the only window the document is alive, under the Inbox mutex (`MetadataRole::Impl::fill_place()`); the main loop copies the current state's text into the public strings, each reserved for its field's cap when the role is built (`ReservedText`), with no lock held. +A metadata state's text, only the fields `MetadataRoleConfig` keeps, each cut to its cap as it is parsed (`MAX_METADATA_TEXT_BYTES`, `MAX_METADATA_URL_BYTES`), is copied into a text block as it arrives, which `on_track_metadata()` views, and out of it into the deprecated public strings `on_metadata()` is handed. `start()` allocates `METADATA_TEXT_BLOCKS` blocks from the `platform_malloc` family (`MetadataRoleConfig::buffer_location`; a failure fails the start, logged) and keeps them for the role's life. Each place a state can be names its block by index (`MetadataStateBuffer`): the slot's oldest and newest places (`ScheduledStateSlot`) and the main loop's current state, the one last delivered. A delivery swaps the delivered place with the current state under the Inbox mutex, so each block always has one place. `METADATA_TEXT_BLOCKS` is the fewest that keep every state the role must: the delivered state the main loop's callback is viewing, the pending one, and room for an arrival meanwhile, since the protocol task does not wait for the main loop. The protocol task copies the kept text from the parsed document into its place's block inside `ParsedJsonMessage::extract()`, the only window the document is alive, under the Inbox mutex (`MetadataRole::Impl::fill_place()`); the main loop copies the current state's text into the deprecated public strings, each reserved for its field's cap when the role is built (`ReservedText`), with no lock held. `operator new` runs after `start()` at these places: diff --git a/examples/basic_client/main.cpp b/examples/basic_client/main.cpp index b398aed3..2d7f5231 100644 --- a/examples/basic_client/main.cpp +++ b/examples/basic_client/main.cpp @@ -77,6 +77,7 @@ #include #include #include +#include #include #include @@ -418,10 +419,11 @@ int main(int argc, char* argv[]) { #ifdef SENDSPIN_ENABLE_METADATA struct BasicMetadataListener : MetadataRoleListener { - void on_metadata(const ServerMetadataStateObject& md) override { + void on_track_metadata(const TrackMetadata& md) override { if (md.title.has_value()) { - fprintf(stderr, ">>> Metadata: %s - %s\n", - md.artist.value_or("Unknown").c_str(), md.title->c_str()); + const std::string_view artist = md.artist.value_or("Unknown"); + fprintf(stderr, ">>> Metadata: %.*s - %.*s\n", static_cast(artist.size()), + artist.data(), static_cast(md.title->size()), md.title->data()); } } }; diff --git a/examples/tui_client/main.cpp b/examples/tui_client/main.cpp index 27778a5a..5fca5ea1 100644 --- a/examples/tui_client/main.cpp +++ b/examples/tui_client/main.cpp @@ -745,7 +745,7 @@ int main(int argc, char* argv[]) { TuiState& state; explicit TuiMetadataListener(TuiState& s) : state(s) {} - void on_metadata(const ServerMetadataStateObject& md) override { + void on_track_metadata(const TrackMetadata& md) override { std::lock_guard lock(state.mutex); state.title = md.title.value_or(""); state.artist = md.artist.value_or(""); @@ -758,13 +758,13 @@ int main(int argc, char* argv[]) { TuiState& state; explicit TuiClientListener(TuiState& s) : state(s) {} - void on_group_update(const GroupUpdateObject& group) override { + void on_group_state(const GroupState& changes) override { std::lock_guard lock(state.mutex); - if (group.playback_state.has_value()) { - state.playback_state = *group.playback_state; + if (changes.playback_state.has_value()) { + state.playback_state = *changes.playback_state; } - if (group.group_name.has_value()) { - state.group_name = *group.group_name; + if (changes.group_name.has_value()) { + state.group_name = *changes.group_name; } } diff --git a/examples/tui_client/tui.cpp b/examples/tui_client/tui.cpp index b04d7146..e3aded8f 100644 --- a/examples/tui_client/tui.cpp +++ b/examples/tui_client/tui.cpp @@ -1113,7 +1113,7 @@ void update_polled_state(TuiState& state, SendspinClient& client) { state.player_volume = client.player() ? client.player()->get_volume() : 0; state.player_muted = client.player() ? client.player()->get_muted() : false; #endif - state.group_name = client.get_group_state().group_name.value_or(""); + state.group_name = client.group_state().group_name.value_or(""); #ifdef SENDSPIN_ENABLE_CONTROLLER if (client.controller()) { diff --git a/include/sendspin/client.h b/include/sendspin/client.h index 49a53406..46b8be6f 100644 --- a/include/sendspin/client.h +++ b/include/sendspin/client.h @@ -57,17 +57,23 @@ class SourceRole; class VisualizerRole; #endif -// Forward declarations for listener types -struct GroupUpdateObject; - /// @brief Listener for SendspinClient events /// All methods fire on the main loop thread class SendspinClientListener { public: virtual ~SendspinClientListener() = default; - /// @brief Called when the group state is updated by the server - virtual void on_group_update(const GroupUpdateObject& /*group*/) {} + /// @brief Called when the server's group/update messages are applied, with the fields changed + /// by those since the last call; SendspinClient::group_state() returns the merged state + /// + /// The library calls this and then the deprecated on_group_update() with the same changes: + /// override one of them. A listener overriding both gets both, this one first. + virtual void on_group_state(const GroupState& /*changes*/) {} + + /// @brief Called after on_group_state() with the same changes + /// @deprecated Override on_group_state(); removed in v0.11.0. + [[deprecated("override on_group_state(); removed in v0.11.0")]] virtual void on_group_update( + const GroupUpdateObject& /*group*/) {} /// @brief Called after a time sync burst completes with the Kalman filter error virtual void on_time_sync_updated(float /*error*/) {} @@ -593,10 +599,16 @@ class SendspinClient { /// @return Equivalent client-side timestamp in microseconds, or 0 with no active connection int64_t get_client_time(int64_t server_time) const; - /// @brief Returns the current group state; fields are optional and may be unset - const GroupUpdateObject& get_group_state() const { - return this->group_state_; - } + /// @brief Returns the group state merged from every group/update applied since start(); a + /// field has no value until an update sets it. Main loop only. The views are valid until + /// control next returns to loop() (from inside a callback: until the callback returns) or + /// until stop(). + GroupState group_state() const; + + /// @brief Returns group_state() with each text field copied into a std::string. Main loop only. + /// @deprecated Use group_state(); removed in v0.11.0. + [[deprecated("use group_state(); removed in v0.11.0")]] const GroupUpdateObject& + get_group_state() const; // ======================================== // State updates @@ -1011,7 +1023,6 @@ class SendspinClient { // Struct fields SendspinClientConfig config_; - GroupUpdateObject group_state_{}; // String fields std::string client_id_; ///< Derived from the static keypair: base64url(public_key). diff --git a/include/sendspin/config.h b/include/sendspin/config.h index 72f9b17b..91a90394 100644 --- a/include/sendspin/config.h +++ b/include/sendspin/config.h @@ -497,18 +497,18 @@ struct PlayerRoleConfig { // Metadata config types // ============================================================================ -/// @brief Configuration for the metadata role: which text fields of ServerMetadataStateObject it -/// keeps. A field set false reaches the listener without a value and costs no storage +/// @brief Configuration for the metadata role: which text fields of TrackMetadata it keeps. A +/// field set false reaches the listener without a value and costs no storage /// (docs/integration-guide.md, MetadataRoleConfig). struct MetadataRoleConfig { - bool title{true}; ///< Keep ServerMetadataStateObject::title - bool artist{true}; ///< Keep ServerMetadataStateObject::artist - bool album_artist{true}; ///< Keep ServerMetadataStateObject::album_artist - bool album{true}; ///< Keep ServerMetadataStateObject::album - bool artwork_url{true}; ///< Keep ServerMetadataStateObject::artwork_url - - /// @brief Placement of the text blocks of the states in flight (ESP-IDF only). The main loop - /// copies each state's text out of them once, so PSRAM-preferring. + bool title{true}; ///< Keep TrackMetadata::title + bool artist{true}; ///< Keep TrackMetadata::artist + bool album_artist{true}; ///< Keep TrackMetadata::album_artist + bool album{true}; ///< Keep TrackMetadata::album + bool artwork_url{true}; ///< Keep TrackMetadata::artwork_url + + /// @brief Placement of the text blocks of the states in flight (ESP-IDF only). Read on the + /// main loop only as each state is delivered, so PSRAM-preferring. MemoryLocation buffer_location{MemoryLocation::PREFER_EXTERNAL}; }; diff --git a/include/sendspin/metadata_role.h b/include/sendspin/metadata_role.h index 7173becf..27f26b88 100644 --- a/include/sendspin/metadata_role.h +++ b/include/sendspin/metadata_role.h @@ -23,6 +23,7 @@ #include #include #include +#include namespace sendspin { @@ -39,11 +40,29 @@ struct MetadataProgressObject { uint32_t playback_speed; }; -/// @brief Track metadata and playback state received from the server +/// @brief Track metadata and playback state received from the server, handed to +/// MetadataRoleListener::on_track_metadata() /// -/// Each text field is at most 255 bytes in UTF-8 (artwork_url 2048), cut at a character boundary, -/// and has no value when its MetadataRoleConfig flag is false. -struct ServerMetadataStateObject { +/// Each text field views at most 255 bytes of UTF-8 (artwork_url 2048), cut at a character +/// boundary, and has no value when its MetadataRoleConfig flag is false. The views are valid for +/// the duration of the callback; copy what is needed past it. +struct TrackMetadata { + int64_t timestamp{}; + std::optional title{}; + std::optional artist{}; + std::optional album_artist{}; + std::optional album{}; + std::optional artwork_url{}; + std::optional year{}; + std::optional track{}; + std::optional progress{}; +}; + +/// @brief TrackMetadata with each text field copied into a std::string +/// @deprecated Use TrackMetadata, handed to MetadataRoleListener::on_track_metadata(); removed in +/// v0.11.0. +struct [[deprecated( + "use TrackMetadata and on_track_metadata(); removed in v0.11.0")]] ServerMetadataStateObject { int64_t timestamp{}; std::optional title; std::optional artist; @@ -60,8 +79,16 @@ class MetadataRoleListener { public: virtual ~MetadataRoleListener() = default; - /// @brief Called when metadata is updated by the server - virtual void on_metadata(const ServerMetadataStateObject& /*metadata*/) {} + /// @brief Called when the server's metadata state is applied + /// + /// The library calls this and then the deprecated on_metadata() for each state: override one + /// of them. A listener overriding both gets both, this one first. + virtual void on_track_metadata(const TrackMetadata& /*metadata*/) {} + + /// @brief Called after on_track_metadata() with the same state + /// @deprecated Override on_track_metadata(); removed in v0.11.0. + [[deprecated("override on_track_metadata(); removed in v0.11.0")]] virtual void on_metadata( + const ServerMetadataStateObject& /*metadata*/) {} /// @brief Called when the cached metadata is dropped: the connection to the server was lost, /// or a server/activate took the metadata role out of the session's active roles @@ -69,7 +96,7 @@ class MetadataRoleListener { /// Implementations should clear any displayed track metadata, since the previous state is no /// longer valid. Idempotent by contract: a second clear with nothing to clear must be a /// no-op. A role removed from an active session - /// can be added back by a later activation, which resumes with a fresh on_metadata(). + /// can be added back by a later activation, which resumes with a fresh on_track_metadata(). virtual void on_metadata_clear() {} }; @@ -93,7 +120,7 @@ class MetadataRoleListener { * * @code * struct MyMetadataListener : MetadataRoleListener { - * void on_metadata(const ServerMetadataStateObject& m) override { + * void on_track_metadata(const TrackMetadata& m) override { * if (m.title) display_title(*m.title); * if (m.artist) display_artist(*m.artist); * } diff --git a/include/sendspin/types.h b/include/sendspin/types.h index e6324f6d..abfab0e1 100644 --- a/include/sendspin/types.h +++ b/include/sendspin/types.h @@ -20,6 +20,7 @@ #include #include #include +#include namespace sendspin { @@ -52,13 +53,29 @@ enum class SendspinPlaybackState : uint8_t { STOPPED, // Group playback is stopped }; -/// @brief Group membership and playback state delta received in group/update messages -struct GroupUpdateObject { +/// @brief Group playback state and membership: the fields the group/update messages since the +/// last call changed, handed to SendspinClientListener::on_group_state(), or the state +/// SendspinClient::group_state() merges from them +/// +/// Each field has no value when those messages leave it unchanged, or, in the merged state, until +/// an update sets it. The views' lifetime is the callback's, or for the merged state the one +/// group_state() documents. +struct GroupState { std::optional playback_state{}; /// 43 base64url characters, or empty when cleared; a server id in another form is reported as /// its SHA-256 in base64url - std::optional group_id{}; + std::optional group_id{}; /// At most 64 bytes in UTF-8, cut at a character boundary + std::optional group_name{}; +}; + +/// @brief GroupState with each text field copied into a std::string +/// @deprecated Use GroupState, handed to SendspinClientListener::on_group_state() and returned by +/// SendspinClient::group_state(); removed in v0.11.0. +struct [[deprecated("use GroupState, on_group_state() and group_state(); removed in " + "v0.11.0")]] GroupUpdateObject { + std::optional playback_state{}; + std::optional group_id{}; std::optional group_name{}; }; diff --git a/src/client.cpp b/src/client.cpp index 92d54cc5..066ba007 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -202,15 +202,30 @@ struct HighPerformanceRequests { return true; } +/// @brief The public view of `group`, its text viewing the payload's inline storage +GroupState group_state_of(const GroupUpdatePayload& group) { + return {.playback_state = group.playback_state, + .group_id = optional_view(group.group_id), + .group_name = optional_view(group.group_name)}; +} + } // namespace /// @brief The main-loop-bound channels the client itself owns, beside the roles' own slots struct SendspinClient::EventState { Inbox inbox; InboxSlot group_slot{inbox, INBOX_TOPIC_GROUP}; - /// The delta on_group_update() is handed, filled in place from each group_slot payload. Main - /// loop only, like the text storage below that it and the client's group state fill from. + /// The client's group state, every group_slot payload the drain applied merged into it, + /// which group_state() views. Main loop only. + GroupUpdatePayload group_state; +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + /// The deprecated public forms of group_state and of each applied payload: the state + /// get_group_state() returns and the delta on_group_update() is handed, filled in place with + /// the text storage below. Main loop only. + GroupUpdateObject group_state_object; GroupUpdateObject group_delta; +#pragma GCC diagnostic pop ReservedText delta_group_id{PEER_ID_SIZE}; ReservedText delta_group_name{MAX_FRIENDLY_NAME_BYTES}; ReservedText state_group_id{PEER_ID_SIZE}; @@ -248,29 +263,36 @@ struct SendspinClient::EventState { [¬e](std::vector& current) { current.push_back(note); }); } - /// @brief Applies `payload` to the client's group state `state` and fills group_delta with - /// it, both in place. Main loop only. - void apply_group_update(GroupUpdateObject& state, const GroupUpdatePayload& payload) { - if (payload.playback_state.has_value()) { - state.playback_state = payload.playback_state; - } - if (payload.group_id.has_value()) { - this->state_group_id.fill(state.group_id, payload.group_id->view()); - } - if (payload.group_name.has_value()) { - this->state_group_name.fill(state.group_name, payload.group_name->view()); - } + /// @brief Merges `payload` into group_state and refills group_state_object from it. Main loop + /// only. + void apply_group_update(const GroupUpdatePayload& payload) { + apply_group_update_deltas(&this->group_state, payload); + this->group_state_object.playback_state = this->group_state.playback_state; + this->state_group_id.fill(this->group_state_object.group_id, + optional_view(this->group_state.group_id)); + this->state_group_name.fill(this->group_state_object.group_name, + optional_view(this->group_state.group_name)); + } + + /// @brief Fills group_delta from `payload` and hands it to the deprecated on_group_update(). + /// Main loop only. + void deliver_group_update(SendspinClientListener& listener, const GroupUpdatePayload& payload) { this->group_delta.playback_state = payload.playback_state; this->delta_group_id.fill(this->group_delta.group_id, optional_view(payload.group_id)); this->delta_group_name.fill(this->group_delta.group_name, optional_view(payload.group_name)); +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + listener.on_group_update(this->group_delta); +#pragma GCC diagnostic pop } - /// @brief Clears the client's group state `state`, keeping its text storage. Main loop only. - void clear_group_state(GroupUpdateObject& state) { - state.playback_state.reset(); - this->state_group_id.fill(state.group_id, std::nullopt); - this->state_group_name.fill(state.group_name, std::nullopt); + /// @brief Clears the client's group state, keeping its text storage. Main loop only. + void clear_group_state() { + this->group_state = {}; + this->group_state_object.playback_state.reset(); + this->state_group_id.fill(this->group_state_object.group_id, std::nullopt); + this->state_group_name.fill(this->group_state_object.group_name, std::nullopt); } }; @@ -588,11 +610,11 @@ void SendspinClient::stop() { // leaves behind is the state a restart begins from. The drain generation bump abandons // the events a drain frame further up the stack (a listener that called stop()) copied // out before this teardown. The group slot is reset too, so the drain below cannot - // repopulate group_state_ from a delta that arrived before the stop. + // repopulate the group state from a delta that arrived before the stop. ++this->event_state_->drain_generation; this->cleanup_connection_state(ALL_ROLES_MASK); this->task_state_->client_state.reset(); - this->event_state_->clear_group_state(this->group_state_); + this->event_state_->clear_group_state(); // A pairing attempt cut short by the stop leaves its code or pairing-window prompt showing. // Queue the dismissals now, after cleanup_connection_state() dropped the pending notes, so the @@ -1178,23 +1200,26 @@ void SendspinClient::drain_inbox() { if (!overtaken() && (slot_bits & INBOX_TOPIC_GROUP) != 0) { GroupUpdatePayload payload; if (es.group_slot.take(payload)) { - es.apply_group_update(this->group_state_, payload); - - if (this->listener_) { - this->listener_->on_group_update(es.group_delta); - } + es.apply_group_update(payload); // Both texts are bounded at parse (IdText, ServerName), so they print whole. - const GroupUpdateObject& group = this->group_state_; - const std::string_view id = - group.group_id.has_value() ? std::string_view(*group.group_id) : ""; - const std::string_view name = - group.group_name.has_value() ? std::string_view(*group.group_name) : ""; + const GroupState group = this->group_state(); + const std::string_view id = group.group_id.value_or(""); + const std::string_view name = group.group_name.value_or(""); SS_LOGD(TAG, "Group update - state: %s, id: %.*s, name: %.*s", group.playback_state.has_value() ? to_cstr(group.playback_state.value()) : "unchanged", static_cast(id.size()), id.data(), static_cast(name.size()), name.data()); + + if (this->listener_) { + this->listener_->on_group_state(group_state_of(payload)); + // A stop() from on_group_state() cleared the group state, so the deprecated form + // of the update must not follow. + if (!overtaken()) { + es.deliver_group_update(*this->listener_, payload); + } + } } } } @@ -1336,6 +1361,17 @@ std::optional SendspinClient::get_server_information() return this->connection_manager_->server_information(); } +GroupState SendspinClient::group_state() const { + return group_state_of(this->event_state_->group_state); +} + +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" +const GroupUpdateObject& SendspinClient::get_group_state() const { + return this->event_state_->group_state_object; +} +#pragma GCC diagnostic pop + // ============================================================================ // State updates // ============================================================================ diff --git a/src/metadata_role.cpp b/src/metadata_role.cpp index 1e87c24d..e87ab599 100644 --- a/src/metadata_role.cpp +++ b/src/metadata_role.cpp @@ -46,8 +46,8 @@ EnumSet kept_fields(const MetadataRoleConfig& config) { return kept; } -/// @brief Text storage for each text field of the public state, reserved for the field's cap when -/// `layout` keeps it, and for nothing when it does not +/// @brief Text storage for each text field of the deprecated public state, reserved for the field's +/// cap when `layout` keeps it, and for nothing when it does not std::array reserve_metadata_text( const MetadataTextLayout& layout) { const auto room = [&layout](MetadataField field) { @@ -59,7 +59,9 @@ std::array reserve_metadata_text( ReservedText(room(MetadataField::ARTWORK_URL))}; } -/// @brief The public state's member for `field` +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" +/// @brief The deprecated public state's member for `field` std::optional& text_field(ServerMetadataStateObject& state, MetadataField field) { switch (field) { case MetadataField::TITLE: @@ -75,6 +77,7 @@ std::optional& text_field(ServerMetadataStateObject& state, Metadat } return state.title; } +#pragma GCC diagnostic pop } // namespace @@ -115,25 +118,27 @@ uint32_t MetadataRole::get_track_progress_ms() const { // ============================================================================ uint32_t MetadataRole::Impl::get_track_duration_ms() const { - if (!this->metadata.progress.has_value()) { + const MetadataStatePayload& payload = this->current.payload; + if (!payload.progress.has_value()) { return 0; } - return this->metadata.progress.value().track_duration; + return payload.progress.value().track_duration; } uint32_t MetadataRole::Impl::get_track_progress_ms() const { - if (!this->metadata.progress.has_value()) { + const MetadataStatePayload& payload = this->current.payload; + if (!payload.progress.has_value()) { return 0; } - const auto& progress = this->metadata.progress.value(); + const auto& progress = payload.progress.value(); // If paused (playback_speed == 0), return the snapshot value directly if (progress.playback_speed == 0) { return progress.track_progress; } - int64_t client_target = this->client->get_client_time(this->metadata.timestamp); + int64_t client_target = this->client->get_client_time(payload.timestamp); if (client_target == 0) { return progress.track_progress; } @@ -225,29 +230,57 @@ std::optional MetadataRole::Impl::text_of(const MetadataStateB return std::string_view(block + this->layout.offsets[i], *size); } -void MetadataRole::Impl::fill_public_state(const MetadataStateBuffer& state) { - const MetadataStatePayload& payload = state.payload; +TrackMetadata MetadataRole::Impl::track_metadata() const { + const MetadataStatePayload& payload = this->current.payload; + return {.timestamp = payload.timestamp, + .title = this->text_of(this->current, MetadataField::TITLE), + .artist = this->text_of(this->current, MetadataField::ARTIST), + .album_artist = this->text_of(this->current, MetadataField::ALBUM_ARTIST), + .album = this->text_of(this->current, MetadataField::ALBUM), + .artwork_url = this->text_of(this->current, MetadataField::ARTWORK_URL), + .year = payload.year, + .track = payload.track, + .progress = payload.progress}; +} + +void MetadataRole::Impl::fill_deprecated_state() { + const MetadataStatePayload& payload = this->current.payload; this->metadata.timestamp = payload.timestamp; this->metadata.year = payload.year; this->metadata.track = payload.track; this->metadata.progress = payload.progress; for (size_t i = 0; i < METADATA_FIELD_COUNT; ++i) { const auto field = static_cast(i); - this->metadata_text[i].fill(text_field(this->metadata, field), this->text_of(state, field)); + this->metadata_text[i].fill(text_field(this->metadata, field), + this->text_of(this->current, field)); } } void MetadataRole::Impl::apply_current_state() { - this->fill_public_state(this->current); - if (this->listener) { - this->listener->on_metadata(this->metadata); + if (this->listener == nullptr) { + return; + } + // Moves only when the main loop runs a teardown's half, which during the callback means the + // listener called stop() (a callback never calls loop()). Not accepts(): the protocol task may + // bump the generation during the callback, and the deprecated form must still follow then; + // the clear that teardown queued comes on a later drain. + const uint32_t completed = this->teardown.completed(); + this->listener->on_track_metadata(this->track_metadata()); + // The teardown delivered its clear: the deprecated form of the state must not follow it. + if (this->teardown.completed() != completed) { + return; } + this->fill_deprecated_state(); +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + this->listener->on_metadata(this->metadata); +#pragma GCC diagnostic pop } void MetadataRole::Impl::drain_events() { // A delivery trades blocks rather than copying text: the delivered place takes the current - // state's block, which no state still to be delivered names, and the text is copied once, out - // of the current block into the public state, with no lock held. + // state's block, which no state still to be delivered names, and the listener views the text + // in the current block, with no lock held. drain_scheduled_state( *this, this->event_state->slot, [this](MetadataStateBuffer& place) { std::swap(place, this->current); }, @@ -265,7 +298,8 @@ void MetadataRole::Impl::cleanup() { } void MetadataRole::Impl::complete_teardown() { - this->fill_public_state(MetadataStateBuffer{}); + this->current.payload = {}; + this->fill_deprecated_state(); this->event_state->slot.forget_pending(); if (this->listener) { this->listener->on_metadata_clear(); diff --git a/src/metadata_role_impl.h b/src/metadata_role_impl.h index 0454d496..cf0d97d2 100644 --- a/src/metadata_role_impl.h +++ b/src/metadata_role_impl.h @@ -110,12 +110,14 @@ struct MetadataRole::Impl : RoleTeardown { void drain_events(); /// Whether a state's server-clock deadline has passed on the synchronized client clock. bool state_is_due(int64_t timestamp) const; - /// @brief Fills the public state from the current state and fires on_metadata(). Main loop, - /// with no lock held. + /// @brief Hands the current state to on_track_metadata(), then to the deprecated + /// on_metadata() unless the first call re-entered teardown. Main loop, with no lock held. void apply_current_state(); - /// @brief Fills the public state from `state`, its text from the block it names; an empty - /// state clears every field, keeping the text storage. Main loop. - void fill_public_state(const MetadataStateBuffer& state); + /// @brief The public view of the current state, its text viewing the current block. Main loop. + TrackMetadata track_metadata() const; + /// @brief Fills the deprecated public state from the current state, keeping the text storage. + /// Main loop. + void fill_deprecated_state(); /// @brief The text `state` carries for `field`, viewing its block; nullopt when it carries /// none std::optional text_of(const MetadataStateBuffer& state, @@ -131,8 +133,9 @@ struct MetadataRole::Impl : RoleTeardown { /// (catch_up_teardown()). void cleanup(); - /// @brief The main-loop teardown half: resets the state only the main loop touches and fires - /// on_metadata_clear(). Main loop only, through catch_up_teardown(). + /// @brief The main-loop teardown half: empties the current state, keeping its block, and the + /// deprecated public state, and fires on_metadata_clear(). Main loop only, through + /// catch_up_teardown(). void complete_teardown(); // ======================================== @@ -153,14 +156,19 @@ struct MetadataRole::Impl : RoleTeardown { /// METADATA_TEXT_BLOCKS blocks of layout.block_bytes, allocated by start(); empty when no /// field is kept. PlatformBuffer text_blocks; - /// The applied state, the one on_metadata() is handed. Main loop only. + /// The state the last delivery handed over, which the listener and the progress getters read. + /// Main loop only; the slot's commit swaps it, under the Inbox mutex, for the state it + /// delivers. + MetadataStateBuffer current{.block = CURRENT_FIRST_BLOCK}; +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + /// The deprecated public state on_metadata() is handed, filled from the current state. Main + /// loop only. ServerMetadataStateObject metadata{}; - /// Storage for the public state's text fields, reserved for each kept field's cap. Main loop - /// only. +#pragma GCC diagnostic pop + /// Storage for the deprecated public state's text fields, reserved for each kept field's cap. + /// Main loop only. std::array metadata_text; - /// The state the last delivery handed over, whose text the public state is filled from. Main - /// loop only; the slot's commit swaps it, under the Inbox mutex, for the state it delivers. - MetadataStateBuffer current{.block = CURRENT_FIRST_BLOCK}; // Pointer fields SendspinClient* client; diff --git a/src/protocol_messages.h b/src/protocol_messages.h index c482beab..8d45fbc0 100644 --- a/src/protocol_messages.h +++ b/src/protocol_messages.h @@ -766,8 +766,8 @@ constexpr MetadataTextLayout metadata_text_layout(EnumSet kept) { return layout; } -/// @brief A server/state metadata object's state: ServerMetadataStateObject with its text in a -/// block laid out by a MetadataTextLayout; the main loop copies the text into the public object +/// @brief A server/state metadata object's state: TrackMetadata with its text in a block laid out +/// by a MetadataTextLayout, which the public object views struct MetadataStatePayload { int64_t timestamp{}; std::optional year{}; @@ -1184,9 +1184,9 @@ struct ServerActivateMessage { std::optional pairing_format; }; -/// @brief A parsed group/update delta: GroupUpdateObject with its text held inline, which the -/// main loop copies into the public object. A field the message does not carry is nullopt, and an -/// empty text clears the field (the wire's null). +/// @brief A parsed group/update delta: GroupState with its text held inline, which the public +/// object views. A field the message does not carry is nullopt, and an empty text clears the field +/// (the wire's null). struct GroupUpdatePayload { std::optional playback_state{}; std::optional group_id{}; diff --git a/src/teardown_tracker.h b/src/teardown_tracker.h index 99ef650b..c29cc2b1 100644 --- a/src/teardown_tracker.h +++ b/src/teardown_tracker.h @@ -62,6 +62,11 @@ class TeardownTracker { return true; } + /// @brief The latest teardown generation whose main-loop half has run + uint32_t completed() const { + return this->completed_; + } + private: /// The latest teardown generation whose main-loop half has run. uint32_t completed_{0}; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 3201a3dc..7df13796 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -147,8 +147,8 @@ endif() # visualizer's handle_stream_start(), each role's cleanup()) and drain step (drain_events(), the # visualizer's catch_up_teardown()), writes the generation-stamped slots (event_state, artwork's # display_slot under its slot_epochs) and queues a STREAM_START (enqueue_stream_event()) with an -# old stamp, and reads cleanup_generation and the applied state (controller_state, metadata, -# color, the player's stream_active, current_stream_params and volume); its player runs with the +# old stamp, and reads cleanup_generation and the applied state (controller_state, the metadata +# role's current, color, the player's stream_active, current_stream_params and volume); its player runs with the # sync thread stopped and the item list bound by hand (SyncTask's stop() and inbound_), so a # STREAM_END never waits for a thread to go idle. The high-performance grant tables # play the protocol task's run_time_sync() on a stand-in in the admitted slot (find_admitted(), @@ -334,6 +334,10 @@ set_source_files_properties(test_pairing_state_machine.cpp PROPERTIES # - MetadataRole::impl_'s slot places (ScheduledStateSlot::slot_, read under the Inbox mutex) # and text_of(): which state waits for a deadline the test never reaches shows nowhere else # before that deadline. +# - MetadataRole::impl_'s cleanup(), the protocol task's half of a teardown, run on the test +# thread as the dispatch fixtures run the protocol task's handlers: from inside a listener +# callback, to show the deprecated form still follows a teardown that is not the listener's +# own stop(), and to show the progress getters read 0 once a teardown is caught up. set_source_files_properties(test_encrypted_lifecycle.cpp PROPERTIES COMPILE_OPTIONS "-fno-access-control") diff --git a/tests/lifecycle_test_fixtures.h b/tests/lifecycle_test_fixtures.h index 04d04ede..03dc99ee 100644 --- a/tests/lifecycle_test_fixtures.h +++ b/tests/lifecycle_test_fixtures.h @@ -1248,7 +1248,7 @@ class CountingPlayerListener : public PlayerRoleListener { /// Records what each metadata update carried, and counts the clears behind a teardown. class RecordingMetadataListener : public MetadataRoleListener { public: - void on_metadata(const ServerMetadataStateObject& metadata) override { + void on_track_metadata(const TrackMetadata& metadata) override { this->last_title = metadata.title.value_or(""); this->last_artist = metadata.artist.value_or(""); this->last_had_progress = metadata.progress.has_value(); diff --git a/tests/test_allocations.cpp b/tests/test_allocations.cpp index 621a79a2..d92791a3 100644 --- a/tests/test_allocations.cpp +++ b/tests/test_allocations.cpp @@ -422,32 +422,61 @@ void pump_while_not(SendspinClient& client, Pred pred) { /// Counts the completed time bursts the main loop reports. on_time_sync_updated() comes only after /// a burst's last reply and its completion have run on the protocol task, so the count bounds a -/// burst on the client's side. Main loop (the test thread) only. +/// burst on the client's side. Also records, without allocating, the size of the group name each +/// group listener form was last handed, and counts the updates. Main loop (the test thread) only. class BurstCountingListener : public SendspinClientListener { public: void on_time_sync_updated(float /*error*/) override { ++this->bursts; } + void on_group_state(const GroupState& changes) override { + this->view_name_size = changes.group_name.has_value() ? changes.group_name->size() : 0; + ++this->group_updates; + } +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + void on_group_update(const GroupUpdateObject& group) override { + this->string_name_size = group.group_name.has_value() ? group.group_name->size() : 0; + } +#pragma GCC diagnostic pop int bursts{0}; + int group_updates{0}; + size_t view_name_size{0}; + size_t string_name_size{0}; }; -/// Records what each metadata state carried without allocating: its text fields' sizes and -/// whether it carried an artist. Main loop (the test thread) only. +/// Records what each metadata state carried without allocating, in both listener forms: its text +/// fields' sizes and whether it carried an artist. Main loop (the test thread) only. class TextSizeMetadataListener : public MetadataRoleListener { public: - void on_metadata(const ServerMetadataStateObject& metadata) override { - this->title_size = metadata.title.has_value() ? metadata.title->size() : 0; - this->artwork_url_size = - metadata.artwork_url.has_value() ? metadata.artwork_url->size() : 0; - this->has_artist = metadata.artist.has_value(); + struct Sizes { + size_t title_size{0}; + size_t artwork_url_size{0}; + bool has_artist{false}; + bool operator==(const Sizes&) const = default; + }; + + void on_track_metadata(const TrackMetadata& metadata) override { + this->sizes = { + .title_size = metadata.title.has_value() ? metadata.title->size() : 0, + .artwork_url_size = metadata.artwork_url.has_value() ? metadata.artwork_url->size() : 0, + .has_artist = metadata.artist.has_value()}; ++this->updates; } +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + void on_metadata(const ServerMetadataStateObject& metadata) override { + this->deprecated_sizes = { + .title_size = metadata.title.has_value() ? metadata.title->size() : 0, + .artwork_url_size = metadata.artwork_url.has_value() ? metadata.artwork_url->size() : 0, + .has_artist = metadata.artist.has_value()}; + } +#pragma GCC diagnostic pop int updates{0}; - size_t title_size{0}; - size_t artwork_url_size{0}; - bool has_artist{false}; + Sizes sizes; + Sizes deprecated_sizes; }; /// Closes the open window in on_stream_start() when asked to. The main loop calls it before it @@ -564,9 +593,10 @@ constexpr size_t UNTAKEN_STREAM_START_ALLOCATIONS = 0; // stream/clear that name their roles, then the sync task's first FLAC codec header, then a FLAC // stream/start the player takes, then a stream/start whose player section the connection, no // longer owning the player, does not take. -// The roles' public command and type lists, the metadata state's texts and the client's group -// texts are reserved when they are built, so even the first fill on the main loop allocates -// nothing. +// The roles' public command and type lists and the strings of the deprecated metadata and group +// objects are reserved when they are built, so even the first fill on the main loop allocates +// nothing; the metadata and group listeners override both forms, so each window counts the views +// and the strings. TEST(Allocations, AcceptToAdmissionAndPerMessagePaths) { // Each string but the MAC address is longer than libc++'s 22-character inline capacity, so the // host counts every copy libstdc++ (15 characters) makes of a realistic one. @@ -665,7 +695,8 @@ TEST(Allocations, AcceptToAdmissionAndPerMessagePaths) { // before the next is sent. const std::string_view normalized_id = "FjYvVmOHs89abpL7Cphsdsog6zoMEsvfvQspUB4MGN8"; const std::string_view conforming_id = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMN-_0"; - const GroupUpdateObject& group = client.get_group_state(); + const std::string_view last_name = "Allocation Test Upstairs Bedroom Group"; + const int group_updates_before = burst_listener.group_updates; open_window(); { TestOwned test_owned; @@ -674,23 +705,25 @@ TEST(Allocations, AcceptToAdmissionAndPerMessagePaths) { R"("group_id":"3f2504e0-4f89-41d3-9a0c-0305e82c3301",)" R"("group_name":"Allocation Test Kitchen and Living Room"}})")); } - pump_while_not(client, [&] { return group.group_id == normalized_id; }); + pump_while_not(client, [&] { return client.group_state().group_id == normalized_id; }); { TestOwned test_owned; ASSERT_TRUE(server->send_app_json( R"({"type":"group/update","payload":{"group_id":")" + std::string(conforming_id) + R"("}})")); } - pump_while_not(client, [&] { return group.group_id == conforming_id; }); + pump_while_not(client, [&] { return client.group_state().group_id == conforming_id; }); { TestOwned test_owned; ASSERT_TRUE(server->send_app_json( R"({"type":"group/update","payload":{"group_id":")" + std::string(conforming_id) + R"(","group_name":"Allocation Test Upstairs Bedroom Group"}})")); } - pump_while_not(client, - [&] { return group.group_name == "Allocation Test Upstairs Bedroom Group"; }); + pump_while_not(client, [&] { return client.group_state().group_name == last_name; }); expect_library_allocations(close_window(), "group/update", GROUP_UPDATE_ALLOCATIONS); + EXPECT_EQ(burst_listener.group_updates - group_updates_before, 3); + EXPECT_EQ(burst_listener.view_name_size, last_name.size()); + EXPECT_EQ(burst_listener.string_name_size, last_name.size()); // A first state carrying every text field, its title over the cap, then a second without an // artist, then one scheduled a little ahead, which normally waits in the role's slot as its @@ -715,16 +748,18 @@ TEST(Allocations, AcceptToAdmissionAndPerMessagePaths) { pump_while_not(client, [&] { return metadata_listener.updates == 1; }); { TestOwned test_owned; - EXPECT_EQ(metadata_listener.title_size, MAX_METADATA_TEXT_BYTES); - EXPECT_EQ(metadata_listener.artwork_url_size, url.size()); - EXPECT_TRUE(metadata_listener.has_artist); + EXPECT_EQ(metadata_listener.sizes.title_size, MAX_METADATA_TEXT_BYTES); + EXPECT_EQ(metadata_listener.sizes.artwork_url_size, url.size()); + EXPECT_TRUE(metadata_listener.sizes.has_artist); + EXPECT_TRUE(metadata_listener.deprecated_sizes == metadata_listener.sizes); ASSERT_TRUE(server->send_app_json( metadata_state(2, R"(,"title":"Allocation Test Second Track Title")" + artwork_url))); } pump_while_not(client, [&] { return metadata_listener.updates == 2; }); { TestOwned test_owned; - EXPECT_FALSE(metadata_listener.has_artist) << "the second state carried no artist"; + EXPECT_FALSE(metadata_listener.sizes.has_artist) << "the second state carried no artist"; + EXPECT_TRUE(metadata_listener.deprecated_sizes == metadata_listener.sizes); ASSERT_TRUE(server->send_app_json( metadata_state(platform_time_us() + 50 * static_cast(US_PER_MS), R"(,"title":"Allocation Test Scheduled Track Title")"))); @@ -737,7 +772,8 @@ TEST(Allocations, AcceptToAdmissionAndPerMessagePaths) { pump_while_not(client, [&] { return metadata_listener.updates == 4; }); expect_library_allocations(close_window(), "metadata server/state", METADATA_STATE_ALLOCATIONS); - EXPECT_EQ(metadata_listener.title_size, 0U) << "the last state cleared the title"; + EXPECT_EQ(metadata_listener.sizes.title_size, 0U) << "the last state cleared the title"; + EXPECT_TRUE(metadata_listener.deprecated_sizes == metadata_listener.sizes); const int starts_before_streams = visualizer_listener.stream_starts; diff --git a/tests/test_client_lifecycle.cpp b/tests/test_client_lifecycle.cpp index 11c73209..e44f041e 100644 --- a/tests/test_client_lifecycle.cpp +++ b/tests/test_client_lifecycle.cpp @@ -197,8 +197,7 @@ class ReentrantMetadataListener : public MetadataRoleListener { void on_metadata_clear() override { ++this->clears; this->started_during_clear = this->client_.is_started(); - this->group_had_state_during_clear = - this->client_.get_group_state().playback_state.has_value(); + this->group_had_state_during_clear = this->client_.group_state().playback_state.has_value(); this->start_result_during_clear = this->client_.start(); this->client_.stop(); // Must be ignored, not recurse this->client_.disconnect(SendspinGoodbyeReason::SHUTDOWN); // Must be ignored @@ -217,9 +216,15 @@ class ReentrantMetadataListener : public MetadataRoleListener { /// A metadata listener that must never be called; every callback aborts the test. class ForbiddenMetadataListener : public MetadataRoleListener { public: + void on_track_metadata(const TrackMetadata& /*metadata*/) override { + ADD_FAILURE() << "on_track_metadata() fired on a listener the consumer already released"; + } +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" void on_metadata(const ServerMetadataStateObject& /*metadata*/) override { ADD_FAILURE() << "on_metadata() fired on a listener the consumer already released"; } +#pragma GCC diagnostic pop void on_metadata_clear() override { ADD_FAILURE() << "on_metadata_clear() fired on a listener the consumer already released"; } @@ -259,16 +264,23 @@ TEST(ClientLifecycle, RestartYieldsALiveClient) { // Some group state for stop() to reset. ASSERT_TRUE(server->send_app_json(group_update_playing_json())); - pump_until(client, [&] { return client.get_group_state().playback_state.has_value(); }); + pump_until(client, [&] { return client.group_state().playback_state.has_value(); }); client.stop(); EXPECT_FALSE(client.is_started()); EXPECT_FALSE(client.is_connected()); EXPECT_FALSE(client.get_server_information().has_value()); - EXPECT_FALSE(client.get_group_state().playback_state.has_value()); - EXPECT_FALSE(client.get_group_state().group_id.has_value()); - EXPECT_FALSE(client.get_group_state().group_name.has_value()); + EXPECT_FALSE(client.group_state().playback_state.has_value()); + EXPECT_FALSE(client.group_state().group_id.has_value()); + EXPECT_FALSE(client.group_state().group_name.has_value()); +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + const GroupUpdateObject& deprecated_group = client.get_group_state(); + EXPECT_FALSE(deprecated_group.playback_state.has_value()); + EXPECT_FALSE(deprecated_group.group_id.has_value()); + EXPECT_FALSE(deprecated_group.group_name.has_value()); +#pragma GCC diagnostic pop // The peer received its goodbye and the close, in that order. wait_until([&] { return server->closed(); }); EXPECT_EQ(server->goodbye_reason().value_or(""), "shutdown"); @@ -414,7 +426,7 @@ TEST(ClientLifecycle, CallbackDuringStopCannotRecurse) { pump_until(client, [&] { return client.is_connected(); }); // Group state the callback must already see reset. ASSERT_TRUE(server->send_app_json(group_update_playing_json())); - pump_until(client, [&] { return client.get_group_state().playback_state.has_value(); }); + pump_until(client, [&] { return client.group_state().playback_state.has_value(); }); client.stop(); @@ -2376,7 +2388,7 @@ class StateRoleLog : public ControllerRoleListener, void on_controller_state_clear() override { this->calls.push_back("clear"); } - void on_metadata(const ServerMetadataStateObject& metadata) override { + void on_track_metadata(const TrackMetadata& metadata) override { this->calls.push_back("state " + std::to_string(metadata.year.value_or(0))); } void on_metadata_clear() override { @@ -2536,7 +2548,7 @@ const StateRoleAccess STATE_ROLES[] = { [](SendspinClient& c) { c.metadata_->impl_->drain_events(); }, [](SendspinClient& c) { return c.metadata_->impl_->cleanup_generation.load(); }, [](SendspinClient& c, const StateRoleLog&) { - return static_cast(c.metadata_->impl_->metadata.year.value_or(0)); + return static_cast(c.metadata_->impl_->current.payload.year.value_or(0)); }, true, false}, {"color", @@ -2870,14 +2882,14 @@ TEST(ClientLifecycle, StopFromInsideADrainCallbackIsSafe) { // test thread plays: same-window arrivals (a third replacing the scheduled second), a pending // state replaced by an arrival, a teardown with states pending, a state stamped before it, an // arrival between a drain's peek and its commit (from the clear its catch-up delivers), a pending -// state that falls due in the callback of the state before it, stop() from on_metadata(), and a -// restart. Reaches the role's private state because which block each place names and which state -// waits show nowhere else before a deadline the test never reaches; the primary clock is installed -// by hand, so a scheduled state waits for its deadline without a server. +// state that falls due in the callback of the state before it, stop() from on_track_metadata(), +// and a restart. Reaches the role's private state because which block each place names and which +// state waits show nowhere else before a deadline the test never reaches; the primary clock is +// installed by hand, so a scheduled state waits for its deadline without a server. TEST(MetadataSlot, ThreeBlocksCarryEveryStateToTheListenerInOrder) { struct TitleLog : MetadataRoleListener { - void on_metadata(const ServerMetadataStateObject& metadata) override { - this->titles.push_back(metadata.title.value_or("")); + void on_track_metadata(const TrackMetadata& metadata) override { + this->titles.emplace_back(metadata.title.value_or("")); if (this->on_title) { std::exchange(this->on_title, nullptr)(this->titles.back()); } diff --git a/tests/test_encrypted_lifecycle.cpp b/tests/test_encrypted_lifecycle.cpp index 939d94b9..413a587f 100644 --- a/tests/test_encrypted_lifecycle.cpp +++ b/tests/test_encrypted_lifecycle.cpp @@ -1639,7 +1639,7 @@ TEST(EncryptedLifecycle, ActivateThatLosesPlaybackCapabilityRemovesTheRoles) { struct ClearRecordingMetadataListener : MetadataRoleListener { std::atomic updates{0}; std::atomic clears{0}; - void on_metadata(const ServerMetadataStateObject& /*m*/) override { + void on_track_metadata(const TrackMetadata& /*m*/) override { this->updates.fetch_add(1); } void on_metadata_clear() override { @@ -2028,7 +2028,7 @@ TEST(EncryptedLifecycle, RoleTrafficBeforeAdmissionIsIgnored) { struct RecordingMetadataListener : MetadataRoleListener { std::atomic updates{0}; std::string last_title; - void on_metadata(const ServerMetadataStateObject& m) override { + void on_track_metadata(const TrackMetadata& m) override { this->last_title = m.title.value_or(""); this->updates.fetch_add(1); } @@ -2172,18 +2172,91 @@ TEST(EncryptedLifecycle, MetadataStateReplacesRatherThanMerges) { EXPECT_FALSE(bundle.listener.last_had_progress) << "an omitted progress clears the position"; } +namespace { + +// One metadata state as a listener form handed it, its text copied, so the two forms compare +struct MetadataCopy { + int64_t timestamp{}; + std::optional title; + std::optional artist; + std::optional album_artist; + std::optional album; + std::optional artwork_url; + std::optional year; + std::optional track; + std::optional> progress; + bool operator==(const MetadataCopy&) const = default; +}; + +std::optional copy_text(std::optional text) { + return text.has_value() ? std::optional(*text) : std::nullopt; +} + +std::optional> copy_progress( + const std::optional& progress) { + if (!progress.has_value()) { + return std::nullopt; + } + return std::array{progress->track_progress, progress->track_duration, + progress->playback_speed}; +} + +MetadataCopy copy_of(const TrackMetadata& m) { + return {.timestamp = m.timestamp, + .title = copy_text(m.title), + .artist = copy_text(m.artist), + .album_artist = copy_text(m.album_artist), + .album = copy_text(m.album), + .artwork_url = copy_text(m.artwork_url), + .year = m.year, + .track = m.track, + .progress = copy_progress(m.progress)}; +} + +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" +MetadataCopy copy_of(const ServerMetadataStateObject& m) { + return {.timestamp = m.timestamp, + .title = m.title, + .artist = m.artist, + .album_artist = m.album_artist, + .album = m.album, + .artwork_url = m.artwork_url, + .year = m.year, + .track = m.track, + .progress = copy_progress(m.progress)}; +} +#pragma GCC diagnostic pop + +} // namespace + // MetadataRoleConfig: the listener is handed the text fields the role keeps and no value // for the others, and a kept field over its cap (roles/metadata/v1.md "server/state metadata // object") is cut to it at a character boundary, on every state that carries one: the warning the -// connection logs once does not stand in for the value. +// connection logs once does not stand in for the value. A state without text has none. Both +// listener forms are handed the same content, and on_track_metadata()'s text views storage of its +// own rather than the strings the deprecated on_metadata() is handed, which carry the same text. TEST(EncryptedLifecycle, MetadataKeepsTheConfiguredFieldsCutToTheirCaps) { - struct StateListener : MetadataRoleListener { - std::vector states; + struct BothFormsListener : MetadataRoleListener { + std::vector views; + std::vector strings; + std::vector view_titles; // Where each view's title points + std::vector string_titles; // Where each deprecated title points + void on_track_metadata(const TrackMetadata& metadata) override { + this->views.push_back(copy_of(metadata)); + this->view_titles.push_back(metadata.title.has_value() ? metadata.title->data() + : nullptr); + } +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" void on_metadata(const ServerMetadataStateObject& metadata) override { - this->states.push_back(metadata); + this->strings.push_back(copy_of(metadata)); + this->string_titles.push_back(metadata.title.has_value() ? metadata.title->data() + : nullptr); } +#pragma GCC diagnostic pop }; - StateListener listener; // Outlives the client + BothFormsListener listener; // Outlives the client DispatchTestClient bundle( "Metadata Fields Test Client", SendspinClientConfig::DEFAULT_JSON_ARENA_SIZE, nullptr, MetadataRoleConfig{.artist = false, .album_artist = false, .album = false}); @@ -2196,7 +2269,9 @@ TEST(EncryptedLifecycle, MetadataKeepsTheConfiguredFieldsCutToTheirCaps) { std::to_string(timestamp) + R"(,"title":")" + title + R"(","artist":"Band","album_artist":"Various","album":"Record",)" R"("artwork_url":")" + - url + R"(","year":1999}}})"; + url + + R"(","year":1999,"track":4,"progress":{"track_progress":10,)" + R"("track_duration":2000,"playback_speed":1000}}}})"; }; // U+00E9 straddles the title's cap, so the cut lands before it. const std::string straddling = std::string(MAX_METADATA_TEXT_BYTES - 1, 'a') + "\xC3\xA9"; @@ -2208,27 +2283,77 @@ TEST(EncryptedLifecycle, MetadataKeepsTheConfiguredFieldsCutToTheirCaps) { bundle.pump(); bundle.deliver(conn, state(3, std::string(MAX_METADATA_TEXT_BYTES + 1, 'b'), "http://x")); bundle.pump(); + bundle.deliver(conn, R"({"type":"server/state","payload":{"metadata":{"timestamp":4}}})"); + bundle.pump(); struct Expected { - std::string title; - std::string artwork_url; + std::optional title; + std::optional artwork_url; + std::optional year; }; const Expected expected[] = { - {"Song", "http://example.com/art.jpg"}, + {"Song", "http://example.com/art.jpg", 1999}, {straddling.substr(0, MAX_METADATA_TEXT_BYTES - 1), - long_url.substr(0, MAX_METADATA_URL_BYTES)}, - {std::string(MAX_METADATA_TEXT_BYTES, 'b'), "http://x"}, + long_url.substr(0, MAX_METADATA_URL_BYTES), 1999}, + {std::string(MAX_METADATA_TEXT_BYTES, 'b'), "http://x", 1999}, + {std::nullopt, std::nullopt, std::nullopt}, }; - ASSERT_EQ(listener.states.size(), std::size(expected)); + ASSERT_EQ(listener.views.size(), std::size(expected)); + ASSERT_EQ(listener.strings.size(), std::size(expected)); for (size_t i = 0; i < std::size(expected); ++i) { SCOPED_TRACE(i); - const ServerMetadataStateObject& got = listener.states[i]; + const MetadataCopy& got = listener.views[i]; + EXPECT_EQ(got.timestamp, static_cast(i + 1)); EXPECT_EQ(got.title, expected[i].title); EXPECT_EQ(got.artwork_url, expected[i].artwork_url); EXPECT_EQ(got.artist, std::nullopt); EXPECT_EQ(got.album_artist, std::nullopt); EXPECT_EQ(got.album, std::nullopt); - EXPECT_EQ(got.year, 1999) << "a field that is not text is always kept"; + EXPECT_EQ(got.year, expected[i].year) << "a field that is not text is always kept"; + EXPECT_EQ(got.track.has_value(), expected[i].year.has_value()); + EXPECT_EQ(got.progress.has_value(), expected[i].year.has_value()); + EXPECT_TRUE(listener.strings[i] == got) << "the deprecated form carried other content"; + if (expected[i].title.has_value()) { + EXPECT_NE(listener.view_titles[i], listener.string_titles[i]) + << "the view reads the deprecated string"; + } + } +} + +// MetadataRole::get_track_duration_ms() reads the applied state's progress, whether or not a +// listener is set: a state's track_duration while it is applied, 0 once a state without progress +// replaces it, and 0 once the role's teardown (its cleanup(), the test thread playing the protocol +// task) has been caught up on the main loop. +TEST(EncryptedLifecycle, TrackDurationReadsTheAppliedStateWithOrWithoutAListener) { + const std::string with_progress = + R"({"type":"server/state","payload":{"metadata":{"timestamp":1,"title":"Song",)" + R"("progress":{"track_progress":10,"track_duration":2000,"playback_speed":0}}}})"; + const std::string without_progress = + R"({"type":"server/state","payload":{"metadata":{"timestamp":2,"title":"Song"}}})"; + for (const bool with_listener : {true, false}) { + SCOPED_TRACE(with_listener ? "Control: with a listener" : "with no listener"); + DispatchTestClient bundle("Track Duration Test Client"); + MetadataRole& metadata = *bundle.client_ref().metadata(); + if (!with_listener) { + metadata.set_listener(nullptr); + } + DispatchTestConnection& conn = bundle.connection(); + bundle.admit(conn); + + bundle.deliver(conn, with_progress); + bundle.pump(); + EXPECT_EQ(metadata.get_track_duration_ms(), 2000U); + EXPECT_EQ(metadata.get_track_progress_ms(), 10U) << "paused: the snapshot"; + bundle.deliver(conn, without_progress); + bundle.pump(); + EXPECT_EQ(metadata.get_track_duration_ms(), 0U) << "a state without progress clears it"; + bundle.deliver(conn, with_progress); + bundle.pump(); + ASSERT_EQ(metadata.get_track_duration_ms(), 2000U); + metadata.impl_->cleanup(); + bundle.pump(); + EXPECT_EQ(metadata.get_track_duration_ms(), 0U) << "the teardown clears it"; + EXPECT_EQ(metadata.get_track_progress_ms(), 0U); } } @@ -2526,50 +2651,319 @@ TEST(EncryptedLifecycle, EveryRoleMessageTypeRunsThroughItsHandler) { << "the server/command never reached the player"; EXPECT_EQ(player_listener.stream_starts, 1) << "the stream/start never reached the player"; EXPECT_EQ(player_listener.stream_ends, 1); - ASSERT_TRUE(bundle.client_ref().get_group_state().group_name.has_value()) + EXPECT_EQ(bundle.client_ref().group_state().group_name, "Kitchen") << "the group/update never reached its handler"; - EXPECT_EQ(*bundle.client_ref().get_group_state().group_name, "Kitchen"); } +namespace { + +// A group state or delta as a listener form or getter handed it, its text copied, so the forms +// compare +struct GroupCopy { + std::optional playback_state; + std::optional group_id; + std::optional group_name; + bool operator==(const GroupCopy&) const = default; +}; + +GroupCopy copy_of(const GroupState& group) { + return {group.playback_state, copy_text(group.group_id), copy_text(group.group_name)}; +} + +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" +GroupCopy copy_of(const GroupUpdateObject& group) { + return {group.playback_state, group.group_id, group.group_name}; +} + +/// The client's group state as the deprecated getter returns it +GroupCopy deprecated_group_state(const SendspinClient& client) { + return copy_of(client.get_group_state()); +} +#pragma GCC diagnostic pop + +} // namespace + // messaging.md "group/update": the delta the listener is handed and the client's group state carry // a conforming group_id as sent, and a UUID, which a server not yet sending the specified format // sends, as base64url of its SHA-256 (the value GroupIdIsKeptNormalizedOrRejected derives). The -// delta carries only the fields its message did, while the group state keeps the others. -TEST(EncryptedLifecycle, GroupIdReachesTheListenerAsSentOrNormalized) { - struct GroupListener : SendspinClientListener { - GroupUpdateObject delta; +// delta carries only the fields its message did, while the group state keeps the others; an empty +// group_id and a null group_name clear their fields to empty, and a group_name over its cap is +// cut. Both listener forms are handed the same delta and both getters return the same state, and +// on_group_state() and group_state() view storage of their own rather than the strings of the +// deprecated forms, which carry the same text. +TEST(EncryptedLifecycle, GroupUpdateReachesBothFormsAsDeltaAndMergedState) { + struct BothFormsListener : SendspinClientListener { + std::optional view_delta; + std::optional string_delta; + const char* view_id{nullptr}; // Where the view's group_id points + const char* string_id{nullptr}; // Where the deprecated group_id points + void reset() { + this->view_delta.reset(); + this->string_delta.reset(); + } + void on_group_state(const GroupState& changes) override { + this->view_delta = copy_of(changes); + this->view_id = changes.group_id.has_value() ? changes.group_id->data() : nullptr; + } +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" void on_group_update(const GroupUpdateObject& group) override { - this->delta = group; + this->string_delta = copy_of(group); + this->string_id = group.group_id.has_value() ? group.group_id->data() : nullptr; } +#pragma GCC diagnostic pop }; - GroupListener group_listener; // Outlives the client + BothFormsListener group_listener; // Outlives the client DispatchTestClient bundle("Group Id Test Client"); - bundle.client_ref().set_listener(&group_listener); + SendspinClient& client = bundle.client_ref(); + client.set_listener(&group_listener); DispatchTestConnection& conn = bundle.connection(); bundle.admit(conn); - const GroupUpdateObject& state = bundle.client_ref().get_group_state(); - // Control: a conforming id is held as sent. const std::string conforming = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMN-_0"; - bundle.deliver(conn, R"({"type":"group/update","payload":{"playback_state":"playing",)" - R"("group_id":")" + - conforming + R"(","group_name":"Kitchen"}})"); - bundle.pump(); - EXPECT_EQ(group_listener.delta.group_id, conforming); - EXPECT_EQ(group_listener.delta.group_name, "Kitchen"); - EXPECT_EQ(state.group_id, conforming); - - const std::string uuid_only = - R"({"type":"group/update","payload":{"group_id":"3f2504e0-4f89-41d3-9a0c-0305e82c3301"}})"; const std::string normalized = "FjYvVmOHs89abpL7Cphsdsog6zoMEsvfvQspUB4MGN8"; - bundle.deliver(conn, uuid_only); - bundle.pump(); - EXPECT_EQ(group_listener.delta.group_id, normalized); - EXPECT_EQ(group_listener.delta.group_name, std::nullopt); - EXPECT_EQ(group_listener.delta.playback_state, std::nullopt); - EXPECT_EQ(state.group_id, normalized); - EXPECT_EQ(state.group_name, "Kitchen"); - EXPECT_EQ(state.playback_state, SendspinPlaybackState::PLAYING); + const std::string long_name = std::string(MAX_FRIENDLY_NAME_BYTES, 'n') + "tail"; + const std::string cut_name = long_name.substr(0, MAX_FRIENDLY_NAME_BYTES); + constexpr auto PLAYING = SendspinPlaybackState::PLAYING; + constexpr auto STOPPED = SendspinPlaybackState::STOPPED; + struct Row { + const char* what; + std::string payload; + GroupCopy delta; + GroupCopy state; + }; + const Row rows[] = { + {"Control: a conforming id is held as sent", + R"("playback_state":"playing","group_id":")" + conforming + R"(","group_name":"Kitchen")", + {PLAYING, conforming, "Kitchen"}, + {PLAYING, conforming, "Kitchen"}}, + {"a UUID is normalized, and the delta carries it alone", + R"("group_id":"3f2504e0-4f89-41d3-9a0c-0305e82c3301")", + {std::nullopt, normalized, std::nullopt}, + {PLAYING, normalized, "Kitchen"}}, + {"an empty id and a null name clear their fields", + R"("playback_state":"stopped","group_id":"","group_name":null)", + {STOPPED, "", ""}, + {STOPPED, "", ""}}, + {"a name over its cap is cut", + R"("group_name":")" + long_name + R"(")", + {std::nullopt, std::nullopt, cut_name}, + {STOPPED, "", cut_name}}, + }; + for (const Row& row : rows) { + SCOPED_TRACE(row.what); + group_listener.reset(); + bundle.deliver(conn, R"({"type":"group/update","payload":{)" + row.payload + "}}"); + bundle.pump(); + ASSERT_TRUE(group_listener.view_delta.has_value()); + EXPECT_TRUE(*group_listener.view_delta == row.delta); + EXPECT_TRUE(group_listener.string_delta == group_listener.view_delta) + << "the deprecated delta carried other content"; + const GroupState state = client.group_state(); + EXPECT_TRUE(copy_of(state) == row.state); + EXPECT_TRUE(deprecated_group_state(client) == row.state); + if (!row.delta.group_id.value_or("").empty()) { + EXPECT_NE(group_listener.view_id, group_listener.string_id) + << "the delta's view reads the deprecated string"; + } + if (!row.state.group_id.value_or("").empty()) { +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + EXPECT_NE(state.group_id->data(), client.get_group_state().group_id->data()) + << "group_state() reads the deprecated string"; +#pragma GCC diagnostic pop + } + } +} + +namespace { + +// Records the state callbacks a listener form overrides, in order: "
", then, when +// `interrupt` is set, runs it from that first callback and logs "read ", the callback's +// argument read again after it. Main loop only. +class FormLog { +public: + std::vector calls; + std::function interrupt; + +protected: + template + void log_state(const char* form, ReadFn read) { + this->calls.push_back(std::string(form) + " " + read()); + if (std::function run = std::exchange(this->interrupt, nullptr)) { + run(); + this->calls.push_back("read " + read()); + } + } +}; + +// The metadata title in each form; a clear logs "clear" +class NewMetadataForm : public MetadataRoleListener, public FormLog { +public: + void on_track_metadata(const TrackMetadata& m) override { + this->log_state("new", [&m] { return std::string(m.title.value_or("")); }); + } + void on_metadata_clear() override { + this->calls.emplace_back("clear"); + } +}; + +// The group name in each form +class NewGroupForm : public SendspinClientListener, public FormLog { +public: + void on_group_state(const GroupState& changes) override { + this->log_state("new", [&changes] { return std::string(changes.group_name.value_or("")); }); + } +}; + +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" +class OldMetadataForm : public MetadataRoleListener, public FormLog { +public: + void on_metadata(const ServerMetadataStateObject& m) override { + this->log_state("old", [&m] { return m.title.value_or(""); }); + } + void on_metadata_clear() override { + this->calls.emplace_back("clear"); + } +}; + +class BothMetadataForms : public NewMetadataForm { +public: + void on_metadata(const ServerMetadataStateObject& m) override { + this->log_state("old", [&m] { return m.title.value_or(""); }); + } +}; + +class OldGroupForm : public SendspinClientListener, public FormLog { +public: + void on_group_update(const GroupUpdateObject& group) override { + this->log_state("old", [&group] { return group.group_name.value_or(""); }); + } +}; + +class BothGroupForms : public NewGroupForm { +public: + void on_group_update(const GroupUpdateObject& group) override { + this->log_state("old", [&group] { return group.group_name.value_or(""); }); + } +}; +#pragma GCC diagnostic pop + +} // namespace + +// Each state reaches on_track_metadata() and then the deprecated on_metadata(), so a listener +// overriding either gets it and one overriding both gets both, the new form first. A stop() from +// on_track_metadata() delivers the clear in the callback, whose views still read the state, and +// the deprecated form of the state the clear replaced does not follow. A teardown the protocol +// task runs during on_track_metadata() (the test thread playing it, through the role's cleanup()) +// does not stop the deprecated form: its clear comes on the next drain. +TEST(EncryptedLifecycle, EachMetadataListenerFormGetsTheStateNewFormFirst) { + NewMetadataForm new_form; + OldMetadataForm old_form; + BothMetadataForms both_forms; + BothMetadataForms both_forms_stopping; + BothMetadataForms both_forms_torn_down; + enum class Interrupt : uint8_t { NONE, STOP, TEARDOWN }; + struct Row { + const char* what; + MetadataRoleListener* listener; + FormLog* log; + Interrupt interrupt; + std::vector calls; // After the loop() that applies the state + std::vector next_calls; // Added by the loop() after it + }; + const Row rows[] = { + {"on_track_metadata() alone", &new_form, &new_form, Interrupt::NONE, {"new Song"}, {}}, + {"on_metadata() alone", &old_form, &old_form, Interrupt::NONE, {"old Song"}, {}}, + {"Control: both, the new form first", + &both_forms, + &both_forms, + Interrupt::NONE, + {"new Song", "old Song"}, + {}}, + {"stop() from on_track_metadata()", + &both_forms_stopping, + &both_forms_stopping, + Interrupt::STOP, + {"new Song", "clear", "read Song"}, + {}}, + {"the protocol task's teardown during on_track_metadata()", + &both_forms_torn_down, + &both_forms_torn_down, + Interrupt::TEARDOWN, + {"new Song", "read Song", "old Song"}, + {"clear"}}, + }; + for (const Row& row : rows) { + SCOPED_TRACE(row.what); + DispatchTestClient bundle("Metadata Form Test Client"); + SendspinClient& client = bundle.client_ref(); + client.metadata()->set_listener(row.listener); + if (row.interrupt == Interrupt::STOP) { + row.log->interrupt = [&client] { client.stop(); }; + } else if (row.interrupt == Interrupt::TEARDOWN) { + row.log->interrupt = [&client] { client.metadata()->impl_->cleanup(); }; + } + DispatchTestConnection& conn = bundle.connection(); + bundle.admit(conn); + bundle.deliver(conn, metadata_state_json(1, "Song")); + // One drain at a time: a teardown's clear waits for the drain after the one that queued it. + client.loop(); + EXPECT_EQ(row.log->calls, row.calls); + std::vector after = row.calls; + after.insert(after.end(), row.next_calls.begin(), row.next_calls.end()); + client.loop(); + EXPECT_EQ(row.log->calls, after); + } +} + +// The group twin of EachMetadataListenerFormGetsTheStateNewFormFirst: each group/update reaches +// on_group_state() and then the deprecated on_group_update(), and a stop() from on_group_state() +// clears the group state with the deprecated form of the update not following. +TEST(EncryptedLifecycle, EachGroupListenerFormGetsTheUpdateNewFormFirst) { + NewGroupForm new_form; + OldGroupForm old_form; + BothGroupForms both_forms; + BothGroupForms both_forms_stopping; + struct Row { + const char* what; + SendspinClientListener* listener; + FormLog* log; + bool stop; + std::vector calls; + }; + const Row rows[] = { + {"on_group_state() alone", &new_form, &new_form, false, {"new Kitchen"}}, + {"on_group_update() alone", &old_form, &old_form, false, {"old Kitchen"}}, + {"Control: both, the new form first", + &both_forms, + &both_forms, + false, + {"new Kitchen", "old Kitchen"}}, + {"stop() from on_group_state()", + &both_forms_stopping, + &both_forms_stopping, + true, + {"new Kitchen", "read Kitchen"}}, + }; + for (const Row& row : rows) { + SCOPED_TRACE(row.what); + DispatchTestClient bundle("Group Form Test Client"); + SendspinClient& client = bundle.client_ref(); + client.set_listener(row.listener); + if (row.stop) { + row.log->interrupt = [&client] { client.stop(); }; + } + DispatchTestConnection& conn = bundle.connection(); + bundle.admit(conn); + bundle.deliver(conn, R"({"type":"group/update","payload":{"group_name":"Kitchen"}})"); + bundle.pump(); + EXPECT_EQ(row.log->calls, row.calls); + EXPECT_EQ(client.group_state().group_name, + row.stop ? std::nullopt : std::optional("Kitchen")); + } } // Role traffic reaches a role only from the admitted connection that owns it, not from any @@ -3270,7 +3664,7 @@ TEST(EncryptedLifecycle, APlayingGroupDoesNotMakeAnIdleServerTheLastPlaybackOne) tick(client); tick(client); - ASSERT_EQ(client.get_group_state().playback_state, SendspinPlaybackState::PLAYING) + ASSERT_EQ(client.group_state().playback_state, SendspinPlaybackState::PLAYING) << "the group/update never reached the client"; EXPECT_FALSE(persistence.blob(persistence_keys::LAST_PLAYED).has_value()) << "a server that never declared 'playback' became the last-playback server"; From bc6a415e1a00600524dc8f18d6e5f50dbf204c07 Mon Sep 17 00:00:00 2001 From: Kevin Ahrendt Date: Wed, 11 Nov 2026 10:51:01 -0500 Subject: [PATCH 2/2] Document the listener contract and the views' terminator Every set_listener() and provider setter now says to set it before start() and leave it in place while the client runs, and the integration guide's wiring step says replacing or clearing one while running is not supported. The deprecated-API section describes the view and string forms in the present tense and notes that a view is not guaranteed to be NUL-terminated. --- docs/integration-guide.md | 6 ++++-- include/sendspin/artwork_role.h | 3 ++- include/sendspin/client.h | 10 ++++++---- include/sendspin/color_role.h | 3 ++- include/sendspin/controller_role.h | 3 ++- include/sendspin/metadata_role.h | 3 ++- include/sendspin/player_role.h | 3 ++- include/sendspin/source_role.h | 4 ++-- include/sendspin/visualizer_role.h | 3 ++- 9 files changed, 24 insertions(+), 14 deletions(-) diff --git a/docs/integration-guide.md b/docs/integration-guide.md index d84104d6..a347d818 100644 --- a/docs/integration-guide.md +++ b/docs/integration-guide.md @@ -779,6 +779,8 @@ client.set_network_provider(&network_provider); // Required client.set_persistence_provider(&persistence_provider); // Optional ``` +Set every listener and provider before `start()` and leave it in place while the client runs: the library calls them without checking whether they changed, so replacing or clearing one (`set_listener(nullptr)`) while the client runs is not supported, including from inside a callback. + ## Step 6: Start and Run ```cpp @@ -1187,7 +1189,7 @@ int64_t client_ts = client.get_client_time(server_timestamp); ## Deprecated String API -The metadata and group text is now handed over as `std::string_view` into storage the library keeps, rather than copied into `std::string`. The `std::string` forms are deprecated in v0.10.0 and removed in v0.11.0; naming one of them draws a `-Wdeprecated-declarations` warning. +The view forms hand the metadata and group text over as `std::string_view` into storage the library keeps; the deprecated forms copy it into `std::string`. The `std::string` forms are deprecated in v0.10.0 and removed in v0.11.0; naming one of them draws a `-Wdeprecated-declarations` warning. | Deprecated | Replacement | |---|---| @@ -1197,7 +1199,7 @@ The metadata and group text is now handed over as `std::string_view` into storag | `SendspinClientListener::on_group_update()` | `SendspinClientListener::on_group_state()` | | `SendspinClient::get_group_state()` | `SendspinClient::group_state()`, which returns by value | -The fields keep their names and meaning; only the text type changes. A view handed to a callback is valid for the duration of the callback, and the views `group_state()` returns until control next returns to `loop()` (from inside a callback: until the callback returns) or until `stop()`: copy what is needed beyond that. Until the removal the library calls the new callback and then the deprecated one with the same content, so a listener overrides one of the two; one that overrides both gets both, the new one first. A `stop()` from the new callback ends the delivery there. +The fields keep their names and meaning; only the text type changes. A view handed to a callback is valid for the duration of the callback, and the views `group_state()` returns until control next returns to `loop()` (from inside a callback: until the callback returns) or until `stop()`: copy what is needed beyond that. A view is not guaranteed to be NUL-terminated: print it with `%.*s` (its size, then its data) or copy it into a `std::string` before passing it to an API that expects a C string. Until the removal the library calls the new callback and then the deprecated one with the same content, so a listener overrides one of the two; one that overrides both gets both, the new one first. A `stop()` from the new callback ends the delivery there. ## Thread Safety Summary diff --git a/include/sendspin/artwork_role.h b/include/sendspin/artwork_role.h index 6b95b462..4efdc33e 100644 --- a/include/sendspin/artwork_role.h +++ b/include/sendspin/artwork_role.h @@ -165,7 +165,8 @@ class ArtworkRole { ArtworkRole(ArtworkRoleConfig config, SendspinClient* client); ~ArtworkRole(); - /// @brief Sets the listener for artwork events; it must outlive this role + /// @brief Sets the listener for artwork events; it must outlive this role. Set it before + /// SendspinClient::start() and leave it in place while the client runs. void set_listener(ArtworkRoleListener* listener); /// @brief Acknowledges the most recent delivery for an ack-gated slot, releasing the gate diff --git a/include/sendspin/client.h b/include/sendspin/client.h index 46b8be6f..259e70bc 100644 --- a/include/sendspin/client.h +++ b/include/sendspin/client.h @@ -703,18 +703,20 @@ class SendspinClient { // Listener and provider setters // ======================================== - /// @brief Sets the listener for client events. The listener must outlive this client + /// @brief Sets the listener for client events. The listener must outlive this client. Set it + /// before start() and leave it in place while the client runs. void set_listener(SendspinClientListener* listener) { this->listener_ = listener; } - /// @brief Sets the network provider (required before start()) - /// The provider must outlive this client + /// @brief Sets the network provider (required). The provider must outlive this client. Set it + /// before start() and leave it in place while the client runs. void set_network_provider(SendspinNetworkProvider* provider) { this->network_provider_ = provider; } - /// @brief Sets the optional persistence provider. The provider must outlive this client + /// @brief Sets the optional persistence provider. The provider must outlive this client. Set it + /// before start() and leave it in place while the client runs. void set_persistence_provider(SendspinPersistenceProvider* provider) { this->persistence_provider_ = provider; } diff --git a/include/sendspin/color_role.h b/include/sendspin/color_role.h index 2d549055..b9e097ea 100644 --- a/include/sendspin/color_role.h +++ b/include/sendspin/color_role.h @@ -105,7 +105,8 @@ class ColorRole { explicit ColorRole(SendspinClient* client); ~ColorRole(); - /// @brief Sets the listener for color events; it must outlive this role + /// @brief Sets the listener for color events; it must outlive this role. Set it before + /// SendspinClient::start() and leave it in place while the client runs. void set_listener(ColorRoleListener* listener); private: diff --git a/include/sendspin/controller_role.h b/include/sendspin/controller_role.h index 6a4192af..58cbfbf4 100644 --- a/include/sendspin/controller_role.h +++ b/include/sendspin/controller_role.h @@ -146,7 +146,8 @@ class ControllerRole { /// @brief Returns the current controller state from the server const ServerStateControllerObject& get_controller_state() const; - /// @brief Sets the listener for controller events; it must outlive this role + /// @brief Sets the listener for controller events; it must outlive this role. Set it before + /// SendspinClient::start() and leave it in place while the client runs. void set_listener(ControllerRoleListener* listener); /// @brief Sends a controller command to the server; callable from any thread diff --git a/include/sendspin/metadata_role.h b/include/sendspin/metadata_role.h index 27f26b88..cef12431 100644 --- a/include/sendspin/metadata_role.h +++ b/include/sendspin/metadata_role.h @@ -143,7 +143,8 @@ class MetadataRole { MetadataRole(MetadataRoleConfig config, SendspinClient* client); ~MetadataRole(); - /// @brief Sets the listener for metadata events; it must outlive this role + /// @brief Sets the listener for metadata events; it must outlive this role. Set it before + /// SendspinClient::start() and leave it in place while the client runs. void set_listener(MetadataRoleListener* listener); /// @brief Returns the track duration in milliseconds, or 0 if unknown or the stream is live diff --git a/include/sendspin/player_role.h b/include/sendspin/player_role.h index 0c8ee719..4405cc1e 100644 --- a/include/sendspin/player_role.h +++ b/include/sendspin/player_role.h @@ -145,7 +145,8 @@ class PlayerRole { PlayerRole(PlayerRoleConfig config, SendspinClient* client); ~PlayerRole(); - /// @brief Sets the listener for player events + /// @brief Sets the listener for player events. Set it before SendspinClient::start() and + /// leave it in place while the client runs. /// @param listener Pointer to the listener implementation; must outlive this role void set_listener(PlayerRoleListener* listener); diff --git a/include/sendspin/source_role.h b/include/sendspin/source_role.h index 01263399..c904f4b3 100644 --- a/include/sendspin/source_role.h +++ b/include/sendspin/source_role.h @@ -89,8 +89,8 @@ class SourceRole { SourceRole(SourceRoleConfig config, SendspinClient* client); ~SourceRole(); - /// @brief Sets the listener for source events; it must outlive this role. Main loop, before - /// SendspinClient::start(). + /// @brief Sets the listener for source events; it must outlive this role. Main loop only: + /// set it before SendspinClient::start() and leave it in place while the client runs. void set_listener(SourceRoleListener* listener); /// @brief Writes captured audio into the input stream diff --git a/include/sendspin/visualizer_role.h b/include/sendspin/visualizer_role.h index 81e3672d..5f069082 100644 --- a/include/sendspin/visualizer_role.h +++ b/include/sendspin/visualizer_role.h @@ -147,7 +147,8 @@ class VisualizerRole { VisualizerRole(VisualizerRoleConfig config, SendspinClient* client); ~VisualizerRole(); - /// @brief Sets the listener for visualizer events; it must outlive this role + /// @brief Sets the listener for visualizer events; it must outlive this role. Set it before + /// SendspinClient::start() and leave it in place while the client runs. void set_listener(VisualizerRoleListener* listener); private: