From a97089e4f54bb8214d7bc3af1a48389767c91157 Mon Sep 17 00:00:00 2001 From: Christian Leithner Date: Mon, 3 Aug 2026 19:32:55 +0000 Subject: [PATCH 1/2] fix(camera): report the camera's negotiation role, not the client's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ep/webrtc negotiationRole resource is the camera's data model, so it now reports the CAMERA's WebRTC role — offerer when the camera generates the offer (SolicitOffer flow), answerer when it answers the client's offer (ProvideOffer flow) — instead of the role the client must take. An intrinsic cameraIsOfferer() predicate drives both the reported role and the (unchanged) Matter flow selection. The reference app inverts the reported value to choose its own role. The Matter signaling sequence is unchanged. Driver, reference app, the camera unit test (adds NegotiationRoleReportsCameraRole), and the webrtc-signaling- endpoint / camera-stream-reference-command specs are updated to the camera perspective; archives the openspec change. --- .../matter/sbmd/specs/camera.sbmd.js | 73 +++++++------- core/test/src/SbmdCameraWebrtcTest.cpp | 98 ++++++++++++++++++- .../.openspec.yaml | 2 + .../design.md | 79 +++++++++++++++ .../proposal.md | 37 +++++++ .../camera-stream-reference-command/spec.md | 34 +++++++ .../specs/webrtc-signaling-endpoint/spec.md | 45 +++++++++ .../tasks.md | 30 ++++++ .../camera-stream-reference-command/spec.md | 16 ++- .../specs/webrtc-signaling-endpoint/spec.md | 30 +++--- reference/src/cameraCategory.c | 9 +- reference/src/cameraDeviceSession.c | 9 +- reference/src/cameraDeviceSession.h | 8 +- 13 files changed, 402 insertions(+), 68 deletions(-) create mode 100644 openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/.openspec.yaml create mode 100644 openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/design.md create mode 100644 openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/proposal.md create mode 100644 openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/specs/camera-stream-reference-command/spec.md create mode 100644 openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/specs/webrtc-signaling-endpoint/spec.md create mode 100644 openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/tasks.md diff --git a/core/deviceDrivers/matter/sbmd/specs/camera.sbmd.js b/core/deviceDrivers/matter/sbmd/specs/camera.sbmd.js index 52fd805e..f320273f 100644 --- a/core/deviceDrivers/matter/sbmd/specs/camera.sbmd.js +++ b/core/deviceDrivers/matter/sbmd/specs/camera.sbmd.js @@ -63,9 +63,10 @@ // 3. Follow entryPoint to the protocol-specific endpoint // (e.g., //ep/webrtc/r/localSdp). Any negotiation details live on that // endpoint, not in the abstract stream result; for WebRTC the client reads -// r/negotiationRole to learn whether it is the 'offerer' (create the SDP -// offer) or 'answerer' (answer the camera's offer). The backing Matter flow -// (ProvideOffer vs SolicitOffer) is hidden from the client. +// r/negotiationRole to learn the CAMERA's role — 'offerer' when the camera +// creates the SDP offer, 'answerer' when it answers — then adopts the opposite +// role. The backing Matter flow (SolicitOffer vs ProvideOffer) is hidden from +// the client. // 4. Complete protocol-specific exchange (SDP, ICE, media URL, etc.) and // subscribe to the protocol endpoint's event resources // 5. Execute destroySession with sessionId when finished @@ -147,11 +148,12 @@ SbmdDriver({ // instead of hardcoding, so the driver stays correct if the requestor endpoint changes. WEBRTC_REQUESTOR_ENDPOINT_ID: 1, - // Negotiation role conveyed to the client in the stream() result. The client uses it to - // drive its WebRTC peer; the Matter command mapping (ProvideOffer vs SolicitOffer + - // ProvideAnswer) stays entirely inside this driver. - // 'offerer' — client creates the SDP offer (ProvideOffer flow) - // 'answerer' — client answers the camera's offer (SolicitOffer flow) + // Negotiation role reported to the client on ep/webrtc r/negotiationRole. Because that + // endpoint is the camera's data model, the value is the CAMERA's role; the client adopts + // the opposite. The Matter command mapping (SolicitOffer vs ProvideOffer + ProvideAnswer) + // stays entirely inside this driver. + // 'offerer' — camera creates the SDP offer (SolicitOffer flow; the client answers) + // 'answerer' — camera answers the client's offer (ProvideOffer flow; the client offers) ROLE_OFFERER: 'offerer', ROLE_ANSWERER: 'answerer', @@ -378,11 +380,11 @@ function executeCreateSession(args) { .success(sessionId); } -function pickNegotiationRole(args) { - // Choose the WebRTC signaling flow from the camera's advertised capabilities. Prefer - // SolicitOffer (camera generates the offer, client answers) since real cameras favor it; use - // ProvideOffer (client offers) only when SolicitOffer is not accepted. Default to SolicitOffer - // when the AcceptedCommandList is unavailable. +function cameraIsOfferer(args) { + // Determine, from the camera's advertised capabilities, whether the CAMERA generates the SDP + // offer (the SolicitOffer flow) or answers the client's offer (the ProvideOffer flow). Prefer + // SolicitOffer (real cameras favor it); fall back to ProvideOffer only when SolicitOffer is not + // accepted; default to SolicitOffer when the AcceptedCommandList is unavailable. var raw = args.supplements && args.supplements.attributes ? args.supplements.attributes.providerAcceptedCommands @@ -401,24 +403,25 @@ function pickNegotiationRole(args) { } } - var role = ROLE_ANSWERER; - - if (Array.isArray(accepted)) { - if (accepted.indexOf(CMD_SOLICIT_OFFER) !== -1) { - role = ROLE_ANSWERER; - } else if (accepted.indexOf(CMD_PROVIDE_OFFER) !== -1) { - role = ROLE_OFFERER; - } + if ( + Array.isArray(accepted) && + accepted.indexOf(CMD_SOLICIT_OFFER) === -1 && + accepted.indexOf(CMD_PROVIDE_OFFER) !== -1 + ) { + // The camera accepts only ProvideOffer: it answers and the client offers. + return false; } - return role; + // SolicitOffer accepted, or the list is unavailable: the camera generates the offer. + return true; } function readNegotiationRole(args) { - // Expose the WebRTC negotiation role ('offerer' | 'answerer') to the client. It is derived from - // the camera's advertised WebRTCTransportProvider commands, so it lives here on the webrtc - // endpoint rather than in the abstract stream result. - return Sbmd.result().success(pickNegotiationRole(args)); + // Report the CAMERA's WebRTC negotiation role, because ep/webrtc is the camera's data model. The + // camera is the 'offerer' when it generates the SDP offer (SolicitOffer flow) and the 'answerer' + // when it answers the client's offer (ProvideOffer flow); the client adopts the opposite role. + // The role lives here on the webrtc endpoint rather than in the abstract stream result. + return Sbmd.result().success(cameraIsOfferer(args) ? ROLE_OFFERER : ROLE_ANSWERER); } function executeStream(args) { @@ -545,14 +548,13 @@ function executeLocalSdp(args) { var input = args.resource.input; var sdp = input ? input.toString() : ''; - var role = pickNegotiationRole(args); - - if (role === ROLE_ANSWERER) { - // SolicitOffer flow. Route by how far negotiation has progressed rather than by the SDP - // payload: until the camera has offered there is no webRTCSessionID, so this call opens the - // flow (allocate a stream, then SolicitOffer in handleAllocateForSolicit). Once the camera's - // offer has arrived (webRTCSessionID recorded, remoteSdp emitted) the next call carries our - // answer, which we relay via ProvideAnswer. + + if (cameraIsOfferer(args)) { + // SolicitOffer flow (the camera generates the offer). Route by how far negotiation has + // progressed rather than by the SDP payload: until the camera has offered there is no + // webRTCSessionID, so this call opens the flow (allocate a stream, then SolicitOffer in + // handleAllocateForSolicit). Once the camera's offer has arrived (webRTCSessionID recorded, + // remoteSdp emitted) the next call carries our answer, which we relay via ProvideAnswer. var haveCameraSession = sessions[sessionId].webRTCSessionID !== undefined && sessions[sessionId].webRTCSessionID !== null; @@ -564,7 +566,8 @@ function executeLocalSdp(args) { return sendProvideAnswer(sessions, sessionId, sdp); } - // ProvideOffer flow: this local SDP is our offer — allocate a stream, then send it directly. + // ProvideOffer flow (the camera answers): this local SDP is our offer — allocate a stream, then + // send it directly. if (sdp === '') { return Sbmd.result().error('SDP string required'); } diff --git a/core/test/src/SbmdCameraWebrtcTest.cpp b/core/test/src/SbmdCameraWebrtcTest.cpp index accbb6f4..fca98639 100644 --- a/core/test/src/SbmdCameraWebrtcTest.cpp +++ b/core/test/src/SbmdCameraWebrtcTest.cpp @@ -28,7 +28,9 @@ * the actual handler functions — no inline copies that can drift. * * Tests cover: - * - executeLocalSdp (offerer flow): TLV encoding (null webRTCSessionID, correct tags), error paths + * - readNegotiationRole: reports the CAMERA's role (offerer/answerer) from the AcceptedCommandList + * - executeLocalSdp (client-offers / ProvideOffer flow): TLV encoding (null webRTCSessionID, correct tags), error + * paths * - executeLocalIceCandidates: valid JSON array → sendCommand, invalid JSON → error * - handleIncomingOffer / handleIncomingAnswer / handleIncomingIceCandidates / handleIncomingEndSession * - executeDestroySession with streaming session: sends EndSession command @@ -70,11 +72,17 @@ namespace constexpr uint32_t CMD_END = 0x03; // providerAcceptedCommands (AcceptedCommandList) as base64 TLV: a top-level TLV array of - // command IDs advertising ProvideOffer (0x02) but NOT SolicitOffer (0x00), so - // pickNegotiationRole selects the offerer (client-offers / ProvideOffer) flow. + // command IDs advertising ProvideOffer (0x02) but NOT SolicitOffer (0x00). The camera answers + // the client's offer (ProvideOffer flow), so cameraIsOfferer() is false — the camera's role is + // 'answerer' and the client drives the offer. // TLV bytes: 0x16(array) 0x04(uint8) 0x02 0x18(end). constexpr const char *OFFERER_ACCEPTED_CMDS = "FgQCGA=="; + // Same shape advertising SolicitOffer (0x00) but NOT ProvideOffer, so the camera generates the + // offer (SolicitOffer flow), cameraIsOfferer() is true, and the camera's role is 'offerer'. + // TLV bytes: 0x16(array) 0x04(uint8) 0x00 0x18(end). + constexpr const char *SOLICIT_ACCEPTED_CMDS = "FgQAGA=="; + // ======================================================================== // Test Fixture — loads the real camera.sbmd.js via SbmdDriver // ======================================================================== @@ -279,6 +287,68 @@ namespace return SbmdHandlerInvoker::InvokeHandler(Ctx(), handler.Get(), args); } + /** + * Build and invoke a resource READ handler (e.g. negotiationRole) with the camera's + * advertised AcceptedCommandList supplied as the providerAcceptedCommands supplement. + */ + std::optional InvokeReadHandler(const std::string &endpointId, + const std::string &resourceId, + const std::string &acceptedCmdsBase64) + { + auto hctx = MakeContext(); + std::lock_guard lock(MQuickJsRuntime::GetMutex()); + + const SbmdHandler *readHandler = nullptr; + const auto ® = s_driver->GetRegistration(); + + for (const auto &ep : reg.endpoints) + { + if (ep.id != endpointId) + { + continue; + } + + for (const auto &r : ep.resources) + { + if (r.id == resourceId && r.read.has_value()) + { + readHandler = &r.read.value(); + } + } + } + + if (readHandler == nullptr) + { + ADD_FAILURE() << "No read handler for " << endpointId << "/" << resourceId; + return std::nullopt; + } + + // Root the handler (see InvokeExecuteHandler): the arg/supplement building below + // allocates and can relocate an unrooted function object under mquickjs's moving GC. + SafeJSValue handler(Ctx(), readHandler->Fn()); + SafeJSValue args = SbmdHandlerInvoker::BuildResourceArgs(Ctx(), hctx, resourceId, ""); + + SbmdSupplements supplements = readHandler->supplements; + + auto fetched = SbmdHandlerInvoker::PrefetchSupplements( + supplements, + [&](const std::string &aliasName) -> std::optional { + if (aliasName == "providerAcceptedCommands" && !acceptedCmdsBase64.empty()) + { + return acceptedCmdsBase64; + } + + return std::nullopt; + }, + [](const std::string &) { return std::nullopt; }, + [](const std::string &) { return std::nullopt; }, + [](const std::string &) { return std::nullopt; }); + + SbmdHandlerInvoker::AddSupplements(Ctx(), args, supplements, fetched); + + return SbmdHandlerInvoker::InvokeHandler(Ctx(), handler.Get(), args); + } + /** * Build and invoke a command handler from the loaded driver by handler name. */ @@ -358,6 +428,28 @@ namespace std::unique_ptr SbmdCameraWebrtcTest::s_driver; + // ======================================================================== + // readNegotiationRole — reports the CAMERA's role + // ======================================================================== + + TEST_F(SbmdCameraWebrtcTest, NegotiationRoleReportsCameraRole) + { + // SolicitOffer accepted: the camera generates the offer, so its role is 'offerer'. + auto solicit = InvokeReadHandler("webrtc", "negotiationRole", SOLICIT_ACCEPTED_CMDS); + ExpectSuccess(solicit); + EXPECT_EQ(std::get(solicit->terminal.data).value, "offerer"); + + // ProvideOffer only: the camera answers the client's offer, so its role is 'answerer'. + auto provide = InvokeReadHandler("webrtc", "negotiationRole", OFFERER_ACCEPTED_CMDS); + ExpectSuccess(provide); + EXPECT_EQ(std::get(provide->terminal.data).value, "answerer"); + + // AcceptedCommandList unavailable: default SolicitOffer flow, camera's role is 'offerer'. + auto def = InvokeReadHandler("webrtc", "negotiationRole", ""); + ExpectSuccess(def); + EXPECT_EQ(std::get(def->terminal.data).value, "offerer"); + } + // ======================================================================== // 5.1 — executeOfferSdp // ======================================================================== diff --git a/openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/.openspec.yaml b/openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/.openspec.yaml new file mode 100644 index 00000000..e08b5f89 --- /dev/null +++ b/openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-03 diff --git a/openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/design.md b/openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/design.md new file mode 100644 index 00000000..68da73c8 --- /dev/null +++ b/openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/design.md @@ -0,0 +1,79 @@ +## Context + +`ep/webrtc` is the camera's Matter data model, surfaced through the SBMD driver `core/deviceDrivers/matter/sbmd/specs/camera.sbmd.js`. Its `negotiationRole` read resource currently answers the question *"what role must the client take?"* (`offerer`/`answerer`). That is the wrong subject: a resource on the camera's endpoint should describe the camera. The reference app (the only in-tree consumer) reads the value and maps `answerer = (role == "answerer")` directly onto its `webrtcbin` configuration. + +Internally the driver overloads the same value twice: +- `pickNegotiationRole()` maps the camera's advertised `WebRTCTransportProvider.AcceptedCommandList` to a client-role string. +- `readNegotiationRole()` returns that string to the client. +- `executeLocalSdp()` *also* calls `pickNegotiationRole()` and branches on `role === ROLE_ANSWERER` to choose the SolicitOffer vs ProvideOffer Matter flow. + +So the client-role string is entangled with the internal flow selection. Inverting the public contract must not disturb the Matter signaling sequence. + +## Goals / Non-Goals + +**Goals:** +- `negotiationRole` reports the **camera's** role: `offerer` when the camera generates the SDP offer (SolicitOffer flow), `answerer` when the camera answers (ProvideOffer flow), defaulting to `offerer`. +- Keep the Matter signaling flow (`VideoStreamAllocate` → `SolicitOffer`/`ProvideOffer` → `ProvideAnswer` → ICE → events) byte-for-byte unchanged. +- The reference app derives its own role by inverting the camera's role and keeps working end-to-end. +- Specs, driver comments, and tests reflect the camera-perspective contract. + +**Non-Goals:** +- No change to the Matter command sequence, video-stream allocation, event resources, or session lifecycle. +- No change to the resource's type/modes (`read` `string`). +- No new public C/GObject/GIR API. + +## Decisions + +### Decision 1: Express the flow selection as an intrinsic camera predicate, not a client role + +Introduce an internal helper that answers the camera-centric question directly — e.g. `cameraIsOfferer(args)` → `true` when the camera accepts `SolicitOffer` (or when the accepted-command list is unavailable, i.e. the default SolicitOffer flow), `false` when it accepts only `ProvideOffer`. Both consumers derive from it: + +- `readNegotiationRole()` → `cameraIsOfferer ? ROLE_OFFERER : ROLE_ANSWERER`. +- `executeLocalSdp()` branches on `cameraIsOfferer` (true → SolicitOffer flow; false → ProvideOffer flow) — the *same* branch it takes today, just keyed off the intrinsic predicate instead of a client-role string. + +*Alternative considered*: simply invert `pickNegotiationRole()` to return the camera role and flip `executeLocalSdp`'s comparison to `role === ROLE_OFFERER`. Rejected because it keeps overloading a role string for flow control, which is exactly the confusion this change removes. The predicate makes the SolicitOffer↔`offerer` correspondence explicit and self-documenting. + +### Decision 2: Reference app inverts at the single read site + +`cameraCategory.c` reads the role once (`cameraDeviceSessionGetRole`) and computes `answerer`. Change that one mapping from `answerer = (role == "answerer")` to `answerer = (role == "offerer")` — the client answers when the camera offers. `cameraDeviceSession.*` and `cameraWebrtcClient.*` keep operating on the client-side `answerer` boolean, so only the interpretation at the read site changes; the rest of the reference-app pipeline is untouched. Comments that describe the role (`cameraCategory.c`, `cameraDeviceSession.*`, `cameraWebrtcClient.*`) are updated to the camera perspective. + +### Decision 3: Spec deltas as MODIFIED + RENAMED + +The `webrtc-signaling-endpoint` requirement "negotiationRole read reports the client's role" is renamed to "...the camera's role" and its body/scenarios inverted; the resource table row description is updated. `camera-stream-reference-command`'s orchestration requirement gains the invert-at-read-site behavior and a scenario asserting it. + +### Data flow + +``` + Matter camera SBMD driver (camera.sbmd.js) reference app + (WebRTCTransportProvider) ep/webrtc (cameraCategory.c) + ┌───────────────────┐ ┌───────────────────────────────┐ ┌────────────────────┐ + │ AcceptedCommandList│ attr │ cameraIsOfferer(args): │ │ role = GetRole() │ + │ - SolicitOffer │ ───────► │ SolicitOffer / (unavailable)│ │ answerer = │ + │ - ProvideOffer │ supplmt │ → true (camera offerer) │ read │ (role=="offerer")│ + └───────────────────┘ │ ProvideOffer only │ ───────► │ SetAnswerer(webrtc)│ + │ → false (camera answerer) │ "offerer"│ │ + │ readNegotiationRole: │ or │ client role = │ + │ true→'offerer' false→'answerer'│ "answerer"│ opposite of camera│ + │ executeLocalSdp: same branch │ └────────────────────┘ + │ keyed on cameraIsOfferer │ + └───────────────────────────────┘ +``` + +## Risks / Trade-offs + +- **[Breaking contract inversion]** Any independent client relying on the old client-perspective value would break. → Mitigation: the only in-tree consumer (reference app) is updated in the same change; the value is validated end-to-end against a real camera before merge; the change is documented as **BREAKING** and lands on `feature/cameras` before any external client depends on it. +- **[Silent flow regression]** If the flow branch is mis-keyed during the predicate refactor, signaling could pick the wrong Matter command and hang. → Mitigation: the predicate maps 1:1 to today's branch (SolicitOffer↔true); covered by the SBMD camera unit test and an end-to-end reference-app run. +- **[Stale role wording]** Comments/spec prose could keep the old perspective. → Mitigation: audit every `negotiationRole`/`offerer`/`answerer` mention in the driver, reference app, and both specs as part of the tasks. + +## Migration Plan + +No data or schema migration. Coordinated code change: driver + reference app + specs in one change. Rollback = revert the change commit; there is no persisted state keyed on the role value. + +## Backward compatibility / API implications + +- No public C API, GObject signal/property, or GIR surface changes — `negotiationRole` is an SBMD data-model resource string, not part of the GObject API. +- Thread safety: unchanged. Driver handlers run on the existing SBMD execution context; the reference app reads the role on its command thread. No new threads, locks, or main-loop interactions. + +## Open Questions + +- None. (Default when `AcceptedCommandList` is unavailable stays the SolicitOffer flow, now reported as `offerer`.) diff --git a/openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/proposal.md b/openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/proposal.md new file mode 100644 index 00000000..b1d7e8a3 --- /dev/null +++ b/openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/proposal.md @@ -0,0 +1,37 @@ +## Why + +The `webrtc` endpoint's `negotiationRole` resource currently reports the role the *client* must take (`offerer`/`answerer`). But `ep/webrtc` is the camera's Matter data model — a resource on the camera's endpoint should describe the *camera*, not the client consuming it. Reporting the client's role inverts the perspective, leaks a client concern into the device model, and forces every reader to know that the value is "the opposite of what the device does." + +## What Changes + +- **BREAKING**: `negotiationRole` (on `ep/webrtc`) SHALL report the **camera's** WebRTC negotiation role — `offerer` when the camera generates the SDP offer, `answerer` when the camera answers the client's offer — instead of the client's role. +- Invert the driver's mapping from the camera's advertised `AcceptedCommandList` to the reported role: camera accepts `SolicitOffer` → camera is the `offerer`; camera accepts `ProvideOffer` → camera is the `answerer`. The default (SolicitOffer flow) therefore reports `offerer`. +- Decouple the internal SolicitOffer/ProvideOffer flow selection in `localSdp` from the public role value so the Matter signaling sequence is byte-for-byte unchanged; only the reported role string and its interpretation change. +- Update the reference app to derive its own role by inverting the camera's reported role: act as the **answerer** when the camera role is `offerer`, and as the **offerer** when the camera role is `answerer`. +- Update the affected specs, driver comments, and tests to the camera-perspective contract. + +## Capabilities + +### New Capabilities + + + +### Modified Capabilities + +- `webrtc-signaling-endpoint`: the `negotiationRole` requirement changes so the read reports the camera's role (not the client's); the `AcceptedCommandList` → role mapping is inverted (`SolicitOffer` → `offerer`, `ProvideOffer` → `answerer`, default `offerer`). +- `camera-stream-reference-command`: the reference app's orchestration requirement changes so it reads the camera's role and inverts it to select its own WebRTC role. + +## Non-goals + +- No change to the underlying Matter signaling flow: the `VideoStreamAllocate` → `SolicitOffer`/`ProvideOffer` → `ProvideAnswer`/`ProvideICECandidates` sequence, the incoming `Offer`/`Answer`/`ICECandidates`/`End` handlers, and the `remoteSdp`/`remoteIceCandidates`/`webrtcError` event resources are all unchanged. +- No change to the `negotiationRole` resource's type or modes (it remains a `read` `string` on `ep/webrtc`). +- No new streaming protocols, camera features, or session-lifecycle changes. + +## Impact + +- **Core driver** (`core/deviceDrivers/matter/sbmd/specs/camera.sbmd.js`): `pickNegotiationRole` / `readNegotiationRole` and the `localSdp` flow-selection logic; the `ROLE_OFFERER`/`ROLE_ANSWERER` usage and the endpoint/flow comments. +- **Reference app** (`reference/src/cameraCategory.c`, `cameraDeviceSession.c` / `.h`, `cameraWebrtcClient.*` comments): the role interpretation, which becomes `answerer = (cameraRole == "offerer")`. +- **Specs**: `openspec/specs/webrtc-signaling-endpoint/spec.md` and `openspec/specs/camera-stream-reference-command/spec.md`. +- **Tests**: SBMD camera unit test(s) that assert `negotiationRole`, and any Python integration test asserting the role value. +- **CMake flags**: the reference-app portion is gated by `BCORE_REFERENCE_CAMERA_SUPPORT`; the driver portion is always built. +- **Compatibility**: the only in-tree consumer of `negotiationRole` is the reference app, which is updated in lockstep, so no runtime mismatch ships. The contract inversion is nonetheless breaking for any independent client that assumed the old client-perspective semantics. diff --git a/openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/specs/camera-stream-reference-command/spec.md b/openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/specs/camera-stream-reference-command/spec.md new file mode 100644 index 00000000..767cff12 --- /dev/null +++ b/openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/specs/camera-stream-reference-command/spec.md @@ -0,0 +1,34 @@ +## MODIFIED Requirements + +### Requirement: cameraStream orchestrates full session lifecycle + +The `cameraStream` command SHALL orchestrate the complete camera streaming flow through Barton's resource API: +1. Execute `createSession` on the device's `ep/camera` endpoint +2. Execute `stream` and obtain the active protocol and entry-point URI from its `{ protocol, entryPoint }` result +3. Read the `negotiationRole` resource on `ep/webrtc` — which reports the **camera's** role — and adopt the opposite role for itself: act as the `answerer` when the camera is the `offerer`, or as the `offerer` when the camera is the `answerer` +4. Create a local GStreamer `webrtcbin` peer connection using host candidates only (no STUN/TURN), configured for the client's derived role +5. Perform the SDP exchange for the client's derived role via the `localSdp` resource on `ep/webrtc`: + - **Client is the offerer** (camera reported `answerer`): generate a local SDP offer from webrtcbin, execute `localSdp` with it, then wait for a `remoteSdp` event (the camera's answer) and set it as the remote description + - **Client is the answerer** (camera reported `offerer`): execute `localSdp` with empty input to open the flow, wait for a `remoteSdp` event (the camera's offer), set it as the remote description, generate a local SDP answer, and execute `localSdp` with the answer +6. Exchange ICE candidates (local → `localIceCandidates`, remote ← `remoteIceCandidates` events) +7. Wait for the peer connection to reach the connected state, subject to a bounded connectivity timeout +8. Route the received media to the destination selected by `--out`: serve it over the built-in HTTP server or record it to a file +9. On user interrupt (Ctrl+C) or a `webrtcError` event: execute `destroySession` and tear down the pipeline + +#### Scenario: Client adopts the opposite of the camera's role +- **WHEN** the `negotiationRole` read returns `offerer` (the camera is the offerer) +- **THEN** the reference app SHALL configure `webrtcbin` as the `answerer` and answer the camera's offer +- **WHEN** the `negotiationRole` read returns `answerer` (the camera is the answerer) +- **THEN** the reference app SHALL configure `webrtcbin` as the `offerer` and generate the SDP offer + +#### Scenario: Successful camera stream served to a browser +- **WHEN** a user executes `cameraStream ` (or with an `http://` `--out`) and the camera responds to signaling +- **THEN** the reference app SHALL serve the live stream over its built-in HTTP server for a browser to play and print status messages for each step, without opening a local display window + +#### Scenario: Successful camera stream to file +- **WHEN** a user executes `cameraStream --out file://recording.mp4` +- **THEN** the reference app SHALL record the video stream to the specified file path + +#### Scenario: User stops the stream +- **WHEN** a user presses Ctrl+C during an active stream +- **THEN** the reference app SHALL execute `destroySession`, stop the GStreamer pipeline, and return to the command prompt diff --git a/openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/specs/webrtc-signaling-endpoint/spec.md b/openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/specs/webrtc-signaling-endpoint/spec.md new file mode 100644 index 00000000..7b542442 --- /dev/null +++ b/openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/specs/webrtc-signaling-endpoint/spec.md @@ -0,0 +1,45 @@ +## MODIFIED Requirements + +### Requirement: WebRTC endpoint declares signaling resources + +The camera SBMD driver SHALL declare an endpoint with id `"webrtc"` and profile `"webrtc"` containing six resources: + +| Resource | Type | Modes | Purpose | +|----------|------|-------|---------| +| `localSdp` | `function` | execute | Client posts its local SDP (offer or answer) to drive signaling | +| `negotiationRole` | `string` | [read] | Reports the **camera's** negotiation role (`offerer` or `answerer`); the client adopts the opposite role | +| `remoteSdp` | `string` | [] (events only) | Delivers the camera's remote SDP (offer or answer) to client | +| `localIceCandidates` | `function` | execute | Client sends local ICE candidates | +| `remoteIceCandidates` | `string` | [] (events only) | Delivers remote ICE candidates to client | +| `webrtcError` | `string` | [volatile] (events only) | Delivers asynchronous session termination/error to client | + +The endpoint SHALL be declared within the same `camera.sbmd.js` file as the `ep/camera` endpoint. The `webrtcError` resource SHALL be declared with the `volatile` mode so that its events are emitted unconditionally (non-cached), independent of the previously emitted value. + +#### Scenario: Endpoint appears on commissioned camera device +- **WHEN** a Matter camera device (deviceType 0x0142) with WebRTCTransportProvider cluster (0x0553) is commissioned +- **THEN** the device SHALL have an endpoint with id `"webrtc"`, profile `"webrtc"`, and all six resources registered + +#### Scenario: Event-only resources are not readable +- **WHEN** a client attempts to read `remoteSdp`, `remoteIceCandidates`, or `webrtcError` +- **THEN** the read SHALL fail or return no value (modes list is empty — no read mode) + +### Requirement: negotiationRole read reports the camera's role + +The `negotiationRole` read handler SHALL report the **camera's** WebRTC negotiation role — `offerer` when the camera generates the SDP offer, or `answerer` when the camera answers the client's offer — derived from the camera's advertised WebRTCTransportProvider `AcceptedCommandList`. When the camera accepts `SolicitOffer` the role SHALL be `offerer` (the camera generates the offer); otherwise, when the camera accepts `ProvideOffer`, the role SHALL be `answerer` (the camera answers). When the accepted-command list is unavailable, the handler SHALL default to `offerer` (the SolicitOffer flow). The resource describes the camera because `ep/webrtc` is the camera's data model; the consuming client is responsible for adopting the opposite role. The negotiation role is a WebRTC concept and lives on the `webrtc` endpoint, not in the abstract `stream` result. + +#### Scenario: Camera supporting SolicitOffer reports offerer +- **WHEN** a client reads `negotiationRole` and the camera's `AcceptedCommandList` includes `SolicitOffer` +- **THEN** the read SHALL return `offerer` (the camera generates the offer and the client answers) + +#### Scenario: Camera supporting only ProvideOffer reports answerer +- **WHEN** a client reads `negotiationRole` and the camera's `AcceptedCommandList` includes `ProvideOffer` but not `SolicitOffer` +- **THEN** the read SHALL return `answerer` (the camera answers and the client offers) + +#### Scenario: Unavailable accepted-command list defaults to offerer +- **WHEN** a client reads `negotiationRole` and the camera's `AcceptedCommandList` is unavailable +- **THEN** the read SHALL return `offerer` (the default SolicitOffer flow, in which the camera generates the offer) + +## RENAMED Requirements + +- FROM: `### Requirement: negotiationRole read reports the client's role` +- TO: `### Requirement: negotiationRole read reports the camera's role` diff --git a/openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/tasks.md b/openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/tasks.md new file mode 100644 index 00000000..0619f4ed --- /dev/null +++ b/openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/tasks.md @@ -0,0 +1,30 @@ +## 1. Driver contract inversion (camera.sbmd.js) + +- [x] 1.1 Add an intrinsic `cameraIsOfferer(args)` predicate deriving from `providerAcceptedCommands`: `true` when `SolicitOffer` is accepted OR the accepted-command list is unavailable (default SolicitOffer flow), `false` when only `ProvideOffer` is accepted +- [x] 1.2 Update `readNegotiationRole` to report the camera's role via the predicate: `cameraIsOfferer ? ROLE_OFFERER : ROLE_ANSWERER` +- [x] 1.3 Rekey `executeLocalSdp`'s SolicitOffer/ProvideOffer branch on `cameraIsOfferer` and confirm the emitted Matter command sequence is identical to today's (SolicitOffer flow when the camera is the offerer) +- [x] 1.4 Update driver docblocks and `ROLE_OFFERER`/`ROLE_ANSWERER` comments (client-flow section, endpoint description) to the camera-perspective wording +- [x] 1.5 Validate the SBMD spec file (`validate-sbmd`/build-time validation) — no schema regressions + +## 2. Driver tests (unit) + +- [x] 2.1 Update the SBMD camera unit test's `negotiationRole` expectations: `SolicitOffer` → `offerer`, `ProvideOffer`-only → `answerer`, unavailable list → `offerer` +- [x] 2.2 Add/adjust a test asserting `executeLocalSdp` still selects the SolicitOffer flow when the camera is the offerer and ProvideOffer when it is the answerer (flow unchanged) +- [x] 2.3 Run the camera SBMD unit tests via `ctest` and confirm green + +## 3. Reference app interpretation (gated by BCORE_REFERENCE_CAMERA_SUPPORT) + +- [x] 3.1 In `reference/src/cameraCategory.c`, invert the single mapping to `answerer = (g_strcmp0(role, "offerer") == 0)` and update the adjacent comment (client answers when the camera offers) +- [x] 3.2 Update role-describing comments/docs in `reference/src/cameraDeviceSession.c` / `.h` and `reference/src/cameraWebrtcClient.c` / `.h` to the camera perspective (the resource reports the camera's role; the client adopts the opposite) +- [x] 3.3 Build the reference app with `-DBCORE_REFERENCE_CAMERA_SUPPORT=ON` and confirm it compiles and formats clean + +## 4. Spec + docs sync + +- [x] 4.1 Confirm the `webrtc-signaling-endpoint` and `camera-stream-reference-command` delta specs in this change match the implemented behavior; run `openspec validate negotiation-role-camera-perspective --strict` +- [x] 4.2 Grep the driver, reference app, and specs for any remaining "client's role" / stale `offerer`/`answerer` wording and reconcile (canonical `specs/` are corrected by this change's delta on archive; `camera-stream` remoteSdp wording legitimately describes the client's own role) + +## 5. End-to-end verification + +- [x] 5.1 Confirm all camera unit tests pass (must precede integration/manual checks) +- [x] 5.2 Update any Python integration test that asserts the `negotiationRole` value to the camera perspective, and run it (requires Docker) — no integration test asserts `negotiationRole`, so none to update +- [x] 5.3 Manual end-to-end run against a camera (requires Docker + a camera source, e.g. `chip-camera-app` on `/dev/video0`): commission, run `cs `, and confirm the role reads `offerer`, negotiation completes, and media flows to the HTTP server / recording diff --git a/openspec/specs/camera-stream-reference-command/spec.md b/openspec/specs/camera-stream-reference-command/spec.md index 9cc11b7e..56f8ee74 100644 --- a/openspec/specs/camera-stream-reference-command/spec.md +++ b/openspec/specs/camera-stream-reference-command/spec.md @@ -26,16 +26,22 @@ The reference app SHALL provide a command named `cameraStream` with short alias The `cameraStream` command SHALL orchestrate the complete camera streaming flow through Barton's resource API: 1. Execute `createSession` on the device's `ep/camera` endpoint 2. Execute `stream` and obtain the active protocol and entry-point URI from its `{ protocol, entryPoint }` result -3. Read the `negotiationRole` resource on `ep/webrtc` to learn whether this client is the `offerer` or the `answerer` -4. Create a local GStreamer `webrtcbin` peer connection using host candidates only (no STUN/TURN), configured for the negotiated role -5. Perform the role-appropriate SDP exchange via the `localSdp` resource on `ep/webrtc`: - - **Offerer**: generate a local SDP offer from webrtcbin, execute `localSdp` with it, then wait for a `remoteSdp` event (the camera's answer) and set it as the remote description - - **Answerer**: execute `localSdp` with empty input to open the flow, wait for a `remoteSdp` event (the camera's offer), set it as the remote description, generate a local SDP answer, and execute `localSdp` with the answer +3. Read the `negotiationRole` resource on `ep/webrtc` — which reports the **camera's** role — and adopt the opposite role for itself: act as the `answerer` when the camera is the `offerer`, or as the `offerer` when the camera is the `answerer` +4. Create a local GStreamer `webrtcbin` peer connection using host candidates only (no STUN/TURN), configured for the client's derived role +5. Perform the SDP exchange for the client's derived role via the `localSdp` resource on `ep/webrtc`: + - **Client is the offerer** (camera reported `answerer`): generate a local SDP offer from webrtcbin, execute `localSdp` with it, then wait for a `remoteSdp` event (the camera's answer) and set it as the remote description + - **Client is the answerer** (camera reported `offerer`): execute `localSdp` with empty input to open the flow, wait for a `remoteSdp` event (the camera's offer), set it as the remote description, generate a local SDP answer, and execute `localSdp` with the answer 6. Exchange ICE candidates (local → `localIceCandidates`, remote ← `remoteIceCandidates` events) 7. Wait for the peer connection to reach the connected state, subject to a bounded connectivity timeout 8. Route the received media to the destination selected by `--out`: serve it over the built-in HTTP server or record it to a file 9. On user interrupt (Ctrl+C) or a `webrtcError` event: execute `destroySession` and tear down the pipeline +#### Scenario: Client adopts the opposite of the camera's role +- **WHEN** the `negotiationRole` read returns `offerer` (the camera is the offerer) +- **THEN** the reference app SHALL configure `webrtcbin` as the `answerer` and answer the camera's offer +- **WHEN** the `negotiationRole` read returns `answerer` (the camera is the answerer) +- **THEN** the reference app SHALL configure `webrtcbin` as the `offerer` and generate the SDP offer + #### Scenario: Successful camera stream served to a browser - **WHEN** a user executes `cameraStream ` (or with an `http://` `--out`) and the camera responds to signaling - **THEN** the reference app SHALL serve the live stream over its built-in HTTP server for a browser to play and print status messages for each step, without opening a local display window diff --git a/openspec/specs/webrtc-signaling-endpoint/spec.md b/openspec/specs/webrtc-signaling-endpoint/spec.md index 27d029fe..9b4c599d 100644 --- a/openspec/specs/webrtc-signaling-endpoint/spec.md +++ b/openspec/specs/webrtc-signaling-endpoint/spec.md @@ -10,7 +10,7 @@ The camera SBMD driver SHALL declare an endpoint with id `"webrtc"` and profile | Resource | Type | Modes | Purpose | |----------|------|-------|---------| | `localSdp` | `function` | execute | Client posts its local SDP (offer or answer) to drive signaling | -| `negotiationRole` | `string` | [read] | Reports the negotiation role (`offerer` or `answerer`) the client must adopt | +| `negotiationRole` | `string` | [read] | Reports the **camera's** negotiation role (`offerer` or `answerer`); the client adopts the opposite role | | `remoteSdp` | `string` | [] (events only) | Delivers the camera's remote SDP (offer or answer) to client | | `localIceCandidates` | `function` | execute | Client sends local ICE candidates | | `remoteIceCandidates` | `string` | [] (events only) | Delivers remote ICE candidates to client | @@ -26,18 +26,6 @@ The endpoint SHALL be declared within the same `camera.sbmd.js` file as the `ep/ - **WHEN** a client attempts to read `remoteSdp`, `remoteIceCandidates`, or `webrtcError` - **THEN** the read SHALL fail or return no value (modes list is empty — no read mode) -### Requirement: negotiationRole read reports the client's role - -The `negotiationRole` read handler SHALL report whether the client is the `offerer` or the `answerer`, derived from the camera's advertised WebRTCTransportProvider `AcceptedCommandList`. When the camera accepts `SolicitOffer` the role SHALL be `answerer` (the camera generates the offer); otherwise, when the camera accepts `ProvideOffer`, the role SHALL be `offerer`. When the accepted-command list is unavailable, the handler SHALL default to `answerer`. The negotiation role is a WebRTC concept and lives on the `webrtc` endpoint, not in the abstract `stream` result. - -#### Scenario: Camera supporting SolicitOffer yields answerer -- **WHEN** a client reads `negotiationRole` and the camera's `AcceptedCommandList` includes `SolicitOffer` -- **THEN** the read SHALL return `answerer` - -#### Scenario: Camera supporting only ProvideOffer yields offerer -- **WHEN** a client reads `negotiationRole` and the camera's `AcceptedCommandList` includes `ProvideOffer` but not `SolicitOffer` -- **THEN** the read SHALL return `offerer` - ### Requirement: localSdp execute drives role-appropriate signaling The `localSdp` execute handler SHALL determine the client's negotiation role from the camera's advertised WebRTCTransportProvider commands (its `AcceptedCommandList`) and drive the corresponding Matter signaling. In every case that requires it, the handler SHALL first allocate a video stream via `VideoStreamAllocate` (cluster 0x0551, command 0x03) before the WebRTC-provider command, and SHALL pass the requestor's `originatingEndpointID` (the endpoint hosting the `WebRTCTransportRequestor` cluster) so the camera knows where to send its commands. @@ -170,3 +158,19 @@ The webrtc endpoint resources, constants, and handler functions SHALL be grouped - **WHEN** the webrtc endpoint code is reviewed - **THEN** all webrtc-specific constants, resources, and handlers SHALL be identifiable as a cohesive group that could be moved to a separate file with only transient data key sharing as the interface +### Requirement: negotiationRole read reports the camera's role + +The `negotiationRole` read handler SHALL report the **camera's** WebRTC negotiation role — `offerer` when the camera generates the SDP offer, or `answerer` when the camera answers the client's offer — derived from the camera's advertised WebRTCTransportProvider `AcceptedCommandList`. When the camera accepts `SolicitOffer` the role SHALL be `offerer` (the camera generates the offer); otherwise, when the camera accepts `ProvideOffer`, the role SHALL be `answerer` (the camera answers). When the accepted-command list is unavailable, the handler SHALL default to `offerer` (the SolicitOffer flow). The resource describes the camera because `ep/webrtc` is the camera's data model; the consuming client is responsible for adopting the opposite role. The negotiation role is a WebRTC concept and lives on the `webrtc` endpoint, not in the abstract `stream` result. + +#### Scenario: Camera supporting SolicitOffer reports offerer +- **WHEN** a client reads `negotiationRole` and the camera's `AcceptedCommandList` includes `SolicitOffer` +- **THEN** the read SHALL return `offerer` (the camera generates the offer and the client answers) + +#### Scenario: Camera supporting only ProvideOffer reports answerer +- **WHEN** a client reads `negotiationRole` and the camera's `AcceptedCommandList` includes `ProvideOffer` but not `SolicitOffer` +- **THEN** the read SHALL return `answerer` (the camera answers and the client offers) + +#### Scenario: Unavailable accepted-command list defaults to offerer +- **WHEN** a client reads `negotiationRole` and the camera's `AcceptedCommandList` is unavailable +- **THEN** the read SHALL return `offerer` (the default SolicitOffer flow, in which the camera generates the offer) + diff --git a/reference/src/cameraCategory.c b/reference/src/cameraCategory.c index 843f975a..5fee766c 100644 --- a/reference/src/cameraCategory.c +++ b/reference/src/cameraCategory.c @@ -538,11 +538,12 @@ static bool runCameraStream(BCoreClient *client, return false; } - // Determine our negotiation role from the driver's stream result. In answerer mode - // (SolicitOffer flow) the camera provides the offer and we answer it; otherwise (offerer / - // ProvideOffer flow) we create the offer. Must be set before the peer starts negotiating. + // Determine our negotiation role by inverting the camera's. negotiationRole reports the + // CAMERA's role: when the camera is the 'offerer' (SolicitOffer flow) it provides the offer and + // we answer it; when the camera is the 'answerer' (ProvideOffer flow) we create the offer. Must + // be set before the peer starts negotiating. const gchar *role = cameraDeviceSessionGetRole(session); - gboolean answerer = (g_strcmp0(role, "answerer") == 0); + gboolean answerer = (g_strcmp0(role, "offerer") == 0); cameraWebrtcClientSetAnswerer(webrtc, answerer); // Step 4: Start client (in offerer mode this triggers negotiation -> creates the SDP offer) diff --git a/reference/src/cameraDeviceSession.c b/reference/src/cameraDeviceSession.c index e6efe141..1bf4fdb9 100644 --- a/reference/src/cameraDeviceSession.c +++ b/reference/src/cameraDeviceSession.c @@ -41,9 +41,9 @@ struct _CameraDeviceSession gchar *sessionId; gchar *protocol; // from the stream execute result gchar *entryPoint; // from the stream execute result - gchar *role; // 'offerer' | 'answerer'; populated lazily from - // /ep/webrtc/r/negotiationRole (see cameraDeviceSessionGetRole), not the - // stream execute result + gchar *role; // the CAMERA's negotiation role ('offerer' | 'answerer'); populated lazily + // from /ep/webrtc/r/negotiationRole (see cameraDeviceSessionGetRole), not the + // stream execute result. The client adopts the opposite role. gboolean opened; }; @@ -229,7 +229,8 @@ const gchar *cameraDeviceSessionGetRole(CameraDeviceSession *session) g_return_val_if_fail(session != NULL, NULL); // The negotiation role is a WebRTC concept, so it lives on the webrtc endpoint rather than in - // the abstract stream result. Read it lazily on first request and cache it for the session. + // the abstract stream result. The resource reports the CAMERA's role (the client adopts the + // opposite). Read it lazily on first request and cache it for the session. if (session->role == NULL) { g_autofree gchar *uri = g_strdup_printf("/%s/ep/webrtc/r/negotiationRole", session->deviceId); diff --git a/reference/src/cameraDeviceSession.h b/reference/src/cameraDeviceSession.h index 981f6eeb..5b03164d 100644 --- a/reference/src/cameraDeviceSession.h +++ b/reference/src/cameraDeviceSession.h @@ -117,12 +117,12 @@ const gchar *cameraDeviceSessionGetProtocol(CameraDeviceSession *session); const gchar *cameraDeviceSessionGetEntryPoint(CameraDeviceSession *session); /** - * Get the negotiation role for the active protocol ("offerer" or "answerer"), or NULL if - * unavailable. For WebRTC this is read from the webrtc endpoint's negotiationRole resource on first - * request and cached. The session owns the string. + * Get the CAMERA's negotiation role for the active protocol ("offerer" or "answerer"), or NULL if + * unavailable; the client adopts the opposite role. For WebRTC this is read from the webrtc + * endpoint's negotiationRole resource on first request and cached. The session owns the string. * * @param session the session - * @return the role string, or NULL + * @return the role string (the camera's role), or NULL */ const gchar *cameraDeviceSessionGetRole(CameraDeviceSession *session); From e33dc71467dbfafaa23211c4b78dcdf6e9b114d9 Mon Sep 17 00:00:00 2001 From: Christian Leithner Date: Tue, 4 Aug 2026 13:59:40 +0000 Subject: [PATCH 2/2] fix(camera): harden negotiation-role handling per review - cameraCategory.c: validate the camera's negotiation role and fail early with a clear error on NULL/unknown, instead of silently defaulting to a role that could drive the wrong signaling flow. - SbmdCameraWebrtcTest.cpp: rename OFFERER_ACCEPTED_CMDS -> CAMERA_ANSWERER_ ACCEPTED_CMDS and SOLICIT_ACCEPTED_CMDS -> CAMERA_OFFERER_ACCEPTED_CMDS to match the camera-perspective contract; un-wrap a header-comment bullet. --- core/test/src/SbmdCameraWebrtcTest.cpp | 43 +++++++++++++------------- reference/src/cameraCategory.c | 11 +++++++ 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/core/test/src/SbmdCameraWebrtcTest.cpp b/core/test/src/SbmdCameraWebrtcTest.cpp index fca98639..9a16ae30 100644 --- a/core/test/src/SbmdCameraWebrtcTest.cpp +++ b/core/test/src/SbmdCameraWebrtcTest.cpp @@ -29,8 +29,7 @@ * * Tests cover: * - readNegotiationRole: reports the CAMERA's role (offerer/answerer) from the AcceptedCommandList - * - executeLocalSdp (client-offers / ProvideOffer flow): TLV encoding (null webRTCSessionID, correct tags), error - * paths + * - executeLocalSdp (client-offers / ProvideOffer flow): TLV encoding (null webRTCSessionID, tags), error paths * - executeLocalIceCandidates: valid JSON array → sendCommand, invalid JSON → error * - handleIncomingOffer / handleIncomingAnswer / handleIncomingIceCandidates / handleIncomingEndSession * - executeDestroySession with streaming session: sends EndSession command @@ -76,12 +75,12 @@ namespace // the client's offer (ProvideOffer flow), so cameraIsOfferer() is false — the camera's role is // 'answerer' and the client drives the offer. // TLV bytes: 0x16(array) 0x04(uint8) 0x02 0x18(end). - constexpr const char *OFFERER_ACCEPTED_CMDS = "FgQCGA=="; + constexpr const char *CAMERA_ANSWERER_ACCEPTED_CMDS = "FgQCGA=="; // Same shape advertising SolicitOffer (0x00) but NOT ProvideOffer, so the camera generates the // offer (SolicitOffer flow), cameraIsOfferer() is true, and the camera's role is 'offerer'. // TLV bytes: 0x16(array) 0x04(uint8) 0x00 0x18(end). - constexpr const char *SOLICIT_ACCEPTED_CMDS = "FgQAGA=="; + constexpr const char *CAMERA_OFFERER_ACCEPTED_CMDS = "FgQAGA=="; // ======================================================================== // Test Fixture — loads the real camera.sbmd.js via SbmdDriver @@ -435,12 +434,12 @@ namespace TEST_F(SbmdCameraWebrtcTest, NegotiationRoleReportsCameraRole) { // SolicitOffer accepted: the camera generates the offer, so its role is 'offerer'. - auto solicit = InvokeReadHandler("webrtc", "negotiationRole", SOLICIT_ACCEPTED_CMDS); + auto solicit = InvokeReadHandler("webrtc", "negotiationRole", CAMERA_OFFERER_ACCEPTED_CMDS); ExpectSuccess(solicit); EXPECT_EQ(std::get(solicit->terminal.data).value, "offerer"); // ProvideOffer only: the camera answers the client's offer, so its role is 'answerer'. - auto provide = InvokeReadHandler("webrtc", "negotiationRole", OFFERER_ACCEPTED_CMDS); + auto provide = InvokeReadHandler("webrtc", "negotiationRole", CAMERA_ANSWERER_ACCEPTED_CMDS); ExpectSuccess(provide); EXPECT_EQ(std::get(provide->terminal.data).value, "answerer"); @@ -457,8 +456,8 @@ namespace TEST_F(SbmdCameraWebrtcTest, ExecuteOfferSdpValidSessionProducesVideoStreamAllocate) { std::string sessions = SessionsJson("1", "streaming"); - auto result = - InvokeExecuteHandler("webrtc", "localSdp", "test-offer-sdp", sessions, "", {}, OFFERER_ACCEPTED_CMDS); + auto result = InvokeExecuteHandler( + "webrtc", "localSdp", "test-offer-sdp", sessions, "", {}, CAMERA_ANSWERER_ACCEPTED_CMDS); auto &cmd = ExpectRequestCommand(result, CL_CAMERA_AV_STREAM_MGMT, CMD_VIDEO_STREAM_ALLOCATE); EXPECT_EQ(cmd.responseCommandId, CMD_VIDEO_STREAM_ALLOCATE_RESP); @@ -472,8 +471,8 @@ namespace TEST_F(SbmdCameraWebrtcTest, ExecuteOfferSdpAllocateTlvHasStreamUsage) { std::string sessions = SessionsJson("1", "streaming"); - auto result = - InvokeExecuteHandler("webrtc", "localSdp", "test-offer-sdp", sessions, "", {}, OFFERER_ACCEPTED_CMDS); + auto result = InvokeExecuteHandler( + "webrtc", "localSdp", "test-offer-sdp", sessions, "", {}, CAMERA_ANSWERER_ACCEPTED_CMDS); auto &cmd = ExpectRequestCommand(result, CL_CAMERA_AV_STREAM_MGMT, CMD_VIDEO_STREAM_ALLOCATE); std::lock_guard lock(MQuickJsRuntime::GetMutex()); @@ -490,8 +489,8 @@ namespace TEST_F(SbmdCameraWebrtcTest, ExecuteOfferSdpAllocateTlvHasCorrectFields) { std::string sessions = SessionsJson("1", "streaming"); - auto result = - InvokeExecuteHandler("webrtc", "localSdp", "test-offer-sdp", sessions, "", {}, OFFERER_ACCEPTED_CMDS); + auto result = InvokeExecuteHandler( + "webrtc", "localSdp", "test-offer-sdp", sessions, "", {}, CAMERA_ANSWERER_ACCEPTED_CMDS); auto &cmd = ExpectRequestCommand(result, CL_CAMERA_AV_STREAM_MGMT, CMD_VIDEO_STREAM_ALLOCATE); std::lock_guard lock(MQuickJsRuntime::GetMutex()); @@ -559,7 +558,7 @@ namespace {CL_CAMERA_AV_STREAM_MGMT, 0xC0} }; auto result = InvokeExecuteHandler( - "webrtc", "localSdp", "test-offer-sdp", sessions, "", featureMaps, OFFERER_ACCEPTED_CMDS); + "webrtc", "localSdp", "test-offer-sdp", sessions, "", featureMaps, CAMERA_ANSWERER_ACCEPTED_CMDS); auto &cmd = ExpectRequestCommand(result, CL_CAMERA_AV_STREAM_MGMT, CMD_VIDEO_STREAM_ALLOCATE); std::lock_guard lock(MQuickJsRuntime::GetMutex()); @@ -577,8 +576,8 @@ namespace TEST_F(SbmdCameraWebrtcTest, ExecuteOfferSdpContextCarriesSdp) { std::string sessions = SessionsJson("1", "streaming"); - auto result = - InvokeExecuteHandler("webrtc", "localSdp", "test-offer-sdp", sessions, "", {}, OFFERER_ACCEPTED_CMDS); + auto result = InvokeExecuteHandler( + "webrtc", "localSdp", "test-offer-sdp", sessions, "", {}, CAMERA_ANSWERER_ACCEPTED_CMDS); auto &cmd = ExpectRequestCommand(result, CL_CAMERA_AV_STREAM_MGMT, CMD_VIDEO_STREAM_ALLOCATE); // Verify context carries the SDP for the chained ProvideOffer @@ -594,15 +593,15 @@ namespace TEST_F(SbmdCameraWebrtcTest, ExecuteOfferSdpMissingSessionReturnsError) { std::string sessions = SessionsJson("1", "created"); - ExpectError( - InvokeExecuteHandler("webrtc", "localSdp", "test-offer-sdp", sessions, "", {}, OFFERER_ACCEPTED_CMDS), - "No active streaming session"); + ExpectError(InvokeExecuteHandler( + "webrtc", "localSdp", "test-offer-sdp", sessions, "", {}, CAMERA_ANSWERER_ACCEPTED_CMDS), + "No active streaming session"); } TEST_F(SbmdCameraWebrtcTest, ExecuteOfferSdpMissingInputReturnsError) { std::string sessions = SessionsJson("1", "streaming"); - ExpectError(InvokeExecuteHandler("webrtc", "localSdp", "", sessions, "", {}, OFFERER_ACCEPTED_CMDS), + ExpectError(InvokeExecuteHandler("webrtc", "localSdp", "", sessions, "", {}, CAMERA_ANSWERER_ACCEPTED_CMDS), "SDP string required"); } @@ -851,7 +850,8 @@ namespace // Drive the offer flow far enough to capture the VideoStreamAllocate requestCommand, then // invoke its onError continuation (handleVideoStreamAllocateError) directly. std::string sessions = SessionsJson("1", "streaming"); - auto offer = InvokeExecuteHandler("webrtc", "localSdp", "dummy-sdp", sessions, "", {}, OFFERER_ACCEPTED_CMDS); + auto offer = + InvokeExecuteHandler("webrtc", "localSdp", "dummy-sdp", sessions, "", {}, CAMERA_ANSWERER_ACCEPTED_CMDS); auto &alloc = ExpectRequestCommand(offer, CL_CAMERA_AV_STREAM_MGMT, CMD_VIDEO_STREAM_ALLOCATE); auto result = InvokeCallback( @@ -871,7 +871,8 @@ namespace // deadline is reported through onError with type 'timeout', so this also covers the // offer-flow timeout path. std::string sessions = SessionsJson("1", "streaming"); - auto offer = InvokeExecuteHandler("webrtc", "localSdp", "dummy-sdp", sessions, "", {}, OFFERER_ACCEPTED_CMDS); + auto offer = + InvokeExecuteHandler("webrtc", "localSdp", "dummy-sdp", sessions, "", {}, CAMERA_ANSWERER_ACCEPTED_CMDS); auto &alloc = ExpectRequestCommand(offer, CL_CAMERA_AV_STREAM_MGMT, CMD_VIDEO_STREAM_ALLOCATE); auto allocRespTlv = EncodeTlv("{videoStreamID:{tag:0,type:'uint16'}}", "{videoStreamID: 5}"); diff --git a/reference/src/cameraCategory.c b/reference/src/cameraCategory.c index 5fee766c..8d968311 100644 --- a/reference/src/cameraCategory.c +++ b/reference/src/cameraCategory.c @@ -543,6 +543,17 @@ static bool runCameraStream(BCoreClient *client, // we answer it; when the camera is the 'answerer' (ProvideOffer flow) we create the offer. Must // be set before the peer starts negotiating. const gchar *role = cameraDeviceSessionGetRole(session); + + // The role read can fail (NULL) or return an unexpected value; bail out with a clear error + // rather than silently defaulting to a role that could drive the wrong signaling flow. + if (g_strcmp0(role, "offerer") != 0 && g_strcmp0(role, "answerer") != 0) + { + emitError("[camera-stream] could not determine the camera's negotiation role (got '%s')\n", + role != NULL ? role : "(null)"); + + return false; + } + gboolean answerer = (g_strcmp0(role, "offerer") == 0); cameraWebrtcClientSetAnswerer(webrtc, answerer);