Optional network clock reference #894

daschuer · 2016-02-16T21:08:12Z

This PR attempts to fix a lot of issues of users with multiplicands and broadcast set-ups.

During my test, it turns out that the soundcard clock is not passed as stable as expected up to the Mixxx engine.
RJ has reported a const drift, that is compensated by a double callback in core audio.
I am experience a double callback in ALSA double pause pair when I use a soundcard in duplex mode.

While these issues are harmless with a single soundcard, there is an overflow risk with the second soundcards or broadcasting.

This PR introduces a new preference option "Engine Clock" = "Network Clock" or "Soundcard Clock".
This allows to run the Mixxx engine on the Network Clock, which removes all syncing artefacts from the broadcast stream. This clock runs much more stable and suits to feet the soundcards with real-time priority as well.

I was also able fixes also lot of tiny issues in the syncing code. These where probably the cause of crackling in USB Mic setups reported in the forums.

.. And not at least it fixes the long standing "no transport" issue if no soundcard is configured.

Conflicts: src/soundio/sounddevicenetwork.cpp src/soundio/sounddevicenetwork.h src/soundio/soundmanager.cpp src/waveform/visualplayposition.cpp

daschuer · 2016-02-24T13:25:47Z

Since 2.0 we have no "steam is broken" reports, but instead "stream is crackling" reports.
This PR may solve some of these errors.
Who is able to check this out or review this code?
Thank you.

daschuer · 2016-03-20T20:06:17Z

Ping!

Conflicts: src/soundio/sounddeviceportaudio.cpp src/soundio/sounddeviceportaudio.h src/soundio/soundmanager.cpp

daschuer · 2016-06-24T14:45:18Z

This works nice again. Who has fun to review?

Conflicts: src/preferences/dialog/dlgprefsounddlg.ui src/soundio/sounddevicenetwork.cpp src/soundio/soundmanager.cpp src/soundio/soundmanager.h

Conflicts: src/preferences/dialog/dlgprefsounddlg.ui src/soundio/soundmanagerconfig.cpp

Be-ing · 2017-07-27T04:52:04Z

What are the use cases for this? Is it only helpful for broadcasting with USB microphones? Does there need to be a preference option? Could Mixxx automatically switch to the network clock if different sound cards are used for input and output?

Be-ing · 2017-11-19T03:19:11Z

src/preferences/dialog/dlgprefsound.cpp

