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

Issue 672468 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
OOO Dec 22 - Jan 8
Closed: May 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature

Blocked on:
issue 687981
issue 688383

Blocking:
issue 672469
issue 647200
issue 694197



Sign in to add a comment

Switch AudioManager device info users to callback model

Project Member Reported by olka@chromium.org, Dec 8 2016

Issue description

In preparation for moving AudioManager to a separate process

AudioSystem design:
https://docs.google.com/a/chromium.org/drawings/d/1BS3MIha5qWcDG8MTr4r08eWOoFJZ9IHt5egPoDOaEyE/edit?usp=sharing
 

Comment 1 by olka@chromium.org, Dec 8 2016

Blocking: 647200

Comment 2 by olka@chromium.org, Dec 8 2016

Blocking: 672469
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 27 2017

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

commit 087876b708dc280dcd15ffb69acd6244571527f1
Author: olka <olka@chromium.org>
Date: Fri Jan 27 12:50:12 2017

Removing SpeechRecognitionManager::HasAudioInputDevices() and ShowAudioInputSettings()
- They do not seem to be used in Chromium codebase;
- ShowAudioInputSettings() is not supported on Mac and ChromeOS.

May we break any external embedders by this? If so, how should we negotiate the change?
If not removed, HasAudioInputDevices() should become asynchronous.

BUG= 672468 

Review-Url: https://codereview.chromium.org/2651373002
Cr-Commit-Position: refs/heads/master@{#446653}

[modify] https://crrev.com/087876b708dc280dcd15ffb69acd6244571527f1/content/browser/browser_main_loop.cc
[modify] https://crrev.com/087876b708dc280dcd15ffb69acd6244571527f1/content/browser/speech/speech_recognition_manager_impl.cc
[modify] https://crrev.com/087876b708dc280dcd15ffb69acd6244571527f1/content/browser/speech/speech_recognition_manager_impl.h
[modify] https://crrev.com/087876b708dc280dcd15ffb69acd6244571527f1/content/public/browser/speech_recognition_manager.h
[modify] https://crrev.com/087876b708dc280dcd15ffb69acd6244571527f1/content/public/test/fake_speech_recognition_manager.cc
[modify] https://crrev.com/087876b708dc280dcd15ffb69acd6244571527f1/content/public/test/fake_speech_recognition_manager.h

Comment 5 by olka@chromium.org, Feb 2 2017

Description: Show this description

Comment 6 by olka@chromium.org, Feb 2 2017

Description: Show this description

Comment 7 by olka@chromium.org, Feb 3 2017

Blockedon: 688383

Comment 8 by olka@chromium.org, Feb 3 2017

Blockedon: 687981
Project Member

Comment 9 by bugdroid1@chromium.org, Feb 6 2017

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

commit ef762c904b723cb488bad237300a5976c29327d3
Author: olka <olka@chromium.org>
Date: Mon Feb 06 16:45:16 2017

Switch Speech Recognition to asynchronous callback-based AudioManager interactions.

1) This introduces AudioSystem, which provides an asynchronous interface to AudioManager and eventually will replace AudioManager.
The plan is to gradually populate it with all the required methods and switch all AudioManager clients to it,
in preparation for moving AudioManager functionality to Mojo audio service.

2) SpeechRecognizerImpl now uses this interface, which required adding an extra state to its FSM.

3) SpeechRecognizerImpl unit tests are updated to take into account the new threading model and the new state.

BUG= 687981 , 672468 
CQ_INCLUDE_TRYBOTS=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

