Skip to content

docs: Add examples and undocumented callback funcs#10756

Open
CodeYan01 wants to merge 1 commit into
obsproject:masterfrom
CodeYan01:docs4
Open

docs: Add examples and undocumented callback funcs#10756
CodeYan01 wants to merge 1 commit into
obsproject:masterfrom
CodeYan01:docs4

Conversation

@CodeYan01
Copy link
Copy Markdown
Contributor

Adds the following undocumented functions:

  • calldata_set_data
  • calldata_get_int
  • calldata_get_float
  • calldata_get_bool
  • calldata_get_ptr
  • calldata_get_string
  • calldata_get_data

Description

Also adds details and examples on the following functions
image

image

image

image

image

image

image

image

image

image

Disclaimer: For the additional text under the Common Source Signals, Source-specific signals and procedures, and Core OBS Signals, I first wrote my own description for it, and had ChatGPT assistance for grammar and conciseness improvements, and manually reviewed its responses, not using them blindly.

Motivation and Context

Undocumented functions, and unclear how to use source signals and procs. Let's lower the barrier to entry for developers.

How Has This Been Tested?

Built the docs and viewed locally. Code examples aren't fully tested, but i wrote those first in an ide (for syntax highlighting and code completion, ensuring the functions exist) and having references in the obs source code that i merely adapted. I've also used these functions before so I know them.

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.

Copy link
Copy Markdown
Member

@RytoEX RytoEX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some notes. Would appreciate a review from someone else who is more familiar with these functions.

Comment thread docs/sphinx/reference-libobs-callback.rst Outdated
Comment thread docs/sphinx/reference-libobs-callback.rst Outdated
Comment thread docs/sphinx/reference-libobs-callback.rst Outdated
Comment thread docs/sphinx/reference-libobs-callback.rst Outdated
Comment thread docs/sphinx/reference-libobs-callback.rst Outdated
Comment thread docs/sphinx/reference-libobs-callback.rst Outdated
Comment thread docs/sphinx/reference-libobs-callback.rst
@CodeYan01
Copy link
Copy Markdown
Contributor Author

CodeYan01 commented Jun 10, 2024

Applied requested changes, thanks

Adds the following undocumented functions:
- calldata_set_data
- calldata_get_int
- calldata_get_float
- calldata_get_bool
- calldata_get_ptr
- calldata_get_string
- calldata_get_data
Copy link
Copy Markdown
Member

@RytoEX RytoEX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me, though would still appreciate a second opinion from someone more familiar with the usage of these items.

@RytoEX
Copy link
Copy Markdown
Member

RytoEX commented May 8, 2026

Maybe cc @gxalpha or @FiniteSingularity

Copy link
Copy Markdown
Member

@gxalpha gxalpha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems mostly fine. A few thoughts attached.

.. function:: void calldata_set_ptr(calldata_t *data, const char *name, void *ptr)

Sets a pointer parameter.
Sets a pointer parameter. This does not have to be a pointer to a pointer.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems superfluous to me.

----------------

Use :c:func:`signal_handler_connect()` to connect callbacks to the following
core OBS signals:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
core OBS signals:
global core libobs signals:

:param data: Calldata structure
:param name: Parameter name
:param in: Pointer to the value to be stored
:param new_size: Size of the value to be stored
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @RytoEX "of value to be stored" sounds off to me, but I'm not a native English speaker. Is this correct?

:param data: Calldata structure
:param name: Parameter name
:param val: Pointer to an integer receiving the parameter
:return: ``true`` if the stored value is successfully cast
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO "is successfully cast" implies more type safety guarantees than there actually are. I guess saying it does a cast is technically correct, but since it doesn't check for anything other than the size of the data (unlike e.g. an std::variant), I find "successful" a bit misleading.

I would just do something like this:

Suggested change
:return: ``true`` if the stored value is successfully cast
:return: ``true`` on success, ``false`` otherwise


.. function:: bool calldata_get_int(const calldata_t *data, const char *name, long long *val)

Gets an integer parameter. See :c:func:`calldata_int` for a simpler call.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just this?

Suggested change
Gets an integer parameter. See :c:func:`calldata_int` for a simpler call.
Gets an integer parameter. See also :c:func:`calldata_int`.

Alternatively, explain the use cases for the respective functions in the docs for both functions. See the relevant header file:

/* ------------------------------------------------------------------------- */
/* call if you know your data is valid */
static inline long long calldata_int(const calldata_t *data, const char *name)
{

Comment on lines +328 to +361
Example connecting to ``rename`` signal from :ref:`source_signal_handler_reference`,
declared as ``void rename(ptr source, string new_name, string prev_name)``:

.. code:: cpp

/* Sample source data */
struct my_source {
obs_source_t *source;
char *prev_name;
...
};

void rename_cb(void *data, calldata_t *cd) {
struct my_source *ms = data;
obs_source_t *source = calldata_ptr(cd, "source");
/* We could also just access the source from our source data */
source = ms->source;
const char *new_name = calldata_string(cd, "new_name");
const char *prev_name = calldata_string(cd, "prev_name");

/* Do processing here */

/* `prev_name` will be freed after the callback, so if we want a
* reference to it after the callback, we must duplicate it.
*/
bfree(ms->prev_name);
ms->prev_name = bstrdup(prev_name);
}

/* Assuming `ms` already contains our source data */
struct my_source *ms;
signal_handler_t *sh = obs_source_get_signal_handler(ms->source);
signal_handler_connect(sh, "rename", rename_cb, ms);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a fan of how long this example is. I think it should be fine to cut it down a bit:

Suggested change
Example connecting to ``rename`` signal from :ref:`source_signal_handler_reference`,
declared as ``void rename(ptr source, string new_name, string prev_name)``:
.. code:: cpp
/* Sample source data */
struct my_source {
obs_source_t *source;
char *prev_name;
...
};
void rename_cb(void *data, calldata_t *cd) {
struct my_source *ms = data;
obs_source_t *source = calldata_ptr(cd, "source");
/* We could also just access the source from our source data */
source = ms->source;
const char *new_name = calldata_string(cd, "new_name");
const char *prev_name = calldata_string(cd, "prev_name");
/* Do processing here */
/* `prev_name` will be freed after the callback, so if we want a
* reference to it after the callback, we must duplicate it.
*/
bfree(ms->prev_name);
ms->prev_name = bstrdup(prev_name);
}
/* Assuming `ms` already contains our source data */
struct my_source *ms;
signal_handler_t *sh = obs_source_get_signal_handler(ms->source);
signal_handler_connect(sh, "rename", rename_cb, ms);
Example connecting to ``rename`` signal from :ref:`source_signal_handler_reference`:
.. code:: cpp
obs_source_t *source = ...;
void rename_cb(void *data, calldata_t *cd) {
obs_source_t *source = calldata_ptr(cd, "source");
const char *new_name = calldata_string(cd, "new_name");
const char *prev_name = calldata_string(cd, "prev_name");
/* Do processing here */
}
signal_handler_t *sh = obs_source_get_signal_handler(source);
signal_handler_connect(sh, "rename", rename_cb, NULL);

If you want to emphasize the lifetime of the calldata_t object, just a comment like this below /* Do processing here */:

/* Keep in mind that any pointers from ``cd`` are only valid in the scope of the callback */

Comment on lines +483 to +500
/* Sample source data */
struct my_source {
obs_source_t *source;
bool active;
...
};

static void proc_activate(void *data, calldata_t *cd)
{
struct my_source *ms = data;
ms->active = calldata_bool(cd, "active");
}

/* Assuming `ms` already contains our source data */
struct my_source *ms;
proc_handler_t *ph = obs_source_get_proc_handler(ms->source);
proc_handler_add(ph, "void activate(bool active)",
proc_activate, ms);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shorter version:

Suggested change
/* Sample source data */
struct my_source {
obs_source_t *source;
bool active;
...
};
static void proc_activate(void *data, calldata_t *cd)
{
struct my_source *ms = data;
ms->active = calldata_bool(cd, "active");
}
/* Assuming `ms` already contains our source data */
struct my_source *ms;
proc_handler_t *ph = obs_source_get_proc_handler(ms->source);
proc_handler_add(ph, "void activate(bool active)",
proc_activate, ms);
obs_source_t *source = ...;
static void proc_activate(void *data, calldata_t *cd)
{
bool active = calldata_bool(cd, "active");
/* Do processing here */
}
proc_handler_t *ph = obs_source_get_proc_handler(source);
proc_handler_add(ph, "void activate(bool active)", proc_activate, NULL);

Comment on lines +462 to +466
Adds a procedure to a procedure handler. Will fail if the declaration string
has invalid syntax or a procedure with the same name was already added. Other
than the function identifier, the ``decl_string`` is mostly for
documentation, as libobs does not strictly enforce how the parameters are
handled.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove anything after "Will fail if the declaration string has invalid syntax or a procedure with the same name was already added.", I'd say the missing verification is more of a somewhat unfortunate implementation detail.

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.

5 participants