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

Add support for stem in the engine #13070

Merged
merged 17 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
5 changes: 3 additions & 2 deletions src/analyzer/analyzersilence.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,11 @@ SINT AnalyzerSilence::findLastSoundInChunk(std::span<const CSAMPLE> samples) {
// static
bool AnalyzerSilence::verifyFirstSound(
std::span<const CSAMPLE> samples,
mixxx::audio::FramePos firstSoundFrame) {
mixxx::audio::FramePos firstSoundFrame,
mixxx::audio::ChannelCount channelCount) {
const SINT firstSoundSample = findFirstSoundInChunk(samples);
if (firstSoundSample < static_cast<SINT>(samples.size())) {
return mixxx::audio::FramePos::fromEngineSamplePos(firstSoundSample)
return mixxx::audio::FramePos::fromEngineSamplePos(firstSoundSample, channelCount)
.toLowerFrameBoundary() == firstSoundFrame.toLowerFrameBoundary();
}
return false;
Expand Down
3 changes: 2 additions & 1 deletion src/analyzer/analyzersilence.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ class AnalyzerSilence : public Analyzer {
/// last analysis run and is an indicator for file edits or decoder
/// changes/issues
static bool verifyFirstSound(std::span<const CSAMPLE> samples,
mixxx::audio::FramePos firstSoundFrame);
mixxx::audio::FramePos firstSoundFrame,
mixxx::audio::ChannelCount channelCount);

private:
UserSettingsPointer m_pConfig;
Expand Down
2 changes: 1 addition & 1 deletion src/analyzer/constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ namespace mixxx {
// depending on the track length. A block size of 4096 frames per block
// seems to do fine. Signal processing during analysis uses the same,
// fixed number of channels like the engine does, usually 2 = stereo.
constexpr audio::ChannelCount kAnalysisChannels = mixxx::kEngineChannelCount;
constexpr audio::ChannelCount kAnalysisChannels = mixxx::kEngineChannelOutputCount;
constexpr SINT kAnalysisFramesPerChunk = 4096;
constexpr SINT kAnalysisSamplesPerChunk =
kAnalysisFramesPerChunk * kAnalysisChannels;
Expand Down
24 changes: 16 additions & 8 deletions src/audio/frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,19 @@ class FramePos final {
/// Return a `FramePos` from a given engine sample position. To catch
/// "invalid" positions (e.g. when parsing values from control objects),
/// use `FramePos::fromEngineSamplePosMaybeInvalid` instead.
static constexpr FramePos fromEngineSamplePos(double engineSamplePos) {
return FramePos(engineSamplePos / mixxx::kEngineChannelCount);
static constexpr FramePos fromEngineSamplePos(double engineSamplePos,
mixxx::audio::ChannelCount channelCount =
mixxx::kEngineChannelOutputCount) {
return FramePos(engineSamplePos / channelCount);
}

/// Return an engine sample position. The `FramePos` is expected to be
/// valid. If invalid positions are possible (e.g. for control object
/// values), use `FramePos::toEngineSamplePosMaybeInvalid` instead.
double toEngineSamplePos() const {
double toEngineSamplePos(mixxx::audio::ChannelCount channelCount =
mixxx::kEngineChannelOutputCount) const {
Copy link
Member

Choose a reason for hiding this comment

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

EngineSamplePos is the term for the fixes stereo assumption that is part of the database and the CO interface. This must not change and therefore this function must not have parameter.

Under the light of this PR, we may consider a new name for the function and a different name for the new use case.

DEBUG_ASSERT(isValid());
double engineSamplePos = value() * mixxx::kEngineChannelCount;
double engineSamplePos = value() * channelCount;
// In the rare but possible instance that the position is valid but
// the engine sample position is exactly -1.0, we nudge the position
// because otherwise fromEngineSamplePosMaybeInvalid() will think
Expand All @@ -63,11 +66,14 @@ class FramePos final {
/// for compatibility with our control objects and legacy parts of the code
/// base. Using a different code path based on the output of `isValid()` is
/// preferable.
static constexpr FramePos fromEngineSamplePosMaybeInvalid(double engineSamplePos) {
static constexpr FramePos fromEngineSamplePosMaybeInvalid(
double engineSamplePos,
mixxx::audio::ChannelCount channelCount =
mixxx::kEngineChannelOutputCount) {
if (engineSamplePos == kLegacyInvalidEnginePosition) {
return {};
}
return fromEngineSamplePos(engineSamplePos);
return fromEngineSamplePos(engineSamplePos, channelCount);
}

/// Return an engine sample position. If the `FramePos` is invalid,
Expand All @@ -77,11 +83,13 @@ class FramePos final {
/// for compatibility with our control objects and legacy parts of the code
/// base. Using a different code path based on the output of `isValid()` is
/// preferable.
double toEngineSamplePosMaybeInvalid() const {
double toEngineSamplePosMaybeInvalid(
mixxx::audio::ChannelCount channelCount =
mixxx::kEngineChannelOutputCount) const {
Copy link
Member

Choose a reason for hiding this comment

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

The same here. pleas remove the parameter and probably introduce a new function for your use case.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have introduced toSample and fromSample functions. I have taken the freedom to use SINT for return with a isFractionnal assert + some doc as to why (TL;DR: doesn't make sense to work with half a sample in engine buffers) . Alternative would be about ~50 static_cast on the engine code...

if (!isValid()) {
return kLegacyInvalidEnginePosition;
}
return toEngineSamplePos();
return toEngineSamplePos(channelCount);
}

/// Return true if the frame position is valid. Any finite value is
Expand Down
17 changes: 17 additions & 0 deletions src/audio/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,14 @@ class ChannelCount {
return ChannelCount(valueFromInt(value));
}

static ChannelCount fromDouble(double value) {
const auto channelCount = ChannelCount(static_cast<value_t>(value));
// The channel count should always be an integer value
// and this conversion is supposed to be lossless.
DEBUG_ASSERT(channelCount.toDouble() == value);
return channelCount;
}

static constexpr ChannelCount mono() {
return ChannelCount(static_cast<value_t>(1));
}
Expand All @@ -88,6 +96,10 @@ class ChannelCount {
return ChannelCount(static_cast<value_t>(2));
}

static constexpr ChannelCount stem() {
return ChannelCount(static_cast<value_t>(8)); // 4 stereo channels
}

explicit constexpr ChannelCount(
value_t value = kValueDefault)
: m_value(value) {
Expand Down Expand Up @@ -115,6 +127,11 @@ class ChannelCount {
return value();
}

// Helper cast for COs
constexpr double toDouble() const {
return static_cast<double>(value());
}

private:
value_t m_value;
};
Expand Down
19 changes: 15 additions & 4 deletions src/engine/bufferscalers/enginebufferscale.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@

#include "engine/engine.h"
#include "moc_enginebufferscale.cpp"
#include "soundio/soundmanagerconfig.h"

EngineBufferScale::EngineBufferScale()
: m_outputSignal(
mixxx::audio::SignalInfo(
mixxx::kEngineChannelCount,
mixxx::kEngineChannelOutputCount,
mixxx::audio::SampleRate())),
m_dBaseRate(1.0),
m_bSpeedAffectsPitch(false),
Expand All @@ -16,12 +17,22 @@ EngineBufferScale::EngineBufferScale()
DEBUG_ASSERT(!m_outputSignal.isValid());
}

void EngineBufferScale::setSampleRate(
mixxx::audio::SampleRate sampleRate) {
void EngineBufferScale::setOutputSignal(
acolombier marked this conversation as resolved.
Show resolved Hide resolved
mixxx::audio::SampleRate sampleRate,
mixxx::audio::ChannelCount channelCount) {
DEBUG_ASSERT(sampleRate.isValid());
DEBUG_ASSERT(channelCount.isValid());
bool changed = false;
if (sampleRate != m_outputSignal.getSampleRate()) {
m_outputSignal.setSampleRate(sampleRate);
onSampleRateChanged();
changed = true;
}
if (channelCount != m_outputSignal.getChannelCount()) {
m_outputSignal.setChannelCount(channelCount);
changed = true;
}
if (changed) {
onOutputSignalChanged();
}
DEBUG_ASSERT(m_outputSignal.isValid());
}
9 changes: 5 additions & 4 deletions src/engine/bufferscalers/enginebufferscale.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@ class EngineBufferScale : public QObject {
m_dPitchRatio = *pPitchRatio;
}

// Set the desired output sample rate.
void setSampleRate(
mixxx::audio::SampleRate sampleRate);
// Set the desired output signal.
void setOutputSignal(
mixxx::audio::SampleRate sampleRate,
mixxx::audio::ChannelCount channelCout);

const mixxx::audio::SignalInfo& getOutputSignal() const {
return m_outputSignal;
Expand All @@ -66,7 +67,7 @@ class EngineBufferScale : public QObject {
private:
mixxx::audio::SignalInfo m_outputSignal;

virtual void onSampleRateChanged() = 0;
virtual void onOutputSignalChanged() = 0;

protected:
double m_dBaseRate;
Expand Down
Loading
Loading