QAVAudioOutput has crackling audio when there are other CPU intensive tasks running on the PC

[geminixdev]

Still on Windows, still with Qt 6.6.x, after zero copy rendering works so well now, here is the next area which possibly can get optimized:

I notice that QAVAudioOutput has crackling, interrupted audio when there are other CPU intensive tasks running on the PC. This could be a compiler running, or simply the usual windows update and telemetry processes. Or wiggling windows on the screen. This is less obvious on powerful PCs, but easy to reproduce on weak Celerons.

Looking into the code I see that in QAVAudioOutput the function doPlayAudio() runs as a loop. There is also QCoreApplication::processEvents(). Tesing it I could see that without QCoreApplication::processEvents() there is no audio output, and the loop has to run often and quickly, so that QCoreApplication::processEvents() is called often enough to get the audio playing uninterrupted.

The reason for the loop in doPlayAudio() seems to be that it is implemented not as a QThread, but as a QFuture, which would end when the function ends.

The loop also calls setChannelConfig, tryinit, and setvolume code every time. I don't see the need for that once audioOutput is initialized. Unnecessary load.

This 'busy' loop makes the code vulnerable to interruptions and slow downs due to busy CPUs.

Before I used QtAVPlayer my AudioOutput was like that, a QThread and no loop. I've done that with QtMM QAudioOutput and also with RTAudio, so I know that it works and that the loop is not needed.

My proposal is to replace the QFuture here with a regular QThread and avoid that busy loop. It's enough to initialize the audioOutput, and readData will pull in the audio it wants, there is no need for a loop or anything, other than to refill the frames for readData.

[valbok]

Thanks for pointing this. QAVAudioOutput should definitely be fixed, yes.

It is been a while since it was implemented. But first version was very naive and

1. found out that the audio output should not be on gui thread, since if there is a hang in GUI, it crackles too, and audio impacts to GUI. And this is why QThread/QFuture invented + processEvents() to deliver events for QAudioOutput. (Need to revise if it is still relevant)
2. Each frame has its own audio format, might be changed suddenly -- need to check and recreate if has changed every time.
3. Setting Volume, Channel config, buffer size etc, should be applied ASAP, especially Volume.
4. QAVAudioOutput::play() should be not blocking

[valbok]

Also increasing the buffer size usually helps with crackling. When decoding is too slow and audio device is waiting for more data.

[valbok]

btw https://github.com/valbok/QtAVPlayer/blob/master/src/QtAVPlayer/qavaudiooutput.cpp#L196

technically it is not busy loop, it is running only if there is a frame in a queue and waiting until new frame is arrived.

[geminixdev]

btw https://github.com/valbok/QtAVPlayer/blob/master/src/QtAVPlayer/qavaudiooutput.cpp#L196

technically it is not busy loop, it is running only if there is a frame in a queue and waiting until new frame is arrived.

yes, correct, which is about every 20 ms.

[geminixdev]

Also increasing the buffer size usually helps with crackling. When decoding is too slow and audio device is waiting for more data.

Yes. However decoding speed is not an issue anymore, it's fast enough. When testing it was obvious that it did show crackling even when there where enough frames available and waiting. It all hinted to slow/interrupted processing.

Interestingly video does not show that anymore since we have hardware decoding and also zero copy rendering.

[geminixdev]

Also increasing the buffer size usually helps with crackling. When decoding is too slow and audio device is waiting for more data.

Yes, I played with that too, and it could not eliminate crackling.

[geminixdev]

1. found out that the audio output should not be on gui thread, since if there is a hang in GUI, it crackles too, and audio impacts to GUI.

Absolutely, it needs to be on its own thread!

And this is why QThread/QFuture invented + processEvents() to deliver events for QAudioOutput. (Need to revise if it is still relevant)

My old solutions were always a QThread with its own event loop. That thread should never sleep. No need for anything running there,

• because the audio device calls readData whenever it needs it, and
• frames can be put into the frame queue for readdata whenever they arrive (Signal/Slot or mutex protected queue)

2. Each frame has its own audio format, might be changed suddenly -- need to check and recreate if has changed every time.

I never did such checks within playing a file or stream, but only at the start of a new file / stream. But possibly I was just lucky that my sources were controlled and uniform.

3. Setting Volume, Channel config, buffer size etc, should be applied ASAP, especially Volume.

Absolutely. A volume change can be sent to the thread with a signal and then applied directly, without a loop checking for it, but signal / slot.

4. QAVAudioOutput::play() should be not blocking

Yes!

play() puts the frames in the buffer / queue, from which readData takes it. it does not block.

And if there are no frames available? For me in my old solutions, I did just output silence (fill the buffer readData wants with zeros). So no waitcondition needed there.

Also I remember that not returning readData (waitcondition and no frames) fast can cause crackling, which returning silence avoids.

[geminixdev]

"Crackling" might not describe the audio correctly, it is more "stuttering".

[geminixdev]

Quick and dirty test, instead of QFuture with processEvents() now QThread with QEventloop, no more Waitconditions, no loop, setting audio directly when changed: it works fine, plays the audio.

