Peak meters calibrated pull #10

brummbrum · 2019-05-20T19:17:36Z

This pull request implements the Peak Level Meters on the Komplete Kontrol Keyboard. It is based on previous pull requests #7 and #9. It thus shows more changes relative to the current master which has not yet pulled #7 and #9 .

Notes:

The meters shown on the displays of S-Series Mk2 show a dB scale. Contrarily to Reaper, this scale is not evenly spaced, i.e. the number of pixels between e.g. the 0dB mark and the -12dB mark differs from the number of pixels between the -12dB mark and the -24dB mark and so on. Above -48dB it is logarithmic. Reaper reports peak values and shows these values on an evenly spaced scale in Reaper's on screen meters. In order to match the reading between Reaper meters on the computer screen and the keyboard's display the non-linearity of the keyboard's display scale is taken into account.
The above will also be relevant when sending Reaper's track fader (volume) positions in upcoming feature additions (aka "volume text").
As a future code optimization maybe I will substitute the currently used pow() function to transform the meter values by exp() which should be computationally more efficient

…ster track

More actions and fixes

Direct track selection in Mixer view w top row buttons

Mixer Mode Track Navigation

Comments for future changes. Branch from here as this modifies architecture an multiple areas

Direct API calls for track selection rather than Reaper actions to overcome 99 track limit

Track selection working Non existing tracks in last bank are now properly marked as not available

Bank selection

VU meter data format ok, but: - Need a better hook (update frequency) - VU meter updates not working properly. CMD_SEL_TRACK_PARAMS_CHANGED not fully understood yet

VU: - functionality is there! - some minor tweaks like scaling still open - code cleanup required

native conversion taken from Reaper SDK

- Calibration also valid for other keyboards than S Mk2?

-Meters working -Precise, non linear calibration for KK Mk2 Display Meters

Update niMidi.cpp

- Stub to isolate this pull request from fork master

efficiency improvement

brummbrum · 2019-05-20T20:02:27Z

well, the pow() vs exp() efficiency bugged me a little - improved this straight away while at it (not that it really matters.... :-)

- Use direct linear logarithmic conversion specifc to NI keyboard meter scaling rather than concatenating a calibration frunction after calling Reaper API standard conversions - Speed improvement - Further meter precision improvement

brummbrum · 2019-05-21T05:35:53Z

Final code optimizations. The conversion from peak values to MIDI values is now implemented as a direct linear logarithmic transformation without any detour via Reaper API conversions. This delivers the highest precision at lowest CPU costs.

brummbrum · 2019-05-26T23:24:59Z

Requires a fix. The fix will be introduced together with mute and solo implementation.
Current behavior:

meters not active when track muted
Fixed behavior:
meters not active when (track muted) AND (not soloed)

brummbrum · 2019-05-26T23:38:49Z

Actually Reaper API Track_GetPeakInfo() seems to be buggy, will raise as a separate issue

brummbrum · 2019-05-27T06:36:40Z

Solved in pull request #18 => issue #17 closed.

jcsteh

Thanks!

I appreciate these being submitted as separate pull requests. However, because they're on top of other work, it's still difficult to review and merge them in isolation. If possible, please branch from master for each piece of work. That's tricky for a couple of these, since they depend on earlier code, but several of them are largely unrelated (including this one) and could be done in separate branches off master.

A few comments below.

jcsteh · 2019-05-28T10:39:26Z

include/reaper/WDL/db2val.h

@@ -0,0 +1,17 @@
+#ifndef _WDL_DB2VAL_H_


It looks like this file isn't needed any more?

correct. can be kicked out.

jcsteh · 2019-05-28T10:39:59Z

readme.md

@@ -1,69 +1,22 @@
 # ReaKontrol
-


This file shouldn't be included in this PR/branch.

yeah, I was surprised it got in there. don't even know how to exclude it ;-) such a rookie I am with git. need to study the documentation better.
... don't be shocked ... the code itself is (despite of style issues) not as terrible

jcsteh · 2019-05-28T10:41:23Z

src/niMidi.cpp

+		// A value of 0 will result in stopping to refresh meters further to right.
+		// The array needs one additional char at peakBank[16] set to 0 as a "end of string" marker.
+		char peakBank[(BANK_NUM_TRACKS * 2) + 1] = { 0 };
+		int j = 0;


