New issue
Advanced search Search tips

Issue 889494 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 27
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug


Show other hotlists

Hotlists containing this issue:
AudioService-APM


Sign in to add a comment

APM in Audio Service delay is zero

Project Member Reported by ossu@chromium.org, Sep 26

Issue description

Did some AEC dumps while testing the APM running in the audio service and the delay is all zero after a while.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 27

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

commit e9826d74d81dc41935af6ce6af60885cd97dce7a
Author: Oskar Sundbom <ossu@chromium.org>
Date: Thu Sep 27 09:51:34 2018

APM in Audio Service: Fix render delay calculation

The render delay is calculated backwards, making it negative. The
time we're comparing with is in the future, not the past.

Bug:  889494 
Change-Id: I3aa8b64096caa9ad5feadcafc1df5bca8e035750
Reviewed-on: https://chromium-review.googlesource.com/1246163
Reviewed-by: Max Morin <maxmorin@chromium.org>
Commit-Queue: Oskar Sundbom <ossu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594648}
[modify] https://crrev.com/e9826d74d81dc41935af6ce6af60885cd97dce7a/media/webrtc/audio_processor.cc

Status: Fixed (was: Started)
Labels: Merge-Request-70
It's a one line fix and would be very helpful to get merged, since otherwise the delay estimation using APM in audio service will be wrong, possibly affecting echo cancellation.
Project Member

Comment 4 by sheriffbot@chromium.org, Oct 1

Labels: -Merge-Request-70 Merge-Review-70 Hotlist-Merge-Review
This bug requires manual review: M70 has already been promoted to the beta branch, so this requires manual review
Please contact the 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
Please mark which OS's this is impacting.
Labels: OS-Linux OS-Mac OS-Windows
Sure, APM in audio service is allowed on Mac, Windows and Linux, since that's where we have sandboxing in place for the audio process.
Labels: -Merge-Review-70 Merge-Approved-70
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 5

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

commit a8e3f7bfcdab61e9847daa8dc2c6d7d5278f0aba
Author: Oskar Sundbom <ossu@chromium.org>
Date: Fri Oct 05 09:04:08 2018

M70: APM in Audio Service: Fix render delay calculation

The render delay is calculated backwards, making it negative. The
time we're comparing with is in the future, not the past.

Bug:  889494 
Change-Id: I3aa8b64096caa9ad5feadcafc1df5bca8e035750
Reviewed-on: https://chromium-review.googlesource.com/1246163
Reviewed-by: Max Morin <maxmorin@chromium.org>
Commit-Queue: Oskar Sundbom <ossu@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#594648}(cherry picked from commit e9826d74d81dc41935af6ce6af60885cd97dce7a)
Reviewed-on: https://chromium-review.googlesource.com/c/1264164
Reviewed-by: Oskar Sundbom <ossu@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#873}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/a8e3f7bfcdab61e9847daa8dc2c6d7d5278f0aba/media/webrtc/audio_processor.cc

Labels: Merge-Merged-70-3538
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/a8e3f7bfcdab61e9847daa8dc2c6d7d5278f0aba

Commit: a8e3f7bfcdab61e9847daa8dc2c6d7d5278f0aba
Author: ossu@chromium.org
Commiter: ossu@chromium.org
Date: 2018-10-05 09:04:08 +0000 UTC

M70: APM in Audio Service: Fix render delay calculation

The render delay is calculated backwards, making it negative. The
time we're comparing with is in the future, not the past.

Bug:  889494 
Change-Id: I3aa8b64096caa9ad5feadcafc1df5bca8e035750
Reviewed-on: https://chromium-review.googlesource.com/1246163
Reviewed-by: Max Morin <maxmorin@chromium.org>
Commit-Queue: Oskar Sundbom <ossu@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#594648}(cherry picked from commit e9826d74d81dc41935af6ce6af60885cd97dce7a)
Reviewed-on: https://chromium-review.googlesource.com/c/1264164
Reviewed-by: Oskar Sundbom <ossu@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#873}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
Labels: Needs-Feedback
@Oskar Sundbom: Could you please provide manual reproducible steps that reproduces the issue with actual and excepted behavior and help us in verifying the fix.

Thanks!
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 11

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

commit 9510693c74322c400c28f384d3279fe4e57feaf5
Author: Henrik Grunell <grunell@chromium.org>
Date: Thu Oct 11 21:59:29 2018

Add APM system delay UMA stats.

Adds logging of render, capture and total system delay stats as
communicated to the audio processing module.

Also adds variance stats of the above delays over one second windows.

Replaces https://chromium-review.googlesource.com/c/chromium/src/+/1254201

Bug:  889494 
Change-Id: Id58897870108b456fa569db7e0ac8663ff01cb19
Reviewed-on: https://chromium-review.googlesource.com/c/1268243
Commit-Queue: Henrik Grunell <grunell@chromium.org>
Reviewed-by: Oskar Sundbom <ossu@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598965}
[modify] https://crrev.com/9510693c74322c400c28f384d3279fe4e57feaf5/content/renderer/media/stream/media_stream_audio_processor.cc
[modify] https://crrev.com/9510693c74322c400c28f384d3279fe4e57feaf5/content/renderer/media/stream/media_stream_audio_processor.h
[modify] https://crrev.com/9510693c74322c400c28f384d3279fe4e57feaf5/media/webrtc/BUILD.gn
[add] https://crrev.com/9510693c74322c400c28f384d3279fe4e57feaf5/media/webrtc/audio_delay_stats_reporter.cc
[add] https://crrev.com/9510693c74322c400c28f384d3279fe4e57feaf5/media/webrtc/audio_delay_stats_reporter.h
[modify] https://crrev.com/9510693c74322c400c28f384d3279fe4e57feaf5/media/webrtc/audio_processor.cc
[modify] https://crrev.com/9510693c74322c400c28f384d3279fe4e57feaf5/media/webrtc/audio_processor.h
[modify] https://crrev.com/9510693c74322c400c28f384d3279fe4e57feaf5/tools/metrics/histograms/histograms.xml

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 17

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

commit 7e48feb0e3f07be25211ee8a98b7392694ee4b44
Author: Henrik Grunell <grunell@chromium.org>
Date: Wed Oct 17 19:25:39 2018

Fix audio delay variance histogram units.

Bug:  889494 
Change-Id: I4e2a36f2ca7e93db9b5bacb6a32a64abf9ae812c
Reviewed-on: https://chromium-review.googlesource.com/c/1282929
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Commit-Queue: Henrik Grunell <grunell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600514}
[modify] https://crrev.com/7e48feb0e3f07be25211ee8a98b7392694ee4b44/tools/metrics/histograms/histograms.xml

Sign in to add a comment