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

Issue 879243 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 6
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug


Participants' hotlists:
AudioService-APM
Audio-Service


Sign in to add a comment

Check if ProcessedLocalAudioSource::GetInputFormat needs to be implemented when running the APM in the audio service

Project Member Reported by maxmorin@chromium.org, Aug 30

Issue description

Check if ProcessedLocalAudioSource::GetInputFormat needs to be implemented when running the APM in the audio service.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 30

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

commit 86d4f131d29c49e761ee20d9b1c5026b1172ef50
Author: Max Morin <maxmorin@chromium.org>
Date: Thu Aug 30 21:51:13 2018

APM move: Configure and use APM in Audio Service

ProcessedLocalAudioSource is changed to either create a
MediaStreamAudioProcessor or an AudioServiceAudioProcessorProxy depending
on whether the WebRtcApmInAudioService flag is set.

AudioServiceAudioProcessorProxy proxies GetStats and AECDump calls to
the remote audio processor. Although the JavaScript getStats call is
asynchronous, we currently collect stats synchronously inside Chrome.
Ideally, this would be changed. For now, the proxy overcomes this
mismatch by polling the remote audio processor for stats at regular
intervals. It uses a heuristic to determine the rate at which the user
is calling getStats and tries to match that, within some reasonable
limits. 878764 has been filed to fix this.

For AECDumps, we already get a file-handle from the browser, so it can
just be sent along to the audio service. So the audio service does not
need to be able to create files for this to work.

For an outline of the project this CL is part of, see: https://docs.google.com/document/d/1u4POff_ts_1LE3WDLA_wDDFnUswdlsuHL5DsiTE0a3U/edit?usp=sharing
It's accessible to everyone @chromium.org.

No-try since the test timing out is unrelated.

No-Try: true
Bug: 851959, 878764, 879133,  879243 , 879296
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: I7473c068aa691d69f6ba90dce2550534c9cb3d8a
Reviewed-on: https://chromium-review.googlesource.com/1169471
Commit-Queue: Max Morin <maxmorin@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Reviewed-by: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587796}
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/content/browser/media/media_internals.cc
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/content/browser/renderer_host/media/audio_output_delegate_impl_unittest.cc
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/content/browser/webrtc/webrtc_audio_browsertest.cc
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/content/renderer/BUILD.gn
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/content/renderer/media/audio/mojo_audio_input_ipc.cc
[add] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/content/renderer/media/stream/audio_service_audio_processor_proxy.cc
[add] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/content/renderer/media/stream/audio_service_audio_processor_proxy.h
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/content/renderer/media/stream/media_stream_audio_processor_options.cc
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/content/renderer/media/stream/media_stream_audio_processor_options.h
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/content/renderer/media/stream/processed_local_audio_source.cc
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/content/renderer/media/stream/processed_local_audio_source.h
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/content/renderer/media/webrtc/webrtc_audio_device_impl.cc
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/content/renderer/media/webrtc/webrtc_audio_device_impl.h
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/content/renderer/media/webrtc/webrtc_audio_renderer.cc
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/content/renderer/media/webrtc/webrtc_audio_renderer_unittest.cc
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/content/renderer/media/webrtc/webrtc_audio_sink.cc
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/content/renderer/media/webrtc/webrtc_audio_sink.h
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/content/renderer/media/webrtc/webrtc_media_stream_track_adapter.cc
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/content/test/BUILD.gn
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/media/audio/BUILD.gn
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/media/audio/audio_logging.h
[add] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/media/audio/audio_processing.cc
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/media/audio/audio_processing.h
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/media/audio/audio_source_parameters.h
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/media/audio/fake_audio_log_factory.cc
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/media/mojo/interfaces/audio_logging.mojom
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/services/audio/input_controller.cc
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/services/audio/input_stream.cc
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/services/audio/log_adapter.cc
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/services/audio/log_adapter.h
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/services/audio/log_factory_manager_unittest.cc
[modify] https://crrev.com/86d4f131d29c49e761ee20d9b1c5026b1172ef50/services/audio/test/mock_log.h

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 4

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

