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

Reworked beat_active control: #3608

Merged
merged 18 commits into from
Mar 20, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
4719d5e
Reworked beat_active control:
JoergAtGithub Jan 30, 2021
143f255
Added switch, to select if findPrevNextBeats should compare with or w…
JoergAtGithub Feb 21, 2021
1e6d066
Added testcases for NoTolerance mode of findPrevNextBeats
JoergAtGithub Feb 21, 2021
f8c60f3
Merge remote-tracking branch 'upstream/main' into improve_beat_indicator
JoergAtGithub Feb 21, 2021
1a8f2c2
Improved if cases for NoTolerance code in findPrevNextBeats
JoergAtGithub Feb 21, 2021
d407101
Merge remote-tracking branch 'upstream/main' into improve_beat_indicator
JoergAtGithub Feb 22, 2021
92c48ce
Merge remote-tracking branch 'upstream/main' into improve_beat_indicator
JoergAtGithub Feb 23, 2021
5cb6696
Moved the 5 states into an internal state machine
JoergAtGithub Feb 27, 2021
dd1b2b1
Added the loop cases:
JoergAtGithub Feb 27, 2021
6adb30c
Removed getBeatContextNoLookup() by using findPrevNextBeats without t…
JoergAtGithub Mar 1, 2021
8b88974
Renamed NoTolerance to snapToNearBeats and inverted the bool, to matc…
JoergAtGithub Mar 1, 2021
93feabe
const -> constexpr
JoergAtGithub Mar 2, 2021
fd64411
Added ToDo comment about moving the updateIndicators to waveform upda…
JoergAtGithub Mar 2, 2021
c3493ea
Handled the cue point case. Don't show beat indication after (hot) cu…
JoergAtGithub Mar 5, 2021
d7ab0d0
Changed reverse beat indication state from -1 to +2
JoergAtGithub Mar 5, 2021
9a00a1e
Merge branch 'main' into improve_beat_indicator
JoergAtGithub Mar 5, 2021
f04c35a
pre-commit
JoergAtGithub Mar 5, 2021
277ed26
Improved responsiveness when pressing play/cue button at while track …
JoergAtGithub Mar 6, 2021
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
2 changes: 1 addition & 1 deletion src/engine/controls/bpmcontrol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ bool BpmControl::getBeatContext(const mixxx::BeatsPointer& pBeats,

double dPrevBeat;
double dNextBeat;
if (!pBeats->findPrevNextBeats(dPosition, &dPrevBeat, &dNextBeat)) {
if (!pBeats->findPrevNextBeats(dPosition, &dPrevBeat, &dNextBeat, false)) {
daschuer marked this conversation as resolved.
Show resolved Hide resolved
return false;
}

Expand Down
98 changes: 84 additions & 14 deletions src/engine/controls/clockcontrol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,16 @@
ClockControl::ClockControl(const QString& group, UserSettingsPointer pConfig)
: EngineControl(group, pConfig) {
m_pCOBeatActive = new ControlObject(ConfigKey(group, "beat_active"));
m_pCOBeatActive->set(0.0);
m_pCOSampleRate = new ControlProxy("[Master]","samplerate");
m_pCOBeatActive->setReadOnly();
m_pCOBeatActive->forceSet(0.0);
m_lastEvaluatedSample = 0;
m_PrevBeatSamples = 0;
m_PrevBeatSamples = 0;
m_blinkIntervalSamples = 0;
}

ClockControl::~ClockControl() {
delete m_pCOBeatActive;
delete m_pCOSampleRate;
}

// called from an engine worker thread
Expand All @@ -30,27 +33,94 @@ void ClockControl::trackLoaded(TrackPointer pNewTrack) {

void ClockControl::trackBeatsUpdated(mixxx::BeatsPointer pBeats) {
// Clear on-beat control
m_pCOBeatActive->set(0.0);
m_pCOBeatActive->forceSet(0.0);
m_pBeats = pBeats;
}

void ClockControl::process(const double dRate,
const double currentSample,
const int iBuffersize) {
const double currentSample,
const int iBuffersize) {
Q_UNUSED(dRate);
Q_UNUSED(currentSample);
Q_UNUSED(iBuffersize);
double samplerate = m_pCOSampleRate->get();
}

void ClockControl::updateIndicators(const double dRate,
const double currentSample) {
/* This method sets the control beat_active is set to the following values:
* +1.0 --> Forward playing, set at the beat and set back to 0.0 at 20% of beat distance
* +0.5 --> Direction changed to reverse playing while forward playing indication was on
* 0.0 --> No beat indication
* -0.5 --> Direction changed to forward playing while reverse playing indication was on
* -1.0 --> Reverse playing, set at the beat and set back to 0.0 at -20% of beat distance
JoergAtGithub marked this conversation as resolved.
Show resolved Hide resolved
*/

// TODO(XXX) should this be customizable, or latency dependent?
const double blinkSeconds = 0.100;
const double kBlinkInterval = 0.20; // LED is on 20% of the beat period
JoergAtGithub marked this conversation as resolved.
Show resolved Hide resolved

// Multiply by two to get samples from frames. Interval is scaled linearly
// by the rate.
const double blinkIntervalSamples = 2.0 * samplerate * (1.0 * dRate) * blinkSeconds;
if ((currentSample == m_lastEvaluatedSample) ||
(dRate == 0.0)) {
return; // No position change (e.g. deck stopped) -> No indicator update needed
}

mixxx::BeatsPointer pBeats = m_pBeats;
if (pBeats) {
double closestBeat = pBeats->findClosestBeat(currentSample);
double distanceToClosestBeat = fabs(currentSample - closestBeat);
m_pCOBeatActive->set(distanceToClosestBeat < blinkIntervalSamples / 2.0);
if ((currentSample >= m_NextBeatSamples) ||
(currentSample <= m_PrevBeatSamples)) {
//qDebug() << "### findPrevNextBeats ### " << " currentSample: " << currentSample << " m_lastEvaluatedSample: " << m_lastEvaluatedSample << " m_PrevBeatSamples: " << m_PrevBeatSamples << " m_NextBeatSamples: " << m_NextBeatSamples;

pBeats->findPrevNextBeats(currentSample,
&m_PrevBeatSamples,
&m_NextBeatSamples,
true); // Precise compare without tolerance needed

m_blinkIntervalSamples = (m_NextBeatSamples - m_PrevBeatSamples) * kBlinkInterval;
}
//qDebug() << "dRate:" << dRate << " m_lastPlayDirection:" << m_lastPlayDirection << " m_pCOBeatActive->get(): " << m_pCOBeatActive->get() << " currentSample: " << currentSample << " m_lastEvaluatedSample: " << m_lastEvaluatedSample << " m_PrevBeatSamples: " << m_PrevBeatSamples << " m_NextBeatSamples: " << m_NextBeatSamples << " m_blinkIntervalSamples: " << m_blinkIntervalSamples;
Copy link
Member

Choose a reason for hiding this comment

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

Please break these long lines.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will remove this line later - it's just for my own debugging.


if (dRate >= 0.0) {
if (m_lastPlayDirection == true) {
if ((currentSample > m_PrevBeatSamples) &&
(currentSample <
m_PrevBeatSamples + m_blinkIntervalSamples) &&
(m_pCOBeatActive->get() < 0.5)) {
m_pCOBeatActive->forceSet(1.0);
} else if ((currentSample > m_PrevBeatSamples +
m_blinkIntervalSamples) &&
(m_pCOBeatActive->get() > 0.0)) {
m_pCOBeatActive->forceSet(0.0);
}
} else {
// Play direction changed while beat indicator was on and forward playing
if ((currentSample < m_NextBeatSamples) &&
(currentSample >=
m_NextBeatSamples - m_blinkIntervalSamples)) {
m_pCOBeatActive->forceSet(-0.5);
}
}
m_lastPlayDirection = true; // Forward
} else {
if (m_lastPlayDirection == false) {
if ((currentSample < m_NextBeatSamples) &&
(currentSample >
m_NextBeatSamples - m_blinkIntervalSamples) &&
(m_pCOBeatActive->get() > -0.5)) {
m_pCOBeatActive->forceSet(-1.0);
} else if ((currentSample < m_NextBeatSamples -
m_blinkIntervalSamples) &&
(m_pCOBeatActive->get() < 0.0)) {
m_pCOBeatActive->forceSet(0.0);
}
} else {
// Play direction changed while beat indicator was on and reverse playing
if ((currentSample > m_PrevBeatSamples) &&
(currentSample <=
m_PrevBeatSamples + m_blinkIntervalSamples)) {
m_pCOBeatActive->forceSet(0.5);
}
}
m_lastPlayDirection = false; // Reverse
}
m_lastEvaluatedSample = currentSample;
}
Copy link
Member

Choose a reason for hiding this comment

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

I think I would prefer

Suggested change
}
} else {
m_pCOBeatActive->forceSet(0.0);
}

Because I experience that the LED is sometimes on and sometimes off when paused at a cue point.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will look how to solve the cue point issue. The proposed else branch will not do this. It will never be reached, because this is captured before, to not switch the indicator off due the jitter of the DVS pitch.

Copy link
Member Author

Choose a reason for hiding this comment

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

I applied a fix for this, which should not break the use case of searching the beat position using DVS or jog. Please test, if this solves the cue point issue for you too.

}
13 changes: 12 additions & 1 deletion src/engine/controls/clockcontrol.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,23 @@ class ClockControl: public EngineControl {
void process(const double dRate, const double currentSample,
const int iBufferSize) override;

void updateIndicators(const double dRate,
const double currentSample);

void trackLoaded(TrackPointer pNewTrack) override;
void trackBeatsUpdated(mixxx::BeatsPointer pBeats) override;

private:
ControlObject* m_pCOBeatActive;
ControlProxy* m_pCOSampleRate;

double m_lastEvaluatedSample;

double m_PrevBeatSamples;
double m_NextBeatSamples;
double m_blinkIntervalSamples;

// True is forward direction, False is reverse
bool m_lastPlayDirection;

// m_pBeats is written from an engine worker thread
mixxx::BeatsPointer m_pBeats;
Expand Down
2 changes: 1 addition & 1 deletion src/engine/controls/loopingcontrol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1194,7 +1194,7 @@ void LoopingControl::slotBeatLoop(double beats, bool keepStartPoint, bool enable
// The closest beat might be ahead of play position and will cause a catching loop.
double prevBeat;
double nextBeat;
pBeats->findPrevNextBeats(currentSample, &prevBeat, &nextBeat);
pBeats->findPrevNextBeats(currentSample, &prevBeat, &nextBeat, false);

if (m_pQuantizeEnabled->toBool() && prevBeat != -1) {
double beatLength = nextBeat - prevBeat;
Expand Down
2 changes: 1 addition & 1 deletion src/engine/controls/quantizecontrol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ void QuantizeControl::lookupBeatPositions(double dCurrentSample) {
mixxx::BeatsPointer pBeats = m_pBeats;
if (pBeats) {
double prevBeat, nextBeat;
pBeats->findPrevNextBeats(dCurrentSample, &prevBeat, &nextBeat);
pBeats->findPrevNextBeats(dCurrentSample, &prevBeat, &nextBeat, false);
m_pCOPrevBeat->set(prevBeat);
m_pCONextBeat->set(nextBeat);
}
Expand Down
2 changes: 2 additions & 0 deletions src/engine/enginebuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1350,6 +1350,8 @@ void EngineBuffer::updateIndicators(double speed, int iBufferSize) {
(double)iBufferSize / m_trackSamplesOld,
fractionalPlayposFromAbsolute(m_dSlipPosition),
tempoTrackSeconds);

m_pClockControl->updateIndicators(speed * m_baserate_old, m_filepos_play);
}

void EngineBuffer::hintReader(const double dRate) {
Expand Down
28 changes: 24 additions & 4 deletions src/test/beatgridtest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,12 @@ TEST(BeatGridTest, TestNthBeatWhenOnBeat) {

// Also test prev/next beat calculation.
double prevBeat, nextBeat;
pGrid->findPrevNextBeats(position, &prevBeat, &nextBeat);
pGrid->findPrevNextBeats(position, &prevBeat, &nextBeat, false);
EXPECT_NEAR(position, prevBeat, kMaxBeatError);
EXPECT_NEAR(position + beatLength, nextBeat, kMaxBeatError);

// Also test prev/next beat calculation without tolerance
pGrid->findPrevNextBeats(position, &prevBeat, &nextBeat, true);
EXPECT_NEAR(position, prevBeat, kMaxBeatError);
EXPECT_NEAR(position + beatLength, nextBeat, kMaxBeatError);

Expand Down Expand Up @@ -115,10 +120,15 @@ TEST(BeatGridTest, TestNthBeatWhenOnBeat_BeforeEpsilon) {

// Also test prev/next beat calculation.
double prevBeat, nextBeat;
pGrid->findPrevNextBeats(position, &prevBeat, &nextBeat);
pGrid->findPrevNextBeats(position, &prevBeat, &nextBeat, false);
EXPECT_NEAR(kClosestBeat, prevBeat, kMaxBeatError);
EXPECT_NEAR(kClosestBeat + beatLength, nextBeat, kMaxBeatError);

// Also test prev/next beat calculation without tolerance
pGrid->findPrevNextBeats(position, &prevBeat, &nextBeat, true);
EXPECT_NEAR(kClosestBeat - beatLength, prevBeat, kMaxBeatError);
EXPECT_NEAR(kClosestBeat, nextBeat, kMaxBeatError);

// Both previous and next beat should return the closest beat.
EXPECT_NEAR(kClosestBeat, pGrid->findNextBeat(position), kMaxBeatError);
EXPECT_NEAR(kClosestBeat, pGrid->findPrevBeat(position), kMaxBeatError);
Expand Down Expand Up @@ -152,7 +162,12 @@ TEST(BeatGridTest, TestNthBeatWhenOnBeat_AfterEpsilon) {

// Also test prev/next beat calculation.
double prevBeat, nextBeat;
pGrid->findPrevNextBeats(position, &prevBeat, &nextBeat);
pGrid->findPrevNextBeats(position, &prevBeat, &nextBeat, false);
EXPECT_NEAR(kClosestBeat, prevBeat, kMaxBeatError);
EXPECT_NEAR(kClosestBeat + beatLength, nextBeat, kMaxBeatError);

// Also test prev/next beat calculation without tolerance
pGrid->findPrevNextBeats(position, &prevBeat, &nextBeat, true);
EXPECT_NEAR(kClosestBeat, prevBeat, kMaxBeatError);
EXPECT_NEAR(kClosestBeat + beatLength, nextBeat, kMaxBeatError);

Expand Down Expand Up @@ -190,7 +205,12 @@ TEST(BeatGridTest, TestNthBeatWhenNotOnBeat) {

// Also test prev/next beat calculation
double foundPrevBeat, foundNextBeat;
pGrid->findPrevNextBeats(position, &foundPrevBeat, &foundNextBeat);
pGrid->findPrevNextBeats(position, &foundPrevBeat, &foundNextBeat, false);
EXPECT_NEAR(previousBeat, foundPrevBeat, kMaxBeatError);
EXPECT_NEAR(nextBeat, foundNextBeat, kMaxBeatError);

// Also test prev/next beat calculation without tolerance
pGrid->findPrevNextBeats(position, &foundPrevBeat, &foundNextBeat, true);
EXPECT_NEAR(previousBeat, foundPrevBeat, kMaxBeatError);
EXPECT_NEAR(nextBeat, foundNextBeat, kMaxBeatError);
}
Expand Down
32 changes: 26 additions & 6 deletions src/test/beatmaptest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,11 @@ TEST_F(BeatMapTest, TestNthBeat) {
EXPECT_EQ(-1, pMap->findNthBeat(firstBeat, -2));

double prevBeat, nextBeat;
pMap->findPrevNextBeats(lastBeat, &prevBeat, &nextBeat);
pMap->findPrevNextBeats(lastBeat, &prevBeat, &nextBeat, false);
EXPECT_EQ(lastBeat, prevBeat);
EXPECT_EQ(-1, nextBeat);

pMap->findPrevNextBeats(firstBeat, &prevBeat, &nextBeat);
pMap->findPrevNextBeats(firstBeat, &prevBeat, &nextBeat, false);
EXPECT_EQ(firstBeat, prevBeat);
EXPECT_EQ(firstBeat + beatLengthSamples, nextBeat);
}
Expand Down Expand Up @@ -137,7 +137,12 @@ TEST_F(BeatMapTest, TestNthBeatWhenOnBeat) {

// Also test prev/next beat calculation.
double prevBeat, nextBeat;
pMap->findPrevNextBeats(position, &prevBeat, &nextBeat);
pMap->findPrevNextBeats(position, &prevBeat, &nextBeat, false);
EXPECT_EQ(position, prevBeat);
EXPECT_EQ(position + beatLengthSamples, nextBeat);

// Also test prev/next beat calculation without tolerance
pMap->findPrevNextBeats(position, &prevBeat, &nextBeat, true);
EXPECT_EQ(position, prevBeat);
EXPECT_EQ(position + beatLengthSamples, nextBeat);

Expand Down Expand Up @@ -175,10 +180,15 @@ TEST_F(BeatMapTest, TestNthBeatWhenOnBeat_BeforeEpsilon) {

// Also test prev/next beat calculation
double prevBeat, nextBeat;
pMap->findPrevNextBeats(position, &prevBeat, &nextBeat);
pMap->findPrevNextBeats(position, &prevBeat, &nextBeat, false);
EXPECT_EQ(kClosestBeat, prevBeat);
EXPECT_EQ(kClosestBeat + beatLengthSamples, nextBeat);

// Also test prev/next beat calculation without tolerance
pMap->findPrevNextBeats(position, &prevBeat, &nextBeat, true);
EXPECT_EQ(kClosestBeat - beatLengthSamples, prevBeat);
EXPECT_EQ(kClosestBeat, nextBeat);

// Both previous and next beat should return the closest beat.
EXPECT_EQ(kClosestBeat, pMap->findNextBeat(position));
EXPECT_EQ(kClosestBeat, pMap->findPrevBeat(position));
Expand Down Expand Up @@ -216,7 +226,12 @@ TEST_F(BeatMapTest, TestNthBeatWhenOnBeat_AfterEpsilon) {

// Also test prev/next beat calculation.
double prevBeat, nextBeat;
pMap->findPrevNextBeats(position, &prevBeat, &nextBeat);
pMap->findPrevNextBeats(position, &prevBeat, &nextBeat, false);
EXPECT_EQ(kClosestBeat, prevBeat);
EXPECT_EQ(kClosestBeat + beatLengthSamples, nextBeat);

// Also test prev/next beat calculation without tolerance
pMap->findPrevNextBeats(position, &prevBeat, &nextBeat, true);
EXPECT_EQ(kClosestBeat, prevBeat);
EXPECT_EQ(kClosestBeat + beatLengthSamples, nextBeat);

Expand Down Expand Up @@ -256,7 +271,12 @@ TEST_F(BeatMapTest, TestNthBeatWhenNotOnBeat) {

// Also test prev/next beat calculation
double foundPrevBeat, foundNextBeat;
pMap->findPrevNextBeats(position, &foundPrevBeat, &foundNextBeat);
pMap->findPrevNextBeats(position, &foundPrevBeat, &foundNextBeat, false);
EXPECT_EQ(previousBeat, foundPrevBeat);
EXPECT_EQ(nextBeat, foundNextBeat);

// Also test prev/next beat calculation without tolerance
pMap->findPrevNextBeats(position, &foundPrevBeat, &foundNextBeat, true);
EXPECT_EQ(previousBeat, foundPrevBeat);
EXPECT_EQ(nextBeat, foundNextBeat);
}
Expand Down
17 changes: 10 additions & 7 deletions src/track/beatgrid.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ double BeatGrid::findClosestBeat(double dSamples) const {
}
double prevBeat;
double nextBeat;
findPrevNextBeats(dSamples, &prevBeat, &nextBeat);
findPrevNextBeats(dSamples, &prevBeat, &nextBeat, false);
if (prevBeat == -1) {
// If both values are -1, we correctly return -1.
return nextBeat;
Expand Down Expand Up @@ -214,8 +214,10 @@ double BeatGrid::findNthBeat(double dSamples, int n) const {
}

bool BeatGrid::findPrevNextBeats(double dSamples,
double* dpPrevBeatSamples,
double* dpNextBeatSamples) const {

double* dpPrevBeatSamples,
double* dpNextBeatSamples,
bool NoTolerance) const {
double dFirstBeatSample;
double dBeatLength;
{
Expand All @@ -233,11 +235,13 @@ bool BeatGrid::findPrevNextBeats(double dSamples,
double prevBeat = floor(beatFraction);
double nextBeat = ceil(beatFraction);

// If the position is within 1/100th of the next or previous beat, treat it
// as if it is that beat.
const double kEpsilon = .01;

if (fabs(nextBeat - beatFraction) < kEpsilon) {
if ((NoTolerance && ((nextBeat - beatFraction) == 0.0)) ||
(!NoTolerance && (fabs(nextBeat - beatFraction) < kEpsilon))) {
// In tolerance mode: If the position is within 1/100th of the next or previous beat,
// treat it as if it is that beat.

beatFraction = nextBeat;
// If we are going to pretend we were actually on nextBeat then prevBeatFraction
// needs to be re-calculated. Since it is floor(beatFraction), that's
Expand All @@ -251,7 +255,6 @@ bool BeatGrid::findPrevNextBeats(double dSamples,
return true;
}


std::unique_ptr<BeatIterator> BeatGrid::findBeats(double startSample, double stopSample) const {
QMutexLocker locker(&m_mutex);
if (!isValid() || startSample > stopSample) {
Expand Down
6 changes: 4 additions & 2 deletions src/track/beatgrid.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,10 @@ class BeatGrid final : public Beats {
double findNextBeat(double dSamples) const override;
double findPrevBeat(double dSamples) const override;
bool findPrevNextBeats(double dSamples,
double* dpPrevBeatSamples,
double* dpNextBeatSamples) const override;

double* dpPrevBeatSamples,
double* dpNextBeatSamples,
bool NoTolerance) const override;
JoergAtGithub marked this conversation as resolved.
Show resolved Hide resolved
double findClosestBeat(double dSamples) const override;
double findNthBeat(double dSamples, int n) const override;
std::unique_ptr<BeatIterator> findBeats(double startSample, double stopSample) const override;
Expand Down
Loading