Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Looping: update loopsize spinbox when recalling loop with any size #12509

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/audio/frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,16 @@ class FramePos final {
return util_isfinite(m_framePosition);
}

// returns true if a is valid and is fairly close to target (within +/- 1 frame).
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// returns true if a is valid and is fairly close to target (within +/- 1 frame).
// returns true if this and target are valid and target is fairly close (within +/- 1 frame).

bool isNear(mixxx::audio::FramePos target) const {
VERIFY_OR_DEBUG_ASSERT(isValid()) {
return false;
}
return target.isValid() &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this isValid() also an debug assert?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO the caller is responsible for testing the position. It may be possible this is called with an invalid FramePos and in that case we simply return false.

value() > target.value() - 1.0 &&
value() < target.value() + 1.0;
};

void setValue(value_t framePosition) {
m_framePosition = framePosition;
}
Expand Down
76 changes: 9 additions & 67 deletions src/engine/controls/bpmcontrol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -641,64 +641,6 @@ double BpmControl::getBeatDistance(mixxx::audio::FramePos thisPosition) const {
return beatPercentage;
}

// static
bool BpmControl::getBeatContext(
const mixxx::BeatsPointer& pBeats,
mixxx::audio::FramePos position,
mixxx::audio::FramePos* pPrevBeatPosition,
mixxx::audio::FramePos* pNextBeatPosition,
mixxx::audio::FrameDiff_t* pBeatLengthFrames,
double* pBeatPercentage) {
if (!pBeats) {
return false;
}

mixxx::audio::FramePos prevBeatPosition;
mixxx::audio::FramePos nextBeatPosition;
if (!pBeats->findPrevNextBeats(position, &prevBeatPosition, &nextBeatPosition, false)) {
return false;
}

if (pPrevBeatPosition != nullptr) {
*pPrevBeatPosition = prevBeatPosition;
}

if (pNextBeatPosition != nullptr) {
*pNextBeatPosition = nextBeatPosition;
}

return getBeatContextNoLookup(position,
prevBeatPosition,
nextBeatPosition,
pBeatLengthFrames,
pBeatPercentage);
}

// static
bool BpmControl::getBeatContextNoLookup(
mixxx::audio::FramePos position,
mixxx::audio::FramePos prevBeatPosition,
mixxx::audio::FramePos nextBeatPosition,
mixxx::audio::FrameDiff_t* pBeatLengthFrames,
double* pBeatPercentage) {
if (!prevBeatPosition.isValid() || !nextBeatPosition.isValid()) {
return false;
}

const mixxx::audio::FrameDiff_t beatLengthFrames = nextBeatPosition - prevBeatPosition;
if (pBeatLengthFrames != nullptr) {
*pBeatLengthFrames = beatLengthFrames;
}

if (pBeatPercentage != nullptr) {
*pBeatPercentage = (beatLengthFrames == 0.0)
? 0.0
: (position - prevBeatPosition) / beatLengthFrames;
}

return true;
}

