Skip to content

Add test for input/output audio devices#1686

Merged
sandboxcoder merged 3 commits intostagingfrom
rno/audio-tests
May 5, 2026
Merged

Add test for input/output audio devices#1686
sandboxcoder merged 3 commits intostagingfrom
rno/audio-tests

Conversation

@sandboxcoder
Copy link
Copy Markdown
Contributor

@sandboxcoder sandboxcoder commented May 5, 2026

Description

  • Adds test for OBS_settings_getOutputAudioDevices & OBS_settings_getInputAudioDevices
  • Update test_osn_replaybuffer to properly release resources when a test fails. Was hoping to copy this pattern over to over tests to help ensure there is no orphaned worker threads running during subsequent tests.

Motivation and Context

Aiming for better code coverage

How Has This Been Tested?

Ran locally and on CI build machine

Types of changes

  • Code cleanup (non-breaking change which makes code smaller or more readable)

Checklist:

  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

any call the invokes the background osn::startWorker() will now always
be released at the end of the test to ensure each test run does not
produce an orphaned worker thread if there is an error

afterEach(async function() {
// Destroying created outputs and encoders in case the test failed before they were destroyed in the test itself
const streamEncoder = recording?.videoEncoder;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This cleanup only releases recording?.videoEncoder, but in the test Start advanced replay buffer - Use Stream through Recording the encoder is assigned to stream.videoEncoder and the JS recording.videoEncoder wrapper is never set. Please capture and release stream?.videoEncoder as well.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Additionally, I wonder why you chose this naming in:

const streamEncoder = recording?.videoEncoder;

The name streamEncoder assumes that all video encoders assigned to recording are used for streaming. Is it generally true?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, addressed in commit fbe7d15, thanks

@sandboxcoder sandboxcoder merged commit 3e13fce into staging May 5, 2026
19 checks passed
@sandboxcoder sandboxcoder deleted the rno/audio-tests branch May 6, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants