add Rate Tap button

[ronso0]

• add tempo_tap which works exactly like BPM tap, just it sets the tempo
• change BPM display overlay and buttons in skins to do
  • tempo tap on left click
  • BPM tap on right-click.

quoting myself from #12086

Admittedly, I never understood why this a track edit tool [BPM tap button] was placed in the rate control section in the first place.

double-clicking any track (text) label opens the Track Properties editor where the (track) BPM can be set (type or tap)
Use case

Reasoning

BPM tap edits a track property (permanently) while the rate section is solely about controlling the playback speed (+, -, sync). Hence, IMO BPM tap doesn't belong there.
IIUC the intention for putting it there was to allow setting the track BPM, i.e. set initial value or correct BPM analysis. I assume it was helpful in the early days of Mixxx but with the improved beats analyzer nowadays I suppose it's not needed (that much) anymore. Manual correction can be done in Track Properties > BPM tab.

My use case

Infrequently I need to "Sync" to external players (vinyl via Aux, other DJ in b2b sessions), i.e. at least estimate the target BPM. Until now I was using a Python BPM counter [1], activated with a global hotkey, which gives very accurate results. However, it'd be much more convenient to do this within Mixxx.
Either I apply the guessed target BPM (e.g. 128) to loaded tracks, or I do a bpm search bpm:125-130.
Rate Tap is a good step forward.

Thinking this further, a BPM tap utility might be helpful, see #12105

• 1 https://github.com/m13253/bpm-counter/blob/master/trunk/bpm-counter.py

[ronso0]

Very weird: I built this locally and it works as expected.
Though, it was built with Qt5 because I forgot to set the Qt6 switch.
So with Qt6 slotUpdateRateSlider(double bpm) is not called when BPM are tapped, even though it should

mixxx/src/engine/controls/bpmcontrol.cpp

Lines 101 to 111 in 0c2faf0

	m_pEngineBpm = new ControlLinPotmeter(
	ConfigKey(group, "bpm"),
	kBpmRangeMin,
	kBpmRangeMax,
	kBpmRangeStep,
	kBpmRangeSmallStep,
	true);
	m_pEngineBpm->set(0.0);
	connect(m_pEngineBpm, &ControlObject::valueChanged,
	this, &BpmControl::slotUpdateRateSlider,
	Qt::DirectConnection);

When I change bpm from dev tools it works, it just doesn't when bpm is set in slotRateTapFilter 🤷
It also works if I make bpm read-only and use connectValueChangeRequest and copy all required calls there.

Also, I have no idea why unrelated tests fail on all OS except ARM 64, for example https://github.com/mixxxdj/mixxx/actions/runs/6517384471/job/17707390983?pr=12104

I'd appreciate if someone could give me some hints what's wrong here.

[ronso0]

I pushed a different (working) version with my findings added inline.

[ronso0]

Very weird: I built this locally and it works as expected. Though, it was built with Qt5 because I forgot to set the Qt6 switch. So with Qt6 slotUpdateRateSlider(double bpm) is not called when BPM are tapped, even though it should
...

Seems this is a known issue? Just stumbled upon

mixxx/src/broadcast/broadcastmanager.cpp

Lines 71 to 78 in 5582536

	void BroadcastManager::setEnabled(bool value) {
	m_pBroadcastEnabled->set(value);
	// TODO(Palakis): apparently, calling set on a ControlObject and not through
	// a ControlProxy doesn't trigger the valueChanged signal here.
	// This is a quick fix, but there has to be a better way to do this, rather
	// than calling the associated slot directly.
	slotControlEnabled(value);
	}

Still wondering why the comment was written for Qt5 whereas it worked here with Qt5 and fails with Qt6 🤷
I changed it to call the slot manually.

[ronso0]

I changed all skin buttons to tap the tempo on left click and tap the BPM on right click.

[ronso0]

I changed it to tempo_tap to make it more clear what it does.

Ready to roll!

[fwcd]

I assume it was helpful in the early days of Mixxx but with the improved beats analyzer nowadays I suppose it's not needed (that much) anymore

I use the BPM tap pretty frequently on tracks where the analyzer still struggles (acapellas or tracks with complex polyrhythms, e.g. Salsa music). Would appreciate to keep having at least an option for making BPM tap available in the UI, having to dig into the track properties makes this too inconvenient to really be useful IMHO.

Edit: Ah, I should've read the entire description, BPM tap on right-click seems like a good compromise, but right-clicking on a laptop trackpad is potentially still a bit cumbersome. I'm not sure of a good UI alternative though, perhaps a switch in the skin settings to toggle left/right-click behavior? Moving the BPM tap to the beatgrid editing sections seems reasonable too, to untangle the BPM editing controls from rate changing.

[ronso0]

Moving the BPM tap to the beatgrid editing sections seems reasonable too, to untangle the BPM editing controls from rate changing.

Good idea!
Feasible in Tango and LateNight, not so nice in Deere but the BPM control section is already interfering anyway with the track dsiplays.

perhaps a switch in the skin settings to toggle left/right-click behavior?

Hmm, the skin settings menu is growing and growing, in LateNight we already had to squeeze so it fits at minimum height.
At some point we need to wrap it into a WScrollable #3890 (still unused in official skins)

[daschuer]

Can you move the first commit to a separate PR?
This way we see here better the proposed functional changes.

[ronso0]

Why not view by commit?

[ronso0]

The essence is a84e206 and it's rather small IMO.

[daschuer]

Now a conflict has developed.

[ronso0]

Moving the BPM tap to the beatgrid editing sections seems reasonable too, to untangle the BPM editing controls from rate changing.

Just filed a feature request to track this.

perhaps a switch in the skin settings to toggle left/right-click behavior?

Hmm, the skin settings menu is growing and growing, in LateNight we already had to squeeze so it fits at minimum height. At some point we need to wrap it into a WScrollable #3890 (still unused in official skins)

I'll look into this during beta.

[ronso0]

Conflicts are resolved.

[daschuer]

Now the unit-test are failing.

[daschuer]

Oh sorry, and new conflicts have developed.

[ronso0]

Oh sorry, and new conflicts have developed.

Yeah, a lot has happend in BpmControl since I started this.
I rebased and it looks & works good now, let's see what CI says.

[ronso0]

small TODO:
highlight the tap button also on right-click. Not sure how to do that, though.

[Swiftb0y]

Only nitpick. rest LGTM. haven't tested though

[daschuer]

Thank you.