Pulling AEC divergent filter fraction.

Pulling AEC divergent filter fraction.

BUG=

Committed: https://crrev.com/5045337133d1da4a657b99e0590eb401515163bd
Cr-Commit-Position: refs/heads/master@{#12279}

[minyue-webrtc, 2016-04-07 11:10:31]

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

[minyue-webrtc, 2016-04-07 11:10:32]

Hi Per,

This CL facilitates the newly added metric "divergent filter fraction" to
propagate.

Would you help reviewing it?

[peah-webrtc, 2016-04-07 11:14:49]

https://codereview.webrtc.org/1862393002/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/aec/echo_cancellation.cc (right):

https://codereview.webrtc.org/1862393002/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/echo_cancellation.cc:457:
&divergent_filter_fraction);
I cannot see that divergent_filter_fraction is used anywhere.

It should be propagated to the metrics struct, right?

[minyue-webrtc, 2016-04-07 11:22:53]

https://codereview.webrtc.org/1862393002/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/aec/echo_cancellation.cc (right):

https://codereview.webrtc.org/1862393002/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/echo_cancellation.cc:457:
&divergent_filter_fraction);
On 2016/04/07 11:14:48, peah-webrtc wrote:
> I cannot see that divergent_filter_fraction is used anywhere.
> 
> It should be propagated to the metrics struct, right?

Good catch. My bad. now fixed

[peah-webrtc, 2016-04-07 11:25:40]

https://codereview.webrtc.org/1862393002/diff/20001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/include/audio_processing.h (right):

https://codereview.webrtc.org/1862393002/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/include/audio_processing.h:676: // Fraction of
filter being divergent, based on a 0.5-second, non-overlapped
I think a more proper description would be
// Fraction of the call where the filter is divergent,...

as it is now, at least to me it sounds as if  a certain part of the filter is
divergent.

[minyue-webrtc, 2016-04-07 11:40:55]

https://codereview.webrtc.org/1862393002/diff/20001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/include/audio_processing.h (right):

https://codereview.webrtc.org/1862393002/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/include/audio_processing.h:676: // Fraction of
filter being divergent, based on a 0.5-second, non-overlapped
On 2016/04/07 11:25:40, peah-webrtc wrote:
> I think a more proper description would be
> // Fraction of the call where the filter is divergent,...
> 
> as it is now, at least to me it sounds as if  a certain part of the filter is
> divergent.

Good point, I also think it is a bit vague. But "of the call" might be a bit
misleading since we use "time windows". How about "Fraction of time that filter
is divergent, in a 0.5-second, non-overlapped aggregation window"

[peah-webrtc, 2016-04-07 12:29:20]

On 2016/04/07 11:40:55, minyue-webrtc wrote:
>
https://codereview.webrtc.org/1862393002/diff/20001/webrtc/modules/audio_proc...
> File webrtc/modules/audio_processing/include/audio_processing.h (right):
> 
>
https://codereview.webrtc.org/1862393002/diff/20001/webrtc/modules/audio_proc...
> webrtc/modules/audio_processing/include/audio_processing.h:676: // Fraction of
> filter being divergent, based on a 0.5-second, non-overlapped
> On 2016/04/07 11:25:40, peah-webrtc wrote:
> > I think a more proper description would be
> > // Fraction of the call where the filter is divergent,...
> > 
> > as it is now, at least to me it sounds as if  a certain part of the filter
is
> > divergent.
> 
> Good point, I also think it is a bit vague. But "of the call" might be a bit
> misleading since we use "time windows". How about "Fraction of time that
filter
> is divergent, in a 0.5-second, non-overlapped aggregation window"

That sounds great!
With that change: lgtm

[peah-webrtc, 2016-04-07 12:29:21]

On 2016/04/07 11:40:55, minyue-webrtc wrote:
>
https://codereview.webrtc.org/1862393002/diff/20001/webrtc/modules/audio_proc...
> File webrtc/modules/audio_processing/include/audio_processing.h (right):
> 
>
https://codereview.webrtc.org/1862393002/diff/20001/webrtc/modules/audio_proc...
> webrtc/modules/audio_processing/include/audio_processing.h:676: // Fraction of
> filter being divergent, based on a 0.5-second, non-overlapped
> On 2016/04/07 11:25:40, peah-webrtc wrote:
> > I think a more proper description would be
> > // Fraction of the call where the filter is divergent,...
> > 
> > as it is now, at least to me it sounds as if  a certain part of the filter
is
> > divergent.
> 
> Good point, I also think it is a bit vague. But "of the call" might be a bit
> misleading since we use "time windows". How about "Fraction of time that
filter
> is divergent, in a 0.5-second, non-overlapped aggregation window"

That sounds great!
With that change: lgtm

[minyue-webrtc, 2016-04-07 12:35:56]

The CQ bit was checked by minyue@webrtc.org

[minyue-webrtc, 2016-04-07 12:35:56]

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/1862393002/#ps40001 (title:
"polishing a comment")

[commit-bot: I haz the power, 2016-04-07 12:36:07]

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

[commit-bot: I haz the power, 2016-04-07 13:36:48]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-07 13:36:53]

Description was changed from

==========
Pulling AEC divergent filter fraction.

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

to

==========
Pulling AEC divergent filter fraction.

BUG=

Committed: https://crrev.com/5045337133d1da4a657b99e0590eb401515163bd
Cr-Commit-Position: refs/heads/master@{#12279}
==========

[commit-bot: I haz the power, 2016-04-07 13:36:54]

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