Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(21)

Issue 1512573003: Moving FFT on farend signal to where it is used in AEC (bit exact). (Closed)

Created:
3 years, 9 months ago by minyue-webrtc
Modified:
3 years, 9 months ago
Reviewers:
peah-webrtc
CC:
webrtc-reviews_webrtc.org, Andrew MacDonald, tterriberry_mozilla.com, audio-team_agora.io, hlundin-webrtc, kwiberg-webrtc, the sun, aluebs-webrtc, bjornv1
Base URL:
https://chromium.googlesource.com/external/webrtc.git@master
Target Ref:
refs/pending/heads/master
Project:
webrtc
Visibility:
Public.

Description

Moving FFT on farend signal to where it is used in AEC (bit exact). Currently, FFT is performance when AEC buffers farend signal. This has some drawbacks 1. memory inefficiency: two ring buffers are needed; 2. computation inefficiency: if ringbuffer gets wrapped around, some FFT computation will be wasted; 3. accessibility: the main AEC function looses accessibility to the time-domain signal. Therefore, this CL tries to buffer time domain data, which is buffered any way if a debugging macro is defined, and calculate the FFTs where they are actually used. BUG= Committed: https://crrev.com/92594a30ce02aed75f8a2a9f21e5b8c5c4e5f199 Cr-Commit-Position: refs/heads/master@{#11091}

Patch Set 1 #

Patch Set 2 : rebasing #

Total comments: 2

Patch Set 3 : removing a trivial static function #

Patch Set 4 : rebasing #

Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -73 lines) Patch
M webrtc/modules/audio_processing/aec/aec_core.c View 1 2 3 13 chunks +45 lines, -65 lines 0 comments Download
M webrtc/modules/audio_processing/aec/aec_core_internal.h View 1 2 3 2 chunks +2 lines, -3 lines 0 comments Download
M webrtc/modules/audio_processing/aec/echo_cancellation.c View 2 chunks +2 lines, -5 lines 0 comments Download

Messages

Total messages: 33 (12 generated)
minyue-webrtc
Hi Per, Would you help me reviewing this CL?
3 years, 9 months ago (2015-12-08 13:04:22 UTC) #3
minyue-webrtc
On 2015/12/08 13:04:22, minyue-webrtc wrote: > Hi Per, > > Would you help me reviewing ...
3 years, 9 months ago (2015-12-11 13:27:15 UTC) #4
peah-webrtc
On 2015/12/11 13:27:15, minyue-webrtc wrote: > On 2015/12/08 13:04:22, minyue-webrtc wrote: > > Hi Per, ...
3 years, 9 months ago (2015-12-11 14:10:51 UTC) #5
minyue-webrtc
On 2015/12/11 14:10:51, peah-webrtc wrote: > On 2015/12/11 13:27:15, minyue-webrtc wrote: > > On 2015/12/08 ...
3 years, 9 months ago (2015-12-11 14:23:54 UTC) #6
peah-webrtc
On 2015/12/11 14:23:54, minyue-webrtc wrote: > On 2015/12/11 14:10:51, peah-webrtc wrote: > > On 2015/12/11 ...
3 years, 9 months ago (2015-12-12 22:30:50 UTC) #7
minyue-webrtc
On 2015/12/12 22:30:50, peah-webrtc wrote: > On 2015/12/11 14:23:54, minyue-webrtc wrote: > > On 2015/12/11 ...
3 years, 9 months ago (2015-12-13 22:22:38 UTC) #8
minyue-webrtc
Hi Per, Now it is rebased. Would you try your bit-exactness test on this?
3 years, 9 months ago (2015-12-14 10:42:21 UTC) #9
minyue-webrtc
Hi Per, Now it is rebased. Would you try your bit-exactness test on this?
3 years, 9 months ago (2015-12-14 10:42:22 UTC) #10
peah-webrtc
Nice CL! I tested the code for bitexactness and it is bitexact on the test ...
3 years, 9 months ago (2015-12-16 12:52:22 UTC) #11
minyue-webrtc
Thanks! It is now updated. https://codereview.webrtc.org/1512573003/diff/20001/webrtc/modules/audio_processing/aec/aec_core.c File webrtc/modules/audio_processing/aec/aec_core.c (right): https://codereview.webrtc.org/1512573003/diff/20001/webrtc/modules/audio_processing/aec/aec_core.c#newcode844 webrtc/modules/audio_processing/aec/aec_core.c:844: static int MoveFarReadPtrWithoutSystemDelayUpdate(AecCore* self, ...
3 years, 9 months ago (2015-12-16 14:47:17 UTC) #12
minyue-webrtc
On 2015/12/16 14:47:17, minyue-webrtc wrote: > Thanks! It is now updated. > > https://codereview.webrtc.org/1512573003/diff/20001/webrtc/modules/audio_processing/aec/aec_core.c > ...
3 years, 9 months ago (2015-12-17 08:51:53 UTC) #13
peah-webrtc
On 2015/12/17 08:51:53, minyue-webrtc wrote: > On 2015/12/16 14:47:17, minyue-webrtc wrote: > > Thanks! It ...
3 years, 9 months ago (2015-12-18 13:13:19 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1512573003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1512573003/40001
3 years, 9 months ago (2015-12-18 13:18:37 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: ios64_sim_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_dbg/builds/4111) ios_arm64_dbg on tryserver.webrtc (JOB_FAILED, ...
3 years, 9 months ago (2015-12-18 13:19:20 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1512573003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1512573003/60001
3 years, 9 months ago (2015-12-18 15:16:31 UTC) #21
commit-bot: I haz the power
Try jobs failed on following builders: android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL) linux_baremetal on ...
3 years, 9 months ago (2015-12-18 17:16:56 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1512573003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1512573003/60001
3 years, 9 months ago (2015-12-18 17:31:05 UTC) #25
commit-bot: I haz the power
Try jobs failed on following builders: mac_baremetal on tryserver.webrtc (JOB_TIMED_OUT, http://build.chromium.org/p/tryserver.webrtc/builders/mac_baremetal/builds/8074)
3 years, 9 months ago (2015-12-18 19:31:29 UTC) #27
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1512573003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1512573003/60001
3 years, 9 months ago (2015-12-18 23:30:19 UTC) #29
commit-bot: I haz the power
Committed patchset #4 (id:60001)
3 years, 9 months ago (2015-12-18 23:31:18 UTC) #31
commit-bot: I haz the power
3 years, 9 months ago (2015-12-18 23:31:27 UTC) #33
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/92594a30ce02aed75f8a2a9f21e5b8c5c4e5f199
Cr-Commit-Position: refs/heads/master@{#11091}

Powered by Google App Engine
This is Rietveld 408576698