Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1456123003:
Ducking fix #3: Removed the state as an input to the FilterAdaptation function (Closed)
|
Created:
5 years, 1 month ago by peah-webrtc Modified:
5 years ago CC:
webrtc-reviews_webrtc.org, 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_Cleanup2_CL Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
Description-Removed the state as an input to the FilterAdaptation function.
-Renamed the TimeToFrequency and FrequencyToTime functions.
-Moved the windowing from the TimeToFrequency function.
-Simplified the EchoSubtraction function.
Note that the aec state is still an input to the EchoSubtraction function, and it currently needs to be that in order to support the output of the debug file. The longer-term goal is, however, to order the state into substates. This will simplify the parameter lists to the EchoCancellation function as well as replace the aec state as a parameter
BUG=webrtc:5201
Committed: https://crrev.com/7e43138c0890bd99f627fa061b122c8d5716a99d
Cr-Commit-Position: refs/heads/master@{#10830}
Patch Set 1 #Patch Set 2 : Various refactoring, among other things simplification of the EchoSubtraction function #
Total comments: 30
Patch Set 3 : Changes in response to reviewer comments #
Total comments: 12
Patch Set 4 : Changes in response to reviewer comments #Patch Set 5 : Merged the loops in the InverseFFT function #Patch Set 6 : Merged with latest master #Patch Set 7 : Moved for loop variable declaration to beginning of function to be compliant with the old C standar… #Patch Set 8 : Removed gcc build-breaking method argument specifiers #
Depends on Patchset: Messages
Total messages: 40 (17 generated)
Description was changed from ========== Removed the state as an input to the FilterAdaptation function BUG=webrtc:5201 ========== to ========== Removed the state as an input to the FilterAdaptation function BUG=webrtc:5201 ==========
peah@webrtc.org changed reviewers: + henrik.lundin@webrtc.org, ivoc@webrtc.org
Description was changed from ========== Removed the state as an input to the FilterAdaptation function BUG=webrtc:5201 ========== to ========== -Removed the state as an input to the FilterAdaptation function. -Renamed the TimeToFrequency and FrequencyToTime functions. -Moved the windowing from the TimeToFrequency function. -Simplified the EchoSubtraction function. Note that the aec state is still an input to the EchoSubtraction function, and it currently needs to be that in order to support the output of the debug file. The longer-term goal is, however, to order the state into substates. This will simplify the parameter lists to the EchoCancellation function as well as replace the aec state as a parameter BUG=webrtc:5201 ==========
LG. I have the same two comments repeating: 1. When you add/modify function parameters, make their name follow the_naming_convention. 2. Consider which of the input array parameters that can be made const. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core.c (right): https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:213: int xfBufBlockPos, Rename parameters. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:214: float xfBuf[2][kExtendedNumPartitions * PART_LEN1], Any of the array input that can be made const? https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:829: static void InverseFft(float freq_data[2][PART_LEN1], Make freq_data const. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:841: static void Fft(float time_data[PART_LEN2], Make time_data const. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:937: int xfBufBlockPos, Change naming format of input parameters. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:944: float xPow[PART_LEN1], Can you make any array parameters const? https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core_internal.h (right): https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core_internal.h:183: float xPow[PART_LEN1], x_pow https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core_internal.h:183: float xPow[PART_LEN1], Can x_pow be made const? https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core_mips.c (right): https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core_mips.c:442: int xfBufBlockPos, Naming format. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core_mips.c:443: float xfBuf[2][kExtendedNumPartitions * PART_LEN1], Any const arrays? https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core_sse2.c (right): https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core_sse2.c:159: const int num_partitions_local = num_partitions; Is the local one needed?
A few comments, See below. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core.c (right): https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:841: static void Fft(float time_data[PART_LEN2], On 2015/11/20 11:55:20, hlundin-webrtc wrote: > Make time_data const. I think the aec_rdft_forward_128 function writes the output over the input (and destoys the input in the process). To be honest, I would prefer a fft function that writes the output directly to an output array, so the copy action below can be avoided as well, but I guess this was already part of the code. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:970: s[i] *= scale; Should scaling be part of the InverseFft function? In my opinion it makes sense. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:975: e[i] = y[i] - s[i]; Loop can be merged with previous loop. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:980: memcpy(e_extended + PART_LEN, e, sizeof(float) * PART_LEN); I think it makes more sense to skip the e signal, and fill e_extended directly in the previous loop. Especially since e does not seem to be used anywhere else.
https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core.c (right): https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:213: int xfBufBlockPos, On 2015/11/20 11:55:20, hlundin-webrtc wrote: > Rename parameters. Done. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:214: float xfBuf[2][kExtendedNumPartitions * PART_LEN1], On 2015/11/20 11:55:20, hlundin-webrtc wrote: > Any of the array input that can be made const? I checked with kwiberg@ and he said it should be fine. I'll re-revise the ScaleErrorSignal in the previous CL function accordingly as well in an upcoming CL. Done. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:829: static void InverseFft(float freq_data[2][PART_LEN1], On 2015/11/20 11:55:20, hlundin-webrtc wrote: > Make freq_data const. Done. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:841: static void Fft(float time_data[PART_LEN2], On 2015/11/20 12:58:02, ivoc wrote: > On 2015/11/20 11:55:20, hlundin-webrtc wrote: > > Make time_data const. > > I think the aec_rdft_forward_128 function writes the output over the input (and > destoys the input in the process). To be honest, I would prefer a fft function > that writes the output directly to an output array, so the copy action below can > be avoided as well, but I guess this was already part of the code. That is fully correct. And you have a great point! Ideally I would have liked the fft to be stored in an interlieved manner, as that would have allowed for in-place calls which would reduce the stack-space. But as it is now, I don't think we have much choice of that as it would require a lot of refactoring. I think that if we redesign this to use a memcpy internally that would increase the complexity by one memcpy in total, but would make the code cleaner and easier to refactor. I'll make that change. Please let me know if you disagree! https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:841: static void Fft(float time_data[PART_LEN2], On 2015/11/20 11:55:20, hlundin-webrtc wrote: > Make time_data const. Done. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:937: int xfBufBlockPos, On 2015/11/20 11:55:20, hlundin-webrtc wrote: > Change naming format of input parameters. Done. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:944: float xPow[PART_LEN1], On 2015/11/20 11:55:20, hlundin-webrtc wrote: > Can you make any array parameters const? Yes, and I'll as a consequense also add const to the the ScaleErrorSignal parameters that I refrained to do in the earlier CL out of being overcautious. On top of that, I needed to properly revise the const-ness of the FilterFar function. Please have a look at that as well! Pls let me know if it is not ok! https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:970: s[i] *= scale; On 2015/11/20 12:58:02, ivoc wrote: > Should scaling be part of the InverseFft function? In my opinion it makes sense. I think the division by PART_LEN could go there, but not the scaling of 2.0. Looking at it from an Ifft/fft perspective I think it makes sense, and even though it is more complex I'll split the scaling in two. Please let me know if it does not make sense! Done. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:975: e[i] = y[i] - s[i]; On 2015/11/20 12:58:02, ivoc wrote: > Loop can be merged with previous loop. Done in a previous CL. Done. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:980: memcpy(e_extended + PART_LEN, e, sizeof(float) * PART_LEN); On 2015/11/20 12:58:02, ivoc wrote: > I think it makes more sense to skip the e signal, and fill e_extended directly > in the previous loop. Especially since e does not seem to be used anywhere else. I agree, but it will be used in an upcoming CL, so if ok I prefer to keep it as it is for now. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core_internal.h (right): https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core_internal.h:183: float xPow[PART_LEN1], On 2015/11/20 11:55:20, hlundin-webrtc wrote: > Can x_pow be made const? Done. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core_internal.h:183: float xPow[PART_LEN1], On 2015/11/20 11:55:20, hlundin-webrtc wrote: > x_pow Done. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core_mips.c (right): https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core_mips.c:442: int xfBufBlockPos, On 2015/11/20 11:55:20, hlundin-webrtc wrote: > Naming format. Done. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core_mips.c:443: float xfBuf[2][kExtendedNumPartitions * PART_LEN1], On 2015/11/20 11:55:20, hlundin-webrtc wrote: > Any const arrays? Done. https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core_sse2.c (right): https://codereview.webrtc.org/1456123003/diff/20001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core_sse2.c:159: const int num_partitions_local = num_partitions; On 2015/11/20 11:55:20, hlundin-webrtc wrote: > Is the local one needed? Done.
LG, but I have a few nits and a concern. https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core.c (right): https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:851: memcpy(time_data_copy, time_data, sizeof(float) * PART_LEN2); I'm not sure I like yet another memcpy of all audio data... https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core_internal.h (right): https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core_internal.h:183: const float xPow[PART_LEN1], x_pow https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core_mips.c (right): https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core_mips.c:714: const float xPow[PART_LEN1], x_pow https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core_sse2.c (right): https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core_sse2.c:86: const float xPow[PART_LEN1], x_pow
https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core.c (right): https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:851: memcpy(time_data_copy, time_data, sizeof(float) * PART_LEN2); On 2015/11/24 13:51:44, hlundin-webrtc wrote: > I'm not sure I like yet another memcpy of all audio data... I'll remote it. Done. https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core_internal.h (right): https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core_internal.h:183: const float xPow[PART_LEN1], On 2015/11/24 13:51:44, hlundin-webrtc wrote: > x_pow Done. https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core_mips.c (right): https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core_mips.c:714: const float xPow[PART_LEN1], On 2015/11/24 13:51:44, hlundin-webrtc wrote: > x_pow Done. https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core_sse2.c (right): https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core_sse2.c:86: const float xPow[PART_LEN1], On 2015/11/24 13:51:44, hlundin-webrtc wrote: > x_pow Done.
lgtm https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core.c (right): https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:851: memcpy(time_data_copy, time_data, sizeof(float) * PART_LEN2); On 2015/11/26 05:55:17, peah-webrtc wrote: > On 2015/11/24 13:51:44, hlundin-webrtc wrote: > > I'm not sure I like yet another memcpy of all audio data... > I'll remote it. > Done. Great. Btw: did you even use it?
On 2015/11/26 08:41:45, hlundin-webrtc wrote: > lgtm > > https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... > File webrtc/modules/audio_processing/aec/aec_core.c (right): > > https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... > webrtc/modules/audio_processing/aec/aec_core.c:851: memcpy(time_data_copy, > time_data, sizeof(float) * PART_LEN2); > On 2015/11/26 05:55:17, peah-webrtc wrote: > > On 2015/11/24 13:51:44, hlundin-webrtc wrote: > > > I'm not sure I like yet another memcpy of all audio data... > > I'll remote it. > > Done. > > Great. Btw: did you even use it? No, I didn't. My mistake :-)
Just one comment for me. https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core.c (right): https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:842: time_data[i] *= scale; I think it is more efficient to merge this loop with the previous one, i.e.: time_data[2*i] = freq_data[0][i] * scale;
https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core.c (right): https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:842: time_data[i] *= scale; On 2015/11/26 12:46:44, ivoc wrote: > I think it is more efficient to merge this loop with the previous one, i.e.: > time_data[2*i] = freq_data[0][i] * scale; I'll do that. Before I did not to that for fear of running into some scale problem with the FFT lib, but this should be verifiable in the bitexactness tests, so I'll try to add that. Done. https://codereview.webrtc.org/1456123003/diff/40001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.c:851: memcpy(time_data_copy, time_data, sizeof(float) * PART_LEN2); On 2015/11/26 08:41:44, hlundin-webrtc wrote: > On 2015/11/26 05:55:17, peah-webrtc wrote: > > On 2015/11/24 13:51:44, hlundin-webrtc wrote: > > > I'm not sure I like yet another memcpy of all audio data... > > I'll remote it. > > Done. > > Great. Btw: did you even use it? No. So safe removal it is! :-)
LGTM!
Thanks for a good review!
The CQ bit was checked by peah@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from henrik.lundin@webrtc.org Link to the patchset: https://codereview.webrtc.org/1456123003/#ps80001 (title: "Merged the loops in the InverseFFT function")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1456123003/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1456123003/80001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios_arm64_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/5842) ios_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/9715) mac_compile_x64_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_x64_dbg/bui...) mac_x64_gn_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_x64_gn_dbg/builds/5558) mac_x64_gn_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_x64_gn_rel/builds/5511) mac_x64_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_x64_rel/builds/10775)
The CQ bit was checked by peah@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from ivoc@webrtc.org, henrik.lundin@webrtc.org Link to the patchset: https://codereview.webrtc.org/1456123003/#ps100001 (title: "Merged with latest master")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1456123003/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1456123003/100001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_rel/builds/9372) ios_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/11065) ios_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/9719)
The CQ bit was checked by peah@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from ivoc@webrtc.org, henrik.lundin@webrtc.org Link to the patchset: https://codereview.webrtc.org/1456123003/#ps140001 (title: "Removed gcc build-breaking method argument specifiers")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1456123003/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1456123003/140001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_msan on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_msan/builds/6267)
The CQ bit was checked by peah@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1456123003/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1456123003/140001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by peah@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1456123003/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1456123003/140001
Message was sent while issue was closed.
Description was changed from ========== -Removed the state as an input to the FilterAdaptation function. -Renamed the TimeToFrequency and FrequencyToTime functions. -Moved the windowing from the TimeToFrequency function. -Simplified the EchoSubtraction function. Note that the aec state is still an input to the EchoSubtraction function, and it currently needs to be that in order to support the output of the debug file. The longer-term goal is, however, to order the state into substates. This will simplify the parameter lists to the EchoCancellation function as well as replace the aec state as a parameter BUG=webrtc:5201 ========== to ========== -Removed the state as an input to the FilterAdaptation function. -Renamed the TimeToFrequency and FrequencyToTime functions. -Moved the windowing from the TimeToFrequency function. -Simplified the EchoSubtraction function. Note that the aec state is still an input to the EchoSubtraction function, and it currently needs to be that in order to support the output of the debug file. The longer-term goal is, however, to order the state into substates. This will simplify the parameter lists to the EchoCancellation function as well as replace the aec state as a parameter BUG=webrtc:5201 ==========
Message was sent while issue was closed.
Committed patchset #8 (id:140001)
Message was sent while issue was closed.
Description was changed from ========== -Removed the state as an input to the FilterAdaptation function. -Renamed the TimeToFrequency and FrequencyToTime functions. -Moved the windowing from the TimeToFrequency function. -Simplified the EchoSubtraction function. Note that the aec state is still an input to the EchoSubtraction function, and it currently needs to be that in order to support the output of the debug file. The longer-term goal is, however, to order the state into substates. This will simplify the parameter lists to the EchoCancellation function as well as replace the aec state as a parameter BUG=webrtc:5201 ========== to ========== -Removed the state as an input to the FilterAdaptation function. -Renamed the TimeToFrequency and FrequencyToTime functions. -Moved the windowing from the TimeToFrequency function. -Simplified the EchoSubtraction function. Note that the aec state is still an input to the EchoSubtraction function, and it currently needs to be that in order to support the output of the debug file. The longer-term goal is, however, to order the state into substates. This will simplify the parameter lists to the EchoCancellation function as well as replace the aec state as a parameter BUG=webrtc:5201 Committed: https://crrev.com/7e43138c0890bd99f627fa061b122c8d5716a99d Cr-Commit-Position: refs/heads/master@{#10830} ==========
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/7e43138c0890bd99f627fa061b122c8d5716a99d Cr-Commit-Position: refs/heads/master@{#10830} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