commit e955a7f67bc4cce4228f842425bbb6a4a02c3458
Author: Max Morin <maxmorin@chromium.org>
Date: Tue Sep 04 15:12:24 2018

Un-implement some unnecessary WebRTC ADM methods.

The value given by a Stereo*IsAvailable method is just fed back to the
ADM, where it's ignored. However, right now, StereoRecordingIsAvailable
may return -1 which causes a LOG(ERROR). In some cases, LOG(ERROR)
causes a crash, so make sure that StereoRecordingIsAvailable never
fails.

Bug: 823970,  879243 
Change-Id: Ibd216537f9216aed2ba806c660ef35ce5d3e7ae6
Reviewed-on: https://chromium-review.googlesource.com/1203732
Reviewed-by: Oskar Sundbom <ossu@chromium.org>
Commit-Queue: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588517}
[modify] https://crrev.com/e955a7f67bc4cce4228f842425bbb6a4a02c3458/content/renderer/media/stream/processed_local_audio_source.cc
[modify] https://crrev.com/e955a7f67bc4cce4228f842425bbb6a4a02c3458/content/renderer/media/stream/processed_local_audio_source.h
[modify] https://crrev.com/e955a7f67bc4cce4228f842425bbb6a4a02c3458/content/renderer/media/webrtc/webrtc_audio_device_impl.cc
[modify] https://crrev.com/e955a7f67bc4cce4228f842425bbb6a4a02c3458/content/renderer/media/webrtc/webrtc_audio_device_impl.h
[modify] https://crrev.com/e955a7f67bc4cce4228f842425bbb6a4a02c3458/content/renderer/media/webrtc/webrtc_audio_device_not_impl.cc
[modify] https://crrev.com/e955a7f67bc4cce4228f842425bbb6a4a02c3458/content/renderer/media/webrtc/webrtc_audio_device_not_impl.h

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 4

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

commit e955a7f67bc4cce4228f842425bbb6a4a02c3458
Author: Max Morin <maxmorin@chromium.org>
Date: Tue Sep 04 15:12:24 2018

Un-implement some unnecessary WebRTC ADM methods.

The value given by a Stereo*IsAvailable method is just fed back to the
ADM, where it's ignored. However, right now, StereoRecordingIsAvailable
may return -1 which causes a LOG(ERROR). In some cases, LOG(ERROR)
causes a crash, so make sure that StereoRecordingIsAvailable never
fails.

Bug: 823970,  879243 
Change-Id: Ibd216537f9216aed2ba806c660ef35ce5d3e7ae6
Reviewed-on: https://chromium-review.googlesource.com/1203732
Reviewed-by: Oskar Sundbom <ossu@chromium.org>
Commit-Queue: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588517}
[modify] https://crrev.com/e955a7f67bc4cce4228f842425bbb6a4a02c3458/content/renderer/media/stream/processed_local_audio_source.cc
[modify] https://crrev.com/e955a7f67bc4cce4228f842425bbb6a4a02c3458/content/renderer/media/stream/processed_local_audio_source.h
[modify] https://crrev.com/e955a7f67bc4cce4228f842425bbb6a4a02c3458/content/renderer/media/webrtc/webrtc_audio_device_impl.cc
[modify] https://crrev.com/e955a7f67bc4cce4228f842425bbb6a4a02c3458/content/renderer/media/webrtc/webrtc_audio_device_impl.h
[modify] https://crrev.com/e955a7f67bc4cce4228f842425bbb6a4a02c3458/content/renderer/media/webrtc/webrtc_audio_device_not_impl.cc
[modify] https://crrev.com/e955a7f67bc4cce4228f842425bbb6a4a02c3458/content/renderer/media/webrtc/webrtc_audio_device_not_impl.h

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 4

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

commit e955a7f67bc4cce4228f842425bbb6a4a02c3458
Author: Max Morin <maxmorin@chromium.org>
Date: Tue Sep 04 15:12:24 2018

Un-implement some unnecessary WebRTC ADM methods.

The value given by a Stereo*IsAvailable method is just fed back to the
ADM, where it's ignored. However, right now, StereoRecordingIsAvailable
may return -1 which causes a LOG(ERROR). In some cases, LOG(ERROR)
causes a crash, so make sure that StereoRecordingIsAvailable never
fails.

Bug: 823970,  879243 
Change-Id: Ibd216537f9216aed2ba806c660ef35ce5d3e7ae6
Reviewed-on: https://chromium-review.googlesource.com/1203732
Reviewed-by: Oskar Sundbom <ossu@chromium.org>
Commit-Queue: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588517}
[modify] https://crrev.com/e955a7f67bc4cce4228f842425bbb6a4a02c3458/content/renderer/media/stream/processed_local_audio_source.cc
[modify] https://crrev.com/e955a7f67bc4cce4228f842425bbb6a4a02c3458/content/renderer/media/stream/processed_local_audio_source.h
[modify] https://crrev.com/e955a7f67bc4cce4228f842425bbb6a4a02c3458/content/renderer/media/webrtc/webrtc_audio_device_impl.cc
[modify] https://crrev.com/e955a7f67bc4cce4228f842425bbb6a4a02c3458/content/renderer/media/webrtc/webrtc_audio_device_impl.h
[modify] https://crrev.com/e955a7f67bc4cce4228f842425bbb6a4a02c3458/content/renderer/media/webrtc/webrtc_audio_device_not_impl.cc
[modify] https://crrev.com/e955a7f67bc4cce4228f842425bbb6a4a02c3458/content/renderer/media/webrtc/webrtc_audio_device_not_impl.h

Cc: ossu@chromium.org
Labels: -Pri-2 Merge-Request-70 Pri-1
Owner: maxmorin@chromium.org
Status: Fixed (was: Assigned)
Wow, bugdroid sure got excited there. Requesting merge for the CL in comment 2 (same as the one in 3 and 4), since it works around a weird crash.
Project Member

Comment 6 by sheriffbot@chromium.org, Sep 7

Labels: -Merge-Request-70 Hotlist-Merge-Approved Merge-Approved-70
Your change meets the bar and is auto-approved for M70. Please go ahead and merge the CL to branch 3538 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 7

Labels: -merge-approved-70 merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/401d34e5d729c7a4bc2bef39f1213800393f9c40

commit 401d34e5d729c7a4bc2bef39f1213800393f9c40
Author: Max Morin <maxmorin@chromium.org>
Date: Fri Sep 07 09:48:26 2018

[M70]Un-implement some unnecessary WebRTC ADM methods.

The value given by a Stereo*IsAvailable method is just fed back to the
ADM, where it's ignored. However, right now, StereoRecordingIsAvailable
may return -1 which causes a LOG(ERROR). In some cases, LOG(ERROR)
causes a crash, so make sure that StereoRecordingIsAvailable never
fails.

Bug: 823970,  879243 
Change-Id: Ibd216537f9216aed2ba806c660ef35ce5d3e7ae6
Reviewed-on: https://chromium-review.googlesource.com/1203732
Reviewed-by: Oskar Sundbom <ossu@chromium.org>
Commit-Queue: Max Morin <maxmorin@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#588517}(cherry picked from commit e955a7f67bc4cce4228f842425bbb6a4a02c3458)
Reviewed-on: https://chromium-review.googlesource.com/1212383
Reviewed-by: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#130}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/401d34e5d729c7a4bc2bef39f1213800393f9c40/content/renderer/media/stream/processed_local_audio_source.cc
[modify] https://crrev.com/401d34e5d729c7a4bc2bef39f1213800393f9c40/content/renderer/media/stream/processed_local_audio_source.h
[modify] https://crrev.com/401d34e5d729c7a4bc2bef39f1213800393f9c40/content/renderer/media/webrtc/webrtc_audio_device_impl.cc
[modify] https://crrev.com/401d34e5d729c7a4bc2bef39f1213800393f9c40/content/renderer/media/webrtc/webrtc_audio_device_impl.h
[modify] https://crrev.com/401d34e5d729c7a4bc2bef39f1213800393f9c40/content/renderer/media/webrtc/webrtc_audio_device_not_impl.cc
[modify] https://crrev.com/401d34e5d729c7a4bc2bef39f1213800393f9c40/content/renderer/media/webrtc/webrtc_audio_device_not_impl.h

Sign in to add a comment