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.

Sign up for GitHub

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

Jump to bottom

Eject crash fix #1924

Merged
merged 27 commits into from
Dec 14, 2018
Merged

Eject crash fix #1924

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
35aca68
Replace connections to EngineControls by direct calls for better trac…
daschuer Nov 23, 2018
82afb2a
use range based loops
daschuer Nov 23, 2018
3f6865b
Axtracted processTrackLocked() for a save Track access.
daschuer Nov 23, 2018
7c87fc9
removed unused pOldTrack from trackLoaded function
daschuer Nov 23, 2018
8345b0d
rename m_pSampleRate to m_pMasterSampleRate to distinguish from track…
daschuer Nov 24, 2018
fb25a89
fix m_trackSampleRateOld handling
daschuer Nov 24, 2018
51bd6b5
fix m_trackSamplesOld handling
daschuer Nov 24, 2018
e675160
removed unused totalSamples from process call
daschuer Nov 25, 2018
b3fd646
store track sample rate with m_sampleOfTrack
daschuer Nov 25, 2018
3681a99
use m_pTrack thread save
daschuer Nov 25, 2018
de532d3
use m_pBeats in a threadsave way
daschuer Nov 25, 2018
44e80f1
LoopingControl: use m_pBeats and m_pTrack in a thread save way
daschuer Nov 25, 2018
eb79a3e
QuantizeControl: use m_pTrack and m_pBeats thread save
daschuer Nov 25, 2018
5f2105f
Removed unused m_pTrack from RateControl
daschuer Nov 25, 2018
3382d2a
VinylControlControl: use m_pTrack thread save
daschuer Nov 25, 2018
9ea73ae
make BpmControl::resetSyncAdjustment thread save
daschuer Nov 25, 2018
fb706ca
Fix typo
daschuer Nov 30, 2018
213ccb3
fix a race condition in bpmcontrol and improve documentation
daschuer Nov 30, 2018
c6a001a
RateControl: Make config options atomic
daschuer Nov 30, 2018
3a2fdc4
remove unused variable
daschuer Dec 1, 2018
c8c5883
make TabFilter thread save
daschuer Dec 2, 2018
0652d02
Initalize atomic values and check trackSamples for 0
daschuer Dec 12, 2018
df5653c
Revert rename of m_pSampleRate
daschuer Dec 12, 2018
3e87069
use qAsConst()
daschuer Dec 12, 2018
562387c
reset play and cue indicator unconditionally after eject, fixing lp18…
daschuer Dec 12, 2018
94756f3
replace single getter with one for the whol atomic struct.
daschuer Dec 13, 2018
d6b9f8b
unlock mutex before emit a signal
daschuer Dec 13, 2018
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
147 changes: 79 additions & 68 deletions src/engine/bpmcontrol.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ const SINT kSamplesPerFrame = 2;
}

BpmControl::BpmControl(QString group,
UserSettingsPointer pConfig) :
EngineControl(group, pConfig),
m_dSyncTargetBeatDistance(0.0),
m_dSyncInstantaneousBpm(0.0),
m_dLastSyncAdjustment(1.0),
m_resetSyncAdjustment(false),
m_dUserOffset(0.0),
m_tapFilter(this, kFilterLength, kMaxInterval),
m_sGroup(group) {
UserSettingsPointer pConfig)
: EngineControl(group, pConfig),
m_tapFilter(this, kFilterLength, kMaxInterval),
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't hurt to explicitly initialize the atomic values. Here they are used on a higher level of abstraction. We should not rely on the implicit default values that are pure numbers without any meaning in the context of BPM control.

