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

isAudible() #3772

Merged
merged 5 commits into from
Apr 27, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 2 additions & 1 deletion src/engine/channels/enginechannel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ EngineChannel::EngineChannel(const ChannelHandleAndGroup& handle_group,
m_pSampleRate(new ControlProxy("[Master]", "samplerate")),
m_sampleBuffer(nullptr),
m_bIsPrimaryDeck(isPrimaryDeck),
m_bIsTalkoverChannel(isTalkoverChannel) {
m_bIsTalkoverChannel(isTalkoverChannel),
m_channelIndex(-1) {
m_pPFL = new ControlPushButton(ConfigKey(getGroup(), "pfl"));
m_pPFL->setButtonMode(ControlPushButton::TOGGLE);
m_pMaster = new ControlPushButton(ConfigKey(getGroup(), "master"));
Expand Down
7 changes: 7 additions & 0 deletions src/engine/channels/enginechannel.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ class EngineChannel : public EngineObject {
inline bool isPrimaryDeck() {
return m_bIsPrimaryDeck;
};
int getChannelIndex() {
return m_channelIndex;
}
void setChannelIndex(int channelIndex) {
m_channelIndex = channelIndex;
}

virtual void process(CSAMPLE* pOut, const int iBufferSize) = 0;
virtual void collectFeatures(GroupFeatureState* pGroupFeatures) const = 0;
Expand Down Expand Up @@ -85,4 +91,5 @@ class EngineChannel : public EngineObject {
ControlPushButton* m_pOrientationCenter;
ControlPushButton* m_pTalkover;
bool m_bIsTalkoverChannel;
int m_channelIndex;
};
1 change: 1 addition & 0 deletions src/engine/enginebuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1226,6 +1226,7 @@ void EngineBuffer::postProcess(const int iBufferSize) {
double local_bpm = m_pBpmControl->updateLocalBpm();
double beat_distance = m_pBpmControl->updateBeatDistance();
m_pSyncControl->setLocalBpm(local_bpm);
m_pSyncControl->updateAudible();
SyncMode mode = m_pSyncControl->getSyncMode();
if (isMaster(mode)) {
m_pEngineSync->notifyBeatDistanceChanged(m_pSyncControl, beat_distance);
Expand Down
6 changes: 6 additions & 0 deletions src/engine/enginebuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ class EngineBuffer : public EngineObject {
// has completed.
void loadTrack(TrackPointer pTrack, bool play);

void setChannelIndex(int channelIndex) {
m_channelIndex = channelIndex;
}

public slots:
void slotControlPlayRequest(double);
void slotControlPlayFromStart(double);
Expand Down Expand Up @@ -226,6 +230,8 @@ class EngineBuffer : public EngineObject {

// Holds the name of the control group
const QString m_group;
int m_channelIndex;

UserSettingsPointer m_pConfig;

friend class CueControlTest;
Expand Down
8 changes: 8 additions & 0 deletions src/engine/enginemaster.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,7 @@ void EngineMaster::processHeadphones(const CSAMPLE_GAIN masterMixGainInHeadphone

void EngineMaster::addChannel(EngineChannel* pChannel) {
ChannelInfo* pChannelInfo = new ChannelInfo(m_channels.size());
pChannel->setChannelIndex(pChannelInfo->m_index);
pChannelInfo->m_pChannel = pChannel;
const QString& group = pChannel->getGroup();
pChannelInfo->m_handle = m_pChannelHandleFactory->getOrCreateHandle(group);
Expand Down Expand Up @@ -836,6 +837,13 @@ EngineChannel* EngineMaster::getChannel(const QString& group) {
return nullptr;
}

CSAMPLE_GAIN EngineMaster::getMasterGain(int channelIndex) {
if (channelIndex < m_channelMasterGainCache.size() && channelIndex >= 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Checking the lower bound first is more common

return m_channelMasterGainCache[channelIndex].m_gain;
}
return CSAMPLE_GAIN_ZERO;
}

const CSAMPLE* EngineMaster::getDeckBuffer(unsigned int i) const {
return getChannelBuffer(PlayerManager::groupForDeck(i));
}
Expand Down
4 changes: 3 additions & 1 deletion src/engine/enginemaster.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ class EngineMaster : public QObject, public AudioSource {
return m_pEngineSideChain;
}

CSAMPLE_GAIN getMasterGain(int channelIndex);
Copy link
Contributor

Choose a reason for hiding this comment

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

const


struct ChannelInfo {
ChannelInfo(int index)
: m_pChannel(NULL),
Expand All @@ -123,7 +125,7 @@ class EngineMaster : public QObject, public AudioSource {
};

struct GainCache {
CSAMPLE m_gain;
CSAMPLE_GAIN m_gain;
bool m_fadeout;
};

Expand Down
13 changes: 7 additions & 6 deletions src/engine/sync/enginesync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ Syncable* EngineSync::pickMaster(Syncable* enabling_syncable) {
}
}

if (pSyncable->isPlaying()) {
if (pSyncable->isPlaying() && pSyncable->isAudible()) {
if (playing_deck_count == 0) {
first_playing_deck = pSyncable;
}
Expand Down Expand Up @@ -135,7 +135,7 @@ Syncable* EngineSync::findBpmMatchTarget(Syncable* requester) {

// If the other deck is playing we stop looking immediately. Otherwise continue looking
// for a playing deck with bpm > 0.0.
if (pOtherSyncable->isPlaying()) {
if (pOtherSyncable->isPlaying() && pOtherSyncable->isAudible()) {
return pOtherSyncable;
}

Expand Down Expand Up @@ -237,18 +237,19 @@ void EngineSync::requestEnableSync(Syncable* pSyncable, bool bEnabled) {
}
}

void EngineSync::notifyPlaying(Syncable* pSyncable, bool playing) {
Q_UNUSED(playing);
void EngineSync::notifyPlaying(Syncable* pSyncable, bool playingAudible) {
qDebug() << "EngineSync::notifyPlaying" << pSyncable->getGroup() << playingAudible;

if (kLogger.traceEnabled()) {
kLogger.trace() << "EngineSync::notifyPlaying" << pSyncable->getGroup() << playing;
kLogger.trace() << "EngineSync::notifyPlaying" << pSyncable->getGroup() << playingAudible;
}
// For now we don't care if the deck is now playing or stopping.
if (!pSyncable->isSynchronized()) {
return;
}

// similar to enablesync -- we pick a new master and maybe reinit.
Syncable* newMaster = pickMaster(playing ? pSyncable : nullptr);
Syncable* newMaster = pickMaster(playingAudible ? pSyncable : nullptr);

if (newMaster != nullptr && newMaster != m_pMasterSyncable) {
activateMaster(newMaster, false);
Expand Down
3 changes: 3 additions & 0 deletions src/engine/sync/internalclock.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ class InternalClock : public QObject, public Clock, public Syncable {
bool isPlaying() const override {
return false;
}
bool isAudible() const override {
return false;
}

double getBeatDistance() const override;
void setMasterBeatDistance(double beatDistance) override;
Expand Down
1 change: 1 addition & 0 deletions src/engine/sync/syncable.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class Syncable {

// Only relevant for player Syncables.
virtual bool isPlaying() const = 0;
virtual bool isAudible() const = 0;

// Gets the current speed of the syncable in bpm (bpm * rate slider), doesn't
// include scratch or FF/REW values.
Expand Down
20 changes: 19 additions & 1 deletion src/engine/sync/synccontrol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "engine/controls/bpmcontrol.h"
#include "engine/controls/ratecontrol.h"
#include "engine/enginebuffer.h"
#include "engine/enginemaster.h"
#include "moc_synccontrol.cpp"
#include "track/track.h"
#include "util/assert.h"
Expand Down Expand Up @@ -171,6 +172,11 @@ bool SyncControl::isPlaying() const {
return m_pPlayButton->toBool();
}

bool SyncControl::isAudible() const {
qDebug() << "isAudible" << m_audible << getGroup();
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this debug log

return m_audible;
}

double SyncControl::adjustSyncBeatDistance(double beatDistance) const {
// Similar to adjusting the target beat distance, when we report our beat
// distance we need to adjust it by the master bpm adjustment factor. If
Expand Down Expand Up @@ -353,7 +359,7 @@ void SyncControl::trackBeatsUpdated(mixxx::BeatsPointer pBeats) {
}

void SyncControl::slotControlPlay(double play) {
m_pEngineSync->notifyPlaying(this, play > 0.0);
m_pEngineSync->notifyPlaying(this, play > 0.0 && m_audible);
Copy link
Contributor

Choose a reason for hiding this comment

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

This changes the semantics SyncControll::isPlaying() and is inconsistent.

}

void SyncControl::slotVinylControlChanged(double enabled) {
Expand Down Expand Up @@ -438,6 +444,18 @@ void SyncControl::setLocalBpm(double local_bpm) {
}
}

void SyncControl::updateAudible() {
int channelIndex = m_pChannel->getChannelIndex();
if (channelIndex >= 0) {
CSAMPLE_GAIN gain = getEngineMaster()->getMasterGain(channelIndex);
bool newAudible = gain > CSAMPLE_GAIN_ZERO ? true : false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Overly complicated. Remove the ternary operator.

if (m_audible != newAudible) {
m_audible = newAudible;
m_pEngineSync->notifyPlaying(this, m_pPlayButton->toBool() && m_audible);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here semantic change that causes an inconsistency. Audible, Playing or a mixture of both?

}
}
}

void SyncControl::slotRateChanged() {
double bpm = m_pLocalBpm ? m_pLocalBpm->get() * m_pRateRatio->get() : 0.0;
if (kLogger.traceEnabled()) {
Expand Down
3 changes: 3 additions & 0 deletions src/engine/sync/synccontrol.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ class SyncControl : public EngineControl, public Syncable {
void notifyOnlyPlayingSyncable() override;
void requestSync() override;
bool isPlaying() const override;
bool isAudible() const override;

double adjustSyncBeatDistance(double beatDistance) const;
double getBeatDistance() const override;
void updateTargetBeatDistance();
double getBaseBpm() const override;
void setLocalBpm(double local_bpm);
void updateAudible();

// Must never result in a call to
// SyncableListener::notifyBeatDistanceChanged or signal loops could occur.
Expand Down Expand Up @@ -109,6 +111,7 @@ class SyncControl : public EngineControl, public Syncable {
// multiplier changes and we need to recalculate the target distance.
double m_unmultipliedTargetBeatDistance;
ControlValueAtomic<double> m_prevLocalBpm;
QAtomicInt m_audible;

QScopedPointer<ControlPushButton> m_pSyncMode;
QScopedPointer<ControlPushButton> m_pSyncMasterEnabled;
Expand Down
50 changes: 50 additions & 0 deletions src/test/enginesynctest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1087,6 +1087,7 @@ TEST_F(EngineSyncTest, SyncToNonSyncDeck) {

ControlObject::set(ConfigKey(m_sGroup1, "play"), 1.0);
ControlObject::set(ConfigKey(m_sGroup2, "play"), 1.0);
ProcessBuffer();

pButtonSyncEnabled2->set(1.0);
pButtonSyncEnabled2->set(0.0);
Expand Down Expand Up @@ -1168,6 +1169,7 @@ TEST_F(EngineSyncTest, MomentarySyncDependsOnPlayingStates) {
ControlObject::set(ConfigKey(m_sGroup2, "rate"), getRateSliderValue(1.0));
ControlObject::set(ConfigKey(m_sGroup2, "play"), 1.0);
ProcessBuffer();
ProcessBuffer();

// Set channel 1 to be enabled momentarily.
pButtonSyncEnabled1->set(1.0);
Expand Down Expand Up @@ -1434,6 +1436,8 @@ TEST_F(EngineSyncTest, ZeroLatencyRateChangeQuant) {
ControlObject::getControl(ConfigKey(m_sGroup1, "beat_distance"))
->get());

ProcessBuffer();

for (int i = 0; i < 50; ++i) {
ProcessBuffer();
// Keep messing with the rate
Expand Down Expand Up @@ -1484,6 +1488,8 @@ TEST_F(EngineSyncTest, ZeroLatencyRateDiffQuant) {
ControlObject::getControl(ConfigKey(m_sGroup1, "beat_distance"))
->get());

ProcessBuffer();

for (int i = 0; i < 50; ++i) {
ProcessBuffer();
// Keep messing with the rate
Expand Down Expand Up @@ -1657,6 +1663,8 @@ TEST_F(EngineSyncTest, HalfDoubleThenPlay) {
ControlObject::getControl(ConfigKey(m_sGroup1, "rate"))
->set(getRateSliderValue(1.0));

ProcessBuffer();

auto pButtonSyncEnabled1 =
std::make_unique<ControlProxy>(m_sGroup1, "sync_enabled");
pButtonSyncEnabled1->slotSet(1.0);
Expand Down Expand Up @@ -1689,9 +1697,13 @@ TEST_F(EngineSyncTest, HalfDoubleThenPlay) {
ControlObject::getControl(ConfigKey(m_sGroup2, "local_bpm"))
->get());

ProcessBuffer();

ControlObject::getControl(ConfigKey(m_sGroup2, "play"))->set(1.0);
ControlObject::getControl(ConfigKey(m_sGroup1, "play"))->set(1.0);

ProcessBuffer();

EXPECT_DOUBLE_EQ(175.0,
ControlObject::getControl(ConfigKey(m_sInternalClockGroup, "bpm"))
->get());
Expand Down Expand Up @@ -2186,6 +2198,8 @@ TEST_F(EngineSyncTest, QuantizeImpliesSyncPhase) {
mixxx::BeatsPointer pBeats2 = BeatFactory::makeBeatGrid(m_pTrack2->getSampleRate(), 100, 0.0);
m_pTrack2->trySetBeats(pBeats2);

ProcessBuffer();

ControlObject::set(ConfigKey(m_sGroup1, "play"), 1.0);
ControlObject::set(ConfigKey(m_sGroup2, "play"), 1.0);
ProcessBuffer();
Expand Down Expand Up @@ -2535,3 +2549,39 @@ TEST_F(EngineSyncTest, BpmAdjustFactor) {
ASSERT_TRUE(isFollower(m_sGroup2));
ASSERT_TRUE(isSoftMaster(m_sInternalClockGroup));
}

TEST_F(EngineSyncTest, ImpliciteMasterToInternalClock) {
m_pMixerDeck1->loadFakeTrack(false, 40.0);
m_pMixerDeck2->loadFakeTrack(false, 150.0);
ProcessBuffer();

EXPECT_DOUBLE_EQ(40.0, ControlObject::get(ConfigKey(m_sGroup1, "bpm")));
EXPECT_DOUBLE_EQ(150.0, ControlObject::get(ConfigKey(m_sGroup2, "bpm")));

// During cue-ing volume is 0.0
ControlObject::set(ConfigKey(m_sGroup1, "volume"), 0.0);
ControlObject::set(ConfigKey(m_sGroup1, "play"), 1.0);
ControlObject::set(ConfigKey(m_sGroup2, "play"), 1.0);
ProcessBuffer();

ControlObject::set(ConfigKey(m_sGroup1, "sync_enabled"), 1.0);
ControlObject::set(ConfigKey(m_sGroup2, "sync_enabled"), 1.0);
ProcessBuffer();

// group 2 should be synced to the first playing deck and becomes master
EXPECT_DOUBLE_EQ(75.0, ControlObject::get(ConfigKey(m_sGroup1, "bpm")));
EXPECT_DOUBLE_EQ(150.0, ControlObject::get(ConfigKey(m_sGroup2, "bpm")));
ASSERT_FALSE(isSoftMaster(m_sGroup1));
ASSERT_TRUE(isSoftMaster(m_sGroup2));
ASSERT_FALSE(isSoftMaster(m_sInternalClockGroup));
ProcessBuffer();

// Drop Track
ControlObject::set(ConfigKey(m_sGroup1, "volume"), 1.0);
ProcessBuffer();
ProcessBuffer();

ASSERT_FALSE(isSoftMaster(m_sGroup1));
ASSERT_FALSE(isSoftMaster(m_sGroup2));
ASSERT_TRUE(isSoftMaster(m_sInternalClockGroup));
}