diff mbox

[19/25] paaudio: fix playback glitches

Message ID 3763f025154490e0b1ec54937010ed6f5f594f07.1438884611.git.DirtY.iCE.hu@gmail.com
State New
Headers show

Commit Message

=?UTF-8?B?Wm9sdMOhbiBLxZF2w6Fnw7M=?= Aug. 6, 2015, 6:28 p.m. UTC
Pulseaudio normally assumes that when the server wants it, the client
can generate the audio samples and send it right away.  Unfortunately
this is not the case with QEMU -- it's up to the emulated system when
does it generate the samples.  Buffering the samples and sending them
from a background thread is just a workaround, that doesn't work too
well.  Instead enable pa's compatibility support and let pa worry about
the details.

Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
---
 audio/paaudio.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Marc-André Lureau Aug. 20, 2015, 10:08 p.m. UTC | #1
Hi

On Thu, Aug 6, 2015 at 8:28 PM, Kővágó, Zoltán <dirty.ice.hu@gmail.com> wrote:
> Pulseaudio normally assumes that when the server wants it, the client
> can generate the audio samples and send it right away.  Unfortunately
> this is not the case with QEMU -- it's up to the emulated system when
> does it generate the samples.  Buffering the samples and sending them
> from a background thread is just a workaround, that doesn't work too
> well.  Instead enable pa's compatibility support and let pa worry about
> the details.

Afaik, it's not a problem if the audio data is not available right
away (live sources): PulseAudio should deal with that. I think it will
simply silence and wait (but it could actually be smarter than that?)

PA_STREAM_EARLY_REQUESTS is going to make PA request data more
frequently (every minreq). I don't get why that would help here. It's
meant for bad behaving applications using usleep and such that aren't
fixable, not a solution for audio glitches.

If qemu produces too much data than what PA can consume, it should
drop the old data, and only play current audio (unfortunately, that
doesn't seem to be the case looking at audio_pcm_sw_write). If qemu
produces too little, it can still write whatever it has when it has
it.

Could you provide a way to reproduce the glitch you experience or
explain more clearly how this flag helps?


thanks

>
> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
> ---
>  audio/paaudio.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/audio/paaudio.c b/audio/paaudio.c
> index 2aee22f..3990a80 100644
> --- a/audio/paaudio.c
> +++ b/audio/paaudio.c
> @@ -510,10 +510,8 @@ static pa_stream *qpa_simple_new (
>
>      flags =
>          PA_STREAM_INTERPOLATE_TIMING
> -#ifdef PA_STREAM_ADJUST_LATENCY
> -        |PA_STREAM_ADJUST_LATENCY
> -#endif
> -        |PA_STREAM_AUTO_TIMING_UPDATE;
> +        |PA_STREAM_AUTO_TIMING_UPDATE
> +        |PA_STREAM_EARLY_REQUESTS;
>      if (dev) {
>          /* don't move the stream if the user specified a sink/source */
>          flags |= PA_STREAM_DONT_MOVE;
> --
> 2.4.5
>
>
=?UTF-8?B?Wm9sdMOhbiBLxZF2w6Fnw7M=?= Aug. 21, 2015, 1:18 p.m. UTC | #2
2015-08-21 00:08 keltezéssel, Marc-André Lureau írta:
> Hi
>
> On Thu, Aug 6, 2015 at 8:28 PM, Kővágó, Zoltán <dirty.ice.hu@gmail.com> wrote:
>> Pulseaudio normally assumes that when the server wants it, the client
>> can generate the audio samples and send it right away.  Unfortunately
>> this is not the case with QEMU -- it's up to the emulated system when
>> does it generate the samples.  Buffering the samples and sending them
>> from a background thread is just a workaround, that doesn't work too
>> well.  Instead enable pa's compatibility support and let pa worry about
>> the details.
>
> Afaik, it's not a problem if the audio data is not available right
> away (live sources): PulseAudio should deal with that. I think it will
> simply silence and wait (but it could actually be smarter than that?)

Yes, it will simply silence and wait, until more data arrives.  The 
problem is that thing happens, like 10 times each second.  (If I watch 
pacmd list-sink-inputs, I can see that status quickly varies between 
drained and running, while playing an audio file in the guest)

> PA_STREAM_EARLY_REQUESTS is going to make PA request data more
> frequently (every minreq). I don't get why that would help here. It's
> meant for bad behaving applications using usleep and such that aren't
> fixable, not a solution for audio glitches.

The problem is that normally pulseaudio sends requests for data very 
late -- assuming that the application will quickly generate the required 
samples and send them back.  The problem is that the audio subsystem 
only "runs" each 10 ms, so it may take up to 10ms until qemu sends back 
data after the request, which is apparently too much.  With 
PA_STREAM_EARLY_REQUEST pulseaudio will request data as soon as it has 
enough free space in it's buffer, and not wait until the last moment.

Maybe tweaking pa buffer settings would help too, I'm not sure.  And 
yes, qemu is pretty much a bad behaving application that, while not 
using usleep directly, only produces audio every 10ms (by default).

>
> If qemu produces too much data than what PA can consume, it should
> drop the old data, and only play current audio (unfortunately, that
> doesn't seem to be the case looking at audio_pcm_sw_write). If qemu
> produces too little, it can still write whatever it has when it has
> it.
>
> Could you provide a way to reproduce the glitch you experience or
> explain more clearly how this flag helps?

It happens for me with a pretty much a default pulseaudio configuration, 
even with the stable release.  For example day24 from last year qemu 
advent calendar with pulseaudio backend.  (Sometimes it fixes itself 
after some time, very rarely it work normally, but most cases I just get 
that constant buffer underflow.)

>
>
> thanks
>
>>
>> Signed-off-by: Kővágó, Zoltán <DirtY.iCE.hu@gmail.com>
>> ---
>>   audio/paaudio.c | 6 ++----
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/audio/paaudio.c b/audio/paaudio.c
>> index 2aee22f..3990a80 100644
>> --- a/audio/paaudio.c
>> +++ b/audio/paaudio.c
>> @@ -510,10 +510,8 @@ static pa_stream *qpa_simple_new (
>>
>>       flags =
>>           PA_STREAM_INTERPOLATE_TIMING
>> -#ifdef PA_STREAM_ADJUST_LATENCY
>> -        |PA_STREAM_ADJUST_LATENCY
>> -#endif
>> -        |PA_STREAM_AUTO_TIMING_UPDATE;
>> +        |PA_STREAM_AUTO_TIMING_UPDATE
>> +        |PA_STREAM_EARLY_REQUESTS;
>>       if (dev) {
>>           /* don't move the stream if the user specified a sink/source */
>>           flags |= PA_STREAM_DONT_MOVE;
>> --
>> 2.4.5
>>
>>
>
>
>
diff mbox

Patch

diff --git a/audio/paaudio.c b/audio/paaudio.c
index 2aee22f..3990a80 100644
--- a/audio/paaudio.c
+++ b/audio/paaudio.c
@@ -510,10 +510,8 @@  static pa_stream *qpa_simple_new (
 
     flags =
         PA_STREAM_INTERPOLATE_TIMING
-#ifdef PA_STREAM_ADJUST_LATENCY
-        |PA_STREAM_ADJUST_LATENCY
-#endif
-        |PA_STREAM_AUTO_TIMING_UPDATE;
+        |PA_STREAM_AUTO_TIMING_UPDATE
+        |PA_STREAM_EARLY_REQUESTS;
     if (dev) {
         /* don't move the stream if the user specified a sink/source */
         flags |= PA_STREAM_DONT_MOVE;