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

repeat: after last track sample, fill buffer with samples from track start #11532

Merged
merged 4 commits into from
Jul 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
19 changes: 19 additions & 0 deletions src/engine/controls/loopingcontrol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,8 @@ LoopingControl::LoopingControl(const QString& group,
this, &LoopingControl::slotLoopDouble);

m_pPlayButton = ControlObject::getControl(ConfigKey(group, "play"));

m_pRepeatButton = ControlObject::getControl(ConfigKey(group, "repeat"));
}

LoopingControl::~LoopingControl() {
Expand Down Expand Up @@ -443,9 +445,26 @@ double LoopingControl::nextTrigger(bool reverse,
}
}
}

// Return trigger if repeat is enabled
if (m_pRepeatButton->toBool()) {
if (reverse) {
*pTarget = getTrackSamples();
return 0.0;
} else {
*pTarget = 0.0;
return getTrackSamples();
}
}

return kNoTrigger;
}

double LoopingControl::getTrackSamples() {
SampleOfTrack trackSamples = getSampleOfTrack();
return trackSamples.total;
}

void LoopingControl::hintReader(HintVector* pHintList) {
LoopSamples loopSamples = m_loopSamples.getValue();
Hint loop_hint;
Expand Down
3 changes: 3 additions & 0 deletions src/engine/controls/loopingcontrol.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ class LoopingControl : public EngineControl {
void trackLoaded(TrackPointer pNewTrack) override;
void trackBeatsUpdated(mixxx::BeatsPointer pBeats) override;

double getTrackSamples();

public slots:
void slotLoopIn(double pressed);
void slotLoopInGoto(double);
Expand Down Expand Up @@ -128,6 +130,7 @@ class LoopingControl : public EngineControl {
ControlObject* m_pSlipEnabled;
RateControl* m_pRateControl;
ControlObject* m_pPlayButton;
ControlObject* m_pRepeatButton;

bool m_bLoopingEnabled;
bool m_bLoopRollActive;
Expand Down
41 changes: 22 additions & 19 deletions src/engine/enginebuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -221,9 +221,6 @@ EngineBuffer::EngineBuffer(const QString& group,
pMixingEngine->getEngineSync()->addSyncableDeck(m_pSyncControl);
addControl(m_pSyncControl);

m_fwdButton = ControlObject::getControl(ConfigKey(group, "fwd"));
m_backButton = ControlObject::getControl(ConfigKey(group, "back"));

m_pKeyControl = new KeyControl(group, pConfig);
addControl(m_pKeyControl);

Expand Down Expand Up @@ -932,6 +929,7 @@ void EngineBuffer::processTrackLocked(
rate = m_rate_old;
}

double playpos_old = m_filepos_play;
bool at_end = m_filepos_play >= m_trackSamplesOld;
bool backwards = rate < 0;

Expand Down Expand Up @@ -973,6 +971,8 @@ void EngineBuffer::processTrackLocked(
// Note: The last buffer of a track is padded with silence.
// This silence is played together with the last samples in the last
// callback and the m_filepos_play is advanced behind the end of the track.
// If repeat is enabled, scaler->scaleBuffer() wraps around at end/start
// and fills the buffer with samples from the other end of the track.

if (m_bCrossfadeReady) {
// Bring pOutput with the new parameters in and fade out the old one,
Expand Down Expand Up @@ -1004,24 +1004,27 @@ void EngineBuffer::processTrackLocked(

m_scratching_old = is_scratching;

// Handle repeat mode
bool at_start = m_filepos_play <= 0;
at_end = m_filepos_play >= m_trackSamplesOld;

bool repeat_enabled = m_pRepeat->toBool();
// If we're repeating and crossed the track boundary, ReadAheadManager already
// wrapped around the playposition.
// To ensure quantize is respected we request a phase sync.
// TODO(ronso) This just restores previous repeat+quantize behaviour. I'm not
// sure whether that was actually desired or just a side effect of seeking.
// Ife it's really desired, should this be moved to looping control in order
// to set the sync'ed playposition right away and fill the wrap-around buffer
// with correct samples from the sync'ed loop in / track start position?
if (m_pRepeat->toBool() && m_pQuantize->toBool() &&
(m_filepos_play > playpos_old) == backwards) {
// TODO() The resulting seek is processed in the following callback
// That is to late
requestSyncPhase();
Copy link
Member

@daschuer daschuer May 6, 2023

Choose a reason for hiding this comment

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

requestSyncPhase() requires an other track playing to be in sync. Did you consider a solution where the repeat + quantize becomes a beat loop that the track stays in sync with itself?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, this is only supposed to sync phase when there's another track. I'm still not sure if that's a good idea, and for which use case it helps, considering users expecting beat and phase sync would set the loop manually, i.e. don't rely on phase sync to snap to the correct beat.

and no, this should not be an extension of the original beat loop fix.

}

bool end_of_track = //(at_start && backwards) ||
(at_end && !backwards);
at_end = m_filepos_play >= m_trackSamplesOld;
bool end_of_track = at_end && !backwards;

// If playbutton is pressed, check if we are at start or end of track
if ((m_playButton->toBool() || (m_fwdButton->toBool() || m_backButton->toBool()))
&& end_of_track) {
if (repeat_enabled) {
double fractionalPos = at_start ? 1.0 : 0;
doSeekFractional(fractionalPos, SEEK_STANDARD);
} else {
m_playButton->set(0.);
}
// If playbutton is pressed and we're at the end of track release play button
if (m_playButton->toBool() && end_of_track) {
m_playButton->set(0.);
}

// Give the Reader hints as to which chunks of the current song we
Expand Down
2 changes: 0 additions & 2 deletions src/engine/enginebuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -329,8 +329,6 @@ class EngineBuffer : public EngineObject {
ControlPushButton* m_stopStartButton;
ControlPushButton* m_stopButton;

ControlObject* m_fwdButton;
ControlObject* m_backButton;
ControlPushButton* m_pSlipButton;

ControlObject* m_pQuantize;
Expand Down
26 changes: 15 additions & 11 deletions src/engine/readaheadmanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ SINT ReadAheadManager::getNextSamples(double dRate, CSAMPLE* pOutput,
//qDebug() << "start" << start_sample << requested_samples;

double target;
// A loop will only limit the amount we can read in one shot.
// A loop (beat loop or track on repeat) will only limit the amount we
// can read in one shot.
const double loop_trigger = m_pLoopingControl->nextTrigger(
in_reverse, m_currentPosition, &target);

Expand Down Expand Up @@ -115,7 +116,7 @@ SINT ReadAheadManager::getNextSamples(double dRate, CSAMPLE* pOutput,
if (reachedTrigger) {
DEBUG_ASSERT(target != kNoTrigger);

// Jump to other end of loop.
// Jump to other end of loop or track.
m_currentPosition = target;
if (preloop_samples > 0) {
// we are up to one frame ahead of the loop trigger
Expand All @@ -140,15 +141,18 @@ SINT ReadAheadManager::getNextSamples(double dRate, CSAMPLE* pOutput,
int loop_read_position = SampleUtil::roundPlayPosToFrameStart(
m_currentPosition + (in_reverse ? preloop_samples : -preloop_samples), kNumChannels);

const auto readResult = m_pReader->read(
loop_read_position, samples_from_reader, in_reverse, m_pCrossFadeBuffer);
if (readResult == CachingReader::ReadResult::UNAVAILABLE) {
qDebug() << "ERROR: Couldn't get all needed samples for crossfade.";
// Cache miss - no samples written
SampleUtil::clear(m_pCrossFadeBuffer, samples_from_reader);
// Set the cache miss flag to decide when to apply ramping
// after the following read attempts.
m_cacheMissHappened = true;
int crossFadeStart = 0;
int crossFadeSamples = samples_from_reader;
if (loop_read_position < 0) {
// we start in the pre-role without suitable samples for crossfading
crossFadeStart = -loop_read_position;
crossFadeSamples -= crossFadeStart;
} else {
int trackSamples = static_cast<int>(m_pLoopingControl->getTrackSamples());
if (loop_read_position > trackSamples) {
crossFadeStart = loop_read_position - trackSamples;
crossFadeSamples -= crossFadeStart;
}
}

// do crossfade from the current buffer into the new loop beginning
Expand Down
2 changes: 2 additions & 0 deletions src/test/readaheadmanager_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ class ReadAheadManagerTest : public MixxxTest {
m_beatPrevCO(ConfigKey(kGroup, "beat_prev")),
m_playCO(ConfigKey(kGroup, "play")),
m_quantizeCO(ConfigKey(kGroup, "quantize")),
m_repeatCO(ConfigKey(kGroup, "repeat")),
m_slipEnabledCO(ConfigKey(kGroup, "slip_enabled")),
m_trackSamplesCO(ConfigKey(kGroup, "track_samples")),
m_pBuffer(SampleUtil::alloc(MAX_BUFFER_LEN)) {
Expand All @@ -105,6 +106,7 @@ class ReadAheadManagerTest : public MixxxTest {
ControlObject m_beatPrevCO;
ControlObject m_playCO;
ControlObject m_quantizeCO;
ControlObject m_repeatCO;
ControlObject m_slipEnabledCO;
ControlObject m_trackSamplesCO;
CSAMPLE* m_pBuffer;
Expand Down