New issue
Advanced search Search tips

Issue 687502 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

webrtc::Ramp in webrtc/modules/audio_mixer/audio_frame_manipulator.cc is slow

Project Member Reported by aleloi@chromium.org, Feb 1 2017

Issue description

Release build generates code that multiplies each sample in each webrtc audio stream (thousands samples/second) with a floating point number, which is almost always 1.0f. This shouldn't be the case.

See https://chromium.googlesource.com/external/webrtc/+/4b8bfb8ed3845ce214f5c586e521a442eb771e56/webrtc/modules/audio_mixer/audio_frame_manipulator.cc
 
 
Labels: OS-All
Cc: hlundin@chromium.org
Labels: M-58
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/4637b6afca431be8caf1d22151a2cbb2e86a33d5

commit 4637b6afca431be8caf1d22151a2cbb2e86a33d5
Author: aleloi <aleloi@webrtc.org>
Date: Wed Feb 01 11:43:31 2017

Consistent 30% improvement in audio mixer running time.

(Or, in less flattering terms, fixing a performance issue introduced
a few months ago by me).

In GN release mode (is_debug = false), the version of the mixer code
before this CL generated code that multiplied each sample (tens of
thousands/second for each input stream) with a floating point number.
This number is almost always exactly 1.0f. The only situation when it's
not 1 is when an audio steam is added or removed.

For one input stream early return leads to a 30% improvement of audio
mixing time profiled on x86-64 under a release build (is_debug = false,
enable_profiling, enable_full_stack_frames_for_profiling) with 16kHz and no
APM limiter. There can be up to 3 streams.

BUG= chromium:687502 

Review-Url: https://codereview.webrtc.org/2659423002
Cr-Commit-Position: refs/heads/master@{#16396}

[modify] https://crrev.com/4637b6afca431be8caf1d22151a2cbb2e86a33d5/webrtc/modules/audio_mixer/audio_frame_manipulator.cc

Status: Fixed (was: Started)
Labels: Merge-Request-57
Project Member

Comment 6 by sheriffbot@chromium.org, Feb 6 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Please merge your change to M57 branch 2987 before 5:00 PM PT, Tuesday (02/07/17) so we can pick it up for next Beta release. Thank you.
Project Member

Comment 8 by bugdroid1@chromium.org, Feb 7 2017

Labels: merge-merged-57
The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/26b86345bdac64c7c6142ff4f4d797d032f27fe3

commit 26b86345bdac64c7c6142ff4f4d797d032f27fe3
Author: aleloi <aleloi@webrtc.org>
Date: Tue Feb 07 13:40:16 2017

Merge to 57: Consistent 30% improvement in audio mixer running time.

This is a cherry pick of https://codereview.webrtc.org/2659423002
BUG= chromium:687502 
TBR=hlundin@webrtc.org
NOTRY=True
NOPRESUBMIT=True

Review-Url: https://codereview.webrtc.org/2681443002
Cr-Commit-Position: refs/branch-heads/57@{#3}
Cr-Branched-From: e5cbc2019003dbb40e03811d7607feb95757a4ec-refs/heads/master@{#16123}

[modify] https://crrev.com/26b86345bdac64c7c6142ff4f4d797d032f27fe3/webrtc/modules/audio_mixer/audio_frame_manipulator.cc

Labels: -Merge-Approved-57
Per comment #8, this is already merged to M57. Hence, removing "Merge-Approved-57" label. Thank you.

Sign in to add a comment