New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 740943 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
OOO Dec 22 - Jan 8
Closed: Jan 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 672469



Sign in to add a comment

Implement Mojo interface for audio device information

Project Member Reported by olka@chromium.org, Jul 11 2017

Issue description

Blocking: 672469
Components: Internals>Media
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6d6c995a6296327105a596efce286777d2e9077d

commit 6d6c995a6296327105a596efce286777d2e9077d
Author: Olga Sharonova <olka@chromium.org>
Date: Thu Aug 31 19:01:33 2017

DeviceMonitorMac: pass |device_task_tunner| in the constructor.

At the moment MediaDevicesManager takes the task runner info from AudioSystem and passes it to DeviceMonitorMac::StartMonitoring().
When AudioSystem is switched to Mojo it won't be aware of the device task runner any more.
So we are removing AudioSystem::GetTaskRunner() everywhere (which is here and in MediaStreamManager).

Bug:  740943 
Change-Id: I5031cb3865cea95d6754f8dbf01dc88064962de1
Reviewed-on: https://chromium-review.googlesource.com/644466
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498940}
[modify] https://crrev.com/6d6c995a6296327105a596efce286777d2e9077d/content/browser/browser_main_loop.cc
[modify] https://crrev.com/6d6c995a6296327105a596efce286777d2e9077d/content/browser/renderer_host/media/media_devices_manager.cc
[modify] https://crrev.com/6d6c995a6296327105a596efce286777d2e9077d/media/device_monitors/device_monitor_mac.h
[modify] https://crrev.com/6d6c995a6296327105a596efce286777d2e9077d/media/device_monitors/device_monitor_mac.mm

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bb578126659ea431171fa29a1bdc493c7fc517d8

commit bb578126659ea431171fa29a1bdc493c7fc517d8
Author: Olga Sharonova <olka@chromium.org>
Date: Thu Aug 31 19:18:38 2017

Removing AudioSystem::GetTaskRunner() and its usage in MediaStreamManager

AudioSystem::GetTaskRunner() has been used in a couple of places to obtain AudioManager device task runner.
When AudioSystem moves to Mojo it won't have this knowledge any more.

Bug:  740943 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I918f4fd6b46c36b6f0c3bd7a783cb553b90546e1
Reviewed-on: https://chromium-review.googlesource.com/645627
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Commit-Queue: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498948}
[modify] https://crrev.com/bb578126659ea431171fa29a1bdc493c7fc517d8/content/browser/browser_main_loop.cc
[modify] https://crrev.com/bb578126659ea431171fa29a1bdc493c7fc517d8/content/browser/renderer_host/media/audio_input_renderer_host_unittest.cc
[modify] https://crrev.com/bb578126659ea431171fa29a1bdc493c7fc517d8/content/browser/renderer_host/media/audio_output_authorization_handler_unittest.cc
[modify] https://crrev.com/bb578126659ea431171fa29a1bdc493c7fc517d8/content/browser/renderer_host/media/audio_renderer_host_unittest.cc
[modify] https://crrev.com/bb578126659ea431171fa29a1bdc493c7fc517d8/content/browser/renderer_host/media/media_devices_dispatcher_host_unittest.cc
[modify] https://crrev.com/bb578126659ea431171fa29a1bdc493c7fc517d8/content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc
[modify] https://crrev.com/bb578126659ea431171fa29a1bdc493c7fc517d8/content/browser/renderer_host/media/media_stream_manager.cc
[modify] https://crrev.com/bb578126659ea431171fa29a1bdc493c7fc517d8/content/browser/renderer_host/media/media_stream_manager.h
[modify] https://crrev.com/bb578126659ea431171fa29a1bdc493c7fc517d8/content/browser/renderer_host/media/media_stream_manager_unittest.cc
[modify] https://crrev.com/bb578126659ea431171fa29a1bdc493c7fc517d8/content/browser/renderer_host/media/video_capture_unittest.cc
[modify] https://crrev.com/bb578126659ea431171fa29a1bdc493c7fc517d8/content/test/renderer_audio_output_stream_factory_context_impl_unittest.cc
[modify] https://crrev.com/bb578126659ea431171fa29a1bdc493c7fc517d8/media/audio/audio_system.h
[modify] https://crrev.com/bb578126659ea431171fa29a1bdc493c7fc517d8/media/audio/audio_system_impl.cc
[modify] https://crrev.com/bb578126659ea431171fa29a1bdc493c7fc517d8/media/audio/audio_system_impl.h

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/62ff2e00b26efc679df630dce0f9da0358b5de03

