Fix BPM validation checks to prevent invalid value access

[shbhmexe]

Fixes 3 critical issues where mixxx::Bpm::value() was accessed without isValid() checks, risking debug assertions and undefined behavior.

Changes

• beatutils.cpp: Added validation in roundBpmWithinRange to return early if inputs are invalid.
• beatutils.cpp: Added check in adjustPhase to prevent division by zero/invalid BPM.
• bpmcontrol.cpp: Added validation in adjustBeatsBpm before value access.

Verification

• Static Check: Verified all new value() calls are guarded by isValid().
• Logic: Confirmed invalid inputs trigger safe fallbacks/returns.
• Build: Compiles successfully; no logic breaking changes.

[daschuer]

Thank you for this fix.

As a first-time contributor we need you to sign the Mixxx Contributor Agreement and comment here when you have done so. It gives us permission to distribute your contribution under the GPL v2 or later license and the Apple Mac App Store. It is also helpful for us to have contact information for contributors in case we may need it in the future.

I think all branches are affected. Can you rebase it to 2.5

git rebase --onto=upstream/2.5 HEAD~1
git push -f

This way it will become outo merged to all branches.

[daschuer]

The Fixme comments have been introduced here:
#4165

[daschuer]

The code looks good, I have added comments for improvement.

[daschuer]

These comments are redundant, because they are explaining C++ not the intention. How about this:

Suggested change

	// First validate all BPM values
	if (!minBpm.isValid() || !centerBpm.isValid() || !maxBpm.isValid()) {
	// If any BPM is invalid, return the centerBpm as-is
	return centerBpm;
	// If any BPM is invalid, return the centerBpm as-is to avoid
	// unexpected results in the following calculations
	if (!minBpm.isValid() || !centerBpm.isValid() || !maxBpm.isValid()) {
	return centerBpm;

[daschuer]

Can you also add here a check:
https://github.com/daschuer/mixxx/blob/8720b7595fbbecd62d20c89bf66ff500adce5408/src/track/beatutils.cpp#L310
else we will have div by zero below.

[daschuer]

Can you add this check as well to avoid division by zero?

[daschuer]

adjustPhase() is called here with a isValid() check before. Now we have it inside which allows us to move it before the call here:
https://github.com/daschuer/mixxx/blob/8720b7595fbbecd62d20c89bf66ff500adce5408/src/track/beatfactory.cpp#L83

The comment has also no value:

Suggested change

	// Validate BPM before using it
	if (!bpm.isValid()) {
	// If BPM is invalid, return firstBeat without adjustment
	if (!bpm.isValid()) {

[daschuer]

The change at the cited code position has not been fixed. Can you have a look?

[shbhmexe]

All changes as requested have been completed. Please let me know if there are any other requests in this PR.

[acolombier]

@daschuer have all your change requests been addresed? Is the rebase the only thing missing?

[daschuer]

I can confirm that @shbhmexe has signed the contributor agreement. Thank you.

[github-actions]

This PR is marked as stale because it has been open 90 days with no activity.