Fix BPM adjust in BpmControl

[uklotzde]

https://bugs.launchpad.net/mixxx/+bug/1836480

[JosepMaJAZ]

@uklotzde
Mmmm.. The solution looks ok, although you simply moved the limit from 200 to 300.
The limit of 300 was set as a random high limit for a tagging issue that was fixed some versions ago, related to an old software that wrote BPM values without decimal dots (as explained in trackmetadatataglib.cpp).

The bpm control is expected to not limit the range artificially, as indicated in this comment of the bpmcontrol.cpp file: "Pick a wide range (kBpmRangeMin to kBpmRangeMax) and allow out of bounds sets".

The range on the control is set so that the MIDI range (0..127) can be mapped to a float range (0..1.0).
In practice, we use pitch sliders that adjust up/down the range, not from absolute values, so the direct MIDI mapping is not relevant, other that if using the float range directly (maybe some sync code made in javascript, although reading mixxxcontrols in the wiki, it doesn't seem that we can set the bpm directly now).

So, I believe the previous math:min and math:max were an error, but the current ifs are also an unneeded limitation (ok, the minimum might be worth it, since it's zero and we don't want to go to negative numbers).

[uklotzde]

The behavior is now predictable when reaching the lower or upper limit and not causing any discontinuities. I don't have any opinion on the range or limits, other than we need to prevent values <= 0.0.

This fix = Upper/lower bounds as before, but without unexpected the behavior

[kazakore]

The Adjust BPM Down button is not limited to 300. I just set a track to 310 BPM, clicked the Adjust BPM Down button and it modified it by 0.01 as expected. Adjust BPM Up should work the same.

Currently the highest BPM in my library is exactly 300 though.....

[kazakore]

Sorry found bit in code now. I see that you only checked a lower boundary when going down and upper when going up, hence why it behaved like this.
Can you explain what the comments stating that a wider allowed range of BPM values would affect controller mappings. I can't really see why as generally everything is controlled in a relative manner, not absolute. I doubt anybody is ever going to map an absolute midi control to set track's BPM, playback speed yes, but BPM seems unlikely....
Is there even really a need for an upper limit? It seems to be 500 when typing it in manually so if there is to be one surely the same one should be used in all places (I guess that's what you're trying to consolidate here with introducing a variable for it.) As we have the three digits 999 feels nice to me.....

Not checked on this PR, only confirming on r6875 from which I posted the original bug report.

[uklotzde]

The reported bug should be fixed now, the unnecessary upper bound has been removed, and the code quality has been improved. I don't plan any other changes, because I'm not aware which side effects this might cause.

[daschuer]

Please break this line.

[daschuer]

nit pick: if (bpm >= kBpmAdjustMin + kBpmAdjustStep)
to avoid a conditional

[uklotzde]

That's not correct if you want to ensure to reach kBpmAdjustMin eventually when repeatedly lowering the tempo.

[uklotzde]

I've found an alternative and correct solution that is even more concise.

[daschuer]

LGTM, I have just added two minor comments jumping in my eyes.

just to clarify a ControlLinPotmeter needs reasonable limits, this is 1 ... 200.
This IMHO reasonable because the average of 120 is somehow in the middle.
You can't adjust more than 200 using a pot but you can use any high value using other methods.

[daschuer]

Thank you.