commit 62ff2e00b26efc679df630dce0f9da0358b5de03
Author: Olga Sharonova <olka@chromium.org>
Date: Thu Aug 31 22:27:26 2017

AudioSystem interface cleanup

Removing const qualifiers from the methods, GetDeviceDescriptions parameter order fixed.

Bug:  740943 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I92778173b140ad65b3bbff7ed88ab772bd326470
Reviewed-on: https://chromium-review.googlesource.com/645636
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Commit-Queue: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499047}
[modify] https://crrev.com/62ff2e00b26efc679df630dce0f9da0358b5de03/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc
[modify] https://crrev.com/62ff2e00b26efc679df630dce0f9da0358b5de03/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_browsertest.cc
[modify] https://crrev.com/62ff2e00b26efc679df630dce0f9da0358b5de03/content/browser/renderer_host/media/media_devices_manager.cc
[modify] https://crrev.com/62ff2e00b26efc679df630dce0f9da0358b5de03/media/audio/audio_system.h
[modify] https://crrev.com/62ff2e00b26efc679df630dce0f9da0358b5de03/media/audio/audio_system_impl.cc
[modify] https://crrev.com/62ff2e00b26efc679df630dce0f9da0358b5de03/media/audio/audio_system_impl.h
[modify] https://crrev.com/62ff2e00b26efc679df630dce0f9da0358b5de03/media/audio/audio_system_impl_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9b1ae1e49d2b7ea85b1c1bad489edbc1e4ed8b7f

commit 9b1ae1e49d2b7ea85b1c1bad489edbc1e4ed8b7f
Author: Olga Sharonova <olka@chromium.org>
Date: Mon Sep 04 12:24:29 2017

Moving reusable AudioSystemImpl logic into AudioSystemHelper

AudioSystemHelper will be reused by mojo implementation.
AudioSystemImpl becomes a trampoline to run AudioSystemHelper on AudopManager thread.

Threading and Trampoline:

At the moment the main thread for AudioSystemImpl is AudioManager thread.
When we have mojo version of AudioSystem, we can choose between:
(a) IO thread as the main thread for AudioSystem implementation on top of Mojo interface.
(b) Each client holds an instance of AudioSystem and accesses the underlying Mojo interface
on whatever thread it likes.

In both cases we don't want the current clients to be aware of AudioManager thread,
since it would complicate transition to mojo. Automatically trampolining calls to
AudioManager thread allows to hide it from the clients.

If we go for option (b) with mojo, we can do a preliminary refactoring to convert
the clients to owning individual AudioSystem(Impl) instances.
Carefully though: AudioSystemImpl is basically stateless,
wheres Mojo-based implementation is bound to the thread it's called on the first time. 

TBR=dalecurtis@chromium.org

