Flip AudioOutputResampler to on by default.

Flip AudioOutputResampler to on by default.

BUG=147572
TEST=none

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

[DaleCurtis, 2012-09-12 15:16:04]

Preemptive CL for when we want to turn this on by default. I'll be OOO tomorrow
(paris time, so that means OOO today/tomorrow mtv hours), so putting this up in
case pepper team wants to experiment. Feel free to CQ/land etc.

[scherkus (not reviewing), 2012-09-13 12:43:09]

lgtm code-wise w/ q

still need to verify remaining issues are indeed non-issues prior to landing

https://chromiumcodereview.appspot.com/10928147/diff/2001/media/audio/audio_m...
File media/audio/audio_manager_base.cc (right):

https://chromiumcodereview.appspot.com/10928147/diff/2001/media/audio/audio_m...
media/audio/audio_manager_base.cc:161: params.format() ==
AudioParameters::AUDIO_PCM_LOW_LATENCY) {
to confirm -- this will affect web audio + web rtc as well, right?

[brettw, 2012-09-13 19:42:48]

lgtm

[DaleCurtis, 2012-09-14 09:04:44]

https://chromiumcodereview.appspot.com/10928147/diff/2001/media/audio/audio_m...
File media/audio/audio_manager_base.cc (right):

https://chromiumcodereview.appspot.com/10928147/diff/2001/media/audio/audio_m...
media/audio/audio_manager_base.cc:161: params.format() ==
AudioParameters::AUDIO_PCM_LOW_LATENCY) {
On 2012/09/13 12:43:09, scherkus wrote:
> to confirm -- this will affect web audio + web rtc as well, right?

Henrik says WebRTC will be okay since it uses the same buffer size in most
cases. We may hit the FIFO every now and then though, but so long as delay is
accurate and buffer sizes relatively small it should be okay. WebAudio uses the
same method for determining buffer size, so it won't be affected.

The automatic fallback though will certainly affect both of them, so they'll
have more users, but some of them may be grumpy about performance.

[commit-bot: I haz the power, 2012-09-14 12:48:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/10928147/6001

[DaleCurtis, 2012-09-14 12:54:23]

There are no outstanding issues that I'm aware of. All relevant CLs are in the
CQ and passing thus far.  Fingers crossed!

To future travelers: Feel free to revert this CL if you seeing any audio issues
with Flash, NaCl, WebAudio, or WebRTC.

[DaleCurtis, 2012-09-14 15:00:24]

+crogers, mac_rel failures are due to AUAudioOutputStream recieving a request
from the OS for a larger buffer than we've specified. I think we're not setting
this information correctly.

http://developer.apple.com/library/mac/#qa/qa1533/_index.html refers to setting
kAudioUnitProperty_MaximumFramesPerSlice , but I'm wary that if the OS is
ignoring our requested buffer size it's doing so for a reason.

The change I'm trybot'ing right now is to resize the AudioBus if a different
requested frame size comes in. The FIFO object will take care of this for us,
but I don't know if it's the right method. WDYT?

[DaleCurtis, 2012-09-14 15:45:50]

On 2012/09/14 15:00:24, DaleCurtis wrote:
> +crogers, mac_rel failures are due to AUAudioOutputStream recieving a request
> from the OS for a larger buffer than we've specified. I think we're not
setting
> this information correctly.
> 
> http://developer.apple.com/library/mac/#qa/qa1533/_index.html refers to
setting
> kAudioUnitProperty_MaximumFramesPerSlice , but I'm wary that if the OS is
> ignoring our requested buffer size it's doing so for a reason.
> 
> The change I'm trybot'ing right now is to resize the AudioBus if a different
> requested frame size comes in. The FIFO object will take care of this for us,
> but I don't know if it's the right method. WDYT?

I've landed this change for now so we have something in. We may also want to
consider adding a AUAudioOutputStream::HardwareBufferSize() method which
retrieves the appropriate property instead of forcing 128. On my system the
hardware buffer size is 512.

[Chris Rogers, 2012-09-14 17:28:45]

Dale, what buffer-size is it failing to take into account?  There is an
upper-limit.

https://chromiumcodereview.appspot.com/10928147/diff/13008/media/audio/mac/au...
File media/audio/mac/audio_low_latency_output_mac.cc (right):

https://chromiumcodereview.appspot.com/10928147/diff/13008/media/audio/mac/au...
media/audio/mac/audio_low_latency_output_mac.cc:244: }
I'm not crazy about this change long-term, and I'd like to discuss alternatives.
 Can you please add a TODO to revisit for potential cleanup?

[DaleCurtis, 2012-09-14 18:03:56]

On 2012/09/14 17:28:45, Chris Rogers wrote:
> Dale, what buffer-size is it failing to take into account?  There is an
> upper-limit.

We are configuring the device for 128 frames and it was asking for 440. Since we
fill based on AudioBus frame count it needs to be consistent. If you expect it
to vary I can come up with a smart resizing algorithm for Audio Bus objects.
> 
>
https://chromiumcodereview.appspot.com/10928147/diff/13008/media/audio/mac/au...
> File media/audio/mac/audio_low_latency_output_mac.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10928147/diff/13008/media/audio/mac/au...
> media/audio/mac/audio_low_latency_output_mac.cc:244: }
> I'm not crazy about this change long-term, and I'd like to discuss
alternatives.
>  Can you please add a TODO to revisit for potential cleanup?

[Chris Rogers, 2012-09-14 18:09:49]

On Fri, Sep 14, 2012 at 11:03 AM, <dalecurtis@chromium.org> wrote:

> On 2012/09/14 17:28:45, Chris Rogers wrote:
>
>> Dale, what buffer-size is it failing to take into account?  There is an
>> upper-limit.
>>
>
> We are configuring the device for 128 frames and it was asking for 440.
> Since we
> fill based on AudioBus frame count it needs to be consistent. If you
> expect it
> to vary I can come up with a smart resizing algorithm for Audio Bus
> objects.


That doesn't sound right - we've always been using a buffer size of 128 in
AUAudioOutputStream (WebAudio for example) and it calls back with the right
size.  What's different in this case?


>
>
>
> https://chromiumcodereview.**appspot.com/10928147/diff/**
>
13008/media/audio/mac/audio_**low_latency_output_mac.cc<https://chromiumcodereview.appspot.com/10928147/diff/13008/media/audio/mac/audio_low_latency_output_mac.cc>
>
>> File media/audio/mac/audio_low_**latency_output_mac.cc (right):
>>
>
>
> https://chromiumcodereview.**appspot.com/10928147/diff/**
>
13008/media/audio/mac/audio_**low_latency_output_mac.cc#**newcode244<https://chromiumcodereview.appspot.com/10928147/diff/13008/media/audio/mac/audio_low_latency_output_mac.cc#newcode244>
>
>> media/audio/mac/audio_low_**latency_output_mac.cc:244: }
>> I'm not crazy about this change long-term, and I'd like to discuss
>>
> alternatives.
>
>>   Can you please add a TODO to revisit for potential cleanup?
>>
>
>
>
>
http://codereview.chromium.**org/10928147/<http://codereview.chromium.org/109...
>

[DaleCurtis, 2012-09-14 18:21:08]

On 2012/09/14 18:09:49, Chris Rogers wrote:
> On Fri, Sep 14, 2012 at 11:03 AM, <mailto:dalecurtis@chromium.org> wrote:
> 
> > On 2012/09/14 17:28:45, Chris Rogers wrote:
> >
> >> Dale, what buffer-size is it failing to take into account?  There is an
> >> upper-limit.
> >>
> >
> > We are configuring the device for 128 frames and it was asking for 440.
> > Since we
> > fill based on AudioBus frame count it needs to be consistent. If you
> > expect it
> > to vary I can come up with a smart resizing algorithm for Audio Bus
> > objects.
> 
> 
> That doesn't sound right - we've always been using a buffer size of 128 in
> AUAudioOutputStream (WebAudio for example) and it calls back with the right
> size.  What's different in this case?
> 

My guess was this is the first time we've used that buffer size in a unit test
running on a bot. WebRTC was previously using 440ish.

I'm definitely planning to debug more. I was unable to reproduce the issue on
laptop though. If you have some time can you try the Web RTC AGC test on your
macs and see if you can trigger the frame size mismatch.

I'll poke so more at it on Monday too.
> 
> >
> >
> >
> > https://chromiumcodereview.**appspot.com/10928147/diff/**
> >
>
13008/media/audio/mac/audio_**low_latency_output_mac.cc<https://chromiumcodereview.appspot.com/10928147/diff/13008/media/audio/mac/audio_low_latency_output_mac.cc>
> >
> >> File media/audio/mac/audio_low_**latency_output_mac.cc (right):
> >>
> >
> >
> > https://chromiumcodereview.**appspot.com/10928147/diff/**
> >
>
13008/media/audio/mac/audio_**low_latency_output_mac.cc#**newcode244<https://chromiumcodereview.appspot.com/10928147/diff/13008/media/audio/mac/audio_low_latency_output_mac.cc#newcode244>
> >
> >> media/audio/mac/audio_low_**latency_output_mac.cc:244: }
> >> I'm not crazy about this change long-term, and I'd like to discuss
> >>
> > alternatives.
> >
> >>   Can you please add a TODO to revisit for potential cleanup?
> >>
> >
> >
> >
> >
>
http://codereview.chromium.**org/10928147/%3Chttp://codereview.chromium.org/1...>
> >