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

Add Sample-Based Play Position Control (m_playposition_samples) #13822

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add Sample-Based Play Position Control (m_playposition_samples) #13822

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions src/engine/enginebuffer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@

m_pRepeat = new ControlPushButton(ConfigKey(m_group, "repeat"));
m_pRepeat->setButtonMode(mixxx::control::ButtonMode::Toggle);
m_playposition_samples = new ControlObject<double>(ConfigKey(m_group, "playposition_samples"));

Check failure on line 170 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / macOS 12 x64 

use of undeclared identifier 'm_playposition_samples'

Check failure on line 170 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / macOS 12 x64 

expected '(' for function-style cast or type construction

Check failure on line 170 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / Windows 2019 (MSVC) 

'm_playposition_samples': undeclared identifier

Check failure on line 170 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / Windows 2019 (MSVC) 

type 'double' unexpected

Check failure on line 170 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / macOS 12 arm64 

use of undeclared identifier 'm_playposition_samples'

Check failure on line 170 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / macOS 12 arm64 

expected '(' for function-style cast or type construction

Check failure on line 170 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / Ubuntu 22.04 

‘m_playposition_samples’ was not declared in this scope

Check failure on line 170 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / Ubuntu 22.04 

‘ControlObject’ is not a template

Check failure on line 170 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / coverage 

‘m_playposition_samples’ was not declared in this scope

Check failure on line 170 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / coverage 

‘ControlObject’ is not a template

Check failure on line 170 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / clang-tidy 

use of undeclared identifier 'm_playposition_samples' [clang-diagnostic-error]

Check failure on line 170 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / clang-tidy 

expected '(' for function-style cast or type construction [clang-diagnostic-error]

Check failure on line 170 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / clazy 

use of undeclared identifier 'm_playposition_samples'

Check failure on line 170 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / clazy 

expected '(' for function-style cast or type construction
Copy link
Contributor

@cr7pt0gr4ph7 cr7pt0gr4ph7 Nov 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

For consistency reasons, m_playposition_samples should probably intialized directly after m_playposSlider to keep playposition and playposition_samples closer together.

mixxx/src/engine/enginebuffer.cpp

Lines 158 to 164 in edae0ca

m_playposSlider = new ControlLinPotmeter(
ConfigKey(m_group, "playposition"), 0.0, 1.0, 0, 0, true);
connect(m_playposSlider, &ControlObject::valueChanged,
this, &EngineBuffer::slotControlSeek,
Qt::DirectConnection);

Copy link
Contributor

Choose a reason for hiding this comment

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

Additionally, I'd suggest the name m_playPositionInSamples instead to adhere to camelCase naming conventions and to make the name a bit more descriptive.


m_pSampleRate = new ControlProxy(kAppGroup, QStringLiteral("samplerate"), this);

Expand Down Expand Up @@ -294,6 +295,7 @@
#endif
delete m_pReadAheadManager;
delete m_pReader;
delete m_playposition_samples;

Check failure on line 298 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / macOS 12 x64 

use of undeclared identifier 'm_playposition_samples'

Check failure on line 298 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / Windows 2019 (MSVC) 

'm_playposition_samples': undeclared identifier

Check failure on line 298 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / Windows 2019 (MSVC) 

'delete': cannot delete objects that are not pointers

Check failure on line 298 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / macOS 12 arm64 

use of undeclared identifier 'm_playposition_samples'

Check failure on line 298 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / Ubuntu 22.04 

‘m_playposition_samples’ was not declared in this scope

Check failure on line 298 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / coverage 

‘m_playposition_samples’ was not declared in this scope

Check failure on line 298 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / clang-tidy 

use of undeclared identifier 'm_playposition_samples' [clang-diagnostic-error]

Check failure on line 298 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / clazy 

use of undeclared identifier 'm_playposition_samples'

delete m_playButton;
delete m_playStartButton;
Expand Down Expand Up @@ -464,6 +466,7 @@
if (kLogger.traceEnabled()) {
kLogger.trace() << m_group << "EngineBuffer::setNewPlaypos" << position;
}
m_playposition_samples->set(m_playPos.toEngineSamplePos());

Check failure on line 469 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / macOS 12 x64 

use of undeclared identifier 'm_playposition_samples'

Check failure on line 469 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / Windows 2019 (MSVC) 

'm_playposition_samples': undeclared identifier

Check failure on line 469 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / macOS 12 arm64 

use of undeclared identifier 'm_playposition_samples'

Check failure on line 469 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / Ubuntu 22.04 

‘m_playposition_samples’ was not declared in this scope

Check failure on line 469 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / coverage 