Not yet stress tested, but it should be "lighter".

[valbok]

could you please share your test?

[valbok]

btw, maybe the magic happens when you return zeros and silence instead of waiting for more data, which avoids any noize.

[geminixdev]

qthread-audio.zip

Yes, please see the attached zip.

When you use the Qthread approach directly from a main method, then apparently a processEvents() call is needed, but it can be in the play() method (as long as the audio buffer size is not smaller than 1 audio frame), which gets called anyway. No extra loop needed.

When I use this approach in my App, which has a QMainWindow and in it another player class in its own QThread, and then the audio thread (as here in the zip in the main method) as a Qthread, then that processEvents() call is not needed.

Please note that this example just shows that it plays audio, it is not yet optimized! It is missing for example to change volume or others using signal/slot, or to manage the size of the frames queue the play() method is putting the audio frame in. (In my App it's modified, integrated, but that does not fit into the widget-video example).

Even reducing the buffer size and wiggling the window like crazy, I could not get audio to stutter with this code.

[valbok]

sorry for delay, I tried your example, and found some issues like need to have a way to stop QThread and at EOF, there is crackling. For now it is the biggest issue to know how to finish playing smoothly, Also QThread itself already contains QEventLoop, looks another one is not needed. QCoreApplication::processEvents(); should not be needed too.

just fyi.

[geminixdev]

Also QThread itself already contains QEventLoop, looks another one is not needed.

Thanks for pointing me to this, I did not realize that. Now I can eliminate some unnecessary code. Thanks !!!!

[valbok]

Also QThread itself already contains QEventLoop, looks another one is not needed.

Thanks for pointing me to this, I did not realize that. Now I can eliminate some unnecessary code. Thanks !!!!

also fyi, if you don't use direct connection for QThread::started, it looks like you will create QAudioOutput on decoding thread, from the player, and not in QThread you expected.

[geminixdev]

also fyi, if you don't use direct connection for QThread::started, it looks like you will create QAudioOutput on decoding thread, from the player, and not in QThread you expected.

hmmm, I don't understand what you mean by "if you don't use direct connection for QThread::started".

This is the relevant code I use in my App:

void MyPlayer::_init()
{
    // the audio thread
    threadAudio = new QThread;
    audioOutput = new QAVAudioOutput;
    audioOutput->moveToThread(threadAudio);
    connect(threadAudio, &QThread::started,  audioOutput, &QAVAudioOutput::start);
    connect(this, &MyPlayer::setVolume, audioOutput, &QAVAudioOutput::onSetVolume);
    ...
    threadAudio->start(); 

and I modified the QAVAudioOutput constructor to not create anything on the heap yet:

QAVAudioOutput::QAVAudioOutput(QIODevice *parent)
    : QIODevice(parent)
{
}

From my understanding, with this code all in QAvAudioOutput should be moved to the QThread, also whatever is created later in the QAvAudioOutput class on the heap.

Did I misunderstand something?

[valbok]

sorry for the delay, finally got working https://github.com/valbok/QtAVPlayer/pull/464 locally, tested on mac, win and lin.

1. Took your idea that QThread should be used
2. Since QThread has its own QEventLoop, it does not require processEvents

Now QAVAudioOutput creates QThread on ctor, and expects QAVAudioOutput::play to be called on demuxer thread:

QObject::connect(&p, &QAVPlayer::audioFrame, &p, [&audioOutput](const QAVAudioFrame &frame) { audioOutput.play(frame); }, Qt::DirectConnection);

This allows to render audio using QThread and to use demuxer thread to put frames to the queue.

3. Fixed also crackling, took your idea to not start the rendering when not enough frames in the queue.
4. Unfortunately kept the waiting if no frames, this allows to block audio thread and looks it helps with noise too.

Thank you for the help, but still testing this

[valbok]

From my understanding, with this code all in QAvAudioOutput should be moved to the QThread, also whatever is created > later in the QAvAudioOutput class on the heap.

1. Need to move QAudioOuput to dedicated thread with event loop to not impact to demuxing or GUI. All events should be delivered and handled there.
2. Unfortunately QAudioOuput uses QTimer which should be stopped on the same thread it was created. So you can't call just audioOutput->stop() where you want. Need to post an event and stop the audio output on QThread. And should be done before releasing by deleteLater()

As I understood from your example, you do processing the audio frames on demuxer thread from the player, and not on QThread you created. This is why it did not work without processEvents().

In #464 added strict impl:

1. Dedicated audio QThread -> handles events for QAudioOutput, and stopping/releasing.
2. Demuxer thread is used only to put the audio frames to the queue.

[valbok]

please reopen if you have any issues

[geminixdev]

Just to confirm:

the code which I use in my App, based on what I proposed here above, is meanwhile running in hundreds of installations at users flawlessly. So in principle the move of QAVAudioOutput/QAudioOutput to a QThread works as I hoped, no issues at all..

The main difference between my App and standard QtAVPlayer is that my App only plays streams, and not files, therefore I have not tested any special handling of playing files.