+void DlgPrefSound::engineClockChanged(int index) {
+    if (index == 0) {
+        // "Soundcard Clock"
+        m_config.setForceNetworkClock(false);


Does this take effect only after the Apply button is pressed?

Yes, or the OK button.

Be-ing · 2017-11-19T03:19:57Z

src/soundio/sounddevicenetwork.cpp

-            m_inputFifo = new FIFO<CSAMPLE>(
-                    m_iNumInputChannels * m_framesPerBuffer * 2);
-        }
+    // Feet the network device buffer directly from the


"Feet"? Do you mean "Feed"?

Be-ing · 2017-11-19T03:22:09Z

src/soundio/sounddevicenetwork.cpp

+    if (isClkRefDevice) {
+        // Update the samplerate and latency ControlObjects, which allow the
+        // waveform view to properly correct for the latency.
+        ControlObject::set(ConfigKey("[Master]", "latency"),


Using ControlObjects for this feels like an ugly hack. Should there be a TODO to pass this information directly to the waveform renderers without requiring a ControlObject?

The code was adopted from here:

mixxx/src/soundio/sounddeviceportaudio.cpp

Line 369 in 2b26c46

ControlObject::set(ConfigKey("[Master]", "latency"), currentLatencyMSec);

And I do not think this is a bad solution, because this way, the controllers can use this info as well.
(Like LPD8RK)

Controller scripts should not need this information. The Akai LPD 8 script seems to use this to hack beatjumping in JS, but we have that in C++ now.

I can update the LPD 8 mapping to fix that. Thanks for the reminder about that controller.

Be-ing · 2017-11-19T03:26:15Z

src/preferences/dialog/dlgprefsounddlg.ui

+     <item row="4" column="1">
+      <widget class="QComboBox" name="engineClockComboBox">
+       <property name="toolTip">
+        <string>Use soundcard clock for low latency setups. Use network clock for broadcasting only setups.</string>


This should explain better in what situations this option might be useful. What is a "broadcasting only setup"? When there is no soundcard in use at all? If a user selects this option while broadcasting and monitoring the Master output on a soundcard, is there a chance of audible artifacts on the Master output?

Broadcasting is just the same as a second soundcard with an individual clock.
Normally, the master output is taken to drive the mixxx engine,which produces a bit perfect output on master, all other soundcards (hardware and broadcasting) suffer from stuffing or lost samples.
This is unbearable in case of a hardware soundcard, but can be heard in broadcasting, because the big buffer sizes.
If you use the Network clock from this PR master will suffer also from unhearable stuffing or lost samples, but Broadcasting is bit perfect then assuming that the receiver runs also on an NTP synced cock.
With broadcast only I mean, that the audience is listening to broadcast only. No one cares about the quality of the master output in this case.

Do you have a better tooltip in mind?
"Use the network clock if the primary output is broadcasting"

How about: "Use soundcard clock for live audience setups. Use network clock for broadcasting without a live audience." In the manual we can explain the reasoning.

An additional critical point is, that we need some extra latency for the syncing code.
On my system I suffer master crackling at <= 10 ms buffer size.

How about this:
"Use soundcard clock for live audience setups and lowest latency. Use network clock for broadcasting without a live audience."

Be-ing · 2017-11-19T03:27:13Z

src/soundio/sounddevicenetwork.cpp

-            m_underflowHappened = 1;
-        } else if (writeAvailable > readAvailable + outChunkSize / 2) {
-            // try to keep PAs buffer filled up to 0.5 chunks
+            // catch up by filling buffer until we are a halve buffer behind


s/halve/half/

Be-ing · 2017-11-19T03:31:22Z

src/soundio/sounddevicenetwork.cpp

+            qDebug() << "SSE: Denormals to zero mode is working";
+        }
+#else
+        qWarning() << "No SSE: No denormals to zero mode available. EQs and effects may suffer high CPU load";


We should explicitly document this in the manual. A user did recently run into an issue with this on an old computer.

Yes! I have filed an issue: mixxxdj/manual#62

Be-ing · 2017-11-19T03:36:45Z

src/soundio/soundmanager.h

@@ -111,6 +113,15 @@ class SoundManager : public QObject {
        return m_pNetworkStream;
    }

+    void underflowHappened(int code) {
+        m_underflowHappened = 1;
+        if (CmdlineArgs::Instance().getDeveloper()) {


Should we always write this to the log? It could help with user support if we could see that in the log.

This would make the situation worse, because we are here in the engine code and a Logging is locking code. A small unhearable glitch can become hearable because of that.
I will drop a comment.

Be-ing · 2017-11-19T03:38:26Z

I have glanced through the code and left some general comments, but I am not familiar enough with the technical background of the issue or how the SoundDevices work to comment much on the technical details.

daschuer · 2017-11-19T22:13:41Z

Done.
The good thing with this PR is that it is optional.
If we discover bugs after Mixxx is release, the user can just switch back to the souncards clock

Be-ing · 2017-11-19T23:12:10Z

That is a good point. In that case I'm okay with merging it as-is, unless @Palakis would like to review it first.

Palakis · 2017-11-20T11:37:28Z

@Be-ing Looking at the code, no comments to make so far. One thing though: I believe this should be merged before #1300, as SoundDeviceNetwork was modified here and there.

daschuer · 2017-11-20T15:33:56Z

Thank you for reviewing. :-)

uklotzde · 2017-12-14T06:47:12Z

@daschuer These changes might have caused a regression:
https://bugs.launchpad.net/mixxx/+bug/1738028

I didn't have time for more testing and in depth analysis. Any ideas?

daschuer · 2017-12-14T10:25:32Z

Ups, that is odd. I hope I find time tonight.
As a band aid we have to revert this commit if we have no solution in time.

Do you have any more info, why do you think this one is the cause? That may help. Thanks.

uklotzde · 2017-12-14T17:25:49Z

@daschuer A first retest with this branch didn't reveal any negative impact.

We have another serious issue that I need to analyze:
https://bugs.launchpad.net/mixxx/+bug/1738227

Hope I will be able to find and fix the cause soon :/

daschuer · 2017-12-14T19:55:54Z

I am trying to finish the #1254 review and than I will also dig into this.

daschuer · 2017-12-14T23:39:32Z

I can reproduce the stuttering issues loading a track using the network clock reference, and low latencies (11 ms) in my case.
I consider this as just too short fort the internal clock. Probably m_pThread->start(QThread::TimeCriticalPriority); i

The soundcard clock runs with no issues here.

Lets see if we can improve the situation.

uklotzde · 2017-12-16T12:39:47Z

@daschuer By doing a git bisect I was able to locate the cause of the buffer underflows in your networkclockref branch.

Unfortunately the code does not compile or Mixxx crashes with this commit range, so I was not able to identify a single commit.

daschuer · 2017-12-16T20:01:29Z

Ok, good news. What was your setup?
Was it with --developer flag? What was the final effected commit range?

daschuer · 2017-12-17T11:24:41Z

I cannot confirm this. :-/
My Setup:

Ubuntu Trusty, ALSA
internal Souncard: Master
USB Soundcard: Headpones
48000 kHz 5,33 ms
Multi Souncard = Default

I have compared current master 24028f2
with the commit just before merging this f76d5c7

I played a track in deck 1 and analyzed a track in deck 2.
No overflows in both version.
I only notice a cpu usage peak in the old version when analysis starts.

What is your test case?

daschuer · 2017-12-17T11:37:37Z

I have also found a segfault with invalid SoundDevice pointers.
I am working on a share pointer solution

daschuer added 19 commits December 16, 2015 23:19

Allow SounddeviceNetwork to be clock reference device.

746c194

force networkdevice as clock reference

a72cba4

Merge remote-tracking branch 'upstream/master' into networkclockref

56a8166

Conflicts: src/soundio/sounddevicenetwork.cpp src/soundio/sounddevicenetwork.h src/soundio/soundmanager.cpp src/waveform/visualplayposition.cpp

use mixxx::Duration

d9c4170

Latency usage calculation fixed

a117ec9

set master timing COs

2ad8a24

improve andling of target time

035abf8

stop clock ref thread

29d1e2d

Move underflow monitoring to SoundManager

f4ad419

prevent m_outputFifo overflow

f99b605

m_pSoundManager->underflowHappened() with debug code to identify callers

251dc46

Ensure that m_inputFifo is filled with a half chunk extra.

d2ca9c4

Tweak buffer settings for start up

75bdf20

UnderflowHappened Warning only in developer mode

1c3d250

Add preferences option to force using network clock

46f18f1

respect new force network clock option in sound manager

5cadcc7

connect settingsChanged slot

2bd0110

Init input fifo to with one chunk

c00c2be

Improve use >= to detect write underflows

b52560c

daschuer mentioned this pull request Mar 1, 2016

MIDI Clock Input #702

Closed

Merge branch 'master' into networkclockref

42683a1

Conflicts: src/soundio/sounddeviceportaudio.cpp src/soundio/sounddeviceportaudio.h src/soundio/soundmanager.cpp

daschuer added 5 commits February 13, 2017 21:32

Merge remote-tracking branch 'upstream/master' into networkclockref

34f6a0f

Conflicts: src/preferences/dialog/dlgprefsounddlg.ui src/soundio/sounddevicenetwork.cpp src/soundio/soundmanager.cpp src/soundio/soundmanager.h

Use std:unique_ptr

077d68c

Use SINT as sample index in SoundDevice

5588ac7

Fix segfault due to bug in drift correction

d2ad4db

Merge remote-tracking branch 'upstream/master' into networkclockref

3f3a28c

Conflicts: src/preferences/dialog/dlgprefsounddlg.ui src/soundio/soundmanagerconfig.cpp

Be-ing reviewed Nov 19, 2017

View reviewed changes

daschuer added 2 commits November 19, 2017 22:35

fix typos

ee6b3bd

Comment why engine warnings are disabled

9772814

Improved engine clock tooltip

f125231

daschuer merged commit f76d5c7 into mixxxdj:master Nov 20, 2017

daschuer deleted the networkclockref branch September 7, 2021 21:06

This was referenced Aug 22, 2022

Songs won't load in the players -- warn when sound devices are not working #6101

Closed

Sound artifacts on broadcasting #8421

Closed

audit syscalls made from the PortAudio callback #8719

Open

Master out audio stutters during drag & drop #9004

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optional network clock reference #894

Optional network clock reference #894

daschuer commented Feb 16, 2016

daschuer commented Feb 24, 2016

daschuer commented Mar 20, 2016

daschuer commented Jun 24, 2016

Be-ing commented Jul 27, 2017

Be-ing Nov 19, 2017

daschuer Nov 19, 2017

Be-ing Nov 19, 2017

Be-ing Nov 19, 2017

daschuer Nov 19, 2017

Be-ing Nov 19, 2017

Be-ing Nov 19, 2017

Be-ing Nov 19, 2017

daschuer Nov 19, 2017

Be-ing Nov 19, 2017

daschuer Nov 19, 2017

Be-ing Nov 19, 2017

Be-ing Nov 19, 2017

daschuer Nov 19, 2017

Be-ing Nov 19, 2017

daschuer Nov 19, 2017

Be-ing commented Nov 19, 2017

daschuer commented Nov 19, 2017

Be-ing commented Nov 19, 2017

Palakis commented Nov 20, 2017 •

edited

Loading

daschuer commented Nov 20, 2017

uklotzde commented Dec 14, 2017

daschuer commented Dec 14, 2017

uklotzde commented Dec 14, 2017

daschuer commented Dec 14, 2017

daschuer commented Dec 14, 2017

uklotzde commented Dec 16, 2017 •

edited

Loading

daschuer commented Dec 16, 2017

daschuer commented Dec 17, 2017

daschuer commented Dec 17, 2017

Optional network clock reference #894

Optional network clock reference #894

Conversation

daschuer commented Feb 16, 2016

daschuer commented Feb 24, 2016

daschuer commented Mar 20, 2016

daschuer commented Jun 24, 2016

Be-ing commented Jul 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Be-ing commented Nov 19, 2017

daschuer commented Nov 19, 2017

Be-ing commented Nov 19, 2017

Palakis commented Nov 20, 2017 • edited Loading

daschuer commented Nov 20, 2017

uklotzde commented Dec 14, 2017

daschuer commented Dec 14, 2017

uklotzde commented Dec 14, 2017

daschuer commented Dec 14, 2017

daschuer commented Dec 14, 2017

uklotzde commented Dec 16, 2017 • edited Loading

daschuer commented Dec 16, 2017

daschuer commented Dec 17, 2017

daschuer commented Dec 17, 2017

Palakis commented Nov 20, 2017 •

edited

Loading

uklotzde commented Dec 16, 2017 •

edited

Loading