From 8ef9dd340c222aff6bf3daa641a02b1c62c40f8b Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Mon, 9 Aug 2021 00:22:31 +0200 Subject: [PATCH] BpmControl: Replace double sample positions with mixxx::audio::FramePos --- src/engine/controls/bpmcontrol.cpp | 561 +++++++++++++------------ src/engine/controls/bpmcontrol.h | 35 +- src/engine/controls/loopingcontrol.cpp | 2 +- src/engine/enginebuffer.cpp | 25 +- src/engine/sync/synccontrol.cpp | 2 +- src/test/bpmcontrol_test.cpp | 22 +- 6 files changed, 347 insertions(+), 300 deletions(-) diff --git a/src/engine/controls/bpmcontrol.cpp b/src/engine/controls/bpmcontrol.cpp index 2887525a044..a2c7d0979f2 100644 --- a/src/engine/controls/bpmcontrol.cpp +++ b/src/engine/controls/bpmcontrol.cpp @@ -40,7 +40,6 @@ constexpr int kBpmTapFilterLength = 80; // The local_bpm is calculated forward and backward this number of beats, so // the actual number of beats is this x2. constexpr int kLocalBpmSpan = 4; -constexpr SINT kSamplesPerFrame = 2; // If we are 1 / 8.0 beat fraction near the previous beat we match that instead // of the next beat. @@ -429,25 +428,34 @@ double BpmControl::calcSyncedRate(double userTweak) { return rate + userTweak; } - const double dPrevBeat = m_pPrevBeat->get(); - const double dNextBeat = m_pNextBeat->get(); + const auto prevBeatPosition = + mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid( + m_pPrevBeat->get()); + const auto nextBeatPosition = + mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid( + m_pNextBeat->get()); - if (dPrevBeat == -1 || dNextBeat == -1) { + if (!prevBeatPosition.isValid() || !nextBeatPosition.isValid()) { m_resetSyncAdjustment = true; return rate + userTweak; } - double dBeatLength = dNextBeat - dPrevBeat; + const double beatLengthFrames = nextBeatPosition - prevBeatPosition; // Now that we have our beat distance we can also check how large the // current loop is. If we are in a <1 beat loop, don't worry about offset. - const bool loop_enabled = m_pLoopEnabled->toBool(); - const double loop_size = (m_pLoopEndPosition->get() - - m_pLoopStartPosition->get()) / - dBeatLength; - if (loop_enabled && loop_size < 1.0 && loop_size > 0) { - m_resetSyncAdjustment = true; - return rate + userTweak; + if (m_pLoopEnabled->toBool()) { + const auto loopStartPosition = + mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid( + m_pLoopStartPosition->get()); + const auto loopEndPosition = + mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid( + m_pLoopEndPosition->get()); + const auto loopSize = (loopEndPosition - loopStartPosition) / beatLengthFrames; + if (loopSize < 1.0 && loopSize > 0) { + m_resetSyncAdjustment = true; + return rate + userTweak; + } } // Now we have all we need to calculate the sync adjustment if any. @@ -532,7 +540,7 @@ double BpmControl::calcSyncAdjustment(bool userTweakingSync) { return adjustment; } -double BpmControl::getBeatDistance(double dThisPosition) const { +double BpmControl::getBeatDistance(mixxx::audio::FramePos thisPosition) const { // We have to adjust our reported beat distance by the user offset to // preserve comparisons of beat distances. Specifically, this beat distance // is used in synccontrol to update the internal clock beat distance, and if @@ -540,140 +548,151 @@ double BpmControl::getBeatDistance(double dThisPosition) const { // sync against itself. if (kLogger.traceEnabled()) { kLogger.trace() << getGroup() - << "BpmControl::getBeatDistance. dThisPosition:" - << dThisPosition; + << "BpmControl::getBeatDistance. thisPosition:" + << thisPosition; } - double dPrevBeat = m_pPrevBeat->get(); - double dNextBeat = m_pNextBeat->get(); + mixxx::audio::FramePos prevBeatPosition = + mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid( + m_pPrevBeat->get()); + mixxx::audio::FramePos nextBeatPosition = + mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid( + m_pNextBeat->get()); - if (dPrevBeat == -1 || dNextBeat == -1) { + if (!prevBeatPosition.isValid() || !nextBeatPosition.isValid()) { return 0.0 - m_dUserOffset.getValue(); } - double dBeatLength = dNextBeat - dPrevBeat; - double dBeatPercentage = dBeatLength == 0.0 ? 0.0 : - (dThisPosition - dPrevBeat) / dBeatLength; - dBeatPercentage -= m_dUserOffset.getValue(); + const mixxx::audio::FrameDiff_t beatLengthFrames = nextBeatPosition - prevBeatPosition; + double beatPercentage = (beatLengthFrames == 0.0) + ? 0.0 + : (thisPosition - prevBeatPosition) / beatLengthFrames; + beatPercentage -= m_dUserOffset.getValue(); // Because findNext and findPrev have an epsilon built in, and because the user // might have tweaked the offset, sometimes the beat percentage is out of range. // Fix it. - if (dBeatPercentage < 0) { - ++dBeatPercentage; + if (beatPercentage < 0) { + ++beatPercentage; } - if (dBeatPercentage > 1) { - --dBeatPercentage; + if (beatPercentage > 1) { + --beatPercentage; } if (kLogger.traceEnabled()) { kLogger.trace() << getGroup() << "BpmControl::getBeatDistance. dBeatPercentage:" - << dBeatPercentage << "- offset " + << beatPercentage << "- offset " << m_dUserOffset.getValue() << " = " - << (dBeatPercentage - m_dUserOffset.getValue()); + << (beatPercentage - m_dUserOffset.getValue()); } - return dBeatPercentage; + return beatPercentage; } // static bool BpmControl::getBeatContext(const mixxx::BeatsPointer& pBeats, - const double dPosition, - double* dpPrevBeat, - double* dpNextBeat, - double* dpBeatLength, - double* dpBeatPercentage) { + mixxx::audio::FramePos position, + mixxx::audio::FramePos* pPrevBeatPosition, + mixxx::audio::FramePos* pNextBeatPosition, + mixxx::audio::FrameDiff_t* pBeatLengthFrames, + double* pBeatPercentage) { if (!pBeats) { return false; } - const auto position = mixxx::audio::FramePos::fromEngineSamplePos(dPosition); mixxx::audio::FramePos prevBeatPosition; mixxx::audio::FramePos nextBeatPosition; if (!pBeats->findPrevNextBeats(position, &prevBeatPosition, &nextBeatPosition, false)) { return false; } - if (dpPrevBeat != nullptr) { - *dpPrevBeat = prevBeatPosition.toEngineSamplePos(); + if (pPrevBeatPosition != nullptr) { + *pPrevBeatPosition = prevBeatPosition; } - if (dpNextBeat != nullptr) { - *dpNextBeat = nextBeatPosition.toEngineSamplePos(); + if (pNextBeatPosition != nullptr) { + *pNextBeatPosition = nextBeatPosition; } - return getBeatContextNoLookup(position.toEngineSamplePos(), - prevBeatPosition.toEngineSamplePos(), - nextBeatPosition.toEngineSamplePos(), - dpBeatLength, - dpBeatPercentage); + return getBeatContextNoLookup(position, + prevBeatPosition, + nextBeatPosition, + pBeatLengthFrames, + pBeatPercentage); } // static bool BpmControl::getBeatContextNoLookup( - const double dPosition, - const double dPrevBeat, - const double dNextBeat, - double* dpBeatLength, - double* dpBeatPercentage) { - if (dPrevBeat == -1 || dNextBeat == -1) { + 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; } - double dBeatLength = dNextBeat - dPrevBeat; - if (dpBeatLength != nullptr) { - *dpBeatLength = dBeatLength; + const mixxx::audio::FrameDiff_t beatLengthFrames = nextBeatPosition - prevBeatPosition; + if (pBeatLengthFrames != nullptr) { + *pBeatLengthFrames = beatLengthFrames; } - if (dpBeatPercentage != nullptr) { - *dpBeatPercentage = dBeatLength == 0.0 ? 0.0 : - (dPosition - dPrevBeat) / dBeatLength; + if (pBeatPercentage != nullptr) { + *pBeatPercentage = (beatLengthFrames == 0.0) + ? 0.0 + : (position - prevBeatPosition) / beatLengthFrames; } return true; } -double BpmControl::getNearestPositionInPhase( - double dThisPosition, bool respectLoops, bool playing) { +mixxx::audio::FramePos BpmControl::getNearestPositionInPhase( + mixxx::audio::FramePos thisPosition, bool respectLoops, bool playing) { // Without a beatgrid, we don't know the phase offset. const mixxx::BeatsPointer pBeats = m_pBeats; if (!pBeats) { - return dThisPosition; + return thisPosition; } SyncMode syncMode = getSyncMode(); // Get the current position of this deck. - double dThisPrevBeat = m_pPrevBeat->get(); - double dThisNextBeat = m_pNextBeat->get(); - double dThisBeatLength; - if (dThisPosition > dThisNextBeat || dThisPosition < dThisPrevBeat) { + mixxx::audio::FramePos thisPrevBeatPosition = + mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid( + m_pPrevBeat->get()); + mixxx::audio::FramePos thisNextBeatPosition = + mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid( + m_pNextBeat->get()); + mixxx::audio::FrameDiff_t thisBeatLengthFrames; + if (thisPrevBeatPosition.isValid() || thisNextBeatPosition.isValid() || + thisPosition > thisNextBeatPosition || + thisPosition < thisPrevBeatPosition) { if (kLogger.traceEnabled()) { kLogger.trace() << "BpmControl::getNearestPositionInPhase out of date" - << dThisPosition << dThisNextBeat << dThisPrevBeat; + << thisPosition << thisNextBeatPosition << thisPrevBeatPosition; } // This happens if dThisPosition is the target position of a requested // seek command if (!getBeatContext(pBeats, - dThisPosition, - &dThisPrevBeat, - &dThisNextBeat, - &dThisBeatLength, + thisPosition, + &thisPrevBeatPosition, + &thisNextBeatPosition, + &thisBeatLengthFrames, nullptr)) { - return dThisPosition; + return thisPosition; } } else { - if (!getBeatContextNoLookup(dThisPosition, - dThisPrevBeat, - dThisNextBeat, - &dThisBeatLength, + if (!getBeatContextNoLookup(thisPosition, + thisPrevBeatPosition, + thisNextBeatPosition, + &thisBeatLengthFrames, nullptr)) { - return dThisPosition; + return thisPosition; } } - double dOtherBeatFraction; + double otherBeatFraction; if (isFollower(syncMode)) { // If we're a follower, it's easy to get the other beat fraction - dOtherBeatFraction = m_dSyncTargetBeatDistance.getValue(); + otherBeatFraction = m_dSyncTargetBeatDistance.getValue(); } else { // If not, we have to figure it out EngineBuffer* pOtherEngineBuffer = pickSyncTarget(); @@ -688,7 +707,7 @@ double BpmControl::getNearestPositionInPhase( if (!pOtherEngineBuffer) { // no suitable sync buffer found - return dThisPosition; + return thisPosition; } TrackPointer otherTrack = pOtherEngineBuffer->getLoadedTrack(); @@ -697,24 +716,24 @@ double BpmControl::getNearestPositionInPhase( // If either track does not have beats, then we can't adjust the phase. if (!otherBeats) { - return dThisPosition; + return thisPosition; } - double dOtherPosition = pOtherEngineBuffer->getExactPlayPos(); - + const auto otherPosition = mixxx::audio::FramePos::fromEngineSamplePos( + pOtherEngineBuffer->getExactPlayPos()); if (!BpmControl::getBeatContext(otherBeats, - dOtherPosition, + otherPosition, nullptr, nullptr, nullptr, - &dOtherBeatFraction)) { - return dThisPosition; + &otherBeatFraction)) { + return thisPosition; } } - bool this_near_next = - dThisNextBeat - dThisPosition <= dThisPosition - dThisPrevBeat; - bool other_near_next = dOtherBeatFraction >= 0.5; + bool thisNearNextBeat = (thisNextBeatPosition - thisPosition) <= + (thisPosition - thisPrevBeatPosition); + bool otherNearNextBeat = otherBeatFraction >= 0.5; // We want our beat fraction to be identical to theirs. @@ -733,79 +752,83 @@ double BpmControl::getNearestPositionInPhase( // infinite beatgrids because the assumption that findNthBeat(-2) always // works will be wrong then. - double dNewPlaypos = - (dOtherBeatFraction + m_dUserOffset.getValue()) * dThisBeatLength; - if (this_near_next == other_near_next) { - dNewPlaypos += dThisPrevBeat; - } else if (this_near_next && !other_near_next) { - dNewPlaypos += dThisNextBeat; - } else { //!this_near_next && other_near_next - const auto thisBeatPosition = mixxx::audio::FramePos::fromEngineSamplePos(dThisPosition); - dThisPrevBeat = pBeats->findNthBeat(thisBeatPosition, -2).toEngineSamplePos(); - dNewPlaypos += dThisPrevBeat; - } - - if (respectLoops) { - // We might be seeking outside the loop. - const bool loop_enabled = m_pLoopEnabled->toBool(); - const double loop_start_position = m_pLoopStartPosition->get(); - const double loop_end_position = m_pLoopEndPosition->get(); - - // Cases for sanity: - // - // CASE 1 - // Two identical 1-beat loops, out of phase by X samples. - // Other deck is at its loop start. - // This deck is half way through. We want to jump forward X samples to the loop end point. - // - // Two identical 1-beat loop, out of phase by X samples. - // Other deck is - - // If sync target is 50% through the beat, - // If we are at the loop end point and hit sync, jump forward X samples. - - - // TODO(rryan): Revise this with something that keeps a broader number of - // cases in sync. This at least prevents breaking out of the loop. - if (loop_enabled && - dThisPosition <= loop_end_position) { - const double loop_length = loop_end_position - loop_start_position; - const double end_delta = dNewPlaypos - loop_end_position; - - // Syncing to after the loop end. - if (end_delta > 0 && loop_length > 0.0) { - double i = end_delta / loop_length; - dNewPlaypos = loop_start_position + end_delta - i * loop_length; - - // Move new position after loop jump into phase as well. - // This is a recursive call, called only twice because of - // respectLoops = false - dNewPlaypos = getNearestPositionInPhase(dNewPlaypos, false, playing); - } + mixxx::audio::FramePos newPlayPosition; + if (thisNearNextBeat == otherNearNextBeat) { + newPlayPosition = thisPrevBeatPosition; + } else if (thisNearNextBeat && !otherNearNextBeat) { + newPlayPosition = thisNextBeatPosition; + } else { //!thisNearNextBeat && otherNearNextBeat + thisPrevBeatPosition = pBeats->findNthBeat(thisPosition, -2); + newPlayPosition = thisPrevBeatPosition; + } + newPlayPosition += (otherBeatFraction + m_dUserOffset.getValue()) * thisBeatLengthFrames; - // Note: Syncing to before the loop beginning is allowed, because - // loops are catching - } + if (!respectLoops) { + return newPlayPosition; + } + + // We might be seeking outside the loop. + const bool loopEnabled = m_pLoopEnabled->toBool(); + const auto loopStartPosition = + mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid( + m_pLoopStartPosition->get()); + const auto loopEndPosition = + mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid( + m_pLoopEndPosition->get()); + + // Cases for sanity: + // + // CASE 1 + // Two identical 1-beat loops, out of phase by X samples. + // Other deck is at its loop start. + // This deck is half way through. We want to jump forward X samples to the loop end point. + // + // Two identical 1-beat loop, out of phase by X samples. + // Other deck is + + // If sync target is 50% through the beat, + // If we are at the loop end point and hit sync, jump forward X samples. + + // TODO(rryan): Revise this with something that keeps a broader number of + // cases in sync. This at least prevents breaking out of the loop. + if (!loopEnabled || !loopStartPosition.isValid() || + !loopEndPosition.isValid() || thisPosition > loopEndPosition) { + return newPlayPosition; + } + + const mixxx::audio::FrameDiff_t loopLengthFrames = loopEndPosition - loopStartPosition; + const mixxx::audio::FrameDiff_t endDeltaFrames = newPlayPosition - loopEndPosition; + + // Syncing to after the loop end. + if (endDeltaFrames > 0 && loopLengthFrames > 0) { + double i = endDeltaFrames / loopLengthFrames; + newPlayPosition = loopStartPosition + endDeltaFrames - i * loopLengthFrames; + + // Move new position after loop jump into phase as well. + // This is a recursive call, called only twice because of + // respectLoops = false + newPlayPosition = getNearestPositionInPhase(newPlayPosition, false, playing); } - return dNewPlaypos; + // Note: Syncing to before the loop beginning is allowed, because + // loops are catching + return newPlayPosition; } -double BpmControl::getBeatMatchPosition( - double dThisPosition, bool respectLoops, bool playing) { +mixxx::audio::FramePos BpmControl::getBeatMatchPosition( + mixxx::audio::FramePos thisPosition, bool respectLoops, bool playing) { // Without a beatgrid, we don't know the phase offset. if (!m_pBeats) { - return dThisPosition; + return thisPosition; } - const double dThisRateRatio = m_pRateRatio->get(); - if (dThisRateRatio == 0.0) { + const double thisRateRatio = m_pRateRatio->get(); + if (thisRateRatio == 0.0) { // We can't continue without a rate. // This avoids also a division by zero in the following calculations - return dThisPosition; + return thisPosition; } - EngineBuffer* pOtherEngineBuffer = nullptr; - pOtherEngineBuffer = pickSyncTarget(); + EngineBuffer* pOtherEngineBuffer = pickSyncTarget(); if (kLogger.traceEnabled()) { if (pOtherEngineBuffer) { kLogger.trace() << "BpmControl::getBeatMatchPosition sync target" @@ -824,48 +847,53 @@ double BpmControl::getBeatMatchPosition( pOtherEngineBuffer = getEngineBuffer(); } } else if (!pOtherEngineBuffer) { - return dThisPosition; + return thisPosition; } // Get the current position of this deck. - double dThisPrevBeat = m_pPrevBeat->get(); - double dThisNextBeat = m_pNextBeat->get(); - double dThisBeatLength = -1; + mixxx::audio::FramePos thisPrevBeatPosition = + mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid( + m_pPrevBeat->get()); + mixxx::audio::FramePos thisNextBeatPosition = + mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid( + m_pNextBeat->get()); + mixxx::audio::FrameDiff_t thisBeatLengthFrames; // Look up the next beat and beat length for the new position - if (dThisNextBeat == -1 || - dThisPosition > dThisNextBeat || - (dThisPrevBeat != -1 && dThisPosition < dThisPrevBeat)) { + if (!thisNextBeatPosition.isValid() || + thisPosition > thisNextBeatPosition || + (thisPrevBeatPosition.isValid() && + thisPosition < thisPrevBeatPosition)) { if (kLogger.traceEnabled()) { kLogger.trace() << "BpmControl::getBeatMatchPosition out of date" - << dThisPrevBeat << dThisPosition << dThisNextBeat; + << thisPrevBeatPosition << thisPosition << thisNextBeatPosition; } // This happens if dThisPosition is the target position of a requested // seek command. Get new prev and next beats for the calculation. getBeatContext( m_pBeats, - dThisPosition, - &dThisPrevBeat, - &dThisNextBeat, - &dThisBeatLength, + thisPosition, + &thisPrevBeatPosition, + &thisNextBeatPosition, + &thisBeatLengthFrames, nullptr); // now we either have a useful next beat or there is none - if (dThisNextBeat == -1) { + if (!thisNextBeatPosition.isValid()) { // We can't match the next beat, give up. - return dThisPosition; + return thisPosition; } } else { if (kLogger.traceEnabled()) { kLogger.trace() << "BpmControl::getBeatMatchPosition up to date" - << dThisPrevBeat << dThisPosition << dThisNextBeat; + << thisPrevBeatPosition << thisPosition << thisNextBeatPosition; } // We are between the previous and next beats so we can try a standard // lookup of the beat length. getBeatContextNoLookup( - dThisPosition, - dThisPrevBeat, - dThisNextBeat, - &dThisBeatLength, + thisPosition, + thisPrevBeatPosition, + thisNextBeatPosition, + &thisBeatLengthFrames, nullptr); } @@ -874,121 +902,130 @@ double BpmControl::getBeatMatchPosition( // If either track does not have beats, then we can't adjust the phase. if (!otherBeats) { - return dThisPosition; + return thisPosition; } - const double dOtherPosition = pOtherEngineBuffer->getExactPlayPos(); - const double dThisSampleRate = m_pBeats->getSampleRate(); + const auto otherPosition = mixxx::audio::FramePos::fromEngineSamplePos( + pOtherEngineBuffer->getExactPlayPos()); + const mixxx::audio::SampleRate thisSampleRate = m_pBeats->getSampleRate(); // Seek our next beat to the other next beat near our beat. // This is the only thing we can do if the track has different BPM, // playing the next beat together. // First calculate the position in the other track where this next beat will be. - const double thisSecs2ToNextBeat = (dThisNextBeat - dThisPosition) / - dThisSampleRate / dThisRateRatio; - const double dOtherPositionOfThisNextBeat = - thisSecs2ToNextBeat * otherBeats->getSampleRate() * pOtherEngineBuffer->getRateRatio() + - dOtherPosition; - - double dOtherPrevBeat = -1; - double dOtherNextBeat = -1; - double dOtherBeatLength = -1; - double dOtherBeatFraction = -1; + const double thisSecs2ToNextBeat = (thisNextBeatPosition - thisPosition) / + thisSampleRate / thisRateRatio; + const mixxx::audio::FramePos otherPositionOfThisNextBeat = otherPosition + + thisSecs2ToNextBeat * otherBeats->getSampleRate() * + pOtherEngineBuffer->getRateRatio(); + + mixxx::audio::FramePos otherPrevBeatPosition; + mixxx::audio::FramePos otherNextBeatPosition; + mixxx::audio::FrameDiff_t otherBeatLengthFrames = -1; + double otherBeatFraction = -1; if (!BpmControl::getBeatContext( otherBeats, - dOtherPositionOfThisNextBeat, - &dOtherPrevBeat, - &dOtherNextBeat, - &dOtherBeatLength, - &dOtherBeatFraction)) { - return dThisPosition; + otherPositionOfThisNextBeat, + &otherPrevBeatPosition, + &otherNextBeatPosition, + &otherBeatLengthFrames, + &otherBeatFraction)) { + return thisPosition; } - if (dOtherBeatLength == -1 || dOtherBeatFraction == -1) { + if (otherBeatLengthFrames == -1 || otherBeatFraction == -1) { // the other Track has no usable beat info, do not seek. - return dThisPosition; + return thisPosition; } // Offset the other deck's user offset, if any. - dOtherBeatFraction -= pOtherEngineBuffer->getUserOffset(); + otherBeatFraction -= pOtherEngineBuffer->getUserOffset(); // We can either match the past beat with dOtherBeatFraction 1.0 // or the next beat with dOtherBeatFraction 0.0 // We prefer the next because this is what will be played, // unless we are close to the previous. // This happens if the user presses play too late. - if (dOtherBeatFraction > 1.0 - kPastBeatMatchThreshold) { + if (otherBeatFraction > 1.0 - kPastBeatMatchThreshold) { // match the past beat - dOtherBeatFraction -= 1.0; + otherBeatFraction -= 1.0; } - double otherDivSec2 = dOtherBeatFraction * - dOtherBeatLength / otherBeats->getSampleRate() / pOtherEngineBuffer->getRateRatio(); + double otherDivSec2 = otherBeatFraction * otherBeatLengthFrames / + otherBeats->getSampleRate() / pOtherEngineBuffer->getRateRatio(); // Transform for this track - double seekMatch = otherDivSec2 * dThisSampleRate * dThisRateRatio; + double seekMatch = otherDivSec2 * thisSampleRate * thisRateRatio; - if (dThisBeatLength > 0) { + if (thisBeatLengthFrames > 0) { // restore phase adjustment - seekMatch += (dThisBeatLength * m_dUserOffset.getValue()); - if (dThisBeatLength / 2 < seekMatch) { + seekMatch += (thisBeatLengthFrames * m_dUserOffset.getValue()); + if (thisBeatLengthFrames / 2 < seekMatch) { // seek to previous beat, because of shorter distance - seekMatch -= dThisBeatLength; - } else if (dThisBeatLength / 2 < -seekMatch) { + seekMatch -= thisBeatLengthFrames; + } else if (thisBeatLengthFrames / 2 < -seekMatch) { // seek to beat after next, because of shorter distance - seekMatch += dThisBeatLength; + seekMatch += thisBeatLengthFrames; } } - double dNewPlaypos = dThisPosition + seekMatch; - - if (respectLoops) { - // We might be seeking outside the loop. - const bool loop_enabled = m_pLoopEnabled->toBool(); - const double loop_start_position = m_pLoopStartPosition->get(); - const double loop_end_position = m_pLoopEndPosition->get(); - - // Cases for sanity: - // - // CASE 1 - // Two identical 1-beat loops, out of phase by X samples. - // Other deck is at its loop start. - // This deck is half way through. We want to jump forward X samples to the loop end point. - // - // Two identical 1-beat loop, out of phase by X samples. - // Other deck is - - // If sync target is 50% through the beat, - // If we are at the loop end point and hit sync, jump forward X samples. - - // TODO(rryan): Revise this with something that keeps a broader number of - // cases in sync. This at least prevents breaking out of the loop. - if (loop_enabled && - dThisPosition <= loop_end_position) { - const double loop_length = loop_end_position - loop_start_position; - const double end_delta = dNewPlaypos - loop_end_position; - - // Syncing to after the loop end. - if (end_delta > 0 && loop_length > 0.0) { - double i = end_delta / loop_length; - dNewPlaypos = loop_start_position + end_delta - i * loop_length; - - // Move new position after loop jump into phase as well. - // This is a recursive call, called only twice because of - // respectLoops = false - dNewPlaypos = getNearestPositionInPhase(dNewPlaypos, false, playing); - } + mixxx::audio::FramePos newPlayPosition = thisPosition + seekMatch; - // Note: Syncing to before the loop beginning is allowed, because - // loops are catching - } + if (!respectLoops) { + return newPlayPosition; } - return dNewPlaypos; + + // We might be seeking outside the loop. + const bool loopEnabled = m_pLoopEnabled->toBool(); + const auto loopStartPosition = + mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid( + m_pLoopStartPosition->get()); + const auto loopEndPosition = + mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid( + m_pLoopEndPosition->get()); + + // Cases for sanity: + // + // CASE 1 + // Two identical 1-beat loops, out of phase by X samples. + // Other deck is at its loop start. + // This deck is half way through. We want to jump forward X samples to the loop end point. + // + // Two identical 1-beat loop, out of phase by X samples. + // Other deck is + + // If sync target is 50% through the beat, + // If we are at the loop end point and hit sync, jump forward X samples. + + // TODO(rryan): Revise this with something that keeps a broader number of + // cases in sync. This at least prevents breaking out of the loop. + if (!loopEnabled || !loopStartPosition.isValid() || + !loopEndPosition.isValid() || thisPosition > loopEndPosition) { + return newPlayPosition; + } + + const mixxx::audio::FrameDiff_t loopLengthFrames = loopEndPosition - loopStartPosition; + const mixxx::audio::FrameDiff_t endDeltaFrames = newPlayPosition - loopEndPosition; + + // Syncing to after the loop end. + if (endDeltaFrames > 0 && loopLengthFrames > 0) { + double i = endDeltaFrames / loopLengthFrames; + newPlayPosition = loopStartPosition + endDeltaFrames - i * loopLengthFrames; + + // Move new position after loop jump into phase as well. + // This is a recursive call, called only twice because of + // respectLoops = false + newPlayPosition = getNearestPositionInPhase(newPlayPosition, false, playing); + } + + // Note: Syncing to before the loop beginning is allowed, because + // loops are catching + return newPlayPosition; } -double BpmControl::getPhaseOffset(double dThisPosition) { +mixxx::audio::FrameDiff_t BpmControl::getPhaseOffset(mixxx::audio::FramePos thisPosition) { // This does not respect looping - double dNewPlaypos = getNearestPositionInPhase(dThisPosition, false, false); - return dNewPlaypos - dThisPosition; + const mixxx::audio::FramePos position = getNearestPositionInPhase(thisPosition, false, false); + return position - thisPosition; } void BpmControl::slotUpdateEngineBpm(double value) { @@ -1061,8 +1098,7 @@ void BpmControl::slotBeatsTranslateMatchAlignment(double v) { // otherwise it will always return 0 if sync lock is active. m_dUserOffset.setValue(0.0); - double sampleOffset = getPhaseOffset(frameInfo().currentPosition.toEngineSamplePos()); - const mixxx::audio::FrameDiff_t frameOffset = sampleOffset / mixxx::kEngineChannelCount; + const mixxx::audio::FrameDiff_t frameOffset = getPhaseOffset(frameInfo().currentPosition); pTrack->trySetBeats(pBeats->translate(-frameOffset)); } } @@ -1093,7 +1129,7 @@ mixxx::Bpm BpmControl::updateLocalBpm() { } double BpmControl::updateBeatDistance() { - double beatDistance = getBeatDistance(frameInfo().currentPosition.toEngineSamplePos()); + double beatDistance = getBeatDistance(frameInfo().currentPosition); m_pThisBeatDistance->set(beatDistance); if (!isSynchronized() && m_dUserOffset.getValue() != 0.0) { m_dUserOffset.setValue(0.0); @@ -1134,26 +1170,29 @@ void BpmControl::collectFeatures(GroupFeatureState* pGroupFeatures) const { } // Get the current position of this deck. - double dThisPrevBeat = m_pPrevBeat->get(); - double dThisNextBeat = m_pNextBeat->get(); - double dThisBeatLength; - double dThisBeatFraction; - if (getBeatContextNoLookup(info.currentPosition.toEngineSamplePos(), - dThisPrevBeat, - dThisNextBeat, - &dThisBeatLength, - &dThisBeatFraction)) { - // Note: dThisBeatLength is fractional frames count * 2 (stereo samples) - double samplesPerSecond = kSamplesPerFrame * info.sampleRate * m_pRateRatio->get(); - if (samplesPerSecond != 0.0) { - pGroupFeatures->beat_length_sec = dThisBeatLength / samplesPerSecond; + mixxx::audio::FramePos prevBeatPosition = + mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid( + m_pPrevBeat->get()); + mixxx::audio::FramePos nextBeatPosition = + mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid( + m_pNextBeat->get()); + mixxx::audio::FrameDiff_t beatLengthFrames; + double beatFraction; + if (getBeatContextNoLookup(info.currentPosition, + prevBeatPosition, + nextBeatPosition, + &beatLengthFrames, + &beatFraction)) { + const double framesPerSecond = info.sampleRate * m_pRateRatio->get(); + if (framesPerSecond != 0.0) { + pGroupFeatures->beat_length_sec = beatLengthFrames / framesPerSecond; pGroupFeatures->has_beat_length_sec = true; } else { pGroupFeatures->has_beat_length_sec = false; } pGroupFeatures->has_beat_fraction = true; - pGroupFeatures->beat_fraction = dThisBeatFraction; + pGroupFeatures->beat_fraction = beatFraction; } } diff --git a/src/engine/controls/bpmcontrol.h b/src/engine/controls/bpmcontrol.h index f3bcaf3de0f..f29505db862 100644 --- a/src/engine/controls/bpmcontrol.h +++ b/src/engine/controls/bpmcontrol.h @@ -39,12 +39,18 @@ class BpmControl : public EngineControl { // out of sync. double calcSyncedRate(double userTweak); // Get the phase offset from the specified position. - double getNearestPositionInPhase(double dThisPosition, bool respectLoops, bool playing); - double getBeatMatchPosition(double dThisPosition, bool respectLoops, bool playing); - double getPhaseOffset(double dThisPosition); + mixxx::audio::FramePos getNearestPositionInPhase( + mixxx::audio::FramePos thisPosition, + bool respectLoops, + bool playing); + mixxx::audio::FramePos getBeatMatchPosition( + mixxx::audio::FramePos thisPosition, + bool respectLoops, + bool playing); + double getPhaseOffset(mixxx::audio::FramePos thisPosition); /// getBeatDistance is adjusted to include the user offset so it's /// transparent to other decks. - double getBeatDistance(double dThisPosition) const; + double getBeatDistance(mixxx::audio::FramePos thisPosition) const; double getUserOffset() const { return m_dUserOffset.getValue(); } @@ -64,20 +70,19 @@ class BpmControl : public EngineControl { // 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, - const double dPosition, - double* dpPrevBeat, - double* dpNextBeat, - double* dpBeatLength, - double* dpBeatPercentage); + 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( - const double dPosition, - const double dPrevBeat, - const double dNextBeat, - double* dpBeatLength, - double* dpBeatPercentage); + 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. diff --git a/src/engine/controls/loopingcontrol.cpp b/src/engine/controls/loopingcontrol.cpp index 33681589965..3ca85d824cb 100644 --- a/src/engine/controls/loopingcontrol.cpp +++ b/src/engine/controls/loopingcontrol.cpp @@ -1450,7 +1450,7 @@ void LoopingControl::slotLoopMove(double beats) { } if (BpmControl::getBeatContext(pBeats, - m_currentPosition.getValue().toEngineSamplePos(), + m_currentPosition.getValue(), nullptr, nullptr, nullptr, diff --git a/src/engine/enginebuffer.cpp b/src/engine/enginebuffer.cpp index b0c286aafa1..c7064aad632 100644 --- a/src/engine/enginebuffer.cpp +++ b/src/engine/enginebuffer.cpp @@ -1247,7 +1247,7 @@ void EngineBuffer::processSeek(bool paused) { const QueuedSeek queuedSeek = m_queuedSeek.getValue(); SeekRequests seekType = queuedSeek.seekType; - mixxx::audio::FramePos framePosition = queuedSeek.position; + mixxx::audio::FramePos position = queuedSeek.position; // Add SEEK_PHASE bit, if any if (m_iSeekPhaseQueued.fetchAndStoreRelease(0)) { @@ -1259,7 +1259,7 @@ void EngineBuffer::processSeek(bool paused) { return; case SEEK_PHASE: // only adjust phase - framePosition = mixxx::audio::FramePos::fromEngineSamplePos(m_filepos_play); + position = mixxx::audio::FramePos::fromEngineSamplePos(m_filepos_play); break; case SEEK_STANDARD: if (m_pQuantize->toBool()) { @@ -1278,38 +1278,35 @@ void EngineBuffer::processSeek(bool paused) { return; } - VERIFY_OR_DEBUG_ASSERT(framePosition.isValid()) { + VERIFY_OR_DEBUG_ASSERT(position.isValid()) { return; } - auto position = framePosition.toEngineSamplePos(); + const auto trackEndPosition = mixxx::audio::FramePos::fromEngineSamplePos(m_trackSamplesOld); // Don't allow the playposition to go past the end. - if (position > m_trackSamplesOld) { - position = m_trackSamplesOld; + if (position > trackEndPosition) { + position = trackEndPosition; } if (!paused && (seekType & SEEK_PHASE)) { if (kLogger.traceEnabled()) { kLogger.trace() << "EngineBuffer::processSeek" << getGroup() << "Seeking phase"; } - double syncPosition = m_pBpmControl->getBeatMatchPosition(position, true, true); - framePosition = - m_pLoopingControl->getSyncPositionInsideLoop(framePosition, - mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid( - syncPosition)); - position = framePosition.toEngineSamplePosMaybeInvalid(); + const mixxx::audio::FramePos syncPosition = + m_pBpmControl->getBeatMatchPosition(position, true, true); + position = m_pLoopingControl->getSyncPositionInsideLoop(position, syncPosition); if (kLogger.traceEnabled()) { kLogger.trace() << "EngineBuffer::processSeek" << getGroup() << "seek info:" << m_filepos_play << "->" << position; } } - if (position != m_filepos_play) { + if (position.toEngineSamplePos() != m_filepos_play) { if (kLogger.traceEnabled()) { kLogger.trace() << "EngineBuffer::processSeek" << getGroup() << "Seek to" << position; } - setNewPlaypos(position); + setNewPlaypos(position.toEngineSamplePos()); m_previousBufferSeek = true; } // Reset the m_queuedSeek value after it has been processed in diff --git a/src/engine/sync/synccontrol.cpp b/src/engine/sync/synccontrol.cpp index 6eb0b1c4dc5..d0307a83d32 100644 --- a/src/engine/sync/synccontrol.cpp +++ b/src/engine/sync/synccontrol.cpp @@ -297,7 +297,7 @@ void SyncControl::updateTargetBeatDistance() { targetDistance *= kBpmHalve; // Our beat distance CO is still a buffer behind, so take the current value. if (m_pBpmControl->getBeatDistance( - frameInfo().currentPosition.toEngineSamplePos()) >= 0.5) { + frameInfo().currentPosition) >= 0.5) { targetDistance += 0.5; } } diff --git a/src/test/bpmcontrol_test.cpp b/src/test/bpmcontrol_test.cpp index 18750d7fc38..33f4b6a3cda 100644 --- a/src/test/bpmcontrol_test.cpp +++ b/src/test/bpmcontrol_test.cpp @@ -34,18 +34,24 @@ TEST_F(BpmControlTest, BeatContext_BeatGrid) { mixxx::Duration::fromSeconds(180)); const auto bpm = mixxx::Bpm(60.0); - const int kFrameSize = 2; - const double expectedBeatLength = (60.0 * sampleRate / bpm.value()) * kFrameSize; + const mixxx::audio::FrameDiff_t expectedBeatLengthFrames = (60.0 * sampleRate / bpm.value()); const mixxx::BeatsPointer pBeats = BeatFactory::makeBeatGrid( pTrack->getSampleRate(), bpm, mixxx::audio::kStartFramePos); // On a beat. - double prevBeat, nextBeat, beatLength, beatPercentage; - EXPECT_TRUE(BpmControl::getBeatContext(pBeats, 0.0, &prevBeat, &nextBeat, - &beatLength, &beatPercentage)); - EXPECT_DOUBLE_EQ(0.0, prevBeat); - EXPECT_DOUBLE_EQ(beatLength, nextBeat); - EXPECT_DOUBLE_EQ(expectedBeatLength, beatLength); + mixxx::audio::FramePos prevBeatPosition; + mixxx::audio::FramePos nextBeatPosition; + mixxx::audio::FrameDiff_t beatLengthFrames; + double beatPercentage; + EXPECT_TRUE(BpmControl::getBeatContext(pBeats, + mixxx::audio::kStartFramePos, + &prevBeatPosition, + &nextBeatPosition, + &beatLengthFrames, + &beatPercentage)); + EXPECT_EQ(mixxx::audio::kStartFramePos, prevBeatPosition); + EXPECT_EQ(mixxx::audio::FramePos{beatLengthFrames}, nextBeatPosition); + EXPECT_DOUBLE_EQ(expectedBeatLengthFrames, beatLengthFrames); EXPECT_DOUBLE_EQ(0.0, beatPercentage); }