Bug:  740943 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I6d1c80f1e9cf697198bd6f43a2bc47b232b69a7f
Reviewed-on: https://chromium-review.googlesource.com/645810
Commit-Queue: Olga Sharonova <olka@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499486}
[modify] https://crrev.com/9b1ae1e49d2b7ea85b1c1bad489edbc1e4ed8b7f/media/audio/BUILD.gn
[modify] https://crrev.com/9b1ae1e49d2b7ea85b1c1bad489edbc1e4ed8b7f/media/audio/audio_manager.h
[modify] https://crrev.com/9b1ae1e49d2b7ea85b1c1bad489edbc1e4ed8b7f/media/audio/audio_system.h
[add] https://crrev.com/9b1ae1e49d2b7ea85b1c1bad489edbc1e4ed8b7f/media/audio/audio_system_helper.cc
[add] https://crrev.com/9b1ae1e49d2b7ea85b1c1bad489edbc1e4ed8b7f/media/audio/audio_system_helper.h
[modify] https://crrev.com/9b1ae1e49d2b7ea85b1c1bad489edbc1e4ed8b7f/media/audio/audio_system_impl.cc
[modify] https://crrev.com/9b1ae1e49d2b7ea85b1c1bad489edbc1e4ed8b7f/media/audio/audio_system_impl.h
[modify] https://crrev.com/9b1ae1e49d2b7ea85b1c1bad489edbc1e4ed8b7f/media/audio/audio_system_impl_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0a579bd1ef4dfeafc6eefcfcdb1673193340c83b

commit 0a579bd1ef4dfeafc6eefcfcdb1673193340c83b
Author: Olga Sharonova <olka@chromium.org>
Date: Thu Sep 07 12:41:50 2017

Switching AudioSystem from invalid AudioParameters to Optional

Ivalid AudioParameters cannot be passed over IPC, and returning fake
parameters masks an absense of a device.

Bug:  740943 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I123506d8d402769be6a39c7ec4abe14d88e1bf0e
Reviewed-on: https://chromium-review.googlesource.com/652549
Commit-Queue: Olga Sharonova <olka@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#500282}
[modify] https://crrev.com/0a579bd1ef4dfeafc6eefcfcdb1673193340c83b/content/browser/renderer_host/media/audio_input_device_manager.cc
[modify] https://crrev.com/0a579bd1ef4dfeafc6eefcfcdb1673193340c83b/content/browser/renderer_host/media/audio_input_device_manager.h
[modify] https://crrev.com/0a579bd1ef4dfeafc6eefcfcdb1673193340c83b/content/browser/renderer_host/media/audio_output_authorization_handler.cc
[modify] https://crrev.com/0a579bd1ef4dfeafc6eefcfcdb1673193340c83b/content/browser/renderer_host/media/audio_output_authorization_handler.h
[modify] https://crrev.com/0a579bd1ef4dfeafc6eefcfcdb1673193340c83b/content/browser/renderer_host/media/media_devices_dispatcher_host.cc
[modify] https://crrev.com/0a579bd1ef4dfeafc6eefcfcdb1673193340c83b/content/browser/renderer_host/media/media_devices_dispatcher_host.h
[modify] https://crrev.com/0a579bd1ef4dfeafc6eefcfcdb1673193340c83b/content/browser/renderer_host/media/media_stream_manager.cc
[modify] https://crrev.com/0a579bd1ef4dfeafc6eefcfcdb1673193340c83b/content/browser/renderer_host/media/media_stream_manager.h
[modify] https://crrev.com/0a579bd1ef4dfeafc6eefcfcdb1673193340c83b/content/browser/speech/speech_recognizer_impl.cc
[modify] https://crrev.com/0a579bd1ef4dfeafc6eefcfcdb1673193340c83b/content/browser/speech/speech_recognizer_impl.h
[modify] https://crrev.com/0a579bd1ef4dfeafc6eefcfcdb1673193340c83b/media/audio/audio_system.h
[modify] https://crrev.com/0a579bd1ef4dfeafc6eefcfcdb1673193340c83b/media/audio/audio_system_helper.cc
[modify] https://crrev.com/0a579bd1ef4dfeafc6eefcfcdb1673193340c83b/media/audio/audio_system_helper.h
[modify] https://crrev.com/0a579bd1ef4dfeafc6eefcfcdb1673193340c83b/media/audio/audio_system_impl_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df

commit e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df
Author: Olga Sharonova <olka@chromium.org>
Date: Wed Sep 13 14:41:47 2017

Use individual instances of AudioSystem for its clients.

