Adds support for 16kHz input sample rate and mono channel config. in WebRTC.

Adds support for 16kHz input sample rate and mono channel config. in WebRTC.


BUG=WebRTC demo doesn't work with Logitech 9000 as microphone
TEST=content_unittest --gtest_filter=WebRTCAudioDeviceTest*

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

[henrika (OOO until Aug 14), 2012-01-17 09:48:54]

The problem I am solving here is that some Web cameras only support recording in
16kHz mono. This CL ensures that 16kHz is supported by WebRTC and adds a new IPC
channel for asking the audio layer about the actual supported channel config
(stereo or mono). If mono is reported, the WebRTC client will try to open the
recording device in mono.

First patch works for Windows only. Mac OS X is next.

[henrika (OOO until Aug 14), 2012-01-17 11:20:29]

After discussions with Tommi we decided that I should add support for 32kHz
(recording) in this CL as well.

[tommi (sloooow) - chröme, 2012-01-17 12:01:08]

looks good overall.  Mostly nits about "GetChannels->GetChannelCount" since the
return value is a count, not channels.

https://chromiumcodereview.appspot.com/9221010/diff/1/content/browser/rendere...
File content/browser/renderer_host/render_message_filter.h (right):

https://chromiumcodereview.appspot.com/9221010/diff/1/content/browser/rendere...
content/browser/renderer_host/render_message_filter.h:174: void
OnGetHardwareInputChannels(size_t* channels);
change to OnGetHardwareInputChannelCount

https://chromiumcodereview.appspot.com/9221010/diff/1/content/common/view_mes...
File content/common/view_messages.h (right):

