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

prgmitchell commented 1 week ago

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:

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

Penwy commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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.

CyBeRoni commented 1 week ago

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 commented 1 week ago

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.

obsproject / obs-browser