Connection to mojo audio system interface is bound to a thread.
If we limit AudioSystem clients to use AudioSystem on a specific thread, it would
mean we are introducing extra thread hops.
So, instead of having a global AudioSystem instance bound to a specific thread, we are
introducing one AudioSystem instance per client, and they are free to use that instance
on any (one) thread they want.


---------One AudioSystem per client:

1) Extentions:

*** ChromeVirtualKeyboardDelegate

*** WebRtcAudioPrivate function (one AudioSystem instance for each function instance)

2) BrowserMainLoop:
Owns AudioSystem instance and passes its pointer to

*** SpeechRecognitionManager 
*** MediaStreamManager (AudioSystem instance is passed around a scary "manager" cluster:
in AudioInputDeviceManager and MediaDevicesmanager members of MediaStreamManager
and also in MediaDevicesDispatcherHost)
*** AudioOutputAuthorizationHandler 


----------AudioSystem::GetInstance factory method is introduced.
It creates AudioSystemImpl instance and passes global AudioManager instance to it.
Later we can make a finch-based decision to create AudioSysteToMojoAdapter there instead;
or just switch to Mojo entirely.


Bug:  740943 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I26a98376fb9edc33aba7c039e392849a460132e4
Reviewed-on: https://chromium-review.googlesource.com/655437
Commit-Queue: Olga Sharonova <olka@chromium.org>
Reviewed-by: Patrik Höglund <phoglund@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501629}
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/chrome/browser/extensions/api/virtual_keyboard_private/chrome_virtual_keyboard_delegate.cc
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/chrome/browser/extensions/api/virtual_keyboard_private/chrome_virtual_keyboard_delegate.h
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.h
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_browsertest.cc
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/content/browser/browser_main_loop.cc
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/content/browser/browser_main_loop.h
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/content/browser/renderer_host/media/audio_input_device_manager_unittest.cc
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/content/browser/renderer_host/media/audio_input_renderer_host_unittest.cc
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/content/browser/renderer_host/media/audio_output_authorization_handler_unittest.cc
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/content/browser/renderer_host/media/audio_renderer_host_unittest.cc
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/content/browser/renderer_host/media/media_devices_dispatcher_host_unittest.cc
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/content/browser/renderer_host/media/media_devices_manager_unittest.cc
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/content/browser/renderer_host/media/media_stream_manager_unittest.cc
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/content/browser/renderer_host/media/video_capture_unittest.cc
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/content/browser/speech/speech_recognition_browsertest.cc
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/content/browser/speech/speech_recognizer_impl_unittest.cc
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/content/browser/webrtc/webrtc_content_browsertest_base.cc
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/content/test/renderer_audio_output_stream_factory_context_impl_unittest.cc
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/media/audio/audio_system.cc
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/media/audio/audio_system.h
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/media/audio/audio_system_helper.cc
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/media/audio/audio_system_helper.h
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/media/audio/audio_system_impl.cc
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/media/audio/audio_system_impl.h
[modify] https://crrev.com/e7b1eb2f4aeb66cf26248f243dd1b608b8c9f9df/media/audio/audio_system_impl_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ba0c9a6b3d090d965f90704d1016cc3d47bcb037

commit ba0c9a6b3d090d965f90704d1016cc3d47bcb037
Author: Olga Sharonova <olka@chromium.org>
Date: Mon Nov 20 12:52:18 2017

Make AudioSystem unit tests reusable for different implementations

Introducing type-parametrized tests verifying that a given AudioSystem implementation is correct.
This will allow to re-use tests for AudioSystem implementation on top of Mojo Audio service.

Bug:  740943 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: If69ee76bce05f0dcfab6abeb31af95959149ca9d
Reviewed-on: https://chromium-review.googlesource.com/707142
Commit-Queue: Olga Sharonova <olka@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517807}
[modify] https://crrev.com/ba0c9a6b3d090d965f90704d1016cc3d47bcb037/media/audio/BUILD.gn
[modify] https://crrev.com/ba0c9a6b3d090d965f90704d1016cc3d47bcb037/media/audio/audio_system_impl_unittest.cc
[add] https://crrev.com/ba0c9a6b3d090d965f90704d1016cc3d47bcb037/media/audio/audio_system_test_util.cc
[add] https://crrev.com/ba0c9a6b3d090d965f90704d1016cc3d47bcb037/media/audio/audio_system_test_util.h

Project Member

Comment 9 by bugdroid1@chromium.org, Jan 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a334d5f07d2fbf48dbcf6933d00027ce16b128a5

commit a334d5f07d2fbf48dbcf6933d00027ce16b128a5
Author: Olga Sharonova <olka@chromium.org>
Date: Thu Jan 11 19:50:12 2018

Basic Audio Service hosting mojom::SystemInfo

In this CL:
* //services/audio infrastructure
* AudioDeviceDescription type mapping - may need to be moved to //media? (Someday we want it all
to be in //audio though.)
* Build files/dependencies - please review vigorously, I'm not sure everything is fine there.
* mojom::SystemInfo interface and implementation on top of AudioManager,
* Service implementation.
* AudioSystemToMojoAdapter: client-side media::AudioSystem implementation on top of
mojom::SystemInfo
* Unit tests.
* A fix for Connector::BindInterface() when there are binder overrides available (see discussion
in PS17)

Upcoming CLs for AudioService infrastructure:
- Run in-process Audio service and
instantiate BrowserMainLoop::audio_system_ with AudioSystemToMojoAdapter, so
that audio system information is served through the service.
- UMA stats for service usage.
- Tear down the service when not in use (service side, client side)
- Out-of-process AudioService implementation + unit tests

Design doc (being updated now):
https://docs.google.com/document/d/1s_Fd1WRDdpb5n6C2MSJjeC3fis6hULZwfKMeDd4K5tI/edit?usp=sharing

Bug:  740943 ,792441
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Iae7f69386a6d2138c69d9566cf41655b57158b7c
Reviewed-on: https://chromium-review.googlesource.com/799791
Commit-Queue: Olga Sharonova <olka@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528714}
[modify] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/media/audio/audio_device_description.h
[modify] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/media/audio/mock_audio_manager.cc
[modify] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/mojo/public/tools/bindings/chromium_bindings_configuration.gni
[modify] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/BUILD.gn
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/BUILD.gn
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/DEPS
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/OWNERS
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/README.md
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/in_process_audio_manager_accessor.cc
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/in_process_audio_manager_accessor.h
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/manifest.json
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/public/cpp/BUILD.gn
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/public/cpp/OWNERS
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/public/cpp/audio_device_description.typemap
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/public/cpp/audio_device_description_struct_traits.cc
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/public/cpp/audio_device_description_struct_traits.h
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/public/cpp/audio_system_to_service_adapter.cc
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/public/cpp/audio_system_to_service_adapter.h
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/public/cpp/typemaps.gni
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/public/interfaces/BUILD.gn
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/public/interfaces/OWNERS
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/public/interfaces/audio_device_description.mojom
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/public/interfaces/constants.mojom
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/public/interfaces/system_info.mojom
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/service.cc
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/service.h
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/system_info.cc
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/system_info.h
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/test/OWNERS
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/test/audio_system_to_service_adapter_test.cc
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/test/in_process_service_test.cc
[add] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/audio/test/service_unittest_manifest.json
[modify] https://crrev.com/a334d5f07d2fbf48dbcf6933d00027ce16b128a5/services/service_manager/public/cpp/connector.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8cd4afa14015d6d8cf2cd90e649f36c1458a4fbf

commit 8cd4afa14015d6d8cf2cd90e649f36c1458a4fbf
Author: Daniel Cheng <dcheng@chromium.org>
Date: Thu Jan 11 20:03:35 2018

Revert "Basic Audio Service hosting mojom::SystemInfo"

This reverts commit a334d5f07d2fbf48dbcf6933d00027ce16b128a5.

Reason for revert: accidentally passed CQ without security review

