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

JACK buffer size fix. #11121

Merged
merged 21 commits into from
Feb 4, 2023
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
c2b1af8
Fix condition in JACK backend check
robbert-vdh Nov 24, 2022
82c7f71
Replace m_framesPerBuffer uses with runtime size
robbert-vdh Dec 6, 2022
fefbfd2
Use std::unique_ptr for FIFOs in PortAudio backend
robbert-vdh Dec 6, 2022
cfeffa7
Remove unused getProcessingLatency() function
robbert-vdh Dec 6, 2022
1451803
renamed m_framesPerBuffer to m_configFramesPerBuffer
daschuer Dec 3, 2022
b1b14b1
Improve the comment that explains paFramesPerBufferUnspecified
daschuer Dec 3, 2022
8f97592
Don't use the m_configFramesPerBuffe and improve debug output and com…
daschuer Dec 4, 2022
4c1eb24
dispose m_iBufferSize and use always the callback value as parameter
daschuer Dec 4, 2022
e52de39
Always use the real framesPerBuffer value received via the callback
daschuer Dec 4, 2022
75d0e29
Deduplicate code in VisualPlayPosition
daschuer Dec 6, 2022
f1bc670
Apply a better offset limit in VisualPlayposition
daschuer Dec 6, 2022
096089d
Dispose audio_buffer_size and use directly the callback parameter
daschuer Dec 6, 2022
6d24c26
Use MAX_BUFFER_LEN for all non clock reference devices in case of JACK.
daschuer Dec 7, 2022
705e097
Don't allow setting device sync and network clock with the JACK API
daschuer Dec 8, 2022
e15cda3
Avoid implicit conversion from double to bool
daschuer Dec 8, 2022
b48e06d
Improve comment about vectorization
daschuer Dec 8, 2022
185a681
Improve comments and debug output
daschuer Dec 8, 2022
44fa755
check channel count for > 0 instead of implicit bool conversion
daschuer Dec 8, 2022
32379a6
Remove unused variable
daschuer Dec 9, 2022
31ba28e
Fix jackApiUsed() implementation
daschuer Dec 16, 2022
f49bf12
Use the correct term frames per period
daschuer Jan 25, 2023
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
9 changes: 6 additions & 3 deletions src/engine/enginebuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ void EngineBuffer::ejectTrack() {
m_pTrackLoaded->forceSet(0);
m_pTrackSamples->set(0);
m_pTrackSampleRate->set(0);
m_visualPlayPos->set(0.0, 0.0, 0.0, 0.0, 0.0);
m_visualPlayPos->set(0.0, 0.0, 0.0, 0.0, 0.0, 0.0);
TrackPointer pTrack = m_pCurrentTrack;
m_pCurrentTrack.reset();
m_playButton->set(0.0);
Expand Down Expand Up @@ -1298,10 +1298,13 @@ void EngineBuffer::updateIndicators(double speed, int iBufferSize) {

// Update visual control object, this needs to be done more often than the
// playpos slider
m_visualPlayPos->set(fFractionalPlaypos, speed * m_baserate_old,
m_visualPlayPos->set(
fFractionalPlaypos,
speed * m_baserate_old,
(double)iBufferSize / m_trackSamplesOld,
fractionalPlayposFromAbsolute(m_dSlipPosition),
tempoTrackSeconds);
tempoTrackSeconds,
iBufferSize / kSamplesPerFrame / static_cast<double>(m_iSampleRate) * 1000000.0);
}

void EngineBuffer::hintReader(const double dRate) {
Expand Down
231 changes: 138 additions & 93 deletions src/engine/enginemaster.cpp

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions src/engine/enginemaster.h
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,10 @@ class EngineMaster : public QObject, public AudioSource {
void processChannels(int iBufferSize);

ChannelHandleFactoryPointer m_pChannelHandleFactory;
void applyMasterEffects();
void processHeadphones(const CSAMPLE_GAIN masterMixGainInHeadphones);
void applyMasterEffects(int iBufferSize);
void processHeadphones(
const CSAMPLE_GAIN masterMixGainInHeadphones,
int iBufferSize);
bool sidechainMixRequired() const;

EngineEffectsManager* m_pEngineEffectsManager;
Expand All @@ -284,7 +286,6 @@ class EngineMaster : public QObject, public AudioSource {
QVarLengthArray<ChannelInfo*, kPreallocatedChannels> m_activeTalkoverChannels;

unsigned int m_iSampleRate;
unsigned int m_iBufferSize;

// Mixing buffers for each output.
CSAMPLE* m_pOutputBusBuffers[3];
Expand All @@ -302,7 +303,6 @@ class EngineMaster : public QObject, public AudioSource {
ControlObject* m_pHeadGain;
ControlObject* m_pMasterSampleRate;
ControlObject* m_pMasterLatency;
ControlObject* m_pMasterAudioBufferSize;
ControlObject* m_pAudioLatencyOverloadCount;
ControlObject* m_pNumMicsConfigured;
ControlPotmeter* m_pAudioLatencyUsage;
Expand Down
5 changes: 5 additions & 0 deletions src/preferences/dialog/dlgprefsound.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -522,10 +522,15 @@ void DlgPrefSound::apiChanged(int index) {
sampleRateComboBox->setEnabled(false);
latencyLabel->setEnabled(false);
audioBufferComboBox->setEnabled(false);
deviceSyncComboBox->setEnabled(false);
engineClockComboBox->setEnabled(false);

} else {
sampleRateComboBox->setEnabled(true);
latencyLabel->setEnabled(true);
audioBufferComboBox->setEnabled(true);
deviceSyncComboBox->setEnabled(true);
engineClockComboBox->setEnabled(true);
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/soundio/sounddevice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ SoundDevice::SoundDevice(UserSettingsPointer config, SoundManager* sm)
m_iNumInputChannels(2),
m_dSampleRate(44100.0),
m_hostAPI("Unknown API"),
m_framesPerBuffer(0) {
m_configFramesPerBuffer(0) {
}

int SoundDevice::getNumInputChannels() const {
Expand All @@ -36,15 +36,15 @@ void SoundDevice::setSampleRate(double sampleRate) {
m_dSampleRate = sampleRate;
}

void SoundDevice::setFramesPerBuffer(unsigned int framesPerBuffer) {
void SoundDevice::setConfigFramesPerBuffer(unsigned int framesPerBuffer) {
if (framesPerBuffer * 2 > MAX_BUFFER_LEN) {
// framesPerBuffer * 2 because a frame will generally end up
// being 2 samples and MAX_BUFFER_LEN is a number of samples
// this isn't checked elsewhere, so...
reportFatalErrorAndQuit("framesPerBuffer too big in "
"SoundDevice::setFramesPerBuffer(uint)");
}
m_framesPerBuffer = framesPerBuffer;
m_configFramesPerBuffer = framesPerBuffer;
JoergAtGithub marked this conversation as resolved.
Show resolved Hide resolved
}

SoundDeviceError SoundDevice::addOutput(const AudioOutputBuffer &out) {
Expand Down
13 changes: 9 additions & 4 deletions src/soundio/sounddevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ class SoundDevice {
return m_hostAPI;
}
void setSampleRate(double sampleRate);
void setFramesPerBuffer(unsigned int framesPerBuffer);
void setConfigFramesPerBuffer(unsigned int framesPerBuffer);
virtual SoundDeviceError open(bool isClkRefDevice, int syncBuffers) = 0;
virtual bool isOpen() const = 0;
virtual SoundDeviceError close() = 0;
virtual void readProcess() = 0;
virtual void writeProcess() = 0;
virtual void readProcess(SINT framesPerBuffer) = 0;
virtual void writeProcess(SINT framesPerBuffer) = 0;
virtual QString getError() const = 0;
virtual unsigned int getDefaultSampleRate() const = 0;
int getNumOutputChannels() const;
Expand Down Expand Up @@ -84,7 +84,12 @@ class SoundDevice {
double m_dSampleRate;
// The name of the audio API used by this device.
QString m_hostAPI;
SINT m_framesPerBuffer;
// The **configured** number of frames per buffer. We'll tell PortAudio we
// want this many frames in a buffer, but PortAudio may still give us have a
// differently sized buffers. As such this value should only be used for
// configuring the audio devices. The actual runtime buffer size should be
// used for any computations working with audio.
SINT m_configFramesPerBuffer;
QList<AudioOutputBuffer> m_audioOutputs;
QList<AudioInputBuffer> m_audioInputs;
};
Expand Down
58 changes: 31 additions & 27 deletions src/soundio/sounddevicenetwork.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,23 +63,24 @@ SoundDeviceError SoundDeviceNetwork::open(bool isClkRefDevice, int syncBuffers)
m_dSampleRate = 44100.0;
}

qDebug() << "framesPerBuffer:" << m_framesPerBuffer;
const SINT framesPerBuffer = m_configFramesPerBuffer;
qDebug() << "framesPerBuffer:" << framesPerBuffer;

m_audioBufferTime = mixxx::Duration::fromSeconds(
m_framesPerBuffer / m_dSampleRate);
const auto requestedBufferTime = mixxx::Duration::fromSeconds(
framesPerBuffer / m_dSampleRate);
qDebug() << "Requested sample rate: " << m_dSampleRate << "Hz, latency:"
<< m_audioBufferTime;
<< requestedBufferTime;

// Feed the network device buffer directly from the
// clock reference device callback
// This is what should work best.
if (m_iNumOutputChannels) {
m_outputFifo = std::make_unique<FIFO<CSAMPLE> >(
m_iNumOutputChannels * m_framesPerBuffer * 2);
m_outputFifo = std::make_unique<FIFO<CSAMPLE>>(
m_iNumOutputChannels * framesPerBuffer * 2);
}
if (m_iNumInputChannels) {
m_inputFifo = std::make_unique<FIFO<CSAMPLE> >(
m_iNumInputChannels * m_framesPerBuffer * 2);
m_inputFifo = std::make_unique<FIFO<CSAMPLE>>(
m_iNumInputChannels * framesPerBuffer * 2);
}

m_pNetworkStream->startStream(m_dSampleRate);
Expand All @@ -89,15 +90,13 @@ SoundDeviceError SoundDeviceNetwork::open(bool isClkRefDevice, int syncBuffers)
// Update the samplerate and latency ControlObjects, which allow the
// waveform view to properly correct for the latency.
ControlObject::set(ConfigKey("[Master]", "latency"),
m_audioBufferTime.toDoubleMillis());
requestedBufferTime.toDoubleMillis());
ControlObject::set(ConfigKey("[Master]", "samplerate"), m_dSampleRate);
ControlObject::set(ConfigKey("[Master]", "audio_buffer_size"),
m_audioBufferTime.toDoubleMillis());
Copy link
Member

Choose a reason for hiding this comment

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

I guess it's highly unlikely that this CO is used somewhere in mappings or skins - and therefore can be removed.

Copy link
Member

Choose a reason for hiding this comment

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

I wanted to mention that it also needs to be removed from the Mixxx Controls section in the manual, but that CO does not seem to be documented anyway. 🤷


// Network stream was just started above so we have to wait until
// we can pass one chunk.
// The first callback runs early to do the one time setups
m_targetTime = m_audioBufferTime.toIntegerMicros();
m_targetTime = requestedBufferTime.toIntegerMicros();

m_pThread = std::make_unique<SoundDeviceNetworkThread>(this);
m_pThread->start(QThread::TimeCriticalPriority);
Expand Down Expand Up @@ -129,12 +128,12 @@ QString SoundDeviceNetwork::getError() const {
return QString();
}

void SoundDeviceNetwork::readProcess() {
void SoundDeviceNetwork::readProcess(SINT framesPerBuffer) {
if (!m_inputFifo || !m_pNetworkStream || !m_iNumInputChannels) {
return;
}

int inChunkSize = m_framesPerBuffer * m_iNumInputChannels;
int inChunkSize = framesPerBuffer * m_iNumInputChannels;
int readAvailable = m_pNetworkStream->getReadExpected()
* m_iNumInputChannels;
int writeAvailable = m_inputFifo->writeAvailable();
Expand Down Expand Up @@ -220,15 +219,15 @@ void SoundDeviceNetwork::readProcess() {
clearInputBuffer(inChunkSize - readCount, readCount);
}

m_pSoundManager->pushInputBuffers(m_audioInputs, m_framesPerBuffer);
m_pSoundManager->pushInputBuffers(m_audioInputs, framesPerBuffer);
}

void SoundDeviceNetwork::writeProcess() {
void SoundDeviceNetwork::writeProcess(SINT framesPerBuffer) {
if (!m_outputFifo || !m_pNetworkStream) {
return;
}

int outChunkSize = m_framesPerBuffer * m_iNumOutputChannels;
int outChunkSize = framesPerBuffer * m_iNumOutputChannels;
int writeAvailable = m_outputFifo->writeAvailable();
int writeCount = outChunkSize;
if (outChunkSize > writeAvailable) {
Expand Down Expand Up @@ -409,8 +408,12 @@ void SoundDeviceNetwork::workerWriteSilence(NetworkOutputStreamWorkerPtr pWorker
}

void SoundDeviceNetwork::callbackProcessClkRef() {
const SINT framesPerBuffer = m_configFramesPerBuffer;

// This must be the very first call, to measure an exact value
updateCallbackEntryToDacTime();
// NOTE: For network streams the buffer size is always the configured buffer
// size
updateCallbackEntryToDacTime(framesPerBuffer);

Trace trace("SoundDeviceNetwork::callbackProcessClkRef %1",
m_deviceId.name);
Expand Down Expand Up @@ -462,33 +465,34 @@ void SoundDeviceNetwork::callbackProcessClkRef() {
}
}

m_pSoundManager->readProcess();
m_pSoundManager->readProcess(framesPerBuffer);

{
ScopedTimer t("SoundDevicePortAudio::callbackProcess prepare %1",
m_deviceId.name);
m_pSoundManager->onDeviceOutputCallback(m_framesPerBuffer);
m_pSoundManager->onDeviceOutputCallback(framesPerBuffer);
}

m_pSoundManager->writeProcess();
m_pSoundManager->writeProcess(framesPerBuffer);

m_pSoundManager->processUnderflowHappened();
m_pSoundManager->processUnderflowHappened(framesPerBuffer);

updateAudioLatencyUsage();
updateAudioLatencyUsage(framesPerBuffer);
}

void SoundDeviceNetwork::updateCallbackEntryToDacTime() {
void SoundDeviceNetwork::updateCallbackEntryToDacTime(SINT framesPerBuffer) {
m_clkRefTimer.start();
qint64 currentTime = m_pNetworkStream->getInputStreamTimeUs();
m_targetTime += m_audioBufferTime.toIntegerMicros();
// This deadline for the next buffer in microseconds since the Unix epoch
m_targetTime += static_cast<qint64>(framesPerBuffer / m_dSampleRate * 1000000);
double callbackEntrytoDacSecs = (m_targetTime - currentTime) / 1000000.0;
callbackEntrytoDacSecs = math_max(callbackEntrytoDacSecs, 0.0001);
Comment on lines +487 to 489
Copy link
Member

Choose a reason for hiding this comment

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

Didn't we have an abstraction for sample and frame related time calculations?

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 am not aware of something for this special case.

VisualPlayPosition::setCallbackEntryToDacSecs(callbackEntrytoDacSecs, m_clkRefTimer);
Copy link
Member

Choose a reason for hiding this comment

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

I understand that for rate syncing with the frequency of the network clock framesPerBuffer is enough. But to show the visual waveform, not only rate, but also latency is needed.
I don't understand this calculation. Shouldn't the waveform be in sync with the sound that the DJ hears (headphones or booth speakers) even if he's broadcasting a network stream?

Copy link
Member Author

Choose a reason for hiding this comment

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

In this case the latency of the main DAC is lost. It would be an extra feature to compensate this.

This calculation is only to compensate the jitter that happens due to the different cycles of the audio and video thread.

At least this is unrelated to this PR. Do we want to file a bug for that?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, please create a bug for this. It would be great, if you could add some pointers to this report, how a possible fix could look like.
I agree, that this is not related to this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Here it is: #11219

//qDebug() << callbackEntrytoDacSecs << timeSinceLastCbSecs;
}

void SoundDeviceNetwork::updateAudioLatencyUsage() {
m_framesSinceAudioLatencyUsageUpdate += m_framesPerBuffer;
void SoundDeviceNetwork::updateAudioLatencyUsage(SINT framesPerBuffer) {
m_framesSinceAudioLatencyUsageUpdate += framesPerBuffer;
if (m_framesSinceAudioLatencyUsageUpdate
> (m_dSampleRate / CPU_USAGE_UPDATE_RATE)) {
double secInAudioCb = m_timeInAudioCallback.toDoubleSeconds();
Expand Down
12 changes: 7 additions & 5 deletions src/soundio/sounddevicenetwork.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,21 @@ class SoundDeviceNetwork : public SoundDevice {
SoundDeviceError open(bool isClkRefDevice, int syncBuffers) override;
bool isOpen() const override;
SoundDeviceError close() override;
void readProcess() override;
void writeProcess() override;
void readProcess(SINT framesPerBuffer) override;
void writeProcess(SINT framesPerBuffer) override;
QString getError() const override;

unsigned int getDefaultSampleRate() const override {
return 44100;
}

// NOTE: This does not take a frames per buffer argument because that is
// always equal to the configured buffer size for network streams
void callbackProcessClkRef();

private:
void updateCallbackEntryToDacTime();
void updateAudioLatencyUsage();
void updateCallbackEntryToDacTime(SINT framesPerBuffer);
void updateAudioLatencyUsage(SINT framesPerBuffer);

void workerWriteProcess(NetworkOutputStreamWorkerPtr pWorker,
int outChunkSize, int readAvailable,
Expand All @@ -61,10 +63,10 @@ class SoundDeviceNetwork : public SoundDevice {

std::unique_ptr<ControlProxy> m_pMasterAudioLatencyUsage;
mixxx::Duration m_timeInAudioCallback;
mixxx::Duration m_audioBufferTime;
int m_framesSinceAudioLatencyUsageUpdate;
std::unique_ptr<SoundDeviceNetworkThread> m_pThread;
bool m_denormals;
/// The deadline for the next buffer, in microseconds since the Unix epoch.
qint64 m_targetTime;
PerformanceTimer m_clkRefTimer;
};
Expand Down
8 changes: 5 additions & 3 deletions src/soundio/sounddevicenotfound.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@ class SoundDeviceNotFound : public SoundDevice {
SoundDeviceError close() override {
return SOUNDDEVICE_ERROR_ERR;
};
void readProcess() override { };
void writeProcess() override { };
QString getError() const override{ return QObject::tr("Device not found"); };
void readProcess(SINT /*framesPerbuffer*/) override{};
void writeProcess(SINT /*framesPerbuffer*/) override{};
QString getError() const override {
return QObject::tr("Device not found");
};

unsigned int getDefaultSampleRate() const override {
return 44100;
Expand Down
Loading