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

libobs: Add missing encoder API functions #10648

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

Conversation

Error-String-Expected-Got-Nil
Copy link

Description

Adds the API functions obs_encoder_get_signal_handler and obs_encoder_get_proc_handler, both of which are mentioned in the API docs but are not actually implemented in the API. Encoders don't use signals for anything, but it was easy enough to include that I didn't see a reason not to.

Motivation and Context

Encoders already have a proc and signal handler stored internally, it simply isn't externally accessible due to the absence of these functions. As such, implementing custom extra functionality for encoders is somewhat difficult at present. There's minimal cost to adding these functions, and some nice possibilities opened up for plugin creators and future feature additions.

How Has This Been Tested?

Created an encoder in a plugin's load function, used function to get its proc handler, and found it did correctly return the encoder's proc handler.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

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.

@Fenrirthviti
Copy link
Member

If this function doesn't really serve any practical purpose, why would you add the function instead of correcting the documentation? What is the actual use case for this?

@tt2468
Copy link
Member

tt2468 commented May 6, 2024

Encoders might have custom functionality via procs/signals that one may want to implement. I can think of some ideas in my head which would necessitate usage of these functions. The proc/signal handlers for encoders are already there, just not accessible via the frontend API like they are with most other OBS objects.

@Error-String-Expected-Got-Nil
Copy link
Author

If this function doesn't really serve any practical purpose, why would you add the function instead of correcting the documentation? What is the actual use case for this?

I'm making this PR specifically because I wanted to use one of these functions in a plugin I'm making and it wasn't available, so yes, I think there is a use case.

@Fenrirthviti
Copy link
Member

Can you provide an example?

To be clear, I don't want to imply I'm against this, I'm mostly just curious the use case, and having a practical example will help in review for others.

@Error-String-Expected-Got-Nil
Copy link
Author

Can you provide an example?

To be clear, I don't want to imply I'm against this, I'm mostly just curious the use case, and having a practical example will help in review for others.

Mine is a little weird, but the plugin I'm making has an encoder that has a counter for a number of frames I want it to encode before it stops. The easiest way to adjust that counter would be to use a procedure.

Encoder API was missing the functions `obs_encoder_get_signal_handler`
and `obs_encoder_get_proc_handler`, despite them being listed in the
API. This adds both (even though encoders don't have signals).
@WizardCM WizardCM added the Bug Fix Non-breaking change which fixes an issue label May 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Fix Non-breaking change which fixes an issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants