Reland "Revert "audio_processing/aec: make delay estimator aware of starving farend buffer""

Reland "Revert "audio_processing/aec: make delay estimator aware of starving farend buffer""

Original review at https://codereview.webrtc.org/1180423006

SystemDelayTests was not updated w.r.t. extended_filter mode and some tests were disabled on Android since DA-AEC is automatically set.
All tests have now been updated for both extended_filter mode as well as DA-AEC, hence are now enabled on Android.

Also
* Moves default settings of extended_filter and DA-AEC form Init() to Create() to avoid unintentional loss of state during a reset.
* Fixes a potential bug of starting from scratch in extended_filter mode + DA-AEC.

This reverts commit 01c9b012e9171c813ace9e405c32fc75f4262bf6.

BUG=
R=henrik.lundin@webrtc.org

Committed: https://chromium.googlesource.com/external/webrtc/+/7101269c61267879b1cc028c2cccc963c2680a17

[bjornv1, 2015-06-17 12:55:58]

bjornv@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org

[bjornv1, 2015-06-17 13:03:56]

Henrik, a new and improved CL. PTAL.
The test diff is kind of messy since I now loop over both extended_filter and
DA-AEC for some tests.

https://codereview.webrtc.org/1187943005/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/aec/aec_core.c (right):

https://codereview.webrtc.org/1187943005/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core.c:1455: aec->reported_delay_enabled
= 0;  // DA-AEC enabled by default.
These are moved from Init()

https://codereview.webrtc.org/1187943005/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/aec/echo_cancellation.c (left):

https://codereview.webrtc.org/1187943005/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/echo_cancellation.c:790: target_delay /= 2;
This should have produced a compile warning.

[hlundin-webrtc, 2015-06-17 14:09:19]

lgtm with nits.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/aec/echo_cancellation.c (right):

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/echo_cancellation.c:238: // We skip the
startup_phase completely if DA-AEC is enabled, but not
Please, clarify that skipping means setting the value to zero.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/aec/system_delay_unittest.cc (right):

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:197:
EXPECT_EQ(WebRtcAec_extended_filter_enabled(self_->aec), extended_filter);
The parameters should be swapped, right?
EXPECT_EQ(expected_val, tested_val)

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:200:
EXPECT_EQ(WebRtcAec_reported_delay_enabled(self_->aec), !da_aec);
Swap parameters.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:224:
EXPECT_EQ(WebRtcAec_extended_filter_enabled(self_->aec), extended_filter);
Swap parameters.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:227:
EXPECT_EQ(WebRtcAec_reported_delay_enabled(self_->aec), !da_aec);
Swap parameters.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:250: // the first
10 ms chunk to determine a reasonable buffer size. Neither does
does does

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:253:
EXPECT_EQ(WebRtcAec_extended_filter_enabled(self_->aec), 0);
Swap parameters.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:255:
EXPECT_EQ(WebRtcAec_reported_delay_enabled(self_->aec), 1);
Swap parameters.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:299: // the first
10 ms chunk to determine a reasonable buffer size. Neither does
does does

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:302:
EXPECT_EQ(WebRtcAec_extended_filter_enabled(self_->aec), 0);
Swap parameters.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:304:
EXPECT_EQ(WebRtcAec_reported_delay_enabled(self_->aec), 1);
Swap parameters.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:367:
EXPECT_EQ(WebRtcAec_extended_filter_enabled(self_->aec), extended_filter);
Swap parameters.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:370:
EXPECT_EQ(WebRtcAec_reported_delay_enabled(self_->aec), !da_aec);
Swap parameters.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:397:
EXPECT_EQ(WebRtcAec_extended_filter_enabled(self_->aec), extended_filter);
Swap parameters.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:400:
EXPECT_EQ(WebRtcAec_reported_delay_enabled(self_->aec), !da_aec);
Swap parameters.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:440:
EXPECT_EQ(WebRtcAec_extended_filter_enabled(self_->aec), extended_filter);
Swap parameters.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:443:
EXPECT_EQ(WebRtcAec_reported_delay_enabled(self_->aec), !da_aec);
Swap parameters.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:495:
EXPECT_EQ(WebRtcAec_extended_filter_enabled(self_->aec), 0);
Swap parameters.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:500:
EXPECT_EQ(WebRtcAec_reported_delay_enabled(self_->aec), !da_aec);
Swap parameters.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:550:
EXPECT_EQ(WebRtcAec_extended_filter_enabled(self_->aec), extended_filter);
Swap parameters.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:553:
EXPECT_EQ(WebRtcAec_reported_delay_enabled(self_->aec), !da_aec);
Swap parameters.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:555: break;
I think "continue" is more correct than "break". In practice it makes no
difference in this case...

[bjornv1, 2015-06-17 14:25:19]

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/aec/echo_cancellation.c (right):

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/echo_cancellation.c:238: // We skip the
startup_phase completely if DA-AEC is enabled, but not
On 2015/06/17 14:09:18, hlundin-webrtc wrote:
> Please, clarify that skipping means setting the value to zero.

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/aec/system_delay_unittest.cc (right):

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:197:
EXPECT_EQ(WebRtcAec_extended_filter_enabled(self_->aec), extended_filter);
On 2015/06/17 14:09:18, hlundin-webrtc wrote:
> The parameters should be swapped, right?
> EXPECT_EQ(expected_val, tested_val)

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:200:
EXPECT_EQ(WebRtcAec_reported_delay_enabled(self_->aec), !da_aec);
On 2015/06/17 14:09:18, hlundin-webrtc wrote:
> Swap parameters.

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:224:
EXPECT_EQ(WebRtcAec_extended_filter_enabled(self_->aec), extended_filter);
On 2015/06/17 14:09:18, hlundin-webrtc wrote:
> Swap parameters.

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:227:
EXPECT_EQ(WebRtcAec_reported_delay_enabled(self_->aec), !da_aec);
On 2015/06/17 14:09:19, hlundin-webrtc wrote:
> Swap parameters.

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:250: // the first
10 ms chunk to determine a reasonable buffer size. Neither does
On 2015/06/17 14:09:18, hlundin-webrtc wrote:
> does does

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:253:
EXPECT_EQ(WebRtcAec_extended_filter_enabled(self_->aec), 0);
On 2015/06/17 14:09:19, hlundin-webrtc wrote:
> Swap parameters.

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:255:
EXPECT_EQ(WebRtcAec_reported_delay_enabled(self_->aec), 1);
On 2015/06/17 14:09:19, hlundin-webrtc wrote:
> Swap parameters.

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:299: // the first
10 ms chunk to determine a reasonable buffer size. Neither does
On 2015/06/17 14:09:19, hlundin-webrtc wrote:
> does does

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:302:
EXPECT_EQ(WebRtcAec_extended_filter_enabled(self_->aec), 0);
On 2015/06/17 14:09:19, hlundin-webrtc wrote:
> Swap parameters.

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:304:
EXPECT_EQ(WebRtcAec_reported_delay_enabled(self_->aec), 1);
On 2015/06/17 14:09:19, hlundin-webrtc wrote:
> Swap parameters.

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:367:
EXPECT_EQ(WebRtcAec_extended_filter_enabled(self_->aec), extended_filter);
On 2015/06/17 14:09:19, hlundin-webrtc wrote:
> Swap parameters.

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:370:
EXPECT_EQ(WebRtcAec_reported_delay_enabled(self_->aec), !da_aec);
On 2015/06/17 14:09:18, hlundin-webrtc wrote:
> Swap parameters.

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:397:
EXPECT_EQ(WebRtcAec_extended_filter_enabled(self_->aec), extended_filter);
On 2015/06/17 14:09:19, hlundin-webrtc wrote:
> Swap parameters.

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:400:
EXPECT_EQ(WebRtcAec_reported_delay_enabled(self_->aec), !da_aec);
On 2015/06/17 14:09:19, hlundin-webrtc wrote:
> Swap parameters.

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:440:
EXPECT_EQ(WebRtcAec_extended_filter_enabled(self_->aec), extended_filter);
On 2015/06/17 14:09:19, hlundin-webrtc wrote:
> Swap parameters.

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:443:
EXPECT_EQ(WebRtcAec_reported_delay_enabled(self_->aec), !da_aec);
On 2015/06/17 14:09:19, hlundin-webrtc wrote:
> Swap parameters.

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:495:
EXPECT_EQ(WebRtcAec_extended_filter_enabled(self_->aec), 0);
On 2015/06/17 14:09:19, hlundin-webrtc wrote:
> Swap parameters.

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:500:
EXPECT_EQ(WebRtcAec_reported_delay_enabled(self_->aec), !da_aec);
On 2015/06/17 14:09:19, hlundin-webrtc wrote:
> Swap parameters.

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:550:
EXPECT_EQ(WebRtcAec_extended_filter_enabled(self_->aec), extended_filter);
On 2015/06/17 14:09:19, hlundin-webrtc wrote:
> Swap parameters.

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:553:
EXPECT_EQ(WebRtcAec_reported_delay_enabled(self_->aec), !da_aec);
On 2015/06/17 14:09:19, hlundin-webrtc wrote:
> Swap parameters.

Done.

https://codereview.webrtc.org/1187943005/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/system_delay_unittest.cc:555: break;
On 2015/06/17 14:09:19, hlundin-webrtc wrote:
> I think "continue" is more correct than "break". In practice it makes no
> difference in this case...

Done.

[hlundin-webrtc, 2015-06-18 07:21:31]

still lgtm

[bjornv1, 2015-06-18 07:32:10]

On 2015/06/18 07:21:31, hlundin-webrtc wrote:
> still lgtm

no it does not, just ran it on N7 and still one piece to solve... :(

[bjornv1, 2015-06-18 08:09:43]

Since the Android bots that are actually running the tests are spammed, I tested
locally on Nexus 7, with a pass.

So please take a (final) look.

[hlundin-webrtc, 2015-06-18 08:55:37]

lgtm

[bjornv1, 2015-06-18 09:05:09]

Committed patchset #5 (id:80001) manually as
7101269c61267879b1cc028c2cccc963c2680a17 (presubmit successful).