Reland 10952007: Pass through small buffer sizes without FIFO on Linux

Pass through small buffer sizes without FIFO on Linux.

TBR=dalecurtis@chromium.org
BUG=150570
TEST=content_unittests, manual tests: WebRTC on linux works fine.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=157524

[no longer working on chromium, 2012-09-17 15:02:26]

Dale, please take a look at this CL, we need to fix it before the cut today.

[scherkus (not reviewing), 2012-09-17 16:27:22]

drive-by...

https://codereview.chromium.org/10910306/diff/3001/media/audio/audio_output_r...
File media/audio/audio_output_resampler.cc (right):

https://codereview.chromium.org/10910306/diff/3001/media/audio/audio_output_r...
media/audio/audio_output_resampler.cc:137: params_.frames_per_buffer() !=
output_params_.frames_per_buffer()) {
doesn't this essentially disable the FIFO when sample rates match?

[no longer working on chromium, 2012-09-17 21:56:28]

Dale, could you please review this?

http://codereview.chromium.org/10910306/diff/3001/media/audio/audio_output_re...
File media/audio/audio_output_resampler.cc (right):

http://codereview.chromium.org/10910306/diff/3001/media/audio/audio_output_re...
media/audio/audio_output_resampler.cc:137: params_.frames_per_buffer() !=
output_params_.frames_per_buffer()) {
On 2012/09/17 16:27:23, scherkus wrote:
> doesn't this essentially disable the FIFO when sample rates match?

True, we should not do this.

http://codereview.chromium.org/10910306/diff/6001/media/audio/audio_output_re...
File media/audio/audio_output_resampler.cc (right):

http://codereview.chromium.org/10910306/diff/6001/media/audio/audio_output_re...
media/audio/audio_output_resampler.cc:113: int out_buffer_size = in_buffer_s *
output_params_.sample_rate();
I will add + 0.5 to do the correct truncation here.

[DaleCurtis, 2012-09-17 23:54:45]

http://codereview.chromium.org/10910306/diff/6001/media/audio/audio_output_re...
File media/audio/audio_output_resampler.cc (right):

http://codereview.chromium.org/10910306/diff/6001/media/audio/audio_output_re...
media/audio/audio_output_resampler.cc:106: if (params_.frames_per_buffer() !=
output_params_.frames_per_buffer()) {
I don't understand what you're trying to do here. You seem to be trying to
autodetect if the selected buffer size matches some magic pattern (which WebRTC
is using). Then choosing another buffer size based on another magic computation.

Can you elaborate on what you're doing here? Chris and I had previously
discussed just passing through the input buffersize if it's < 2048.

[no longer working on chromium, 2012-09-18 09:37:30]

http://codereview.chromium.org/10910306/diff/6001/media/audio/audio_output_re...
File media/audio/audio_output_resampler.cc (right):

http://codereview.chromium.org/10910306/diff/6001/media/audio/audio_output_re...
media/audio/audio_output_resampler.cc:106: if (params_.frames_per_buffer() !=
output_params_.frames_per_buffer()) {
On 2012/09/17 23:54:45, DaleCurtis wrote:
> I don't understand what you're trying to do here. You seem to be trying to
> autodetect if the selected buffer size matches some magic pattern (which
WebRTC
> is using). Then choosing another buffer size based on another magic
computation.
> 
> Can you elaborate on what you're doing here? Chris and I had previously
> discussed just passing through the input buffersize if it's < 2048.

The idea is simple. Basically, we should allow the clients to set the buffer
size they want, but in the unit of ms.
For example, if the clients want to use 882 samples for sample rate 22050, which
is 40ms data. But the OS native sample rate is 44100, then we should set the
output_params_ buffer size to 1764 (40ms data). By doing this, we don't need to
force the clients to use a buffer size that does not fit their needs.

So that this resampler will only kick in when the clients sample rate is
different from the native sample rate.

This is not purely for webrtc, other clients like PPAPI, preferably they should
use a big buffer size(like html5 does) to achieve low complexity and more
reliable performance.

And I think I have already mentioned it in another thread,
GetAudioHardwareBufferSize() is mainly for webaudio, it is not a generic
"preferred" buffer size for all the clients.

Passing through the input buffersize if it's < 2048 is a workaround to this
webrtc linux issue, it is OK for me if we don't have other better solution for
now, but it does not resolve all the problems.

[DaleCurtis, 2012-09-18 15:13:28]

+crogers who can comment on the audio specifics more than I can.

My concern is this only works for WebRTC which asks for buffer sizes like this.
WebAudio will always set via the audio until method and Flash will use 2048
(maybe 1024 in the future). To be specific:

One, WebAudio will hit the FIFO now. Two, Windows seems picky about what buffer
size you use. Three, on Mac the hw buffer size must be < 2048 samples, it seems
this equation could go over that in some circumstances.

What do you think?

[no longer working on chromium, 2012-09-18 15:51:41]

On 2012/09/18 15:13:28, DaleCurtis wrote:
> +crogers who can comment on the audio specifics more than I can.
> 
> My concern is this only works for WebRTC which asks for buffer sizes like
this.
> WebAudio will always set via the audio until method and Flash will use 2048
> (maybe 1024 in the future). To be specific:
> 
> One, WebAudio will hit the FIFO now. Two, Windows seems picky about what
buffer
> size you use. Three, on Mac the hw buffer size must be < 2048 samples, it
seems
> this equation could go over that in some circumstances.
> 
> What do you think?

One. Good point, I missed some part of code on the rebase. Webaudio should never
hit FIFO, this can be resolved by skipping checking the buffer size when
creating the FIFO, simply we should only use the resampler when the sample rates
are not matched.

Two. Really? This is not what I know, I think windows is picky on sample rate,
but not buffer size, could you please tell me which number of buffer size it
does not support?

Three. That is exactly the whole point, each client has their own optimal buffer
size, we should not use the lowest buffer size until we are using a mixer to
reduce the number of streams.

[Chris Rogers, 2012-09-18 17:27:02]

On 2012/09/18 15:51:41, xians1 wrote:
> On 2012/09/18 15:13:28, DaleCurtis wrote:
> > +crogers who can comment on the audio specifics more than I can.
> > 
> > My concern is this only works for WebRTC which asks for buffer sizes like
> this.
> > WebAudio will always set via the audio until method and Flash will use 2048
> > (maybe 1024 in the future). To be specific:
> > 
> > One, WebAudio will hit the FIFO now. Two, Windows seems picky about what
> buffer
> > size you use. Three, on Mac the hw buffer size must be < 2048 samples, it
> seems
> > this equation could go over that in some circumstances.
> > 
> > What do you think?
> 
> One. Good point, I missed some part of code on the rebase. Webaudio should
never
> hit FIFO, this can be resolved by skipping checking the buffer size when
> creating the FIFO, simply we should only use the resampler when the sample
rates
> are not matched.
> 
> Two. Really? This is not what I know, I think windows is picky on sample rate,
> but not buffer size, could you please tell me which number of buffer size it
> does not support?

From what Henrik has indicated, WASAPI strongly prefers the recommended size.


> 
> Three. That is exactly the whole point, each client has their own optimal
buffer
> size, we should not use the lowest buffer size until we are using a mixer to
> reduce the number of streams.

[Chris Rogers, 2012-09-18 17:36:29]

http://codereview.chromium.org/10910306/diff/14001/media/audio/audio_output_r...
File media/audio/audio_output_resampler.cc (right):

http://codereview.chromium.org/10910306/diff/14001/media/audio/audio_output_r...
media/audio/audio_output_resampler.cc:215: }
Why do we need special logic here when we have
AudioManagerBase::GetPreferredLowLatencyOutputStreamParameters().  Can't this
method be made to do the right thing for all platforms?