Review-Url: https://codereview.chromium.org/2675713002
Cr-Commit-Position: refs/heads/master@{#448273}

[modify] https://crrev.com/ef762c904b723cb488bad237300a5976c29327d3/content/browser/browser_main_loop.cc
[modify] https://crrev.com/ef762c904b723cb488bad237300a5976c29327d3/content/browser/browser_main_loop.h
[modify] https://crrev.com/ef762c904b723cb488bad237300a5976c29327d3/content/browser/speech/speech_recognition_browsertest.cc
[modify] https://crrev.com/ef762c904b723cb488bad237300a5976c29327d3/content/browser/speech/speech_recognition_manager_impl.cc
[modify] https://crrev.com/ef762c904b723cb488bad237300a5976c29327d3/content/browser/speech/speech_recognition_manager_impl.h
[modify] https://crrev.com/ef762c904b723cb488bad237300a5976c29327d3/content/browser/speech/speech_recognizer_impl.cc
[modify] https://crrev.com/ef762c904b723cb488bad237300a5976c29327d3/content/browser/speech/speech_recognizer_impl.h
[modify] https://crrev.com/ef762c904b723cb488bad237300a5976c29327d3/content/browser/speech/speech_recognizer_impl_unittest.cc
[modify] https://crrev.com/ef762c904b723cb488bad237300a5976c29327d3/media/audio/BUILD.gn
[add] https://crrev.com/ef762c904b723cb488bad237300a5976c29327d3/media/audio/audio_system.h
[add] https://crrev.com/ef762c904b723cb488bad237300a5976c29327d3/media/audio/audio_system_impl.cc
[add] https://crrev.com/ef762c904b723cb488bad237300a5976c29327d3/media/audio/audio_system_impl.h
[add] https://crrev.com/ef762c904b723cb488bad237300a5976c29327d3/media/audio/audio_system_impl_unittest.cc
[modify] https://crrev.com/ef762c904b723cb488bad237300a5976c29327d3/media/audio/mock_audio_manager.cc
[modify] https://crrev.com/ef762c904b723cb488bad237300a5976c29327d3/media/audio/mock_audio_manager.h

Comment 10 by olka@chromium.org, Feb 9 2017

Status: Started (was: Assigned)
Project Member

Comment 11 by bugdroid1@chromium.org, Feb 10 2017

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

commit f60887063fcf9c142a478a46f3c052c51256d5ca
Author: olka <olka@chromium.org>
Date: Fri Feb 10 19:52:09 2017

Switching VirtualKeyboardPrivate keyboard config call stack to receive HasInputDevices() responce asyncrhonously

* AudioSystem::HasInputDevices() method added;
* AudioSystem::Get() introduced to access AudioSystem instance outside of content/browser and on any browser thread (approach is similar to AudioManager one).
* HasInputDevices call is moved from ui/keyboard/keyboard_util to ChromeVirtualKeyboardDelegate;
* ChromeVirtualKeyboardDelegate::GetKeyboardConfig() replies asynchronously now, as well as VirtualKeyboardPrivateGetKeyboardConfigFunction.
* Drive-by fix in SpeechRecognition browser tests (MocKAudioManager was not being deleted there because its message loop was shut down during unique_ptr destruction.)

TESTING=manually tested virtual keyboard on a chromebook.
BUG= 688383 ,  672468 
CQ_INCLUDE_TRYBOTS=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

Review-Url: https://codereview.chromium.org/2687853002
Cr-Commit-Position: refs/heads/master@{#449704}

[add] https://crrev.com/f60887063fcf9c142a478a46f3c052c51256d5ca/chrome/browser/extensions/api/virtual_keyboard_private/DEPS
[modify] https://crrev.com/f60887063fcf9c142a478a46f3c052c51256d5ca/chrome/browser/extensions/api/virtual_keyboard_private/chrome_virtual_keyboard_delegate.cc
[modify] https://crrev.com/f60887063fcf9c142a478a46f3c052c51256d5ca/chrome/browser/extensions/api/virtual_keyboard_private/chrome_virtual_keyboard_delegate.h
[modify] https://crrev.com/f60887063fcf9c142a478a46f3c052c51256d5ca/content/browser/speech/speech_recognition_browsertest.cc
[modify] https://crrev.com/f60887063fcf9c142a478a46f3c052c51256d5ca/extensions/browser/api/virtual_keyboard_private/virtual_keyboard_delegate.h
[modify] https://crrev.com/f60887063fcf9c142a478a46f3c052c51256d5ca/extensions/browser/api/virtual_keyboard_private/virtual_keyboard_private_api.cc
[modify] https://crrev.com/f60887063fcf9c142a478a46f3c052c51256d5ca/extensions/browser/api/virtual_keyboard_private/virtual_keyboard_private_api.h
[modify] https://crrev.com/f60887063fcf9c142a478a46f3c052c51256d5ca/media/audio/BUILD.gn
[add] https://crrev.com/f60887063fcf9c142a478a46f3c052c51256d5ca/media/audio/audio_system.cc
[modify] https://crrev.com/f60887063fcf9c142a478a46f3c052c51256d5ca/media/audio/audio_system.h
[modify] https://crrev.com/f60887063fcf9c142a478a46f3c052c51256d5ca/media/audio/audio_system_impl.cc
[modify] https://crrev.com/f60887063fcf9c142a478a46f3c052c51256d5ca/media/audio/audio_system_impl.h
[modify] https://crrev.com/f60887063fcf9c142a478a46f3c052c51256d5ca/media/audio/audio_system_impl_unittest.cc
[modify] https://crrev.com/f60887063fcf9c142a478a46f3c052c51256d5ca/ui/keyboard/BUILD.gn
[modify] https://crrev.com/f60887063fcf9c142a478a46f3c052c51256d5ca/ui/keyboard/keyboard_util.cc
[modify] https://crrev.com/f60887063fcf9c142a478a46f3c052c51256d5ca/ui/keyboard/keyboard_util.h

Project Member

Comment 12 by bugdroid1@chromium.org, Feb 14 2017

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

commit e7803cf27acc67ddc9625e0fc3bfd18541786132
Author: olka <olka@chromium.org>
Date: Tue Feb 14 10:23:11 2017

Rmoving the notion of task runner from MediaStreamProvider interface.

AudioInputDeviceManager (which is MediaStreamProvider) does not require a task runner,
since it gets one from AudioManager. So, removing that from the base class interface to
simplify further refactoring of AudioInputDeviceManager.

BUG= 672468 

Review-Url: https://codereview.chromium.org/2696533002
Cr-Commit-Position: refs/heads/master@{#450311}

[modify] https://crrev.com/e7803cf27acc67ddc9625e0fc3bfd18541786132/content/browser/renderer_host/media/audio_input_device_manager.cc
[modify] https://crrev.com/e7803cf27acc67ddc9625e0fc3bfd18541786132/content/browser/renderer_host/media/audio_input_device_manager.h
[modify] https://crrev.com/e7803cf27acc67ddc9625e0fc3bfd18541786132/content/browser/renderer_host/media/audio_input_device_manager_unittest.cc
[modify] https://crrev.com/e7803cf27acc67ddc9625e0fc3bfd18541786132/content/browser/renderer_host/media/media_devices_manager_unittest.cc
[modify] https://crrev.com/e7803cf27acc67ddc9625e0fc3bfd18541786132/content/browser/renderer_host/media/media_stream_manager.cc
[modify] https://crrev.com/e7803cf27acc67ddc9625e0fc3bfd18541786132/content/browser/renderer_host/media/media_stream_provider.h
[modify] https://crrev.com/e7803cf27acc67ddc9625e0fc3bfd18541786132/content/browser/renderer_host/media/video_capture_manager.cc
[modify] https://crrev.com/e7803cf27acc67ddc9625e0fc3bfd18541786132/content/browser/renderer_host/media/video_capture_manager.h
[modify] https://crrev.com/e7803cf27acc67ddc9625e0fc3bfd18541786132/content/browser/renderer_host/media/video_capture_manager_unittest.cc

Comment 13 by olka@chromium.org, Feb 14 2017

Cc: ossu@chromium.org solenberg@chromium.org maxmorin@chromium.org
Project Member

Comment 14 by bugdroid1@chromium.org, Feb 16 2017

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

commit 04ff52bb66284467ccb43d90800013b89ee8db75
Author: olka <olka@chromium.org>
Date: Thu Feb 16 12:36:17 2017

Switching AudioOutputAuthorizationHandler from using AudioManager interface to AudioSystem one.

BUG= 672468 
CQ_INCLUDE_TRYBOTS=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

Review-Url: https://codereview.chromium.org/2692203003
Cr-Commit-Position: refs/heads/master@{#450939}

[modify] https://crrev.com/04ff52bb66284467ccb43d90800013b89ee8db75/content/browser/renderer_host/media/audio_output_authorization_handler.cc
[modify] https://crrev.com/04ff52bb66284467ccb43d90800013b89ee8db75/content/browser/renderer_host/media/audio_output_authorization_handler.h
[modify] https://crrev.com/04ff52bb66284467ccb43d90800013b89ee8db75/content/browser/renderer_host/media/audio_output_authorization_handler_unittest.cc
[modify] https://crrev.com/04ff52bb66284467ccb43d90800013b89ee8db75/content/browser/renderer_host/media/audio_renderer_host.cc
[modify] https://crrev.com/04ff52bb66284467ccb43d90800013b89ee8db75/content/browser/renderer_host/media/audio_renderer_host.h
[modify] https://crrev.com/04ff52bb66284467ccb43d90800013b89ee8db75/content/browser/renderer_host/media/audio_renderer_host_unittest.cc
[modify] https://crrev.com/04ff52bb66284467ccb43d90800013b89ee8db75/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/04ff52bb66284467ccb43d90800013b89ee8db75/media/audio/audio_system.h
[modify] https://crrev.com/04ff52bb66284467ccb43d90800013b89ee8db75/media/audio/audio_system_impl.cc
[modify] https://crrev.com/04ff52bb66284467ccb43d90800013b89ee8db75/media/audio/audio_system_impl.h
[modify] https://crrev.com/04ff52bb66284467ccb43d90800013b89ee8db75/media/audio/audio_system_impl_unittest.cc
[modify] https://crrev.com/04ff52bb66284467ccb43d90800013b89ee8db75/media/audio/mock_audio_manager.cc
[modify] https://crrev.com/04ff52bb66284467ccb43d90800013b89ee8db75/media/audio/mock_audio_manager.h

Project Member

Comment 15 by bugdroid1@chromium.org, Feb 16 2017

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

commit 156ba9177eb266804d52f98c1e17033705645b5d
Author: perkj <perkj@chromium.org>
Date: Thu Feb 16 14:22:12 2017

Revert of Switching AudioOutputAuthorizationHandler from using AudioManager interface to AudioSystem one. (patchset #2 id:20001 of https://codereview.chromium.org/2692203003/ )

Reason for revert:
Seems to break PPAPINaClPNaClTest.MediaStreamAudioTrack on Mac.
https://build.chromium.org/p/chromium.mac/builders/Mac10.10%20Tests/builds/13657

Original issue's description:
> Switching AudioOutputAuthorizationHandler from using AudioManager interface to AudioSystem one.
>
> BUG= 672468 
> CQ_INCLUDE_TRYBOTS=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
>
> Review-Url: https://codereview.chromium.org/2692203003
> Cr-Commit-Position: refs/heads/master@{#450939}
> Committed: https://chromium.googlesource.com/chromium/src/+/04ff52bb66284467ccb43d90800013b89ee8db75

TBR=tommi@chromium.org,guidou@chromium.org,avi@chromium.org,olka@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 672468 

Review-Url: https://codereview.chromium.org/2696253004
Cr-Commit-Position: refs/heads/master@{#450952}

[modify] https://crrev.com/156ba9177eb266804d52f98c1e17033705645b5d/content/browser/renderer_host/media/audio_output_authorization_handler.cc
[modify] https://crrev.com/156ba9177eb266804d52f98c1e17033705645b5d/content/browser/renderer_host/media/audio_output_authorization_handler.h
[modify] https://crrev.com/156ba9177eb266804d52f98c1e17033705645b5d/content/browser/renderer_host/media/audio_output_authorization_handler_unittest.cc
[modify] https://crrev.com/156ba9177eb266804d52f98c1e17033705645b5d/content/browser/renderer_host/media/audio_renderer_host.cc
[modify] https://crrev.com/156ba9177eb266804d52f98c1e17033705645b5d/content/browser/renderer_host/media/audio_renderer_host.h
[modify] https://crrev.com/156ba9177eb266804d52f98c1e17033705645b5d/content/browser/renderer_host/media/audio_renderer_host_unittest.cc
[modify] https://crrev.com/156ba9177eb266804d52f98c1e17033705645b5d/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/156ba9177eb266804d52f98c1e17033705645b5d/media/audio/audio_system.h
[modify] https://crrev.com/156ba9177eb266804d52f98c1e17033705645b5d/media/audio/audio_system_impl.cc
[modify] https://crrev.com/156ba9177eb266804d52f98c1e17033705645b5d/media/audio/audio_system_impl.h
[modify] https://crrev.com/156ba9177eb266804d52f98c1e17033705645b5d/media/audio/audio_system_impl_unittest.cc
[modify] https://crrev.com/156ba9177eb266804d52f98c1e17033705645b5d/media/audio/mock_audio_manager.cc
[modify] https://crrev.com/156ba9177eb266804d52f98c1e17033705645b5d/media/audio/mock_audio_manager.h

Comment 16 by olka@chromium.org, Feb 20 2017

Blocking: 694197
Project Member

Comment 17 by bugdroid1@chromium.org, Mar 13 2017

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

commit f85f4d03170997f6d8fbf843e647e58a3923fd9e
Author: olka <olka@chromium.org>
Date: Mon Mar 13 20:27:29 2017

CL https://codereview.chromium.org/2692203003/ makes the code to use UnavailableDeviceParameters in case there are no real output devices available.

WebAudio is unhappy when 100 ms buffer size is used for audio sink.
(CL got reverted because of that). Switching to 10 ms.

BUG= 672468 ,701000

Review-Url: https://codereview.chromium.org/2738403002
Cr-Commit-Position: refs/heads/master@{#456464}

[modify] https://crrev.com/f85f4d03170997f6d8fbf843e647e58a3923fd9e/media/base/audio_parameters.cc

Project Member

Comment 18 by bugdroid1@chromium.org, Mar 14 2017

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

commit a39bab40d833b265d8ea764001ec3f872c49b8d2
Author: olka <olka@chromium.org>
Date: Tue Mar 14 17:10:15 2017

Reland of Switching AudioOutputAuthorizationHandler from using AudioManager interface to AudioSystem one. (patchset #1 id:1 of https://codereview.chromium.org/2696253004/ )

Reason for revert:
Relanding the original CL after landing the work around (https://codereview.chromium.org/2738403002/)

Original issue's description:
> Revert of Switching AudioOutputAuthorizationHandler from using AudioManager interface to AudioSystem one. (patchset #2 id:20001 of https://codereview.chromium.org/2692203003/ )
>
> Reason for revert:
> Seems to break PPAPINaClPNaClTest.MediaStreamAudioTrack on Mac.
> https://build.chromium.org/p/chromium.mac/builders/Mac10.10%20Tests/builds/13657
>
> Original issue's description:
> > Switching AudioOutputAuthorizationHandler from using AudioManager interface to AudioSystem one.
> >
> > BUG= 672468 
> > CQ_INCLUDE_TRYBOTS=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
> >
> > Review-Url: https://codereview.chromium.org/2692203003
> > Cr-Commit-Position: refs/heads/master@{#450939}
> > Committed: https://chromium.googlesource.com/chromium/src/+/04ff52bb66284467ccb43d90800013b89ee8db75
>
> TBR=tommi@chromium.org,guidou@chromium.org,avi@chromium.org,olka@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= 672468 
>
> Review-Url: https://codereview.chromium.org/2696253004
> Cr-Commit-Position: refs/heads/master@{#450952}
> Committed: https://chromium.googlesource.com/chromium/src/+/156ba9177eb266804d52f98c1e17033705645b5d

TBR=tommi@chromium.org,guidou@chromium.org,avi@chromium.org,perkj@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 672468 

Review-Url: https://codereview.chromium.org/2748873003
Cr-Commit-Position: refs/heads/master@{#456742}

[modify] https://crrev.com/a39bab40d833b265d8ea764001ec3f872c49b8d2/content/browser/renderer_host/media/audio_output_authorization_handler.cc
[modify] https://crrev.com/a39bab40d833b265d8ea764001ec3f872c49b8d2/content/browser/renderer_host/media/audio_output_authorization_handler.h
[modify] https://crrev.com/a39bab40d833b265d8ea764001ec3f872c49b8d2/content/browser/renderer_host/media/audio_output_authorization_handler_unittest.cc
[modify] https://crrev.com/a39bab40d833b265d8ea764001ec3f872c49b8d2/content/browser/renderer_host/media/audio_renderer_host.cc
[modify] https://crrev.com/a39bab40d833b265d8ea764001ec3f872c49b8d2/content/browser/renderer_host/media/audio_renderer_host.h
[modify] https://crrev.com/a39bab40d833b265d8ea764001ec3f872c49b8d2/content/browser/renderer_host/media/audio_renderer_host_unittest.cc
[modify] https://crrev.com/a39bab40d833b265d8ea764001ec3f872c49b8d2/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/a39bab40d833b265d8ea764001ec3f872c49b8d2/media/audio/audio_system.h
[modify] https://crrev.com/a39bab40d833b265d8ea764001ec3f872c49b8d2/media/audio/audio_system_impl.cc
[modify] https://crrev.com/a39bab40d833b265d8ea764001ec3f872c49b8d2/media/audio/audio_system_impl.h
[modify] https://crrev.com/a39bab40d833b265d8ea764001ec3f872c49b8d2/media/audio/audio_system_impl_unittest.cc
[modify] https://crrev.com/a39bab40d833b265d8ea764001ec3f872c49b8d2/media/audio/mock_audio_manager.cc
[modify] https://crrev.com/a39bab40d833b265d8ea764001ec3f872c49b8d2/media/audio/mock_audio_manager.h

Project Member

Comment 19 by bugdroid1@chromium.org, Mar 17 2017

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

commit e309da24090a50c7252333bc2c844d7089bf17ce
Author: olka <olka@chromium.org>
Date: Fri Mar 17 17:38:11 2017

Switching MediaStreamManager from using AudioManager to AudioSystem

MediaDevicesManager and AudioInputDeviceManager will be addressed in separate follow-up CLs, to keep it simpler.

BUG= 672468 
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

Review-Url: https://codereview.chromium.org/2697033006
Cr-Commit-Position: refs/heads/master@{#457807}

[modify] https://crrev.com/e309da24090a50c7252333bc2c844d7089bf17ce/content/browser/browser_main_loop.cc
[modify] https://crrev.com/e309da24090a50c7252333bc2c844d7089bf17ce/content/browser/renderer_host/media/audio_input_renderer_host_unittest.cc
[modify] https://crrev.com/e309da24090a50c7252333bc2c844d7089bf17ce/content/browser/renderer_host/media/audio_output_authorization_handler_unittest.cc
[modify] https://crrev.com/e309da24090a50c7252333bc2c844d7089bf17ce/content/browser/renderer_host/media/audio_output_delegate_impl_unittest.cc
[modify] https://crrev.com/e309da24090a50c7252333bc2c844d7089bf17ce/content/browser/renderer_host/media/audio_renderer_host_unittest.cc
[modify] https://crrev.com/e309da24090a50c7252333bc2c844d7089bf17ce/content/browser/renderer_host/media/media_devices_dispatcher_host_unittest.cc
[modify] https://crrev.com/e309da24090a50c7252333bc2c844d7089bf17ce/content/browser/renderer_host/media/media_stream_dispatcher_host_unittest.cc
[modify] https://crrev.com/e309da24090a50c7252333bc2c844d7089bf17ce/content/browser/renderer_host/media/media_stream_manager.cc
[modify] https://crrev.com/e309da24090a50c7252333bc2c844d7089bf17ce/content/browser/renderer_host/media/media_stream_manager.h
[modify] https://crrev.com/e309da24090a50c7252333bc2c844d7089bf17ce/content/browser/renderer_host/media/media_stream_manager_unittest.cc
[modify] https://crrev.com/e309da24090a50c7252333bc2c844d7089bf17ce/content/browser/renderer_host/media/video_capture_unittest.cc
[modify] https://crrev.com/e309da24090a50c7252333bc2c844d7089bf17ce/media/audio/audio_system.h
[modify] https://crrev.com/e309da24090a50c7252333bc2c844d7089bf17ce/media/audio/audio_system_impl.h

Project Member

Comment 20 by bugdroid1@chromium.org, Mar 21 2017

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

commit 4e2655ac5428e02f7aeb2051b0b43f1c961cbe95
Author: olka <olka@chromium.org>
Date: Tue Mar 21 18:17:15 2017

Switching MediaStreamsManager from using AudioManager interface to AudioSystem one.

BUG= 672468 
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

Review-Url: https://codereview.chromium.org/2761273003
Cr-Commit-Position: refs/heads/master@{#458476}

[modify] https://crrev.com/4e2655ac5428e02f7aeb2051b0b43f1c961cbe95/content/browser/renderer_host/media/media_devices_manager.cc
[modify] https://crrev.com/4e2655ac5428e02f7aeb2051b0b43f1c961cbe95/content/browser/renderer_host/media/media_devices_manager.h
[modify] https://crrev.com/4e2655ac5428e02f7aeb2051b0b43f1c961cbe95/content/browser/renderer_host/media/media_devices_manager_unittest.cc
[modify] https://crrev.com/4e2655ac5428e02f7aeb2051b0b43f1c961cbe95/content/browser/renderer_host/media/media_stream_manager.cc
[modify] https://crrev.com/4e2655ac5428e02f7aeb2051b0b43f1c961cbe95/media/audio/audio_system.h
[modify] https://crrev.com/4e2655ac5428e02f7aeb2051b0b43f1c961cbe95/media/audio/audio_system_impl.cc
[modify] https://crrev.com/4e2655ac5428e02f7aeb2051b0b43f1c961cbe95/media/audio/audio_system_impl.h
[modify] https://crrev.com/4e2655ac5428e02f7aeb2051b0b43f1c961cbe95/media/audio/audio_system_impl_unittest.cc
[modify] https://crrev.com/4e2655ac5428e02f7aeb2051b0b43f1c961cbe95/media/audio/mock_audio_manager.cc
[modify] https://crrev.com/4e2655ac5428e02f7aeb2051b0b43f1c961cbe95/media/audio/mock_audio_manager.h

Project Member

Comment 21 by bugdroid1@chromium.org, Mar 30 2017

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

commit b6082fccb8e9fc2f21871508bdcf79ca567326ad
Author: olka <olka@chromium.org>
Date: Thu Mar 30 15:24:10 2017

Removing AudioSystem::GetAudioManager() usage from speech recognition.

After https://codereview.chromium.org/2763383002/ lands, AudioSystem::GetAudioManager() will be removed: it's only purpose was to assist in gradually switching clients from AudioManager to AudioSystem interface.
However, it leaked a bit in speech recognition, so - fixing it.

The plan is:

1) After the last client (WebRTC audio private API) is switched to AudioSystem interface, all the related AudioManager methods will be made protected to be used by AudioSystem only; so essentially AudioManager interface will become a factory for audio streams.

2) Further, all audio stream clients will be switched to asynchronous stream creation interface, and then to Mojo.

BUG= 672468 

Review-Url: https://codereview.chromium.org/2785463005
Cr-Commit-Position: refs/heads/master@{#460763}

[modify] https://crrev.com/b6082fccb8e9fc2f21871508bdcf79ca567326ad/content/browser/browser_main_loop.cc
[modify] https://crrev.com/b6082fccb8e9fc2f21871508bdcf79ca567326ad/content/browser/speech/speech_recognition_browsertest.cc
[modify] https://crrev.com/b6082fccb8e9fc2f21871508bdcf79ca567326ad/content/browser/speech/speech_recognition_manager_impl.cc
[modify] https://crrev.com/b6082fccb8e9fc2f21871508bdcf79ca567326ad/content/browser/speech/speech_recognition_manager_impl.h
[modify] https://crrev.com/b6082fccb8e9fc2f21871508bdcf79ca567326ad/content/browser/speech/speech_recognizer_impl.cc
[modify] https://crrev.com/b6082fccb8e9fc2f21871508bdcf79ca567326ad/content/browser/speech/speech_recognizer_impl.h
[modify] https://crrev.com/b6082fccb8e9fc2f21871508bdcf79ca567326ad/content/browser/speech/speech_recognizer_impl_unittest.cc

Project Member

Comment 22 by bugdroid1@chromium.org, Apr 6 2017

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

commit ebd3c9846d8a2340d947fae30220d607d95eae7c
Author: olka <olka@chromium.org>
Date: Thu Apr 06 14:42:59 2017

Remove AudioManager::GetDefaultOutputStreamParameters() calls from AudioOutputDelegate unit tests.

BUG= 672468 

Review-Url: https://codereview.chromium.org/2802933003
Cr-Commit-Position: refs/heads/master@{#462468}

[modify] https://crrev.com/ebd3c9846d8a2340d947fae30220d607d95eae7c/content/browser/renderer_host/media/audio_output_delegate_impl_unittest.cc

Project Member

Comment 23 by bugdroid1@chromium.org, Apr 7 2017

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

commit e1920d9dc4d5cb935cbf0f27c0e5beb06c8a0c25
Author: olka <olka@chromium.org>
Date: Fri Apr 07 14:52:31 2017

Switching WebRTC browser test from AudioManager::HasAudioOutputDevices() to AudioSystem interface.

BUG= 672468 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2801983003
Cr-Commit-Position: refs/heads/master@{#462859}

[modify] https://crrev.com/e1920d9dc4d5cb935cbf0f27c0e5beb06c8a0c25/content/browser/webrtc/webrtc_audio_browsertest.cc
[modify] https://crrev.com/e1920d9dc4d5cb935cbf0f27c0e5beb06c8a0c25/content/browser/webrtc/webrtc_audio_debug_recordings_browsertest.cc
[modify] https://crrev.com/e1920d9dc4d5cb935cbf0f27c0e5beb06c8a0c25/content/browser/webrtc/webrtc_content_browsertest_base.cc
[modify] https://crrev.com/e1920d9dc4d5cb935cbf0f27c0e5beb06c8a0c25/content/browser/webrtc/webrtc_content_browsertest_base.h
[modify] https://crrev.com/e1920d9dc4d5cb935cbf0f27c0e5beb06c8a0c25/media/audio/audio_system.h
[modify] https://crrev.com/e1920d9dc4d5cb935cbf0f27c0e5beb06c8a0c25/media/audio/audio_system_impl.cc
[modify] https://crrev.com/e1920d9dc4d5cb935cbf0f27c0e5beb06c8a0c25/media/audio/audio_system_impl.h
[modify] https://crrev.com/e1920d9dc4d5cb935cbf0f27c0e5beb06c8a0c25/media/audio/audio_system_impl_unittest.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Apr 11 2017

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

commit 3dfb4cfaa34efcf58fbf79206197e94402266f18
Author: olka <olka@chromium.org>
Date: Tue Apr 11 15:43:28 2017

Switching AudioInputDeviceManager from using AudioManager interface to AudioSystem one.

1) AudioSystem::GetAssociatedOutputDeviceID() is added + unit tests for it;

2) DeviceOpener helper for AudioInputDeviceManager is introduced, which operates on audio thread and navigates through a sequence of calls to AudioSystem, to receive the reply asynchronously and make the next request until the whole device opening sequence is completed.

3) I chose to make DeviceOpener to work on audio thread to reduce the number of thread hops. But it comes with a cost of accessing AudioSystem on audio thread. To make that work correctly during shutdown, weak pointer access to AudioSystem is added + deleter which destroys it on audio thread.
Alternatively, we can make all the requests to AudioSystem from IO thread - then we don't need to care about AudioSystem lifetime, but will be hopping between audio thread and IO several times.

4) AudioInputDeviceManager unit tests are updated to take threading into account.

5) Now we have multiple post-reply events in the device opening sequence instead of one synchronous call. "Media.AudioInputDeviceManager.OpenOnDeviceThreadTime" metric already exists to measure the time of device opening. I reused it, so we'll be able to measure the effect of the change.

6) In the future this sequence of posts and replies will be a series of Mojo IPC exchanges. So we may reconsider the interface later to minimize IPC hops if the metric is seriously affected after switching to Mojo.

BUG= 672468 
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

TBR=alokp@chromium.org

Review-Url: https://codereview.chromium.org/2763383002
Cr-Commit-Position: refs/heads/master@{#463640}

[modify] https://crrev.com/3dfb4cfaa34efcf58fbf79206197e94402266f18/content/browser/browser_main_loop.h
[modify] https://crrev.com/3dfb4cfaa34efcf58fbf79206197e94402266f18/content/browser/renderer_host/media/audio_input_device_manager.cc
[modify] https://crrev.com/3dfb4cfaa34efcf58fbf79206197e94402266f18/content/browser/renderer_host/media/audio_input_device_manager.h
[modify] https://crrev.com/3dfb4cfaa34efcf58fbf79206197e94402266f18/content/browser/renderer_host/media/audio_input_device_manager_unittest.cc
[modify] https://crrev.com/3dfb4cfaa34efcf58fbf79206197e94402266f18/content/browser/renderer_host/media/media_stream_manager.cc
[modify] https://crrev.com/3dfb4cfaa34efcf58fbf79206197e94402266f18/content/browser/speech/speech_recognizer_impl_unittest.cc
[modify] https://crrev.com/3dfb4cfaa34efcf58fbf79206197e94402266f18/media/audio/audio_manager.cc
[modify] https://crrev.com/3dfb4cfaa34efcf58fbf79206197e94402266f18/media/audio/audio_system.h
[modify] https://crrev.com/3dfb4cfaa34efcf58fbf79206197e94402266f18/media/audio/audio_system_impl.cc
[modify] https://crrev.com/3dfb4cfaa34efcf58fbf79206197e94402266f18/media/audio/audio_system_impl.h
[modify] https://crrev.com/3dfb4cfaa34efcf58fbf79206197e94402266f18/media/audio/audio_system_impl_unittest.cc
[modify] https://crrev.com/3dfb4cfaa34efcf58fbf79206197e94402266f18/media/audio/mock_audio_manager.cc
[modify] https://crrev.com/3dfb4cfaa34efcf58fbf79206197e94402266f18/media/audio/mock_audio_manager.h

Project Member

Comment 25 by bugdroid1@chromium.org, Apr 21 2017

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

commit bc189a38337d30fd6a5450658ab3547126d97485
Author: olka <olka@chromium.org>
Date: Fri Apr 21 11:19:43 2017

WebRTC Audio private API: removing WebRtcAudioPrivate(Set/Get)ActiveSinkFunction
together with GetOutputControllers() logic from renderer hosts.

They are meant to be replaced with HTMLMediaElement's sinkId and setSinkId()

*PENDING on external change - do not submit yet*

BUG= 647185 , 672468 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:closure_compilation;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

Review-Url: https://codereview.chromium.org/2784563003
Cr-Commit-Position: refs/heads/master@{#466312}

[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.cc
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_api.h
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/chrome/browser/extensions/api/webrtc_audio_private/webrtc_audio_private_browsertest.cc
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/chrome/browser/resources/hangout_services/manifest.json
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/chrome/browser/resources/hangout_services/thunk.js
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/chrome/common/extensions/api/webrtc_audio_private.idl
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/chrome/test/data/extensions/hangout_services_test.html
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/chrome/test/data/extensions/hangout_services_test.js
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/content/browser/renderer_host/media/audio_output_delegate_impl.cc
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/content/browser/renderer_host/media/audio_output_delegate_impl.h
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/content/browser/renderer_host/media/audio_output_delegate_impl_unittest.cc
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/content/browser/renderer_host/media/audio_renderer_host.cc
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/content/browser/renderer_host/media/audio_renderer_host.h
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/content/public/browser/render_process_host.h
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/content/public/test/mock_render_process_host.h
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/extensions/browser/extension_function_histogram_value.h
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/media/audio/audio_output_controller.cc
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/media/audio/audio_output_controller.h
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/media/audio/audio_output_controller_unittest.cc
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/media/audio/audio_output_delegate.h
[modify] https://crrev.com/bc189a38337d30fd6a5450658ab3547126d97485/tools/metrics/histograms/histograms.xml

Project Member

Comment 26 by bugdroid1@chromium.org, Apr 21 2017

Project Member

Comment 27 by bugdroid1@chromium.org, Apr 21 2017

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

commit 402945dae4329cd6e3b2cbeaaf8846818ca92ec5
Author: olka <olka@chromium.org>
Date: Fri Apr 21 14:02:21 2017

AudioSystemImpl binding cleanup

BUG= 672468 
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

Review-Url: https://codereview.chromium.org/2828143006
Cr-Commit-Position: refs/heads/master@{#466324}

[modify] https://crrev.com/402945dae4329cd6e3b2cbeaaf8846818ca92ec5/media/audio/audio_system_impl.cc

Project Member

Comment 28 by bugdroid1@chromium.org, May 3 2017

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

commit 0abd2199c67e65774e8fff05a31068d17b2ae805
Author: olka <olka@chromium.org>
Date: Wed May 03 16:18:31 2017

Removing public access to AudioManager device info interface.

With audio moving to a separate process, device info access is allowed through AudioSystem interface only.
We want to avoid AudioManager device info interface leakage.

* AudioManager device info methods moved to protected.
* AudioSystem becomes AudioManager's friend.
* For the unit tests of the functionality moving to audio process (media unit tests):
AudioDeviceInfoAccessorForTests class is introduced, which is AudioManager's friend and is compiled only for media_unittests target.

BUG= 672468 
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

Review-Url: https://codereview.chromium.org/2799363005
Cr-Commit-Position: refs/heads/master@{#468998}

[modify] https://crrev.com/0abd2199c67e65774e8fff05a31068d17b2ae805/content/browser/renderer_host/media/renderer_audio_output_stream_factory_context_impl_unittest.cc
[modify] https://crrev.com/0abd2199c67e65774e8fff05a31068d17b2ae805/media/audio/BUILD.gn
[modify] https://crrev.com/0abd2199c67e65774e8fff05a31068d17b2ae805/media/audio/android/audio_android_unittest.cc
[add] https://crrev.com/0abd2199c67e65774e8fff05a31068d17b2ae805/media/audio/audio_device_info_accessor_for_tests.cc
[add] https://crrev.com/0abd2199c67e65774e8fff05a31068d17b2ae805/media/audio/audio_device_info_accessor_for_tests.h
[modify] https://crrev.com/0abd2199c67e65774e8fff05a31068d17b2ae805/media/audio/audio_input_unittest.cc
[modify] https://crrev.com/0abd2199c67e65774e8fff05a31068d17b2ae805/media/audio/audio_low_latency_input_output_unittest.cc
[modify] https://crrev.com/0abd2199c67e65774e8fff05a31068d17b2ae805/media/audio/audio_manager.h
[modify] https://crrev.com/0abd2199c67e65774e8fff05a31068d17b2ae805/media/audio/audio_manager_base.h
[modify] https://crrev.com/0abd2199c67e65774e8fff05a31068d17b2ae805/media/audio/audio_manager_unittest.cc
[modify] https://crrev.com/0abd2199c67e65774e8fff05a31068d17b2ae805/media/audio/audio_output_proxy_unittest.cc
[modify] https://crrev.com/0abd2199c67e65774e8fff05a31068d17b2ae805/media/audio/audio_system_impl.cc
[modify] https://crrev.com/0abd2199c67e65774e8fff05a31068d17b2ae805/media/audio/audio_system_impl.h
[modify] https://crrev.com/0abd2199c67e65774e8fff05a31068d17b2ae805/media/audio/mac/audio_auhal_mac_unittest.cc
[modify] https://crrev.com/0abd2199c67e65774e8fff05a31068d17b2ae805/media/audio/mac/audio_low_latency_input_mac_unittest.cc
[modify] https://crrev.com/0abd2199c67e65774e8fff05a31068d17b2ae805/media/audio/mock_audio_manager.h
[modify] https://crrev.com/0abd2199c67e65774e8fff05a31068d17b2ae805/media/audio/win/audio_low_latency_input_win_unittest.cc
[modify] https://crrev.com/0abd2199c67e65774e8fff05a31068d17b2ae805/media/audio/win/audio_low_latency_output_win_unittest.cc
[modify] https://crrev.com/0abd2199c67e65774e8fff05a31068d17b2ae805/media/audio/win/audio_output_win_unittest.cc

Comment 29 by olka@chromium.org, May 4 2017

Labels: M-60
Status: Fixed (was: Started)

Sign in to add a comment