-
Notifications
You must be signed in to change notification settings - Fork 60
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
QAVAudioOutput has crackling audio when there are other CPU intensive tasks running on the PC #460
Comments
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
|
Also increasing the buffer size usually helps with crackling. When decoding is too slow and audio device is waiting for more data. |
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. |
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. |
Yes, I played with that too, and it could not eliminate crackling. |
Absolutely, it needs to be on its own thread!
My old solutions were always a QThread with its own event loop. That thread should never sleep.
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.
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.
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. |
"Crackling" might not describe the audio correctly, it is more "stuttering". |
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". |
could you please share your test? |
btw, maybe the magic happens when you return zeros and silence instead of waiting for more data, which avoids any noize. |
Yes, please see the attached zip. When you use the Qthread approach directly from a main method, then apparently a When I use this approach in my App, which has a Please note that this example just shows that it plays audio, it is not yet optimized! Even reducing the buffer size and wiggling the window like crazy, I could not get audio to stutter with this code. |
sorry for delay, I tried your example, and found some issues just fyi. |
Thanks for pointing me to this, I did not realize that. |
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:
and I modified the QAVAudioOutput constructor to not create anything on the heap yet:
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? |
sorry for the delay, finally got working #464 locally, tested on mac, win and lin.
Now QAVAudioOutput creates QThread on ctor, and expects QAVAudioOutput::play to be called on demuxer thread:
This allows to render audio using QThread and to use demuxer thread to put frames to the queue.
Thank you for the help, but still testing this |
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:
|
please reopen if you have any issues |
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. |
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 alsoQCoreApplication::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 thatQCoreApplication::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 regularQThread
and avoid that busy loop. It's enough to initialize the audioOutput, andreadData
will pull in the audio it wants, there is no need for a loop or anything, other than to refill the frames forreadData
.The text was updated successfully, but these errors were encountered: