Refactor AudioParameters member setting.

Refactor AudioParameters member setting.

Permit AudioParameters members to be set individually. Allows us to:
- Cut back to the one non-default constructor which is frequently used.
- Remove a bunch of boilerplate in the common case of updating one
member.
- Make adding future optional members (e.g. mic_positions)
transparent to most callers.

Maintain the Reset method but remove the channels parameter to have it
mirror the constructor.

Make it clear that channels_ is only required to be set explicitly with
CHANNEL_LAYOUT_DISCRETE.

BUG=497001
Committed: https://crrev.com/2e2f1c71bec505f3ae9d718dd8e875fedd4962e7
Cr-Commit-Position: refs/heads/master@{#347781}

[ajm, 2015-09-04 05:25:32]

The CQ bit was checked by ajm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-04 05:25:34]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1304973005/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1304973005/1

[commit-bot: I haz the power, 2015-09-04 05:38:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-04 05:38:28]

Dry run: Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  cast_shell_android on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_andr...)

[ajm, 2015-09-04 06:06:37]

The CQ bit was checked by ajm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-04 06:06:59]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1304973005/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1304973005/20001

[ajm, 2015-09-04 06:17:48]

ajm@chromium.org changed reviewers:
+ dalecurtis@chromium.org, tommi@chromium.org

[commit-bot: I haz the power, 2015-09-04 06:21:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-04 06:21:34]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[ajm, 2015-09-04 06:46:05]

The CQ bit was checked by ajm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-04 06:46:16]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1304973005/2
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1304973005/2

[commit-bot: I haz the power, 2015-09-04 07:00:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-04 07:00:56]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_android_r...)

[DaleCurtis, 2015-09-04 17:03:39]

Nice cleanup! Thanks for doing this! I like everything except for the discrete
channel calling pattern... but I don't have a better idea w/o adding another
constructor.

lgtm % nits unless tommi@ has a better idea

https://codereview.chromium.org/1304973005/diff/2/chrome/browser/media/webrtc...
File chrome/browser/media/webrtc_browsertest_audio.cc (right):

https://codereview.chromium.org/1304973005/diff/2/chrome/browser/media/webrtc...
chrome/browser/media/webrtc_browsertest_audio.cc:84:
file_parameters->set_channels_for_discrete(wav_audio_handler->num_channels());
Hmm, I'm not a fan of this calling pattern, but it's not really any worse than
what we have today... and I really like the 1-constructor cleanup. :|

https://codereview.chromium.org/1304973005/diff/2/content/browser/speech/spee...
File content/browser/speech/speech_recognizer_impl.cc (right):

https://codereview.chromium.org/1304973005/diff/2/content/browser/speech/spee...
content/browser/speech/speech_recognizer_impl.cc:564:
input_parameters.Reset(in_params.format(),
Shouldn't this just use set_frames_per_buffer() ?

https://codereview.chromium.org/1304973005/diff/2/media/audio/android/audio_a...
File media/audio/android/audio_android_unittest.cc (right):

https://codereview.chromium.org/1304973005/diff/2/media/audio/android/audio_a...
media/audio/android/audio_android_unittest.cc:592: AudioParameters params =
audio_input_parameters;
()?

https://codereview.chromium.org/1304973005/diff/2/media/audio/audio_parameters.h
File media/audio/audio_parameters.h (right):

https://codereview.chromium.org/1304973005/diff/2/media/audio/audio_parameter...
media/audio/audio_parameters.h:120: void set_format(Format format) { format_ =
format; }
Pair each setter and getter?

https://codereview.chromium.org/1304973005/diff/2/media/audio/audio_parameter...
media/audio/audio_parameters.h:148: AudioParameters(const AudioParameters&) =
default;
Don't we get these automatically? Is this the new preferred style or something?

[Charlie, 2015-09-04 17:51:46]

ckehoe@chromium.org changed reviewers:
+ ckehoe@chromium.org

[Charlie, 2015-09-04 17:51:47]

Audio modem changes LGTM.

[ajm, 2015-09-04 19:02:53]

Tommi, let us know if you can think of anything better than
set_channels_for_discrete().

https://codereview.chromium.org/1304973005/diff/2/chrome/browser/media/webrtc...
File chrome/browser/media/webrtc_browsertest_audio.cc (right):

https://codereview.chromium.org/1304973005/diff/2/chrome/browser/media/webrtc...
chrome/browser/media/webrtc_browsertest_audio.cc:84:
file_parameters->set_channels_for_discrete(wav_audio_handler->num_channels());
On 2015/09/04 17:03:39, DaleCurtis wrote:
> Hmm, I'm not a fan of this calling pattern, but it's not really any worse than
> what we have today... and I really like the 1-constructor cleanup. :|

Yeah I'm not completely satisfied with it either, but it was the least bad thing
I could come up with. Open to alternatives.

https://codereview.chromium.org/1304973005/diff/2/content/browser/speech/spee...
File content/browser/speech/speech_recognizer_impl.cc (right):

https://codereview.chromium.org/1304973005/diff/2/content/browser/speech/spee...
content/browser/speech/speech_recognizer_impl.cc:564:
input_parameters.Reset(in_params.format(),
On 2015/09/04 17:03:39, DaleCurtis wrote:
> Shouldn't this just use set_frames_per_buffer() ?

Totally. Missed this one.

https://codereview.chromium.org/1304973005/diff/2/media/audio/android/audio_a...
File media/audio/android/audio_android_unittest.cc (right):

https://codereview.chromium.org/1304973005/diff/2/media/audio/android/audio_a...
media/audio/android/audio_android_unittest.cc:592: AudioParameters params =
audio_input_parameters;
On 2015/09/04 17:03:39, DaleCurtis wrote:
> ()?

Yep; this didn't compile of course, but my try runs hadn't gotten to it yet.

https://codereview.chromium.org/1304973005/diff/2/media/audio/audio_parameters.h
File media/audio/audio_parameters.h (right):

https://codereview.chromium.org/1304973005/diff/2/media/audio/audio_parameter...
media/audio/audio_parameters.h:120: void set_format(Format format) { format_ =
format; }
On 2015/09/04 17:03:39, DaleCurtis wrote:
> Pair each setter and getter?

Done, and also revealed that I was missing a set_bit_per_sample().

https://codereview.chromium.org/1304973005/diff/2/media/audio/audio_parameter...
media/audio/audio_parameters.h:148: AudioParameters(const AudioParameters&) =
default;
On 2015/09/04 17:03:39, DaleCurtis wrote:
> Don't we get these automatically? Is this the new preferred style or
something?

We do, but I prefer to make it explicit that I want a copy constructor and
assignment operator, and not that I just forgot to add DISALLOW_COPY_AND_ASSIGN.

It's permitted (encouraged?) by the style guide:
"Prefer to define copy and move operations with = default."
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Copyable_Mov...

and allowed for C++11 use in Chromium.

I don't know what the preferred style is, so I'm happy to remove it, but it may
just be that it hasn't caught on yet! :)

[ajm, 2015-09-04 19:03:01]

The CQ bit was checked by ajm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-04 19:03:38]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1304973005/50001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1304973005/50001

[commit-bot: I haz the power, 2015-09-04 20:18:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-04 20:18:36]

Dry run: This issue passed the CQ dry run.

[tommi (sloooow) - chröme, 2015-09-05 09:49:58]

lgtm. nice cleanup!

https://codereview.chromium.org/1304973005/diff/2/chrome/browser/media/webrtc...
File chrome/browser/media/webrtc_browsertest_audio.cc (right):

https://codereview.chromium.org/1304973005/diff/2/chrome/browser/media/webrtc...
chrome/browser/media/webrtc_browsertest_audio.cc:84:
file_parameters->set_channels_for_discrete(wav_audio_handler->num_channels());
On 2015/09/04 19:02:53, ajm wrote:
> On 2015/09/04 17:03:39, DaleCurtis wrote:
> > Hmm, I'm not a fan of this calling pattern, but it's not really any worse
than
> > what we have today... and I really like the 1-constructor cleanup. :|
> 
> Yeah I'm not completely satisfied with it either, but it was the least bad
thing
> I could come up with. Open to alternatives.

Ack.  No better suggestions atm.

https://codereview.chromium.org/1304973005/diff/50001/media/audio/audio_param...
File media/audio/audio_parameters.h (right):

https://codereview.chromium.org/1304973005/diff/50001/media/audio/audio_param...
media/audio/audio_parameters.h:85: int sample_rate,
this is a great improvement over how things were.  It's still possible to make
mistakes since we have three ints in a row. Maybe we should strongly type these
in the future. E.g. have an enum, typedef or even a class for sample rate, bits
per sample etc.

[tommi (sloooow) - chröme, 2015-09-05 10:23:12]

The CQ bit was checked by tommi@chromium.org

[tommi (sloooow) - chröme, 2015-09-05 10:23:13]

The patchset sent to the CQ was uploaded after l-g-t-m from ckehoe@chromium.org,
dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/1304973005/#ps50001
(title: "dalecurtis comments.")

[commit-bot: I haz the power, 2015-09-05 10:23:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1304973005/50001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1304973005/50001

[commit-bot: I haz the power, 2015-09-05 10:34:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-05 10:34:58]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[ajm, 2015-09-06 07:24:14]

https://codereview.chromium.org/1304973005/diff/50001/media/audio/audio_param...
File media/audio/audio_parameters.h (right):

https://codereview.chromium.org/1304973005/diff/50001/media/audio/audio_param...
media/audio/audio_parameters.h:85: int sample_rate,
On 2015/09/05 09:49:58, tommi wrote:
> this is a great improvement over how things were.  It's still possible to make
> mistakes since we have three ints in a row. Maybe we should strongly type
these
> in the future. E.g. have an enum, typedef or even a class for sample rate,
bits
> per sample etc.

I could see using an enum for bits_per_sample, since there aren't a lot of
possibilities.

[ajm, 2015-09-06 07:34:44]

ajm@chromium.org changed reviewers:
+ dcheng@chromium.org, jam@chromium.org

[ajm, 2015-09-06 07:34:45]

I need some OWNER approvals for minor changes. PTAL.

ckehoe:
chrome/browser/copresence/chrome_whispernet_client_browsertest.cc
(in addition to the audio_modem change you already approved)

jam:
chromecast/renderer/media/chromecast_media_renderer_factory.cc
content/renderer/pepper/pepper_platform_audio_input.cc
content/renderer/renderer_blink_platform_impl.cc

dcheng:
IPC changes to
content/common/media/media_param_traits.cc

Thanks!

[jam, 2015-09-08 16:54:34]

On 2015/09/06 07:34:45, ajm wrote:
> I need some OWNER approvals for minor changes. PTAL.
> 
> ckehoe:
> chrome/browser/copresence/chrome_whispernet_client_browsertest.cc
> (in addition to the audio_modem change you already approved)
> 
> jam:
> chromecast/renderer/media/chromecast_media_renderer_factory.cc
> content/renderer/pepper/pepper_platform_audio_input.cc
> content/renderer/renderer_blink_platform_impl.cc
lgtm
> 
> dcheng:
> IPC changes to
> content/common/media/media_param_traits.cc
> 
> Thanks!

[dcheng, 2015-09-08 18:22:03]

ipc changes lgtm

[Charlie, 2015-09-08 18:51:45]

Changes to ChromeWhispernetClientTest LGTM.

[ajm, 2015-09-08 19:25:04]

The CQ bit was checked by ajm@chromium.org

[commit-bot: I haz the power, 2015-09-08 19:26:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1304973005/50001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1304973005/50001

[commit-bot: I haz the power, 2015-09-08 20:33:03]

Committed patchset #4 (id:50001)

[commit-bot: I haz the power, 2015-09-08 20:33:50]

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