Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1805633006:
Adding BlockMeanCalculator for AEC. (Closed)
|
Created:
4 years, 9 months ago by minyue-webrtc Modified:
4 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, minyue-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. |
DescriptionAdding BlockMeanCalculator for AEC.
This will improve the readability of AEC code.
BUG=
Committed: https://crrev.com/84db6fa7f539f59e53c5a2af6e89856505db62c0
Cr-Commit-Position: refs/heads/master@{#12123}
Patch Set 1 : #
Total comments: 19
Patch Set 2 : address comments #
Total comments: 1
Patch Set 3 : renaming files #
Total comments: 8
Patch Set 4 : adding more comments #
Total comments: 7
Patch Set 5 : adding DCHECK #Patch Set 6 : rebase #Patch Set 7 : use delete instead of free #Messages
Total messages: 48 (25 generated)
Description was changed from ========== reformat Adding MeanCalculator for AEC break to change to a new branch to merge to another branch after per's comment renaming aaa Merge branch 'temp' into aec_diverge_fraction Adding fraction of filter divergence in AEC metrics. With the current AEC algorithm, the divergence of the echo cancelling linear filter is a strong signal of non-transparency. During double talk, it can result in a ducking artifacts. In this CL, a metric that tells the fraction of filter divergence is added. This can measure the severity of non-transparency. BUG= patch from issue 1739993003 at patchset 1 (http://crrev.com/1739993003#ps1) Merge branch 'temp' into aec_diverge_fraction aa Adding fraction of filter divergence in AEC metrics. With the current AEC algorithm, the divergence of the echo cancelling linear filter is a strong signal of non-transparency. During double talk, it can result in a ducking artifacts. In this CL, a metric that tells the fraction of filter divergence is added. This can measure the severity of non-transparency. BUG= patch from issue 1739993003 at patchset 1 (http://crrev.com/1739993003#ps1) ========== to ========== Adding MeanCalculator for AEC. This will improve the readability of AEC code. BUG= ==========
minyue@webrtc.org changed reviewers: + peah@webrtc.org
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #1 (id:40001) has been deleted
Patchset #1 (id:60001) has been deleted
Hi Per, Here is a clean up on the mean calculation in audio levels. I found that it was calculated wrong (wrong window length). And the wrong result is protected by our unittest. I do not plan to fix the unittest in this work, since there are a few other changes planned, after which, we can fix the unittest at once. Thanks in advance for reviewing!
Nice! The code definitely becomes cleaner! I have some comments though. https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core.cc (right): https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.cc:579: if (level->framelevel.GetNumberNewSamples() == 0) { I see what you want to achieve with the GetNumberNewSamples call, but it look really strange when the test is like this. When reading this code without knowing the implementation of MeanCalculator it sounds as if the mean is collected every time there are no new samples, which to me does not make sense. I think it would be better if you renamed GetNumberNewSamples() to NumberOfSamplesSinceLastMeanComputation() or something along that line. https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.cc:581: if (new_frame_level > 0) { Why is this if statement there? The result is that the minimum can never be 0, right? https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.cc:1428: Intended new line? https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/utility/mean_calculator.cc (right): https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/utility/mean_calculator.cc:32: // line should become "++count_ == window_length_" when bit exactness test in What do you mean by this todo? As far as I can see, the following line is what you state. https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/utility/mean_calculator.cc:34: if (++count_ == window_length_) { Please move the increement outside of the if-statement. I think it increases the readability and it is also easier when attaching a debugger. https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/utility/mean_calculator.cc:55: void MeanCalculator::KahanSum(float sample) { I don't think we need to compute the sum using KahanSum. That method is at least 5 times as complex as a simple add and I doubt that we need it any longer since we are no longer doing recursive windowed averages. At any rate, I don't think it should be there if there is no clear need for it. https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/utility/mean_calculator.h (right): https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/utility/mean_calculator.h:17: #include "webrtc/base/criticalsection.h" criticalsection.h should be after before constructormagic.h https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/utility/mean_calculator.h:23: // MeanCalculator calculates the mean of a sample sequence of a certain length. I think it is better if this class is about the mean of values rather than of samples. It is actually used in the code for storing levels and not samples. Please also change the name of the member variables according to the term you choose. https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/utility/mean_calculator.h:24: class MeanCalculator { As I said in the previous review I think you should change the class name to better reflect what it does. https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/utility/mean_calculator.h:35: size_t GetNumberNewSamples() const; What about GetNumberOfNewSamples()?
Patchset #3 (id:120001) has been deleted
Hi Per, Thanks for the review! I've made changes. To make your review easier, Patch set 2 does the actual change and patch set 3 does renaming of the files. This way, you can do side-by-side review. https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/aec/aec_core.cc (right): https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.cc:579: if (level->framelevel.GetNumberNewSamples() == 0) { On 2016/03/18 09:18:34, peah-webrtc wrote: > I see what you want to achieve with the GetNumberNewSamples call, but it look > really strange when the test is like this. > > When reading this code without knowing the implementation of MeanCalculator it > sounds as if the mean is collected every time there are no new samples, which to > me does not make sense. > > I think it would be better if you renamed GetNumberNewSamples() to > NumberOfSamplesSinceLastMeanComputation() or something along that line. I changed to a similar but slightly shorter name. https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.cc:581: if (new_frame_level > 0) { On 2016/03/18 09:18:34, peah-webrtc wrote: > Why is this if statement there? The result is that the minimum can never be 0, > right? Good question, this is the way it was. I guess it is to make sure minlevel never collapse to zero. https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/aec/aec_core.cc:1428: On 2016/03/18 09:18:34, peah-webrtc wrote: > Intended new line? no, I guess it was due to mistake or formatter. https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/utility/mean_calculator.cc (right): https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/utility/mean_calculator.cc:32: // line should become "++count_ == window_length_" when bit exactness test in On 2016/03/18 09:18:34, peah-webrtc wrote: > What do you mean by this todo? As far as I can see, the following line is what > you state. Sorry. I forgot to remove this when I came up with a better solution. https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/utility/mean_calculator.cc:34: if (++count_ == window_length_) { On 2016/03/18 09:18:34, peah-webrtc wrote: > Please move the increement outside of the if-statement. I think it increases the > readability and it is also easier when attaching a debugger. Done. https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/utility/mean_calculator.cc:55: void MeanCalculator::KahanSum(float sample) { On 2016/03/18 09:18:34, peah-webrtc wrote: > I don't think we need to compute the sum using KahanSum. That method is at least > 5 times as complex as a simple add and I doubt that we need it any longer since > we are no longer doing recursive windowed averages. At any rate, I don't think > it should be there if there is no clear need for it. KahanSum is better in accuracy in any long-window sum, regardless of recursive or not. But yes, it is more complex. I can remove it, if accuracy is not a big deal (and it should not be, since inaccuracy has been in many other places.). https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/utility/mean_calculator.h (right): https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/utility/mean_calculator.h:17: #include "webrtc/base/criticalsection.h" On 2016/03/18 09:18:34, peah-webrtc wrote: > criticalsection.h should be after before constructormagic.h Done. https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/utility/mean_calculator.h:23: // MeanCalculator calculates the mean of a sample sequence of a certain length. On 2016/03/18 09:18:34, peah-webrtc wrote: > I think it is better if this class is about the mean of values rather than of > samples. It is actually used in the code for storing levels and not samples. > Please also change the name of the member variables according to the term you > choose. In statistics, sample has its own meaning, and should not be confused with "audio sample". If you think it can still be confusing, I am fine with changing it to "value". https://codereview.webrtc.org/1805633006/diff/80001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/utility/mean_calculator.h:35: size_t GetNumberNewSamples() const; On 2016/03/18 09:18:34, peah-webrtc wrote: > What about GetNumberOfNewSamples()? Done. https://codereview.webrtc.org/1805633006/diff/100001/webrtc/modules/audio_pro... File webrtc/modules/audio_processing/utility/mean_calculator.h (right): https://codereview.webrtc.org/1805633006/diff/100001/webrtc/modules/audio_pro... webrtc/modules/audio_processing/utility/mean_calculator.h:20: // MeanCalculator calculates the mean of a block of samples. Samples are added This should be called BlockMeanCalculator. I will not forget it.
Hi Per, Do you have more comments on this?
https://codereview.webrtc.org/1805633006/diff/140001/webrtc/modules/audio_pro... File webrtc/modules/audio_processing/aec/aec_core.cc (right): https://codereview.webrtc.org/1805633006/diff/140001/webrtc/modules/audio_pro... webrtc/modules/audio_processing/aec/aec_core.cc:624: const float linout_average_level = Please move this closer to where it is used. https://codereview.webrtc.org/1805633006/diff/140001/webrtc/modules/audio_pro... webrtc/modules/audio_processing/aec/aec_core.cc:626: const float nlpout_average_level = Please move this closer to where it is used. https://codereview.webrtc.org/1805633006/diff/140001/webrtc/modules/audio_pro... File webrtc/modules/audio_processing/utility/block_mean_calculator.cc (right): https://codereview.webrtc.org/1805633006/diff/140001/webrtc/modules/audio_pro... webrtc/modules/audio_processing/utility/block_mean_calculator.cc:27: void BlockMeanCalculator::AddSample(float sample) { I think it is better to not call this sample and the class is more general than not just to apply to samples (and it is not used for storing samples in aec_core.cc). What about value resp AddValue? https://codereview.webrtc.org/1805633006/diff/140001/webrtc/modules/audio_pro... webrtc/modules/audio_processing/utility/block_mean_calculator.cc:36: size_t BlockMeanCalculator::SamplesSinceLastUpdate() const { To me the usage of this method requires a lot of understanding of the class internals to be able to use. I.e., you need to know that 1) The class produces a new mean in a periodic manner. 2) You need to know that it resets the counter for the number of values added when the new mean is produced. Is there another way to do this that would be better? What about bool BlockMeanCalculator::NewMeanAvailable() const?
Thanks for he comments! I have a new patch now. https://codereview.webrtc.org/1805633006/diff/140001/webrtc/modules/audio_pro... File webrtc/modules/audio_processing/aec/aec_core.cc (right): https://codereview.webrtc.org/1805633006/diff/140001/webrtc/modules/audio_pro... webrtc/modules/audio_processing/aec/aec_core.cc:624: const float linout_average_level = On 2016/03/24 07:18:41, peah-webrtc wrote: > Please move this closer to where it is used. Done. https://codereview.webrtc.org/1805633006/diff/140001/webrtc/modules/audio_pro... webrtc/modules/audio_processing/aec/aec_core.cc:626: const float nlpout_average_level = On 2016/03/24 07:18:41, peah-webrtc wrote: > Please move this closer to where it is used. Done. https://codereview.webrtc.org/1805633006/diff/140001/webrtc/modules/audio_pro... File webrtc/modules/audio_processing/utility/block_mean_calculator.cc (right): https://codereview.webrtc.org/1805633006/diff/140001/webrtc/modules/audio_pro... webrtc/modules/audio_processing/utility/block_mean_calculator.cc:27: void BlockMeanCalculator::AddSample(float sample) { On 2016/03/24 07:18:41, peah-webrtc wrote: > I think it is better to not call this sample and the class is more general than > not just to apply to samples (and it is not used for storing samples in > aec_core.cc). What about value resp AddValue? Done. https://codereview.webrtc.org/1805633006/diff/140001/webrtc/modules/audio_pro... webrtc/modules/audio_processing/utility/block_mean_calculator.cc:36: size_t BlockMeanCalculator::SamplesSinceLastUpdate() const { On 2016/03/24 07:18:41, peah-webrtc wrote: > To me the usage of this method requires a lot of understanding of the class > internals to be able to use. I.e., you need to know that > 1) The class produces a new mean in a periodic manner. > 2) You need to know that it resets the counter for the number of values added > when the new mean is produced. > > Is there another way to do this that would be better? What about > bool BlockMeanCalculator::NewMeanAvailable() const? Per offline discussion, we think if it better to use EndOfBlock()
https://codereview.webrtc.org/1805633006/diff/160001/webrtc/modules/audio_pro... File webrtc/modules/audio_processing/aec/aec_core.cc (right): https://codereview.webrtc.org/1805633006/diff/160001/webrtc/modules/audio_pro... webrtc/modules/audio_processing/aec/aec_core.cc:618: // Estimate in active far-end segments only Please move the comment so that it is not inside the if-statement. Also, add a "." to the end of the comment (https://google.github.io/styleguide/cppguide.html#Implementation_Comments) https://codereview.webrtc.org/1805633006/diff/160001/webrtc/modules/audio_pro... File webrtc/modules/audio_processing/utility/block_mean_calculator.cc (right): https://codereview.webrtc.org/1805633006/diff/160001/webrtc/modules/audio_pro... webrtc/modules/audio_processing/utility/block_mean_calculator.cc:19: mean_(0.0) { You should add a DCHECK on the block_length not being zero. https://codereview.webrtc.org/1805633006/diff/160001/webrtc/modules/audio_pro... webrtc/modules/audio_processing/utility/block_mean_calculator.cc:31: mean_ = sum_ / block_length_; You should add a DCHECK on the block_length not being zero. https://codereview.webrtc.org/1805633006/diff/160001/webrtc/modules/audio_pro... webrtc/modules/audio_processing/utility/block_mean_calculator.cc:36: bool BlockMeanCalculator::EndOfBlock() const { Another suggestion instead of "EndOfBlock" could be "FullBlockCollected". What do you think of that? I think that is a bit more clear.
Description was changed from ========== Adding MeanCalculator for AEC. This will improve the readability of AEC code. BUG= ========== to ========== Adding BlockMeanCalculator for AEC. This will improve the readability of AEC code. BUG= ==========
Thanks, Per! Now comes another patch. https://codereview.webrtc.org/1805633006/diff/160001/webrtc/modules/audio_pro... File webrtc/modules/audio_processing/utility/block_mean_calculator.cc (right): https://codereview.webrtc.org/1805633006/diff/160001/webrtc/modules/audio_pro... webrtc/modules/audio_processing/utility/block_mean_calculator.cc:19: mean_(0.0) { On 2016/03/24 11:21:58, peah-webrtc wrote: > You should add a DCHECK on the block_length not being zero. Done. https://codereview.webrtc.org/1805633006/diff/160001/webrtc/modules/audio_pro... webrtc/modules/audio_processing/utility/block_mean_calculator.cc:31: mean_ = sum_ / block_length_; On 2016/03/24 11:21:58, peah-webrtc wrote: > You should add a DCHECK on the block_length not being zero. since block_length_ is qualified const, it is enough to check in ctor, I think. https://codereview.webrtc.org/1805633006/diff/160001/webrtc/modules/audio_pro... webrtc/modules/audio_processing/utility/block_mean_calculator.cc:36: bool BlockMeanCalculator::EndOfBlock() const { On 2016/03/24 11:21:58, peah-webrtc wrote: > Another suggestion instead of "EndOfBlock" could be "FullBlockCollected". What > do you think of that? I think that is a bit more clear. I prefer EndOfBlock. Let's consider what people would expect when adding another value after a whole block. Naturally, EndOfBlock should return false, since the new added value does not end a block. But it is not obvious that FullBlockCollected should return false, since one may argue that a full block has been collected.
On 2016/03/24 11:43:33, minyue-webrtc wrote: > Thanks, Per! Now comes another patch. > > https://codereview.webrtc.org/1805633006/diff/160001/webrtc/modules/audio_pro... > File webrtc/modules/audio_processing/utility/block_mean_calculator.cc (right): > > https://codereview.webrtc.org/1805633006/diff/160001/webrtc/modules/audio_pro... > webrtc/modules/audio_processing/utility/block_mean_calculator.cc:19: mean_(0.0) > { > On 2016/03/24 11:21:58, peah-webrtc wrote: > > You should add a DCHECK on the block_length not being zero. > > Done. > > https://codereview.webrtc.org/1805633006/diff/160001/webrtc/modules/audio_pro... > webrtc/modules/audio_processing/utility/block_mean_calculator.cc:31: mean_ = > sum_ / block_length_; > On 2016/03/24 11:21:58, peah-webrtc wrote: > > You should add a DCHECK on the block_length not being zero. > > since block_length_ is qualified const, it is enough to check in ctor, I think. > > https://codereview.webrtc.org/1805633006/diff/160001/webrtc/modules/audio_pro... > webrtc/modules/audio_processing/utility/block_mean_calculator.cc:36: bool > BlockMeanCalculator::EndOfBlock() const { > On 2016/03/24 11:21:58, peah-webrtc wrote: > > Another suggestion instead of "EndOfBlock" could be "FullBlockCollected". What > > do you think of that? I think that is a bit more clear. > > I prefer EndOfBlock. Let's consider what people would expect when adding another > value after a whole block. > > Naturally, EndOfBlock should return false, since the new added value does not > end a block. > > But it is not obvious that FullBlockCollected should return false, since one may > argue that a full block has been collected. lgtm
On 2016/03/24 11:43:33, minyue-webrtc wrote: > Thanks, Per! Now comes another patch. > > https://codereview.webrtc.org/1805633006/diff/160001/webrtc/modules/audio_pro... > File webrtc/modules/audio_processing/utility/block_mean_calculator.cc (right): > > https://codereview.webrtc.org/1805633006/diff/160001/webrtc/modules/audio_pro... > webrtc/modules/audio_processing/utility/block_mean_calculator.cc:19: mean_(0.0) > { > On 2016/03/24 11:21:58, peah-webrtc wrote: > > You should add a DCHECK on the block_length not being zero. > > Done. > > https://codereview.webrtc.org/1805633006/diff/160001/webrtc/modules/audio_pro... > webrtc/modules/audio_processing/utility/block_mean_calculator.cc:31: mean_ = > sum_ / block_length_; > On 2016/03/24 11:21:58, peah-webrtc wrote: > > You should add a DCHECK on the block_length not being zero. > > since block_length_ is qualified const, it is enough to check in ctor, I think. > > https://codereview.webrtc.org/1805633006/diff/160001/webrtc/modules/audio_pro... > webrtc/modules/audio_processing/utility/block_mean_calculator.cc:36: bool > BlockMeanCalculator::EndOfBlock() const { > On 2016/03/24 11:21:58, peah-webrtc wrote: > > Another suggestion instead of "EndOfBlock" could be "FullBlockCollected". What > > do you think of that? I think that is a bit more clear. > > I prefer EndOfBlock. Let's consider what people would expect when adding another > value after a whole block. > > Naturally, EndOfBlock should return false, since the new added value does not > end a block. > > But it is not obvious that FullBlockCollected should return false, since one may > argue that a full block has been collected. lgtm
The CQ bit was checked by minyue@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1805633006/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1805633006/180001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_clang_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_clang_dbg/build...) android_compile_x86_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_compile_x86_dbg...) ios32_sim_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_dbg/builds/6230) ios64_sim_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_dbg/builds/6218) ios_arm64_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/8556) ios_arm64_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/8475) ios_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/13758) ios_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/12389) linux_gn_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_gn_rel/builds/9649) linux_tsan2 on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_tsan2/builds/10905) mac_asan on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_asan/builds/13410) mac_gn_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_dbg/builds/1871) mac_gn_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_rel/builds/8100) mac_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_rel/builds/13802) presubmit on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/4398)
The CQ bit was checked by minyue@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from peah@webrtc.org Link to the patchset: https://codereview.webrtc.org/1805633006/#ps200001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1805633006/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1805633006/200001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_clang_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_clang_dbg/build...) android_compile_x86_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_compile_x86_dbg...) android_gn_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_gn_dbg/builds/9790) ios32_sim_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_dbg/builds/6232) ios64_sim_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_dbg/builds/6220) ios_arm64_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/8558) ios_arm64_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/8477) ios_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/13760) ios_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/12391) linux_asan on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_asan/builds/13494) linux_compile_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_compile_dbg/build...) linux_gn_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_gn_rel/builds/9651) linux_tsan2 on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_tsan2/builds/10907) linux_ubsan on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_ubsan/builds/791) linux_ubsan_vptr on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_ubsan_vptr/builds...) mac_asan on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_asan/builds/13412) mac_compile_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...) mac_gn_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_rel/builds/8102) mac_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_rel/builds/13804)
Patchset #6 (id:200001) has been deleted
The CQ bit was checked by minyue@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from peah@webrtc.org Link to the patchset: https://codereview.webrtc.org/1805633006/#ps220001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1805633006/220001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1805633006/220001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_asan on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_asan/builds/13496)
The CQ bit was checked by minyue@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from peah@webrtc.org Link to the patchset: https://codereview.webrtc.org/1805633006/#ps240001 (title: "use delete instead of free")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1805633006/240001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1805633006/240001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_dbg on tryserver.webrtc (JOB_TIMED_OUT, no build URL) android_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL) linux_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL) mac_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL) win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by minyue@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1805633006/240001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1805633006/240001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL) linux_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL) win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by minyue@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1805633006/240001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1805633006/240001
Message was sent while issue was closed.
Description was changed from ========== Adding BlockMeanCalculator for AEC. This will improve the readability of AEC code. BUG= ========== to ========== Adding BlockMeanCalculator for AEC. This will improve the readability of AEC code. BUG= ==========
Message was sent while issue was closed.
Committed patchset #7 (id:240001)
Message was sent while issue was closed.
Description was changed from ========== Adding BlockMeanCalculator for AEC. This will improve the readability of AEC code. BUG= ========== to ========== Adding BlockMeanCalculator for AEC. This will improve the readability of AEC code. BUG= Committed: https://crrev.com/84db6fa7f539f59e53c5a2af6e89856505db62c0 Cr-Commit-Position: refs/heads/master@{#12123} ==========
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/84db6fa7f539f59e53c5a2af6e89856505db62c0 Cr-Commit-Position: refs/heads/master@{#12123} |
