diff --git a/CLAUDE.md b/CLAUDE.md index 51a7a481..226da92b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -29,6 +29,7 @@ The library provides `SendspinClient` as the main public API. It handles the ful - `NoiseTransport` (`noise_transport.h`): per-connection encrypted framing, owns fragmentation and reassembly around the session - `RecordStore` (`record_store.h`): pairing records and the Pairing PSK (configured, stored, or generated), seeded from the client config and the persistence provider - `Inbox` / `InboxSlot` / `GenerationSlot` (`inbox.h`): single-mutex mailbox for all main-loop-bound cross-thread state - atomic topic bitmask polled lock-free by `loop()`, plus a fixed event ring for ordered lifecycle events +- `ScheduledStateSlot` (`scheduled_state_slot.h`): the metadata and color roles' generation-stamped slot, which also keeps the state waiting for its server-clock timestamp, and `drain_scheduled_state()`, their shared peek / decide-with-no-lock / commit drain - `RoleTeardown` / `TeardownTracker` (`teardown_tracker.h`): every role's teardown generation and the main-loop half of a role's teardown - `MessageTracer` (`message_trace.h`): one connection's handle on the opt-in plaintext message trace (`SendspinClientConfig::message_trace`), built only with `SENDSPIN_ENABLE_MESSAGE_TRACE` - `SendspinTimeFilter` (`time_filter.h`): 2D Kalman filter for NTP-style time sync @@ -95,7 +96,7 @@ Core source files in `src/` have no `#ifdef ESP_PLATFORM` guards; all platform d - **ESP-IDF**: Used as an IDF component via `idf_component.yml`. Sources defined in `cmake/sources.cmake`. - **Host (CMake)**: `cmake -B build && cmake --build build`. Uses ccache when installed (`SENDSPIN_USE_CCACHE`, default on when standalone). Fetches dependencies via FetchContent: ArduinoJson, noise-c, IXWebSocket, micro-flac for the player or source role, and micro-opus for the player or source role when `SENDSPIN_ENABLE_OPUS` is on. -- **Tests**: `cmake -B build-tests -DSENDSPIN_BUILD_TESTS=ON -DENABLE_SANITIZERS=ON -DBUILD_EXAMPLES=OFF .`, then `cmake --build build-tests --target sendspin_tests` and `ctest --test-dir build-tests --output-on-failure`. The target also builds `sendspin_alloc_tests`, which counts the library's `operator new` calls from accept to admission, from `connect_to()` to admission, for a `client/state`, for a time burst, for a controller `server/state`, for `group/update`, for the `stream/*` messages, for the sync task's first FLAC codec header, for a FLAC `stream/start` the player takes, for a `stream/start` section the connection's roles do not take, for the first spectrum frame, and through the record flush that follows each of a playback handoff to another paired server, a `server/unpair`, a `pairing_psk` pairing and a dynamic pairing code pairing in each emission format; `SENDSPIN_ALLOC_REPORT=1` prints them grouped by call site. +- **Tests**: `cmake -B build-tests -DSENDSPIN_BUILD_TESTS=ON -DENABLE_SANITIZERS=ON -DBUILD_EXAMPLES=OFF .`, then `cmake --build build-tests --target sendspin_tests` and `ctest --test-dir build-tests --output-on-failure`. The target also builds `sendspin_alloc_tests`, which counts the library's `operator new` calls from accept to admission, from `connect_to()` to admission, for a `client/state`, for a time burst, for a controller `server/state`, for `group/update`, for metadata `server/state` (a first state, a second, a scheduled one and one without text), for the `stream/*` messages, for the sync task's first FLAC codec header, for a FLAC `stream/start` the player takes, for a `stream/start` section the connection's roles do not take, for the first spectrum frame, and through the record flush that follows each of a playback handoff to another paired server, a `server/unpair`, a `pairing_psk` pairing and a dynamic pairing code pairing in each emission format; `SENDSPIN_ALLOC_REPORT=1` prints them grouped by call site. - **ThreadSanitizer tests**: `cmake -B build-tsan -DSENDSPIN_BUILD_TESTS=ON -DENABLE_TSAN=ON -DBUILD_EXAMPLES=OFF .`, then `cmake --build build-tsan --target sendspin_tests` and `TSAN_OPTIONS=halt_on_error=1 ctest --test-dir build-tsan --output-on-failure`. `ENABLE_TSAN` applies the thread sanitizer to every target, including the fetched dependencies, and cannot be combined with `ENABLE_SANITIZERS`. - **Message trace**: `-DSENDSPIN_ENABLE_MESSAGE_TRACE=ON` compiles in `SendspinClientConfig::message_trace` and `tests/test_message_trace.cpp`; CI's test and TSan jobs build it on, the build and no-opus jobs off. - **Stack budgets**: the ESP task stack defaults in `config.h` are static call-graph bounds from `tools/stack_usage/` (recipe and per-task figures in its README); re-derive them, and update the README table with them, after a change that deepens a task's call chain. diff --git a/docs/conventions.md b/docs/conventions.md index 7d1a626b..f2c57ad2 100644 --- a/docs/conventions.md +++ b/docs/conventions.md @@ -73,9 +73,10 @@ checklists in `.claude/skills/` apply these standards to a diff. - An event or a slot payload whose delivery must not survive its producer being torn down carries the producer's teardown generation and is checked against it at the drain (`push_event_or_log()` / `event_is_current()` for - events, `GenerationSlot` for a role's slots), rather than relying on the - ring or the slot being reset: a teardown that leaves other producers running - cannot reset it, and a drain can take a slot on either side of a teardown. + events, `GenerationSlot` or `ScheduledStateSlot` for a role's slots), rather + than relying on the ring or the slot being reset: a teardown that leaves + other producers running cannot reset it, and a drain can take a slot on + either side of a teardown. - Callback dispatch must tolerate re-entrant teardown: a listener callback may call back into the client. See "Re-entrant Teardown During Callback Dispatch" in `docs/internals.md` for the guard patterns in use. @@ -91,8 +92,9 @@ checklists in `.claude/skills/` apply these standards to a diff. stamps the role's events and slot payloads with the new teardown generation, and the main loop runs the role's own half once per generation, through the role's `TeardownTracker` (`catch_up_teardown()`), before acting on anything - stamped with it. A role drain takes its slot before it catches up, then - applies only a payload stamped with the current generation. + stamped with it. A role drain takes (or peeks at) its slot before it + catches up, then applies only a payload stamped with the current + generation. - A protocol-task step has a bounded wait or none: the Noise DH operations, a ring acquire bounded by `INBOUND_ACQUIRE_TIMEOUT_MS`, and a transport send bounded by the transport's own send timeout; no step waits on an diff --git a/docs/integration-guide.md b/docs/integration-guide.md index 073e6440..3b15a325 100644 --- a/docs/integration-guide.md +++ b/docs/integration-guide.md @@ -114,10 +114,12 @@ auto& controller = client.add_controller(); ### Metadata Role (Track Information) -Receives track metadata (title, artist, album, progress, etc.) from the server. +Receives track metadata (title, artist, album, progress, etc.) from the server. Takes an optional configuration struct with a flag per text field, all kept by default; a field turned off reaches the listener without a value and costs no storage. ```cpp -auto& metadata = client.add_metadata(); +auto& metadata = client.add_metadata(); // Every text field +// Or only what the display shows: +// auto& metadata = client.add_metadata({.album_artist = false, .album = false, .artwork_url = false}); ``` ### Artwork Role (Album Art) @@ -315,17 +317,19 @@ The `ServerMetadataStateObject` contains these fields (all optional except `time | 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 | -| `artist` | `std::optional` | Track artist | -| `album_artist` | `std::optional` | Album artist | -| `album` | `std::optional` | Album name | -| `artwork_url` | `std::optional` | Artwork URL | +| `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`. + 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. You can also poll track progress at any time: @@ -1426,6 +1430,17 @@ Each entry in `audio_formats` is an `AudioSupportedFormatObject`: --- +### MetadataRoleConfig + +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. | +| `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. | + +--- + ### ArtworkRoleConfig Configuration passed to `client.add_artwork()`. @@ -1675,4 +1690,4 @@ Set with `SendspinClient::set_log_level()`. Only affects host builds; ESP-IDF bu | `PREFER_EXTERNAL` | Prefer SPIRAM, fall back to internal RAM (ESP-IDF only) | | `PREFER_INTERNAL` | Prefer internal RAM, fall back to SPIRAM (ESP-IDF only) | -Used by `SendspinClientConfig::inbound_ring_location` to control where the shared inbound ring and the per-connection fallback buffers are allocated, by `SendspinClientConfig::noise_buffer_location` to control where the Noise transport's fragment reassembly and send buffers are allocated, by `PlayerRoleConfig::decode_buffer_location` to control where the player's decode transfer buffer is allocated, and by `SourceRoleConfig::buffer_location` to control where the source's capture and chunk buffers are allocated. Ignored on host platforms (no internal/external distinction). +Used by `SendspinClientConfig::inbound_ring_location` to control where the shared inbound ring and the per-connection fallback buffers are allocated, by `SendspinClientConfig::noise_buffer_location` to control where the Noise transport's fragment reassembly and send buffers are allocated, by `PlayerRoleConfig::decode_buffer_location` to control where the player's decode transfer buffer is allocated, by `MetadataRoleConfig::buffer_location` to control where the metadata role's text blocks are allocated, and by `SourceRoleConfig::buffer_location` to control where the source's capture and chunk buffers are allocated. Ignored on host platforms (no internal/external distinction). diff --git a/docs/internals.md b/docs/internals.md index 11ce4360..6e53ebef 100644 --- a/docs/internals.md +++ b/docs/internals.md @@ -71,7 +71,7 @@ The same ordering bounds the control traffic. Everything an admitted connection The Inbox is a single-mutex mailbox with a lock-free dirty-topic bitmask. It offers two endpoint styles: a fixed-capacity event ring for ordered lifecycle events, and `InboxSlot` latest-value slots for state, each owning one `INBOX_TOPIC_*` bit. The main loop calls `poll()` to read the bitmask without locking and only locks to drain topics whose bit is set; a bit set after the snapshot stays set, so the next drain sees it. It is the only way work reaches the main loop: `loop()` polls the network provider and runs `drain_inbox()`, nothing else (while stopped it only frees parked outbound attempts), and while running, the only library state it reads outside the Inbox to find work is each role's teardown generation, loaded once per drain for the catch-up that heads it (see [One Main Loop Drain](#one-main-loop-drain)). -Every role state slot is a `GenerationSlot`: each payload carries the stamp its producer passes, the role's teardown generation it was admitted under, and the slot keeps the newest generation's content; it is the same stamp the role's ring events carry (see [Cleanup](#cleanup)). The player's `sync_idle_slot` is a plain `InboxSlot`: it carries no state, only the wake. +Every role state slot is a `GenerationSlot`, except the metadata and color roles', whose state applies at its server-clock timestamp: theirs is a `ScheduledStateSlot` (`src/scheduled_state_slot.h`), which also keeps the pending state a drain left waiting, so their drains peek at it and commit rather than take it. Both kinds follow one rule: each payload carries the stamp its producer passes, the role's teardown generation it was admitted under, and the slot keeps the newest generation's content; it is the same stamp the role's ring events carry (see [Cleanup](#cleanup)). The player's `sync_idle_slot` is a plain `InboxSlot`: it carries no state, only the wake. | Endpoint | Topic bit | Producer | |----------|-----------|----------| @@ -82,8 +82,8 @@ Every role state slot is a `GenerationSlot`: each payload carries the stamp i | `SendspinClient::EventState::pairing_slot` | `INBOX_TOPIC_PAIRING` | Protocol task (pairing notes); main loop (`stop()`'s dismissals) | | `SendspinClient::EventState::high_performance_slot` | `INBOX_TOPIC_HIGH_PERFORMANCE` | Protocol task (time-burst acquires and releases, counted separately) | | `ControllerRole::Impl::EventState::slot` (generation-stamped) | `INBOX_TOPIC_CONTROLLER` | Protocol task | -| `MetadataRole::Impl::EventState::slot` (generation-stamped) | `INBOX_TOPIC_METADATA` | Protocol task | -| `ColorRole::Impl::EventState::slot` (generation-stamped) | `INBOX_TOPIC_COLOR` | Protocol task | +| `MetadataRole::Impl::EventState::slot` (scheduled, generation-stamped) | `INBOX_TOPIC_METADATA` | Protocol task | +| `ColorRole::Impl::EventState::slot` (scheduled, generation-stamped) | `INBOX_TOPIC_COLOR` | Protocol task | | `PlayerRole::Impl::EventState::stream_params_slot` (generation-stamped) | `INBOX_TOPIC_PLAYER_STREAM_PARAMS` | Protocol task | | `PlayerRole::Impl::EventState::command_slot` (generation-stamped) | `INBOX_TOPIC_PLAYER_COMMAND` | Protocol task | | `PlayerRole::Impl::EventState::sync_idle_slot` | `INBOX_TOPIC_PLAYER_SYNC_IDLE` | Sync task (returned to idle from a stream) | @@ -159,11 +159,11 @@ While the client is stopped, `SendspinClient::loop()` only frees the outbound at 8. Drain group_slot: apply deltas, fire 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' future-dated state, artwork's held displays) add a carry-over term to `needs_drain()`, since no inbox bit tracks that wait; 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. +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. The high-performance hold is a ref-counted request, surfaced to the consumer through `on_request_high_performance()` / `on_release_high_performance()`, for the platform to keep networking responsive (e.g. disable WiFi power saving) during time sync and playback. A time burst requests it through `high_performance_slot` when it comes due, and sends its first time frame only once the main loop has granted that request: step 2 calls the listener for every acquire it took, then adds them to `SendspinClient::high_performance_granted_` and wakes the protocol task, which compares the burst's ticket against that count (`high_performance_granted()`). Acquires and releases are counted separately, so an acquire and a release queued inside one stalled tick (a connection dropped before its grant) still reach the listener as a request followed by its release. Releases wait for nothing. The player holds the same ref count from its drained STREAM_START to its STREAM_END on the main loop, and the source from its drained STREAMING_STARTED to its STREAMING_STOPPED (or the teardown that owes it). -Every role drain in step 7 has the same head: it takes its slot first, catches the role up on its current teardown generation (`catch_up_teardown()`), then applies the taken payload only if its stamp is still the current generation, dropping a stale one with a debug log. Controller, metadata, and color then fire their state callback, metadata and color holding a future-dated state until its server-clock timestamp. Artwork folds its displays into its holds and fires `on_image_display()` when each deadline passes. The player's drain is the main loop's half of the stream end/start handshake (see [Stream End and Start](#stream-end-and-start)). Visualizer and source have no drain; they are driven entirely by their ring events (the visualizer's STREAM_START applying the config written with the same stamp) and by their teardown catch-up. +Every role drain in step 7 has the same head: it takes (metadata and color: peeks at) its slot first, catches the role up on its current teardown generation (`catch_up_teardown()`), then applies the payload only if its stamp is still the current generation, dropping a stale one with a debug log. Controller, metadata, and color then fire their state callback. Metadata and color leave their states in the slot until `drain_scheduled_state()` (`src/scheduled_state_slot.h`) decides them, which it does with no lock held, since a deadline reads the client clock behind a lock of its own. Artwork folds its displays into its holds and fires `on_image_display()` when each deadline passes. The player's drain is the main loop's half of the stream end/start handshake (see [Stream End and Start](#stream-end-and-start)). Visualizer and source have no drain; they are driven entirely by their ring events (the visualizer's STREAM_START applying the config written with the same stamp) and by their teardown catch-up. ## Ordering Guarantees @@ -220,7 +220,7 @@ When a connection leaves the admitted array, `detach_inbound()` runs first. It d `cleanup_connection_state()` then tears down the roles no remaining admitted connection owns, on the protocol task. When that is every role it also resets the inbox ring and the group and time-sync slots, and drops the pending pairing notes other than a dismissal whose prompt an earlier drain delivered. Each role's teardown is split in two halves, with a `TeardownTracker` (`src/teardown_tracker.h`) on each role recording which teardowns the main loop has caught up with: - The protocol-task half, the role's `cleanup()`, bumps its `cleanup_generation`, resets its Inbox slots, signals its thread, and pushes its `*_CLEARED` event stamped with the new generation. Everything the role queues from then on, events and slot payloads alike, carries the new generation. A teardown runs on the protocol task between two handlers, or from `stop()` once that task is joined, so a role handler loads the generation once at entry, runs whole under it and does not check it: the stamp is for the main loop and the role threads, which take what the handler queued on either side of a later teardown. -- The main-loop half, the role's `complete_teardown()`, resets what only the main loop touches (the player's held stream events and playback high-performance hold, the controller, metadata and color state, artwork's held displays) and delivers the role's clear: the controller, metadata and color clear callbacks, artwork's `on_image_clear()` for every channel, the visualizer's `on_visualizer_stream_end()`, and the player's STREAM_END, which it queues in `awaiting_sync_idle_events` for the drain to fire once the sync task reads idle. `catch_up_teardown()` runs it once per teardown generation: at the head of every drain, from the event drain before the main loop acts on an event stamped with a newer generation, and from each role drain before it applies a slot payload. A role drain takes its slot first and only then catches up to the role's current generation, so a payload the next connection wrote right after the teardown is applied after the clear, never wiped by it, and a payload from the torn-down connection, stamped with the older generation, is dropped instead of applied after its clear. The controller's supported-commands mask carries the same stamp, so `send_command()` reads a torn-down connection's mask as empty, and the command it queues carries the stamp it passed under, so a teardown between the check and the protocol task's drain drops the command there. +- The main-loop half, the role's `complete_teardown()`, resets what only the main loop touches (the player's held stream events and playback high-performance hold, the controller, metadata and color state, artwork's held displays) and delivers the role's clear: the controller, metadata and color clear callbacks, artwork's `on_image_clear()` for every channel, the visualizer's `on_visualizer_stream_end()`, and the player's STREAM_END, which it queues in `awaiting_sync_idle_events` for the drain to fire once the sync task reads idle. `catch_up_teardown()` runs it once per teardown generation: at the head of every drain, from the event drain before the main loop acts on an event stamped with a newer generation, and from each role drain before it applies a slot payload. A role drain takes (metadata and color: peeks at) its slot first and only then catches up to the role's current generation, so a payload the next connection wrote right after the teardown is applied after the clear, never wiped by it, and a payload from the torn-down connection, stamped with the older generation, is dropped instead of applied after its clear. The controller's supported-commands mask carries the same stamp, so `send_command()` reads a torn-down connection's mask as empty, and the command it queues carries the stamp it passed under, so a teardown between the check and the protocol task's drain drops the command there. The source's `cleanup()` closes the stream gate without a message and clears the authorization; it recalls nothing, since the source holds no inbound ring item. The player's, the visualizer's and the artwork role's `cleanup()` also return what the sync task, the visualizer drain thread or the artwork decode thread has not taken, right after the generation bump and on the thread that hands the items over (`InboundConsumer::recall()`), so every item recalled carries an older stamp; a consumer that takes such an item between the bump and the recall returns it, by its stamp, without acting on it. The visualizer's `cleanup()` also moves its boundary sequence on (see the `stream/end` row under [Dispatch](#dispatch-protocol-task)), so its drain thread returns a frame it holds for its delivery time. The artwork decode thread, woken by its `cleanup()`, also drops the images it assembles or parks once it sees the generation move on. A role removed by a `server/activate` takes the same path. In `stop()` the role threads are joined and their lists returned and unbound (`InboundConsumer::unbind()`) before the main loop runs the roles' `cleanup()`, whose recall then finds no ring bound. @@ -229,7 +229,7 @@ The source's `cleanup()` closes the stream gate without a message and clears the Requests a listener callback makes (`connect_to()`, `disconnect()`, ...) are queued or posted to the protocol task, so no callback tears a connection down under the drain that fired it. `stop()` is the exception: called from a callback, it joins the protocol task, tears every role down on the main loop and runs a drain of its own beneath the one that fired the callback, and a `start()` that follows it in the same callback begins a new run. Two generation counters keep the outer drain from acting on what it already took: - `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 after applying the earlier of two taken states, and artwork after each display or clear, and each abandons the rest of its work when it moved on. +- 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. ## Message Flow @@ -279,7 +279,7 @@ Role-bound traffic reaches a role only from the admitted connection that owns it | `server/hello` | Records server info and the codecs its source@v1 support object lists on the connection, and sets `server_hello_received_` | | `server/activate` | `ConnectionManager::on_server_activate()`: admissibility and pairing-method checks, what the roles owed until the activation (`SendspinClient::on_activation_applied()`), role ownership and removals, and admission of a nursery connection the activation makes operational | | `server/time` | Claims the connection's `client/time` frame in flight, dropping a reply that answers none, and feeds the measurement to the connection's own time burst | -| `server/state` | Writes the controller, metadata, and color `InboxSlot`s | +| `server/state` | Writes the controller and color slots; the metadata role copies its kept text into its slot place (`MetadataRole::Impl::handle_server_state()`, see [Memory](#memory)). A text field over its cap is cut, with one warning per connection; with `METADATA_STRICT`, it closes the connection and nothing in the message is applied | | `server/command` | Merges the player object into the player's `command_slot`; the source object starts or ends the input stream (see [Source Stream Start and End](#source-stream-start-and-end)) | | `group/update` | Merges into `group_slot`, from the primary admitted connection only; a `group_id` that is not 43 base64url characters is normalized, or closes the connection when its hash state cannot be allocated; with `GROUP_ID_STRICT`, one that is absent or not in that format closes the connection (`group_id_from_wire()`) | | `stream/start` | Player writes the codec header, numbered with the stream's ordinal, into an item it acquires from the inbound ring (a bounded wait) and appends it to the sync task's list, then writes its params slot and pushes STREAM_START; visualizer moves its boundary sequence on (below), writes its config slot, and pushes STREAM_START; artwork marks its stream active, discards the pending image of each channel whose configuration changed, and appends a marker naming those channels to its decode thread's list | @@ -460,11 +460,12 @@ The library allocates from heaps that fail differently. `operator new`, behind e 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`). +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. + `operator new` runs after `start()` at these places: | What | When | |------|------| -| Message fields held on the heap: track metadata | Each message that carries them, parsed on the protocol task, only for a connection that owns the metadata role | | The listener's `std::string` arguments: a pairing's `server_id`, and a `qr_code` pairing code's token | Each pairing callback that takes one, on the main loop | | The pairing notes for the main loop beyond the room both of their buffers keep | A batch of more than `PAIRING_NOTE_CAPACITY` notes between two drains, on the protocol task; the grown room is kept | | Another drain buffer for the pairing notes, with room for `PAIRING_NOTE_CAPACITY` | A pairing listener callback that calls `stop()` while a prompt shows: the drain `stop()` runs finds the buffer lent to the dispatch that called it, and reserves one before it takes the dismissals, on the main loop | diff --git a/include/sendspin/client.h b/include/sendspin/client.h index 124fde13..7894637d 100644 --- a/include/sendspin/client.h +++ b/include/sendspin/client.h @@ -434,8 +434,9 @@ class SendspinClient { #endif #ifdef SENDSPIN_ENABLE_METADATA - /// @brief Adds the metadata role. Returns a reference for setting callbacks - MetadataRole& add_metadata(); + /// @brief Adds the metadata role; `config` selects which text fields it keeps. Returns a + /// reference for setting callbacks + MetadataRole& add_metadata(MetadataRoleConfig config = {}); #endif #ifdef SENDSPIN_ENABLE_ARTWORK diff --git a/include/sendspin/config.h b/include/sendspin/config.h index a0a3acca..72f9b17b 100644 --- a/include/sendspin/config.h +++ b/include/sendspin/config.h @@ -493,6 +493,25 @@ struct PlayerRoleConfig { MemoryLocation decode_buffer_location{MemoryLocation::PREFER_EXTERNAL}; }; +// ============================================================================ +// 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 +/// (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. + MemoryLocation buffer_location{MemoryLocation::PREFER_EXTERNAL}; +}; + // ============================================================================ // Artwork config types // ============================================================================ diff --git a/include/sendspin/metadata_role.h b/include/sendspin/metadata_role.h index 7cee36f6..7173becf 100644 --- a/include/sendspin/metadata_role.h +++ b/include/sendspin/metadata_role.h @@ -17,6 +17,8 @@ #pragma once +#include "sendspin/config.h" + #include #include #include @@ -38,6 +40,9 @@ struct MetadataProgressObject { }; /// @brief Track metadata and playback state received from the server +/// +/// 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 { int64_t timestamp{}; std::optional title; @@ -81,7 +86,8 @@ class MetadataRoleListener { * * Usage: * 1. Implement MetadataRoleListener to receive metadata updates - * 2. Add the role to the client via SendspinClient::add_metadata() + * 2. Add the role to the client via SendspinClient::add_metadata(), turning off in + * MetadataRoleConfig the text fields not to keep * 3. Call set_listener() with your listener implementation * 4. Poll get_track_progress_ms() and get_track_duration_ms() as needed for UI updates * @@ -94,7 +100,9 @@ class MetadataRoleListener { * }; * * MyMetadataListener listener; - * auto& metadata = client.add_metadata(); + * // Only the fields the listener shows: the others are dropped and cost no storage + * auto& metadata = + * client.add_metadata({.album_artist = false, .album = false, .artwork_url = false}); * metadata.set_listener(&listener); * uint32_t progress = metadata.get_track_progress_ms(); * @endcode @@ -105,7 +113,7 @@ class MetadataRole { public: struct Impl; - explicit MetadataRole(SendspinClient* client); + MetadataRole(MetadataRoleConfig config, SendspinClient* client); ~MetadataRole(); /// @brief Sets the listener for metadata events; it must outlive this role diff --git a/src/client.cpp b/src/client.cpp index c680becc..92d54cc5 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -470,8 +470,9 @@ bool SendspinClient::start() { return false; } - // Start the role threads. A failure part-way stops the roles that did start, so the client - // is back in the stopped state and a corrected retry begins clean. + // Start the role threads, and allocate the metadata role's text blocks. A failure part-way + // stops the roles that did start, so the client is back in the stopped state and a corrected + // retry begins clean. bool roles_started = true; #ifdef SENDSPIN_ENABLE_PLAYER if (roles_started && this->player_) { @@ -493,6 +494,11 @@ bool SendspinClient::start() { if (roles_started && this->source_) { roles_started = this->source_->impl_->start(this->protocol_task_.get()); } +#endif +#ifdef SENDSPIN_ENABLE_METADATA + if (roles_started && this->metadata_) { + roles_started = this->metadata_->impl_->start(); + } #endif if (!roles_started) { this->stop_role_threads(); @@ -1222,11 +1228,11 @@ ControllerRole& SendspinClient::add_controller() { #endif #ifdef SENDSPIN_ENABLE_METADATA -MetadataRole& SendspinClient::add_metadata() { +MetadataRole& SendspinClient::add_metadata(MetadataRoleConfig config) { if (this->lifecycle_.load(std::memory_order_relaxed) != LifecycleState::STOPPED) { SS_LOGW(TAG, "add_metadata() called while started"); } - this->metadata_ = std::make_unique(this); + this->metadata_ = std::make_unique(config, this); this->metadata_->impl_->attach_inbox(this->event_state_->inbox); return *this->metadata_; } diff --git a/src/client_dispatch.cpp b/src/client_dispatch.cpp index 07327eb9..e71ec502 100644 --- a/src/client_dispatch.cpp +++ b/src/client_dispatch.cpp @@ -371,7 +371,10 @@ void SendspinClient::process_json_message(SendspinConnection& connection, const // copies the message into its plain struct through parsed.extract(), which releases the // document before any handler runs, so a reply a handler builds (client/state after // server/activate, a pairing reply, a re-handshake's msg2) fits the arena beside nothing. A - // case that reads no payload calls parsed.release() before acting. + // case that reads no payload calls parsed.release() before acting. The one exception is the + // metadata role's handler, which runs inside server/state's extract() to copy its kept text + // straight from the document into its slot; it builds no reply and keeps no view of the + // document. ParsedJsonMessage parsed(*this->json_arena_); if (!parsed.parse(data, len)) { SS_LOGW(TAG, "Failed to parse JSON message"); @@ -586,11 +589,12 @@ void SendspinClient::process_json_message(SendspinConnection& connection, const break; } case SendspinServerToClientMessageType::SERVER_STATE: { - // The three sections are live together so one parse serves all three: every section a - // role takes is extracted in the one pass that releases the document, at the cost of - // the sections' combined size in this frame (the protocol task's stack is a fixed - // budget on ESP-IDF). Each section's role gate is decided first, so a section no role - // takes is not parsed. + // One parse serves all three sections: every section a role takes is extracted in the + // one pass that releases the document, the controller and color sections at the cost + // of their combined size in this frame (the protocol task's stack is a fixed budget on + // ESP-IDF), and the metadata section handed to its role's slot there, its text copied + // straight from the document. Each section's role gate is decided first, so a section + // no role takes is not parsed. #ifdef SENDSPIN_ENABLE_CONTROLLER const bool take_controller = this->controller_ && @@ -602,8 +606,7 @@ void SendspinClient::process_json_message(SendspinConnection& connection, const const bool take_metadata = this->metadata_ && role_accepts_traffic(*this->connection_manager_, conn, SendspinRole::METADATA); - ServerMetadataStateObject metadata_state; - bool metadata_valid = false; + MetadataStateOutcome metadata_outcome = MetadataStateOutcome::IGNORED; #endif #ifdef SENDSPIN_ENABLE_COLOR const bool take_color = this->color_ && role_accepts_traffic(*this->connection_manager_, @@ -617,14 +620,26 @@ void SendspinClient::process_json_message(SendspinConnection& connection, const take_controller && process_server_state_controller(root, &controller_state); #endif #ifdef SENDSPIN_ENABLE_METADATA - metadata_valid = - take_metadata && process_server_state_metadata(root, &metadata_state); + if (take_metadata) { + metadata_outcome = this->metadata_->impl_->handle_server_state(root); + } #endif #ifdef SENDSPIN_ENABLE_COLOR color_valid = take_color && process_server_state_color(root, &color_state); #endif }); +#ifdef SENDSPIN_ENABLE_METADATA + if (metadata_outcome == MetadataStateOutcome::REJECTED) { + // messaging.md "Out-of-range values": a text field over its cap is a protocol + // error the client MUST close the connection on, so nothing else in the message is + // applied. Closed silently, like every other protocol error the receive path finds + // (see close_silently()). + conn->close_silently(SendspinGoodbyeReason::UNAUTHORIZED); + break; + } +#endif + #ifdef SENDSPIN_ENABLE_CONTROLLER if (controller_valid) { this->controller_->impl_->handle_server_state(controller_state); @@ -632,8 +647,10 @@ void SendspinClient::process_json_message(SendspinConnection& connection, const #endif #ifdef SENDSPIN_ENABLE_METADATA - if (metadata_valid) { - this->metadata_->impl_->handle_server_state(std::move(metadata_state)); + if (metadata_outcome == MetadataStateOutcome::CUT && conn->note_metadata_cut()) { + SS_LOGW(TAG, "Server sent metadata text over its cap (roles/metadata/v1.md " + "\"server/state metadata object\"); cut to it, a future release " + "closes the connection"); } #endif diff --git a/src/color_role.cpp b/src/color_role.cpp index 8cdc61ba..f9144020 100644 --- a/src/color_role.cpp +++ b/src/color_role.cpp @@ -17,29 +17,10 @@ #include "protocol_messages.h" #include "sendspin/client.h" -#include - static const char* const TAG = "sendspin.color"; namespace sendspin { -namespace { - -/// @brief Folds a newly arrived palette into the palettes the main loop has not taken yet -/// -/// `incoming` always carries exactly one palette, in `oldest`. The first one to arrive after a -/// drain keeps that place; every later one becomes `newest` (see PendingColorStates). Runs under -/// the Inbox mutex, so it stays a pure data operation. -void coalesce_color_states(PendingColorStates& current, const PendingColorStates& incoming) { - if (!current.oldest.has_value()) { - current.oldest = incoming.oldest; - return; - } - current.newest = incoming.oldest; -} - -} // namespace - // ============================================================================ // Impl constructor / destructor // ============================================================================ @@ -75,11 +56,9 @@ void ColorRole::Impl::build_hello_fields(ClientHelloMessage& msg) { void ColorRole::Impl::handle_server_state(const ServerColorStateObject& color) const { const uint32_t generation = this->cleanup_generation.load(std::memory_order_acquire); // messaging.md "server/state": each included color object is the role's full palette, never - // an overlay on the one before it (see coalesce_color_states). - PendingColorStates arrival; - arrival.oldest = color; - // NOLINTNEXTLINE(performance-move-const-arg): merge() takes the delta as T&& - this->event_state->slot.merge(coalesce_color_states, std::move(arrival), generation); + // an overlay on the one before it, so it replaces the place's palette whole. + this->event_state->slot.arrive([&color](ServerColorStateObject& place) { place = color; }, + generation); } bool ColorRole::Impl::state_is_due(int64_t timestamp) const { @@ -90,62 +69,16 @@ bool ColorRole::Impl::state_is_due(int64_t timestamp) const { return client_ts == 0 || client_ts <= platform_time_us(); } -void ColorRole::Impl::apply_due_state() { - if (!this->held_state.has_value() || !this->state_is_due(this->held_state->timestamp)) { - return; - } - - this->color = this->held_state.value(); - this->held_state.reset(); - if (this->listener) { - this->listener->on_color(this->color); - } -} - void ColorRole::Impl::drain_events() { - PendingColorStates taken; - bool have_taken = false; - const uint32_t generation = - take_current_payload(*this, this->event_state->slot, taken, have_taken, TAG, "color"); - // Every check below against `generation` also catches a listener callback that re-entered - // teardown (a listener calling stop()): its cleanup() moved the generation on, and its own - // drain already reset this role. - if (!this->accepts(generation)) { - return; - } - - // InboxSlot has no take_if (a deadline predicate must not run under the shared Inbox mutex; - // see inbox.h), so the server-clock deadline gate is split in two: take() unconditionally - // moves the pending palettes into held_state, then each deadline is evaluated below with no - // lock held at all. - // - // roles/color/v1.md "Scheduled color updates": a palette whose timestamp is still in the - // future is the pending update and a newer one replaces it, while a past or present one is - // applied at once and discards the pending update. Both fall out of replacing held_state with - // each taken palette in arrival order, applying whatever is due in between. - if (have_taken) { - const bool collapses = - taken.newest.has_value() && this->state_is_due(taken.newest->timestamp); - if (taken.oldest.has_value() && !collapses) { - // Two palettes that are both due collapse instead: the older would be superseded - // within this tick, so no listener could observe it. - this->held_state = taken.oldest; - this->apply_due_state(); - if (!this->accepts(generation)) { - return; + drain_scheduled_state( + *this, this->event_state->slot, + [this](const ServerColorStateObject& place) { this->color = place; }, + [this] { + if (this->listener) { + this->listener->on_color(this->color); } - } - if (taken.newest.has_value()) { - this->held_state = taken.newest; - } - } - - // A future-dated palette is held across ticks without any topic bit set (take() above cleared - // it). It is re-evaluated against its deadline on later ticks only because needs_drain() ORs - // in held_state.has_value() alongside the INBOX_TOPIC_COLOR bit test, so this - // drain_events() keeps running each tick until the deadline fires. Dropping that OR term - // would strand the palette until an unrelated new one re-set the topic bit. - this->apply_due_state(); + }, + TAG, "color"); } void ColorRole::Impl::cleanup() { @@ -160,7 +93,7 @@ void ColorRole::Impl::cleanup() { void ColorRole::Impl::complete_teardown() { this->color = {}; - this->held_state.reset(); + this->event_state->slot.forget_pending(); if (this->listener) { this->listener->on_color_clear(); } diff --git a/src/color_role_impl.h b/src/color_role_impl.h index e7449b66..8b9bb153 100644 --- a/src/color_role_impl.h +++ b/src/color_role_impl.h @@ -19,6 +19,7 @@ #include "inbox.h" #include "protocol_messages.h" +#include "scheduled_state_slot.h" #include "sendspin/color_role.h" #include "teardown_tracker.h" @@ -30,17 +31,10 @@ namespace sendspin { class SendspinClient; struct ClientHelloMessage; -/// @brief Color palettes handed to the main loop but not yet drained -/// -/// messaging.md "server/state" lets a server bring a client up to date and then schedule the next -/// update straight after it, so two palettes can land between two main-loop ticks. Both are kept, -/// in arrival order, so the current one is still applied on the tick that also takes the -/// scheduled one. A third palette in the same window replaces `newest`, which no observer has -/// seen. -struct PendingColorStates { - std::optional oldest; - std::optional newest; -}; +/// @brief The server-clock timestamp `color` applies at (ScheduledStateSlot) +inline int64_t scheduled_time(const ServerColorStateObject& color) { + return color.timestamp; +} /// @brief Private implementation of the color role struct ColorRole::Impl : RoleTeardown { @@ -52,7 +46,7 @@ struct ColorRole::Impl : RoleTeardown { // ======================================== struct EventState { - GenerationSlot slot; + ScheduledStateSlot slot; }; // ======================================== @@ -61,24 +55,22 @@ struct ColorRole::Impl : RoleTeardown { void attach_inbox(Inbox& inbox); void build_hello_fields(ClientHelloMessage& msg); - // Takes a const reference, unlike the metadata and controller overloads: a - // ServerColorStateObject holds only optional RGB triples and a timestamp, so there is nothing - // for an rvalue reference to move out of. + /// @brief Hands a parsed palette to the slot, which copies it into the place it takes. + /// Protocol task. void handle_server_state(const ServerColorStateObject& color) const; - // True if a slot palette needs taking, or a palette already held from a prior tick (see - // held_state) is still waiting out its server-clock deadline: the deadline itself sets no - // inbox bit, so held_state must be polled every tick until it fires. + /// @brief True if a palette arrived, or the pending palette the last drain left in the slot + /// has come due: the deadline sets no inbox bit, so it is checked every tick, against + /// ScheduledStateSlot::pending_at(), without the Inbox mutex. bool needs_drain(uint32_t pending_bits) const { - return (pending_bits & INBOX_TOPIC_COLOR) != 0 || this->held_state.has_value(); + const std::optional pending_at = this->event_state->slot.pending_at(); + return (pending_bits & INBOX_TOPIC_COLOR) != 0 || + (pending_at.has_value() && this->state_is_due(*pending_at)); } - /// @brief Takes the slot, catches the role up on any teardown (complete_teardown()), then - /// holds the taken palettes if they were admitted under the current generation and applies - /// whichever is due. Main loop. + /// @brief Catches the role up on any teardown (complete_teardown()), then applies whichever + /// palette the slot holds is due (drain_scheduled_state()). Main loop. void drain_events(); /// Whether a palette's server-clock deadline has passed on the synchronized client clock. bool state_is_due(int64_t timestamp) const; - /// Applies the held palette and fires the listener once its server-clock deadline has passed. - void apply_due_state(); /// @brief Stops the role and discards the state the protocol task can reach. Protocol task, /// or the main loop in SendspinClient::stop() once every other thread is joined. @@ -99,10 +91,9 @@ struct ColorRole::Impl : RoleTeardown { // ======================================== // Struct fields + /// The applied palette, the one on_color() is handed. Main loop only; the slot's commit copies + /// the palette it delivers into it, under the Inbox mutex. ServerColorStateObject color{}; - // Palette taken from the inbox slot, awaiting its server-clock deadline. Main-thread only: - // written and read exclusively from drain_events()/complete_teardown() on the loop thread. - std::optional held_state; // Pointer fields SendspinClient* client; diff --git a/src/connection.h b/src/connection.h index b9a1760a..7f77c4ed 100644 --- a/src/connection.h +++ b/src/connection.h @@ -590,6 +590,14 @@ class SendspinConnection : public std::enable_shared_from_thisgroup_id_normalized_, true); } + /// @brief Notes that this connection's server sent metadata text over its cap, which the + /// parse cut (process_server_state_metadata()). + /// @return true the first time, so the server/state dispatch warns once per connection. + /// Protocol task only. + bool note_metadata_cut() { + return !std::exchange(this->metadata_cut_, true); + } + /// @brief Returns true if `role` is active on this connection, judged on the exact versioned /// name this library implements. /// @@ -1244,6 +1252,10 @@ class SendspinConnection : public std::enable_shared_from_this slot; }; diff --git a/src/inbox.h b/src/inbox.h index cee7c2eb..77551955 100644 --- a/src/inbox.h +++ b/src/inbox.h @@ -115,7 +115,10 @@ class InboxSlot; * the inbox mutex is held. merge() functors passed to InboxSlot must be pure data operations on * the slot value, so nothing that calls back into application code. The Inbox mutex is a leaf, like * every library lock (docs/conventions.md): nothing takes another library lock while holding it, - * and nothing locks the Inbox while holding another library lock. + * and nothing locks the Inbox while holding another library lock. So no slot takes a payload + * conditionally on a server-clock deadline, whose clock read takes a lock of its own: a role whose + * state waits for one decides with no lock held (ScheduledStateSlot). Every critical section is a + * short copy of bounded size; the largest is documented at MetadataRole::Impl::fill_place(). * * @code * Inbox inbox; @@ -333,6 +336,10 @@ class InboxSlot { this->bind(inbox, topic_bit); } + /// @brief Constructs unbound, holding `initial`, for a value read() and settle() keep in the + /// slot between drains rather than reset (see bind()) + explicit InboxSlot(T initial) : slot_(std::move(initial)) {} + ~InboxSlot() { // Release the bit claim so a replacement slot (e.g. a role re-added before start) can // bind it. Requires the bound Inbox to outlive this slot. @@ -426,6 +433,37 @@ class InboxSlot { this->inbox_->clear_bit_locked(this->topic_bit_); } + /// @brief Calls fn(const T& current) under the Inbox mutex, pending or not, and returns what it + /// returns: for a value that stays in the slot after a drain decided on it. `fn` runs under + /// the Inbox mutex: a pure data operation only. + template + auto read(ReadFn&& fn) const { + using Result = std::invoke_result_t; + if (!this->check_bound()) { + return Result{}; + } + std::lock_guard lock(this->inbox_->mutex_); + return fn(static_cast(this->slot_)); + } + + /// @brief Changes the value in place with fn(T& current), pending or not; fn returns whether + /// anything is still pending, as for edit(): false cleans the slot and clears its topic bit, + /// keeping the value as fn left it, and true leaves the slot as pending as it was. For a value + /// that stays in the slot after a drain decided on it, unlike edit(), which leaves a clean slot + /// untouched. `fn` runs under the Inbox mutex: a pure data operation only. + template + void settle(SettleFn&& fn) { + if (!this->check_bound()) { + return; + } + std::lock_guard lock(this->inbox_->mutex_); + if (fn(this->slot_)) { + return; + } + this->dirty_ = false; + this->inbox_->clear_bit_locked(this->topic_bit_); + } + /// @brief Move the accumulated value out if dirty /// @return true if a value was taken, false if the slot was clean or unbound. bool take(T& out) { @@ -536,7 +574,7 @@ class GenerationSlot { /// @brief Merges a delta into the pending payload with fn(T& current, T&& delta), under the /// rule in the class comment. `fn` runs under the Inbox mutex: a pure data operation only. /// The delta is taken and merged by reference: a by-value or stamped copy would put another - /// payload, a metadata state's strings among them, on the protocol task's stack. + /// payload on the protocol task's stack. template void merge(MergeFn&& fn, T&& delta, uint32_t generation) { this->slot_.update([&fn, &delta, generation](Stamped& current) { diff --git a/src/metadata_role.cpp b/src/metadata_role.cpp index 81ff00b0..1e87c24d 100644 --- a/src/metadata_role.cpp +++ b/src/metadata_role.cpp @@ -19,6 +19,7 @@ #include "sendspin/client.h" #include +#include #include static const char* const TAG = "sendspin.metadata"; @@ -27,17 +28,52 @@ namespace sendspin { namespace { -/// @brief Folds a newly arrived state into the states the main loop has not taken yet -/// -/// `incoming` always carries exactly one state, in `oldest`. The first one to arrive after a -/// drain keeps that place; every later one becomes `newest` (see PendingMetadataStates). Runs -/// under the Inbox mutex, so it stays a pure data operation. -void coalesce_metadata_states(PendingMetadataStates& current, PendingMetadataStates&& incoming) { - if (!current.oldest.has_value()) { - current.oldest = std::move(incoming.oldest); - return; +/// @brief The text fields `config` keeps +EnumSet kept_fields(const MetadataRoleConfig& config) { + EnumSet kept; + const std::pair flags[] = { + {config.title, MetadataField::TITLE}, + {config.artist, MetadataField::ARTIST}, + {config.album_artist, MetadataField::ALBUM_ARTIST}, + {config.album, MetadataField::ALBUM}, + {config.artwork_url, MetadataField::ARTWORK_URL}, + }; + for (const auto& [keep, field] : flags) { + if (keep) { + kept.insert(field); + } } - current.newest = std::move(incoming.oldest); + 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 +std::array reserve_metadata_text( + const MetadataTextLayout& layout) { + const auto room = [&layout](MetadataField field) { + return layout.kept.contains(field) ? metadata_field_cap(field) : 0; + }; + return {ReservedText(room(MetadataField::TITLE)), ReservedText(room(MetadataField::ARTIST)), + ReservedText(room(MetadataField::ALBUM_ARTIST)), + ReservedText(room(MetadataField::ALBUM)), + ReservedText(room(MetadataField::ARTWORK_URL))}; +} + +/// @brief The public state's member for `field` +std::optional& text_field(ServerMetadataStateObject& state, MetadataField field) { + switch (field) { + case MetadataField::TITLE: + return state.title; + case MetadataField::ARTIST: + return state.artist; + case MetadataField::ALBUM_ARTIST: + return state.album_artist; + case MetadataField::ALBUM: + return state.album; + case MetadataField::ARTWORK_URL: + return state.artwork_url; + } + return state.title; } } // namespace @@ -46,14 +82,19 @@ void coalesce_metadata_states(PendingMetadataStates& current, PendingMetadataSta // Impl constructor / destructor // ============================================================================ -MetadataRole::Impl::Impl(SendspinClient* client) - : client(client), event_state(std::make_unique()) {} +MetadataRole::Impl::Impl(MetadataRoleConfig config, SendspinClient* client) + : config(config), + layout(metadata_text_layout(kept_fields(config))), + metadata_text(reserve_metadata_text(this->layout)), + client(client), + event_state(std::make_unique()) {} // ============================================================================ // MetadataRole forwarding (public API → Impl) // ============================================================================ -MetadataRole::MetadataRole(SendspinClient* client) : impl_(std::make_unique(client)) {} +MetadataRole::MetadataRole(MetadataRoleConfig config, SendspinClient* client) + : impl_(std::make_unique(config, client)) {} MetadataRole::~MetadataRole() = default; @@ -119,17 +160,49 @@ void MetadataRole::Impl::attach_inbox(Inbox& inbox) { this->event_state->slot.bind(inbox, INBOX_TOPIC_METADATA); } +bool MetadataRole::Impl::start() { + const size_t bytes = METADATA_TEXT_BLOCKS * static_cast(this->layout.block_bytes); + if (bytes == 0 || this->text_blocks) { + return true; + } + if (!this->text_blocks.allocate(bytes, this->config.buffer_location)) { + SS_LOGE(TAG, "Failed to allocate the %zu byte metadata text buffer", bytes); + return false; + } + return true; +} + void MetadataRole::Impl::build_hello_fields(ClientHelloMessage& msg) { msg.supported_roles.push_back(SendspinRole::METADATA); } -void MetadataRole::Impl::handle_server_state(ServerMetadataStateObject&& metadata) const { +MetadataStateOutcome MetadataRole::Impl::handle_server_state(JsonObject root) { const uint32_t generation = this->cleanup_generation.load(std::memory_order_acquire); - // messaging.md "server/state": each included metadata object is the role's full state, never - // an overlay on the one before it (see coalesce_metadata_states). - PendingMetadataStates arrival; - arrival.oldest = std::move(metadata); - this->event_state->slot.merge(coalesce_metadata_states, std::move(arrival), generation); + // Parsed, and rejected or cut (logged), with no lock held; only the copy runs under it. + MetadataStateView parsed; + const MetadataStateOutcome outcome = + process_server_state_metadata(root, this->layout.kept, &parsed); + if (outcome == MetadataStateOutcome::PARSED || outcome == MetadataStateOutcome::CUT) { + // messaging.md "server/state": each included metadata object is the role's full state, + // never an overlay on the one before it, so it replaces the place's state whole. + this->event_state->slot.arrive( + [this, &parsed](MetadataStateBuffer& place) { this->fill_place(place, parsed); }, + generation); + } + return outcome; +} + +void MetadataRole::Impl::fill_place(MetadataStateBuffer& place, const MetadataStateView& parsed) { + place.payload = parsed.payload; + char* block = reinterpret_cast(this->text_blocks.data()) + + static_cast(place.block) * this->layout.block_bytes; + for (size_t i = 0; i < METADATA_FIELD_COUNT; ++i) { + const uint16_t size = parsed.payload.text_sizes[i].value_or(0); + if (size > 0) { + // At most the field's cap, the room the layout gives it + std::memcpy(block + this->layout.offsets[i], parsed.text[i], size); + } + } } bool MetadataRole::Impl::state_is_due(int64_t timestamp) const { @@ -140,62 +213,45 @@ bool MetadataRole::Impl::state_is_due(int64_t timestamp) const { return client_ts == 0 || client_ts <= platform_time_us(); } -void MetadataRole::Impl::apply_due_state() { - if (!this->held_state.has_value() || !this->state_is_due(this->held_state->timestamp)) { - return; +std::optional MetadataRole::Impl::text_of(const MetadataStateBuffer& state, + MetadataField field) const { + const auto i = static_cast(field); + const std::optional size = state.payload.text_sizes[i]; + if (!size.has_value()) { + return std::nullopt; } + const char* block = reinterpret_cast(this->text_blocks.data()) + + static_cast(state.block) * this->layout.block_bytes; + return std::string_view(block + this->layout.offsets[i], *size); +} - this->metadata = std::move(this->held_state.value()); - this->held_state.reset(); +void MetadataRole::Impl::fill_public_state(const MetadataStateBuffer& state) { + const MetadataStatePayload& payload = state.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)); + } +} + +void MetadataRole::Impl::apply_current_state() { + this->fill_public_state(this->current); if (this->listener) { this->listener->on_metadata(this->metadata); } } void MetadataRole::Impl::drain_events() { - PendingMetadataStates taken; - bool have_taken = false; - const uint32_t generation = - take_current_payload(*this, this->event_state->slot, taken, have_taken, TAG, "metadata"); - // Every check below against `generation` also catches a listener callback that re-entered - // teardown (a listener calling stop()): its cleanup() moved the generation on, and its own - // drain already reset this role. - if (!this->accepts(generation)) { - return; - } - - // InboxSlot has no take_if (a deadline predicate must not run under the shared Inbox mutex; - // see inbox.h), so the server-clock deadline gate is split in two: take() unconditionally - // moves the pending states into held_state, then each deadline is evaluated below with no - // lock held at all. - // - // roles/metadata/v1.md "Scheduled metadata updates": a state whose timestamp is still in the - // future is the pending update and a newer one replaces it, while a past or present one is - // applied at once and discards the pending update. Both fall out of replacing held_state with - // each taken state in arrival order, applying whatever is due in between. - if (have_taken) { - const bool collapses = - taken.newest.has_value() && this->state_is_due(taken.newest->timestamp); - if (taken.oldest.has_value() && !collapses) { - // Two states that are both due collapse instead: the older would be superseded - // within this tick, so no listener could observe it. - this->held_state = std::move(taken.oldest); - this->apply_due_state(); - if (!this->accepts(generation)) { - return; - } - } - if (taken.newest.has_value()) { - this->held_state = std::move(taken.newest); - } - } - - // A future-dated state is held across ticks without any topic bit set (take() above cleared - // it). It is re-evaluated against its deadline on later ticks only because needs_drain() ORs - // in held_state.has_value() alongside the INBOX_TOPIC_METADATA bit test, so this - // drain_events() keeps running each tick until the deadline fires. Dropping that OR term - // would strand the state until an unrelated new one re-set the topic bit. - this->apply_due_state(); + // 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. + drain_scheduled_state( + *this, this->event_state->slot, + [this](MetadataStateBuffer& place) { std::swap(place, this->current); }, + [this] { this->apply_current_state(); }, TAG, "metadata"); } void MetadataRole::Impl::cleanup() { @@ -209,8 +265,8 @@ void MetadataRole::Impl::cleanup() { } void MetadataRole::Impl::complete_teardown() { - this->metadata = {}; - this->held_state.reset(); + this->fill_public_state(MetadataStateBuffer{}); + 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 dba05321..0454d496 100644 --- a/src/metadata_role_impl.h +++ b/src/metadata_role_impl.h @@ -18,40 +18,64 @@ #pragma once #include "inbox.h" +#include "platform/memory.h" #include "protocol_messages.h" +#include "reserved_text.h" +#include "scheduled_state_slot.h" #include "sendspin/metadata_role.h" #include "teardown_tracker.h" +#include +#include +#include #include #include +#include namespace sendspin { class SendspinClient; struct ClientHelloMessage; -/// @brief Metadata states handed to the main loop but not yet drained -/// -/// messaging.md "server/state" lets a server bring a client up to date and then schedule the next -/// update straight after it, so two states can land between two main-loop ticks. Both are kept, -/// in arrival order, so the current one is still applied on the tick that also takes the -/// scheduled one. A third state in the same window replaces `newest`, which no observer has seen. -struct PendingMetadataStates { - std::optional oldest; - std::optional newest; +/// @brief Text blocks a metadata role keeps, one state's kept text each (MetadataTextLayout): one +/// for each place a state can be, the slot's oldest and newest places and the main loop's current +/// state (ScheduledStateSlot). Each place names its block by index; a delivery swaps the +/// delivered place's block with the current state's, so each block always has one place. +constexpr uint8_t METADATA_TEXT_BLOCKS = 3; + +/// @brief A metadata state and the text block it names, an index into the role's text blocks; +/// the block stays with the place when the place holds no state +struct MetadataStateBuffer { + MetadataStatePayload payload{}; + uint8_t block{0}; }; +/// @brief The server-clock timestamp `state` applies at (ScheduledStateSlot) +inline int64_t scheduled_time(const MetadataStateBuffer& state) { + return state.payload.timestamp; +} + /// @brief Private implementation of the metadata role struct MetadataRole::Impl : RoleTeardown { - explicit Impl(SendspinClient* client); + Impl(MetadataRoleConfig config, SendspinClient* client); ~Impl() = default; // ======================================== // Event state // ======================================== + /// The block each place names at first: the slot's oldest and newest places, then the + /// current state + static constexpr uint8_t OLDEST_FIRST_BLOCK = 0; + static constexpr uint8_t NEWEST_FIRST_BLOCK = 1; + static constexpr uint8_t CURRENT_FIRST_BLOCK = 2; + static_assert(CURRENT_FIRST_BLOCK + 1 == METADATA_TEXT_BLOCKS, + "one block for each place a state can be"); + struct EventState { - GenerationSlot slot; + ScheduledStateSlot slot{ + MetadataStateBuffer{.block = OLDEST_FIRST_BLOCK}, + MetadataStateBuffer{.block = NEWEST_FIRST_BLOCK}}; }; // ======================================== @@ -59,22 +83,43 @@ struct MetadataRole::Impl : RoleTeardown { // ======================================== void attach_inbox(Inbox& inbox); + /// @brief Allocates the text blocks, once: they are kept across restarts, since the states + /// that own them hold only their indices. Main loop, from SendspinClient::start(). + /// @return false (logged) when they cannot be allocated, which fails the start. + bool start(); void build_hello_fields(ClientHelloMessage& msg); - void handle_server_state(ServerMetadataStateObject&& metadata) const; - // True if a slot state needs taking, or a state already held from a prior tick (see - // held_state) is still waiting out its server-clock deadline: the deadline itself sets no - // inbox bit, so held_state must be polled every tick until it fires. + /// @brief Parses a server/state metadata object and hands it to the slot, copying its kept + /// text from the document into the block of the place it takes (fill_place()). Protocol task, + /// inside ParsedJsonMessage::extract(), the only window the document is alive. + /// @return What the parse made of it; a REJECTED or IGNORED object reaches no slot. + MetadataStateOutcome handle_server_state(JsonObject root); + /// @brief Writes `parsed` into `place`, its text into the block `place` names. Runs under the + /// Inbox mutex (ScheduledStateSlot::arrive()): a pure data copy of at most a text block, the + /// largest critical section the Inbox has. + void fill_place(MetadataStateBuffer& place, const MetadataStateView& parsed); + /// @brief True if a state arrived, or the pending state the last drain left in the slot has + /// come due: the deadline sets no inbox bit, so it is checked every tick, against + /// ScheduledStateSlot::pending_at(), without the Inbox mutex. bool needs_drain(uint32_t pending_bits) const { - return (pending_bits & INBOX_TOPIC_METADATA) != 0 || this->held_state.has_value(); + const std::optional pending_at = this->event_state->slot.pending_at(); + return (pending_bits & INBOX_TOPIC_METADATA) != 0 || + (pending_at.has_value() && this->state_is_due(*pending_at)); } - /// @brief Takes the slot, catches the role up on any teardown (complete_teardown()), then - /// holds the taken states if they were admitted under the current generation and applies - /// whichever is due. Main loop. + /// @brief Catches the role up on any teardown (complete_teardown()), then applies whichever + /// state the slot holds is due (drain_scheduled_state()). Main loop. 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; - /// Applies the held state and fires the listener once its server-clock deadline has passed. - void apply_due_state(); + /// @brief Fills the public state from the current state and fires on_metadata(). 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 text `state` carries for `field`, viewing its block; nullopt when it carries + /// none + std::optional text_of(const MetadataStateBuffer& state, + MetadataField field) const; /// @brief Stops the role and discards the state the protocol task can reach. Protocol task, /// or the main loop in SendspinClient::stop() once every other thread is joined. @@ -102,10 +147,20 @@ struct MetadataRole::Impl : RoleTeardown { // ======================================== // Struct fields + MetadataRoleConfig config; + /// The kept fields and where each lies in a text block. Fixed from add_metadata(). + MetadataTextLayout layout; + /// 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. ServerMetadataStateObject metadata{}; - // State taken from the inbox slot, awaiting its server-clock deadline. Main-thread only: - // written and read exclusively from drain_events()/complete_teardown() on the loop thread. - std::optional held_state; + /// Storage for the 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/platform/json_arena.h b/src/platform/json_arena.h index 7192dd4d..984bd374 100644 --- a/src/platform/json_arena.h +++ b/src/platform/json_arena.h @@ -293,11 +293,13 @@ struct RefersIntoDocument> : RefersIntoDocument* out) { - if (var.is()) { - *out = var.as(); - } else if (!var.isUnbound() && !var.isNull()) { - SS_LOGW(TAG, "Ignoring field '%s': expected string", name); - } -} - // Parses a single `[R, G, B]` color field of a server/state color object. A field the object does // not carry, or carries malformed (logged), leaves `out` without a value. Each component is read // as a uint8, so its type check is also its range check. @@ -684,16 +678,30 @@ bool process_server_command_source(JsonObject root, SourceCommand* source_cmd) { return true; } -// server/state is parsed per section rather than into a single aggregate struct. Each section -// here is an out-of-line function that fills a caller-owned struct directly, so no section's -// fields are materialized twice (once in a parser frame, once in the caller's). The caller keeps -// every section a role takes live together in its own frame for one parse of the document; that -// combined frame is on the protocol task, whose stack is bounded on ESP-IDF and covers it (see -// SendspinClientConfig::DEFAULT_PROTOCOL_TASK_STACK_SIZE). +MetadataTextOutcome metadata_text_from_wire(JsonVariantConst var, const char* name, size_t cap, + bool strict, std::string_view* out) { + if (!var.is()) { + if (!var.isUnbound() && !var.isNull()) { + SS_LOGW(TAG, "Ignoring field '%s': expected string", name); + } + return MetadataTextOutcome::ABSENT; + } + const auto text = var.as(); + if (text.size() <= cap) { + *out = text; + return MetadataTextOutcome::KEPT; + } + if (strict) { + return MetadataTextOutcome::REJECTED; + } + *out = utf8_truncate(text, cap); + return MetadataTextOutcome::CUT; +} -bool process_server_state_metadata(JsonObject root, ServerMetadataStateObject* metadata) { +MetadataStateOutcome process_server_state_metadata(JsonObject root, EnumSet kept, + MetadataStateView* metadata) { if (metadata == nullptr || !root["payload"]["metadata"].is()) { - return false; + return MetadataStateOutcome::IGNORED; } const JsonObject metadata_object = root["payload"]["metadata"]; @@ -701,26 +709,48 @@ bool process_server_state_metadata(JsonObject root, ServerMetadataStateObject* m const JsonVariantConst timestamp = metadata_object["timestamp"]; if (!timestamp.is()) { SS_LOGE(TAG, "Invalid metadata state object: missing or non-integer timestamp"); - return false; + return MetadataStateOutcome::IGNORED; } // messaging.md "server/state": every message carries the full state of each role object it // includes, so an included metadata object is parsed into a fresh state rather than overlaid // on what came before. - *metadata = ServerMetadataStateObject{}; - metadata->timestamp = timestamp.as(); - - parse_metadata_string_field(metadata_object["title"], "title", &metadata->title); - parse_metadata_string_field(metadata_object["artist"], "artist", &metadata->artist); - parse_metadata_string_field(metadata_object["album_artist"], "album_artist", - &metadata->album_artist); - parse_metadata_string_field(metadata_object["album"], "album", &metadata->album); - parse_metadata_string_field(metadata_object["artwork_url"], "artwork_url", - &metadata->artwork_url); + *metadata = MetadataStateView{}; + MetadataStatePayload& payload = metadata->payload; + payload.timestamp = timestamp.as(); + + // A field the role does not keep is still measured against its cap: the cap holds for every + // field of an active role's object, kept or not. + bool cut = false; + for (size_t i = 0; i < METADATA_FIELD_COUNT; ++i) { + const auto field = static_cast(i); + const char* name = to_cstr(field); + std::string_view value; + switch (metadata_text_from_wire(metadata_object[name], name, metadata_field_cap(field), + METADATA_STRICT, &value)) { + case MetadataTextOutcome::ABSENT: + break; + case MetadataTextOutcome::CUT: + cut = true; + [[fallthrough]]; + case MetadataTextOutcome::KEPT: + if (kept.contains(field)) { + metadata->text[i] = value.data(); + payload.text_sizes[i] = static_cast(value.size()); + } + break; + case MetadataTextOutcome::REJECTED: + SS_LOGW(TAG, + "Invalid server/state metadata: %s is longer than %zu bytes; closing " + "connection", + name, metadata_field_cap(field)); + return MetadataStateOutcome::REJECTED; + } + } if (auto v = read_uint_field(metadata_object["year"], "year")) { - metadata->year = v; + payload.year = v; } if (auto v = read_uint_field(metadata_object["track"], "track")) { - metadata->track = v; + payload.track = v; } // roles/metadata/v1.md "server/state metadata object": omitting progress clears the client's @@ -740,12 +770,19 @@ bool process_server_state_metadata(JsonObject root, ServerMetadataStateObject* m read_uint_field(progress_object["playback_speed"], "playback_speed")) { progress.playback_speed = *v; } - metadata->progress = progress; + payload.progress = progress; } - return true; + return cut ? MetadataStateOutcome::CUT : MetadataStateOutcome::PARSED; } +// server/state is parsed per section rather than into a single aggregate struct. The color and +// controller sections are out-of-line functions that each fill a caller-owned struct directly, so +// no section's fields are materialized twice (once in a parser frame, once in the caller's). The +// caller keeps both live together in its own frame for one parse of the document; that combined +// frame is on the protocol task, whose stack is bounded on ESP-IDF and covers it (see +// SendspinClientConfig::DEFAULT_PROTOCOL_TASK_STACK_SIZE). + bool process_server_state_color(JsonObject root, ServerColorStateObject* color) { if (color == nullptr || !root["payload"]["color"].is()) { return false; diff --git a/src/protocol_messages.h b/src/protocol_messages.h index 5f86a0b2..c482beab 100644 --- a/src/protocol_messages.h +++ b/src/protocol_messages.h @@ -696,6 +696,116 @@ struct ControllerStatePayload { static_assert(std::is_trivially_copyable_v, "parsed on the protocol task and carried to the main loop with nothing on the heap"); +// --- metadata_role.h --- + +/// @brief Longest title, artist, album_artist and album a server sends, in UTF-8 bytes +/// (roles/metadata/v1.md "server/state metadata object") +constexpr size_t MAX_METADATA_TEXT_BYTES = 255; + +/// @brief Longest artwork_url a server sends, in UTF-8 bytes (roles/metadata/v1.md "server/state +/// metadata object") +constexpr size_t MAX_METADATA_URL_BYTES = 2048; + +/// @brief A text field of a server/state metadata object, in the order a text block lays them out +/// (MetadataTextLayout) +enum class MetadataField : uint8_t { + TITLE, + ARTIST, + ALBUM_ARTIST, + ALBUM, + ARTWORK_URL, +}; + +/// @brief How many text fields a metadata state has +inline constexpr size_t METADATA_FIELD_COUNT = static_cast(MetadataField::ARTWORK_URL) + 1; + +static_assert(METADATA_FIELD_COUNT <= 8, "EnumSet needs 8 bits"); + +/// @brief The field's member name in the server/state metadata object +inline const char* to_cstr(MetadataField field) { + switch (field) { + case MetadataField::TITLE: + return "title"; + case MetadataField::ARTIST: + return "artist"; + case MetadataField::ALBUM_ARTIST: + return "album_artist"; + case MetadataField::ALBUM: + return "album"; + case MetadataField::ARTWORK_URL: + return "artwork_url"; + } + return "title"; +} + +/// @brief The most UTF-8 bytes `field` holds +constexpr size_t metadata_field_cap(MetadataField field) { + return field == MetadataField::ARTWORK_URL ? MAX_METADATA_URL_BYTES : MAX_METADATA_TEXT_BYTES; +} + +/// @brief Where a metadata state keeps each text field the role keeps: one region per kept field +/// at that field's cap, in field order, in a block of `block_bytes` the state owns +struct MetadataTextLayout { + EnumSet kept{}; + std::array offsets{}; + uint16_t block_bytes{0}; +}; + +/// @brief The layout of the fields in `kept` +constexpr MetadataTextLayout metadata_text_layout(EnumSet kept) { + MetadataTextLayout layout; + layout.kept = kept; + for (size_t i = 0; i < METADATA_FIELD_COUNT; ++i) { + const auto field = static_cast(i); + if (kept.contains(field)) { + layout.offsets[i] = layout.block_bytes; + layout.block_bytes = + static_cast(layout.block_bytes + metadata_field_cap(field)); + } + } + 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 +struct MetadataStatePayload { + int64_t timestamp{}; + std::optional year{}; + std::optional track{}; + std::optional progress{}; + /// Each text field's size in its block, by MetadataField; nullopt when the state carries no + /// value for it, or it is not kept + std::array, METADATA_FIELD_COUNT> text_sizes{}; +}; + +static_assert(std::is_trivially_copyable_v, + "parsed on the protocol task and carried to the main loop with nothing on the heap"); + +/// @brief A parsed server/state metadata object whose text still lies in the parsed document, so +/// it is valid only while the document is (ParsedJsonMessage::extract()) +struct MetadataStateView { + /// The state, its text sizes those of the fields kept and carried + MetadataStatePayload payload{}; + /// Each such field's text in the document, payload.text_sizes bytes, by MetadataField + std::array text{}; +}; + +/// @brief What metadata_text_from_wire() made of a text field of a server/state metadata object +enum class MetadataTextOutcome : uint8_t { + ABSENT, ///< Absent, null or not a string (logged): the state carries no value + KEPT, ///< Within its cap: held as sent + CUT, ///< While not strict, over its cap: held cut to it at a UTF-8 character boundary + REJECTED, ///< While strict, over its cap: a protocol error +}; + +/// @brief What process_server_state_metadata() made of a server/state metadata object +enum class MetadataStateOutcome : uint8_t { + IGNORED, ///< Absent, or rejected for its timestamp (logged): nothing to apply + PARSED, ///< Parsed, every text field within its cap + CUT, ///< Parsed, a text field over its cap cut to it (or dropped, when not kept) + REJECTED, ///< A text field over its cap while METADATA_STRICT: a protocol error +}; + // --- artwork_role.h --- inline const char* to_cstr(SendspinImageFormat format) { @@ -1222,17 +1332,26 @@ bool process_server_command_message(JsonObject root, ServerCommandMessage* cmd_m /// is absent or invalid bool process_server_command_source(JsonObject root, SourceCommand* source_cmd); -/// @brief Parses the metadata section of a server/state JSON message; true if the section was -/// present and parsed -/// -/// The server/state sections are parsed per section, each into a caller-owned struct, not into -/// an aggregate a parser frame would copy out of. The caller holds every section a role takes -/// live together for one parse, a combined frame on the protocol task, whose stack is bounded on -/// ESP-IDF (at least SendspinClientConfig::DEFAULT_PROTOCOL_TASK_STACK_SIZE). -bool process_server_state_metadata(JsonObject root, ServerMetadataStateObject* metadata); +/// @brief Reads a text field of a server/state metadata object, at most `cap` bytes +/// (roles/metadata/v1.md "server/state metadata object"). While not `strict`, a longer one is cut +/// to `cap` at a UTF-8 character boundary; while `strict` it is rejected (messaging.md +/// "Out-of-range values"). +/// @param out Views the text in the document for KEPT and CUT; left untouched otherwise. +MetadataTextOutcome metadata_text_from_wire(JsonVariantConst var, const char* name, size_t cap, + bool strict, std::string_view* out); + +/// @brief Parses the metadata section of a server/state JSON message into `metadata`, viewing +/// each text field `kept` holds in the document and dropping the others +MetadataStateOutcome process_server_state_metadata(JsonObject root, EnumSet kept, + MetadataStateView* metadata); /// @brief Parses the color section of a server/state JSON message; true if the section was /// present and parsed +/// +/// The color and controller sections are parsed each into a caller-owned struct, not into an +/// aggregate a parser frame would copy out of. The caller holds both live together for one parse, +/// a combined frame on the protocol task, whose stack is bounded on ESP-IDF (at least +/// SendspinClientConfig::DEFAULT_PROTOCOL_TASK_STACK_SIZE). bool process_server_state_color(JsonObject root, ServerColorStateObject* color); /// @brief Parses the controller section of a server/state JSON message; true if the section was diff --git a/src/scheduled_state_slot.h b/src/scheduled_state_slot.h new file mode 100644 index 00000000..cd9a9fac --- /dev/null +++ b/src/scheduled_state_slot.h @@ -0,0 +1,295 @@ +// Copyright 2026 Sendspin Contributors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/// @file scheduled_state_slot.h +/// @brief The inbox slot of a role whose server/state object applies at its server-clock +/// timestamp, and the drain shared by those roles + +#pragma once + +#include "inbox.h" +#include "teardown_tracker.h" + +#include +#include +#include + +namespace sendspin { + +/** + * @brief Generation-stamped inbox slot for a role whose state applies at its server-clock + * timestamp: the states the protocol task hands the main loop, and the one a drain left waiting + * + * roles/metadata/v1.md "Scheduled metadata updates" and roles/color/v1.md "Scheduled color + * updates": a state whose timestamp is still in the future is the pending update and a newer one + * replaces it, while a past or present one is applied at once and discards the pending update. + * messaging.md "server/state" lets a server bring a client up to date and then schedule the next + * update straight after it, so two states can land between two drains. Both are kept, in arrival + * order, in the oldest and newest places, so the current one is still applied on the tick that + * also takes the scheduled one. A third state in the same window replaces the newest, which no + * observer has seen, and a state that finds the pending one replaces it. + * + * The pending state waits here, in the oldest place. A drain (drain_scheduled_state()) peeks at + * the timestamps under the Inbox mutex, decides with no lock held, since a deadline reads the + * client clock behind a lock of its own (see Inbox), and commits under the mutex only if nothing + * arrived or was reset since the peek. So a state is never copied out to be decided on: a commit + * hands the delivered one over through `deliver`, which for a state whose content lies elsewhere + * trades storage rather than copying it. + * + * Every arrival keeps the newest generation's states, as GenerationSlot does: one stamped with a + * later generation than the held states discards them, and one stamped with an earlier generation + * is dropped, so a producer that lost a race with a teardown cannot revive what the teardown + * discarded. + * + * `State` is a place's storage. A discard keeps it and only marks the place empty, so a State that + * names storage of its own (a metadata state's text block) keeps naming it, and the places and + * whatever `deliver` trades with always hold distinct storage. `scheduled_time(const State&)`, + * found by argument-dependent lookup, returns the server-clock timestamp it applies at. + */ +template +class ScheduledStateSlot { +public: + /// @brief A place of the slot + enum class Place : uint8_t { + NONE, + OLDEST, + NEWEST, + }; + + /// @brief What a drain decides on, copied out under the Inbox mutex: the timestamps and stamp, + /// never the states + struct Peek { + /// The oldest place's timestamp, when it holds a state + std::optional oldest; + /// The newest place's timestamp, when it holds a state; only ever beside an oldest + std::optional newest; + /// The teardown generation the states were admitted under + uint32_t generation{0}; + /// Changes with every arrival and reset, so a commit can tell the slot changed + uint32_t sequence{0}; + }; + + /// @brief What a drain does with the states it peeked; every other state is discarded + struct Decision { + Place deliver{Place::NONE}; ///< The state applied now + Place pending{Place::NONE}; ///< The state left waiting for its timestamp + }; + + ScheduledStateSlot() = default; + + /// @brief Constructs with the oldest and newest places holding the storage `oldest` and + /// `newest` name + ScheduledStateSlot(State oldest, State newest) + : slot_(Places{.oldest = {.state = oldest}, .newest = {.state = newest}}) {} + + /// @brief Binds to `inbox` and the topic bit this slot owns exclusively (see InboxSlot::bind()) + void bind(Inbox& inbox, uint32_t topic_bit) { + this->slot_.bind(inbox, topic_bit); + } + + /// @brief Hands the main loop a state, which fill(State& place) writes into the place it takes: + /// the oldest when the slot holds none or only the pending state, which it replaces, and the + /// newest otherwise, replacing any state there. `fill` writes the content and keeps the + /// storage the place names. Protocol task. `fill` runs under the Inbox mutex: a pure data + /// operation only. + /// @param generation The role's teardown generation the state was admitted under. + template + void arrive(FillFn&& fill, uint32_t generation) { + this->slot_.update([&fill, generation](Places& places) { + if (places.oldest.present && places.generation != generation) { + if (!count_after(generation, places.generation)) { + return; // Older than what is held: the teardown discarded it. + } + discard(places); + } + places.generation = generation; + Entry& place = places.oldest.present && !places.pending ? places.newest : places.oldest; + fill(place.state); + place.present = true; + places.pending = false; + ++places.sequence; + }); + } + + /// @brief The timestamps and stamp of the states the slot holds, pending or not. Main loop. + Peek peek() const { + return this->slot_.read([](const Places& places) { + Peek peek; + if (places.oldest.present) { + peek.oldest = scheduled_time(places.oldest.state); + } + if (places.newest.present) { + peek.newest = scheduled_time(places.newest.state); + } + peek.generation = places.generation; + peek.sequence = places.sequence; + return peek; + }); + } + + /// @brief What to do with the states `peek` saw, by the rules in the class comment; + /// is_due(timestamp) says whether a timestamp's moment has come, and is asked about the oldest + /// only when the newest is not due. Runs with no lock held. + template + static Decision decide(const Peek& peek, IsDueFn&& is_due) { + if (peek.newest.has_value() && is_due(*peek.newest)) { + // Two states that are both due collapse instead: the older would be superseded within + // this tick, so no listener could observe it. + return {.deliver = Place::NEWEST, .pending = Place::NONE}; + } + if (!peek.oldest.has_value()) { + return {}; + } + const Place later = peek.newest.has_value() ? Place::NEWEST : Place::NONE; + if (is_due(*peek.oldest)) { + return {.deliver = Place::OLDEST, .pending = later}; + } + return {.deliver = Place::NONE, .pending = later != Place::NONE ? later : Place::OLDEST}; + } + + /// @brief Carries `decision` out unless the slot changed since `peek`: calls deliver(State& + /// place) for the delivered state, discards every state but the pending one, which waits in + /// the oldest place, and cleans the slot, keeping its timestamp as pending_at(). Main loop. + /// `deliver` runs under the Inbox mutex: a pure data operation only. + /// @return false, changing nothing, when a state arrived or the slot was reset after `peek`: + /// decide again on a fresh peek. + template + bool commit(const Peek& peek, Decision decision, DeliverFn&& deliver) { + bool committed = false; + std::optional pending_at; + this->slot_.settle([&](Places& places) { + if (places.sequence != peek.sequence) { + return true; // Still pending: what arrived since the peek + } + if (decision.deliver != Place::NONE) { + deliver(entry(places, decision.deliver).state); + } + if (decision.pending == Place::NEWEST) { + std::swap(places.oldest, places.newest); + } + places.oldest.present = decision.pending != Place::NONE; + places.newest.present = false; + places.pending = places.oldest.present; + if (places.pending) { + pending_at = scheduled_time(places.oldest.state); + } + committed = true; + return false; + }); + if (committed) { + this->pending_at_ = pending_at; + } + return committed; + } + + /// @brief Discards every state, the pending one included. Protocol task, or the main loop in + /// SendspinClient::stop() once every other thread is joined. + void reset() { + this->slot_.settle([](Places& places) { + discard(places); + ++places.sequence; + return false; + }); + } + + /// @brief The pending state's timestamp as the last commit left it, so a drain waiting for it + /// takes no lock until it is due. Exact while the slot's topic bit is clear and its role has + /// no teardown to catch up on. Main loop only. + std::optional pending_at() const { + return this->pending_at_; + } + + /// @brief Forgets pending_at() for a teardown, whose cleanup() reset the slot. Main loop only, + /// from the role's complete_teardown(). + void forget_pending() { + this->pending_at_.reset(); + } + +private: + struct Entry { + State state{}; + /// Whether `state` holds a state; false keeps the storage it names + bool present{false}; + }; + + struct Places { + Entry oldest; + Entry newest; + /// Changes with every arrival and reset (Peek::sequence) + uint32_t sequence{0}; + /// The teardown generation the held states were admitted under + uint32_t generation{0}; + /// Whether `oldest` holds the state a drain left waiting for its timestamp + bool pending{false}; + }; + + static Entry& entry(Places& places, Place place) { + return place == Place::NEWEST ? places.newest : places.oldest; + } + + static void discard(Places& places) { + places.oldest.present = false; + places.newest.present = false; + places.pending = false; + } + + InboxSlot slot_; + /// See pending_at(). Main loop only. + std::optional pending_at_; +}; + +/// @brief The drain of a role whose state applies at its server-clock timestamp (see +/// ScheduledStateSlot): peeks at the slot, catches the role up on any teardown and keeps only +/// states stamped with its current generation (keep_current_payload()), decides with no lock held +/// (RoleImpl::state_is_due()), and commits, peeking again when the slot changed meanwhile. A +/// delivered state goes through deliver(State& place) under the Inbox mutex and then apply(), the +/// role's listener callback, outside it. Repeats while the state left pending has come due, so one +/// that falls due during the callback of the state before it is applied in the same tick. Main +/// loop. +template +void drain_scheduled_state(RoleImpl& role, ScheduledStateSlot& slot, DeliverFn&& deliver, + ApplyFn&& apply, const char* tag, const char* what) { + using Slot = ScheduledStateSlot; + const auto is_due = [&role](int64_t timestamp) { return role.state_is_due(timestamp); }; + for (;;) { + const typename Slot::Peek peek = slot.peek(); + bool current = peek.oldest.has_value(); + const uint32_t generation = keep_current_payload(role, current, peek.generation, tag, what); + // Also catches a catch-up whose clear callback re-entered teardown (a listener calling + // stop()): its cleanup() moved the generation on, and its own drain already reset this + // role. + if (!role.accepts(generation)) { + return; + } + // States stamped before the teardown are decided away: none delivered, none pending. + const typename Slot::Decision decision = + current ? Slot::decide(peek, is_due) : typename Slot::Decision{}; + if (!slot.commit(peek, decision, deliver)) { + continue; + } + if (decision.deliver == Slot::Place::NONE) { + return; + } + apply(); + // A callback that re-entered teardown (a listener calling stop()) reset the slot, and the + // drain stop() ran forgot pending_at() (the role's complete_teardown()), so this ends the + // loop too. + const std::optional pending_at = slot.pending_at(); + if (!pending_at.has_value() || !is_due(*pending_at)) { + return; + } + } +} + +} // namespace sendspin diff --git a/src/teardown_tracker.h b/src/teardown_tracker.h index 5865b598..99ef650b 100644 --- a/src/teardown_tracker.h +++ b/src/teardown_tracker.h @@ -38,7 +38,8 @@ namespace sendspin { * Every main-loop path that is about to act on something stamped with a generation calls * catch_up_teardown() with it first: the event drain with a stamped event's generation, and a * role drain with the role's current generation right after it has taken its GenerationSlot - * payload (inbox.h). Several teardowns between two catch-ups collapse into one main-loop half. + * payload (inbox.h) or peeked at its ScheduledStateSlot (scheduled_state_slot.h). Several + * teardowns between two catch-ups collapse into one main-loop half. * * Main loop only: written and read on the main loop, so it needs no lock or atomic. */ @@ -104,6 +105,24 @@ void catch_up_teardown(RoleImpl& role, uint32_t generation) { } } +/// @brief The second half of take_current_payload(), for a drain that peeks at its slot rather +/// than taking it (drain_scheduled_state()): catches the role up to its current teardown +/// generation, then keeps a payload stamped `stamp` only if it is that generation, dropping an +/// older one with a debug log +/// @param[in,out] have Whether there is a payload; false once an older one is dropped. +/// @return The role's generation the catch-up ran to. +template +uint32_t keep_current_payload(RoleImpl& role, bool& have, uint32_t stamp, const char* tag, + const char* what) { + const uint32_t generation = role.cleanup_generation.load(std::memory_order_acquire); + catch_up_teardown(role, generation); + if (have && stamp != generation) { + SS_LOGD(tag, "Dropping %s queued before the role was torn down", what); + have = false; + } + return generation; +} + /// @brief The head of every slot role's drain: takes the role's GenerationSlot payload, catches /// the role up to its current teardown generation (catch_up_teardown()), then keeps the payload /// only if it is stamped with that generation, dropping an older one with a debug log @@ -121,13 +140,7 @@ uint32_t take_current_payload(RoleImpl& role, GenerationSlot& slot, T& out, b const char* tag, const char* what) { uint32_t stamp = 0; have = slot.take(out, stamp); - const uint32_t generation = role.cleanup_generation.load(std::memory_order_acquire); - catch_up_teardown(role, generation); - if (have && stamp != generation) { - SS_LOGD(tag, "Dropping %s queued before the role was torn down", what); - have = false; - } - return generation; + return keep_current_payload(role, have, stamp, tag, what); } } // namespace sendspin diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 5309c302..3201a3dc 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -73,6 +73,11 @@ add_executable(sendspin_tests # SendspinDecoder::flac_decoder(), since a decoder object kept across streams shows nowhere else. set_source_files_properties(test_decoder.cpp PROPERTIES COMPILE_OPTIONS -fno-access-control) +# test_inbox.cpp probes the Inbox's mutex_ from another thread in the scheduled-state drain test, +# since whether a deadline check runs under it shows in no outcome: the decision is the same +# either way, and only the lock the check would take beneath it breaks the leaf-lock rule. +set_source_files_properties(test_inbox.cpp PROPERTIES COMPILE_OPTIONS -fno-access-control) + # Built only with the trace it tests. if(SENDSPIN_ENABLE_MESSAGE_TRACE) target_sources(sendspin_tests PRIVATE test_message_trace.cpp) @@ -164,17 +169,27 @@ endif() # IDLE wait: it calls SyncTask's wait_for_codec_header() and load_next_chunk() and returns items # through its inbound_, # resets the flags with event_flags_.clear_all() as start() would, sets and reads SyncContext's -# active_ordinal and next_header, and reads PlayerRole::Impl::stream_ordinal. +# active_ordinal and next_header, and reads PlayerRole::Impl::stream_ordinal. The metadata slot +# table plays the protocol task's half of the metadata role (handle_server_state(), and for a +# state stamped before a teardown, fill_place() through the slot's arrive() with the old stamp +# and the role's layout, then cleanup()) and runs the role's drain_events() alone where an arrival +# must land between its peek and its commit; it reads which text block each place names (the +# slot's places through ScheduledStateSlot::slot_, the role's current state), the pending +# state's text (text_of()) and ScheduledStateSlot::pending_at(), which nothing public shows, and +# installs the primary clock (ConnectionManager::published_ under published_mutex_) so a scheduled +# state waits for its deadline without a server, and moves it so one falls due inside a +# callback. set_source_files_properties(test_client_lifecycle.cpp PROPERTIES COMPILE_OPTIONS -fno-access-control) # test_role_deactivation.cpp reads what a removed role left behind and drives the pieces the # public surface does not expose: the sync task's run state and its item list, the artwork # transfer in flight and its decode thread's assembly buffers, the visualizer's stream flag and -# list, a state role's held scheduled update and its teardown generation (including the metadata -# slot written with that generation, a payload a drain takes across the removal), the controller's -# supported-commands word (seeded, stamped with the role's generation, so a command reaches the -# inactive-role send gate), plus the queued-event probe that proves ordering without a sleep (a -# role inbox's topic bits). +# list, a state role's pending scheduled update in its slot and the main loop's copy of its +# deadline (ScheduledStateSlot::pending_at()) and its teardown generation (including a metadata +# state handed to the slot with that generation, a payload a drain takes across the removal), the +# controller's supported-commands word (seeded, stamped with the role's generation, so a command +# reaches the inactive-role send gate), plus the queued-event probe that proves ordering without a +# sleep (a role inbox's topic bits). set_source_files_properties(test_role_deactivation.cpp PROPERTIES COMPILE_OPTIONS -fno-access-control) # test_player_role.cpp drives PlayerRole::Impl directly (no connected server, no sync-task thread), @@ -316,6 +331,9 @@ set_source_files_properties(test_pairing_state_machine.cpp PROPERTIES # reply a handler builds shares the arena with the parse that triggered it shows only in where # the parse leaves the bump pointer and in the arena's peak, which is rewound so it measures # one delivery; nothing a peer receives depends on either. +# - 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. set_source_files_properties(test_encrypted_lifecycle.cpp PROPERTIES COMPILE_OPTIONS "-fno-access-control") diff --git a/tests/README.md b/tests/README.md index c85c81b4..b046d492 100644 --- a/tests/README.md +++ b/tests/README.md @@ -59,8 +59,8 @@ a watchdog report rather than a flaky elapsed-time assertion. Each `test_*.cpp` file covers one unit of cross-platform logic: - `test_protocol.cpp`: wire-protocol parsing/formatting: enum round-trips, message dispatch, the - full-state metadata/color objects, and the hand-rolled `client/time` formatter checked against - `snprintf`. + full-state metadata/color objects, the metadata text caps and kept fields, and the hand-rolled + `client/time` formatter checked against `snprintf`. - `test_json_arena.cpp`: `SendspinArenaAllocator` wipes every byte its own buffer gives up when a block is freed, shrunk or moved (the wipe of a freed heap fallback block cannot be read back and is a named gap); a parse released through `ParsedJsonMessage` leaves only its @@ -74,7 +74,9 @@ Each `test_*.cpp` file covers one unit of cross-platform logic: and swap. - `test_fixed_block_pool.cpp`: `FixedBlockPool` claims each block once until released, also under concurrent claims. -- `test_inbox.cpp`: `Inbox`/`InboxSlot` topic bits, event ring ordering, and slot binding. +- `test_inbox.cpp`: `Inbox`/`InboxSlot` topic bits, event ring ordering, slot binding, the + `GenerationSlot` generation rules, and the `ScheduledStateSlot` decision table, generation + rules and a drain that decides with the Inbox mutex free. - `test_shared_ring_buffer.cpp`: the host `SharedRingBuffer`: ring-order reclamation under out-of-order returns, no-split wrap placement, the full-ring acquire policies, completion order, and concurrent producers. @@ -111,19 +113,22 @@ Each `test_*.cpp` file covers one unit of cross-platform logic: pairing over the pairing PSK, `server/unpair`, pre-admission traffic, role dispatch by ownership, `client/leave` gating, the `client/state` role-object rules, the combined `['playback','pairing']` activate, the re-prove watchdog, the liveness tick and the arrival - stamp it reads, persistence written from the main loop, and controller command validation. + stamp it reads, persistence written from the main loop, controller command validation, and the + metadata fields the listener is handed. - `test_client_lifecycle.cpp`: `start()`/`stop()`/restart: goodbyes, clear callbacks delivered inside `stop()`, re-entrancy from callbacks, role start rollback, accepts refused at `stop()`, the command queue's refusals and the lifecycle requests it never refuses, the protocol task's next deadline, the teardown reorder guarantee for every role with main-loop state, stream - starts acknowledged by number, the high-performance grant, the time filter slot, and the - inbound ring's size. + starts acknowledged by number, the high-performance grant, the time filter slot, the + inbound ring's size, and the metadata role's scheduled-state slot, whose three text blocks + carry every state to the listener in order. - `test_client_teardown.cpp`: destroying a running client joins every threaded role. - `test_role_deactivation.cpp`: a later `server/activate` that removes a role: output stopped, buffers and state dropped, the roles it keeps left alone, and a removed role added back. - `test_allocations.cpp`: how many times the library calls `operator new` from a connection's accept to its admission, from `connect_to()` to admission, for a `client/state`, for a time burst, for a - controller `server/state`, for `group/update`, for the `stream/*` messages, for the sync task's + controller `server/state`, for `group/update`, for metadata `server/state` (a first state, a + second, a scheduled one and one without text), for the `stream/*` messages, for the sync task's first FLAC codec header, for a FLAC `stream/start` the player takes, for a `stream/start` section the connection's roles do not take, for the first spectrum frame, and through the record flush that follows each of a playback handoff to another paired server, a `server/unpair`, a diff --git a/tests/test_allocations.cpp b/tests/test_allocations.cpp index 49575a03..621a79a2 100644 --- a/tests/test_allocations.cpp +++ b/tests/test_allocations.cpp @@ -33,12 +33,15 @@ // print each window's allocations grouped by call site, with each frame's offset into the binary // for atos or addr2line; a count that differs prints its window. +#include "constants.h" #include "crypto/cpace.h" #include "crypto/pairing_code.h" #include "crypto/psk_wrap.h" #include "fake_persistence.h" #include "lifecycle_test_fixtures.h" #include "platform/base64.h" +#include "platform/time.h" +#include "protocol_messages.h" #include "sendspin/client.h" #include "sendspin/config.h" #include "wrap_test_helpers.h" @@ -429,6 +432,24 @@ class BurstCountingListener : public SendspinClientListener { int bursts{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. +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(); + ++this->updates; + } + + int updates{0}; + size_t title_size{0}; + size_t artwork_url_size{0}; + bool has_artist{false}; +}; + /// Closes the open window in on_stream_start() when asked to. The main loop calls it before it /// signals the sync task to start the stream, and the sync task decodes nothing before that /// signal, so the window holds the protocol task's and the main loop's work on a stream/start and @@ -527,6 +548,7 @@ constexpr size_t CLIENT_STATE_ALLOCATIONS = 0; constexpr size_t TIME_BURST_ALLOCATIONS = 0; constexpr size_t CONTROLLER_STATE_ALLOCATIONS = 0; constexpr size_t GROUP_UPDATE_ALLOCATIONS = 0; +constexpr size_t METADATA_STATE_ALLOCATIONS = 0; constexpr size_t STREAM_MESSAGES_ALLOCATIONS = 0; constexpr size_t PLAYER_FLAC_STREAM_START_ALLOCATIONS = 0; constexpr size_t SYNC_TASK_FIRST_FLAC_HEADER_ALLOCATIONS = 0; @@ -537,12 +559,14 @@ constexpr size_t UNTAKEN_STREAM_START_ALLOCATIONS = 0; // application roles and the player, artwork and visualizer objects of client/state are built: from // the fake server's connect through admission and the first client/state, then one client/state // publish, then one full burst of client/time and server/time exchanges, then the first -// controller server/state, then the first group/update messages, then an artwork and visualizer -// stream/start and a stream/end and a 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 and the client's group texts are reserved when they -// are built, so even the first fill on the main loop allocates nothing. +// controller server/state, then the first group/update messages, then the first metadata +// server/state messages, then an artwork and visualizer stream/start and a stream/end and a +// 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. 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. @@ -571,7 +595,8 @@ TEST(Allocations, AcceptToAdmissionAndPerMessagePaths) { player_config.audio_formats.push_back({SendspinCodecFormat::FLAC, 2, 48000, 16}); client.add_player(std::move(player_config)).set_listener(&player_listener); ControllerRole& controller = client.add_controller(); - client.add_metadata(); + TextSizeMetadataListener metadata_listener; + client.add_metadata().set_listener(&metadata_listener); ArtworkRoleConfig artwork_config; artwork_config.preferred_formats = { {SendspinImageSource::ALBUM, SendspinImageFormat::JPEG, 320, 320}, @@ -667,6 +692,53 @@ TEST(Allocations, AcceptToAdmissionAndPerMessagePaths) { [&] { return group.group_name == "Allocation Test Upstairs Bedroom Group"; }); expect_library_allocations(close_window(), "group/update", GROUP_UPDATE_ALLOCATIONS); + // 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 + // pending state until a later tick finds it due, then one with no text at all, each applied + // by the main loop before the next is sent. The server clock is the client's, both in this + // process. + const auto metadata_state = [](int64_t timestamp, const std::string& texts) { + return R"({"type":"server/state","payload":{"metadata":{"timestamp":)" + + std::to_string(timestamp) + texts + "}}}"; + }; + const std::string url = "http://allocation-test.local/artwork/" + std::string(1900, 'u'); + const std::string artwork_url = R"(,"artwork_url":")" + url + R"(")"; + open_window(); + { + TestOwned test_owned; + ASSERT_TRUE(server->send_app_json(metadata_state( + 1, R"(,"title":")" + std::string(MAX_METADATA_TEXT_BYTES + 1, 't') + + R"(","artist":"Allocation Test Artist Name","album_artist":)" + R"("Allocation Test Album Artist","album":"Allocation Test Album Title")" + + artwork_url))); + } + 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); + 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"; + ASSERT_TRUE(server->send_app_json( + metadata_state(platform_time_us() + 50 * static_cast(US_PER_MS), + R"(,"title":"Allocation Test Scheduled Track Title")"))); + } + pump_while_not(client, [&] { return metadata_listener.updates == 3; }); + { + TestOwned test_owned; + ASSERT_TRUE(server->send_app_json(metadata_state(3, ""))); + } + 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"; + const int starts_before_streams = visualizer_listener.stream_starts; // Closes on the client/state the trailing volume command publishes, which the protocol task diff --git a/tests/test_client_lifecycle.cpp b/tests/test_client_lifecycle.cpp index 6e563f64..11c73209 100644 --- a/tests/test_client_lifecycle.cpp +++ b/tests/test_client_lifecycle.cpp @@ -46,8 +46,10 @@ #include "sendspin/visualizer_role.h" #include "sync_task.h" #include "time_burst.h" // ms_until_due(), staged directly +#include "time_filter.h" #include "visualizer_role_impl.h" // Ring state after stop(); private access, see tests/CMakeLists.txt +#include #include #include @@ -56,6 +58,7 @@ #include #include +#include #include #include #include @@ -65,7 +68,9 @@ #include #include #include +#include #include +#include #include #include @@ -2450,10 +2455,22 @@ ControllerStatePayload controller_state_with(uint8_t value) { return state; } -ServerMetadataStateObject metadata_state_with(uint8_t value) { - ServerMetadataStateObject state; // Timestamp 0: due at once - state.year = value; - return state; +/// Hands the metadata role's slot a state whose year is `value`, due at once, stamped with +/// `generation`. +void hand_over_metadata_state(SendspinClient& c, uint8_t value, uint32_t generation) { + c.metadata_->impl_->event_state->slot.arrive( + [value](MetadataStateBuffer& place) { + place.payload = MetadataStatePayload{.year = value}; // Timestamp 0: due at once + }, + generation); +} + +/// The metadata role's handler for a state whose year is `value`, due at once. +void handle_metadata_state(SendspinClient& c, uint8_t value) { + JsonDocument doc; + ASSERT_FALSE(deserializeJson(doc, R"({"payload":{"metadata":{"timestamp":0,"year":)" + + std::to_string(value) + "}}}")); + c.metadata_->impl_->handle_server_state(doc.as()); } ServerColorStateObject color_state_with(uint8_t value) { @@ -2514,14 +2531,7 @@ const StateRoleAccess STATE_ROLES[] = { return c.controller_->impl_->controller_state.volume; }, true, false}, - {"metadata", - [](SendspinClient& c, uint8_t v) { - c.metadata_->impl_->handle_server_state(metadata_state_with(v)); - }, - [](SendspinClient& c, uint8_t v, uint32_t g) { - c.metadata_->impl_->event_state->slot.write( - PendingMetadataStates{.oldest = metadata_state_with(v)}, g); - }, + {"metadata", handle_metadata_state, hand_over_metadata_state, [](SendspinClient& c) { c.metadata_->impl_->cleanup(); }, [](SendspinClient& c) { c.metadata_->impl_->drain_events(); }, [](SendspinClient& c) { return c.metadata_->impl_->cleanup_generation.load(); }, @@ -2534,8 +2544,8 @@ const StateRoleAccess STATE_ROLES[] = { c.color_->impl_->handle_server_state(color_state_with(v)); }, [](SendspinClient& c, uint8_t v, uint32_t g) { - c.color_->impl_->event_state->slot.write( - PendingColorStates{.oldest = color_state_with(v)}, g); + c.color_->impl_->event_state->slot.arrive( + [v](ServerColorStateObject& place) { place = color_state_with(v); }, g); }, [](SendspinClient& c) { c.color_->impl_->cleanup(); }, [](SendspinClient& c) { c.color_->impl_->drain_events(); }, @@ -2834,7 +2844,7 @@ TEST(ClientLifecycle, StopFromInsideADrainCallbackIsSafe) { pump_until(client, [&] { return !client.is_started(); }); } else { client.controller_->impl_->handle_server_state(controller_state_with(11)); - client.metadata_->impl_->handle_server_state(metadata_state_with(5)); + handle_metadata_state(client, 5); client.loop(); } @@ -2850,6 +2860,218 @@ TEST(ClientLifecycle, StopFromInsideADrainCallbackIsSafe) { } } +// The metadata role's states wait in its ScheduledStateSlot and reach the listener through +// METADATA_TEXT_BLOCKS text blocks, one each for the slot's oldest and newest places and the main +// loop's current state, which a delivery swaps with the delivered place. So after every step the +// three name each block once, every state reaches the listener with the text it arrived with and +// in order, and the slot leaves pending the state roles/metadata/v1.md "Scheduled metadata +// updates" leaves waiting, with the main loop's copy of its deadline (pending_at()) set exactly +// while one is, once a drain has run. The rows run in order on one client whose protocol task the +// 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. +TEST(MetadataSlot, ThreeBlocksCarryEveryStateToTheListenerInOrder) { + struct TitleLog : MetadataRoleListener { + void on_metadata(const ServerMetadataStateObject& metadata) override { + this->titles.push_back(metadata.title.value_or("")); + if (this->on_title) { + std::exchange(this->on_title, nullptr)(this->titles.back()); + } + } + void on_metadata_clear() override { + ++this->clears; + if (this->on_clear) { + std::exchange(this->on_clear, nullptr)(); + } + } + std::vector titles; + int clears{0}; + std::function on_title; + std::function on_clear; + }; + enum class Step : uint8_t { + DUE, // A state due at once arrives + SCHEDULED, // A state an hour ahead arrives + STALE, // A due state stamped before the last teardown arrives + DRAIN, // A main-loop tick + DRAIN_ARRIVING, // The role's drain, a scheduled state arriving from the clear it + // catches up on, between its peek and its commit + DRAIN_FALLING_DUE, // A tick whose first callback brings every scheduled state due + DRAIN_STOPPING, // A tick whose first callback stops the client + CLEANUP, // The protocol task's teardown + START, + RESTART, + }; + struct Row { + const char* what; + Step step; + const char* title; + std::vector delivered; // Titles the step hands the listener, in order + const char* pending; // The pending state's title, nullptr when none + int clears; + }; + const Row rows[] = { + {"Control: a due state arrives", Step::DUE, "A", {}, nullptr, 0}, + {"Control: and is applied", Step::DRAIN, nullptr, {"A"}, nullptr, 0}, + {"a due state", Step::DUE, "B", {}, nullptr, 0}, + {"a scheduled one behind it", Step::SCHEDULED, "C", {}, nullptr, 0}, + {"a third replaces the second", Step::SCHEDULED, "D", {}, nullptr, 0}, + {"the drain applies the first and the third waits", Step::DRAIN, nullptr, {"B"}, "D", 0}, + {"Control: and goes on waiting", Step::DRAIN, nullptr, {}, "D", 0}, + {"a due state replaces the pending one", Step::DUE, "E", {}, nullptr, 0}, + {"a scheduled one behind it", Step::SCHEDULED, "F", {}, nullptr, 0}, + {"the drain applies it and the scheduled one waits", Step::DRAIN, nullptr, {"E"}, "F", 0}, + {"a due state replaces the pending one again", Step::DUE, "G", {}, nullptr, 0}, + {"a scheduled one behind it", Step::SCHEDULED, "H", {}, nullptr, 0}, + {"a teardown with both held", Step::CLEANUP, nullptr, {}, nullptr, 0}, + {"the drain delivers the clear alone", Step::DRAIN, nullptr, {}, nullptr, 1}, + {"a scheduled state after it", Step::SCHEDULED, "J", {}, nullptr, 1}, + {"waits", Step::DRAIN, nullptr, {}, "J", 1}, + {"a state stamped before the teardown", Step::STALE, "K", {}, "J", 1}, + {"is dropped, and the pending one goes on waiting", Step::DRAIN, nullptr, {}, "J", 1}, + {"a teardown", Step::CLEANUP, nullptr, {}, nullptr, 1}, + {"a due state after it", Step::DUE, "L", {}, nullptr, 1}, + {"a scheduled one arriving mid-drain is seen by a fresh peek", Step::DRAIN_ARRIVING, "M", + {"L"}, "M", 2}, + {"a due state replaces the pending one", Step::DUE, "N", {}, nullptr, 2}, + {"a scheduled one behind it", Step::SCHEDULED, "O", {}, nullptr, 2}, + {"falling due in the first callback, it follows in the same tick", Step::DRAIN_FALLING_DUE, + nullptr, {"N", "O"}, nullptr, 2}, + {"a due state", Step::DUE, "P", {}, nullptr, 2}, + {"a scheduled one behind it", Step::SCHEDULED, "Q", {}, nullptr, 2}, + {"stop() from the first callback leaves nothing behind", Step::DRAIN_STOPPING, nullptr, + {"P"}, nullptr, 3}, + {"a start", Step::START, nullptr, {}, nullptr, 3}, + {"a scheduled state", Step::SCHEDULED, "R", {}, nullptr, 3}, + {"waits", Step::DRAIN, nullptr, {}, "R", 3}, + {"a restart drops it", Step::RESTART, nullptr, {}, nullptr, 4}, + {"a due state after it", Step::DUE, "S", {}, nullptr, 4}, + {"is applied", Step::DRAIN, nullptr, {"S"}, nullptr, 4}, + }; + + TitleLog log; + TestNetworkProvider network; + SendspinClient client(make_config(0)); + client.set_network_provider(&network); + client.add_metadata().set_listener(&log); + MetadataRole::Impl& impl = *client.metadata_->impl_; + // The primary clock, in sync with the client's; `offset` moves the server's clock ahead. + const auto set_clock = [&client](double offset) { + ConnectionManager& manager = *client.connection_manager_; + std::lock_guard lock(manager.published_mutex_); + manager.published_.primary_slot = 0; + manager.published_.clocks[0] = + TimeTransform{.offset = offset, .last_update = platform_time_us(), .synced = true}; + }; + const auto start = [&client, &set_clock] { + ASSERT_TRUE(client.start()); + client.protocol_task_->stop(); + set_clock(0.0); + }; + start(); + const int64_t hour = 3600 * static_cast(US_PER_SECOND); + const int64_t scheduled_at = platform_time_us() + hour; + const auto state_json = [](const char* title, int64_t timestamp) { + return R"({"payload":{"metadata":{"timestamp":)" + std::to_string(timestamp) + + R"(,"title":")" + title + R"("}}})"; + }; + // The protocol task's handler, for a state carrying `title`. + const auto arrive = [&impl, &state_json](const char* title, int64_t timestamp) { + JsonDocument doc; + ASSERT_FALSE(deserializeJson(doc, state_json(title, timestamp))); + ASSERT_EQ(impl.handle_server_state(doc.as()), MetadataStateOutcome::PARSED); + }; + uint32_t before_teardown = 0; + std::vector delivered; + + for (const Row& row : rows) { + SCOPED_TRACE(row.what); + switch (row.step) { + case Step::DUE: + arrive(row.title, 1); + break; + case Step::SCHEDULED: + arrive(row.title, scheduled_at); + break; + case Step::STALE: { + JsonDocument doc; + ASSERT_FALSE(deserializeJson(doc, state_json(row.title, 1))); + MetadataStateView parsed; + ASSERT_EQ(process_server_state_metadata(doc.as(), impl.layout.kept, + &parsed), + MetadataStateOutcome::PARSED); + impl.event_state->slot.arrive( + [&impl, &parsed](MetadataStateBuffer& place) { + impl.fill_place(place, parsed); + }, + before_teardown); + break; + } + case Step::DRAIN: + client.loop(); + break; + case Step::DRAIN_ARRIVING: + log.on_clear = [&arrive, &row, scheduled_at] { arrive(row.title, scheduled_at); }; + impl.drain_events(); + break; + case Step::DRAIN_FALLING_DUE: + log.on_title = [&set_clock](const std::string&) { + set_clock(2.0 * static_cast(hour)); + }; + client.loop(); + set_clock(0.0); + break; + case Step::DRAIN_STOPPING: + log.on_title = [&client](const std::string&) { + client.stop(); + }; + client.loop(); + break; + case Step::CLEANUP: + before_teardown = impl.cleanup_generation.load(); + impl.cleanup(); + break; + case Step::START: + start(); + break; + case Step::RESTART: + client.stop(); + start(); + break; + } + + delivered.insert(delivered.end(), row.delivered.begin(), row.delivered.end()); + EXPECT_EQ(log.titles, delivered); + EXPECT_EQ(log.clears, row.clears); + + std::array blocks{}; + std::optional pending; + impl.event_state->slot.slot_.read([&](const auto& places) { + blocks = {places.oldest.state.block, places.newest.state.block, impl.current.block}; + if (places.pending) { + pending = std::string(impl.text_of(places.oldest.state, MetadataField::TITLE) + .value_or("(no title)")); + } + return 0; + }); + std::sort(blocks.begin(), blocks.end()); + static_assert(METADATA_TEXT_BLOCKS == 3); + EXPECT_EQ(blocks, (std::array{0, 1, 2})) << "a text block has two places"; + EXPECT_EQ(pending, row.pending != nullptr ? std::optional(row.pending) + : std::nullopt); + // The main loop's copy of the deadline is exact once a drain or a stop() has run. + if (row.step != Step::DUE && row.step != Step::SCHEDULED && row.step != Step::STALE && + row.step != Step::CLEANUP) { + EXPECT_EQ(impl.event_state->slot.pending_at().has_value(), row.pending != nullptr); + } + } + client.stop(); +} + /// Counts the pairing prompts and their dismissals. Main loop only. class PairingPromptLog : public SendspinClientListener { public: diff --git a/tests/test_encrypted_lifecycle.cpp b/tests/test_encrypted_lifecycle.cpp index 0e730525..939d94b9 100644 --- a/tests/test_encrypted_lifecycle.cpp +++ b/tests/test_encrypted_lifecycle.cpp @@ -34,6 +34,7 @@ #include "inbox.h" #include "lifecycle_test_fixtures.h" #include "log_capture.h" +#include "metadata_role_impl.h" #include "platform/crypto.h" #include "platform/json_arena.h" #include "platform/logging.h" @@ -1924,7 +1925,8 @@ class DispatchTestClient { explicit DispatchTestClient( const char* name, size_t json_arena_size = SendspinClientConfig::DEFAULT_JSON_ARENA_SIZE, - PlayerRoleListener* player_listener = nullptr) { + PlayerRoleListener* player_listener = nullptr, + MetadataRoleConfig metadata_config = {}) { SendspinClientConfig config; config.name = name; // Port 0: an ephemeral listener nothing connects to; every message is delivered directly. @@ -1932,7 +1934,7 @@ class DispatchTestClient { config.json_arena_size = json_arena_size; this->client_storage = std::make_unique(std::move(config)); this->client_storage->set_network_provider(&this->network); - this->client_storage->add_metadata().set_listener(&this->listener); + this->client_storage->add_metadata(metadata_config).set_listener(&this->listener); // The other roles a delivered message can be dispatched to, so delivery runs their real // handlers. Without a player listener the sync task never starts and the player's stream // handlers find no item list to hand their codec header to. @@ -2170,10 +2172,72 @@ TEST(EncryptedLifecycle, MetadataStateReplacesRatherThanMerges) { EXPECT_FALSE(bundle.listener.last_had_progress) << "an omitted progress clears the position"; } +// 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. +TEST(EncryptedLifecycle, MetadataKeepsTheConfiguredFieldsCutToTheirCaps) { + struct StateListener : MetadataRoleListener { + std::vector states; + void on_metadata(const ServerMetadataStateObject& metadata) override { + this->states.push_back(metadata); + } + }; + StateListener listener; // Outlives the client + DispatchTestClient bundle( + "Metadata Fields Test Client", SendspinClientConfig::DEFAULT_JSON_ARENA_SIZE, nullptr, + MetadataRoleConfig{.artist = false, .album_artist = false, .album = false}); + bundle.client_ref().metadata()->set_listener(&listener); + DispatchTestConnection& conn = bundle.connection(); + bundle.admit(conn); + + const auto state = [](int timestamp, const std::string& title, const std::string& url) { + return R"({"type":"server/state","payload":{"metadata":{"timestamp":)" + + std::to_string(timestamp) + R"(,"title":")" + title + + R"(","artist":"Band","album_artist":"Various","album":"Record",)" + R"("artwork_url":")" + + url + R"(","year":1999}}})"; + }; + // 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"; + const std::string long_url = "http://example.com/" + std::string(MAX_METADATA_URL_BYTES, 'u'); + // Control: within their caps, the kept fields arrive as sent. + bundle.deliver(conn, state(1, "Song", "http://example.com/art.jpg")); + bundle.pump(); + bundle.deliver(conn, state(2, straddling, long_url)); + bundle.pump(); + bundle.deliver(conn, state(3, std::string(MAX_METADATA_TEXT_BYTES + 1, 'b'), "http://x")); + bundle.pump(); + + struct Expected { + std::string title; + std::string artwork_url; + }; + const Expected expected[] = { + {"Song", "http://example.com/art.jpg"}, + {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"}, + }; + ASSERT_EQ(listener.states.size(), std::size(expected)); + for (size_t i = 0; i < std::size(expected); ++i) { + SCOPED_TRACE(i); + const ServerMetadataStateObject& got = listener.states[i]; + 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"; + } +} + // messaging.md "server/state": the first state a server sends for a role carries a past or // present timestamp, so the client is brought up to date, and a scheduled update may follow it // immediately. Both land before the main loop runs, and the state describing what is playing now -// must still be applied rather than skipped in favor of the one timed to the next track. +// must still be applied rather than skipped in favor of the one timed to the next track, which the +// role holds until its moment. That the pending state is the scheduled one is read from the role's +// slot: no observable shows it before its deadline, which is kept out of the test's reach. TEST(EncryptedLifecycle, ImmediateMetadataSurvivesAScheduledStateInTheSameTick) { RecordingMetadataListener listener; @@ -2212,6 +2276,18 @@ TEST(EncryptedLifecycle, ImmediateMetadataSurvivesAScheduledStateInTheSameTick) << "the state describing the current track was dropped for the scheduled one"; EXPECT_EQ(listener.last_title, "Now Playing"); + // The scheduled one waits in the slot rather than being dropped by the drain that applied the + // other. Read under the Inbox mutex, which the protocol task writes the slot under. + const MetadataRole::Impl& impl = *client.metadata()->impl_; + const std::optional pending = + impl.event_state->slot.slot_.read([&impl](const auto& places) { + return places.pending ? std::optional( + impl.text_of(places.oldest.state, MetadataField::TITLE) + .value_or("(no title)")) + : std::nullopt; + }); + EXPECT_EQ(pending, "Next Track"); + client.disconnect(SendspinGoodbyeReason::SHUTDOWN); pump_for(client, 100); } diff --git a/tests/test_inbox.cpp b/tests/test_inbox.cpp index 2887c176..3d9e4388 100644 --- a/tests/test_inbox.cpp +++ b/tests/test_inbox.cpp @@ -14,16 +14,20 @@ /// @file test_inbox.cpp /// @brief Tests for the Inbox/InboxSlot primitives: slot write/take/merge/exchange/edit -/// semantics, the generation-stamped slot and the teardown tracker, event ring FIFO and overflow -/// behavior, and a concurrent producer/consumer smoke test +/// semantics, the generation-stamped slot, the scheduled-state slot and its drain, and the +/// teardown tracker, event ring FIFO and overflow behavior, and a concurrent producer/consumer +/// smoke test #include "inbox.h" +#include "scheduled_state_slot.h" #include "teardown_tracker.h" #include #include #include +#include +#include #include #include @@ -240,6 +244,166 @@ TEST(GenerationSlot, KeepsTheNewestGenerationsContent) { } } +namespace { + +/// A ScheduledStateSlot state: only its timestamp +struct TimedState { + int64_t timestamp{0}; +}; + +int64_t scheduled_time(const TimedState& state) { + return state.timestamp; +} + +using TimedSlot = ScheduledStateSlot; + +/// Hands `slot` a state applying at `timestamp`, admitted under `generation`. +void arrive_at(TimedSlot& slot, int64_t timestamp, uint32_t generation) { + slot.arrive([timestamp](TimedState& place) { place.timestamp = timestamp; }, generation); +} + +/// A role on a ScheduledStateSlot whose deadline check counts the times it found the Inbox mutex +/// held, probed from another thread: a real deadline reads the client clock behind a lock of its +/// own, so it must never run under the Inbox mutex. A state is due when its timestamp is at most +/// `now`, and the drain records each delivered state's timestamp. +struct ProbingRole : RoleTeardown { + explicit ProbingRole(Inbox& inbox) : inbox(inbox) { + this->slot.bind(inbox, INBOX_TOPIC_COLOR); + } + bool state_is_due(int64_t timestamp) const { + bool locked = false; + std::thread([this, &locked] { + locked = !this->inbox.mutex_.try_lock(); + if (!locked) { + this->inbox.mutex_.unlock(); + } + }).join(); + this->checks_under_lock += locked ? 1 : 0; + return timestamp <= this->now; + } + void complete_teardown() { + this->slot.forget_pending(); + } + void drain() { + drain_scheduled_state( + *this, this->slot, + [this](const TimedState& place) { this->delivered.push_back(place.timestamp); }, [] {}, + "test", "state"); + } + + Inbox& inbox; + TimedSlot slot; + std::vector delivered; + int64_t now{0}; + mutable int checks_under_lock{0}; +}; + +} // namespace + +// roles/metadata/v1.md "Scheduled metadata updates" and roles/color/v1.md "Scheduled color +// updates", as ScheduledStateSlot::decide() applies them to the states a drain finds: a due state +// is applied and a future one waits, replacing any older one, and two due states collapse to the +// newer, since the older would be superseded within the tick. A state at timestamp 1 is due, one +// at 2 is not. +TEST(ScheduledStateSlot, DecidesWhichStateIsAppliedAndWhichWaits) { + using Place = TimedSlot::Place; + struct Row { + const char* name; + std::optional oldest; + std::optional newest; + Place deliver; + Place pending; + }; + const Row rows[] = { + {"Control: nothing held", std::nullopt, std::nullopt, Place::NONE, Place::NONE}, + {"Control: a due state is applied", 1, std::nullopt, Place::OLDEST, Place::NONE}, + {"a future state waits", 2, std::nullopt, Place::NONE, Place::OLDEST}, + {"a due state is applied and a future one behind it waits", 1, 2, Place::OLDEST, + Place::NEWEST}, + {"two due states collapse to the newer", 1, 1, Place::NEWEST, Place::NONE}, + {"a due newer state discards a future older one", 2, 1, Place::NEWEST, Place::NONE}, + {"a future newer state replaces a future older one", 2, 2, Place::NONE, Place::NEWEST}, + }; + for (const Row& row : rows) { + SCOPED_TRACE(row.name); + const TimedSlot::Peek peek{.oldest = row.oldest, .newest = row.newest}; + const TimedSlot::Decision decision = + TimedSlot::decide(peek, [](int64_t timestamp) { return timestamp == 1; }); + EXPECT_EQ(decision.deliver, row.deliver); + EXPECT_EQ(decision.pending, row.pending); + } +} + +// ScheduledStateSlot::arrive() keeps the newest generation's states, as GenerationSlot does: a +// state from a later generation discards the held ones, one from an earlier generation (a +// producer that lost a race with a teardown) is dropped, and a state from the same generation is +// kept beside the one before it. +TEST(ScheduledStateSlot, KeepsTheNewestGenerationsStates) { + struct Row { + const char* name; + uint32_t first; + uint32_t second; + std::optional oldest; + std::optional newest; + uint32_t generation; + }; + const Row rows[] = { + {"Control: the same generation keeps both", 3, 3, 1, 2, 3}, + {"a later generation discards the held state", 3, 4, 2, std::nullopt, 4}, + {"an earlier generation is dropped", 4, 3, 1, std::nullopt, 4}, + {"ordered across the counter's wrap", UINT32_MAX, 0, 2, std::nullopt, 0}, + }; + for (const Row& row : rows) { + SCOPED_TRACE(row.name); + Inbox inbox; + TimedSlot slot; + slot.bind(inbox, INBOX_TOPIC_COLOR); + arrive_at(slot, 1, row.first); + arrive_at(slot, 2, row.second); + + const TimedSlot::Peek peek = slot.peek(); + EXPECT_EQ(peek.oldest, row.oldest); + EXPECT_EQ(peek.newest, row.newest); + EXPECT_EQ(peek.generation, row.generation); + } +} + +// A drain decides on the states it peeked with no lock held: the deadline check reads the client +// clock behind a lock of its own, and the Inbox mutex is a leaf (inbox.h), so the drain copies the +// timestamps out under it and asks each deadline after releasing it. Each step's checks probe the +// mutex from another thread. A committed drain and a reset each leave the slot clean, so a state +// left pending waits on pending_at() rather than a topic bit that would drain every tick. +TEST(ScheduledStateSlot, ADrainDecidesWithTheInboxMutexFree) { + Inbox inbox; + ProbingRole role(inbox); + + { + std::lock_guard lock(inbox.mutex_); + role.state_is_due(0); + } + ASSERT_EQ(role.checks_under_lock, 1) << "Control: the probe finds a held mutex"; + role.checks_under_lock = 0; + + role.now = 5; + arrive_at(role.slot, 1, 0); + arrive_at(role.slot, 10, 0); + role.drain(); + EXPECT_EQ(role.delivered, (std::vector{1})); + EXPECT_EQ(role.slot.pending_at(), 10); + EXPECT_EQ(inbox.poll() & INBOX_TOPIC_COLOR, 0U) << "the commit left the slot dirty"; + + role.now = 10; + role.drain(); + EXPECT_EQ(role.delivered, (std::vector{1, 10})); + EXPECT_EQ(role.slot.pending_at(), std::nullopt); + EXPECT_EQ(role.checks_under_lock, 0) << "a deadline was checked under the Inbox mutex"; + + arrive_at(role.slot, 20, 0); + ASSERT_NE(inbox.poll() & INBOX_TOPIC_COLOR, 0U) << "Control: an arrival marks the slot dirty"; + role.slot.reset(); + EXPECT_EQ(inbox.poll() & INBOX_TOPIC_COLOR, 0U) << "the reset left the slot dirty"; +} + // TeardownTracker::advance() reports a teardown generation pending exactly once, so a role's // main-loop half runs once per teardown however many paths (stamped events, slot payloads) carry // that generation, and never for a generation a later catch-up already passed. diff --git a/tests/test_protocol.cpp b/tests/test_protocol.cpp index b18a74d5..a98519c4 100644 --- a/tests/test_protocol.cpp +++ b/tests/test_protocol.cpp @@ -61,6 +61,42 @@ bool parse(const std::string& json, JsonDocument& doc, JsonObject& root) { const EnumSet STREAM_START_ROLES{SendspinRole::PLAYER, SendspinRole::ARTWORK, SendspinRole::VISUALIZER}; +const EnumSet ALL_METADATA_FIELDS{ + MetadataField::TITLE, MetadataField::ARTIST, MetadataField::ALBUM_ARTIST, MetadataField::ALBUM, + MetadataField::ARTWORK_URL}; + +// A metadata state parsed for the fields it keeps, its text read back from the document it views. +struct ParsedMetadata { + explicit ParsedMetadata(EnumSet kept = ALL_METADATA_FIELDS) : kept(kept) {} + + // Parses `root`'s metadata object into this state. + MetadataStateOutcome parse(JsonObject root) { + return process_server_state_metadata(root, this->kept, &this->view); + } + + // The text the state holds for `field`. + std::optional text(MetadataField field) const { + const auto i = static_cast(field); + if (!this->view.payload.text_sizes[i].has_value()) { + return std::nullopt; + } + return std::string(this->view.text[i], *this->view.payload.text_sizes[i]); + } + + // Holds `text` for `field`, as an earlier parse would have left it. + void hold(MetadataField field, const std::string& text) { + const auto i = static_cast(field); + this->held[i] = text; + this->view.text[i] = this->held[i].data(); + this->view.payload.text_sizes[i] = static_cast(text.size()); + } + + EnumSet kept; + MetadataStateView view; + std::array held; + MetadataStatePayload& payload{view.payload}; +}; + } // namespace // ============================================================================ @@ -285,42 +321,154 @@ TEST(Protocol, MetadataObjectIsParsedIntoState) { R"("playback_speed":1000}}}})", doc, root)); - ServerMetadataStateObject metadata; - ASSERT_TRUE(process_server_state_metadata(root, &metadata)); + ParsedMetadata metadata; + ASSERT_EQ(metadata.parse(root), MetadataStateOutcome::PARSED); - EXPECT_EQ(metadata.timestamp, 123); - ASSERT_TRUE(metadata.title.has_value()); - EXPECT_EQ(metadata.title.value(), "Song"); - ASSERT_TRUE(metadata.artist.has_value()); - EXPECT_EQ(metadata.artist.value(), "Band"); - EXPECT_EQ(metadata.year.value_or(0), 1999); - EXPECT_EQ(metadata.track.value_or(0), 4); - ASSERT_TRUE(metadata.progress.has_value()); - EXPECT_EQ(metadata.progress->track_progress, 1000u); - EXPECT_EQ(metadata.progress->track_duration, 240000u); - EXPECT_EQ(metadata.progress->playback_speed, 1000u); + EXPECT_EQ(metadata.payload.timestamp, 123); + EXPECT_EQ(metadata.text(MetadataField::TITLE), "Song"); + EXPECT_EQ(metadata.text(MetadataField::ARTIST), "Band"); + EXPECT_EQ(metadata.payload.year.value_or(0), 1999); + EXPECT_EQ(metadata.payload.track.value_or(0), 4); + ASSERT_TRUE(metadata.payload.progress.has_value()); + EXPECT_EQ(metadata.payload.progress->track_progress, 1000u); + EXPECT_EQ(metadata.payload.progress->track_duration, 240000u); + EXPECT_EQ(metadata.payload.progress->playback_speed, 1000u); } // A field the object omits has no value, whatever the target struct held before. TEST(Protocol, MetadataObjectReplacesEveryFieldItOmits) { - ServerMetadataStateObject metadata; - metadata.title = "Old title"; - metadata.artist = "Old artist"; - metadata.progress = MetadataProgressObject{5000, 240000, 1000}; + ParsedMetadata metadata; + metadata.hold(MetadataField::TITLE, "Old title"); + metadata.hold(MetadataField::ARTIST, "Old artist"); + metadata.payload.progress = MetadataProgressObject{5000, 240000, 1000}; JsonDocument doc; JsonObject root; ASSERT_TRUE(parse(R"({"type":"server/state","payload":{"metadata":)" R"({"timestamp":200,"artist":"New artist"}}})", doc, root)); - ASSERT_TRUE(process_server_state_metadata(root, &metadata)); + ASSERT_EQ(metadata.parse(root), MetadataStateOutcome::PARSED); - EXPECT_EQ(metadata.timestamp, 200); - ASSERT_TRUE(metadata.artist.has_value()); - EXPECT_EQ(metadata.artist.value(), "New artist"); - EXPECT_FALSE(metadata.title.has_value()); + EXPECT_EQ(metadata.payload.timestamp, 200); + EXPECT_EQ(metadata.text(MetadataField::ARTIST), "New artist"); + EXPECT_EQ(metadata.text(MetadataField::TITLE), std::nullopt); // roles/metadata/v1.md "server/state metadata object": omitting progress clears the position. - EXPECT_FALSE(metadata.progress.has_value()); + EXPECT_FALSE(metadata.payload.progress.has_value()); +} + +// roles/metadata/v1.md "server/state metadata object": each text field is at most 255 bytes in +// UTF-8, artwork_url 2048. While not strict, a longer one is cut to its cap at a character +// boundary, so a character straddling the cap is dropped whole; while strict it is a protocol +// error (messaging.md "Out-of-range values"). A value that is not a string is no value in either +// mode. Every row runs at both caps. +TEST(Protocol, MetadataTextIsKeptCutOrRejected) { + struct Row { + const char* what; + // The value as JSON, built from the cap: `fill` ASCII bytes followed by `tail`, quoted, + // or `json` as is when set + int fill_from_cap; + const char* tail; + const char* json; + MetadataTextOutcome lenient; + int kept_from_cap; // Bytes kept while not strict, relative to the cap + MetadataTextOutcome strict; + }; + const Row rows[] = { + {"Control: at the cap", 0, "", nullptr, MetadataTextOutcome::KEPT, 0, + MetadataTextOutcome::KEPT}, + {"Control: a 2-byte character ending at the cap", -2, "\xC3\xA9", nullptr, + MetadataTextOutcome::KEPT, 0, MetadataTextOutcome::KEPT}, + {"one byte over", 1, "", nullptr, MetadataTextOutcome::CUT, 0, + MetadataTextOutcome::REJECTED}, + {"a 2-byte character straddling the cap", -1, "\xC3\xA9", nullptr, + MetadataTextOutcome::CUT, -1, MetadataTextOutcome::REJECTED}, + {"a 3-byte character straddling the cap", -2, "\xE2\x82\xAC", nullptr, + MetadataTextOutcome::CUT, -2, MetadataTextOutcome::REJECTED}, + {"a 4-byte character straddling the cap", -1, "\xF0\x9F\x98\x80", nullptr, + MetadataTextOutcome::CUT, -1, MetadataTextOutcome::REJECTED}, + {"null", 0, "", "null", MetadataTextOutcome::ABSENT, 0, MetadataTextOutcome::ABSENT}, + {"a number", 0, "", "7", MetadataTextOutcome::ABSENT, 0, MetadataTextOutcome::ABSENT}, + }; + for (const size_t cap : {MAX_METADATA_TEXT_BYTES, MAX_METADATA_URL_BYTES}) { + for (const Row& row : rows) { + SCOPED_TRACE(std::string(row.what) + " at " + std::to_string(cap)); + const std::string sent = + std::string(cap + row.fill_from_cap, 'a') + std::string(row.tail); + const std::string value = + row.json != nullptr ? std::string(row.json) : R"(")" + sent + R"(")"; + JsonDocument doc; + JsonObject root; + ASSERT_TRUE(parse(R"({"field":)" + value + "}", doc, root)); + + std::string_view lenient_text; + ASSERT_EQ(metadata_text_from_wire(root["field"], "field", cap, false, &lenient_text), + row.lenient); + if (row.lenient != MetadataTextOutcome::ABSENT) { + EXPECT_EQ(lenient_text, sent.substr(0, cap + row.kept_from_cap)); + } + + std::string_view strict_text; + ASSERT_EQ(metadata_text_from_wire(root["field"], "field", cap, true, &strict_text), + row.strict); + if (row.strict == MetadataTextOutcome::KEPT) { + EXPECT_EQ(strict_text, sent); + } + } + } +} + +// MetadataRoleConfig: a field the role does not keep is dropped as it is parsed, so the +// state carries no value for it and its text is never viewed, while every kept field is. An +// unkept field over its cap still reports the cut, since the cap holds whether or not the client +// keeps it; a kept one is cut in place. +TEST(Protocol, MetadataFieldsNotKeptAreDropped) { + struct Row { + const char* what; + EnumSet kept; + std::string title; + MetadataStateOutcome outcome; + }; + const std::string long_title(MAX_METADATA_TEXT_BYTES + 1, 't'); + const Row rows[] = { + {"Control: every field kept", ALL_METADATA_FIELDS, "Song", MetadataStateOutcome::PARSED}, + {"title and artwork_url kept", {MetadataField::TITLE, MetadataField::ARTWORK_URL}, "Song", + MetadataStateOutcome::PARSED}, + {"no field kept", {}, "Song", MetadataStateOutcome::PARSED}, + {"a kept title over its cap", ALL_METADATA_FIELDS, long_title, MetadataStateOutcome::CUT}, + {"an unkept title over its cap", {MetadataField::ARTIST}, long_title, + MetadataStateOutcome::CUT}, + }; + const std::pair sent[] = { + {MetadataField::ARTIST, "Band"}, + {MetadataField::ALBUM_ARTIST, "Various"}, + {MetadataField::ALBUM, "Record"}, + {MetadataField::ARTWORK_URL, "http://example.com/art.jpg"}, + }; + for (const Row& row : rows) { + SCOPED_TRACE(row.what); + std::string json = R"({"type":"server/state","payload":{"metadata":{"timestamp":1,)"; + json += R"("title":")" + row.title + R"(")"; + for (const auto& [field, text] : sent) { + json += std::string(R"(,")") + to_cstr(field) + R"(":")" + text + R"(")"; + } + json += "}}}"; + JsonDocument doc; + JsonObject root; + ASSERT_TRUE(parse(json, doc, root)); + + ParsedMetadata metadata(row.kept); + ASSERT_EQ(metadata.parse(root), row.outcome); + const auto expected = [&row](MetadataField field, const std::string& text) { + return row.kept.contains(field) + ? std::optional(text.substr(0, metadata_field_cap(field))) + : std::nullopt; + }; + EXPECT_EQ(metadata.text(MetadataField::TITLE), expected(MetadataField::TITLE, row.title)); + for (const auto& [field, text] : sent) { + SCOPED_TRACE(to_cstr(field)); + EXPECT_EQ(metadata.text(field), expected(field, text)); + } + } } // roles/metadata/v1.md and roles/color/v1.md "server/state ... object": timestamp is a required @@ -348,11 +496,11 @@ TEST(Protocol, StateObjectTimestampMustBeAnInteger) { ASSERT_TRUE(parse(std::string(R"({"type":"server/state","payload":{"metadata":{)") + row.timestamp_field + R"("title":"New"}}})", doc, root)); - ServerMetadataStateObject metadata; - metadata.title = "Kept"; - EXPECT_EQ(process_server_state_metadata(root, &metadata), row.accepted); - ASSERT_TRUE(metadata.title.has_value()); - EXPECT_EQ(metadata.title.value(), row.accepted ? "New" : "Kept"); + ParsedMetadata metadata; + metadata.hold(MetadataField::TITLE, "Kept"); + EXPECT_EQ(metadata.parse(root), row.accepted ? MetadataStateOutcome::PARSED + : MetadataStateOutcome::IGNORED); + EXPECT_EQ(metadata.text(MetadataField::TITLE), row.accepted ? "New" : "Kept"); ASSERT_TRUE(parse(std::string(R"({"type":"server/state","payload":{"color":{)") + row.timestamp_field + R"("primary":[10,20,30]}}})", diff --git a/tests/test_role_deactivation.cpp b/tests/test_role_deactivation.cpp index 2bbf5db7..58e58550 100644 --- a/tests/test_role_deactivation.cpp +++ b/tests/test_role_deactivation.cpp @@ -420,8 +420,8 @@ TEST(RoleDeactivation, RemovedStateRolesDiscardCurrentAndScheduledState) { ASSERT_TRUE(server->send_app_json(metadata_state_json(scheduled_ts, "Up Next"))); ASSERT_TRUE(server->send_app_json(color_state_json(scheduled_ts))); pump_until(client, [&] { - return client.metadata()->impl_->held_state.has_value() && - client.color()->impl_->held_state.has_value(); + return client.metadata()->impl_->event_state->slot.pending_at().has_value() && + client.color()->impl_->event_state->slot.pending_at().has_value(); }); ASSERT_EQ(metadata_listener.updates, 1) << "a scheduled update was applied early"; @@ -431,9 +431,13 @@ TEST(RoleDeactivation, RemovedStateRolesDiscardCurrentAndScheduledState) { controller_listener.clears == 1; }); - EXPECT_FALSE(client.metadata()->impl_->held_state.has_value()) + // The slot's states and the main loop's copy of the pending deadline alike. + const auto holds_scheduled = [](const auto& slot) { + return slot.peek().oldest.has_value() || slot.pending_at().has_value(); + }; + EXPECT_FALSE(holds_scheduled(client.metadata()->impl_->event_state->slot)) << "the removed metadata role kept its scheduled update"; - EXPECT_FALSE(client.color()->impl_->held_state.has_value()) + EXPECT_FALSE(holds_scheduled(client.color()->impl_->event_state->slot)) << "the removed color role kept its scheduled update"; EXPECT_EQ(client.metadata()->get_track_duration_ms(), 0U); EXPECT_EQ(client.metadata()->get_track_progress_ms(), 0U); @@ -450,25 +454,24 @@ TEST(RoleDeactivation, RemovedStateRolesDiscardCurrentAndScheduledState) { stream_audio_until(client, *server, player_listener, 1); // A state stamped with the pre-removal generation, the payload a drain takes on the far side - // of the removal, is discarded at the drain: writing the role's slot with that stamp forces - // the interleaving rather than racing for it. - ServerMetadataStateObject overtaken; - overtaken.timestamp = 1; - overtaken.title = "Overtaken By The Removal"; - client.metadata()->impl_->event_state->slot.write( - PendingMetadataStates{.oldest = std::move(overtaken)}, metadata_generation); + // of the removal, is discarded at the drain: handing the role's slot a state with that stamp + // forces the interleaving rather than racing for it. + MetadataRole::Impl& metadata_impl = *client.metadata()->impl_; + const auto hand_over_stamped = [&metadata_impl](uint32_t generation) { + metadata_impl.event_state->slot.arrive( + [](MetadataStateBuffer& place) { + place.payload = MetadataStatePayload{.timestamp = 1}; + }, + generation); + }; + hand_over_stamped(metadata_generation); pump_for(client, SETTLE_MS); EXPECT_EQ(metadata_listener.updates, 1) << "a payload stamped with the pre-removal generation was applied"; - // Control: the same write carrying the generation the role reports now is applied, so the + // Control: the same hand-over carrying the generation the role reports now is applied, so the // refusal above came from the stale generation and nothing else. - ServerMetadataStateObject current; - current.timestamp = 1; - current.title = "Current Generation"; - client.metadata()->impl_->event_state->slot.write( - PendingMetadataStates{.oldest = std::move(current)}, - client.metadata()->impl_->cleanup_generation.load(std::memory_order_acquire)); + hand_over_stamped(metadata_impl.cleanup_generation.load(std::memory_order_acquire)); pump_until(client, [&] { return metadata_listener.updates == 2; }); // roles/controller/v1.md "client/command controller object": the removal discarded the offered diff --git a/tools/stack_usage/README.md b/tools/stack_usage/README.md index 86d846f6..2e9e2d1c 100644 --- a/tools/stack_usage/README.md +++ b/tools/stack_usage/README.md @@ -137,7 +137,7 @@ Measured on xtensa-esp32 with ESP-IDF 5.5 and GCC 14.2, at `-Os` (ESPHome's defa | --- | --- | --- | --- | --- | --- | --- | | httpd | `httpd_thread` | esp_http_server's own WebSocket upgrade response (544 to 576 bytes) | 4,032 | 3,968 | 4,608 (`DEFAULT_HTTPD_STACK_SIZE`) | shared ESP-IDF tail | | websocket client | `esp_websocket_client_task` | library event handler (3,520 / 3,600) closing on a stalled protocol task via `fail_inbound()` through the inbound transport's close | 3,904 | 3,968 | 4,608 (`DEFAULT_WEBSOCKET_STACK_SIZE`) | shared ESP-IDF tail | -| protocol task | task entry | as bounded: an esp_websocket_client send error into the event handler, `handle_data()`, `fail_inbound()` and `httpd_sess_trigger_close` (unreachable); reachable: a pairing confirm dropping an outbound connection whose goodbye send fails (5,920 at -Os, 6,144 at -Og) | 6,544 | 6,688 | 7,168 (`DEFAULT_PROTOCOL_TASK_STACK_SIZE`) | shared ESP-IDF tail | +| protocol task | task entry | as bounded: an esp_websocket_client send error into the event handler, `handle_data()`, `fail_inbound()` and `httpd_sess_trigger_close` (unreachable); reachable: a pairing confirm dropping an outbound connection whose goodbye send fails (5,888 at -Os, 6,080 at -Og) | 6,512 | 6,624 | 7,168 (`DEFAULT_PROTOCOL_TASK_STACK_SIZE`) | shared ESP-IDF tail | | source, PCM | `thread_entry()` | `begin_chunk()`'s outbound acquire | 1,104 | 1,168 | 2,048 (`DEFAULT_SOURCE_TASK_STACK_SIZE`) | FreeRTOS critical-section assert | | source, Opus | `thread_entry()` | the Opus encode, `quant_partition()` charged one pass | 5,152 | 5,168 | 6,656 (`DEFAULT_OPUS_SOURCE_TASK_STACK_SIZE`) | shared ESP-IDF tail | | source, FLAC | `thread_entry()` | the FLAC encode through micro-flac's LPC design | 4,464 | 4,432 | 5,120 (`DEFAULT_FLAC_SOURCE_TASK_STACK_SIZE`) | libgcc `__divdi3` | @@ -151,8 +151,8 @@ Notes per task: connection takes. 3,968 + 384 = 4,352, rounded up. - protocol task: noise-c and libsodium included. The bounded chain's tail cannot execute (an outbound connection never reaches the server connection's close override), but the default - covers it anyway: 6,688 + 384 = 7,072, rounded up to 7,168. The reachable chain alone needs - 6,144 + 384 = 6,528. + covers it anyway: 6,624 + 384 = 7,008, rounded up to 7,168. The reachable chain alone needs + 6,080 + 384 = 6,464. - source, PCM: the task sends nothing itself and logs nothing. The `std::thread` entry frames above `thread_entry()` are not counted and come out of the rounding slack. 1,168 + 384 = 1,552, rounded up.