Original change's description:
> Basic Audio Service hosting mojom::SystemInfo
> 
> In this CL:
> * //services/audio infrastructure
> * AudioDeviceDescription type mapping - may need to be moved to //media? (Someday we want it all
> to be in //audio though.)
> * Build files/dependencies - please review vigorously, I'm not sure everything is fine there.
> * mojom::SystemInfo interface and implementation on top of AudioManager,
> * Service implementation.
> * AudioSystemToMojoAdapter: client-side media::AudioSystem implementation on top of
> mojom::SystemInfo
> * Unit tests.
> * A fix for Connector::BindInterface() when there are binder overrides available (see discussion
> in PS17)
> 
> Upcoming CLs for AudioService infrastructure:
> - Run in-process Audio service and
> instantiate BrowserMainLoop::audio_system_ with AudioSystemToMojoAdapter, so
> that audio system information is served through the service.
> - UMA stats for service usage.
> - Tear down the service when not in use (service side, client side)
> - Out-of-process AudioService implementation + unit tests
> 
> Design doc (being updated now):
> https://docs.google.com/document/d/1s_Fd1WRDdpb5n6C2MSJjeC3fis6hULZwfKMeDd4K5tI/edit?usp=sharing
> 
> Bug:  740943 ,792441
> Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
> Change-Id: Iae7f69386a6d2138c69d9566cf41655b57158b7c
> Reviewed-on: https://chromium-review.googlesource.com/799791
> Commit-Queue: Olga Sharonova <olka@chromium.org>
> Reviewed-by: Ken Rockot <rockot@chromium.org>
> Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
> Reviewed-by: Max Morin <maxmorin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#528714}

TBR=dalecurtis@chromium.org,rockot@chromium.org,olka@chromium.org,maxmorin@chromium.org

Change-Id: Ib5c26464a3dacf0d2a5b3255295ebb268bd6fe20
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  740943 , 792441
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/862107
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528721}
[modify] https://crrev.com/8cd4afa14015d6d8cf2cd90e649f36c1458a4fbf/media/audio/audio_device_description.h
[modify] https://crrev.com/8cd4afa14015d6d8cf2cd90e649f36c1458a4fbf/media/audio/mock_audio_manager.cc
[modify] https://crrev.com/8cd4afa14015d6d8cf2cd90e649f36c1458a4fbf/mojo/public/tools/bindings/chromium_bindings_configuration.gni
[modify] https://crrev.com/8cd4afa14015d6d8cf2cd90e649f36c1458a4fbf/services/BUILD.gn
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/BUILD.gn
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/DEPS
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/OWNERS
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/README.md
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/in_process_audio_manager_accessor.cc
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/in_process_audio_manager_accessor.h
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/manifest.json
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/public/cpp/BUILD.gn
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/public/cpp/OWNERS
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/public/cpp/audio_device_description.typemap
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/public/cpp/audio_device_description_struct_traits.cc
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/public/cpp/audio_device_description_struct_traits.h
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/public/cpp/audio_system_to_service_adapter.cc
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/public/cpp/audio_system_to_service_adapter.h
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/public/cpp/typemaps.gni
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/public/interfaces/BUILD.gn
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/public/interfaces/OWNERS
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/public/interfaces/audio_device_description.mojom
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/public/interfaces/constants.mojom
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/public/interfaces/system_info.mojom
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/service.cc
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/service.h
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/system_info.cc
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/system_info.h
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/test/OWNERS
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/test/audio_system_to_service_adapter_test.cc
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/test/in_process_service_test.cc
[delete] https://crrev.com/e0cf8e1b851449b628d17431943b7a6c6286b5c2/services/audio/test/service_unittest_manifest.json
[modify] https://crrev.com/8cd4afa14015d6d8cf2cd90e649f36c1458a4fbf/services/service_manager/public/cpp/connector.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Jan 16 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3caa45ea110f61dc795782963b829946714ecb38

commit 3caa45ea110f61dc795782963b829946714ecb38
Author: Olga Sharonova <olka@chromium.org>
Date: Tue Jan 16 15:37:59 2018

Reland "Basic Audio Service hosting mojom::SystemInfo":

Now with security review.
Comparing to the original CL (PS1 vs PS5), the changes are cosmetic.
The most noticeable one is that for consistency
SystemInfo::GetDeviceDescriptions(bool for_input) is replaced with
SystemInfo::GetInputDeviceDescriptions() and SystemInfo::GetOutputDeviceDescriptions().

TBR=dalecurtis@chromium.org,rockot@chromium.org

This is a reland of a334d5f07d2fbf48dbcf6933d00027ce16b128a5
Original change's description:
> Basic Audio Service hosting mojom::SystemInfo
>
> In this CL:
> * //services/audio infrastructure
> * AudioDeviceDescription type mapping - may need to be moved to //media? (Someday we want it all
> to be in //audio though.)
> * Build files/dependencies - please review vigorously, I'm not sure everything is fine there.
> * mojom::SystemInfo interface and implementation on top of AudioManager,
> * Service implementation.
> * AudioSystemToMojoAdapter: client-side media::AudioSystem implementation on top of
> mojom::SystemInfo
> * Unit tests.
> * A fix for Connector::BindInterface() when there are binder overrides available (see discussion
> in PS17)
>
> Upcoming CLs for AudioService infrastructure:
> - Run in-process Audio service and
> instantiate BrowserMainLoop::audio_system_ with AudioSystemToMojoAdapter, so
> that audio system information is served through the service.
> - UMA stats for service usage.
> - Tear down the service when not in use (service side, client side)
> - Out-of-process AudioService implementation + unit tests
>
> Design doc (being updated now):
> https://docs.google.com/document/d/1s_Fd1WRDdpb5n6C2MSJjeC3fis6hULZwfKMeDd4K5tI/edit?usp=sharing
>
> Bug:  740943 ,792441
> Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
> Change-Id: Iae7f69386a6d2138c69d9566cf41655b57158b7c
> Reviewed-on: https://chromium-review.googlesource.com/799791
> Commit-Queue: Olga Sharonova <olka@chromium.org>
> Reviewed-by: Ken Rockot <rockot@chromium.org>
> Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
> Reviewed-by: Max Morin <maxmorin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#528714}

Bug:  740943 , 792441
Change-Id: I2316aad895c35636a43ff990d378b6b5d97093a4
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/861790
Commit-Queue: Olga Sharonova <olka@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529427}
[modify] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/media/audio/audio_device_description.h
[modify] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/media/audio/mock_audio_manager.cc
[modify] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/mojo/public/tools/bindings/chromium_bindings_configuration.gni
[modify] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/BUILD.gn
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/BUILD.gn
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/DEPS
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/OWNERS
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/README.md
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/in_process_audio_manager_accessor.cc
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/in_process_audio_manager_accessor.h
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/manifest.json
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/public/cpp/BUILD.gn
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/public/cpp/OWNERS
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/public/cpp/audio_device_description.typemap
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/public/cpp/audio_device_description_struct_traits.cc
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/public/cpp/audio_device_description_struct_traits.h
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/public/cpp/audio_system_to_service_adapter.cc
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/public/cpp/audio_system_to_service_adapter.h
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/public/cpp/typemaps.gni
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/public/interfaces/BUILD.gn
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/public/interfaces/OWNERS
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/public/interfaces/audio_device_description.mojom
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/public/interfaces/constants.mojom
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/public/interfaces/system_info.mojom
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/service.cc
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/service.h
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/system_info.cc
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/system_info.h
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/test/OWNERS
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/test/audio_system_to_service_adapter_test.cc
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/test/in_process_service_test.cc
[add] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/audio/test/service_unittest_manifest.json
[modify] https://crrev.com/3caa45ea110f61dc795782963b829946714ecb38/services/service_manager/public/cpp/connector.cc

Comment 12 by olka@chromium.org, Jan 24 2018

Status: Fixed (was: Started)

Sign in to add a comment