Fix the choppiness for the pulse impl when using a smaller buffer size like 512 samples

Fix the choppiness for the pulse impl when using a smaller buffer size like 512 samples.

The audio using pulse can be choppy when the buffer size is small. It is simply because we do not feed the data quick enough.
The patch check if the available space in the driver, if it is more than a packet, it will call FulfillWriteRequest immediately to fill the buffer.

TEST=enable pulse, using https://webrtc-demos.appspot.com/html/pc1.html to make a call, make sure the audio is smooth.

[no longer working on chromium, 2013-02-25 14:33:50]

Dale, could you please take a look at the quick fix?

Thanks
SX

[DaleCurtis, 2013-02-25 19:01:29]

https://codereview.chromium.org/12328084/diff/1/media/audio/pulse/pulse_outpu...
File media/audio/pulse/pulse_output.cc (right):

https://codereview.chromium.org/12328084/diff/1/media/audio/pulse/pulse_outpu...
media/audio/pulse/pulse_output.cc:292: size_t avialable_space =
pa_stream_writable_size(pa_stream_);
s/avialable_space/available_space/

https://codereview.chromium.org/12328084/diff/1/media/audio/pulse/pulse_outpu...
media/audio/pulse/pulse_output.cc:297:
FulfillWriteRequest(params_.GetBytesPerBuffer());
Hmm, won't this cause faster than normal playback?  Can you try this with some
flash and html5 videos?

[no longer working on chromium, 2013-02-25 20:03:19]

https://codereview.chromium.org/12328084/diff/1/media/audio/pulse/pulse_outpu...
File media/audio/pulse/pulse_output.cc (right):

https://codereview.chromium.org/12328084/diff/1/media/audio/pulse/pulse_outpu...
media/audio/pulse/pulse_output.cc:292: size_t avialable_space =
pa_stream_writable_size(pa_stream_);
On 2013/02/25 19:01:29, DaleCurtis wrote:
> s/avialable_space/available_space/

Sorry, I will change it to writable_space

https://codereview.chromium.org/12328084/diff/1/media/audio/pulse/pulse_outpu...
media/audio/pulse/pulse_output.cc:297:
FulfillWriteRequest(params_.GetBytesPerBuffer());
On 2013/02/25 19:01:29, DaleCurtis wrote:
> Hmm, won't this cause faster than normal playback?  Can you try this with some
> flash and html5 videos?

It should not cause fast playback, since we query the pulse server on the
writable space and write data to the driver, and the pulse is still playing
sound in its normal speed.

I have already tried some html5 sounds, webrtc and webaudio, they all sound
great (and much better for webrtc), and will do some more tests on flash
tomorrow.

[DaleCurtis, 2013-02-26 00:53:00]

In local testing this still caused glitching for me.  I was able to fix it with
this patch:

https://codereview.chromium.org/12328097/

However even that only works if when media is played back at the native sample
rate.  Otherwise glitching occurs.  This change essentially allows Pulse to
prebuffer as much as it needs and keep an internal buffer of arbitrary size. 
Yet still tells it to call us back with our requested buffer size for steady
state.

So, given that, I'd ask that you first plumb a way for us to get the native
output sample rate, so we can have it returned by the AudioUtil method for now. 
With that we can ensure Pulse runs with the correct native sample rate.

However, once we have that, we'll also need to listen for device changes so we
can recreate streams with the correct native sample rate when it changes.  But
we can handle that down the line.

[no longer working on chromium, 2013-02-26 13:31:20]

On 2013/02/26 00:53:00, DaleCurtis wrote:
> In local testing this still caused glitching for me.  I was able to fix it
with

The audio sounds great yesterday with the patch, but today I tried more, it was
glitchy. I am not sure if it is related to that I rebooted my OS yesterday
morning.

> this patch:
> 
> https://codereview.chromium.org/12328097/
> 
> However even that only works if when media is played back at the native sample
> rate.  Otherwise glitching occurs.  This change essentially allows Pulse to
> prebuffer as much as it needs and keep an internal buffer of arbitrary size. 
> Yet still tells it to call us back with our requested buffer size for steady
> state.
> 

Nice. Can you land it instead then?

> So, given that, I'd ask that you first plumb a way for us to get the native
> output sample rate, so we can have it returned by the AudioUtil method for
now. 
> With that we can ensure Pulse runs with the correct native sample rate.
> 

We can't do it with the current static AudioUtil methods but have to add member
function to AudioManager because
#1 we need to dynamically check if pulse is available, this is done in
AudioManagerPulse.
#2 in order to query the sample rate for pulse, we need a pa_context object to
do it. Ideally we should not create a new threaded_mainloop and pa_context for
each query, so we have to do it in AudioManagerPulse.

I can start making a CL to refactor AudioUtil methods to AudioManager
interfaces.

> However, once we have that, we'll also need to listen for device changes so we
> can recreate streams with the correct native sample rate when it changes.  But
> we can handle that down the line.

Does the sample rate of pulse changes when switching to different devices? I
thought we are using the native sample rate of pulse, which if configured in
pulse daemon.conf.

[no longer working on chromium, 2013-02-26 18:33:31]

Dale, I uploaded a CL to make the pulse to use native sample rate there,
https://codereview.chromium.org/12316131/, if we are able to land that CL and
your CL, we should be able to enable Pulse as the default IO.

Since this CL does not fix the audio problem, I am going to close it.