obs-studio icon indicating copy to clipboard operation
obs-studio copied to clipboard

UI: Add headphone checkbox to mixer

Open cg2121 opened this issue 8 years ago • 43 comments

This adds a headphone icon to the mixer area. It replaces the audio monitor selection in the advanced audio properties.

screenshot from 2018-04-06 14-34-48

cg2121 avatar Apr 06 '18 20:04 cg2121

OMG, I was totally ready to do the same. I'm glad I checked the existing PRs first. I'm going to PR into this PR to fix the left margin and try to make the obs_monitoring_type boolean.

UPD: The screenshot lies, margins are totally OK

image

Himura2la avatar Apr 24 '18 17:04 Himura2la

I fixed the margin, just didn't update the image. I added the output to monitor only in the right click menu of the mixer.

cg2121 avatar Apr 24 '18 23:04 cg2121

@cg2121 This appears to require a rebase.

RytoEX avatar Apr 25 '18 10:04 RytoEX

I don't think that this is good idea. Is this "Add headphone checkbox to mixer" or Move monitoring controls to the mixer?

Also, converting three (and more) states UI elements into two becomes feature of the application...

Edit: For example, I don't understand is this normal to have headphones sign always in OFF state or I did something wrong and it should be fixed.

SuslikV avatar Apr 27 '18 16:04 SuslikV

@SuslikV Call it as you like, but monitoring control Must be easily accessible, I'm very new to OBSS and I was shocked how inconvenient the monitoring is now. I was forced to google how to do it and swear each time I add a new source.

Two buttons determines four states, three of which are defined in the monitoring_type enum. It's way more intuitive than the drop-down in Advanced Configuration. Monitoring is an essential feature, why Advanced?

(Sorry, I really hate the way it's done now)

Himura2la avatar Apr 27 '18 16:04 Himura2la

We could have it a three-state widget or something. I kind of wish we could communicate to the user more easily what mode it's in so they don't get confused by the "monitor but mute output" mode. Tooltip? Status bar message? Context menu?

Additionally, this could be put as three menu items in the "configure" context menu instead, and the options and what they do would become more clear.

Just throwing out ideas.

jp9000 avatar Apr 27 '18 16:04 jp9000

There's no need in the third state, all states are easily covered by two Boolean widgets. Check my PR into this PR https://github.com/cg2121/obs-studio/pull/2
Monitor-and-mute is easily recognizable by red speaker icon with cross

Himura2la avatar Apr 27 '18 17:04 Himura2la

I suppose that's true, if you monitor then mute then that would basically do the same thing as the third state.

jp9000 avatar Apr 27 '18 17:04 jp9000

I built it for Windows to send my friend for testing. This build is made from the https://github.com/cg2121/obs-studio/pull/2/commits/3a4d175bfb8fcb4fcf7d25576bac59054856f554 commit merged with the current master (dcbad4af).

You guys may also test it to ensure it's convenient! MinSizeRel.zip

Himura2la avatar Apr 27 '18 21:04 Himura2la

Pretty useful button for live performances of any kind) Love it)

Alex-Dash avatar Apr 27 '18 21:04 Alex-Dash

@Himura2la Users struggling to select right tracks to record them all at once... Two and more switches makes any task harder to complete. For example, when I see headphones sign - I think that headphones are connected. If only this button could bring up window to select the device.

I think, speaker - OFF should mute the audio and monitoring should reflect the current behavior of the sound (muted) and shouldn't change its own type.

You want to place this feature (that itself has config at Advanced section of the Settings) to the main UI - then add tool tip - it is unknown where to find the monitoring device settings. At least, when it was at advanced properties, the corresponding part was in advanced settings.

My view.

I think of three options:

  • audio monitoring disabled = Off (default)
  • monitor audio but do not output it = Listen
  • monitor audio and output it = Listen + Output

The button: listen-obs may act like gear icon button - bring up menu with 3 radio boxes mentioned above.

I think, there is no need to remove the monitoring controls from Advanced Audio Properties.

P.S. listen-obs-svg-source.zip if needed.

