limiter parameters settings

[scar20]

As I used a limiter to prevent distortion of my filters on hi Q, I struggled to get it right. At first, I though the thing was not working but trying with extreme values finally give some results although it didn't make sense as for what the values are supposed to be. So I've taken a closer look at what are really those values and I find interesting things.

First of all, inside the class, pKnee variable seem to be just a switch leading to think this class is meant to be subclassed for further implementation details. Perhaps, a boolean could be added to the signature to be able to have different instance of limiter using the two "knee" type.

Then I've made a Max patch that reproduced the variables setting in Limiter::recalculate() to be able to play with the input value and see the actual variables changes in real time (I'm not good at math...) and validated against Log of the variables in the file. ( I've just seen you recommend using syslog so I'll try that next time). So here is the interesting part:

Attack values in/out:

3.1 = 0 3 = 0.000001 2.5 = 0.00001 2 = 0.0001 1.5 = 0.001 1 = 0.01 0.5 = 0.1 0 = 1 -0.5 = 10 -1 = 100

Release values in/out:

1.66 = 0 1.66 = 0.000001 1.33 = 0.00001 0.99 = 0.0001 0.66 = 0.001 0.33 = 0.01 -0.34 = 0.1 -0.67 = 1 -1 = 10 -2 = 10000

Threshold values in/out: <-2.2 = 0, -1 = 0.0001, 0 = 0.01, 1 = 1, 2 = 100, 3 = 10000

Trim values (hard coded): 0.6 = 1.584893 0.5 = 1 0.4 = 0.630957

Looking at those numbers, its easy to understand that if you put what you think is 100ms as attack, you'll get nothing since if ( lev > th ) { g = g - ( at * ( lev - th )); } so gain = gain - 0... wont do very much same for release; gain = gain + 0... As for threshold, anythings below -2.2 will be 0 and at 3 its already 1000 but this is compared to the input signal which is normally between -1:1. So if you think make the limiter kick-in at -14db you'll end up always on. Even though db is logarithmic, the scaling do not look right.

So here I think the variables labels are misleading and the range incorrect; they are definitely attack, release and threshold factor but thinking of them as ms (or microsec) and db will lead you confused. I don't know how or what to relabel or rescale them but most effective value lies around 0 and 1. Maybe when I'll be better at dsp I'll tackle this one.

Now I have set those values on the limiter (which use "soft knee" by hard coding 0.6f as the value) - att 0.8f, rel 1.0f, thres 0.55f - and I get exellent result with my filters Q set all the way up to ringing without any distort. The modified MWEngineActivity is at https://bitbucket.org/androfone/filtertest/src/filter/ Filters and limiter are on the "upper" sound - the "bach" sound pushes quite hard with Q=max and still ok.

below is the max patch that I used to decipher the values

[igorski]

Thanks for the (extensive) research on this topic! I shall have a look at the setters (and a general code cleanup) and come up with a more logical scale where it's broken.

I don't have the rights to view the bitbucket link you sent though.

[scar20]

Oops, Sorry about that. I have just uncheck the "This is a private repository" checkbox which shouldn't have been set. Right now I've try to add a constructor with an extra "knee" float var - default to "hard" <0.5 - but I got errors as it seem the JNI/SWIG stuff don't get regenerated. Be able to specify the knee allow for having different limiters for different purposes. Now as I hard coded my soft knee, I don't get the hard one for channel clipping. Thanks for fast answer! PS As I will get better and more confident at this, I need to learn how to send diff or if I understand correctly, pull request.

[igorski]

No worries, I can see it now.

I'll make some time, likely next week to investigate this. I believe the limiter was one of the earliest processor types and the implementation might be a little naieve. Coming up with meaningful variable names should be step 1 in fixing this :)

[igorski]

Alright, I created a feature branch limiter which contains an updated limiter for your reviewing. You can also view the changed file contents here on GitHub.

The values were actually listed correctly:

• microseconds for attack (as is common for limiters)
• milliseconds for release
• threshold in -20 to +20 dB range

...but they were to be supplied in percentile 0 - 1 range where the scaling would happen internally. This in itself is OK, but the function arguments would be misleading. For backwards compatibility I have kept the old methods and constructors intact (though with updated variable names), but I have extended the class by adding alternative getters/setters which work with the actual units (micro/milliseconds) and perform the scaling internally. Additionally, soft knee is now settable and a boolean value.

The internal variable names should now make more sense too and be more in line with the MWEngine coding conventions.

[scar20]

