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

Issue 834702 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 18
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocked on:
issue 834704

Blocking:
issue 834674
issue 851692


Participants' hotlists:
AudioService-FixIt


Sign in to add a comment

Switch chromeos::assistant::PlatformAudioInputHost() to Audio service API

Project Member Reported by olka@chromium.org, Apr 19 2018

Issue description

Currently it accesses media::AudioManager through AudioManager::Get().
AudioManager is being moved to the Audio service (which is providing Mojo interfaces to platform audio) and should not be accessed directly. Furthermore, its lifetime in the browser process is not guaranteed.
 

Comment 1 by olka@chromium.org, Apr 19 2018

Blocking: 834674

Comment 2 by olka@chromium.org, Apr 19 2018

Blockedon: 834704

Comment 3 by olka@chromium.org, Apr 19 2018

Status: Assigned (was: Untriaged)
Cc: xiaoh...@chromium.org
Owner: muyuanli@chromium.org
Project Member

Comment 5 by bugdroid1@chromium.org, May 7 2018

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

commit 82e2ba92e41b1deb4836fbd3c4a8936341186a21
Author: Jonas Olsson <jonasolsson@chromium.org>
Date: Mon May 07 11:11:36 2018

Make observer and log optional in StreamFactory::CreateInputStream

Not every client cares about observing or logs. This change saves those clients from providing a dummy observer/log.
If the log isn't provided, explicitly clear log_. This makes it easier to check if the scoped_refptr<media::mojom::ThreadSafeAudioLogPtr> actually holds a value, and skips some unnecessary message forwarding.

Bug:  834702 
Change-Id: I4c405a7dd90d241847ee60e4c3b8db58bf0a3fc9
Reviewed-on: https://chromium-review.googlesource.com/1043907
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Commit-Queue: Jonas Olsson <jonasolsson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556413}
[modify] https://crrev.com/82e2ba92e41b1deb4836fbd3c4a8936341186a21/services/audio/input_stream.cc
[modify] https://crrev.com/82e2ba92e41b1deb4836fbd3c4a8936341186a21/services/audio/public/mojom/stream_factory.mojom

Project Member

Comment 6 by bugdroid1@chromium.org, May 8 2018

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

commit 950400ee7eecf49dfd5b66c99da6ac0b2e7e5185
Author: Marina Ciocea <marinaciocea@chromium.org>
Date: Tue May 08 11:56:29 2018

Fix crash when passing nullptr log to audio::InputStream.

Turns out RepeatingCallback() doesn't create a bound callback, resulting in crash on Run():
https://chromium.googlesource.com/chromium/src/+/master/docs/callback.md#creating-a-callback-that-does-nothing

Unittests could be prettier, I'll fix that next week.

Bug:  834702 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I6fa404cc195e709d581015c48aaf65580a47525a
Reviewed-on: https://chromium-review.googlesource.com/1049625
Commit-Queue: Marina Ciocea <marinaciocea@chromium.org>
Commit-Queue: Olga Sharonova <olka@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556761}
[modify] https://crrev.com/950400ee7eecf49dfd5b66c99da6ac0b2e7e5185/media/audio/audio_input_sync_writer.cc
[modify] https://crrev.com/950400ee7eecf49dfd5b66c99da6ac0b2e7e5185/services/audio/input_stream.cc
[modify] https://crrev.com/950400ee7eecf49dfd5b66c99da6ac0b2e7e5185/services/audio/input_stream_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, May 10 2018

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

commit 8c97dbc39ecc653c073cdeff47c77cffd1706157
Author: Jonas Olsson <jonasolsson@chromium.org>
Date: Thu May 10 16:20:09 2018

Audio-service-based media::AudioInputIPC, to make AudioInputDevice work with the audio service.

Internally it uses a new, slightly simplified copy of MojoAudioInputIPC.

Bug:  834702 
Change-Id: I878ada69d82c72a431b7fdfe1905ae79b3d8963d
Reviewed-on: https://chromium-review.googlesource.com/1041686
Reviewed-by: Olga Sharonova <olka@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Commit-Queue: Jonas Olsson <jonasolsson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557545}
[modify] https://crrev.com/8c97dbc39ecc653c073cdeff47c77cffd1706157/services/audio/BUILD.gn
[modify] https://crrev.com/8c97dbc39ecc653c073cdeff47c77cffd1706157/services/audio/public/cpp/BUILD.gn
[add] https://crrev.com/8c97dbc39ecc653c073cdeff47c77cffd1706157/services/audio/public/cpp/device_factory.cc
[add] https://crrev.com/8c97dbc39ecc653c073cdeff47c77cffd1706157/services/audio/public/cpp/device_factory.h
[add] https://crrev.com/8c97dbc39ecc653c073cdeff47c77cffd1706157/services/audio/public/cpp/input_ipc.cc
[add] https://crrev.com/8c97dbc39ecc653c073cdeff47c77cffd1706157/services/audio/public/cpp/input_ipc.h
[add] https://crrev.com/8c97dbc39ecc653c073cdeff47c77cffd1706157/services/audio/public/cpp/input_ipc_unittest.cc

Comment 8 by olka@chromium.org, Jun 12 2018

Blocking: 851692
Labels: -Pri-2 Pri-1
Muyuan, is there anything else remaining to do for this bug?
Labels: M-72
Owner: xiaoh...@chromium.org
xiaohuic@ Is the conversion completed? Can we close it?
Status: Fixed (was: Assigned)

Sign in to add a comment