m_dSyncInstantaneousBpm(0.0),
m_dLastSyncAdjustment(1.0),
m_sGroup(group) {
m_dSyncTargetBeatDistance.setValue(0.0);
m_dUserOffset.setValue(0.0);

m_pPlayButton = new ControlProxy(group, "play", this);
m_pReverseButton = new ControlProxy(group, "reverse", this);
m_pRateSlider = new ControlProxy(group, "rate", this);
Expand Down Expand Up @@ -125,7 +125,7 @@ BpmControl::BpmControl(QString group,

// Measures distance from last beat in percentage: 0.5 = half-beat away.
m_pThisBeatDistance = new ControlProxy(group, "beat_distance", this);
m_pSyncMode = ControlObject::getControl(ConfigKey(group, "sync_mode"));
m_pSyncMode = new ControlProxy(group, "sync_mode", this);
}

BpmControl::~BpmControl() {
Expand Down Expand Up @@ -153,10 +153,11 @@ void BpmControl::slotFileBpmChanged(double bpm) {
// Adjust the file-bpm with the current setting of the rate to get the
// engine BPM. We only do this for SYNC_NONE decks because EngineSync will
// set our BPM if the file BPM changes. See SyncControl::fileBpmChanged().
if (m_pBeats) {
BeatsPointer pBeats = m_pBeats;
if (pBeats) {
const double beats_bpm =
m_pBeats->getBpmAroundPosition(getCurrentSample(),
kLocalBpmSpan);
pBeats->getBpmAroundPosition(
getSampleOfTrack().current, kLocalBpmSpan);
if (beats_bpm != -1) {
m_pLocalBpm->set(beats_bpm);
} else {
Expand All @@ -172,34 +173,37 @@ void BpmControl::slotFileBpmChanged(double bpm) {
}

void BpmControl::slotAdjustBeatsFaster(double v) {
if (v > 0 && m_pBeats && (m_pBeats->getCapabilities() & Beats::BEATSCAP_SETBPM)) {
double new_bpm = math_min(200.0, m_pBeats->getBpm() + .01);
m_pBeats->setBpm(new_bpm);
BeatsPointer pBeats = m_pBeats;
if (v > 0 && pBeats && (pBeats->getCapabilities() & Beats::BEATSCAP_SETBPM)) {
double new_bpm = math_min(200.0, pBeats->getBpm() + .01);
pBeats->setBpm(new_bpm);
}
}

void BpmControl::slotAdjustBeatsSlower(double v) {
if (v > 0 && m_pBeats && (m_pBeats->getCapabilities() & Beats::BEATSCAP_SETBPM)) {
double new_bpm = math_max(10.0, m_pBeats->getBpm() - .01);
m_pBeats->setBpm(new_bpm);
BeatsPointer pBeats = m_pBeats;
if (v > 0 && pBeats && (pBeats->getCapabilities() & Beats::BEATSCAP_SETBPM)) {
double new_bpm = math_max(10.0, pBeats->getBpm() - .01);
pBeats->setBpm(new_bpm);
}
}

void BpmControl::slotTranslateBeatsEarlier(double v) {
if (v > 0 && m_pTrack && m_pBeats &&
(m_pBeats->getCapabilities() & Beats::BEATSCAP_TRANSLATE)) {
// TODO(rryan): Track::getSampleRate is possibly inaccurate!
const int translate_dist = m_pTrack->getSampleRate() * -.01;
m_pBeats->translate(translate_dist);
BeatsPointer pBeats = m_pBeats;
if (v > 0 && pBeats &&
(pBeats->getCapabilities() & Beats::BEATSCAP_TRANSLATE)) {
const int translate_dist = getSampleOfTrack().rate * -.01;
pBeats->translate(translate_dist);
}
}

void BpmControl::slotTranslateBeatsLater(double v) {
if (v > 0 && m_pTrack && m_pBeats &&
(m_pBeats->getCapabilities() & Beats::BEATSCAP_TRANSLATE)) {
BeatsPointer pBeats = m_pBeats;
if (v > 0 && pBeats &&
(pBeats->getCapabilities() & Beats::BEATSCAP_TRANSLATE)) {
// TODO(rryan): Track::getSampleRate is possibly inaccurate!
const int translate_dist = m_pTrack->getSampleRate() * .01;
m_pBeats->translate(translate_dist);
const int translate_dist = getSampleOfTrack().rate * .01;
pBeats->translate(translate_dist);
}
}

Expand All @@ -219,7 +223,7 @@ void BpmControl::slotTapFilter(double averageLength, int numSamples) {
if (numSamples < 4)
return;

auto pBeats = m_pBeats;
BeatsPointer pBeats = m_pBeats;
if (!pBeats)
return;

Expand Down Expand Up @@ -390,14 +394,14 @@ double BpmControl::calcSyncedRate(double userTweak) {
// If we are not quantized, or there are no beats, or we're master,
// or we're in reverse, just return the rate as-is.
if (!m_pQuantize->get() || getSyncMode() == SYNC_MASTER ||
m_pBeats == NULL || m_pReverseButton->get()) {
!m_pBeats || m_pReverseButton->get()) {
m_resetSyncAdjustment = true;
return rate + userTweak;
}

// Now we need to get our beat distance so we can figure out how
// out of phase we are.
double dThisPosition = getCurrentSample();
double dThisPosition = getSampleOfTrack().current;
double dBeatLength;
double my_percentage;
if (!BpmControl::getBeatContextNoLookup(dThisPosition,
Expand All @@ -424,8 +428,8 @@ double BpmControl::calcSyncedRate(double userTweak) {
}

double BpmControl::calcSyncAdjustment(double my_percentage, bool userTweakingSync) {
if (m_resetSyncAdjustment) {
m_resetSyncAdjustment = false;
int resetSyncAdjustment = m_resetSyncAdjustment.fetchAndStoreRelaxed(0);
if (resetSyncAdjustment) {
m_dLastSyncAdjustment = 1.0;
}

Expand All @@ -442,7 +446,7 @@ double BpmControl::calcSyncAdjustment(double my_percentage, bool userTweakingSyn
// than modular 1.0 beat fractions. This will allow sync to work across loop
// boundaries too.

double master_percentage = m_dSyncTargetBeatDistance;
double master_percentage = m_dSyncTargetBeatDistance.getValue();
double shortest_distance = shortestPercentageChange(
master_percentage, my_percentage);

Expand All @@ -457,9 +461,9 @@ double BpmControl::calcSyncAdjustment(double my_percentage, bool userTweakingSyn
if (userTweakingSync) {
// Don't do anything else, leave it
adjustment = 1.0;
m_dUserOffset = shortest_distance;
m_dUserOffset.setValue(shortest_distance);
} else {
double error = shortest_distance - m_dUserOffset;
double error = shortest_distance - m_dUserOffset.getValue();
// Threshold above which we do sync adjustment.
const double kErrorThreshold = 0.01;
// Threshold above which sync is really, really bad, so much so that we
Expand Down Expand Up @@ -505,7 +509,7 @@ double BpmControl::getBeatDistance(double dThisPosition) const {
double dNextBeat = m_pNextBeat->get();

if (dPrevBeat == -1 || dNextBeat == -1) {
return 0.0 - m_dUserOffset;
return 0.0 - m_dUserOffset.getValue();
}

double dBeatLength = dNextBeat - dPrevBeat;
Expand All @@ -516,7 +520,7 @@ double BpmControl::getBeatDistance(double dThisPosition) const {
if (dBeatPercentage < 0) ++dBeatPercentage;
if (dBeatPercentage > 1) --dBeatPercentage;

return dBeatPercentage - m_dUserOffset;
return dBeatPercentage - m_dUserOffset.getValue();
}

// static
Expand Down Expand Up @@ -576,9 +580,11 @@ bool BpmControl::getBeatContextNoLookup(
return true;
}

double BpmControl::getNearestPositionInPhase(double dThisPosition, bool respectLoops, bool playing) {
double BpmControl::getNearestPositionInPhase(
double dThisPosition, bool respectLoops, bool playing) {
// Without a beatgrid, we don't know the phase offset.
if (!m_pBeats) {
BeatsPointer pBeats = m_pBeats;
if (!pBeats) {
return dThisPosition;
}
// Master buffer is always in sync!
Expand All @@ -594,7 +600,7 @@ double BpmControl::getNearestPositionInPhase(double dThisPosition, bool respectL
// There's a chance the COs might be out of date, so do a lookup.
// TODO: figure out a way so that quantized control can take care of
// this so this call isn't necessary.
if (!getBeatContext(m_pBeats, dThisPosition,
if (!getBeatContext(pBeats, dThisPosition,
&dThisPrevBeat, &dThisNextBeat,
&dThisBeatLength, NULL)) {
return dThisPosition;
Expand All @@ -610,7 +616,7 @@ double BpmControl::getNearestPositionInPhase(double dThisPosition, bool respectL
double dOtherBeatFraction;
if (getSyncMode() == SYNC_FOLLOWER) {
// If we're a follower, it's easy to get the other beat fraction
dOtherBeatFraction = m_dSyncTargetBeatDistance;
dOtherBeatFraction = m_dSyncTargetBeatDistance.getValue();
} else {
// If not, we have to figure it out
EngineBuffer* pOtherEngineBuffer = pickSyncTarget();
Expand Down Expand Up @@ -665,13 +671,13 @@ double BpmControl::getNearestPositionInPhase(double dThisPosition, bool respectL
// infinite beatgrids because the assumption that findNthBeat(-2) always
// works will be wrong then.

double dNewPlaypos = (dOtherBeatFraction + m_dUserOffset) * dThisBeatLength;
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
dThisPrevBeat = m_pBeats->findNthBeat(dThisPosition, -2);
dThisPrevBeat = pBeats->findNthBeat(dThisPosition, -2);
dNewPlaypos += dThisPrevBeat;
}

Expand Down Expand Up @@ -745,8 +751,8 @@ void BpmControl::slotUpdateRateSlider() {
m_pRateSlider->set(dRateSlider);
}

void BpmControl::trackLoaded(TrackPointer pNewTrack, TrackPointer pOldTrack) {
Q_UNUSED(pOldTrack);
// called from an engine worker thread
void BpmControl::trackLoaded(TrackPointer pNewTrack) {
if (m_pTrack) {
disconnect(m_pTrack.get(), SIGNAL(beatsUpdated()),
this, SLOT(slotUpdatedTrackBeats()));
Expand All @@ -767,41 +773,45 @@ void BpmControl::trackLoaded(TrackPointer pNewTrack, TrackPointer pOldTrack) {
}

void BpmControl::slotUpdatedTrackBeats() {
if (m_pTrack) {
TrackPointer pTrack = m_pTrack;
if (pTrack) {
resetSyncAdjustment();
m_pBeats = m_pTrack->getBeats();
m_pBeats = pTrack->getBeats();
}
}

void BpmControl::slotBeatsTranslate(double v) {
if (v > 0 && m_pBeats && (m_pBeats->getCapabilities() & Beats::BEATSCAP_TRANSLATE)) {
double currentSample = getCurrentSample();
double closestBeat = m_pBeats->findClosestBeat(currentSample);
BeatsPointer pBeats = m_pBeats;
if (v > 0 && pBeats && (pBeats->getCapabilities() & Beats::BEATSCAP_TRANSLATE)) {
double currentSample = getSampleOfTrack().current;
double closestBeat = pBeats->findClosestBeat(currentSample);
int delta = currentSample - closestBeat;
if (delta % 2 != 0) {
delta--;
}
m_pBeats->translate(delta);
pBeats->translate(delta);
}
}

void BpmControl::slotBeatsTranslateMatchAlignment(double v) {
if (v > 0 && m_pBeats && (m_pBeats->getCapabilities() & Beats::BEATSCAP_TRANSLATE)) {
BeatsPointer pBeats = m_pBeats;
if (v > 0 && pBeats && (pBeats->getCapabilities() & Beats::BEATSCAP_TRANSLATE)) {
// Must reset the user offset *before* calling getPhaseOffset(),
// otherwise it will always return 0 if master sync is active.
m_dUserOffset = 0.0;
m_dUserOffset.setValue(0.0);

double offset = getPhaseOffset(getCurrentSample());
m_pBeats->translate(-offset);
double offset = getPhaseOffset(getSampleOfTrack().current);
pBeats->translate(-offset);
}
}

double BpmControl::updateLocalBpm() {
double prev_local_bpm = m_pLocalBpm->get();
double local_bpm = 0;
if (m_pBeats) {
local_bpm = m_pBeats->getBpmAroundPosition(getCurrentSample(),
kLocalBpmSpan);
BeatsPointer pBeats = m_pBeats;
if (pBeats) {
local_bpm = pBeats->getBpmAroundPosition(
getSampleOfTrack().current, kLocalBpmSpan);
if (local_bpm == -1) {
local_bpm = m_pFileBpm->get();
}
Expand All @@ -816,16 +826,16 @@ double BpmControl::updateLocalBpm() {
}

double BpmControl::updateBeatDistance() {
double beat_distance = getBeatDistance(getCurrentSample());
double beat_distance = getBeatDistance(getSampleOfTrack().current);
m_pThisBeatDistance->set(beat_distance);
if (getSyncMode() == SYNC_NONE) {
m_dUserOffset = 0.0;
m_dUserOffset.setValue(0.0);
}
return beat_distance;
}

void BpmControl::setTargetBeatDistance(double beatDistance) {
m_dSyncTargetBeatDistance = beatDistance;
m_dSyncTargetBeatDistance.setValue(beatDistance);
}

void BpmControl::setInstantaneousBpm(double instantaneousBpm) {
Expand All @@ -834,31 +844,32 @@ void BpmControl::setInstantaneousBpm(double instantaneousBpm) {

void BpmControl::resetSyncAdjustment() {
// Immediately edit the beat distance to reflect the new reality.
double new_distance = m_pThisBeatDistance->get() + m_dUserOffset;
double new_distance = m_pThisBeatDistance->get() + m_dUserOffset.getValue();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why getValue/setValue instead of fetchAndStore?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is ControlValueAtomic, which has no fetchAndStore().

m_pThisBeatDistance->set(new_distance);
m_dUserOffset = 0.0;
m_dUserOffset.setValue(0.0);
m_resetSyncAdjustment = true;
}

void BpmControl::collectFeatures(GroupFeatureState* pGroupFeatures) const {
// Without a beatgrid we don't know any beat details.
if (!m_pBeats) {
SampleOfTrack sot = getSampleOfTrack();
if (!sot.rate || !m_pBeats) {
return;
}

// Get the current position of this deck.
double dThisPosition = getCurrentSample();
double dThisPrevBeat = m_pPrevBeat->get();
double dThisNextBeat = m_pNextBeat->get();
double dThisBeatLength;
double dThisBeatFraction;
if (getBeatContextNoLookup(dThisPosition,
if (getBeatContextNoLookup(sot.current,
dThisPrevBeat, dThisNextBeat,
&dThisBeatLength, &dThisBeatFraction)) {
pGroupFeatures->has_beat_length_sec = true;

// Note: dThisBeatLength is fractional frames count * 2 (stereo samples)
pGroupFeatures->beat_length_sec = dThisBeatLength / kSamplesPerFrame
/ m_pTrack->getSampleRate() / calcRateRatio();
Copy link
Contributor

Choose a reason for hiding this comment

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

Might getTrackSampleRate() return 0.0 due to race conditions if the track has just been unloaded? We better should add a check for this.

Unrelated (or maybe related): Is calcRateRatio() guaranteed to always return a value that is strictly != 0 in any situation?

/ sot.rate / calcRateRatio();

pGroupFeatures->has_beat_fraction = true;
pGroupFeatures->beat_fraction = dThisBeatFraction;
Expand Down
Loading