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

Feature Request: Output Limiter via GUI #4894

Closed
capital-G opened this issue Apr 24, 2020 · 21 comments
Closed

Feature Request: Output Limiter via GUI #4894

capital-G opened this issue Apr 24, 2020 · 21 comments
Assignees
Milestone

Comments

@capital-G
Copy link
Contributor

capital-G commented Apr 24, 2020

Motivation

I think everyone knows the problem that if you set an unfortunate value the volume of Supercollider can increase drastically which can easily lead to permanent hearing damage - especially if you work with headphones.
In order to avoid this you could add a toggle switch for a limiter in the GUI which will cut the volume at a certain level so that it does not increase anymore.
Of course you could add a limiter as a SynthDef into the NodeTree but in my opinion a limiter should be always be in GUI reach because some kind of misconfiguration can always happen and should therefore be easily switched on/off.

Description of Proposed Feature

I think it would be best to add an option in the global server menu in the bottom left and indicate an active limiter with a red L in the bottom left (like R for record)

image

Plan for Implementation

If I can get some help I would volunteer to implement this myself - I don't have much Qt/C++ experience but I always appreciate to learn new programming languages/libraries.

@joshpar joshpar self-assigned this Apr 24, 2020
@jamshark70
Copy link
Contributor

That's a good idea.

I'd suggest to create the master limiter audio processing as a SuperCollider class, not in C++, so that you can place a limiter by code as well.

Also include arguments for bus, number of channels and target group, so that the limiter can be used anywhere, not only on the master channel. It's reasonable for it to default to master -- not reasonable to provide no options.

The IDE command would have to be added in C++ -- the action would be just to execute the SC statement (same way that "Start recording" does now).

@nhthn
Copy link
Contributor

nhthn commented Apr 26, 2020

I think first and foremost, macOS SC should do a server-side hard clip of the audio output from -1 to +1, for both safety and for consistency with Windows and Linux. I wrote up a proposal about this that I was planning on making into an RFC, but didn't get around to it:

https://gist.github.com/nhthn/607c3b05a4d624c02afd2ecad6769330

I posted this to the list some time back and IIRC James McCartney, our resident expert on CoreAudio, has semi-endorsed this and stated that he believes clipping should have been part of the API.

On Linux, I normally don't use a safety limiter when messing around. I do try to work with very little headroom, maybe 6 dB. That way if something blows up it's mostly just startling and distorted, but never dangerous. macOS users don't have that luxury, and that's why I think hard clipping is the most important first step to improving the safety of SC.

@muellmusik
Copy link
Contributor

I think first and foremost, macOS SC should do a server-side hard clip of the audio output from -1 to +1

One small point, this should not affect recording. The extra headroom offered by float sample formats is very useful in some cases.

@joshpar
Copy link
Member

joshpar commented Apr 27, 2020

My thought would be working this in to the Volume class, with a default of ON, along with a control to set clip level (0db, etc).

@fghibellini
Copy link

fghibellini commented Jul 11, 2020

Related question posted on discord: https://scsynth.org/t/way-to-limit-maximum-output-volume/2392/3

TLDR as a newbie I accidentally ran exactly into this issue even with code that does not touch amplitude. As a result I've had pain in my ear for the past 3 days.

@brunoruviaro
Copy link
Contributor

a big +1 for this feature. I now regularly use the Safety quark but as I often work with tons of student beginners, it would be amazing to have a built-in limiter that is always on by default without having to install a Quark.

@dyfer
Copy link
Member

dyfer commented Jul 24, 2020

I would lean towards limiting to +-1 in the backend for two reasons:

  • one can accidentally put a synth after any limiter we put in the node tree
  • this seems to be the behavior on other platforms

Also, I personally find such behavior more reasonable.

IMO there are very few examples when the user needs signals above +-1 (I can only imagine a situation when using routing software to process the audio elsewhere). I'd also suggest adding a command line switch to optionally disable limiting, but I believe it would be okay to leave it on for 99% percent of use cases. Additionally, many (most?) (semi-)pro soundcards do not allow software volume control and always drive their converters with full scale signal (i.e. signal above +-1 would cause distortion on the converter), i.e. proposed limiting would not affect users of most external soudcards.

I think it would be worthwhile to additionally implement configurable limiter like @jamshark70 and @joshpar suggested to allow limiting different channels, at different places of the node tree, maybe with configurable lookahead/soft clipping etc. But IMO basic hard clip should be implemented regardless for all CoreAudio outputs, right before the signal goes out to the backend (i.e. recording would not be affected).

