Convert audio samples in Whispernet.

Convert audio samples in Whispernet.

Currently we record at the system default sample rate and convert it to the
sample rate required by whispernet. This is no longer needed since Whispernet
now accepts a "broadcast sample rate" which can be different than the encoding
sample rate. To avoid the expensive resampling, we now need to pass the samples
to Whispernet without conversion and tell whispernet what the recording sample
rate was.

Also change the tests to test this functionality.

Reviews requested,
ckehoe@ - General review.
dalecurtis@ - DEPS review for //media

R=ckehoe@chromium.org, dalecurtis@chromium.org
BUG=454885
Committed: https://crrev.com/acb75a037014b791126cb5109da672325cd95203
Cr-Commit-Position: refs/heads/master@{#317411}

[Charlie, 2015-02-20 00:58:21]

LGTM with comments.

https://codereview.chromium.org/940123004/diff/1/chrome/browser/copresence/ch...
File chrome/browser/copresence/chrome_whispernet_client_browsertest.cc (right):

https://codereview.chromium.org/940123004/diff/1/chrome/browser/copresence/ch...
chrome/browser/copresence/chrome_whispernet_client_browsertest.cc:90:
coder_params_ = media::AudioParameters(
nit: extra space before =

https://codereview.chromium.org/940123004/diff/1/chrome/browser/copresence/ch...
chrome/browser/copresence/chrome_whispernet_client_browsertest.cc:186: double
ProvideInput(media::AudioBus* dest,
This can be private.

https://codereview.chromium.org/940123004/diff/1/chrome/browser/copresence/ch...
chrome/browser/copresence/chrome_whispernet_client_browsertest.cc:200:
std::ceil(static_cast<double>(default_params_.sample_rate()) /
Instead of static_cast<double>, just multiply by 1.0. The compiler will optimize
it away and leave the correct type.

https://codereview.chromium.org/940123004/diff/1/chrome/browser/copresence/ch...
chrome/browser/copresence/chrome_whispernet_client_browsertest.cc:220: while
(saved_samples_->frames() - saved_samples_index_ != 0) {
while (saved_samples_index_ < saved_samples_->frames())

https://codereview.chromium.org/940123004/diff/1/chrome/browser/copresence/ch...
chrome/browser/copresence/chrome_whispernet_client_browsertest.cc:266: //
DetectBroadcast will be removed soon, it isn't supported or needed anymore.
The CL I just checked in deletes this. And leaves some merge work for you :-)

https://codereview.chromium.org/940123004/diff/1/components/audio_modem/audio...
File components/audio_modem/audio_recorder_unittest.cc (right):

https://codereview.chromium.org/940123004/diff/1/components/audio_modem/audio...
components/audio_modem/audio_recorder_unittest.cc:196: TEST_F(AudioRecorderTest,
BasicRecordAndStop) {
Cool! I had a bug open about this.

[Charlie, 2015-02-20 18:14:02]

Please address igorp's comments in cl/86742707 and re-upload the binary.

Please also put that CL number in the nacl manifest instead of the arbitrary
"version" number. We should be able to get the CL checked in.

[DaleCurtis, 2015-02-20 18:31:42]

https://codereview.chromium.org/940123004/diff/1/chrome/browser/copresence/ch...
File chrome/browser/copresence/chrome_whispernet_client_browsertest.cc (right):

https://codereview.chromium.org/940123004/diff/1/chrome/browser/copresence/ch...
chrome/browser/copresence/chrome_whispernet_client_browsertest.cc:221:
scoped_ptr<media::AudioBus> converted_source = media::AudioBus::Create(
Why not just make a single call to Convert asking for new_size frames? You can
also convert directly into converted_samples since an AudioBusRefCounted is
still an AudioBus.

https://codereview.chromium.org/940123004/diff/1/components/audio_modem/audio...
File components/audio_modem/audio_recorder_impl.cc (right):

https://codereview.chromium.org/940123004/diff/1/components/audio_modem/audio...
components/audio_modem/audio_recorder_impl.cc:176: if (buffer_->frames() ==
buffer_frame_index_) {
Is this always going to be an exact match? It seems like it wouldn't be,
especially since you're copying remaining frames below.

[rkc, 2015-02-20 18:52:44]

New patchsets have been uploaded after l-g-t-m from ckehoe@chromium.org

[rkc, 2015-02-20 18:53:08]

https://codereview.chromium.org/940123004/diff/1/chrome/browser/copresence/ch...
File chrome/browser/copresence/chrome_whispernet_client_browsertest.cc (right):

https://codereview.chromium.org/940123004/diff/1/chrome/browser/copresence/ch...
chrome/browser/copresence/chrome_whispernet_client_browsertest.cc:90:
coder_params_ = media::AudioParameters(
On 2015/02/20 00:58:20, Charlie wrote:
> nit: extra space before =

There is a space between the coder_params_ and the =
Why do we want two spaces there? (BTW, this is git cl format'ed code).

https://codereview.chromium.org/940123004/diff/1/chrome/browser/copresence/ch...
chrome/browser/copresence/chrome_whispernet_client_browsertest.cc:186: double
ProvideInput(media::AudioBus* dest,
On 2015/02/20 00:58:20, Charlie wrote:
> This can be private.

Done.

https://codereview.chromium.org/940123004/diff/1/chrome/browser/copresence/ch...
chrome/browser/copresence/chrome_whispernet_client_browsertest.cc:200:
std::ceil(static_cast<double>(default_params_.sample_rate()) /
On 2015/02/20 00:58:21, Charlie wrote:
> Instead of static_cast<double>, just multiply by 1.0. The compiler will
optimize
> it away and leave the correct type.

I prefer casting here since it makes it very explicit what I am trying to do.
Plus it doesn't have a runtime or compile time cost anyway.

https://codereview.chromium.org/940123004/diff/1/chrome/browser/copresence/ch...
chrome/browser/copresence/chrome_whispernet_client_browsertest.cc:220: while
(saved_samples_->frames() - saved_samples_index_ != 0) {
On 2015/02/20 00:58:20, Charlie wrote:
> while (saved_samples_index_ < saved_samples_->frames())

Done.

https://codereview.chromium.org/940123004/diff/1/chrome/browser/copresence/ch...
chrome/browser/copresence/chrome_whispernet_client_browsertest.cc:221:
scoped_ptr<media::AudioBus> converted_source = media::AudioBus::Create(
On 2015/02/20 18:31:42, DaleCurtis wrote:
> Why not just make a single call to Convert asking for new_size frames? You can
> also convert directly into converted_samples since an AudioBusRefCounted is
> still an AudioBus.

Done.

https://codereview.chromium.org/940123004/diff/1/chrome/browser/copresence/ch...
chrome/browser/copresence/chrome_whispernet_client_browsertest.cc:266: //
DetectBroadcast will be removed soon, it isn't supported or needed anymore.
On 2015/02/20 00:58:21, Charlie wrote:
> The CL I just checked in deletes this. And leaves some merge work for you :-)

Acknowledged.

https://codereview.chromium.org/940123004/diff/1/components/audio_modem/audio...
File components/audio_modem/audio_recorder_impl.cc (right):

https://codereview.chromium.org/940123004/diff/1/components/audio_modem/audio...
components/audio_modem/audio_recorder_impl.cc:176: if (buffer_->frames() ==
buffer_frame_index_) {
On 2015/02/20 18:31:42, DaleCurtis wrote:
> Is this always going to be an exact match? It seems like it wouldn't be,
> especially since you're copying remaining frames below.

It'll still be an exact match since if buffer_frame_index_ will only be
increased to the match buffer_->frames() and never over (since if we don't have
enough frames left in the buffer, the std::min 3 lines above will change
frames_to_copy to only the number of frames left in the buffer).

https://codereview.chromium.org/940123004/diff/1/components/audio_modem/audio...
File components/audio_modem/audio_recorder_unittest.cc (right):

https://codereview.chromium.org/940123004/diff/1/components/audio_modem/audio...
components/audio_modem/audio_recorder_unittest.cc:196: TEST_F(AudioRecorderTest,
BasicRecordAndStop) {
On 2015/02/20 00:58:21, Charlie wrote:
> Cool! I had a bug open about this.

I added a few more changes to make sure that these tests won't break on other
platforms now. We need to make sure we use the default system parameters on
Windows in particular for things to work.

[DaleCurtis, 2015-02-20 18:57:21]

lgtm

[rkc, 2015-02-20 19:05:28]

New patchsets have been uploaded after l-g-t-m from dalecurtis@chromium.org

[rkc, 2015-02-20 19:06:24]

The CQ bit was checked by rkc@chromium.org

[commit-bot: I haz the power, 2015-02-20 19:06:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/940123004/40001

[rkc, 2015-02-20 20:43:04]

The CQ bit was checked by rkc@chromium.org

[commit-bot: I haz the power, 2015-02-20 20:44:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/940123004/60001

[commit-bot: I haz the power, 2015-02-20 21:58:33]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-02-20 21:59:11]

Patchset 4 (id:??) landed as
https://crrev.com/acb75a037014b791126cb5109da672325cd95203
Cr-Commit-Position: refs/heads/master@{#317411}