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

Issue 1454983006: Ducking fix #2: Removed the aec state as an input parameter to the FilterFar function. (Closed)

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

Description

Removed the aec state as an input parameter to the FilterFar function. BUG=webrtc:5201 Committed: https://crrev.com/54eb5e2e9ae91e9ad6dcb297de7b918ebe706d5f Cr-Commit-Position: refs/heads/master@{#10787}

Patch Set 1 #

Total comments: 7

Patch Set 2 : Changes in response to reviewer comments #

Total comments: 4

Patch Set 3 : Corrected line alignment #

Messages

Total messages: 14 (4 generated)
peah-webrtc
Pure refactoring of the function parameter list. The changes are bitexact.
5 years, 1 month ago (2015-11-19 10:50:30 UTC) #3
hlundin-webrtc
Nits. https://codereview.webrtc.org/1454983006/diff/1/webrtc/modules/audio_processing/aec/aec_core_internal.h File webrtc/modules/audio_processing/aec/aec_core_internal.h (right): https://codereview.webrtc.org/1454983006/diff/1/webrtc/modules/audio_processing/aec/aec_core_internal.h#newcode175 webrtc/modules/audio_processing/aec/aec_core_internal.h:175: int xfBufBlockPos, Let the new parameters be named ...
5 years, 1 month ago (2015-11-20 10:59:40 UTC) #4
ivoc
I have no further comments to add to Henrik's. So LGTM, assuming you address those.
5 years, 1 month ago (2015-11-20 12:59:52 UTC) #5
peah-webrtc
https://codereview.webrtc.org/1454983006/diff/1/webrtc/modules/audio_processing/aec/aec_core_internal.h File webrtc/modules/audio_processing/aec/aec_core_internal.h (right): https://codereview.webrtc.org/1454983006/diff/1/webrtc/modules/audio_processing/aec/aec_core_internal.h#newcode175 webrtc/modules/audio_processing/aec/aec_core_internal.h:175: int xfBufBlockPos, On 2015/11/20 10:59:40, hlundin-webrtc wrote: > Let ...
5 years, 1 month ago (2015-11-24 06:48:56 UTC) #6
peah-webrtc
According to kwiberg@ it seems like removing the local constant copy of the input should ...
5 years ago (2015-11-24 11:49:21 UTC) #7
hlundin-webrtc
lgtm https://codereview.webrtc.org/1454983006/diff/1/webrtc/modules/audio_processing/aec/aec_core_sse2.c File webrtc/modules/audio_processing/aec/aec_core_sse2.c (right): https://codereview.webrtc.org/1454983006/diff/1/webrtc/modules/audio_processing/aec/aec_core_sse2.c#newcode39 webrtc/modules/audio_processing/aec/aec_core_sse2.c:39: const int num_partitions_local = num_partitions; On 2015/11/24 06:48:56, ...
5 years ago (2015-11-25 12:49:40 UTC) #8
peah-webrtc
https://codereview.webrtc.org/1454983006/diff/20001/webrtc/modules/audio_processing/aec/aec_core_mips.c File webrtc/modules/audio_processing/aec/aec_core_mips.c (right): https://codereview.webrtc.org/1454983006/diff/20001/webrtc/modules/audio_processing/aec/aec_core_mips.c#newcode730 webrtc/modules/audio_processing/aec/aec_core_mips.c:730: "lwc1 %[f0], 0(%[x_pow]) \n\t" On 2015/11/25 12:49:40, hlundin-webrtc wrote: ...
5 years ago (2015-11-25 13:20:41 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1454983006/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1454983006/40001
5 years ago (2015-11-25 13:20:56 UTC) #12
commit-bot: I haz the power
Committed patchset #3 (id:40001)
5 years ago (2015-11-25 15:43:17 UTC) #13
commit-bot: I haz the power
5 years ago (2015-11-25 15:43:29 UTC) #14
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/54eb5e2e9ae91e9ad6dcb297de7b918ebe706d5f
Cr-Commit-Position: refs/heads/master@{#10787}

Powered by Google App Engine
This is Rietveld 408576698