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

obs-browser: Enable "Control audio via OBS" by default #445

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

Conversation

prgmitchell
Copy link
Member

@prgmitchell prgmitchell commented Jun 30, 2024

Description

Changes the default behavior for a new browser source to tick the "Control audio via OBS" box by default. This is a simple change so I thought it'd be better to just PR and have the discussion happen here to get thoughts from everyone.

EDIT: This has been converted to a draft PR as I will need to handle settings migration properly, holding off on this until further discussion is had.

Motivation and Context

When this behavior was first set, there were no other built-in audio output capture options apart from capturing an entire device (Desktop Audio). Since v28, Application Audio Capture sources have been available and they only continue to grow in use with users opting to disable Desktop Audio entirely. Because of this it is not very clear to a user who has added a browser source and can hear it themselves why it is not being heard in their output when Desktop Audio is disabled. This change is not only in line with that, but also follows the same behavior other sources exhibit such as the media source.

How Has This Been Tested?

Added a new source, the box was ticked.

Types of changes

  • Tweak (non-breaking change to improve existing functionality)

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.

Set the "Control audio via OBS" box to be ticked by default for new browser sources.
@Penwy
Copy link

Penwy commented Jun 30, 2024

The main objection I'd have to this PR is the fact that there is a bug in windows causing browser source audio to stutter when this is ticked and the obs window minimized, which is, in my opinion, enough to deter from having it default.

A second minor objection is that, unless something is done for migration, as it is right now it risks breaking a fair amount of setups.

I do however agree fully on the motivation.

@prgmitchell
Copy link
Member Author

The main objection I'd have to this PR is the fact that there is a bug in windows causing browser source audio to stutter when this is ticked and the obs window minimized, which is, in my opinion, enough to deter from having it default.

I was under the impression that this had been either fixed or at least very mitigated in the last release (23H2) of Windows 11. I previously could reproduce the issue on my system and was pretty active with the original discussion/issue and after the release of 23H2 have no longer been able to.

A second minor objection is that, unless something is done for migration, as it is right now it risks breaking a fair amount of setups.

There is a chance I am misunderstanding something here but what sort of migration would there be? This is not impacting any existing sources that have been added, it is the default behavior for new sources. Once again, there is a chance I am misunderstanding as I'm slowly familiarizing myself with the codebase.

@Fenrirthviti
Copy link
Member

I am still against this change as a default, as it means all browser sources are (to the user) muted by default since they're sent to output only, which is a significant behavioral change. Users will have to enable monitoring, and monitoring is a fairly... unreliable feature at the moment.

However, since this is a defaults change, this shouldn't (assuming it is accounting for it) have any impact on existing sources, so I'm less concerned about that aspect.

@Penwy
Copy link

Penwy commented Jun 30, 2024

  • I definitively saw cases of it on 23H2, but I cannot speak to the frequency of it.
  • Afaik, source defaults are not only applied at source creation, but also used for any settings which doesn't have a value assigned to it in the source settings, whcich means any settings that hasn't been modified by the user yet. With this, any source for which the "control via obs" hasn't been touched since its creation would switch from not controlled to controlled.

@prgmitchell
Copy link
Member Author

prgmitchell commented Jun 30, 2024

Afaik, source defaults are not only applied at source creation, but also used for any settings which doesn't have a value assigned to it in the source settings, whcich means any settings that hasn't been modified by the user yet. With this, any source for which the "control via obs" hasn't been touched since its creation would switch from not controlled to controlled.

Ah, seems I misunderstood and this is correct. In that case if this is considered I would definitely need to account for existing sources too. With that in mind, I'd still like to keep this open as-is for the discussion around it before making any further changes. In the meantime I have made this a draft PR.

@prgmitchell prgmitchell marked this pull request as draft June 30, 2024 19:20
@CyBeRoni
Copy link

CyBeRoni commented Jun 30, 2024

as it means all browser sources are (to the user) muted by default since they're sent to output only, which is a significant behavioral change.

But this is true for other sources as well (e.g. media source, video capture device, etc.) so it would actually be more consistent overall.

In addition, someone adding a browser source but who is not capturing desktop audio would have their source muted from the output instead which is arguably worse.

@Fenrirthviti
Copy link
Member

But this is true for other sources as well (e.g. media source, video capture device, etc.) so it would actually be more consistent overall.

While true, I'd argue that the amount of people who get confused by that is justification enough to not use that as the standard.

However, all the above aside, this is a UX decision that @Warchamp7 will need to decide. Most of the team is on a well-needed vacation at the moment, so it may be a few weeks until this is looked at.

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.

5 participants