mixxx::audio::FramePos BpmControl::getNearestPositionInPhase(
mixxx::audio::FramePos thisPosition, bool respectLoops, bool playing) {
// Without a beatgrid, we don't know the phase offset.
Expand Down Expand Up @@ -726,7 +668,7 @@ mixxx::audio::FramePos BpmControl::getNearestPositionInPhase(
}
// This happens if dThisPosition is the target position of a requested
// seek command
if (!getBeatContext(pBeats,
if (!pBeats->getContext(
thisPosition,
&thisPrevBeatPosition,
&thisNextBeatPosition,
Expand All @@ -735,7 +677,8 @@ mixxx::audio::FramePos BpmControl::getNearestPositionInPhase(
return thisPosition;
}
} else {
if (!getBeatContextNoLookup(thisPosition,
if (!mixxx::Beats::getContextNoLookup(
thisPosition,
thisPrevBeatPosition,
thisNextBeatPosition,
&thisBeatLengthFrames,
Expand Down Expand Up @@ -775,7 +718,7 @@ mixxx::audio::FramePos BpmControl::getNearestPositionInPhase(
}

const auto otherPosition = pOtherEngineBuffer->getExactPlayPos();
if (!BpmControl::getBeatContext(otherBeats,
if (!otherBeats->getContext(
otherPosition,
nullptr,
nullptr,
Expand Down Expand Up @@ -923,8 +866,7 @@ mixxx::audio::FramePos BpmControl::getBeatMatchPosition(
}
// This happens if thisPosition is the target position of a requested
// seek command. Get new prev and next beats for the calculation.
getBeatContext(
m_pBeats,
m_pBeats->getContext(
thisPosition,
&thisPrevBeatPosition,
&thisNextBeatPosition,
Expand All @@ -942,7 +884,7 @@ mixxx::audio::FramePos BpmControl::getBeatMatchPosition(
}
// We are between the previous and next beats so we can try a standard
// lookup of the beat length.
getBeatContextNoLookup(
mixxx::Beats::getContextNoLookup(
thisPosition,
thisPrevBeatPosition,
thisNextBeatPosition,
Expand Down Expand Up @@ -979,8 +921,7 @@ mixxx::audio::FramePos BpmControl::getBeatMatchPosition(
mixxx::audio::FramePos otherNextBeatPosition;
mixxx::audio::FrameDiff_t otherBeatLengthFrames = -1;
double otherBeatFraction = -1;
if (!BpmControl::getBeatContext(
otherBeats,
if (!otherBeats->getContext(
otherPositionOfThisNextBeat,
&otherPrevBeatPosition,
&otherNextBeatPosition,
Expand Down Expand Up @@ -1277,7 +1218,8 @@ void BpmControl::collectFeatures(GroupFeatureState* pGroupFeatures) const {
m_pNextBeat.get());
mixxx::audio::FrameDiff_t beatLengthFrames;
double beatFraction;
if (getBeatContextNoLookup(info.currentPosition,
if (mixxx::Beats::getContextNoLookup(
info.currentPosition,
prevBeatPosition,
nextBeatPosition,
&beatLengthFrames,
Expand Down
19 changes: 0 additions & 19 deletions src/engine/controls/bpmcontrol.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,25 +67,6 @@ class BpmControl : public EngineControl {

void collectFeatures(GroupFeatureState* pGroupFeatures) const;

// Calculates contextual information about beats: the previous beat, the
// next beat, the current beat length, and the beat ratio (how far dPosition
// lies within the current beat). Returns false if a previous or next beat
// does not exist. NULL arguments are safe and ignored.
static bool getBeatContext(const mixxx::BeatsPointer& pBeats,
mixxx::audio::FramePos position,
mixxx::audio::FramePos* pPrevBeatPosition,
mixxx::audio::FramePos* pNextBeatPosition,
mixxx::audio::FrameDiff_t* pBeatLengthFrames,
double* pBeatPercentage);

// Alternative version that works if the next and previous beat positions
// are already known.
static bool getBeatContextNoLookup(mixxx::audio::FramePos position,
mixxx::audio::FramePos prevBeatPosition,
mixxx::audio::FramePos nextBeatPosition,
mixxx::audio::FrameDiff_t* pBeatLengthFrames,
double* pBeatPercentage);

// Returns the shortest change in percentage needed to achieve
// target_percentage.
// Example: shortestPercentageChange(0.99, 0.01) == 0.02
Expand Down
28 changes: 16 additions & 12 deletions src/engine/controls/cuecontrol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2356,10 +2356,13 @@ void CueControl::setCurrentSavedLoopControlAndActivate(HotcueControl* pControl)
mixxx::CueType type = pCue->getType();
Cue::StartAndEndPositions pos = pCue->getStartAndEndPosition();

VERIFY_OR_DEBUG_ASSERT(
type == mixxx::CueType::Loop &&
pos.startPosition.isValid() &&
pos.endPosition.isValid()) {
VERIFY_OR_DEBUG_ASSERT(type == mixxx::CueType::Loop) {
return;
}
VERIFY_OR_DEBUG_ASSERT(pos.startPosition.isValid()) {
return;
}
VERIFY_OR_DEBUG_ASSERT(pos.endPosition.isValid()) {
return;
}

Expand All @@ -2380,14 +2383,15 @@ void CueControl::slotLoopEnabledChanged(bool enabled) {
}

DEBUG_ASSERT(pSavedLoopControl->getStatus() != HotcueControl::Status::Empty);
DEBUG_ASSERT(pSavedLoopControl->getCue() &&
pSavedLoopControl->getCue()->getPosition() ==
mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(
m_pLoopStartPosition->get()));
DEBUG_ASSERT(pSavedLoopControl->getCue() &&
pSavedLoopControl->getCue()->getEndPosition() ==
mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(
m_pLoopEndPosition->get()));
DEBUG_ASSERT(pSavedLoopControl->getCue());
// Don't compare with == because there might be a tiny round-trip offset
// because LoopingControl::slotBeatLoop() uses beats to set the loop_in/_out COs
DEBUG_ASSERT(pSavedLoopControl->getCue()->getPosition().isNear(
mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(
m_pLoopStartPosition->get())));
DEBUG_ASSERT(pSavedLoopControl->getCue()->getEndPosition().isNear(
mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(
m_pLoopEndPosition->get())));

if (enabled) {
pSavedLoopControl->setStatus(HotcueControl::Status::Active);
Expand Down
18 changes: 7 additions & 11 deletions src/engine/controls/loopingcontrol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

#include "control/controlobject.h"
#include "control/controlpushbutton.h"
#include "engine/controls/bpmcontrol.h"
#include "engine/controls/enginecontrol.h"
#include "engine/controls/ratecontrol.h"
#include "engine/enginebuffer.h"
Expand All @@ -17,11 +16,6 @@

namespace {
constexpr mixxx::audio::FrameDiff_t kMinimumAudibleLoopSizeFrames = 150;

// returns true if a is valid and is fairly close to target (within +/- 1 frame).
bool positionNear(mixxx::audio::FramePos a, mixxx::audio::FramePos target) {
return a.isValid() && a > target - 1 && a < target + 1;
}
} // namespace

double LoopingControl::s_dBeatSizes[] = { 0.03125, 0.0625, 0.125, 0.25, 0.5,
Expand Down Expand Up @@ -1400,7 +1394,7 @@ bool LoopingControl::currentLoopMatchesBeatloopSize(const LoopInfo& loopInfo) co
const auto loopEndPosition = pBeats->findNBeatsFromPosition(
loopInfo.startPosition, m_pCOBeatLoopSize->get());

return positionNear(loopInfo.endPosition, loopEndPosition);
return loopEndPosition.isNear(loopInfo.endPosition);
}

bool LoopingControl::quantizeEnabledAndHasTrueTrackBeats() const {
Expand All @@ -1423,7 +1417,9 @@ double LoopingControl::findBeatloopSizeForLoop(
}
}
}
return -1;

// No hit. Calculate the fractional beat length
return pBeats->numFractionalBeatsInRange(startPosition, endPosition);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why numBeatsInRange() does not returns factions in the first place.
It could be also required here:

m_pCOBeatLoopSize->setAndConfirm(pBeats->numBeatsInRange(

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branch is for when quantize is enabled, we don't need fractional lengths then.

}

void LoopingControl::updateBeatLoopingControls() {
Expand Down Expand Up @@ -1646,8 +1642,8 @@ void LoopingControl::slotBeatLoop(double beats,
// or if the endpoints are nearly the same, do not seek forward into the adjusted loop.
if (!keepSetPoint ||
!(enable || m_bLoopingEnabled) ||
(positionNear(newloopInfo.startPosition, loopInfo.startPosition) &&
positionNear(newloopInfo.endPosition, loopInfo.endPosition))) {
(newloopInfo.startPosition.isNear(loopInfo.startPosition) &&
newloopInfo.endPosition.isNear(loopInfo.endPosition))) {
newloopInfo.seekMode = LoopSeekMode::MovedOut;
} else {
newloopInfo.seekMode = LoopSeekMode::Changed;
Expand Down Expand Up @@ -1806,7 +1802,7 @@ void LoopingControl::slotLoopMove(double beats) {
}

FrameInfo info = frameInfo();
if (BpmControl::getBeatContext(pBeats,
if (pBeats->getContext(
info.currentPosition,
nullptr,
nullptr,
Expand Down
2 changes: 1 addition & 1 deletion src/test/bpmcontrol_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ TEST_F(BpmControlTest, BeatContext_BeatGrid) {
mixxx::audio::FramePos nextBeatPosition;
mixxx::audio::FrameDiff_t beatLengthFrames;
double beatPercentage;
EXPECT_TRUE(BpmControl::getBeatContext(pBeats,
EXPECT_TRUE(pBeats->getContext(
mixxx::audio::kStartFramePos,
&prevBeatPosition,
&nextBeatPosition,
Expand Down
76 changes: 76 additions & 0 deletions src/track/beats.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -615,6 +615,59 @@ mixxx::Bpm Beats::getBpmAroundPosition(audio::FramePos position, int n) const {
return BeatUtils::calculateAverageBpm(2 * n, m_sampleRate, startPosition, endPosition);
}

bool Beats::getContext(
mixxx::audio::FramePos position,
mixxx::audio::FramePos* pPrevBeatPosition,
mixxx::audio::FramePos* pNextBeatPosition,
mixxx::audio::FrameDiff_t* pBeatLengthFrames,
double* pBeatPercentage) const {
mixxx::audio::FramePos prevBeatPosition;
mixxx::audio::FramePos nextBeatPosition;
if (!findPrevNextBeats(position, &prevBeatPosition, &nextBeatPosition, false)) {
return false;
}

if (pPrevBeatPosition != nullptr) {
*pPrevBeatPosition = prevBeatPosition;
}

if (pNextBeatPosition != nullptr) {
*pNextBeatPosition = nextBeatPosition;
}

return getContextNoLookup(
position,
prevBeatPosition,
nextBeatPosition,
pBeatLengthFrames,
pBeatPercentage);
}

// static
bool Beats::getContextNoLookup(
mixxx::audio::FramePos position,
mixxx::audio::FramePos prevBeatPosition,
mixxx::audio::FramePos nextBeatPosition,
mixxx::audio::FrameDiff_t* pBeatLengthFrames,
double* pBeatPercentage) {
if (!prevBeatPosition.isValid() || !nextBeatPosition.isValid()) {
return false;
}

const mixxx::audio::FrameDiff_t beatLengthFrames = nextBeatPosition - prevBeatPosition;
if (pBeatLengthFrames != nullptr) {
*pBeatLengthFrames = beatLengthFrames;
}

if (pBeatPercentage != nullptr) {
*pBeatPercentage = (beatLengthFrames == 0.0)
? 0.0
: (position - prevBeatPosition) / beatLengthFrames;
}

return true;
}

std::optional<BeatsPointer> Beats::tryTranslate(audio::FrameDiff_t offsetFrames) const {
std::vector<BeatMarker> markers;
std::transform(m_markers.cbegin(),
Expand Down Expand Up @@ -740,6 +793,29 @@ int Beats::numBeatsInRange(audio::FramePos startPosition, audio::FramePos endPos
return i - 2;
};

double Beats::numFractionalBeatsInRange(audio::FramePos startPos, audio::FramePos endPos) const {
double pBeatPercentage;
// get the ratio of first beat / position:
// 1 - ((startPos - beat before range) / first beat length)
if (!getContext(startPos, nullptr, nullptr, nullptr, &pBeatPercentage)) {
return -1;
}
double numBeats = 1 - pBeatPercentage;

// get the last beat ratio:
// (endPos - last beat in range) / last beat length
if (!getContext(endPos, nullptr, nullptr, nullptr, &pBeatPercentage)) {
return -1;
}
numBeats += pBeatPercentage;

// get the beats inside the range
// subtract 1 because we already counted the first beat
numBeats += numBeatsInRange(findNextBeat(startPos), endPos) - 1;

return numBeats;
}

audio::FramePos Beats::findNextBeat(audio::FramePos position) const {
return findNthBeat(position, 1);
}
Expand Down
Loading
Loading