Handles for addressing scenes, inputs, sources, scene items and filters - #31
Merged
Conversation
66 of the 147 requests address something by a name-or-uuid pair, and the
protocol never says so: it repeats an optional {X}Name beside an
optional {X}Uuid. Reading that shape back out gives five operation
types, so the set follows a protocol refresh instead of being
transcribed. DuplicateSceneItem's second scene falls out of the rule
without anyone having thought about it.
Not overloads of the existing methods: M(XRequestData) and M(XHandle)
are ambiguous the moment a caller writes M(new(...)), and that idiom is
all over the surface and the README.
…ween handles client.SceneItem(logo).SetSceneItemEnabledAsync(false) says scene item twice, once in the thing addressed and once in the verb. The operations types name the request without it, and the protocol name stays in the documentation and on the category group. The set is named at once so a collision falls back to full names rather than one request shadowing another; none collide today. Navigation covers what the protocol does not describe: a scene contains items, an input carries filters, and both are sources.
Found by the request sweep: GetOutputList became unreadable because an idle virtual camera reported outputHeight as 2586032160. The field is uint32_t in libobs and obs-websocket passes it through unclamped, so an int could not hold it, and one bad field fails the whole response rather than just itself. Swept the rest of the numeric table against the obs-websocket and obs-studio sources. A field is safe as an int only where obs-websocket validates a range; where it copies out of libobs or a settings blob the C type decides. That reclassifies: outputWidth/Height (stub) uint32_t, unclamped render/output frames uint32_t, monotonic webSocketSession messages uint64_t sceneItemId int64_t, validated >= 0 with no upper bound transitionDuration int64_t out of private settings alignment/boundsAlignment uint32_t, validated to the full range sceneIndex becomes nullable for the same reason: GetCanvasSceneList has no index to report and sends null. Removes FindSceneItemIdInt32Async, which named a return type it no longer has.
The handle API had no entry in the README at all, so nothing pointed at it. Adds a table up front for choosing between handles, the category groups and CallAsync, then a section on handles themselves. Trims what was explaining implementation rather than behaviour: the options-validation note, the non-null response note, and the parallel batch section, which spent ninety lines on an upstream labelling bug.
The interactive commands all addressed things by name through the category groups, so the handle API had no demonstration. Moves the ones that gain from a handle: mute, list-filters, toggle-filter, the blend mode in add-browser-source, and watch, which now acts on the uuid the event already carries. Adds resolve, for the round trip and what a miss reports. set-text and get-input-settings stay on the group deliberately, because the typed-settings helpers are hand-written and so have no handle form. That contrast is the point; converting everything would hide the rule. The settings-mode checks now create their browser source and gain filter and remove them again. Discovering an existing input made the result depend on the machine: a fresh OBS reported the modes as failing when all that was missing was a source to try them on.
The stubs were written from what one OBS instance happened to return, so
fields that instance did not exercise were never typed. Diffing every
stub against the C++ that builds the JSON turns up eight:
SceneItemStub inputKind, sourceType, sceneItemBlendMode,
sceneItemBlendMethod
InputStub inputKindCaps
OutputStub outputFlags
TransformStub cropToBounds
They were landing in ExtensionData, so nothing failed, but none of them
were reachable as typed members.
OutputStub also carried an outputSettings member that GetOutputList never
sends; settings come from GetOutputSettings, per output.
inputKind and isGroup are nullable because OBS sends null for the case
that does not apply, and sceneItemBlendMethod because OBS 32.2.2 does not
send it at all.
CI checks formatting and the last three commits were not formatted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft. Verified against a live OBS 32.2.2 on both transports.
Addresses #19.
What it does
Why a handle is the accurate type
From
Request::AcquireSourceupstream: a uuid wins outright, a name is read only when no uuid was sent,canvasUuidis consulted only on the name path, and neither field present isMissingRequestField. So todaynew SetCurrentProgramSceneRequestData()compiles and fails at runtime, and sending both silently ignores the name.A handle is that choice made once. It holds identity only, never cached state.
Generated, not transcribed
The protocol never says "this request takes a scene". It says the request has an optional
sceneNamebeside an optionalsceneUuid, repeated across 68 requests. Reading that shape back out gives 66 requests across five operation types: Input 30, SceneItem 13, Scene 11, Filter 7, Source 5.DuplicateSceneItem's second scene falls out of the rule as adestinationSceneparameter without anyone having thought about it, which is the test that it is a rule and not a list.The same reading of events and responses gives 47 handles that cost no round trip, including
CreateSceneandCreateInput, which already answer with a uuid.Two decisions worth reviewing
These are not overloads of the existing methods. That was the first attempt and it broke 48 call sites:
M(XRequestData)andM(XHandle)are ambiguous the moment a caller writesM(new(...)), and that idiom is all over the surface and the README.Guid, not a wrapper struct. OBS writes RFC 4122 uuids, soGuid's"D"format matches exactly. A rawGuiddoes not distinguish a scene from an input, but OBS does not either:obs_get_source_by_uuidtakes any source uuid, andAcquireScenecatches the mismatch withInvalidResourceType. Handles carry the uuid as the wire string and never parse it, so a value OBS sends that is not a Guid cannot break a response.Numbers OBS does not bound
The request sweep caught
GetOutputListfailing outright: an idle virtual camera reportedoutputHeightas 2586032160. It comes fromobs_output_get_width/_height, which areuint32_tand pass through unclamped, so anintcould not hold it — and one bad field fails the whole response, not just itself. Intermittent, and only for whoever has a virtual camera or a capture plugin installed.That prompted a sweep of the whole numeric table against the obs-websocket and obs-studio sources. The rule that came out of it: a field is safe as an
intonly where obs-websocket validates a range; where it copies a value out of libobs or a settings blob, the C type decides. Reclassified:outputWidth/outputHeight(stub)uint32_t, unclampedrender*Frames,output*Framesuint32_t, monotonic — passes int32 at ~414 days at 60fpswebSocketSession*Messagesuint64_tsceneItemIdint64_t, validated>= 0with no upper boundtransitionDurationint64_tout of private settings, not revalidated on readalignment,boundsAlignmentuint32_t, validated to0 .. uint32_t maxsceneIndexbecomes nullable for the same class of reason:GetCanvasSceneListenumerates through a callback with no index to report and sends null.Everything else stayed
inton evidence, not assumption — the resolutions are validated 8..4096, the indices 0..8192, and the monitor fields are Qtint.sceneItemIdwidening tolongis the one API-visible cost, and it is why this is in the handle PR rather than a follow-up:SceneItemHandleis built around that type, and changing it now costs a compile error at ~20 call sites instead of a break after release.Verified against OBS 32.2.2, both transports
The rename check is the one that matters:
Also live: a scene item resolving by source name and navigating back to its scene, and a failed lookup naming what does exist. That last one is free, because resolving already fetched the list.
The settings-mode checks now build their own browser source and gain filter and remove them again, so a fresh OBS install runs the same suite. Verified by clearing OBS to a single input first: 0 failures, both transports, and nothing left behind afterwards.
340 tests, 0 warnings, full forced regeneration clean, zero unreadable payloads across the request sweep.
Not done
SetCurrentProgramAsyncis what the naming rule produces fromSetCurrentProgramScene. Mechanically right, reads slightly oddly.CanvasOperations: the canvases category holds onlyGetCanvasList, so there is nothing to scope.CanvasHandlestill exists, because it scopes scene and source names.