|
|
Created:
4 years, 9 months ago by aluebs-webrtc Modified:
4 years, 9 months ago Reviewers:
peah-webrtc CC:
webrtc-reviews_webrtc.org, Andrew MacDonald, henrika_webrtc, 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. |
DescriptionReland: Drop the 16kHz sample rate restriction on AECM and zero out higher bands
Landed originally here: https://codereview.webrtc.org/1774553002/
Revertede here: https://codereview.webrtc.org/1781893002/
TBR=solenberg@webrtc.org, tina.legrand@webrtc.org
Committed: https://crrev.com/776593b1399a448de3886301e0614921fdd58892
Cr-Commit-Position: refs/heads/master@{#12005}
Patch Set 1 #Patch Set 2 : Fix Android bots #Patch Set 3 : More Android fixing #Patch Set 4 : Inf SNR should not fail #
Total comments: 12
Patch Set 5 : Rebasing #
Messages
Total messages: 17 (6 generated)
Description was changed from ========== Reland: Drop the 16kHz sample rate restriction on AECM and zero out higher bands Landed originally here: https://codereview.webrtc.org/1774553002/ Revertede here: https://codereview.webrtc.org/1781893002/ ========== to ========== Reland: Drop the 16kHz sample rate restriction on AECM and zero out higher bands Landed originally here: https://codereview.webrtc.org/1774553002/ Revertede here: https://codereview.webrtc.org/1781893002/ TBR=solenberg@webrtc.org, tina.legrand@webrtc.org ==========
aluebs@webrtc.org changed reviewers: + peah@webrtc.org
peah, this CL was already reviewed, but please have a look at the patches that fix the breaks on Android.
https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/echo_control_mobile_impl.cc (right): https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/echo_control_mobile_impl.cc:321: Regarding proc_split_sample_rate_hz(), can that ever be higher than 16 kHz? Furthermore, can that be anything other than 16 kHz? https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/test/audio_processing_unittest.cc (right): https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/test/audio_processing_unittest.cc:2638: std::cout << "SNR=inf dB" << std::endl; From offline discussions, I got the impression that this is needed as for some platforms the 48 kHz processing is done in 32 kHz internally and then upsampled to 48 kHz, right? I don't see why that should cause this change to be needed? It seem like this change narrows the scope of the test. I'd rather have expected a change in the test inputs that specifically for this platforms specifies a different behavior. Afaics, this change makes the test less specific for the platforms where the 48 kHz processing is actually done in 48 kHz internally. I don't know this test code that well, but I think that only a flaw in the actual test should motivate a weakening of the testcases thare are currently in place.
https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/echo_control_mobile_impl.cc (right): https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/echo_control_mobile_impl.cc:321: On 2016/03/11 11:34:39, peah-webrtc wrote: > Regarding proc_split_sample_rate_hz(), can that ever be higher than 16 kHz? > Furthermore, can that be anything other than 16 kHz? As it is today, it is always 16kHz except when the sample_rate is 8kHz, then the split_sample_rate is also 8kHz. https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/test/audio_processing_unittest.cc (right): https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/test/audio_processing_unittest.cc:2638: std::cout << "SNR=inf dB" << std::endl; On 2016/03/11 11:34:39, peah-webrtc wrote: > From offline discussions, I got the impression that this is needed as for some > platforms the 48 kHz processing is done in 32 kHz internally and then upsampled > to 48 kHz, right? > > I don't see why that should cause this change to be needed? It seem like this > change narrows the scope of the test. I'd rather have expected a change in the > test inputs that specifically for this platforms specifies a different behavior. > > Afaics, this change makes the test less specific for the platforms where the 48 > kHz processing is actually done in 48 kHz internally. > I don't know this test code that well, but I think that only a flaw in the > actual test should motivate a weakening of the testcases thare are currently in > place. This is not a weaken of the test. When we specify the minimum expected SNR for each test case, we happen to use "0" to represent an infinite SNR, which is weird. If the actual SNR is infinite (no error), any minimum expected SNR specified should be ok, since infinite is larger than any number. This is necessary for the case when certain platforms have an infinite SNR and other have not. There we specify the minimum, but we don't want the infinite SNR breaking.
https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/echo_control_mobile_impl.cc (right): https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/echo_control_mobile_impl.cc:321: On 2016/03/11 13:27:05, aluebs-webrtc wrote: > On 2016/03/11 11:34:39, peah-webrtc wrote: > > Regarding proc_split_sample_rate_hz(), can that ever be higher than 16 kHz? > > Furthermore, can that be anything other than 16 kHz? > > As it is today, it is always 16kHz except when the sample_rate is 8kHz, then the > split_sample_rate is also 8kHz. We really need to change the names of these functions. In my mind, split rate relates to the rate of the split signal. And the 8 and 16 kHz are not split so then the name is quite misleading. https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/test/audio_processing_unittest.cc (right): https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/test/audio_processing_unittest.cc:2638: std::cout << "SNR=inf dB" << std::endl; On 2016/03/11 13:27:05, aluebs-webrtc wrote: > On 2016/03/11 11:34:39, peah-webrtc wrote: > > From offline discussions, I got the impression that this is needed as for some > > platforms the 48 kHz processing is done in 32 kHz internally and then > upsampled > > to 48 kHz, right? > > > > I don't see why that should cause this change to be needed? It seem like this > > change narrows the scope of the test. I'd rather have expected a change in the > > test inputs that specifically for this platforms specifies a different > behavior. > > > > Afaics, this change makes the test less specific for the platforms where the > 48 > > kHz processing is actually done in 48 kHz internally. > > I don't know this test code that well, but I think that only a flaw in the > > actual test should motivate a weakening of the testcases thare are currently > in > > place. > > This is not a weaken of the test. When we specify the minimum expected SNR for > each test case, we happen to use "0" to represent an infinite SNR, which is > weird. If the actual SNR is infinite (no error), any minimum expected SNR > specified should be ok, since infinite is larger than any number. This is > necessary for the case when certain platforms have an infinite SNR and other > have not. There we specify the minimum, but we don't want the infinite SNR > breaking. I actually think that is a weakening of the test. What you do is to increase the range of all platforms as certain platforms behave differently. And furthermore, there is an EXPECT_EQ which is removed when sq_error ==0. https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/test/audio_processing_unittest.cc:2733: std::tr1::make_tuple(32000, 48000, 48000, 48000, 35, 0), Why did these values need to be changed? Are these only used when AECM is active?
https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/echo_control_mobile_impl.cc (right): https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/echo_control_mobile_impl.cc:321: On 2016/03/11 13:27:05, aluebs-webrtc wrote: > On 2016/03/11 11:34:39, peah-webrtc wrote: > > Regarding proc_split_sample_rate_hz(), can that ever be higher than 16 kHz? > > Furthermore, can that be anything other than 16 kHz? > > As it is today, it is always 16kHz except when the sample_rate is 8kHz, then the > split_sample_rate is also 8kHz. We really need to change the names of these functions. In my mind, split rate relates to the rate of the split signal. And the 8 and 16 kHz are not split so then the name is quite misleading. https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/test/audio_processing_unittest.cc (right): https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/test/audio_processing_unittest.cc:2638: std::cout << "SNR=inf dB" << std::endl; On 2016/03/11 13:27:05, aluebs-webrtc wrote: > On 2016/03/11 11:34:39, peah-webrtc wrote: > > From offline discussions, I got the impression that this is needed as for some > > platforms the 48 kHz processing is done in 32 kHz internally and then > upsampled > > to 48 kHz, right? > > > > I don't see why that should cause this change to be needed? It seem like this > > change narrows the scope of the test. I'd rather have expected a change in the > > test inputs that specifically for this platforms specifies a different > behavior. > > > > Afaics, this change makes the test less specific for the platforms where the > 48 > > kHz processing is actually done in 48 kHz internally. > > I don't know this test code that well, but I think that only a flaw in the > > actual test should motivate a weakening of the testcases thare are currently > in > > place. > > This is not a weaken of the test. When we specify the minimum expected SNR for > each test case, we happen to use "0" to represent an infinite SNR, which is > weird. If the actual SNR is infinite (no error), any minimum expected SNR > specified should be ok, since infinite is larger than any number. This is > necessary for the case when certain platforms have an infinite SNR and other > have not. There we specify the minimum, but we don't want the infinite SNR > breaking. I actually think that is a weakening of the test. What you do is to increase the range of all platforms as certain platforms behave differently. And furthermore, there is an EXPECT_EQ which is removed when sq_error ==0. https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/test/audio_processing_unittest.cc:2733: std::tr1::make_tuple(32000, 48000, 48000, 48000, 35, 0), Why did these values need to be changed? Are these only used when AECM is active?
https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/echo_control_mobile_impl.cc (right): https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/echo_control_mobile_impl.cc:321: On 2016/03/11 21:37:28, peah-webrtc wrote: > On 2016/03/11 13:27:05, aluebs-webrtc wrote: > > On 2016/03/11 11:34:39, peah-webrtc wrote: > > > Regarding proc_split_sample_rate_hz(), can that ever be higher than 16 kHz? > > > Furthermore, can that be anything other than 16 kHz? > > > > As it is today, it is always 16kHz except when the sample_rate is 8kHz, then > the > > split_sample_rate is also 8kHz. > > We really need to change the names of these functions. In my mind, split rate > relates to the rate of the split signal. And the 8 and 16 kHz are not split so > then the name is quite misleading. I agree it probably can have a better name, but the rename should be done in a separate CL. https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/test/audio_processing_unittest.cc (right): https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/test/audio_processing_unittest.cc:2638: std::cout << "SNR=inf dB" << std::endl; On 2016/03/11 21:37:28, peah-webrtc wrote: > On 2016/03/11 13:27:05, aluebs-webrtc wrote: > > On 2016/03/11 11:34:39, peah-webrtc wrote: > > > From offline discussions, I got the impression that this is needed as for > some > > > platforms the 48 kHz processing is done in 32 kHz internally and then > > upsampled > > > to 48 kHz, right? > > > > > > I don't see why that should cause this change to be needed? It seem like > this > > > change narrows the scope of the test. I'd rather have expected a change in > the > > > test inputs that specifically for this platforms specifies a different > > behavior. > > > > > > Afaics, this change makes the test less specific for the platforms where the > > 48 > > > kHz processing is actually done in 48 kHz internally. > > > I don't know this test code that well, but I think that only a flaw in the > > > actual test should motivate a weakening of the testcases thare are currently > > in > > > place. > > > > This is not a weaken of the test. When we specify the minimum expected SNR for > > each test case, we happen to use "0" to represent an infinite SNR, which is > > weird. If the actual SNR is infinite (no error), any minimum expected SNR > > specified should be ok, since infinite is larger than any number. This is > > necessary for the case when certain platforms have an infinite SNR and other > > have not. There we specify the minimum, but we don't want the infinite SNR > > breaking. > > I actually think that is a weakening of the test. What you do is to increase the > range of all platforms as certain platforms behave differently. And furthermore, > there is an EXPECT_EQ which is removed when sq_error ==0. > > I see that a EXPECT_EQ has been removed, but it is one that didn't make sense in the first place. Let me try to explain using an example. As it is today, if in one platform the SNR is 20dB and on another 30dB, we just use 20dB as a minimum SNR and both test pass. Similarly if one platform has a SNR of 30dB and the other one infdB (so an sq_error of 0) it would make sense to use 30dB as the minimum SNR (since inf is higher than 30) and have both test pass, but it would actually fail because in the inf case it would EXPECT_EQ the minimum SNR to be 0 (our weird way of representing inf). And when having a restriction of 32kHz for ARM it makes sense that only on those platforms the SNR is below inf, so this would be a valid use-case. If you still think think we should keep the EXPECT_EQ, I am happy to revert this test to always use 16kHz (as it was before) and have less coverage, but in my opinion that would be sub-optimal. Else I am open to suggestions of how we should handle this case. https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/test/audio_processing_unittest.cc:2733: std::tr1::make_tuple(32000, 48000, 48000, 48000, 35, 0), On 2016/03/11 21:37:28, peah-webrtc wrote: > Why did these values need to be changed? Are these only used when AECM is > active? Here I only improved the SNRs, because the test stopped being limited to the 16kHz because of the AECM.
On 2016/03/14 17:30:40, aluebs-webrtc wrote: > https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... > File webrtc/modules/audio_processing/echo_control_mobile_impl.cc (right): > > https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... > webrtc/modules/audio_processing/echo_control_mobile_impl.cc:321: > On 2016/03/11 21:37:28, peah-webrtc wrote: > > On 2016/03/11 13:27:05, aluebs-webrtc wrote: > > > On 2016/03/11 11:34:39, peah-webrtc wrote: > > > > Regarding proc_split_sample_rate_hz(), can that ever be higher than 16 > kHz? > > > > Furthermore, can that be anything other than 16 kHz? > > > > > > As it is today, it is always 16kHz except when the sample_rate is 8kHz, then > > the > > > split_sample_rate is also 8kHz. > > > > We really need to change the names of these functions. In my mind, split rate > > relates to the rate of the split signal. And the 8 and 16 kHz are not split so > > then the name is quite misleading. > > I agree it probably can have a better name, but the rename should be done in a > separate CL. > > https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... > File webrtc/modules/audio_processing/test/audio_processing_unittest.cc (right): > > https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... > webrtc/modules/audio_processing/test/audio_processing_unittest.cc:2638: > std::cout << "SNR=inf dB" << std::endl; > On 2016/03/11 21:37:28, peah-webrtc wrote: > > On 2016/03/11 13:27:05, aluebs-webrtc wrote: > > > On 2016/03/11 11:34:39, peah-webrtc wrote: > > > > From offline discussions, I got the impression that this is needed as for > > some > > > > platforms the 48 kHz processing is done in 32 kHz internally and then > > > upsampled > > > > to 48 kHz, right? > > > > > > > > I don't see why that should cause this change to be needed? It seem like > > this > > > > change narrows the scope of the test. I'd rather have expected a change in > > the > > > > test inputs that specifically for this platforms specifies a different > > > behavior. > > > > > > > > Afaics, this change makes the test less specific for the platforms where > the > > > 48 > > > > kHz processing is actually done in 48 kHz internally. > > > > I don't know this test code that well, but I think that only a flaw in the > > > > actual test should motivate a weakening of the testcases thare are > currently > > > in > > > > place. > > > > > > This is not a weaken of the test. When we specify the minimum expected SNR > for > > > each test case, we happen to use "0" to represent an infinite SNR, which is > > > weird. If the actual SNR is infinite (no error), any minimum expected SNR > > > specified should be ok, since infinite is larger than any number. This is > > > necessary for the case when certain platforms have an infinite SNR and other > > > have not. There we specify the minimum, but we don't want the infinite SNR > > > breaking. > > > > I actually think that is a weakening of the test. What you do is to increase > the > > range of all platforms as certain platforms behave differently. And > furthermore, > > there is an EXPECT_EQ which is removed when sq_error ==0. > > > > > > I see that a EXPECT_EQ has been removed, but it is one that didn't make sense in > the first place. Let me try to explain using an example. As it is today, if in > one platform the SNR is 20dB and on another 30dB, we just use 20dB as a minimum > SNR and both test pass. Similarly if one platform has a SNR of 30dB and the > other one infdB (so an sq_error of 0) it would make sense to use 30dB as the > minimum SNR (since inf is higher than 30) and have both test pass, but it would > actually fail because in the inf case it would EXPECT_EQ the minimum SNR to be 0 > (our weird way of representing inf). And when having a restriction of 32kHz for > ARM it makes sense that only on those platforms the SNR is below inf, so this > would be a valid use-case. If you still think think we should keep the > EXPECT_EQ, I am happy to revert this test to always use 16kHz (as it was before) > and have less coverage, but in my opinion that would be sub-optimal. Else I am > open to suggestions of how we should handle this case. > > https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... > webrtc/modules/audio_processing/test/audio_processing_unittest.cc:2733: > std::tr1::make_tuple(32000, 48000, 48000, 48000, 35, 0), > On 2016/03/11 21:37:28, peah-webrtc wrote: > > Why did these values need to be changed? Are these only used when AECM is > > active? > > Here I only improved the SNRs, because the test stopped being limited to the > 16kHz because of the AECM. Nice work! lgtm
https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... File webrtc/modules/audio_processing/test/audio_processing_unittest.cc (right): https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/test/audio_processing_unittest.cc:2638: std::cout << "SNR=inf dB" << std::endl; On 2016/03/14 17:30:40, aluebs-webrtc wrote: > On 2016/03/11 21:37:28, peah-webrtc wrote: > > On 2016/03/11 13:27:05, aluebs-webrtc wrote: > > > On 2016/03/11 11:34:39, peah-webrtc wrote: > > > > From offline discussions, I got the impression that this is needed as for > > some > > > > platforms the 48 kHz processing is done in 32 kHz internally and then > > > upsampled > > > > to 48 kHz, right? > > > > > > > > I don't see why that should cause this change to be needed? It seem like > > this > > > > change narrows the scope of the test. I'd rather have expected a change in > > the > > > > test inputs that specifically for this platforms specifies a different > > > behavior. > > > > > > > > Afaics, this change makes the test less specific for the platforms where > the > > > 48 > > > > kHz processing is actually done in 48 kHz internally. > > > > I don't know this test code that well, but I think that only a flaw in the > > > > actual test should motivate a weakening of the testcases thare are > currently > > > in > > > > place. > > > > > > This is not a weaken of the test. When we specify the minimum expected SNR > for > > > each test case, we happen to use "0" to represent an infinite SNR, which is > > > weird. If the actual SNR is infinite (no error), any minimum expected SNR > > > specified should be ok, since infinite is larger than any number. This is > > > necessary for the case when certain platforms have an infinite SNR and other > > > have not. There we specify the minimum, but we don't want the infinite SNR > > > breaking. > > > > I actually think that is a weakening of the test. What you do is to increase > the > > range of all platforms as certain platforms behave differently. And > furthermore, > > there is an EXPECT_EQ which is removed when sq_error ==0. > > > > > > I see that a EXPECT_EQ has been removed, but it is one that didn't make sense in > the first place. Let me try to explain using an example. As it is today, if in > one platform the SNR is 20dB and on another 30dB, we just use 20dB as a minimum > SNR and both test pass. Similarly if one platform has a SNR of 30dB and the > other one infdB (so an sq_error of 0) it would make sense to use 30dB as the > minimum SNR (since inf is higher than 30) and have both test pass, but it would > actually fail because in the inf case it would EXPECT_EQ the minimum SNR to be 0 > (our weird way of representing inf). And when having a restriction of 32kHz for > ARM it makes sense that only on those platforms the SNR is below inf, so this > would be a valid use-case. If you still think think we should keep the > EXPECT_EQ, I am happy to revert this test to always use 16kHz (as it was before) > and have less coverage, but in my opinion that would be sub-optimal. Else I am > open to suggestions of how we should handle this case. Thanks for the explanation! Now I get it :-) Awesome change!!! https://codereview.webrtc.org/1777093004/diff/60001/webrtc/modules/audio_proc... webrtc/modules/audio_processing/test/audio_processing_unittest.cc:2733: std::tr1::make_tuple(32000, 48000, 48000, 48000, 35, 0), On 2016/03/14 17:30:40, aluebs-webrtc wrote: > On 2016/03/11 21:37:28, peah-webrtc wrote: > > Why did these values need to be changed? Are these only used when AECM is > > active? > > Here I only improved the SNRs, because the test stopped being limited to the > 16kHz because of the AECM. Awesome! Thanks for the explanation!
The CQ bit was checked by aluebs@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/1777093004/#ps80001 (title: "Rebasing")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1777093004/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1777093004/80001
Message was sent while issue was closed.
Description was changed from ========== Reland: Drop the 16kHz sample rate restriction on AECM and zero out higher bands Landed originally here: https://codereview.webrtc.org/1774553002/ Revertede here: https://codereview.webrtc.org/1781893002/ TBR=solenberg@webrtc.org, tina.legrand@webrtc.org ========== to ========== Reland: Drop the 16kHz sample rate restriction on AECM and zero out higher bands Landed originally here: https://codereview.webrtc.org/1774553002/ Revertede here: https://codereview.webrtc.org/1781893002/ TBR=solenberg@webrtc.org, tina.legrand@webrtc.org ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Reland: Drop the 16kHz sample rate restriction on AECM and zero out higher bands Landed originally here: https://codereview.webrtc.org/1774553002/ Revertede here: https://codereview.webrtc.org/1781893002/ TBR=solenberg@webrtc.org, tina.legrand@webrtc.org ========== to ========== Reland: Drop the 16kHz sample rate restriction on AECM and zero out higher bands Landed originally here: https://codereview.webrtc.org/1774553002/ Revertede here: https://codereview.webrtc.org/1781893002/ TBR=solenberg@webrtc.org, tina.legrand@webrtc.org Committed: https://crrev.com/776593b1399a448de3886301e0614921fdd58892 Cr-Commit-Position: refs/heads/master@{#12005} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/776593b1399a448de3886301e0614921fdd58892 Cr-Commit-Position: refs/heads/master@{#12005} |