Skip to content

fix(camera): report the camera's negotiation role, not the client's #256

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cleithner-comcast merged 2 commits into from
Aug 4, 2026
Merged

fix(camera): report the camera's negotiation role, not the client's #256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a97089e
fix(camera): report the camera's negotiation role, not the client's
cleithner-comcast Aug 3, 2026
e33dc71
fix(camera): harden negotiation-role handling per review
cleithner-comcast Aug 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 38 additions & 35 deletions core/deviceDrivers/matter/sbmd/specs/camera.sbmd.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,10 @@
// 3. Follow entryPoint to the protocol-specific endpoint
// (e.g., /<id>/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
Expand Down Expand Up @@ -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',

Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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');
}
Expand Down
131 changes: 112 additions & 19 deletions core/test/src/SbmdCameraWebrtcTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
* 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, tags), error paths
* - executeLocalIceCandidates: valid JSON array → sendCommand, invalid JSON → error
* - handleIncomingOffer / handleIncomingAnswer / handleIncomingIceCandidates / handleIncomingEndSession
* - executeDestroySession with streaming session: sends EndSession command
Expand Down Expand Up @@ -70,10 +71,16 @@ 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==";
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 *CAMERA_OFFERER_ACCEPTED_CMDS = "FgQAGA==";

// ========================================================================
// Test Fixture — loads the real camera.sbmd.js via SbmdDriver
Expand Down Expand Up @@ -279,6 +286,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<ParsedResult> InvokeReadHandler(const std::string &endpointId,
const std::string &resourceId,
const std::string &acceptedCmdsBase64)
{
auto hctx = MakeContext();
std::lock_guard<std::mutex> lock(MQuickJsRuntime::GetMutex());

const SbmdHandler *readHandler = nullptr;
const auto &reg = 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<std::string> {
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.
*/
Expand Down Expand Up @@ -358,15 +427,37 @@ namespace

std::unique_ptr<SbmdDriver> 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", CAMERA_OFFERER_ACCEPTED_CMDS);
ExpectSuccess(solicit);
EXPECT_EQ(std::get<ResultTerminal::Success>(solicit->terminal.data).value, "offerer");

// ProvideOffer only: the camera answers the client's offer, so its role is 'answerer'.
auto provide = InvokeReadHandler("webrtc", "negotiationRole", CAMERA_ANSWERER_ACCEPTED_CMDS);
ExpectSuccess(provide);
EXPECT_EQ(std::get<ResultTerminal::Success>(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<ResultTerminal::Success>(def->terminal.data).value, "offerer");
}

// ========================================================================
// 5.1 — executeOfferSdp
// ========================================================================

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);
Expand All @@ -380,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<std::mutex> lock(MQuickJsRuntime::GetMutex());
Expand All @@ -398,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<std::mutex> lock(MQuickJsRuntime::GetMutex());
Expand Down Expand Up @@ -467,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<std::mutex> lock(MQuickJsRuntime::GetMutex());
Expand All @@ -485,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
Expand All @@ -502,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");
}

Expand Down Expand Up @@ -759,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(
Expand All @@ -779,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}");
Expand Down
2 changes: 2 additions & 0 deletions openspec/changes/archive/2026-08-03-negotiation-role-camera-perspective/.openspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-08-03
Loading
Loading