Skip to content
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

docs: Clarify obs_source_update can accept NULL for settings argument #11189

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

norihiro
Copy link
Contributor

Description

This PR adds the parameter description of the API obs_source_update.

Also fixed a gramatical error; will be not be called --> will not be called.

Motivation and Context

In the implementation, settings can be NULL and it will be just ignored if it is NULL.

if (settings) {
obs_data_apply(source->context.settings, settings);
}

Passing NULL as the settings argument is useful to trigger update callback.

I realized AJA source calls obs_source_update with the existing settings. This is redundant, we can just pass NULL.
https://github.com/obsproject/obs-studio/blob/master/plugins/aja/aja-source.cpp#L327

How Has This Been Tested?

OS: Fedora 39

Called obs_source_update with the NULL argument and confirmed the settings was not changed.

Types of changes

  • Documentation (a change to documentation pages)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

Sidenote:

When passing a valid obs_data_t object, elements that exists in the current settings but does not exist in the passed object are not removed.
This is because obs_source_update traverse each element in the given obs_data_t object and apply it to the current settings.
It would be good for the document to describe this behavior. However, I don't have a good idea to phrase this in concise sentence.

@gxalpha
Copy link
Member

gxalpha commented Aug 26, 2024

I would argue that if you’re calling this with NULL, you’re probably doing something in unintended ways. Personally I hate that it’s even possible to update source settings without calling obs_source_update (e.g. by directly changing the settings returned via obs_source_settings).

@norihiro
Copy link
Contributor Author

We have already had a usage in the image slideshow, which is recently added by Lain.

obs_source_update(source, NULL);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants