Conversation
Commit obsproject/obs-studio@a4f3c16 implemented this upstream, so since obs-websocket follows that util file, we should do the same.
This is a slight behavior change, in that in the event of undefined video info for the main canvas, it will no longer throw an error, and will instead return values as `null`. This, however, brings the behavior in-line with the GetCanvasList, so I see that as preferred.
Uses an object of individual boolean values rather than the raw bitflag that libobs uses internally. This is much easier for users to grasp without needing to look up source code.
|
Tagging @derrod and @exeldro for visibility. I removed the canvas fields from the non-canvas events and added filters to prevent non-main canvas events from triggering them. We need to do an RPC version bump in order to expose events from non-main canvases. Additionally, I think that emitting events for non-main canvases in the normal OBS global signal handlers should be blocked. Those global signal handlers should really just emit events for the main canvas, and it should require using a canvas' signal handler in order to receive events for that canvas. |
|
@Warchamp7 asked to use canvas as category instead of canvases With this change the scene item events don't exist for extra canvases, which is required in my opinion. If an other way is preferred I would like to see the other solution, not only the previous solution removed. Removing canvasName from the calls I don't like, using the name makes doing a quick test much easier. |
|
initial thoughts here: I'm not too bothered by removing canvasName from function calls. While it makes quick testing easier than UUID, it's an antipattern you don't want to actively teach people to use. I'm definitely missing the scene and scene item events for other canvases. Would be nice for those to be available again, maybe under new events so it doesn't mess with existing setups. Will perform further testing when able. |
Per an off thread discussion with @tt2468 this would be a breaking change to the API. It will require changes to libobs to support those events correctly and then an update to websockets with an RPC change.
The plan is to remove |
Warchamp7
force-pushed
the
fix/canvases
branch
2 times, most recently
from
4da7d5d to
812fac8
Compare
|
I will fix some of the wonky commit authorship tomorrow. |
Co-authored-by: Warchamp7 <warchamp2003@hotmail.com>
Warchamp7
force-pushed
the
fix/canvases
branch
from
812fac8 to
e85b609
Compare
Attempt to resolve sourceUuid first, now that it is the recommended approach to resolving a source. Then, try via canvas. This makes the uuid code path much faster. The stacked logic of being able to pass a name or uuid for both canvases AND sources was just too much, and so if people want to pass a canvas, they'll need to just suck it up and use UUIDs
Just a visual change. These function more as getters than validators.
A previous commit of mine within this PR made GetCanvas() return a reference to the main canvas if `canvasUuid` was not specified. As such, that removed the need to check the requestStatus value, and we can just rely on the canvas ptr being != nullptr. This also fixes a theoretical uninitialized memory access from `statusCode` being unset.
Canvases don't have ordered scenes, so it's better to return null values than to lie to the user.
Existing clients often track scene state based on name, and emitting events for non-main canvases under the existing event names is an API break. The correct solution is to increment the obs-websocket RPC version, in order to require clients to explicitly acknowledge that they have been designed to take into account the context of canvases. In order to preserve consistent behavior, we will instead ignore events from canvases other than the main canvas.
Just a visual change. "Acquire" is used to indicate that these methods return resources with an incremented ref.
Warchamp7
force-pushed
the
fix/canvases
branch
from
e85b609 to
23ea344
Compare
5 participants
Description
Fixes a number of issues and nitpicks related to canvas support in obs-websocket. Both adds needed functionality and removes some functionality which still requires further thought.
Motivation and Context
We want obs-websocket features to be consistent and behave intuitively.
How Has This Been Tested?
Tested OS(s): Compiles on Ubuntu 24.04
Have not tested every single commit, but things appear to function.
Types of changes
Checklist:
masteror arelease/*branch.