Can we have a better name for this variable? outPos might work.

jcsteh · 2019-05-28T10:43:52Z

src/niMidi.cpp

+			if (*(bool*)GetSetMediaTrackInfo(track, "B_MUTE", nullptr)) {
+				peakBank[j] = 1;
+				peakBank[j+1] = 1;
+			}


nit: I'd prefer } else { on same line, rather than separated.

yeah, better readability. will consider in future edits.

jcsteh · 2019-05-28T10:44:59Z

src/niMidi.cpp

+			}
+			else {
+				peakValue = Track_GetPeakInfo(track, 0); // left channel
+				if (peakValue < 0.0000000298023223876953125) {


Please use a named constant for this value. I assume it's the float representation of -150dB?

I wonder if it makes sense to move this check into peakToChar?

yup. I did exactly that in later rework of the code (see later pull requests). And yes, sorry for making you read through these ventures with the incremental modifications style....

jcsteh · 2019-05-28T10:45:53Z

src/niMidi.cpp

+//-----------------------------------------------------------------------------------------------------------------------
+
+static unsigned char peakToChar_KkMk2(double peak)
+{


nit: I'd prefer { on the same line as the function definition.

jcsteh · 2019-05-28T10:46:17Z

src/niMidi.cpp

+	double a = -3.2391538612390192E+01;
+	double b = 3.0673618643561021E+01;
+	double c = 8.6720798984917224E+01;
+	double d = 4.4920143012996103E+00;


What do these magic numbers represent? Please use named constants.

they are the result of curve fitting, i.e. reverse engineering to fit Reaper's meters exactly to what is shown on the KK display. The KK display scale is a fixed pixel image. The function calculates the MIDI values that are required to show the bar at exactly the correct height to coincide with the dB value that Reaper reports. I.e. midi = a + b * ln(c * volume +d).
With these parameters the maximum error is 0.3% between Reaper values and display reading at the curve fitting points noted in the comments, i.e. far less then the quantization due to the pixel nature of the display.

I should add that this is the most CPU friendly function I found.

jcsteh · 2019-05-28T10:49:55Z

src/main.cpp

@@ -145,6 +145,7 @@ BaseSurface::~BaseSurface() {
 }

 void BaseSurface::Run() {
+	this->_peakMixerUpdate(); // ToDo: Maybe update only every 2nd call to save CPU?


Rather than putting this in BaseSurface, you could just override Run() in NiMidiSurface and call the superclass: BaseSurface::Run()

yes, that would look cleaner and is also more logical (I don't even know if Mk1 has meters - do you?)

brummbrum · 2019-05-28T16:35:46Z

Thanks!

I appreciate these being submitted as separate pull requests. However, because they're on top of other work, it's still difficult to review and merge them in isolation. If possible, please branch from master for each piece of work. That's tricky for a couple of these, since they depend on earlier code, but several of them are largely unrelated (including this one) and could be done in separate branches off master.

A few comments below.

You are very right about this - sorry for making your merging efforts harder than maybe needed. Originally I had planned to work in this fashion (separating parallel branches from a base commit) but abandoned it when I came to realize that there were more inter-dependencies than I had anticipated. This is also owed to the fact that for me it is like "double reverse engineering": the NI API is not documented at all and Reaper's API is also not documented that extensively... As a result I ventured back and forth with some approaches as you can easily see when you look at the individual pull request.
As it turned out, the meters have a lot of of inter-dependencies. They are linked to mute, solo, muted by solo, volume markers,...
I finally decided that - considering the relative small size of the code - it could be acceptable to work in this incremental fashion and at least splitting up the branches. You can still diff between the different pull requests.
PS: I'm in my second week of working with git... don't be surprised by my clunky way of using it

brummbrum · 2019-05-28T17:53:00Z

PPS: "offline" for the next couple of days, hence I won't be able to change anything for a week or so

Transport Button Lights working

Leonard de Ruijter and others added 30 commits April 24, 2019 07:06

Fix off by one for knobs. The first knob in bank 0 now changes the ma…

8258c9f

…ster track

Do not pause when pressing restart and we are already playing

04f71c9

Repeat repeats the whole project

af9ebd0

Merge pull request #1 from leonardder/moreActionsAndFixes

d184ae8

More actions and fixes

Update niMidi.cpp

8e434f3

Direct track selection in Mixer view w top row buttons

Track navigation in Mixer mode

6be3125

Merge pull request #2 from brummbrum/Test

543e7c1

Mixer Mode Track Navigation

Stable

e5b1ef1

Comments for future changes. Branch from here as this modifies architecture an multiple areas

TrackSelect

4229c69

Direct API calls for track selection rather than Reaper actions to overcome 99 track limit

More fixes

9b54422

Track selection working Non existing tracks in last bank are now properly marked as not available

BankSelection Working

9031be9

Merge pull request #3 from brummbrum/BankSelection

59ccf32

Bank selection

Fixed Volume & Pan

928a00e

VU meter tests

d871095

VU meter data format ok, but: - Need a better hook (update frequency) - VU meter updates not working properly. CMD_SEL_TRACK_PARAMS_CHANGED not fully understood yet

VU meter working

50ebd55

VU: - functionality is there! - some minor tweaks like scaling still open - code cleanup required

Cleanup1

87884f7

Cleanup2

44cd80b

Merge branch 'MoreMixerUpdates' into master

f3d2fcb

Add log conversion library functions

a4fda2c

native conversion taken from Reaper SDK

Log VU meters calibrated

47a503f

- Calibration also valid for other keyboards than S Mk2?

fix & comment

77d5f22

Meter precision calibration finished

cf79108

-Meters working -Precise, non linear calibration for KK Mk2 Display Meters

Update niMidi.cpp

3170cbd

Ready for pulling Nav Patch

b484e00

Merge branch 'master' into IncrNavPatch

ef46618

Merge pull request #6 from brummbrum/IncrNavPatch

b6af0eb

Update niMidi.cpp

Update Readme

0729a36

Peak Meters Ready

24eda7a

Merge branch 'master' of https://github.com/brummbrum/reaKontrol

3b62589

Peak Meters Calibrated

e96e5f9

- Stub to isolate this pull request from fork master

Substitute pow() with exp()

8844e20

efficiency improvement

Code optimization (see comments)

f9e6eba

- Use direct linear logarithmic conversion specifc to NI keyboard meter scaling rather than concatenating a calibration frunction after calling Reaper API standard conversions - Speed improvement - Further meter precision improvement

brummbrum mentioned this pull request May 23, 2019

Volume text & Volume markers in Mixer View, Volume Callback #11

Open

jcsteh requested changes May 28, 2019

View reviewed changes

jcsteh pushed a commit that referenced this pull request Apr 25, 2022

Merge pull request #10 from brummbrum/Control_of_Button_Lights

4414a8a

Transport Button Lights working

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Peak meters calibrated pull #10

Peak meters calibrated pull #10

brummbrum commented May 20, 2019 •

edited

Loading

brummbrum commented May 20, 2019

brummbrum commented May 21, 2019

brummbrum commented May 26, 2019

brummbrum commented May 26, 2019

brummbrum commented May 27, 2019

jcsteh left a comment

jcsteh May 28, 2019

brummbrum May 28, 2019

jcsteh May 28, 2019

brummbrum May 28, 2019

jcsteh May 28, 2019

jcsteh May 28, 2019

brummbrum May 28, 2019

jcsteh May 28, 2019

brummbrum May 28, 2019

jcsteh May 28, 2019

brummbrum May 28, 2019

jcsteh May 28, 2019

brummbrum May 28, 2019

brummbrum May 28, 2019

jcsteh May 28, 2019

brummbrum May 28, 2019

brummbrum commented May 28, 2019

brummbrum commented May 28, 2019

Peak meters calibrated pull #10

Are you sure you want to change the base?

Peak meters calibrated pull #10

Conversation

brummbrum commented May 20, 2019 • edited Loading

brummbrum commented May 20, 2019

brummbrum commented May 21, 2019

brummbrum commented May 26, 2019

brummbrum commented May 26, 2019

brummbrum commented May 27, 2019

jcsteh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brummbrum commented May 28, 2019

brummbrum commented May 28, 2019

brummbrum commented May 20, 2019 •

edited

Loading