Some minor (bitexact) AEC echo suppressor refactoring (#2)

Some minor (bitexact) AEC echo suppressor refactoring
-Moved filter reset from the echo suppression
 into the echo subtraction code where it belongs
 (the echo subtractor should own its filter reset).
-Moved the selection between using the microphone sinal and
 the echo subtractor output down to the lowest level in the
 EchoSuppression function. This makes sense as that selection
 was very hidden in an unrelated sub-sub-function call and
 as the selection is critical for what the AEC outputs.

The changes have been tested for bitexactness.

BUG=webrtc:5201
Committed: https://crrev.com/b14f00113ed054a94f406fb58348e2cedf098cc0
Cr-Commit-Position: refs/heads/master@{#10956}

[peah-webrtc, 2015-12-03 10:25:28]

Description was changed from

==========
Some minor (bitexact) AEC echo suppressor refactoring
-Moved filter reset from the echo suppression
 into the echo subtraction code where it belongs
 (the echo subtractor should own its filter reset).
-Moved the selection between using the microphone sinal and
 the echo subtractor output down to the lowest level in the
 EchoSuppression function. This makes sense as that selection
 was very hidden in an unrelated sub-sub-function call and
 as the selection is critical for what the AEC outputs.

The changes have been tested for bitexactness.


BUG=webrtc:5201
==========

to

==========
Some minor (bitexact) AEC echo suppressor refactoring
-Moved filter reset from the echo suppression
 into the echo subtraction code where it belongs
 (the echo subtractor should own its filter reset).
-Moved the selection between using the microphone sinal and
 the echo subtractor output down to the lowest level in the
 EchoSuppression function. This makes sense as that selection
 was very hidden in an unrelated sub-sub-function call and
 as the selection is critical for what the AEC outputs.

The changes have been tested for bitexactness.


BUG=webrtc:5201
==========

[peah-webrtc, 2015-12-03 10:25:29]

peah@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org, minyue@webrtc.org

[minyue-webrtc, 2015-12-04 10:11:39]

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

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core.c:950:
aec->esup_detected_extreme_filter_divergence) {
Error

extreme_filter_divergence may not be
aec->esup_detected_extreme_filter_divergence

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/aec/aec_core_internal.h (right):

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core_internal.h:157: int
esup_detected_extreme_filter_divergence;
what is esup_?

extreme_filter_divergence may be clear enough

[hlundin-webrtc, 2015-12-04 10:27:56]

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

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core.c:336: int *const
extreme_filter_divergence) {
This const declaration doesn't give much, in my opinion.

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core.c:416: int *const
extreme_filter_divergence) {
Skip const.

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core.c:954: 
Remove (at least) one blank line.

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core.c:1085: 
Remove (at least) one blank line.

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core.c:1087: // Select the microphone
signal as output if the filter is deemed to have
Is this new behavior? Or did I miss where you took it from?

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core.c:1092: 
Remove (at least) one blank line.

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/aec/aec_core_internal.h (right):

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core_internal.h:157: int
esup_detected_extreme_filter_divergence;
On 2015/12/04 10:11:39, minyue-webrtc wrote:
> what is esup_?
> 
> extreme_filter_divergence may be clear enough

I guess esup_ is echo suppressor. But unless we have another
extreme_filter_divergence elsewhere, I don't think the extra qualification is
needed.

[peah-webrtc, 2015-12-04 22:11:48]

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

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core.c:336: int *const
extreme_filter_divergence) {
On 2015/12/04 10:27:55, hlundin-webrtc wrote:
> This const declaration doesn't give much, in my opinion.

Removed.
Done.

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core.c:416: int *const
extreme_filter_divergence) {
On 2015/12/04 10:27:55, hlundin-webrtc wrote:
> Skip const.

Done.

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core.c:950:
aec->esup_detected_extreme_filter_divergence) {
On 2015/12/04 10:11:38, minyue-webrtc wrote:
> Error
> 
> extreme_filter_divergence may not be
> aec->esup_detected_extreme_filter_divergence

Could you please elaborate? I don't see the error.

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core.c:954: 
On 2015/12/04 10:27:55, hlundin-webrtc wrote:
> Remove (at least) one blank line.

Done.

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core.c:1085: 
On 2015/12/04 10:27:55, hlundin-webrtc wrote:
> Remove (at least) one blank line.

Done.

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core.c:1087: // Select the microphone
signal as output if the filter is deemed to have
On 2015/12/04 10:27:55, hlundin-webrtc wrote:
> Is this new behavior? Or did I miss where you took it from?

This is moved from the SmoothedPSD function where the divergeState is computed.
So no new behavior.

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core.c:1092: 
On 2015/12/04 10:27:55, hlundin-webrtc wrote:
> Remove (at least) one blank line.

Done.

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/aec/aec_core_internal.h (right):

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core_internal.h:157: int
esup_detected_extreme_filter_divergence;
On 2015/12/04 10:27:55, hlundin-webrtc wrote:
> On 2015/12/04 10:11:39, minyue-webrtc wrote:
> > what is esup_?
> > 
> > extreme_filter_divergence may be clear enough
> 
> I guess esup_ is echo suppressor. But unless we have another
> extreme_filter_divergence elsewhere, I don't think the extra qualification is
> needed.

True, I will change that!

Done.

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core_internal.h:157: int
esup_detected_extreme_filter_divergence;
On 2015/12/04 10:11:39, minyue-webrtc wrote:
> what is esup_?
> 
> extreme_filter_divergence may be clear enough

Agree! Will change.

Done.

[minyue-webrtc, 2015-12-05 20:55:20]

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

https://codereview.webrtc.org/1499573003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core.c:950:
aec->esup_detected_extreme_filter_divergence) {
On 2015/12/04 22:11:48, peah-webrtc wrote:
> On 2015/12/04 10:11:38, minyue-webrtc wrote:
> > Error
> > 
> > extreme_filter_divergence may not be
> > aec->esup_detected_extreme_filter_divergence
> 
> Could you please elaborate? I don't see the error. 

I think the folding made me think this lines belonged to SubbandCoherence(),
where extreme_filter_divergence is an argument. Never mind.

[hlundin-webrtc, 2015-12-08 11:51:11]

lgtm with nits.

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

https://codereview.webrtc.org/1499573003/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/aec_core_neon.c:514: int
*extreme_filter_divergence) {
int* extreme...

https://codereview.webrtc.org/1499573003/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/aec_core_neon.c:682: int
*extreme_filter_divergence) {
int* extreme...

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

https://codereview.webrtc.org/1499573003/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/aec_core_sse2.c:493: int
*extreme_filter_divergence) {
int* extreme...

https://codereview.webrtc.org/1499573003/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/aec/aec_core_sse2.c:668: int
*extreme_filter_divergence) {
int* extreme...

[minyue-webrtc, 2015-12-08 12:28:19]

lgtm

[peah-webrtc, 2015-12-09 17:42:05]

The CQ bit was checked by peah@webrtc.org

[peah-webrtc, 2015-12-09 17:42:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
henrik.lundin@webrtc.org, minyue@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1499573003/#ps40001 (title:
"Corrected the positions of the pointer indicators")

[commit-bot: I haz the power, 2015-12-09 17:42:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1499573003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1499573003/40001

[commit-bot: I haz the power, 2015-12-09 19:07:24]

Description was changed from

==========
Some minor (bitexact) AEC echo suppressor refactoring
-Moved filter reset from the echo suppression
 into the echo subtraction code where it belongs
 (the echo subtractor should own its filter reset).
-Moved the selection between using the microphone sinal and
 the echo subtractor output down to the lowest level in the
 EchoSuppression function. This makes sense as that selection
 was very hidden in an unrelated sub-sub-function call and
 as the selection is critical for what the AEC outputs.

The changes have been tested for bitexactness.


BUG=webrtc:5201
==========

to

==========
Some minor (bitexact) AEC echo suppressor refactoring
-Moved filter reset from the echo suppression
 into the echo subtraction code where it belongs
 (the echo subtractor should own its filter reset).
-Moved the selection between using the microphone sinal and
 the echo subtractor output down to the lowest level in the
 EchoSuppression function. This makes sense as that selection
 was very hidden in an unrelated sub-sub-function call and
 as the selection is critical for what the AEC outputs.

The changes have been tested for bitexactness.


BUG=webrtc:5201
==========

[commit-bot: I haz the power, 2015-12-09 19:07:24]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-12-09 19:07:31]

Description was changed from

==========
Some minor (bitexact) AEC echo suppressor refactoring
-Moved filter reset from the echo suppression
 into the echo subtraction code where it belongs
 (the echo subtractor should own its filter reset).
-Moved the selection between using the microphone sinal and
 the echo subtractor output down to the lowest level in the
 EchoSuppression function. This makes sense as that selection
 was very hidden in an unrelated sub-sub-function call and
 as the selection is critical for what the AEC outputs.

The changes have been tested for bitexactness.


BUG=webrtc:5201
==========

to

==========
Some minor (bitexact) AEC echo suppressor refactoring
-Moved filter reset from the echo suppression
 into the echo subtraction code where it belongs
 (the echo subtractor should own its filter reset).
-Moved the selection between using the microphone sinal and
 the echo subtractor output down to the lowest level in the
 EchoSuppression function. This makes sense as that selection
 was very hidden in an unrelated sub-sub-function call and
 as the selection is critical for what the AEC outputs.

The changes have been tested for bitexactness.

BUG=webrtc:5201

Committed: https://crrev.com/b14f00113ed054a94f406fb58348e2cedf098cc0
Cr-Commit-Position: refs/heads/master@{#10956}
==========

[commit-bot: I haz the power, 2015-12-09 19:07:32]

Patchset 3 (id:??) landed as
https://crrev.com/b14f00113ed054a94f406fb58348e2cedf098cc0
Cr-Commit-Position: refs/heads/master@{#10956}