SuslikV avatar Apr 27 '18 21:04 SuslikV

@SuslikV I agree we need a tooltip.

I don't agree that muting should disable monitoring. It's a comon task to listen to the output before sending it live. And monitoring should not be linked with stream, 'cause it's just two different audio targets, why the heck to link them? I want to monitor -- I start monitoring, I'm ready to stream -- I unmute... I just don't get it, why stream muting should prevent monitoring, and we should overcomplicate the UI to enable monitoring-only as a separate state... It's not a special state, it's just stream==off && monitor==on (users never know how it's made in the code [it's strange, I tried to fix this])

I believe monitoring feature must be one click away, because it's essential. Hiding it in context menus and/or doubling in advanced settings does not feel good for me.

And we need more opinions, especially from the decision-making guys ))

Himura2la avatar Apr 27 '18 21:04 Himura2la

@Himura2la Users will decide. And, to be clear, I said that "mute" shouldn't change monitoring type.

SuslikV avatar Apr 27 '18 22:04 SuslikV

@SuslikV

I think, speaker - OFF should mute the audio and monitoring....

Sorry, I stopped understanding this paragraph on ellipsis, and the meaning I got was probably incorrect. (you can explain it in Russian if it's more convenient for you). Anyway, your idea of three states involves the Listen and Listen + Output options, but the output is controled by the Mute button. How are they supposed to live together? Like how it is now? I strongly disagree to control output in two places, it's dangerous and may easily cause errors. I've got to be sure that the sound goes to stream when unmuting and does not when muting. No other hidden options should interfere this.

Users struggling to select right tracks to record them all at once... If only this button could bring up window to select the device.

OBS Studio is widely used for live streaming where the timing is EXTREMELY important. It's very easy to learn what two bottons does and very difficult to fight tons of windows menus and dialogs when the timing is on fire. I think my next contribution would be putting the Target Bitrate setting to the main window, because I should never need to invoke settings while straming.

As for the icon, your option seems way less elegant from the design point of view, I'd better stick with the headphones....

As for monitoring device, I was also wondering about this, seems like it's only possible to use the default output now.

Aaaaand, the good news! I added the tooltip in https://github.com/cg2121/obs-studio/pull/2/commits/24948593332668804a09a4e04202ef95e25dd86d image

Himura2la avatar Apr 27 '18 22:04 Himura2la

@Himura2la In Russian, it will be too rude to understand, I think. I will try in English.

Look,

if (mute_checked) {
		obs_source_set_monitoring_type(source,
				OBS_MONITORING_TYPE_MONITOR_ONLY);
...
} else {
		obs_source_set_monitoring_type(source,
				OBS_MONITORING_TYPE_MONITOR_AND_OUTPUT);
...

This part of code changes monitoring type, isn't it?

What about the volume meter and mute? User had explicit control over the "mute" sound by single click or by hotkey. Now, it is mess of the controls and visuals. Simply add "mute" hotkey to your monitoring source and click the speaker icon while you monitoring or not. You'll notice the difference. And you said that 2+3 logic easily fits into two boolean variables?

The streamers is casual users, it is desirable to make intuitive interface. While two buttons solution is not intuitive at all. The icon disabled/inactive state is bad idea too, because it is unknown what it does when it enabled (the "eye" icon and "lock" icon, for example, has difference in drawing - this is two completely different pictures) and what to choose (I prefer to enable it). Also, when I first saw the OBS Studio's mixer UI, I didn't knew that the "speaker" icon - interactive, and you may on/off the sound (it's because icon doesn't changes on hover).

SuslikV avatar Apr 28 '18 05:04 SuslikV

Please see the code from my PR into this PR. I still not sure I understand what's wrong, because two botton logic is the only possible intuitive way here, the context menu item used in this PR is a mess, I removed it. There's no 2+3 logic, it's four separate states.

  1. No stream no monitor
  2. Stream no monitor
  3. No stream monitor
  4. Stream monitor

And these states are perfectly controlled by two switches

Volume meter does not work only in state 1, because I may need it to fix the levels before going live.

If you say that there's some problem with shortcuts, we should research it, because personally I didn't test shortcuts.

The crossed out icon is a nice idea, but I don't think it's a showstopper if everything else works.

Himura2la avatar Apr 28 '18 09:04 Himura2la

@Himura2la idea IMO, doesn't seem very intuitive to me. I think it might confuse some users. I don't really like the right click menu for the 'output to monitory only' either. Another idea: Single click on the icon: toggle monitor on and off Double click on icon: toggle the 'output to monitor only', turning the icon red when on

cg2121 avatar Apr 28 '18 10:04 cg2121

Not bad, but I still don't understand how the 'make Mute button useless' state may help, sorry

Himura2la avatar Apr 28 '18 12:04 Himura2la

Now that I thinking about it more, your idea currently makes the most sense. There is not really an ideal solution for this.

cg2121 avatar Apr 28 '18 13:04 cg2121

I have now merged @Himura2la PR into this. https://github.com/obsproject/obs-studio/pull/1253/commits/32b071c21e020b97dcf6bfe76bc14d910bfc36e7

cg2121 avatar Apr 28 '18 14:04 cg2121

Used to do broadcast work, @Himura2la 's approach is definitely what I'd go w/ and for all the reasons he laid out. And I'll definitely play around w/ this pr when I get back home. I'd throw in check-boxes in the right click drop down as well, but that's no deal breaker.

Another thought*, if this pr does eventually get added, chrome_2018-06-11_14-45-21 monitoring device selection through right click menus? In a new pr of course.

Andersama avatar Jun 11 '18 21:06 Andersama

After experimenting with this a little, I agree removing the existing options in Advanced Audio Properties is a bad idea. What happens to the users who have already configured their monitoring settings? What happens to the third option that it used to provide?

WizardCM avatar Jun 14 '18 09:06 WizardCM

This PR does not go into massive refactoring involving the removal of three OBS_MONITORING_TYPE_* states (though, I have such a changeset in my repo) The existing configured behavior should not change with the update. The headphone button will be pushed in these cases https://github.com/obsproject/obs-studio/pull/1253/files#diff-b0fb175ce6adf3beb2f5402ebdc434c3R210 The Mute button won't be pushed automatically. If someone was so mad to set the OBS_MONITORING_TYPE_MONITOR_ONLY, the behavior will change for him and the sound will go to the stream unless he mute. No big deal, I think. It's OK to make things better in updates

If you think it's significant, I think it's possible to auto-mute if the OBS_MONITORING_TYPE_MONITOR_ONLY state came from existing settings

Himura2la avatar Jun 14 '18 15:06 Himura2la

Hm, I'm starting to think that yea, we may just have to change the API after all like @Himura2la suggested. I'm beginning to see that @Himura2la was correct about this from the beginning. I might help out with this, because I realize now quite clearly that what @Himura2la suggested is clearly the better design. I wish I had thought of that from the beginning.

I might take a crack at modifying this to his specifications -- but I do plan to merge this sooner than later. This is actually a pretty important UX improvement if done that way.

jp9000 avatar Jul 18 '18 07:07 jp9000

@jp9000 Thank you very much for appreciation, I'm glad to be helpful. If you are interested in a massive refactoring of this OBS Studio part, I've already made some sketchy work in that direction: https://github.com/cg2121/obs-studio/compare/a5a808c...Himura2la:virtual-mute

We can create another PR for this

Himura2la avatar Jul 18 '18 08:07 Himura2la

I've now fixed where the mute hotkey wouldn't work, and I've also added enable/disable audio monitor hotkeys.

cg2121 avatar Aug 22 '18 08:08 cg2121

Note: a variation of this PR has been included as part of the following PR: #1469

WizardCM avatar Jan 31 '19 04:01 WizardCM

I'm going to close this because #1469 is a better implementation of this.

cg2121 avatar Feb 21 '19 12:02 cg2121

I'm reopening this because the master mixer was put on the back burner for now.

cg2121 avatar Oct 16 '19 05:10 cg2121

I've now updated and rebased to master.

cg2121 avatar Oct 16 '19 06:10 cg2121