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

Conversation

daschuer
Copy link
Member

@daschuer daschuer commented May 2, 2020

Did this ever work? Now it finally does. :-)
The issue was, that the buffer used from the engine to the soundDeviceNetwork was not filled.

@daschuer
Copy link
Member Author

daschuer commented May 2, 2020

@@ -665,8 +669,7 @@ void EngineMaster::process(const int iBufferSize) {
// 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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, also update the comment, since now it is no longer valid

@daschuer
Copy link
Member Author

daschuer commented May 2, 2020

Done.

// passing samples to the network reads directly from m_pSidechainMix.
// If recording/broadcasting from a sound card input.
// Note: In case the broadcast/recording input is configured,
// SoundManager has copied the input buffer to m_pSidechainMix before.
Copy link
Contributor

@JosepMaJAZ JosepMaJAZ May 2, 2020

Choose a reason for hiding this comment

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

Seems the "If recording/broadcasting from a sound card input." line is not needed now.

Also, it might be confusing to say that SoundManager has copied the input buffer.
The copy happens on EngineSideChain::receiveBuffer, when called SoundManager::pushInputBuffers when called from SoundDeviceNetwork::readProcess when called from SoundManager::readProcess() when called from SoundDevicePortAudio::callbackProcessClkRef". So, maybe mention at least pushInputBuffers.

And if you want, you might also mention that "the network reads directly from m_pSideChainMix registering it with SoundDevice::addOutput()"

Copy link
Contributor

@JosepMaJAZ JosepMaJAZ left a comment

Choose a reason for hiding this comment

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

change the if to copy buffer on lines 566 and 603 too.

@@ -632,7 +634,8 @@ void EngineMaster::process(const int iBufferSize) {
SampleUtil::applyRampingGain(m_pMaster, m_masterGainOld,
master_gain, m_iBufferSize);
m_masterGainOld = master_gain;
if (!m_bExternalRecordBroadcastInputConnected) {
if (m_pEngineSideChain &&
!m_bExternalRecordBroadcastInputConnected) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I just realised that this if/copy is also done on line 566 and 603. You need to modify those too.

@daschuer
Copy link
Member Author

daschuer commented May 2, 2020

OK, done.

Copy link
Contributor

@JosepMaJAZ JosepMaJAZ left a comment

Choose a reason for hiding this comment

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

LGTM.

src/engine/enginemaster.cpp Outdated Show resolved Hide resolved
@daschuer
Copy link
Member Author

daschuer commented May 3, 2020

Done.

@@ -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

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

@daschuer
Copy link
Member Author

daschuer commented May 3, 2020

Done.

@Be-ing Be-ing added this to the 2.2.4 milestone May 7, 2020
@Be-ing
Copy link
Contributor

Be-ing commented May 7, 2020

What is the status of this? Ready for merge?

@daschuer
Copy link
Member Author

daschuer commented May 7, 2020

Yes.

@Be-ing Be-ing merged commit ae03e5e into mixxxdj:2.2 May 7, 2020
@daschuer daschuer deleted the lp1876222 branch September 26, 2021 17:43
This was referenced Aug 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants