Bypass audio resampler on XP / non-WASAPI wave out path.

Bypass audio resampler on XP / non-WASAPI wave out path.

The non-WASAPI path (WaveOut) will automatically handle resampling and bits
per sample differences, so skip invoking AudioOutputResampler's resampler and
bits per sample adjustment code.

BUG=148418
TEST=windows build / henrika
TBR=scherkus

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

[DaleCurtis, 2012-09-14 09:28:55]

Compiles and runs on my remote Windows box. Henrik, let me know if this works
out for you.

[scherkus (not reviewing), 2012-09-14 09:45:32]

chatted offline -- this is pretty subtle

can you beef up w/ comments?

[henrika (OOO until Aug 14), 2012-09-14 09:46:46]

One quick initial comment before testing.

https://codereview.chromium.org/10913267/diff/3001/media/audio/win/audio_mana...
File media/audio/win/audio_manager_win.cc (right):

https://codereview.chromium.org/10913267/diff/3001/media/audio/win/audio_mana...
media/audio/win/audio_manager_win.cc:325: int bits_per_sample =
input_params.bits_per_sample();
Do we want this for XP?

[DaleCurtis, 2012-09-14 09:59:05]

https://codereview.chromium.org/10913267/diff/3001/media/audio/win/audio_mana...
File media/audio/win/audio_manager_win.cc (right):

https://codereview.chromium.org/10913267/diff/3001/media/audio/win/audio_mana...
media/audio/win/audio_manager_win.cc:325: int bits_per_sample =
input_params.bits_per_sample();
On 2012/09/14 09:46:46, henrika wrote:
> Do we want this for XP?

It's the existing behavior, so I think so.

[henrika (OOO until Aug 14), 2012-09-14 10:19:20]

It is a bit tricky to follow the chain and see where the actual fallback "kicks
in". Can you add some more comments please.

Initial test on Win 7 where I forced WASAPIIsSupported to return false =>

[11376:1088:0914/120712:VERBOSE1:audio_manager_win.cc(323)]
AudioManagerWin::GetPreferredLowLatencyOutputStreamParameters()
[11376:1088:0914/120712:VERBOSE1:audio_manager_win.cc(324)]
input_params.sample_rate(): 44100
[11376:1088:0914/120712:VERBOSE1:audio_manager_win.cc(325)]
input_params.bits_per_sample(): 16
[11376:1088:0914/120712:VERBOSE1:audio_manager_win.cc(261)] Using WaveOut since
WASAPI requires at least Vista.
[11376:1088:0914/120712:VERBOSE1:waveout_output_win.cc(113)] nSamplesPerSec
44100
[11376:1088:0914/120712:VERBOSE1:waveout_output_win.cc(114)] frames_per_buffer
2048

and AudioOutputResampler::Init is never called.

And the audio is good for several different sample rates on my audio device.

[henrika (OOO until Aug 14), 2012-09-14 10:22:54]

Just for the record; using proper IsWASAPISupported => 

[15680:12932:0914/122026:VERBOSE1:audio_manager_win.cc(323)]
AudioManagerWin::GetPreferredLowLatencyOutputStreamParameters()
[15680:12932:0914/122026:VERBOSE1:audio_manager_win.cc(324)]
input_params.sample_rate(): 44100
[15680:12932:0914/122026:VERBOSE1:audio_manager_win.cc(325)]
input_params.bits_per_sample(): 16

+

[15680:12932:0914/122026:VERBOSE1:audio_output_resampler.cc(101)] Resampling
from 44100 to 48000
[15680:12932:0914/122026:VERBOSE1:audio_output_resampler.cc(125)] Rebuffering
from 2048 to 480
[15680:12932:0914/122026:VERBOSE1:audio_output_resampler.cc(133)] I/O ratio is
0.91875

using the same settings.

[henrika (OOO until Aug 14), 2012-09-14 10:42:42]

My bad.

Extended logging in AOR =>

[16988:9144:0914/124043:VERBOSE1:audio_manager_win.cc(323)]
AudioManagerWin::GetPreferredLowLatencyOutputStreamParameters()
[16988:9144:0914/124043:VERBOSE1:audio_manager_win.cc(324)]
input_params.sample_rate(): 44100
[16988:9144:0914/124043:VERBOSE1:audio_manager_win.cc(325)]
input_params.bits_per_sample(): 16
*[16988:9144:0914/124043:VERBOSE1:audio_output_resampler.cc(91)] I/O ratio
is 1*
[16988:9144:0914/124043:VERBOSE1:audio_manager_win.cc(261)] Using WaveOut
since WASAPI requires at least Vista.
[16988:9144:0914/124043:VERBOSE1:waveout_output_win.cc(113)] nSamplesPerSec
44100
[16988:9144:0914/124043:VERBOSE1:waveout_output_win.cc(114)]
frames_per_buffer 2048


On Fri, Sep 14, 2012 at 12:22 PM, <henrika@chromium.org> wrote:

> Just for the record; using proper IsWASAPISupported =>
>
> [15680:12932:0914/122026:VERBOSE1:audio_**manager_win.cc(323)]
> AudioManagerWin::**GetPreferredLowLatencyOutputSt**reamParameters()
> [15680:12932:0914/122026:VERBOSE1:audio_**manager_win.cc(324)]
> input_params.sample_rate(): 44100
> [15680:12932:0914/122026:VERBOSE1:audio_**manager_win.cc(325)]
> input_params.bits_per_sample()**: 16
>
> +
>
> [15680:12932:0914/122026:VERBOSE1:audio_**output_resampler.cc(101)]
> Resampling
> from 44100 to 48000
> [15680:12932:0914/122026:VERBOSE1:audio_**output_resampler.cc(125)]
> Rebuffering
> from 2048 to 480
> [15680:12932:0914/122026:VERBOSE1:audio_**output_resampler.cc(133)] I/O
> ratio is
> 0.91875
>
> using the same settings.
>
>
https://codereview.chromium.**org/10913267/<https://codereview.chromium.org/1...
>

[DaleCurtis, 2012-09-14 10:53:57]

Thanks Henrik! I've added some additional logging. I also noticed my min/max
values were flipped so we were using a much larger buffer size than we wanted to
use on fallback.

Please give it another test when you have a moment. I'm building to retest on my
Mac right now.

[henrika (OOO until Aug 14), 2012-09-14 11:19:25]

Works better. New output when I tweak and force "XP" to be used:

[7628:7380:0914/131808:VERBOSE1:audio_output_resampler.cc(136)] Input and output
params are the same; in pass-through mode.
[7628:7380:0914/131808:VERBOSE1:audio_manager_win.cc(261)] Using WaveOut since
WASAPI requires at least Vista.

[henrika (OOO until Aug 14), 2012-09-14 11:33:40]

LGTM.

As we discussed off-line, we don't log an error message signaling that we fall
back on XP since we have nothing to "fall back from".

[DaleCurtis, 2012-09-14 11:34:41]

Thanks Hernik, scherkus has already left for the day, but we discussed TBR'ing
this as necessary.

[commit-bot: I haz the power, 2012-09-14 12:11:54]

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

[scherkus (not reviewing), 2012-09-14 14:29:14]

(took a look, lgtm still stands)
On Sep 14, 2012 2:11 PM, <commit-bot@chromium.org> wrote:

> CQ is trying da patch. Follow status at
> https://chromium-status.**appspot.com/cq/dalecurtis@**
>
chromium.org/10913267/9005<https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/10913267/9005>
>
>
>
https://chromiumcodereview.**appspot.com/10913267/<https://chromiumcodereview...
>