Skip to content

Commit

Permalink
Post-merge fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
xeruf committed Nov 16, 2021
1 parent 91d0937 commit 796b43a
Show file tree
Hide file tree
Showing 16 changed files with 125 additions and 149 deletions.
17 changes: 8 additions & 9 deletions src/engine/controls/enginecontrol.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,16 @@ class EngineControl : public QObject {
UserSettingsPointer pConfig);
~EngineControl() override;

// Called by EngineBuffer::process every latency period. See the above
// comments for information about guarantees that hold during this call. An
// EngineControl can perform any upkeep operations that are necessary during
// this call.
/// Called by EngineBuffer::process every latency period.
/// See the above comments for information about guarantees that hold during this call.
/// An EngineControl can perform any upkeep operations necessary here.
virtual void process(const double dRate,
mixxx::audio::FramePos currentPosition,
const int iBufferSize);

// hintReader allows the EngineControl to provide hints to the reader to
// indicate that the given portion of a song is a potential imminent seek
// target.
/// hintReader allows the EngineControl to provide hints to the reader
/// to indicate that the given portion of a song
/// is a potential imminent seek target.
virtual void hintReader(HintVector* pHintList);

virtual void setEngineMaster(EngineMaster* pEngineMaster);
Expand All @@ -66,7 +65,7 @@ class EngineControl : public QObject {
mixxx::audio::FramePos endPosition,
bool enabled);

// Called to collect player features for effects processing.
/// Collect player features for effects processing.
virtual void collectFeatureState(GroupFeatureState* pGroupFeatures) const {
Q_UNUSED(pGroupFeatures);
}
Expand Down Expand Up @@ -99,7 +98,7 @@ class EngineControl : public QObject {
/// Seek to an exact sample, no quantizing
/// virtual only for testing!
virtual void seekExact(mixxx::audio::FramePos position);
// Returns an EngineBuffer to target for syncing. Returns nullptr if none found
/// Return an EngineBuffer to target for syncing. Returns nullptr if none found.
EngineBuffer* pickSyncTarget();

UserSettingsPointer getConfig();
Expand Down
49 changes: 24 additions & 25 deletions src/engine/controls/macrocontrol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,28 +76,29 @@ MacroControl::MacroControl(const QString& group, UserSettingsPointer pConfig, in
&MacroControl::slotClear);
}

