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

Fill sidechainMix from external sidechain input. This fixes lp1876222 #2743

Merged
merged 6 commits into from
May 7, 2020
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
1 change: 1 addition & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* Add controller mapping for Hercules DJControl Inpulse 300 #2465
* Add controller mapping for Denon MC7000 #2546
* Add controller mapping for Stanton DJC.4 #2607
* Fix broadcasting via broadcast/recording input lp:1876222 #2743
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
* Fix broadcasting via broadcast/recording input lp:1876222 #2743
* Fix broadcasting via Record/Broadcast input lp:1876222 #2743

to match the GUI in the Sound Hardware Preferences


==== 2.2.3 2019-11-24 ====
* Don't make users reconfigure sound hardware when it has not changed #2253
Expand Down
73 changes: 40 additions & 33 deletions src/engine/enginemaster.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,9 @@ EngineMaster::EngineMaster(UserSettingsPointer pConfig,
}

// Starts a thread for recording and broadcast
m_pEngineSideChain = bEnableSidechain ? new EngineSideChain(pConfig) : NULL;
m_pEngineSideChain =
bEnableSidechain ?
new EngineSideChain(pConfig, m_pSidechainMix) : nullptr;

// X-Fader Setup
m_pXFaderMode = new ControlPushButton(
Expand Down Expand Up @@ -562,7 +564,7 @@ void EngineMaster::process(const int iBufferSize) {
m_masterGainOld = master_gain;

// Record/broadcast signal is the same as the master output
if (!m_bExternalRecordBroadcastInputConnected) {
if (sidechainMixRequired()) {
SampleUtil::copy(m_pSidechainMix, m_pMaster, m_iBufferSize);
}
} else if (configuredMicMonitorMode == MicMonitorMode::MASTER_AND_BOOTH) {
Expand Down Expand Up @@ -599,7 +601,7 @@ void EngineMaster::process(const int iBufferSize) {
m_masterGainOld = master_gain;

// Record/broadcast signal is the same as the master output
if (!m_bExternalRecordBroadcastInputConnected) {
if (sidechainMixRequired()) {
SampleUtil::copy(m_pSidechainMix, m_pMaster, m_iBufferSize);
}
} else if (configuredMicMonitorMode == MicMonitorMode::DIRECT_MONITOR) {
Expand Down Expand Up @@ -632,41 +634,42 @@ void EngineMaster::process(const int iBufferSize) {
SampleUtil::applyRampingGain(m_pMaster, m_masterGainOld,
master_gain, m_iBufferSize);
m_masterGainOld = master_gain;
if (!m_bExternalRecordBroadcastInputConnected) {
if (sidechainMixRequired()) {
SampleUtil::copy(m_pSidechainMix, m_pMaster, m_iBufferSize);
}

// The talkover signal Mixxx receives is delayed by the round trip latency.
// There is an output latency between the time Mixxx processes the audio
// and the user hears it. So if the microphone user plays on beat with
// what they hear, they will be playing out of sync with the engine's
// processing by the output latency. Additionally, Mixxx gets input signals
// delayed by the input latency. By the time Mixxx receives the input signal,
// a full round trip through the signal chain has elapsed since Mixxx
// processed the output signal.
// Although Mixxx receives the input signal delayed, the user hears it mixed
// in hardware with the master & booth outputs without that
// latency, so to record/broadcast the same signal that is heard
// on the master & booth outputs, the master mix must be delayed before
// mixing the talkover signal for the record/broadcast mix.
// If not using microphone inputs or recording/broadcasting from
// a sound card input, skip unnecessary processing here.
if (m_pNumMicsConfigured->get() > 0
&& !m_bExternalRecordBroadcastInputConnected) {
// Copy the master mix to a separate buffer before delaying it
// to avoid delaying the master output.
m_pLatencyCompensationDelay->process(m_pSidechainMix, m_iBufferSize);
SampleUtil::add(m_pSidechainMix, m_pTalkover, m_iBufferSize);
if (m_pNumMicsConfigured->get() > 0) {
// The talkover signal Mixxx receives is delayed by the round trip latency.
// There is an output latency between the time Mixxx processes the audio
// and the user hears it. So if the microphone user plays on beat with
// what they hear, they will be playing out of sync with the engine's
// processing by the output latency. Additionally, Mixxx gets input signals
// delayed by the input latency. By the time Mixxx receives the input signal,
// a full round trip through the signal chain has elapsed since Mixxx
// processed the output signal.
// Although Mixxx receives the input signal delayed, the user hears it mixed
// in hardware with the master & booth outputs without that
// latency, so to record/broadcast the same signal that is heard
// on the master & booth outputs, the master mix must be delayed before
// mixing the talkover signal for the record/broadcast mix.
// If not using microphone inputs or recording/broadcasting from
// a sound card input, skip unnecessary processing here.

// Copy the master mix to a separate buffer before delaying it
// to avoid delaying the master output.
m_pLatencyCompensationDelay->process(m_pSidechainMix, m_iBufferSize);
SampleUtil::add(m_pSidechainMix, m_pTalkover, m_iBufferSize);
}
}
}

// Submit buffer to the side chain to do broadcasting, recording,
// etc. (CPU intensive non-realtime tasks)
// If recording/broadcasting from a sound card input,
// SoundManager will send the input buffer from the sound card to m_pSidechain
// so skip sending a buffer to m_pSidechain here.
if (!m_bExternalRecordBroadcastInputConnected
&& m_pEngineSideChain != nullptr) {
// Submit buffer to the side chain to do CPU intensive non-realtime
// tasks like recording. The SoundDeviceNetwork, responsible for
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
// tasks like recording. The SoundDeviceNetwork, responsible for
// tasks (recording and broadcasting). The SoundDeviceNetwork, responsible for

Copy link
Contributor

@JosepMaJAZ JosepMaJAZ May 3, 2020

Choose a reason for hiding this comment

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

Currently (i.e. with this PR), SideChain is not used for broadcasting, only the pSidechainMix

// passing samples to the network reads directly from m_pSidechainMix,
// registering it with SoundDevice::addOutput().
// Note: In case the broadcast/recording input is configured,
uklotzde marked this conversation as resolved.
Show resolved Hide resolved
// EngineSideChain::receiveBuffer has copied the input buffer to m_pSidechainMix
// via before (called by SoundManager::pushInputBuffers())
if (m_pEngineSideChain) {
m_pEngineSideChain->writeSamples(m_pSidechainMix, iFrames);
}

Expand Down Expand Up @@ -963,3 +966,7 @@ void EngineMaster::registerNonEngineChannelSoundIO(SoundManager* pSoundManager)
}
pSoundManager->registerOutput(AudioOutput(AudioOutput::RECORD_BROADCAST, 0, 2), this);
}

bool EngineMaster::sidechainMixRequired() const {
return m_pEngineSideChain && !m_bExternalRecordBroadcastInputConnected;
}
1 change: 1 addition & 0 deletions src/engine/enginemaster.h
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ class EngineMaster : public QObject, public AudioSource {
ChannelHandleFactory* m_pChannelHandleFactory;
void applyMasterEffects();
void processHeadphones(const double masterMixGainInHeadphones);
bool sidechainMixRequired() const;

EngineEffectsManager* m_pEngineEffectsManager;

Expand Down
14 changes: 10 additions & 4 deletions src/engine/sidechain/enginesidechain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <QMutexLocker>

#include "engine/sidechain/sidechainworker.h"
#include "engine/engine.h"
#include "util/counter.h"
#include "util/event.h"
#include "util/sample.h"
Expand All @@ -36,11 +37,14 @@

#define SIDECHAIN_BUFFER_SIZE 65536

EngineSideChain::EngineSideChain(UserSettingsPointer pConfig)
EngineSideChain::EngineSideChain(
UserSettingsPointer pConfig,
CSAMPLE* sidechainMix)
: m_pConfig(pConfig),
m_bStopThread(false),
m_sampleFifo(SIDECHAIN_BUFFER_SIZE),
m_pWorkBuffer(SampleUtil::alloc(SIDECHAIN_BUFFER_SIZE)) {
m_pWorkBuffer(SampleUtil::alloc(SIDECHAIN_BUFFER_SIZE)),
m_pSidechainMix(sidechainMix) {
// We use HighPriority to prevent starvation by lower-priority processes (Qt
// main thread, analysis, etc.). This used to be LowPriority but that is not
// a suitable choice since we do semi-realtime tasks
Expand Down Expand Up @@ -78,11 +82,13 @@ void EngineSideChain::addSideChainWorker(SideChainWorker* pWorker) {
void EngineSideChain::receiveBuffer(AudioInput input,
const CSAMPLE* pBuffer,
unsigned int iFrames) {
if (input.getType() != AudioInput::RECORD_BROADCAST) {
VERIFY_OR_DEBUG_ASSERT(input.getType() == AudioInput::RECORD_BROADCAST) {
qDebug() << "WARNING: AudioInput type is not RECORD_BROADCAST. Ignoring incoming buffer.";
return;
}
writeSamples(pBuffer, iFrames);
// Just copy the received samples form the sound card input to the
// engine. After processing we get it back via writeSamples()
SampleUtil::copy(m_pSidechainMix, pBuffer, iFrames * mixxx::kEngineChannelCount);
}

void EngineSideChain::writeSamples(const CSAMPLE* pBuffer, int iFrames) {
Expand Down
3 changes: 2 additions & 1 deletion src/engine/sidechain/enginesidechain.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
class EngineSideChain : public QThread, public AudioDestination {
Q_OBJECT
public:
EngineSideChain(UserSettingsPointer pConfig);
EngineSideChain(UserSettingsPointer pConfig, CSAMPLE* sidechainMix);
virtual ~EngineSideChain();

// Not thread-safe, wait-free. Submit buffer of samples to the sidechain for
Expand All @@ -58,6 +58,7 @@ class EngineSideChain : public QThread, public AudioDestination {

FIFO<CSAMPLE> m_sampleFifo;
CSAMPLE* m_pWorkBuffer;
CSAMPLE* m_pSidechainMix;

// Provides thread safety around the wait condition below.
QMutex m_waitLock;
Expand Down