Fix WebAudio support for discrete channel layouts.

Fix WebAudio support for discrete channel layouts.

The code would fire DCHECKs in debug builds since it was not properly
handling creation requests for discrete layouts > 8 channels.

BUG=671308
TEST=new unittest.

Review-Url: https://codereview.chromium.org/2708933005
Cr-Commit-Position: refs/heads/master@{#452672}
Committed: https://chromium.googlesource.com/chromium/src/+/9bcabb08699d2b7c0fc6ac18d8b74b821f5b53d7

[DaleCurtis, 2017-02-21 22:22:43]

dalecurtis@chromium.org changed reviewers:
+ esprehn@chromium.org, hongchan@chromium.org

[DaleCurtis, 2017-02-21 22:22:44]

hongchan: Per our earlier discussion. The layouts change slightly, but I don't
think it's going to matter for WebAudio users since they likely didn't care
about the layout anyways. We can keep the old code if you prefer and just add
DISCRETE case if this changes anything on your 16 channel test device.

https://codereview.chromium.org/2708933005/diff/1/content/renderer/renderer_b...
File content/renderer/renderer_blink_platform_impl.cc (left):

https://codereview.chromium.org/2708933005/diff/1/content/renderer/renderer_b...
content/renderer/renderer_blink_platform_impl.cc:687: layout =
media::CHANNEL_LAYOUT_2_1;
New code will return SURROUND here:

Old: Front L, Front R, Back C
New: Front L, Front R, Front C

https://codereview.chromium.org/2708933005/diff/1/content/renderer/renderer_b...
content/renderer/renderer_blink_platform_impl.cc:690: layout =
media::CHANNEL_LAYOUT_4_0;
New code will return QUAD here:

Old: Front L, Front R, Front C, Back C
New: Front L, Front R, Back L, Back R

https://codereview.chromium.org/2708933005/diff/1/content/renderer/renderer_b...
content/renderer/renderer_blink_platform_impl.cc:699: layout =
media::CHANNEL_LAYOUT_7_0;
New code will return 6.1 here:

Old: Front L, Front R, Front C, Side L, Side R, Front LofC, Front RofC
New: Front L, Front R, Front C, LFE, Side L, Side R, Rear Center

https://codereview.chromium.org/2708933005/diff/1/content/renderer/renderer_b...
File content/renderer/renderer_blink_platform_impl.cc (right):

https://codereview.chromium.org/2708933005/diff/1/content/renderer/renderer_b...
content/renderer/renderer_blink_platform_impl.cc:684:
!base::StringToInt(input_device_id.utf8(), &session_id)) {
Note: This method may write to |session_id| even when failing, so the old path
was incorrect. It needs to clear session_id if the conversion fails.
|input_channels| is not used and I don't think the DLOG was adding anything, so
removed too.

[DaleCurtis, 2017-02-21 22:23:09]

@esprehn for: content/renderer/renderer_blink_platform_impl.cc

[esprehn, 2017-02-22 01:37:27]

lgtm for content/renderer, I'd also love to see this code moved into blink
instead so you wouldn't need content reviews and we could remove much of this
abstraction.

[hongchan, 2017-02-23 18:43:37]

lgtm

Confirmed the WebAudio multichannel output test runs fine with a 16 channel
audio interface.

https://codereview.chromium.org/2708933005/diff/1/content/renderer/media/rend...
File content/renderer/media/renderer_webaudiodevice_impl.cc (right):

https://codereview.chromium.org/2708933005/diff/1/content/renderer/media/rend...
content/renderer/media/renderer_webaudiodevice_impl.cc:140:
sink_params_.set_channels_for_discrete(channels);
Can we add a comment why this is necessary here? Without this, the discrete
layout will crash. Right?

[DaleCurtis, 2017-02-23 20:06:02]

The CQ bit was checked by dalecurtis@chromium.org

[DaleCurtis, 2017-02-23 20:06:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
esprehn@chromium.org, hongchan@chromium.org
Link to the patchset: https://codereview.chromium.org/2708933005/#ps20001
(title: "Add comment.")

[DaleCurtis, 2017-02-23 20:06:07]

https://codereview.chromium.org/2708933005/diff/1/content/renderer/media/rend...
File content/renderer/media/renderer_webaudiodevice_impl.cc (right):

https://codereview.chromium.org/2708933005/diff/1/content/renderer/media/rend...
content/renderer/media/renderer_webaudiodevice_impl.cc:140:
sink_params_.set_channels_for_discrete(channels);
On 2017/02/23 at 18:43:37, hongchan wrote:
> Can we add a comment why this is necessary here? Without this, the discrete
layout will crash. Right?

Done.

[commit-bot: I haz the power, 2017-02-23 20:06:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 20:40:19]

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

[commit-bot: I haz the power, 2017-02-23 20:40:20]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[DaleCurtis, 2017-02-23 22:34:14]

The CQ bit was checked by dalecurtis@chromium.org

[DaleCurtis, 2017-02-23 22:34:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
esprehn@chromium.org, hongchan@chromium.org
Link to the patchset: https://codereview.chromium.org/2708933005/#ps40001
(title: "Zero session_id.")

[commit-bot: I haz the power, 2017-02-23 22:34:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 23:34:06]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1487889254130220,
"parent_rev": "a2f40415f68f600ff00656187b0f091ccfb5a6bb", "commit_rev":
"9bcabb08699d2b7c0fc6ac18d8b74b821f5b53d7"}

[commit-bot: I haz the power, 2017-02-23 23:34:43]

Description was changed from

==========
Fix WebAudio support for discrete channel layouts.

The code would fire DCHECKs in debug builds since it was not properly
handling creation requests for discrete layouts > 8 channels.

BUG=671308
TEST=new unittest.
==========

to

==========
Fix WebAudio support for discrete channel layouts.

The code would fire DCHECKs in debug builds since it was not properly
handling creation requests for discrete layouts > 8 channels.

BUG=671308
TEST=new unittest.

Review-Url: https://codereview.chromium.org/2708933005
Cr-Commit-Position: refs/heads/master@{#452672}
Committed:
https://chromium.googlesource.com/chromium/src/+/9bcabb08699d2b7c0fc6ac18d8b7...
==========

[commit-bot: I haz the power, 2017-02-23 23:34:45]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/9bcabb08699d2b7c0fc6ac18d8b7...