-
Notifications
You must be signed in to change notification settings - Fork 228
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.
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
Add function to resize the browser source from JavaScript #423
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would love to have more feedback from others, but otherwise I don't mind one way or another. Maybe @Warchamp7 has an opinion on this one?
But here's one super tiny nitpick.
browser-client.cpp
Outdated
obs_data_get_int(s, "height") != height)) { | ||
obs_data_set_int(s, "width", width); | ||
obs_data_set_int(s, "height", height); | ||
obs_source_update(bs->source, nullptr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bs->source
probably should be s
just to be safe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure what you mean as bs->source
is a obs_source_t*
and s
is a obs_data_t*
.
browser-client.cpp
Outdated
obs_data_get_int(s, "height") != height)) { | ||
obs_data_set_int(s, "width", width); | ||
obs_data_set_int(s, "height", height); | ||
obs_source_update(bs->source, nullptr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this isn't really how obs_source_update
is meant to be used.
Sure, you can do it like this, but really you're meant to pass a new obs_data_t
object containing the new settings (in place of the nullptr
), instead of modifying the object obtained by obs_source_get_settings
directly.
And while you're at it, you should probably use the C++-RAII-types (OBSData
/ OBSDataAutoRelease
) in C++-code.
What's the use case for adding a new obs-browser js binding over say using obs-websockets? Is it just the hurdle of connecting to obs-ws? Ever since obs-ws was made a core plugin I'm generally against adding new obs-browser bindings to avoid maintaining essentially two browser source APIs. At the very least, I think a better approach here would be a more abstract system that allows browser sources to dispatch() an event to OBS plugins. Then your plugin could handle the source resizing in response to that event. |
Description
Add function to resize the browser source from javascript
Motivation and Context
Allows browser sources to be sized to exactly fit its contents.
This can be very useful for my Markdown Source plugin.
How Has This Been Tested?
On windows 11 64 bit using Markdown Source with the following javascript:
Types of changes
Checklist: