Add groupid for media devices. Group audio devices.

Add groupid for media devices. Group audio devices.

At present, the renderer tries to use matched_output_device
to assign groupids to audio devices. matched_output_device isn't set
before devices are sent to the renderer, and we wouldn't be able to
handle all the cases with it anyways.

BUG=636300, 627793
Committed: https://crrev.com/f07d1047986130ef0277a87b5b0b6175c4362c98
Cr-Commit-Position: refs/heads/master@{#416140}

[Max Morin, 2016-08-23 13:38:12]

Description was changed from

==========
Add groupid for media devices. Group audio devices.

At present, the renderer tries to use matched_output_device
to assign groupids to audio devices. matched_output_device isn't set
before devices are sent to the renderer, and we wouldn't be able to
handle all the cases with it anyways.

BUG=636300
==========

to

==========
Add groupid for media devices. Group audio devices.

At present, the renderer tries to use matched_output_device
to assign groupids to audio devices. matched_output_device isn't set
before devices are sent to the renderer, and we wouldn't be able to
handle all the cases with it anyways.

BUG=636300, 627793
==========

[Max Morin, 2016-08-24 07:35:20]

maxmorin@chromium.org changed reviewers:
+ guidou@chromium.org

[Max Morin, 2016-08-24 07:39:25]

Guido: PTAL when you are back in office. This should make the grouping work on
Windows.

[Guido Urdaneta, 2016-08-29 13:59:46]

guidou@chromium.org changed reviewers:
+ tommi@chromium.org

[Guido Urdaneta, 2016-08-29 13:59:48]

lgtm % nits

Adding tommi for owner review.

https://codereview.chromium.org/2273653002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/media/media_stream_manager.cc (right):

https://codereview.chromium.org/2273653002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_stream_manager.cc:1047: request->salt,
request->security_origin, device->group_id);
according to the spec, this ID should be session-unique (instead of
origin-unique), but we can address than in an upcoming CL.

https://codereview.chromium.org/2273653002/diff/20001/media/audio/audio_manag...
File media/audio/audio_manager.h (right):

https://codereview.chromium.org/2273653002/diff/20001/media/audio/audio_manag...
media/audio/audio_manager.h:247: virtual std::string
GetGroupIDOutput(std::string output_id) = 0;
rename output_id to output_device_id

https://codereview.chromium.org/2273653002/diff/20001/media/audio/audio_manag...
media/audio/audio_manager.h:248: virtual std::string GetGroupIDInput(std::string
input_id) = 0;
rename input_id to input_device_id

https://codereview.chromium.org/2273653002/diff/20001/media/audio/audio_manag...
File media/audio/audio_manager_base.cc (right):

https://codereview.chromium.org/2273653002/diff/20001/media/audio/audio_manag...
media/audio/audio_manager_base.cc:348: std::string
AudioManagerBase::GetGroupIDOutput(std::string output_id) {
ditto

https://codereview.chromium.org/2273653002/diff/20001/media/audio/audio_manag...
media/audio/audio_manager_base.cc:358: std::string
AudioManagerBase::GetGroupIDInput(std::string input_id) {
ditto

https://codereview.chromium.org/2273653002/diff/20001/media/audio/audio_manag...
File media/audio/audio_manager_base.h (right):

https://codereview.chromium.org/2273653002/diff/20001/media/audio/audio_manag...
media/audio/audio_manager_base.h:96: std::string GetGroupIDOutput(std::string
output_id) override;
ditto

https://codereview.chromium.org/2273653002/diff/20001/media/audio/audio_manag...
media/audio/audio_manager_base.h:97: std::string GetGroupIDInput(std::string
input_id) override;
ditto

[Max Morin, 2016-08-29 14:13:07]

maxmorin@chromium.org changed reviewers:
+ nasko@chromium.org

[Max Morin, 2016-08-29 14:14:33]

Guido: variables were renamed.
Nasko: PTAL at content/common/media/media_stream_messages.h and
content/public/common/media_stream_request.h.
Tommi: PTAL at everything.

https://codereview.chromium.org/2273653002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/media/media_stream_manager.cc (right):

https://codereview.chromium.org/2273653002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_stream_manager.cc:1047: request->salt,
request->security_origin, device->group_id);
On 2016/08/29 13:59:47, Guido Urdaneta wrote:
> according to the spec, this ID should be session-unique (instead of
> origin-unique), but we can address than in an upcoming CL.

Acknowledged.

[tommi (sloooow) - chröme, 2016-08-29 16:19:54]

https://codereview.chromium.org/2273653002/diff/40001/media/audio/audio_manag...
File media/audio/audio_manager.h (right):

https://codereview.chromium.org/2273653002/diff/40001/media/audio/audio_manag...
media/audio/audio_manager.h:247: virtual std::string
GetGroupIDOutput(std::string output_device_id) = 0;
const std::string& output_device_id

https://codereview.chromium.org/2273653002/diff/40001/media/audio/audio_manag...
media/audio/audio_manager.h:248: virtual std::string GetGroupIDInput(std::string
input_device_id) = 0;
ditto

https://codereview.chromium.org/2273653002/diff/40001/media/audio/audio_manag...
File media/audio/audio_manager_base.cc (right):

https://codereview.chromium.org/2273653002/diff/40001/media/audio/audio_manag...
media/audio/audio_manager_base.cc:359: std::string output_device_id =
GetAssociatedOutputDeviceID(input_id);
what is input_id? (does this compile?)

https://codereview.chromium.org/2273653002/diff/40001/media/audio/audio_manag...
File media/audio/audio_manager_unittest.cc (right):

https://codereview.chromium.org/2273653002/diff/40001/media/audio/audio_manag...
media/audio/audio_manager_unittest.cc:675: else if (input_id == "input2")
nit: 'else' isn't needed.

[nasko, 2016-08-29 16:58:14]

The IPC change looks good, but I will stamp the CL once it has been signed off
by other owners.

[Max Morin, 2016-08-30 08:17:45]

Tommi: PTAL

https://codereview.chromium.org/2273653002/diff/40001/media/audio/audio_manag...
File media/audio/audio_manager.h (right):

https://codereview.chromium.org/2273653002/diff/40001/media/audio/audio_manag...
media/audio/audio_manager.h:247: virtual std::string
GetGroupIDOutput(std::string output_device_id) = 0;
On 2016/08/29 16:19:53, tommi (chrömium) wrote:
> const std::string& output_device_id

Done.

https://codereview.chromium.org/2273653002/diff/40001/media/audio/audio_manag...
media/audio/audio_manager.h:248: virtual std::string GetGroupIDInput(std::string
input_device_id) = 0;
On 2016/08/29 16:19:53, tommi (chrömium) wrote:
> ditto

Done.

https://codereview.chromium.org/2273653002/diff/40001/media/audio/audio_manag...
File media/audio/audio_manager_base.cc (right):

https://codereview.chromium.org/2273653002/diff/40001/media/audio/audio_manag...
media/audio/audio_manager_base.cc:359: std::string output_device_id =
GetAssociatedOutputDeviceID(input_id);
On 2016/08/29 16:19:53, tommi (chrömium) wrote:
> what is input_id? (does this compile?)

Done.

https://codereview.chromium.org/2273653002/diff/40001/media/audio/audio_manag...
File media/audio/audio_manager_unittest.cc (right):

https://codereview.chromium.org/2273653002/diff/40001/media/audio/audio_manag...
media/audio/audio_manager_unittest.cc:675: else if (input_id == "input2")
On 2016/08/29 16:19:53, tommi (chrömium) wrote:
> nit: 'else' isn't needed.

Done.

[tommi (sloooow) - chröme, 2016-08-30 09:02:32]

lgtm

[tommi (sloooow) - chröme, 2016-08-30 09:02:38]

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

[commit-bot: I haz the power, 2016-08-30 09:02:59]

Dry run: 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, 2016-08-30 09:28:24]

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

[commit-bot: I haz the power, 2016-08-30 09:28:25]

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

[Max Morin, 2016-08-30 12:08:48]

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

[commit-bot: I haz the power, 2016-08-30 12:09:10]

Dry run: 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, 2016-08-30 13:47:41]

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

[commit-bot: I haz the power, 2016-08-30 13:47:43]

Dry run: This issue passed the CQ dry run.

[Max Morin, 2016-09-01 09:31:52]

On 2016/08/30 09:02:32, tommi (chrömium) wrote:
> lgtm

nasko: Since everyone else has LGTMed this, PTAL now.

[nasko, 2016-09-01 15:35:11]

LGTM

[Max Morin, 2016-09-01 20:43:01]

The CQ bit was checked by maxmorin@chromium.org

[Max Morin, 2016-09-01 20:43:01]

The patchset sent to the CQ was uploaded after l-g-t-m from guidou@chromium.org,
tommi@chromium.org
Link to the patchset: https://codereview.chromium.org/2273653002/#ps80001
(title: "Various small fixes.")

[commit-bot: I haz the power, 2016-09-01 20:43:34]

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, 2016-09-02 00:17:44]

Description was changed from

==========
Add groupid for media devices. Group audio devices.

At present, the renderer tries to use matched_output_device
to assign groupids to audio devices. matched_output_device isn't set
before devices are sent to the renderer, and we wouldn't be able to
handle all the cases with it anyways.

BUG=636300, 627793
==========

to

==========
Add groupid for media devices. Group audio devices.

At present, the renderer tries to use matched_output_device
to assign groupids to audio devices. matched_output_device isn't set
before devices are sent to the renderer, and we wouldn't be able to
handle all the cases with it anyways.

BUG=636300, 627793
==========

[commit-bot: I haz the power, 2016-09-02 00:17:45]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-02 00:19:39]

Description was changed from

==========
Add groupid for media devices. Group audio devices.

At present, the renderer tries to use matched_output_device
to assign groupids to audio devices. matched_output_device isn't set
before devices are sent to the renderer, and we wouldn't be able to
handle all the cases with it anyways.

BUG=636300, 627793
==========

to

==========
Add groupid for media devices. Group audio devices.

At present, the renderer tries to use matched_output_device
to assign groupids to audio devices. matched_output_device isn't set
before devices are sent to the renderer, and we wouldn't be able to
handle all the cases with it anyways.

BUG=636300, 627793

Committed: https://crrev.com/f07d1047986130ef0277a87b5b0b6175c4362c98
Cr-Commit-Position: refs/heads/master@{#416140}
==========

[commit-bot: I haz the power, 2016-09-02 00:19:40]

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

[Max Morin, 2016-09-02 09:00:36]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/2296393004/ by maxmorin@chromium.org.

The reason for reverting is: Breaks webrtc FYI bots on mac
https://build.chromium.org/p/chromium.webrtc/builders/Mac%20Tester/builds/59143.
I'll have to figure out what's up with that ¯\_(ツ)_/¯.