Calculating ERLE in AEC more properly.

Calculating ERLE in AEC more properly.

The audio level of the AEC's output level was calculated before overlapping add, and therefore, a compensation was needed. The compensation is multiplying the level by 2 since, before overlapping add, the level is roughly halved due to windowing.

This had to be that way because the level was calculated in frequency domain and the signal after overlapping add has only its time domain representation.

The level calculation has been updated to work on time domain signal and therefore the problem is not there any longer.

This CL is to put the calculation of the AEC output level after overlapping add and remove the compensation.

BUG=
R=peah@webrtc.org

Committed: https://crrev.com/944744b25c76810e576516d2f676b1d9105e302f
Cr-Commit-Position: refs/heads/master@{#11810}

[minyue-webrtc, 2016-01-29 13:35:00]

Description was changed from

==========
Calculating ERLE properly.

BUG=
==========

to

==========
Calculating ERLE in AEC more properly.

The audio level of the AEC's output level was calculated before overlapping add,
and therefore, a compensation was needed. The compensation is multiplying the
level by 2 since, before overlapping add, the level is roughly halved due to
windowing.

This had to be that way because the level was calculated in frequency domain and
the signal after overlapping add has only its time domain representation.

The level calculation has been updated to work on time domain signal and
therefore the problem is not there any longer.

This CL is to put the calculation of the AEC output level after overlapping add
and remove the compensation.

BUG=
==========

[minyue-webrtc, 2016-01-29 13:35:00]

minyue@webrtc.org changed reviewers:
+ peah@webrtc.org

[minyue-webrtc, 2016-01-29 13:37:43]

Hi Per,

There is no hurry on this CL, but I'd like you to review it.

The unittest that checks the exactness of the metrics becomes loose now. We can
rewrite the reference files at some point. Since there can be some more changes
in this field, I prefer that we change files at all these work.

[peah-webrtc, 2016-02-10 09:57:28]

On 2016/01/29 13:37:43, minyue-webrtc wrote:
> Hi Per,
> 
> There is no hurry on this CL, but I'd like you to review it.
> 
> The unittest that checks the exactness of the metrics becomes loose now. We
can
> rewrite the reference files at some point. Since there can be some more
changes
> in this field, I prefer that we change files at all these work.

The changes looks good, but what do you mean that the unittest becomes loose?
Does the test fail with these changes?

[minyue-webrtc, 2016-02-22 08:47:40]

On 2016/02/10 09:57:28, peah-webrtc wrote:
> On 2016/01/29 13:37:43, minyue-webrtc wrote:
> > Hi Per,
> > 
> > There is no hurry on this CL, but I'd like you to review it.
> > 
> > The unittest that checks the exactness of the metrics becomes loose now. We
> can
> > rewrite the reference files at some point. Since there can be some more
> changes
> > in this field, I prefer that we change files at all these work.
> 
> The changes looks good, but what do you mean that the unittest becomes loose?
> Does the test fail with these changes?

As you can see in the changes in audio_processing_unittest.cc, the values differ
2 (dB) at the most from the original values

[peah-webrtc, 2016-02-22 09:04:22]

https://codereview.webrtc.org/1644133002/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/test/audio_processing_unittest.cc (left):

https://codereview.webrtc.org/1644133002/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/test/audio_processing_unittest.cc:206:
EXPECT_NEAR(reference.instant(), test.instant, 1);
I think it would be better if you could change the reference rather than change
the distance. The inaccuracy does not come from that the new computation has a
lower precision, but rather that the computation is now differently done.
Therefore it would make sense to change the reference (as it is that one that
has actually changed) an keep 1 as the numerical tolerance. Does that make
sense?

[minyue-webrtc, 2016-02-22 09:10:51]

On 2016/02/22 09:04:22, peah-webrtc wrote:
>
https://codereview.webrtc.org/1644133002/diff/1/webrtc/modules/audio_processi...
> File webrtc/modules/audio_processing/test/audio_processing_unittest.cc (left):
> 
>
https://codereview.webrtc.org/1644133002/diff/1/webrtc/modules/audio_processi...
> webrtc/modules/audio_processing/test/audio_processing_unittest.cc:206:
> EXPECT_NEAR(reference.instant(), test.instant, 1);
> I think it would be better if you could change the reference rather than
change
> the distance. The inaccuracy does not come from that the new computation has a
> lower precision, but rather that the computation is now differently done.
> Therefore it would make sense to change the reference (as it is that one that
> has actually changed) an keep 1 as the numerical tolerance. Does that make
> sense?

True. I actually plan to change the ref values. However, since there can be a
couple of changes in a row, it would be best to make a change at the end.

Now, the test is to show that no error is made in the calculation, since the new
values should not be too far from the old ones.

[minyue-webrtc, 2016-02-26 14:06:37]

rebasing after aec_core.c becoming aec_core.cc.

[peah-webrtc, 2016-02-29 09:25:28]

On 2016/02/22 09:10:51, minyue-webrtc wrote:
> On 2016/02/22 09:04:22, peah-webrtc wrote:
> >
>
https://codereview.webrtc.org/1644133002/diff/1/webrtc/modules/audio_processi...
> > File webrtc/modules/audio_processing/test/audio_processing_unittest.cc
(left):
> > 
> >
>
https://codereview.webrtc.org/1644133002/diff/1/webrtc/modules/audio_processi...
> > webrtc/modules/audio_processing/test/audio_processing_unittest.cc:206:
> > EXPECT_NEAR(reference.instant(), test.instant, 1);
> > I think it would be better if you could change the reference rather than
> change
> > the distance. The inaccuracy does not come from that the new computation has
a
> > lower precision, but rather that the computation is now differently done.
> > Therefore it would make sense to change the reference (as it is that one
that
> > has actually changed) an keep 1 as the numerical tolerance. Does that make
> > sense?
> 
> True. I actually plan to change the ref values. However, since there can be a
> couple of changes in a row, it would be best to make a change at the end.
> 
> Now, the test is to show that no error is made in the calculation, since the
new
> values should not be too far from the old ones.

lgtm conditioned than an upcoming CL is created to change the reference values.

[minyue-webrtc, 2016-02-29 12:09:12]

Description was changed from

==========
Calculating ERLE in AEC more properly.

The audio level of the AEC's output level was calculated before overlapping add,
and therefore, a compensation was needed. The compensation is multiplying the
level by 2 since, before overlapping add, the level is roughly halved due to
windowing.

This had to be that way because the level was calculated in frequency domain and
the signal after overlapping add has only its time domain representation.

The level calculation has been updated to work on time domain signal and
therefore the problem is not there any longer.

This CL is to put the calculation of the AEC output level after overlapping add
and remove the compensation.

BUG=
==========

to

==========
Calculating ERLE in AEC more properly.

The audio level of the AEC's output level was calculated before overlapping add,
and therefore, a compensation was needed. The compensation is multiplying the
level by 2 since, before overlapping add, the level is roughly halved due to
windowing.

This had to be that way because the level was calculated in frequency domain and
the signal after overlapping add has only its time domain representation.

The level calculation has been updated to work on time domain signal and
therefore the problem is not there any longer.

This CL is to put the calculation of the AEC output level after overlapping add
and remove the compensation.

BUG=
R=peah@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/944744b25c76810e576516d2f...
==========

[minyue-webrtc, 2016-02-29 12:09:13]

Committed patchset #2 (id:20001) manually as
944744b25c76810e576516d2f676b1d9105e302f (presubmit successful).

[commit-bot: I haz the power, 2016-02-29 12:09:17]

Description was changed from

==========
Calculating ERLE in AEC more properly.

The audio level of the AEC's output level was calculated before overlapping add,
and therefore, a compensation was needed. The compensation is multiplying the
level by 2 since, before overlapping add, the level is roughly halved due to
windowing.

This had to be that way because the level was calculated in frequency domain and
the signal after overlapping add has only its time domain representation.

The level calculation has been updated to work on time domain signal and
therefore the problem is not there any longer.

This CL is to put the calculation of the AEC output level after overlapping add
and remove the compensation.

BUG=
R=peah@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/944744b25c76810e576516d2f...
==========

to

==========
Calculating ERLE in AEC more properly.

The audio level of the AEC's output level was calculated before overlapping add,
and therefore, a compensation was needed. The compensation is multiplying the
level by 2 since, before overlapping add, the level is roughly halved due to
windowing.

This had to be that way because the level was calculated in frequency domain and
the signal after overlapping add has only its time domain representation.

The level calculation has been updated to work on time domain signal and
therefore the problem is not there any longer.

This CL is to put the calculation of the AEC output level after overlapping add
and remove the compensation.

BUG=
R=peah@webrtc.org

Committed: https://crrev.com/944744b25c76810e576516d2f676b1d9105e302f
Cr-Commit-Position: refs/heads/master@{#11810}
==========

[commit-bot: I haz the power, 2016-02-29 12:09:18]

Patchset 2 (id:??) landed as
https://crrev.com/944744b25c76810e576516d2f676b1d9105e302f
Cr-Commit-Position: refs/heads/master@{#11810}