Rename APM Config ReportedDelay to DelayAgnostic

Rename APM Config ReportedDelay to DelayAgnostic

We use this Config struct for enabling/disabling the delay agnostic
AEC. This change renames it to DelayAgnostic for readability reasons.

NOTE: The logic is reversed in this CL. The old ReportedDelay config
turned DA-AEC off, while the new DelayAgnostic turns it on.

The old Config is kept in parallel with the new during a transition
period. This is to avoid problems with API breakages. During this
period, ReportedDelay is disabled or DelayAgnostic is enabled, DA-AEC
is engaged in APM.

BUG=webrtc:4651
R=bjornv@webrtc.org, tommi@webrtc.org

Committed: https://crrev.com/0f133b99c655cbdb347b4a71ac872c071532189f
Cr-Commit-Position: refs/heads/master@{#9531}

[hlundin-webrtc, 2015-07-01 13:51:03]

Björn, Tommi,

Please, take a look.

I tested it in Chrome already, to avoid the same kind of problems I ran into
with the other config renaming I did.

Thanks!

[tommi, 2015-07-01 14:00:45]

lgtm. just have a qq

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

https://codereview.webrtc.org/1211053006/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core_internal.h:149: int
delay_agnostic_enabled;
why do we use int for these?
(e.g. why not int foo:1 assuming this is C code)

[bjornv1, 2015-07-01 18:13:35]

lgtm if you fix the error, see comment.

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

https://codereview.webrtc.org/1211053006/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core.c:1455: aec->delay_agnostic_enabled
= 0;  // DA-AEC enabled by default.
You should change 0 to 1. Otherwise it will break on Androids, where the Config
isn't set.

[hlundin-webrtc, 2015-07-02 06:35:57]

Fix error in android test

[hlundin-webrtc, 2015-07-02 06:36:31]

Thanks!

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

https://codereview.webrtc.org/1211053006/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core.c:1455: aec->delay_agnostic_enabled
= 0;  // DA-AEC enabled by default.
On 2015/07/01 18:13:34, bjornv1 wrote:
> You should change 0 to 1. Otherwise it will break on Androids, where the
Config
> isn't set.

Good. You found the error I planted there. I already had a patch prepared for
that. :)

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

https://codereview.webrtc.org/1211053006/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core_internal.h:149: int
delay_agnostic_enabled;
On 2015/07/01 14:00:45, tommi (webrtc) wrote:
> why do we use int for these?
> (e.g. why not int foo:1 assuming this is C code)

Because... legacy. And for the record, int foo:1 is a bad idea, since at least
our compiler implements a signed 1-bit field to have the range {-1, 0}. This
leads to very awkward logic (0 == false, -1 == true). Changing to unsigned foo:1
works as expected.

But, I doubt that saving one int is worth the effort.

[hlundin-webrtc, 2015-07-02 06:36:38]

The CQ bit was checked by henrik.lundin@webrtc.org

[hlundin-webrtc, 2015-07-02 06:36:38]

The patchset sent to the CQ was uploaded after l-g-t-m from tommi@webrtc.org,
bjornv@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1211053006/#ps20001 (title:
"Fix error in android test")

[commit-bot: I haz the power, 2015-07-02 06:36:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1211053006/20001

[commit-bot: I haz the power, 2015-07-02 07:17:59]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-07-02 07:18:06]

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

[tommi, 2015-07-08 13:44:43]

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

https://codereview.webrtc.org/1211053006/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core_internal.h:149: int
delay_agnostic_enabled;
On 2015/07/02 06:36:30, hlundin-webrtc-VACATIONtoAUG3 wrote:
> On 2015/07/01 14:00:45, tommi (webrtc) wrote:
> > why do we use int for these?
> > (e.g. why not int foo:1 assuming this is C code)
> 
> Because... legacy. And for the record, int foo:1 is a bad idea, since at least
> our compiler implements a signed 1-bit field to have the range {-1, 0}. This
> leads to very awkward logic (0 == false, -1 == true). Changing to unsigned
foo:1
> works as expected.
> 
> But, I doubt that saving one int is worth the effort.

Well, -1 is true, so is 0xffff and 0xffffffff.  (see e.g.
http://blogs.msdn.com/b/oldnewthing/archive/2004/12/22/329884.aspx)
The only constant is 0==FALSE==false==VARIANT_FALSE etc, so whenever I see code
that checks for |== true| I need to make sure that that's the only possible
"true" value.  is !false == 1 or is !false==!0?  Anyway, agree on unsigned being
better regardless.

I just saw a few would-be bool member variables declared as int, so had to ask.

[kwiberg-webrtc, 2015-07-08 14:47:59]

kwiberg@webrtc.org changed reviewers:
+ kwiberg@webrtc.org

[kwiberg-webrtc, 2015-07-08 14:47:59]

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

https://codereview.webrtc.org/1211053006/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/aec/aec_core_internal.h:149: int
delay_agnostic_enabled;
On 2015/07/02 06:36:30, hlundin-webrtc-VACATIONtoAUG3 wrote:
> And for the record, int foo:1 is a bad idea, since at least
> our compiler implements a signed 1-bit field to have the range {-1, 0}.

I hope you won't find a compiler where this isn't the case. An n-bit two's
complement integer should have 1 sign bit and n-1 "value" bits, which gives
minimum value -2^(n-1) and maximum value 2^(n-1) - 1. For n = 1, that's -1 and
0.