Thanks, Now I'm a bit stuck in my own separated version of MWEngine but I will integrate on next iteration. As you can tell, I'm not very used to handle git project, seems like I created too much sub-branch and not commit and merge enough. I've looked though on the diffs and it looks easier to read as the percentile ranging was not immediately obvious. I'll give more feedback when I'll have integrated and used.

[igorski]

No rush, the beauty of version control is that we can leave these branches be for a while without risk (well unless we start changing the same files on the same lines in other branches :p Even then, all is logged for easy diffing and fixing. :)

[scar20]

OK, I'm back after a long break. I need this to be integrated to the lib instead of having to add custom version on my own lib version - I needed soft knee for the filters.

Just a few things I noticed:

...but they were to be supplied in percentile 0 - 1 range

You mean for the threshold value only right? So 0 = -20db and 1 = +20db? I suppose not for attack and decay (0-1 of how many micro or milli sec)? Perhaps some comments in the code will be welcome to clarify this. Still I was able to use it from the original constructor with those values: 0.3f, 0.5f, 0.9f with an added method to set soft knee. So if nothing changed in the legacy constructor I can still use the same values + the soft knee bool so I would like you to make the push into the lib if you are ready.

Also in light of this, it seem that the values supplied in the MWEngineActivity.java are still incorrect (10f, 500f, 0.6f); those were the starting point of my investigation.

Lastly I have reorganized my work as you pointed out about synchronizing the lib in issue #133 ; I have now forked github.com/igorski/MWEngine to my own github space then clone that one as the working copy with origin set to my space and upstream set to yours as it seem it facilitate integrating updates from upstream and to make pull requests. I left though Fonofone on bitbucket. I was just wondering the best way to recreate the "filter" and "loop" branch here, which need the new limiter version; should I create new branch<loop, filter>, then origin/limiter-> "pull into current using rebase" or "pull into current using merge" as the start point before I drop the custom filters and sample.cpp/h files? Sorry, a bit out of scope here but my last setup was a mess...

Otherwise I think we can close this thread if you are ready to push the code.

[igorski]

Good to see you back as I was always interested in your project (and your great contributions).

I had to do some reading up on what had been done in the limiter feature branch and have updated the code comments after backtracking what the differences were (I should make a mental note of note leave pull requests unattended for several months 🙈 ).

I initially would have very much liked to kill the legacy constructor, but for the time being I'm keeping it in for both backwards compatibility (I have updated the example Activity to use a default constructor and not wildly useless values) but also because I'm wondering if for certain scenarios it is a more useful constructor than the the unit specific one. Same for the getters / setters.

You mean for the threshold value only right? So 0 = -20db and 1 = +20db? I suppose not for attack and decay (0-1 of how many micro or milli sec)?

Attack, Release and threshold were all normalized (not percentile, misnomer on my end) values. I have added comments to the legacy constructor and setters to clarify this. Curiously the values supplied in the example activity were beyond the intended range ! Basically max (1.0f) attack equals 1.56 millis, max release equals 1571.755 millis and max threshold equals +20 dB.

I am merging the limiter branch into master and closing the pull request - since the code is now commented more appropriately, features extra getter/setters for unit-specific control and should have no regressions with the original implementation. For your GitHub workflow I would suggest that your fork of the repository (in which you have your filter and loop implementations) are rebased against the MWEngine repository on origin/master. This implies that each fix done (such as the AAudio stability improvement several months back) are applied to your custom branch while keeping your added features intact. Worst thing that could happen is a merge conflict on a shared file such a CMakeLists.txt but that should be minor.

[scar20]

Ok, it look great. The comments and parameters names cannot be clearer. No rush to depreciate the legacy constructor though, as that could break others peoples works - and I use it (along with the softKnee bool) for my filter for which I did not have recalculated the usec, msec and dB actual value, I'm lazy... And about that, I just realized that if I want the signal being cutoff at 0dB, it should be set at 0.5 if I'm right? I've just tried with the value at 0.5 and didn't notice any "pumping" artifact - well those tiny speakers dont help much... Mmmm, I'll have to experiment further with this

I will fetch the changes on the server and clone again since I'm not satisfy on my new branch set up and I didn't push anything yet. I'm a bit rushed to produce an APK on GooglePlay for internal review so I wont be able to work on the loop and filter branch for now, but I still need them included in the aar. The fact the limiter is now part of main will simplify my branching scheme and redoing this early allow me to get a better hang of git without risking screwing up.

So my intent is to create develop, loop and filter branches, then add/replace the relevant files for loop and filter, then merge loop and filter on develop from which I'll be able to build the aar I need while keeping alive loop and filter for later development.

You can consider this topic closed if you don't have further comments to add.