void MacroControl::process(const double dRate, const double dCurrentSample, const int iBufferSize) {
void MacroControl::process(const double dRate,
mixxx::audio::FramePos currentPosition,
const int iBufferSize) {
Q_UNUSED(dRate);
if (m_queuedJumpTarget >= 0) {
if (m_queuedJumpTarget.value() >= 0) {
// if a cue press doesn't change the position, notifySeek isn't called, thus m_queuedJumpTarget isn't reset
if (getStatus() == Status::Armed) {
// start the recording on a cue press even when there is no jump
notifySeek(dCurrentSample);
notifySeek(currentPosition);
}
m_queuedJumpTarget = -1;
m_queuedJumpTarget = mixxx::audio::FramePos(-1);
}
if (getStatus() != Status::Playing) {
return;
}
double framePos = dCurrentSample / mixxx::kEngineChannelCount;
const MacroAction& nextAction = m_pMacro->getActions().at(m_iNextAction);
double nextActionPos = nextAction.getSourcePosition();
auto nextActionPos = nextAction.getSourcePosition();
int bufFrames = iBufferSize / 2;
// The process method is called roughly every iBufferSize/2 samples, the
// tolerance range is double that to be safe. It is ahead of the position
// because the seek is executed in the next EngineBuffer process cycle.
if (framePos > nextActionPos - bufFrames && framePos < nextActionPos + bufFrames) {
seekExact(nextAction.getTargetPositionSample());
if (currentPosition > nextActionPos - bufFrames && currentPosition < nextActionPos + bufFrames) {
seekExact(nextAction.getTargetPosition());
m_iNextAction++;
if (m_iNextAction == m_pMacro->size()) {
if (m_pMacro->isLooped()) {
Expand Down Expand Up @@ -134,26 +135,25 @@ void MacroControl::trackLoaded(TrackPointer pNewTrack) {
}
}

void MacroControl::notifySeek(double dNewPlaypos) {
if (m_queuedJumpTarget < 0) {
void MacroControl::notifySeek(mixxx::audio::FramePos position) {
if (m_queuedJumpTarget.value() < 0) {
return;
}
double originalDestFramePos = m_queuedJumpTarget;
m_queuedJumpTarget = -1;
auto originalDestFramePos = m_queuedJumpTarget;
m_queuedJumpTarget = mixxx::audio::FramePos(-1);
if (getStatus() == Status::Armed) {
setStatus(Status::Recording);
}
if (getStatus() != Status::Recording) {
return;
}
double sourceFramePos = getSampleOfTrack().current / mixxx::kEngineChannelCount;
double destFramePos = dNewPlaypos / mixxx::kEngineChannelCount;
double diff = originalDestFramePos - destFramePos;
m_recordedActions.try_emplace(sourceFramePos + diff, originalDestFramePos);
auto sourceFramePos = frameInfo().currentPosition / mixxx::kEngineChannelCount;
auto diff = originalDestFramePos - position;
m_recordedActions.try_emplace(sourceFramePos + diff, position);
}

void MacroControl::slotJumpQueued(double samplePos) {
m_queuedJumpTarget = samplePos / mixxx::kEngineChannelCount;
void MacroControl::slotJumpQueued(mixxx::audio::FramePos samplePos) {
m_queuedJumpTarget = samplePos;
}

MacroControl::Status MacroControl::getStatus() const {
Expand Down Expand Up @@ -218,13 +218,12 @@ bool MacroControl::stopRecording() {
} else {
// This will still be the position of the previous track when called from trackLoaded
// since trackLoaded is invoked before the SampleOfTrack of the controls is updated.
m_pMacro->setEnd(getSampleOfTrack().current / mixxx::kEngineChannelCount);
m_pMacro->setEnd(frameInfo().currentPosition);
if (m_pMacro->getLabel().isEmpty()) {
// Automatically set the start position in seconds as label if there
// is no user-defined one
double secPos = m_pMacro->getStartSamplePos() /
mixxx::kEngineChannelCount / getSampleOfTrack().rate;
m_pMacro->setLabel(QString::number(secPos, 'f', 2));
// Automatically set the start position in seconds as label
// if there is no user-defined one
auto secPos = m_pMacro->getStart() / frameInfo().sampleRate;
m_pMacro->setLabel(QString::number(secPos.value(), 'f', 2));
}
setStatus(Status::Recorded);
if (m_pMacro->isEnabled()) {
Expand Down Expand Up @@ -324,7 +323,7 @@ void MacroControl::slotToggle(double value) {

void MacroControl::slotGotoPlay(double value) {
if (value > 0 && getStatus() > Status::Recording) {
seekExact(m_pMacro->getStartSamplePos());
seekExact(m_pMacro->getStart());
play();
}
}
10 changes: 6 additions & 4 deletions src/engine/controls/macrocontrol.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ class MacroControl : public EngineControl {
MacroControl(const QString& group, UserSettingsPointer pConfig, int slot);

void trackLoaded(TrackPointer pNewTrack) override;
void process(const double dRate, const double dCurrentSample, const int iBufferSize) override;
void notifySeek(double dNewPlaypos) override;
void process(const double dRate,
mixxx::audio::FramePos currentPosition,
const int iBufferSize) override;
void notifySeek(mixxx::audio::FramePos position) override;

bool isRecording() const;

Expand All @@ -42,7 +44,7 @@ class MacroControl : public EngineControl {
void slotToggle(double value = 1);
void slotClear(double value = 1);

void slotJumpQueued(double samplePos);
void slotJumpQueued(mixxx::audio::FramePos samplePos);

private:
/// Returns whether a new action was recorded
Expand All @@ -60,7 +62,7 @@ class MacroControl : public EngineControl {
QString m_controlPattern;

/// The unquantized FramePos of a jump that is yet to be processed
double m_queuedJumpTarget;
mixxx::audio::FramePos m_queuedJumpTarget;

rigtorp::SPSCQueue<MacroAction> m_recordedActions;
QTimer m_updateRecordingTimer;
Expand Down
16 changes: 8 additions & 8 deletions src/engine/enginebuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,20 +44,20 @@
namespace {
const mixxx::Logger kLogger("EngineBuffer");

// This value is used to make sure the initial seek after loading a track is
// not omitted. Therefore this value must be different for 0.0 or any likely
// value for the main cue
/// Used to ensure the initial seek after loading a track is not omitted.
/// Therefore this value must stray away from 0.0 or any probably value for the main cue.
constexpr auto kInitialPlayPosition =
mixxx::audio::FramePos::fromEngineSamplePos(
std::numeric_limits<double>::lowest());

constexpr double kLinearScalerElipsis =
1.00058; // 2^(0.01/12): changes < 1 cent allows a linear scaler
/// 2^(0.01/12): changes < 1 cent allows a linear scaler
constexpr double kLinearScalerElipsis = 1.00058;

constexpr SINT kSamplesPerFrame = 2; // Engine buffer uses Stereo frames only
/// Engine buffer uses Stereo frames only
constexpr SINT kSamplesPerFrame = 2;

// Rate at which the playpos slider is updated
constexpr int kPlaypositionUpdateRate = 15; // updates per second
/// Updates per second of the playpos slider
constexpr int kPlaypositionUpdateRate = 15;

} // anonymous namespace

Expand Down
12 changes: 6 additions & 6 deletions src/engine/enginebuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,13 @@ class EngineBuffer : public EngineObject {
SEEK_PHASE = 1u,
/// Bypass Quantization
SEEK_EXACT = 2u,
/// This is an artificial state that happens if an exact seek and a
/// phase seek are scheduled at the same time.
/// This artificial state occurs when an exact seek and a phase seek
/// are scheduled at the same time.
SEEK_EXACT_PHASE = SEEK_PHASE | SEEK_EXACT,
/// #SEEK_PHASE if Quantize enables, otherwise SEEK_EXACT
/// SEEK_PHASE if Quantization is enabled, otherwise SEEK_EXACT
SEEK_STANDARD = 4u,
/// This is an artificial state that happens if a standard seek and a
/// phase seek are scheduled at the same time.
/// This artificial state occurs when a standard seek and a phase seek
/// are scheduled at the same time.
SEEK_STANDARD_PHASE = SEEK_STANDARD | SEEK_PHASE,
};
Q_DECLARE_FLAGS(SeekRequests, SeekRequest);
Expand Down Expand Up @@ -185,7 +185,7 @@ class EngineBuffer : public EngineObject {
signals:
void trackLoaded(TrackPointer pNewTrack, TrackPointer pOldTrack);
void trackLoadFailed(TrackPointer pTrack, const QString& reason);
void cueJumpQueued(double samplePos);
void cueJumpQueued(mixxx::audio::FramePos targetPosition);

private slots:
void slotTrackLoading();
Expand Down
2 changes: 0 additions & 2 deletions src/library/library.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
#include "library/rhythmbox/rhythmboxfeature.h"
#include "library/serato/seratofeature.h"
#include "library/sidebarmodel.h"
#include "library/trackcollection.h"
#include "library/trackcollectionmanager.h"
#include "library/trackmodel.h"
#include "library/trackset/crate/cratefeature.h"
Expand All @@ -49,7 +48,6 @@

namespace {
const mixxx::Logger kLogger("Library");
const QString kConfigGroup("[Library]");
} // anonymous namespace

using namespace mixxx::library::prefs;
Expand Down
34 changes: 17 additions & 17 deletions src/test/macros/macrocontrol_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,33 +40,33 @@ TEST_F(MacroControlTest, RecordSeek) {
EXPECT_EQ(getMacro()->getLabel(), " [Recording]");
// Prepare recording
int frameRate = 1'000;
auto seek = [this, frameRate](double position) {
auto seek = [this, frameRate](mixxx::audio::FramePos position) {
notifySeek(position);
setCurrentSample(position, 99'000, frameRate);
setFrameInfo(position, mixxx::audio::FramePos(99'000), frameRate);
process(0, position, 2);
};
seek(0);
ASSERT_EQ(getStatus(), MacroControl::Status::Armed);

// Initial jump
double startPos = 1'008;
auto startPos = mixxx::audio::FramePos(1'008);
slotJumpQueued(startPos);
seek(startPos);
// Jump with quantization adjustment
TestMacro testMacro;
double diff = 20;
seek(testMacro.action.getSourcePositionSample() - diff);
slotJumpQueued(testMacro.action.getTargetPositionSample());
seek(testMacro.action.getTargetPositionSample() - diff);
auto diff = mixxx::audio::FrameDiff_t(20);
seek(testMacro.action.getSourcePosition() - diff);
slotJumpQueued(testMacro.action.getTargetPosition());
seek(testMacro.action.getTargetPosition() - diff);

// Stop recording
seek(testMacro.action.getTargetPositionSample());
seek(testMacro.action.getTargetPosition());
EXPECT_CALL(*this, seekExact(startPos));
slotRecord(0);
EXPECT_EQ(getStatus(), MacroControl::Status::Playing);
// Check recording result
testMacro.checkMacroAction(getMacro());
EXPECT_EQ(getMacro()->getActions().first().getTargetPositionSample(), startPos);
EXPECT_EQ(getMacro()->getActions().first().getTargetPosition(), startPos);
EXPECT_TRUE(pLoadedTrack->isDirty());
// Check generated label
EXPECT_EQ(startPos / mixxx::kEngineChannelCount / frameRate, 0.504);
Expand Down Expand Up @@ -146,23 +146,23 @@ TEST_F(MacroControlTest, LoadTrackAndPlayAndClear) {
slotActivate();
EXPECT_EQ(getStatus(), MacroControl::Status::Playing);
EXPECT_CALL(*this, seekExact(jumpAction.getTargetPosition()));
process(0, jumpAction.getSourcePositionSample(), 2);
process(0, jumpAction.getSourcePosition(), 2);
EXPECT_EQ(getStatus(), MacroControl::Status::Recorded);

// LOOP
slotLoop(1);
EXPECT_CALL(*this, seekExact(testMacro.action.getTargetPositionSample()));
EXPECT_CALL(*this, seekExact(testMacro.action.getTargetPosition()));
slotActivate();
slotActivate();
// Jump
EXPECT_CALL(*this, seekExact(jumpAction.getTargetPositionSample()));
process(0, jumpAction.getSourcePositionSample(), 2);
EXPECT_CALL(*this, seekExact(jumpAction.getTargetPosition()));
process(0, jumpAction.getSourcePosition(), 2);
// Loop back
EXPECT_CALL(*this, seekExact(testMacro.action.getTargetPositionSample()));
process(0, testMacro.action.getSourcePositionSample(), 2);
EXPECT_CALL(*this, seekExact(testMacro.action.getTargetPosition()));
process(0, testMacro.action.getSourcePosition(), 2);
// Jump again
EXPECT_CALL(*this, seekExact(jumpAction.getTargetPositionSample()));
process(0, jumpAction.getSourcePositionSample(), 2);
EXPECT_CALL(*this, seekExact(jumpAction.getTargetPosition()));
process(0, jumpAction.getSourcePosition(), 2);
EXPECT_EQ(getStatus(), MacroControl::Status::Playing);

// Stop playing
Expand Down
20 changes: 10 additions & 10 deletions src/test/macros/macrorecording_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,16 @@ class MacroRecordingTest : public BaseSignalPathTest {

TEST_F(MacroRecordingTest, RecordSeekAndPlay) {
TestMacro testMacro;
prepRecording(testMacro.action.getSourcePositionSample());
prepRecording(testMacro.action.getSourcePosition());

m_pEngineBuffer1->slotControlSeekAbs(testMacro.action.getTargetPositionSample());
m_pEngineBuffer1->slotControlSeekAbs(testMacro.action.getTargetPosition());
ProcessBuffer();

m_record.set(0);
testMacro.checkMacroAction(getMacro());
// Should activate automatically
EXPECT_EQ(getStatus(), MacroControl::Status::Playing);
EXPECT_EQ(getMacro()->getActions().first().getTargetPositionSample(), kStartPos);
EXPECT_EQ(getMacro()->getActions().first().getTargetPosition(), kStartPos);
ProcessBuffer();
EXPECT_EQ(m_pEngineBuffer1->getExactPlayPos(), kStartPos);

Expand All @@ -67,23 +67,23 @@ TEST_F(MacroRecordingTest, RecordSeekAndPlay) {
EXPECT_EQ(getMacro()->size(), 4);

// Seek to first action
m_pEngineBuffer1->slotControlSeekExact(testMacro.action.getSourcePositionSample());
m_pEngineBuffer1->slotControlSeekExact(testMacro.action.getSourcePosition());
ProcessBuffer();

ProcessBuffer();
ProcessBuffer();
EXPECT_EQ(m_pEngineBuffer1->getExactPlayPos(), testMacro.action.getTargetPositionSample());
EXPECT_EQ(m_pEngineBuffer1->getExactPlayPos(), testMacro.action.getTargetPosition());
ASSERT_EQ(getStatus(), MacroControl::Status::Playing);

// Seek to next action
m_pEngineBuffer1->slotControlSeekExact(action2.getSourcePositionSample());
m_pEngineBuffer1->slotControlSeekExact(action2.getSourcePosition());
ProcessBuffer();

// Trigger remaining actions
ProcessBuffer();
EXPECT_EQ(m_pEngineBuffer1->getExactPlayPos(), action2.getTargetPositionSample());
EXPECT_EQ(m_pEngineBuffer1->getExactPlayPos(), action2.getTargetPosition());
ProcessBuffer();
EXPECT_EQ(m_pEngineBuffer1->getExactPlayPos(), action3.getTargetPositionSample());
EXPECT_EQ(m_pEngineBuffer1->getExactPlayPos(), action3.getTargetPosition());
EXPECT_EQ(getStatus(), MacroControl::Status::Recorded);
}

Expand All @@ -92,11 +92,11 @@ TEST_F(MacroRecordingTest, RecordHotcueAndPlay) {
ControlObject::set(ConfigKey(kChannelGroup, "hotcue_1_set"), 1);

TestMacro testMacro(10'000, 0);
prepRecording(testMacro.action.getSourcePositionSample());
prepRecording(testMacro.action.getSourcePosition());

ControlObject::set(ConfigKey(kChannelGroup, "hotcue_1_goto"), 1);
ProcessBuffer();
EXPECT_EQ(m_pEngineBuffer1->getExactPlayPos(), testMacro.action.getTargetPositionSample());
EXPECT_EQ(m_pEngineBuffer1->getExactPlayPos(), testMacro.action.getTargetPosition());
MacroPointer pMacro = getMacro();

// Check that recording stops gracefully when ejecting
Expand Down
2 changes: 1 addition & 1 deletion src/test/signalpathtest.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ class BaseSignalPathTest : public MixxxTest, SoundSourceProviderRegistration {

const QString kTrackLocationTest = QDir::currentPath() + "/src/test/sine-30.wav";
TrackPointer getTestTrack() const {
return Track::newTemporary(kTrackLocationTest);
return Track::newTemporary(mixxx::FileAccess(mixxx::FileInfo(kTrackLocationTest)));
}

void loadTrack(Deck* pDeck, TrackPointer pTrack) {
Expand Down
Loading

0 comments on commit 796b43a

Please sign in to comment.