Fix debug assertion in BeatFactory::makePreferredBeats() #4270
Conversation
| auto pGrid = mixxx::BeatGrid::makeBeatGrid( | ||
| sampleRate, constBPM, firstBeat.toNearestFrameBoundary(), subVersion); | ||
| return pGrid; | ||
| if (firstBeat.isValid()) { |
There was a problem hiding this comment.
The method is only called by analyzerbeats. Can we move the check there and leave the debug assertion intact?
There was a problem hiding this comment.
Sure. I only wonder why the assertion was triggered by firstBeat.toNearestFrameBoundary(), because adjustPhase() already requires a valid value? firstBeat is invalidated by makeConstBpm().
There was a problem hiding this comment.
Em, no, I don't think we can't move this check out of BeatFactory::makePreferredBeats(). Will test again.
Also edge cases with a single beat are not handled correctly. It doesn't make any sense to infer a tempo for less than 2 beats.
…erredBeats # Conflicts: # src/track/beatutils.cpp
…erredBeats # Conflicts: # src/track/beatfactory.cpp
|
Ping. I suggest to keep the input validation and handling inside the functions instead of making assumptions by whom and how they are called. The only exception is to assume that all mandatory parameters are valid, e.g. the sample rate. |
|
I'm currently traveling so I cannot retest this. But the code looks good to me. No objections. |
|
Ok than I will hit merge. Thank you. |
Fixes #4267 (comment)