@jamshark70
Copy link
Contributor

I would lean towards limiting to +-1 in the backend

This makes sense to me, except for the casual assumption of +-1 as the limit.

IMO there are very few examples when the user needs signals above +-1 (I can only imagine a situation when using routing software to process the audio elsewhere).

The existence of even one potential case for "out of range" signals means that we must make the limit configurable, and must not hardcode +-1. I am ok with the default being +-1. I'm not ok with it if there's no way to override. (I'm a bit uneasy with the way that this thread keeps stating "+-1" as if this were the only thing an end stage limit should do.)

An easy way would be a command line option, say -M 1 for +-1 ("M" for maximum -- "c" is already taken for control buses and "C" for clock sync) and -M 1.7 to allow an extra 3 dB. -M - could disable hard clip altogether. I'd even be ok with assuming +-1 if -M is absent from the command.

Again, the issue is that signal values approaching the upper limit of 32 bit floats are over 700 dB, and if macOS's volume control is attenuating by 60 dB, that isn't doing much. If we hard clip to +-2, there is a touch of headroom (say for inter-app routing) but we prevent the >600 dB volume blowup. Personally I like this idea better but I won't stand in the way if people feel very strongly that the default must be +-1.

@dyfer
Copy link
Member

dyfer commented Jul 24, 2020

I'm happy with the limit to be above 0dB, i.e. 1.25, or 1.5, or 2 etc.
On one hand I like the idea of configurability, but I really think that one either will want to run with the limiter on (99% of cases) or turn it off all together. I'd be OK with either providing a switch to configure the clipping amount, OR the one that just turns the limiter on or off (default on). Also any command-line option is inherently inflexible, so configurability is even less "useful". I'd say that for configurable limiter it'd be better to turn it off in the command line and use one running at the tail of the root node.

@capital-G
Copy link
Contributor Author

I started to lurk a bit into the source code of supercollider and would suggest to attach the limiter to the server, so e.g. s.limiter(1.0), but as suggested it would be great to overload this so one could set the limiter for each channel.
The Limiter could also be configured from the SC IDE dropup menu like the audio level.

I would also suggest to not have the ability to put something accidentally behind the Limiter Node.
I don't know if there is some secret/privat parts in the NodeTree where this could be archived.

We should think about if the divergence between what one hears and what one records is actually a good idea if such a limiter is driven hard and used as part of the sound. Maybe make it configurable if it will also affect the recording?

If it is OK with you I would really use this as a first issue to contribute to SC and dig deeper into SC but I may have to ask some stuff like if it is better to implement this on the Server Side C++ code or as SC code as some kind of special Node.

@elgiano
Copy link
Contributor

elgiano commented Jul 25, 2020

I think we could see these as two different features:

  1. solving the issue with macos dealing with output volume significantly differently from other platforms. I think this should be done on the server (c++).
  2. providing a "general purpose" stage limiter, maybe like the Monitor class is: a language utility to create a specific synthdef and instantiate a synth from it (100% doable in sclang).

@capital-G
Copy link
Contributor Author

I made a POC how the GUI for a Limiter on a Master Chanel could look like. For the channel configuration I would adapt the behaviour of recording.

image

If the Limiter is active, it is indicated by a red L. Also it is maybe a good idea to make the limiter configurable like the volume meter.

A general purpose stage limiter is generally a good Idea but IMO it does not help beginners to avoid blowing up their speakers and their ears - therefore a limiter should be switched on by default as forced last part of the node tree if this is possible. Is it possible to force a Def to be always the last node or is this a dead end?

Regarding the macos output:
From my understanding the following lines forward buffers to CoreAudio https://github.com/supercollider/supercollider/blob/develop/server/scsynth/SC_CoreAudio.cpp#L1307-L1336 - at least recuding the values results in quieter audio. So from my understanding we should clamp those busdata / bufdata arrays to -1.0 and 1.0, right?

@joshpar
Copy link
Member

joshpar commented Jul 27, 2020 via email

@dyfer
Copy link
Member

dyfer commented Jul 28, 2020

Thanks Josh.
I still think the "hard" limit should be set in the backend. Then having a configurable limiter in addition to limiting in the backend is going to be great, too.
IMO there shan't be a way to accidentally go "around" the limiter, thus I support limiting in the backend (with a command line switch to remove it for "special use cases").

@jamshark70
Copy link
Contributor

I still think the "hard" limit should be set in the backend

I tend to agree that the signal should be controlled in a way that doesn't introduce any delay. It's ok if there's a class-library limiter that the user can control before that, but I do think it would be good to have a zero-delay clip under the hood.

@fghibellini
Copy link

@capital-G

Some people's first reaction to a new tool is to try to toggle on&off all buttons to see what they do, often leaving the ones with an unclear purpose in a random state.
A first-time user will see no effect of the "Limiter" checkbox and so it's possible that it's gonna be left unchecked.

Also it doesn't sound like an option that people will want to switch on&off very often - more like a per-user choice.

I think it would be better placed somewhere in the "Pereferences" menu with a long description/warning of what it does.

The Github "Danger zone" might be a good source of inspiration

Screen Shot 2020-07-28 at 23 26 41

@capital-G
Copy link
Contributor Author

capital-G commented Jul 28, 2020

Implemented a first draft as a PR - happy for reviews. See #5106 .

@elgiano
Copy link
Contributor

elgiano commented Jul 29, 2020

I've tried clipping on the server, it prevents infinite signals to sound very loud when system volume is low, and doesn't affect recording. Note: here the threshold is hardcoded, but if course it would be important to have a config option.

diff --git a/server/scsynth/SC_CoreAudio.cpp b/server/scsynth/SC_CoreAudio.cpp
index ffcce57d2..9aaef864f 100644
--- a/server/scsynth/SC_CoreAudio.cpp
+++ b/server/scsynth/SC_CoreAudio.cpp
@@ -1264,6 +1264,10 @@ OSStatus appIOProc(AudioDeviceID device, const AudioTimeStamp*
inNow, const Audi
     return kAudioHardwareNoError;
 }

+float clip2 (const float x, const float t) {
+  return (x < -t) ? (-t) : (x>t) ? (t) : (x);
+}
+
void SC_CoreAudioDriver::Run(const AudioBufferList* inInputData, AudioBufferList* out
OutputData, int64 oscTime) {
     int64 systemTimeBefore = AudioGetCurrentHostTime();
     World* world = mWorld;
@@ -1375,7 +1379,7 @@ void SC_CoreAudioDriver::Run(const AudioBufferList* inInputData,
 AudioBufferList
                     if (nchan == 1) {
                         if (outputTouched[b] == bufCounter) {

                             for (int k = 0; k < bufFrames; ++k) {
-                                bufdata[k] = busdata[k];
+                                bufdata[k] = clip2(busdata[k], 2.);
                             }
                         }
                     } else {
@@ -1383,7 +1387,7 @@ void SC_CoreAudioDriver::Run(const AudioBufferList* inInputData,
 AudioBufferList
                         for (int j = 0; j < minchan; ++j, busdata += bufFrames) {
                             if (outputTouched[b + j] == bufCounter) {
                                 for (int k = 0, m = j; k < bufFrames; ++k, m += nchan
) {
-                                    bufdata[m] = busdata[k];
+                                    bufdata[m] = clip2(busdata[k], 2.);
                                 }
                             }
                         }

I did some very naive tests (running lots of ugens) and didn't see noticeable performance losses (in ScIDE's performance meter). Could this be a way to go? I have a feeling that what comes next (configurability) will also need some thorough discussion.

@jamshark70
Copy link
Contributor

jamshark70 commented Jul 29, 2020

I've tried clipping on the server, it prevents infinite signals to sound very loud when system volume is low, and doesn't affect recording.

I tend to agree with this approach. It's not a limiter, but doesn't preclude the user (or SC) adding a limiter synth into the chain.

Also you don't even need to declare the function -- see:

https://github.com/supercollider/supercollider/blob/develop/include/plugin_interface/SC_InlineBinaryOp.h#L482

template <typename T> inline T sc_clip2(T a, T b) { return sc_clip(a, -b, b); }

😁

@elgiano
Copy link
Contributor

elgiano commented Jul 30, 2020

While I support the creation of an sclang based Limiter, and its integration with ScIDE, as @capital-G is developing it, I made a PR proposing server-side clipping for macos:

#5110

While an sclang-based limiter will be a great addition IMO, I think it's important to handle CoreAudio behavior on scsynth, thus being reliably present, allowing configurable headroom as a ServerOption, and not affecting recording.

@mossheim mossheim added this to the 3.12.0 milestone Nov 18, 2020
@capital-G
Copy link
Contributor Author

Closed b/c #5110 implements what this feature wanted to achieve and any sclang solution would not change anything now but would introduce breaking changes. For further discussion, see #5106

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants