New issue
Advanced search Search tips

Issue 739379 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug

Blocked on:
issue 708475

Blocking:
issue 696930



Sign in to add a comment

Private API for controlling WebRTC's AEC3 doesn't always take effect when call starts

Project Member Reported by hlundin@chromium.org, Jul 5 2017

Issue description

Chrome Version: 59.0.3071.91 
OS: ChromeOS

What steps will reproduce the problem?
(1) Set up a WebRTC call
(2) Immediately thereafter, call setAudioExperiments to control the AEC3 feature.
(3) Take a diagnostic audio recording (from chrome://webrtc-internals).

What is the expected result?
The state of AEC3 in the recording should match what was set to setAudioExperiments.

What happens instead?
It does not always match.

 
Blockedon: 708475
Description: Show this description
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 7 2017

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

commit 7933c57f94e3cedbfdbda3e61b55ede90ef2dac5
Author: hlundin <hlundin@chromium.org>
Date: Fri Jul 07 15:22:30 2017

Cache the override for WebRTC's AEC3 in RenderProcessHostImpl

If setAudioExperiments is called before the audio track has been
created, the value of the AEC3 experiment state should be chached and
later used when a track is created.

BUG= 708475 ,  739379 

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

[modify] https://crrev.com/7933c57f94e3cedbfdbda3e61b55ede90ef2dac5/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/7933c57f94e3cedbfdbda3e61b55ede90ef2dac5/content/browser/renderer_host/render_process_host_impl.h

Blocking: 696930
Cc: peah@chromium.org
Labels: Merge-Request-60
Status: Fixed (was: Assigned)
This has been fixed. The patch has already been tested and verified manually on M60.

I would like to merge this fix to M60. 

WebRTC's new echo canceller (called AEC3) is currently being rolled out as an experiment; see issue 696930. This is being rolled out across all desktop platforms using Finch. However, we would like to be able to control the feature separately for "Chromebox for Meetings devices", since they may be used in significantly different acoustic environments than regular desktop use cases. In feature 708475, an API was implemented to allow the application to control the AEC3 feature. However, a bug was found making this switch unreliable. This CL fixes that, and I'd like to have it merged to M60 to unblock the continued rollout of AEC3 on ChromeOS.

Project Member

Comment 6 by sheriffbot@chromium.org, Jul 9 2017

Labels: -Merge-Request-60 Hotlist-Merge-Review Merge-Review-60
This bug requires manual review: M60 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-60 Merge-Approved-60
Approved for M60. 
Please merge this to M60 ASAP. branch:3112
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 11 2017

Labels: -merge-approved-60 merge-merged-3112
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a897318436de09adc6b966fe343996d6abd3c702

commit a897318436de09adc6b966fe343996d6abd3c702
Author: henrika <henrika@chromium.org>
Date: Tue Jul 11 11:16:51 2017

Cache the override for WebRTC's AEC3 in RenderProcessHostImpl

If setAudioExperiments is called before the audio track has been
created, the value of the AEC3 experiment state should be chached and
later used when a track is created.

BUG= 708475 ,  739379 
TBR=hlundin@chromium.org

(cherry picked from commit 7933c57f94e3cedbfdbda3e61b55ede90ef2dac5)

Review-Url: https://codereview.chromium.org/2973643002
Cr-Original-Commit-Position: refs/heads/master@{#484932}
Change-Id: Ia806f1c9f829549b39759bcacdc0c7cf13642bee
Reviewed-on: https://chromium-review.googlesource.com/566998
Reviewed-by: Henrik Andreasson <henrika@webrtc.org>
Cr-Commit-Position: refs/branch-heads/3112@{#581}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}
[modify] https://crrev.com/a897318436de09adc6b966fe343996d6abd3c702/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/a897318436de09adc6b966fe343996d6abd3c702/content/browser/renderer_host/render_process_host_impl.h

Sign in to add a comment