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

UI: Add audio track mixer #4741

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

cg2121
Copy link
Contributor

@cg2121 cg2121 commented May 21, 2021

Description

This adds a dock, hidden by default, which shows volume controls for each audio track. Each track can be hidden.

Screenshot from 2022-11-17 19-48-55

Motivation and Context

IMO, this feature is sorely needed in OBS.

I took the code from #1469, and tried to simplify it as much as possible, so it would be more likely to be merged.

How Has This Been Tested?

Controlled different audio tracks and used the audio monitor with them to make sure it functions as expected.

Types of changes

  • New feature (non-breaking change which adds 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.

@WizardCM WizardCM added the Enhancement Improvement to existing functionality label May 21, 2021
@derrod derrod added the Seeking Testers Build artifacts on CI label May 22, 2021
@pkviet
Copy link
Member

pkviet commented May 22, 2021

Nice work. I'll be sure to check it out

@cg2121 cg2121 force-pushed the master-mixer branch 3 times, most recently from 78e75ac to 376c2c8 Compare May 27, 2021 14:06
@Fenrirthviti
Copy link
Member

I really like this, but I can already see people asking us why this isn't part of the normal mixer, or its own dock. Would it be possible to add this as a dock, or optionally to the main mixer dock itself?

@WizardCM
Copy link
Member

That'd be a design decision - the original PR had it as its own optional dock (hidden by default). I used the original PR for a while and agree that it'd be best as its own dock, which'd allow for choosing multiple tracks in future.

@WizardCM

This comment was marked as resolved.

@cg2121

This comment was marked as resolved.

@WizardCM

This comment was marked as resolved.

@WizardCM

This comment was marked as resolved.

@cg2121

This comment was marked as resolved.

@Warchamp7
Copy link
Member

I really like this, but I can already see people asking us why this isn't part of the normal mixer, or its own dock. Would it be possible to add this as a dock, or optionally to the main mixer dock itself?

I also agree that this should at the very least be it's own dock, or part of the main audio mixer (Perhaps pinned to the top of the list)

@WizardCM

This comment was marked as resolved.

@WizardCM

This comment was marked as resolved.

@cg2121

This comment was marked as resolved.

@Warchamp7
Copy link
Member

Warchamp7 commented Aug 15, 2021

I didn't realize this was a meter for every audio track that could be swapped between.

I'd like this to just be a single master meter for all tracks and pinned to the top of the audio mixer dock

I know there was a very large advanced mixer PR this got splintered off from, but I think there are better ways to approach audio controls for other tracks

@Fenrirthviti Fenrirthviti marked this pull request as draft September 3, 2021 20:21
@Fenrirthviti
Copy link
Member

Marking this draft for now as there are some design issues that need to be addressed on the UI/UX side.

@cg2121
Copy link
Contributor Author

cg2121 commented Sep 3, 2021

If it were to put in the mixer dock, how would it be differentiated from the other meters?

@Fenrirthviti
Copy link
Member

My current understanding is that this should be a single master mix for all tracks, pinned at the top of the mixer dock. Perhaps we can put a border around it to indicate it's a static part when enabled?

@Warchamp7
Copy link
Member

If it were to put in the mixer dock, how would it be differentiated from the other meters?

Lots of ways that themes can determine

image

But it's not necessary either. The naming is often enough, especially if it can be "pinned" as always the first entry in the list

image

@Vrekktec
Copy link

Vrekktec commented Sep 6, 2021

I have been looking for a Master Fader for a while now, I would love to test your Plug-In!

As far as I can see there is no download link in this post? Sorry to bother, but I don't use Github very often. 😅

@Fenrirthviti
Copy link
Member

This is not a plugin, it's a proposed code change for the base OBS Studio. There is no download available currently as this is not finished yet.

@MattyFresh68
Copy link

Better than what is currently implemented, but I really think what pkv has done with music edition is exactly what the majority of users would benefit from. It is super clean, not bloated, and seems very user friendly overall.

Copy link
Member

@Warchamp7 Warchamp7 left a comment

Choose a reason for hiding this comment

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

Per a discussion many months ago, this was renamed/recontextualized as an 'Audio Track Mixer', which while nitpicky is much more accurate to the functionality here. As noted above, a master mixer is not actually technically possible currently.

With that in mind, I am feeling better with this from a UX perspective.

Prior to merging I would like to see the code refactored to update all naming conventions away from 'Master mixer', PR/commit title updated to match, and more testing.

@cg2121 cg2121 removed this from the OBS Studio 29.1 milestone Mar 26, 2023
@Zoxjib
Copy link

Zoxjib commented Apr 24, 2023

So maybe it would be appreciated to add a VST filter intergration on this "virtual master bus", to let customers use they own metering VST plugins. (If I understood, it can't cause real-time problem because this audio bus is not broadcasted/recorded, only used for metering)

It is common practice to use compression/limiting on the Master track, and is supported by every DAW and professional video editing software. I thought it was implied that it would work the same way?

This capability would also allow using loudness monitoring VST plugins, which incidentally is the way it is done by all DAWs as they don't natively measure loudness in LUFS.

@WizardCM
Copy link
Member

WizardCM commented Jun 3, 2023

Per a discussion many months ago, this was renamed/recontextualized as an 'Audio Track Mixer', which while nitpicky is much more accurate to the functionality here. As noted above, a master mixer is not actually technically possible currently.

With that in mind, I am feeling better with this from a UX perspective.

Prior to merging I would like to see the code refactored to update all naming conventions away from 'Master mixer', PR/commit title updated to match, and more testing.

@cg2121 Just a friendly poke about this feedback, specifically about internal naming. Additionally, there are code conflicts.

@cg2121 cg2121 changed the title UI: Add master audio controls UI: Add audio track mixer Jun 4, 2023
@cg2121
Copy link
Contributor Author

cg2121 commented Jun 4, 2023

Updated to rename master mixer to audio track mixer in all of the code, as per @Warchamp7 request. I also removed unnecessary/duplicate code.

@cg2121 cg2121 requested a review from Warchamp7 June 4, 2023 09:54
@derrod
Copy link
Member

derrod commented Jul 29, 2023

Unfortunately it would appear that it is conflicted again.

Copy link
Contributor

@norihiro norihiro left a comment

Choose a reason for hiding this comment

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

I have not fully understood how audio_track_active behaves but the timing of incrementing/decrementing it looks wierd.

libobs/audio-monitoring/osx/coreaudio-output.c Outdated Show resolved Hide resolved
libobs/audio-monitoring/pulse/pulseaudio-output.c Outdated Show resolved Hide resolved
libobs/audio-monitoring/win32/wasapi-output.c Outdated Show resolved Hide resolved
libobs/audio-monitoring/osx/coreaudio-output.c Outdated Show resolved Hide resolved
@cg2121 cg2121 force-pushed the master-mixer branch 5 times, most recently from 1cad799 to 87ec585 Compare October 10, 2023 17:14
This makes each audio track a source.
This adds an audio track mixer, so users can monitor different
audio tracks.
@DAcodedBEAT
Copy link

I was just looking for something like this in OBS. Is there a reason why this hasn't been merged yet?

@MrKlorox
Copy link

MrKlorox commented Jan 3, 2024

So maybe it would be appreciated to add a VST filter intergration on this "virtual master bus", to let customers use they own metering VST plugins. (If I understood, it can't cause real-time problem because this audio bus is not broadcasted/recorded, only used for metering)

It is common practice to use compression/limiting on the Master track, and is supported by every DAW and professional video editing software. I thought it was implied that it would work the same way?

This capability would also allow using loudness monitoring VST plugins, which incidentally is the way it is done by all DAWs as they don't natively measure loudness in LUFS.

This is the entire point of having a Master track, afaik. Not being able to broadcast the track would be pretty pointless.

I want to be able to use an advanced compressor/limiter plugin that essentially sounds like multiband sidechain ducking. The one I use will turn up the quiet audio (game), then make that quieter when something louder comes in (mic), while keeping the same loudness. There is no way to do this without a master track that has plugins (audio and video for possible render/sync delay if not using one in the filters for the scene) as well as being able to output.

Does this need a new issue/Pull request? Thanks.

@PatTheMav
Copy link
Member

@norihiro Do you still have open concerns regarding audio_track_active? Otherwise I'd like to get this merged soon-ish.

@cg2121 Do I understand it right that the sliders can adjust the tracks' output volume? Because that makes it obviously different from the meters named in other applications (which usually have only one output track, so only one meter next to the preview).

@PatTheMav
Copy link
Member

So maybe it would be appreciated to add a VST filter intergration on this "virtual master bus", to let customers use they own metering VST plugins. (If I understood, it can't cause real-time problem because this audio bus is not broadcasted/recorded, only used for metering)

It is common practice to use compression/limiting on the Master track, and is supported by every DAW and professional video editing software. I thought it was implied that it would work the same way?
This capability would also allow using loudness monitoring VST plugins, which incidentally is the way it is done by all DAWs as they don't natively measure loudness in LUFS.

This is the entire point of having a Master track, afaik. Not being able to broadcast the track would be pretty pointless.

I want to be able to use an advanced compressor/limiter plugin that essentially sounds like multiband sidechain ducking. The one I use will turn up the quiet audio (game), then make that quieter when something louder comes in (mic), while keeping the same loudness. There is no way to do this without a master track that has plugins (audio and video for possible render/sync delay if not using one in the filters for the scene) as well as being able to output.

Does this need a new issue/Pull request? Thanks.

A proper master output track (and being able to interact with it similar to how DAWs expose it) is indeed a separate concern and would require a separate PR.

Having a master output channel (post-fader) that can have filters applied to it as well is a valid request (one that many on the team share) but audio is hard and there is a big pile of other things vying for attention obviously.

Usually laying the architectural groundwork (without exposing anything to users) should come first, followed by specific features (another feature often asked for is true output monitoring which would require tapping into post-fader audio).

@norihiro
Copy link
Contributor

norihiro commented Jan 11, 2024

@norihiro Do you still have open concerns regarding audio_track_active?

The implementation looks as follows. UX-wise, is this acceptable?

  • If there is at least one audio-track in monitoring, the monitoring of all individual sources is blocked.

For example, an audio source is not assigned to any audio-tracks but set to monitoring, the user cannot hear the audio source if unrelated audio-track is in monitoring, though the audio source is expected to monitor.

Other implementation can be not to block audio-monitoring or to have a more complicated condition to avoid monitoring the audio twice.

@Kobi-Blade
Copy link

@norihiro Do you still have open concerns regarding audio_track_active?

The implementation looks as follows. UX-wise, is this acceptable?

  • If there is at least one audio-track in monitoring, the monitoring of all individual sources is blocked.

For example, an audio source is not assigned to any audio-tracks but set to monitoring, the user cannot hear the audio source if unrelated audio-track is in monitoring, though the audio source is expected to monitor.

Other implementation can be not to block audio-monitoring or to have a more complicated condition to avoid monitoring the audio twice.

I hope this is entirely optional, cause if this behaviour will be default in OBS when this is merged, is going to cause a lot of problems.

@eazuka
Copy link

eazuka commented Mar 4, 2024

any update on this PR, why hasn't this been merged yet?

@Fenrirthviti
Copy link
Member

any update on this PR, why hasn't this been merged yet?

I'd recommend taking the time to read the comments before posting like this in the future. In general "why isn't this merged" comments with no substance are annoying, but asking a question that is clearly answered in the discussions in the comments above you even more so.

@eazuka
Copy link

eazuka commented Mar 5, 2024

any update on this PR, why hasn't this been merged yet?

I'd recommend taking the time to read the comments before posting like this in the future. In general "why isn't this merged" comments with no substance are annoying, but asking a question that is clearly answered in the discussions in the comments above you even more so.

Thank you for taking the time to respond. You shouldn't get angry or confrontational over a why question, it's just a question friend. You can just ignore and not respond or just point out that the question is already answered above. Thanks

@0x5066
Copy link

0x5066 commented Jun 12, 2024

any update on this PR, why hasn't this been merged yet?

I'd recommend taking the time to read the comments before posting like this in the future. In general "why isn't this merged" comments with no substance are annoying, but asking a question that is clearly answered in the discussions in the comments above you even more so.

So, I took some time to read the comments on this PR, and I'm not even sure if this PR is supposed to add a Master track (like you'd find in DAWs and other software), and where OP even is with this PR.
Last commit was 8 months ago, so it can be assumed that this is dead.
The question of "why hasn't this been merged yet?" is fairly reasonable, because there is no reason stated as to why this hasn't been merged yet, you can speculate, but it's pointless.
It's also not clear if this Master track acts like any other track, where you can add VSTs and what have you (again, something you'd see in DAWs).

Maybe instead of taking your anger out, summarize what happened, not everyone has all the time in the world. :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Improvement to existing functionality Seeking Testers Build artifacts on CI UI/UX Anything to do with changes or additions to UI/UX elements.
Projects
None yet
Development

Successfully merging this pull request may close these issues.