https://chromiumcodereview.appspot.com/9221010/diff/1/content/common/view_mes...
content/common/view_messages.h:696:
IPC_SYNC_MESSAGE_CONTROL0_1(ViewHostMsg_GetHardwareInputChannels,
change to ViewHostMsg_GetHardwareInputChannelCount

https://chromiumcodereview.appspot.com/9221010/diff/1/content/renderer/media/...
File content/renderer/media/audio_hardware.cc (right):

https://chromiumcodereview.appspot.com/9221010/diff/1/content/renderer/media/...
content/renderer/media/audio_hardware.cc:14: static size_t input_channels = 0;
input_channel_count

https://chromiumcodereview.appspot.com/9221010/diff/1/content/renderer/media/...
File content/renderer/media/audio_hardware.h (right):

https://chromiumcodereview.appspot.com/9221010/diff/1/content/renderer/media/...
content/renderer/media/audio_hardware.h:31: CONTENT_EXPORT size_t
GetInputChannels();
change to GetInputChannelCount.

https://chromiumcodereview.appspot.com/9221010/diff/1/content/renderer/media/...
File content/renderer/media/webrtc_audio_device_impl.cc (right):

https://chromiumcodereview.appspot.com/9221010/diff/1/content/renderer/media/...
content/renderer/media/webrtc_audio_device_impl.cc:316: input_sample_rate !=
16000) {
also support 32kHz as discussed

https://chromiumcodereview.appspot.com/9221010/diff/1/content/renderer/media/...
File content/renderer/media/webrtc_audio_device_unittest.cc (right):

https://chromiumcodereview.appspot.com/9221010/diff/1/content/renderer/media/...
content/renderer/media/webrtc_audio_device_unittest.cc:41: virtual size_t
GetAudioInputHardwareChannels() OVERRIDE {
GetAudioInputHardwareChannelCount

https://chromiumcodereview.appspot.com/9221010/diff/1/content/test/webrtc_aud...
File content/test/webrtc_audio_device_test.cc (right):

https://chromiumcodereview.appspot.com/9221010/diff/1/content/test/webrtc_aud...
content/test/webrtc_audio_device_test.cc:241:
audio_util_callback_->GetAudioInputHardwareChannels() : 0;
4 spaces

https://chromiumcodereview.appspot.com/9221010/diff/1/content/test/webrtc_aud...
File content/test/webrtc_audio_device_test.h (right):

https://chromiumcodereview.appspot.com/9221010/diff/1/content/test/webrtc_aud...
content/test/webrtc_audio_device_test.h:112: virtual size_t
GetAudioInputHardwareChannels() = 0;
GetAudioInputHardwareChanneCount

https://chromiumcodereview.appspot.com/9221010/diff/1/media/audio/audio_util.cc
File media/audio/audio_util.cc (right):

https://chromiumcodereview.appspot.com/9221010/diff/1/media/audio/audio_util....
media/audio/audio_util.cc:341: enum channel_layout { MONO = 1, STEREO = 2 };
nice

https://chromiumcodereview.appspot.com/9221010/diff/1/media/audio/win/audio_l...
File media/audio/win/audio_low_latency_input_win.cc (right):

https://chromiumcodereview.appspot.com/9221010/diff/1/media/audio/win/audio_l...
media/audio/win/audio_low_latency_input_win.cc:220: if (FAILED(hr)) {
nit: remove {}

https://chromiumcodereview.appspot.com/9221010/diff/1/media/audio/win/audio_l...
File media/audio/win/audio_low_latency_input_win.h (right):

https://chromiumcodereview.appspot.com/9221010/diff/1/media/audio/win/audio_l...
media/audio/win/audio_low_latency_input_win.h:98: static size_t
HardwareChannels(ERole device_role);
HardwareChannelCount

[henrika (OOO until Aug 14), 2012-01-17 12:54:59]

Did changes proposed by Tommi and added support for 32kHz input as well.

http://codereview.chromium.org/9221010/diff/1/content/browser/renderer_host/r...
File content/browser/renderer_host/render_message_filter.h (right):

http://codereview.chromium.org/9221010/diff/1/content/browser/renderer_host/r...
content/browser/renderer_host/render_message_filter.h:174: void
OnGetHardwareInputChannels(size_t* channels);
On 2012/01/17 12:01:08, tommi wrote:
> change to OnGetHardwareInputChannelCount

Done.

http://codereview.chromium.org/9221010/diff/1/content/common/view_messages.h
File content/common/view_messages.h (right):

http://codereview.chromium.org/9221010/diff/1/content/common/view_messages.h#...
content/common/view_messages.h:696:
IPC_SYNC_MESSAGE_CONTROL0_1(ViewHostMsg_GetHardwareInputChannels,
On 2012/01/17 12:01:08, tommi wrote:
> change to ViewHostMsg_GetHardwareInputChannelCount

Done.

http://codereview.chromium.org/9221010/diff/1/content/renderer/media/audio_ha...
File content/renderer/media/audio_hardware.cc (right):

http://codereview.chromium.org/9221010/diff/1/content/renderer/media/audio_ha...
content/renderer/media/audio_hardware.cc:14: static size_t input_channels = 0;
On 2012/01/17 12:01:08, tommi wrote:
> input_channel_count

Done.

http://codereview.chromium.org/9221010/diff/1/content/renderer/media/audio_ha...
File content/renderer/media/audio_hardware.h (right):

http://codereview.chromium.org/9221010/diff/1/content/renderer/media/audio_ha...
content/renderer/media/audio_hardware.h:31: CONTENT_EXPORT size_t
GetInputChannels();
On 2012/01/17 12:01:08, tommi wrote:
> change to GetInputChannelCount.

Done.

http://codereview.chromium.org/9221010/diff/1/content/renderer/media/webrtc_a...
File content/renderer/media/webrtc_audio_device_impl.cc (right):

http://codereview.chromium.org/9221010/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_impl.cc:316: input_sample_rate !=
16000) {
On 2012/01/17 12:01:08, tommi wrote:
> also support 32kHz as discussed

Done.

http://codereview.chromium.org/9221010/diff/1/content/renderer/media/webrtc_a...
File content/renderer/media/webrtc_audio_device_unittest.cc (right):

http://codereview.chromium.org/9221010/diff/1/content/renderer/media/webrtc_a...
content/renderer/media/webrtc_audio_device_unittest.cc:41: virtual size_t
GetAudioInputHardwareChannels() OVERRIDE {
On 2012/01/17 12:01:08, tommi wrote:
> GetAudioInputHardwareChannelCount

Done.

http://codereview.chromium.org/9221010/diff/1/content/test/webrtc_audio_devic...
File content/test/webrtc_audio_device_test.cc (right):

http://codereview.chromium.org/9221010/diff/1/content/test/webrtc_audio_devic...
content/test/webrtc_audio_device_test.cc:241:
audio_util_callback_->GetAudioInputHardwareChannels() : 0;
On 2012/01/17 12:01:08, tommi wrote:
> 4 spaces

Done.

http://codereview.chromium.org/9221010/diff/1/content/test/webrtc_audio_devic...
File content/test/webrtc_audio_device_test.h (right):

http://codereview.chromium.org/9221010/diff/1/content/test/webrtc_audio_devic...
content/test/webrtc_audio_device_test.h:112: virtual size_t
GetAudioInputHardwareChannels() = 0;
On 2012/01/17 12:01:08, tommi wrote:
> GetAudioInputHardwareChanneCount

Done.

http://codereview.chromium.org/9221010/diff/1/media/audio/win/audio_low_laten...
File media/audio/win/audio_low_latency_input_win.cc (right):

http://codereview.chromium.org/9221010/diff/1/media/audio/win/audio_low_laten...
media/audio/win/audio_low_latency_input_win.cc:220: if (FAILED(hr)) {
On 2012/01/17 12:01:08, tommi wrote:
> nit: remove {}

Done.

http://codereview.chromium.org/9221010/diff/1/media/audio/win/audio_low_laten...
File media/audio/win/audio_low_latency_input_win.h (right):

http://codereview.chromium.org/9221010/diff/1/media/audio/win/audio_low_laten...
media/audio/win/audio_low_latency_input_win.h:98: static size_t
HardwareChannels(ERole device_role);
On 2012/01/17 12:01:08, tommi wrote:
> HardwareChannelCount

Done.

[tommi (sloooow) - chröme, 2012-01-17 13:01:58]

lgtm

[henrika (OOO until Aug 14), 2012-01-18 16:14:42]

John: would appreciate a quick check by you as an owner in content/browser. Only
trivial changes.

Andrew: need your OK as owner in content/renderer/media and media/audio.

Basically what this CL does is to ensure that a WebRTC user can plug in a WebCam
which only support 16 or 32kHz mono recording. The solution has been verified on
all platforms.

Thanks!

[scherkus (not reviewing), 2012-01-18 18:19:37]

LGTM w/ nits

channel count should be fine (as opposed to channel layout enum) as I don't
think the industry is quite there with multichannel microphones just yet :)

https://chromiumcodereview.appspot.com/9221010/diff/14001/content/renderer/me...
File content/renderer/media/audio_hardware.cc (right):

https://chromiumcodereview.appspot.com/9221010/diff/14001/content/renderer/me...
content/renderer/media/audio_hardware.cc:57: new
ViewHostMsg_GetHardwareInputChannelCount(&channels));
indent

https://chromiumcodereview.appspot.com/9221010/diff/14001/content/renderer/me...
File content/renderer/media/webrtc_audio_device_impl.cc (right):

https://chromiumcodereview.appspot.com/9221010/diff/14001/content/renderer/me...
content/renderer/media/webrtc_audio_device_impl.cc:369: input_sample_rate !=
32000 && input_sample_rate != 16000) {
your initial CL comment says only windows is supported -- does this need to be
reverted?

https://chromiumcodereview.appspot.com/9221010/diff/14001/content/renderer/me...
File content/renderer/media/webrtc_audio_device_unittest.cc (right):

https://chromiumcodereview.appspot.com/9221010/diff/14001/content/renderer/me...
content/renderer/media/webrtc_audio_device_unittest.cc:52: :
output_rate_(output_rate),
indent by 2 more spaces

[jam, 2012-01-18 18:44:37]

content/browser and content/common lgtm with nit

http://codereview.chromium.org/9221010/diff/14001/content/common/view_messages.h
File content/common/view_messages.h (right):

http://codereview.chromium.org/9221010/diff/14001/content/common/view_message...
content/common/view_messages.h:697: size_t /* channels */)
we avoid size_t in IPC messages by convention, because we do have 32/64 bit
processes taking to each other. while that doesn't happen for browser<>renderer,
it's easier to just say avoid size_t in all IPCs since sometimes it's not clear
which channels an IPC goes over

[henrika (OOO until Aug 14), 2012-01-19 09:42:24]

Did final changes based on review input.

http://codereview.chromium.org/9221010/diff/14001/content/common/view_messages.h
File content/common/view_messages.h (right):

http://codereview.chromium.org/9221010/diff/14001/content/common/view_message...
content/common/view_messages.h:697: size_t /* channels */)
I will change to int instead.

http://codereview.chromium.org/9221010/diff/14001/content/renderer/media/audi...
File content/renderer/media/audio_hardware.cc (right):

http://codereview.chromium.org/9221010/diff/14001/content/renderer/media/audi...
content/renderer/media/audio_hardware.cc:57: new
ViewHostMsg_GetHardwareInputChannelCount(&channels));
On 2012/01/18 18:19:38, scherkus wrote:
> indent

Done.

http://codereview.chromium.org/9221010/diff/14001/content/renderer/media/webr...
File content/renderer/media/webrtc_audio_device_impl.cc (right):

http://codereview.chromium.org/9221010/diff/14001/content/renderer/media/webr...
content/renderer/media/webrtc_audio_device_impl.cc:369: input_sample_rate !=
32000 && input_sample_rate != 16000) {
You might be referring to "First patch works for Windows only. Mac OS X is
next." What I meant to describe was that I started out adding support for
Windows and then added Mac OSX, in the same CL. Might not have been clear. 

Can I rewrite old comments?

http://codereview.chromium.org/9221010/diff/14001/content/renderer/media/webr...
File content/renderer/media/webrtc_audio_device_unittest.cc (right):

http://codereview.chromium.org/9221010/diff/14001/content/renderer/media/webr...
content/renderer/media/webrtc_audio_device_unittest.cc:52: :
output_rate_(output_rate),
On 2012/01/18 18:19:38, scherkus wrote:
> indent by 2 more spaces

Done.

[scherkus (not reviewing), 2012-01-19 17:43:01]

On 2012/01/19 09:42:24, henrika wrote:
> Did final changes based on review input.
> 
>
http://codereview.chromium.org/9221010/diff/14001/content/common/view_messages.h
> File content/common/view_messages.h (right):
> 
>
http://codereview.chromium.org/9221010/diff/14001/content/common/view_message...
> content/common/view_messages.h:697: size_t /* channels */)
> I will change to int instead.
> 
>
http://codereview.chromium.org/9221010/diff/14001/content/renderer/media/audi...
> File content/renderer/media/audio_hardware.cc (right):
> 
>
http://codereview.chromium.org/9221010/diff/14001/content/renderer/media/audi...
> content/renderer/media/audio_hardware.cc:57: new
> ViewHostMsg_GetHardwareInputChannelCount(&channels));
> On 2012/01/18 18:19:38, scherkus wrote:
> > indent
> 
> Done.
> 
>
http://codereview.chromium.org/9221010/diff/14001/content/renderer/media/webr...
> File content/renderer/media/webrtc_audio_device_impl.cc (right):
> 
>
http://codereview.chromium.org/9221010/diff/14001/content/renderer/media/webr...
> content/renderer/media/webrtc_audio_device_impl.cc:369: input_sample_rate !=
> 32000 && input_sample_rate != 16000) {
> You might be referring to "First patch works for Windows only. Mac OS X is
> next." What I meant to describe was that I started out adding support for
> Windows and then added Mac OSX, in the same CL. Might not have been clear. 
> 
> Can I rewrite old comments?

Not that I'm aware of!

In the end it's the CL description that really matters so I wouldn't worry about
it :)

>
http://codereview.chromium.org/9221010/diff/14001/content/renderer/media/webr...
> File content/renderer/media/webrtc_audio_device_unittest.cc (right):
> 
>
http://codereview.chromium.org/9221010/diff/14001/content/renderer/media/webr...
> content/renderer/media/webrtc_audio_device_unittest.cc:52: :
> output_rate_(output_rate),
> On 2012/01/18 18:19:38, scherkus wrote:
> > indent by 2 more spaces
> 
> Done.