‘m_playposition_samples’ was not declared in this scope

Check failure on line 469 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / clang-tidy 

use of undeclared identifier 'm_playposition_samples' [clang-diagnostic-error]

Check failure on line 469 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / clazy 

use of undeclared identifier 'm_playposition_samples'
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you intend to set m_playposition_samples to the old value before the new play position is applied?

Otherwise, I'm confused why you set m_playposition_samples here instead of at the bottom of setNewPlaypos.

Although, as mentioned in the other comments, m_playposition_samples probably shouldn't even be set here at all, but only in updateIndicators.


m_playPos = position;

Expand Down Expand Up @@ -712,12 +715,16 @@
void EngineBuffer::seekAbs(mixxx::audio::FramePos position) {
DEBUG_ASSERT(position.isValid());
doSeekPlayPos(position, SEEK_STANDARD);
m_playposition_samples->set(position.toEngineSamplePos());

Check failure on line 718 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / macOS 12 x64 

use of undeclared identifier 'm_playposition_samples'

Check failure on line 718 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / Windows 2019 (MSVC) 

'm_playposition_samples': undeclared identifier

Check failure on line 718 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / macOS 12 arm64 

use of undeclared identifier 'm_playposition_samples'

Check failure on line 718 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / Ubuntu 22.04 

‘m_playposition_samples’ was not declared in this scope

Check failure on line 718 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / coverage 

‘m_playposition_samples’ was not declared in this scope

Check failure on line 718 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / clang-tidy 

use of undeclared identifier 'm_playposition_samples' [clang-diagnostic-error]

Check failure on line 718 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / clazy 

use of undeclared identifier 'm_playposition_samples'

Comment on lines +718 to +719
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
m_playposition_samples->set(position.toEngineSamplePos());

m_playposition_samples should only be set in updateIndicators to ensure consistent behavior with the other indicators.

}

// WARNING: This method is called by EngineControl and runs in the engine thread
void EngineBuffer::seekExact(mixxx::audio::FramePos position) {
DEBUG_ASSERT(position.isValid());
doSeekPlayPos(position, SEEK_EXACT);
m_playposition_samples->set(position.toEngineSamplePos());

Check failure on line 726 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / macOS 12 x64 

use of undeclared identifier 'm_playposition_samples'

Check failure on line 726 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / Windows 2019 (MSVC) 

'm_playposition_samples': undeclared identifier

Check failure on line 726 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / macOS 12 arm64 

use of undeclared identifier 'm_playposition_samples'

Check failure on line 726 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / Ubuntu 22.04 

‘m_playposition_samples’ was not declared in this scope

Check failure on line 726 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / coverage 

‘m_playposition_samples’ was not declared in this scope

Check failure on line 726 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / clang-tidy 

use of undeclared identifier 'm_playposition_samples' [clang-diagnostic-error]

Check failure on line 726 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / clazy 

use of undeclared identifier 'm_playposition_samples'

Comment on lines +726 to +727
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
m_playposition_samples->set(position.toEngineSamplePos());

m_playposition_samples should only be set in updateIndicators to ensure consistent behavior with the other indicators (see comment above).

}

double EngineBuffer::fractionalPlayposFromAbsolute(mixxx::audio::FramePos absolutePlaypos) {
Expand Down Expand Up @@ -1457,6 +1464,7 @@
// called yet.
return;
}
m_playposition_samples->set(m_playPos.toEngineSamplePos());

Check failure on line 1467 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / macOS 12 x64 

use of undeclared identifier 'm_playposition_samples'

Check failure on line 1467 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / Windows 2019 (MSVC) 

'm_playposition_samples': undeclared identifier

Check failure on line 1467 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / macOS 12 arm64 

use of undeclared identifier 'm_playposition_samples'

Check failure on line 1467 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / Ubuntu 22.04 

‘m_playposition_samples’ was not declared in this scope

Check failure on line 1467 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / coverage 

‘m_playposition_samples’ was not declared in this scope

Check failure on line 1467 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / clang-tidy 

use of undeclared identifier 'm_playposition_samples' [clang-diagnostic-error]

Check failure on line 1467 in src/engine/enginebuffer.cpp

View workflow job for this annotation

GitHub Actions / clazy 

use of undeclared identifier 'm_playposition_samples'
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
m_playposition_samples->set(m_playPos.toEngineSamplePos());
// Update playposition_samples CO
m_playposition_samples->set(m_playPos.toEngineSamplePos());


// Increase samplesCalculated by the buffer size
m_iSamplesSinceLastIndicatorUpdate += iBufferSize;
Expand Down
Loading