Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(189)

Issue 110303003: Revert 240548 "Enable platform echo cancellation through the Aud..." (Closed)

Created:
7 years ago by hashimoto
Modified:
7 years ago
Reviewers:
ajm
CC:
chromium-reviews
Visibility:
Public.

Description

Revert 240548 "Enable platform echo cancellation through the Aud..." Causing compile failure on Chromium OS bots: chromeos-chrome-33.0.1738.0_alpha-r1: media/audio/cras/audio_manager_cras.cc:126:10: error: no matching constructor chromeos-chrome-33.0.1738.0_alpha-r1: for initialization of 'media::AudioParameters' chromeos-chrome-33.0.1738.0_alpha-r1: return AudioParameters( Logs: http://build.chromium.org/p/chromium.chromiumos/builders/ChromiumOS%20%28x86%29/builds/18585 http://build.chromium.org/p/chromium.chromiumos/builders/ChromiumOS%20%28amd64%29/builds/12929 http://build.chromium.org/p/chromium.chromiumos/builders/ChromiumOS%20%28daisy%29/builds/14979 http://build.chromium.org/p/chromium.memory/builders/Chromium%20OS%20%28amd64%29%20ASAN/builds/8140 http://build.chromium.org/p/chromium.memory/builders/Chromium%20OS%20%28x86%29%20ASAN/builds/7509 > Enable platform echo cancellation through the AudioRecord path. > > Add a platform effects mask member to AudioParameters. This allows the > availability of platform effects (currently AEC) to be plumbed up to > MediaStreamDependencyFactory, where they can be reconciled with the > media constraints to determine if the effects should be enabled. When > this is the case, the constraints will be modified to disable the > corresponding software effect in PeerConnection. > > The availability is controlled by a whitelist of device models in > AudioManagerAndroid, which for AEC, currently consists of Nexus 5 and > Nexus 7. > > AudioManagerAndroid will use the AudioRecord path iff the platform > AEC is enabled. > > TESTED=Using apprtc on a N5 and N7 (whitelisted): > - The AudioRecord input path is used. > - The platform AEC is enabled and the software AEC (in PeerConnection) > is disabled. > - Calls have good echo cancellation quality. > > Using apprtc with ?audio=googEchoCancellation=false on a N5 and N7: > - The OpenSLES input path is used. > - Both the platform and software AEC are disabled. > > Using apprtc on Nexus 4 (non-whitelisted): > - The OpenSLES input path is used. > - The platform AEC is disabled and the software AEC is enabled. > > Using apprtc on Galaxy S2 (running ICS): > - The OpenSLES input path is used. > > audio_android_unittest.cc passes on N5, N7 and Galaxy S2 > > TBR=jschuh > > Review URL: https://codereview.chromium.org/99033003 TBR=ajm@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=240592

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+151 lines, -366 lines) Patch
M trunk/src/content/browser/renderer_host/media/audio_input_device_manager.cc View 1 chunk +0 lines, -1 line 0 comments Download
M trunk/src/content/browser/renderer_host/media/audio_renderer_host_unittest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M trunk/src/content/common/media/media_param_traits.cc View 2 chunks +5 lines, -9 lines 0 comments Download
M trunk/src/content/common/media/media_stream_messages.h View 1 chunk +0 lines, -1 line 0 comments Download
M trunk/src/content/public/common/media_stream_request.h View 2 chunks +3 lines, -7 lines 0 comments Download
M trunk/src/content/renderer/media/media_stream_dependency_factory.cc View 4 chunks +3 lines, -42 lines 0 comments Download
M trunk/src/content/renderer/media/webrtc_audio_capturer.h View 3 chunks +3 lines, -6 lines 0 comments Download
M trunk/src/content/renderer/media/webrtc_audio_capturer.cc View 7 chunks +15 lines, -19 lines 0 comments Download
M trunk/src/content/renderer/media/webrtc_audio_capturer_unittest.cc View 1 chunk +2 lines, -4 lines 0 comments Download
M trunk/src/content/renderer/media/webrtc_audio_device_unittest.cc View 1 chunk +1 line, -2 lines 0 comments Download
M trunk/src/content/renderer/media/webrtc_local_audio_track_unittest.cc View 4 chunks +5 lines, -9 lines 0 comments Download
M trunk/src/content/renderer/renderer_webkitplatformsupport_impl.cc View 1 chunk +1 line, -2 lines 0 comments Download
M trunk/src/media/audio/alsa/audio_manager_alsa.cc View 1 chunk +1 line, -1 line 0 comments Download
M trunk/src/media/audio/android/audio_android_unittest.cc View 19 chunks +58 lines, -106 lines 0 comments Download
M trunk/src/media/audio/android/audio_manager_android.h View 1 chunk +1 line, -1 line 0 comments Download
M trunk/src/media/audio/android/audio_manager_android.cc View 4 chunks +6 lines, -25 lines 0 comments Download
M trunk/src/media/audio/android/audio_record_input.h View 1 chunk +0 lines, -3 lines 0 comments Download
M trunk/src/media/audio/android/audio_record_input.cc View 1 chunk +1 line, -2 lines 0 comments Download
M trunk/src/media/audio/audio_parameters.h View 5 chunks +5 lines, -19 lines 0 comments Download
M trunk/src/media/audio/audio_parameters.cc View 3 chunks +9 lines, -23 lines 0 comments Download
M trunk/src/media/audio/fake_audio_manager.cc View 1 chunk +1 line, -1 line 0 comments Download
M trunk/src/media/audio/mac/audio_manager_mac.cc View 1 chunk +4 lines, -8 lines 0 comments Download
M trunk/src/media/audio/pulse/audio_manager_pulse.cc View 1 chunk +1 line, -1 line 0 comments Download
M trunk/src/media/audio/win/audio_manager_win.cc View 1 chunk +1 line, -1 line 0 comments Download
M trunk/src/media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java View 4 chunks +3 lines, -15 lines 0 comments Download
M trunk/src/media/base/android/java/src/org/chromium/media/AudioRecordInput.java View 8 chunks +15 lines, -44 lines 0 comments Download
M trunk/src/media/base/channel_mixer_unittest.cc View 1 chunk +6 lines, -12 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
hashimoto
7 years ago (2013-12-13 11:41:16 UTC) #1
hashimoto
Committed patchset #1 manually as r240592.
7 years ago (2013-12-13 11:42:01 UTC) #2
hashimoto
On 2013/12/13 11:42:01, hashimoto wrote: > Committed patchset #1 manually as r240592. FYI: You can ...
7 years ago (2013-12-13 11:44:23 UTC) #3
ajm
7 years ago (2013-12-13 18:44:56 UTC) #4
Message was sent while issue was closed.
lgtm

Powered by Google App Engine
This is Rietveld 408576698