Macros: Migrate to FramePos

src/engine/controls/enginecontrol.h

@@ -41,17 +41,17 @@ 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.
+ /// @param dRate current playback rate in audio frames per second
  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);
@@ -66,7 +66,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);
  }
@@ -96,10 +96,10 @@ class EngineControl : public QObject {
  }
  void seek(double fractionalPosition);
  void seekAbs(mixxx::audio::FramePos position);
- /// Seek to an exact sample, no quantizing
- /// virtual only for testing!
+ /// Seek to an exact frame, no quantizing
+ /// virtual only for tests!
  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();

src/engine/controls/macrocontrol.cpp

@@ -76,28 +76,28 @@ 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();
- int bufFrames = iBufferSize / 2;
- // The process method is called roughly every iBufferSize/2 samples, the
+ // The process method is called roughly every iBufferSize frames, 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 > nextAction.getSourcePosition() - iBufferSize &&
+ currentPosition < nextAction.getSourcePosition() + iBufferSize) {
+ seekExact(nextAction.getTargetPosition());
  m_iNextAction++;
  if (m_iNextAction == m_pMacro->size()) {
  if (m_pMacro->isLooped()) {
@@ -134,26 +134,24 @@ 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 queuedTarget = 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);
+ // Account for quantization so the jump stays in-phase but snaps exactly to the original target
+ m_recordedActions.try_emplace(frameInfo().currentPosition + (queuedTarget - position), queuedTarget);
 }
 
-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 {
@@ -218,13 +216,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()) {
@@ -324,7 +321,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();
  }
 }

src/engine/controls/macrocontrol.h

@@ -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;
 
@@ -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
@@ -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;

src/engine/enginebuffer.cpp

@@ -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
 
@@ -999,8 +999,8 @@ void EngineBuffer::processTrackLocked(
  // The way we treat rate inside of EngineBuffer is actually a
  // description of "sample consumption rate" or percentage of samples
  // consumed relative to playing back the track at its native sample
- // rate and normal speed. pitch_adjust does not change the playback
- // rate.
+ // rate and normal speed.
+ // pitch_adjust does not change the playback rate.
  rate = baserate * speed;
 
  // Scaler is up to date now.

src/engine/enginebuffer.h

@@ -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);
@@ -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();

src/test/macros/macro_test.cpp

@@ -3,7 +3,7 @@
 #include <gtest/gtest.h>
 
 TEST(MacroTest, SerializeMacroActions) {
- QList<MacroAction> actions{MacroAction(0, 1)};
+ QList<MacroAction> actions{MacroAction(mixxx::audio::FramePos(0), mixxx::audio::FramePos(1))};
 
  QString filename(QDir::currentPath() % "/src/test/macros/macro_proto");
  ASSERT_TRUE(QFile::exists(filename));

src/test/macros/macro_test.h

@@ -11,16 +11,16 @@ struct TestMacro {
  MacroAction action;
 
  TestMacro(double sourceFramePos = 25'000, double targetFramePos = 7'500)
- : action(sourceFramePos, targetFramePos) {
+ : action(mixxx::audio::FramePos(sourceFramePos), mixxx::audio::FramePos(targetFramePos)) {
  }
 
  /// Checks that the recorded Macro corresponds to the parameters,
  /// including the extra first loop action.
  void checkMacroAction(MacroPointer macro) {
  ASSERT_EQ(macro->size(), 2);
- EXPECT_EQ(macro->getActions().last().getSourcePosition(), action.getSourcePosition());
- EXPECT_EQ(macro->getActions().last().getTargetPosition(), action.getTargetPosition());
+ EXPECT_EQ(macro->getActions().last().getSourcePosition().value(), action.getSourcePosition().value());
+ EXPECT_EQ(macro->getActions().last().getTargetPosition().value(), action.getTargetPosition().value());
  // Loopback action
- EXPECT_EQ(macro->getActions().first().getSourcePosition(), action.getTargetPosition());
+ EXPECT_EQ(macro->getActions().first().getSourcePosition().value(), action.getTargetPosition().value());
  }
 };

src/test/macros/macrocontrol_test.cpp

@@ -20,7 +20,7 @@ class MacroControlTest : public MacroControl, public testing::Test {
  EXPECT_FALSE(isRecording());
  }
 
- MOCK_METHOD1(seekExact, void(double));
+ MOCK_METHOD1(seekExact, void(mixxx::audio::FramePos));
 
  const TrackPointer pLoadedTrack = Track::newTemporary();
 };
@@ -39,37 +39,37 @@ TEST_F(MacroControlTest, RecordSeek) {
  EXPECT_TRUE(isRecording());
  EXPECT_EQ(getMacro()->getLabel(), " [Recording]");
  // Prepare recording
- int frameRate = 1'000;
- auto seek = [this, frameRate](double position) {
+ mixxx::audio::SampleRate frameRate(1'000);
+ auto seek = [this, frameRate](mixxx::audio::FramePos position) {
  notifySeek(position);
- setCurrentSample(position, 99'000, frameRate);
- process(0, position, 2);
+ setFrameInfo(position, mixxx::audio::FramePos(99'000), frameRate);
+ process(frameRate, position, 1);
  };
- seek(0);
+ seek(mixxx::audio::FramePos(0));
  ASSERT_EQ(getStatus(), MacroControl::Status::Armed);
 
  // Initial jump
- double startPos = 1'008;
+ mixxx::audio::FramePos startPos(504);
  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);
+ mixxx::audio::FrameDiff_t diff(10);
+ 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);
+ EXPECT_EQ((startPos / frameRate).value(), 0.504);
  EXPECT_EQ(getMacro()->getLabel().toStdString(), "0.50");
  // Activate
  EXPECT_CALL(*this, seekExact(startPos));
@@ -107,16 +107,16 @@ TEST_F(MacroControlTest, ControlObjects) {
  ControlProxy(kChannelGroup, "macro_2_enable").set(0);
  ControlProxy(kChannelGroup, "macro_2_loop").set(1);
  EXPECT_TRUE(getMacro()->isLooped());
- slotJumpQueued(0);
- notifySeek(0);
+ slotJumpQueued(mixxx::audio::FramePos(0));
+ notifySeek(mixxx::audio::FramePos(0));
  activate.set(1);
  ASSERT_STATUS(MacroControl::Status::Recorded);
  EXPECT_EQ(getMacro()->getLabel(), label);
  activate.set(1);
  ASSERT_STATUS(MacroControl::Status::Playing);
 
  // Restart
- EXPECT_CALL(*this, seekExact(0));
+ EXPECT_CALL(*this, seekExact(mixxx::audio::FramePos(0)));
  activate.set(1);
  ASSERT_STATUS(MacroControl::Status::Playing);
 
@@ -131,7 +131,7 @@ TEST_F(MacroControlTest, ControlObjects) {
 }
 
 TEST_F(MacroControlTest, LoadTrackAndPlayAndClear) {
- MacroAction jumpAction(40'000, 0);
+ MacroAction jumpAction(mixxx::audio::FramePos(40'000), mixxx::audio::FramePos(0));
  TestMacro testMacro;
 
  QString label = QStringLiteral("test");
@@ -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