Deactivated the intelligibility enhancement functionality by default

Deactivated the intelligibility enhancement functionality by default

NOTRY=true
BUG=

Committed: https://crrev.com/1bcfce5ff2cda67aeaf796cbb4779afaace2847e
Cr-Commit-Position: refs/heads/master@{#13937}

[peah-webrtc, 2016-08-25 13:07:56]

Description was changed from

==========
Deactivated the intelligibility enhancement functionality by default

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

to

==========
Deactivated the intelligibility enhancement functionality by default

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

[peah-webrtc, 2016-08-25 13:08:13]

The CQ bit was checked by peah@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-25 13:08:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-08-25 13:10:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-25 13:10:31]

Dry run: Try jobs failed on following builders:
  ios64_gn_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_gn_dbg/builds/2169)
  mac_baremetal on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_baremetal/builds/13674)

[peah-webrtc, 2016-08-25 14:04:58]

The CQ bit was checked by peah@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-25 14:05:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-08-25 14:09:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-25 14:09:24]

Dry run: Try jobs failed on following builders:
  mac_baremetal on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_baremetal/builds/13684)

[peah-webrtc, 2016-08-25 16:10:16]

The CQ bit was checked by peah@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-25 16:10:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-08-25 16:12:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-25 16:12:19]

Dry run: Try jobs failed on following builders:
  ios64_gn_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_gn_rel/builds/2214)
  mac_compile_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...)

[peah-webrtc, 2016-08-25 17:09:03]

The CQ bit was checked by peah@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-25 17:09:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-08-25 19:09:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-25 19:09:47]

Dry run: Try jobs failed on following builders:
  android_arm64_rel on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[peah-webrtc, 2016-08-26 05:04:21]

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

[hlundin-webrtc, 2016-08-26 06:53:03]

LG mostly, but I wonder if you could avoid hard-commenting out the test files,
and use conditions instead.

https://codereview.webrtc.org/2272423003/diff/60001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/audio_processing_tests.gypi (right):

https://codereview.webrtc.org/2272423003/diff/60001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/audio_processing_tests.gypi:64: # {
Is it possible to condition this on the build variables for the IE?

https://codereview.webrtc.org/2272423003/diff/60001/webrtc/modules/audio_proc...
File
webrtc/modules/audio_processing/intelligibility/test/intelligibility_proc.cc
(right):

https://codereview.webrtc.org/2272423003/diff/60001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/intelligibility/test/intelligibility_proc.cc:23:
#if (WEBRTC_INTELLIGIBILITY_ENHANCER == 1)
Isn't it sufficient that this file is only compiled when
WEBRTC_INTELLIGIBILITY_ENHANCER == 1? If so, you could just make sure that
WEBRTC_INTELLIGIBILITY_ENHANCER == 1 here.

https://codereview.webrtc.org/2272423003/diff/60001/webrtc/modules/modules.gyp
File webrtc/modules/modules.gyp (right):

https://codereview.webrtc.org/2272423003/diff/60001/webrtc/modules/modules.gy...
webrtc/modules/modules.gyp:252: #
'audio_processing/intelligibility/intelligibility_enhancer_unittest.cc',
Condition instead of commenting out.

[hlundin-webrtc, 2016-08-26 06:54:21]

And you'll need OWNERS for webrtc/media/engine/webrtcvoiceengine.cc.

[peah-webrtc, 2016-08-26 09:29:35]

Patchset #5 (id:80001) has been deleted

[peah-webrtc, 2016-08-26 09:29:42]

Patchset #5 (id:100001) has been deleted

[peah-webrtc, 2016-08-26 09:29:50]

Patchset #5 (id:120001) has been deleted

[peah-webrtc, 2016-08-26 11:56:48]

peah@webrtc.org changed reviewers:
+ kjellander@webrtc.org, solenberg@webrtc.org

[peah-webrtc, 2016-08-26 11:56:49]

+kjellander@ and solenberg@ as OWNERS

[hlundin-webrtc, 2016-08-26 11:58:37]

lgtm

[the sun, 2016-08-26 12:08:56]

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/media/engine/webr...
File webrtc/media/engine/webrtcvoiceengine.cc (right):

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvoiceengine.cc:69: #if
!defined(WEBRTC_INTELLIGIBILITY_ENHANCER) || \
Why not just
#if WEBRTC_INTELLIGIBILITY_ENHANCER
?
(unset defaults to 0)

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/media/media.gyp
File webrtc/media/media.gyp (right):

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/media/media.gyp#n...
webrtc/media/media.gyp:137: 
dd

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/modules/audio_pro...
File webrtc/modules/audio_processing/audio_processing_impl.cc (right):

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/modules/audio_pro...
webrtc/modules/audio_processing/audio_processing_impl.cc:33: #if
(WEBRTC_INTELLIGIBILITY_ENHANCER == 1)
#if WEBRTC_INTELLIGIBILITY_ENHANCER
should be enough

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/modules/audio_pro...
webrtc/modules/audio_processing/audio_processing_impl.cc:57: #if
!defined(WEBRTC_INTELLIGIBILITY_ENHANCER) || \
Don't think you need this

[kjellander_webrtc, 2016-08-26 12:52:01]

Please add the GYP declaration to webrtc/build/common.gypi instead.

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/media/engine/webr...
File webrtc/media/engine/webrtcvoiceengine.cc (right):

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvoiceengine.cc:69: #if
!defined(WEBRTC_INTELLIGIBILITY_ENHANCER) || \
On 2016/08/26 12:08:56, the sun wrote:
> Why not just
> #if WEBRTC_INTELLIGIBILITY_ENHANCER
> ?
> (unset defaults to 0)

I think there was a similar discussion for an AEC define and there was a good
reason for having it set to 0 or 1. I can't recall exactly why it was safer
though.

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/modules/audio_pro...
File webrtc/modules/audio_processing/audio_processing.gypi (right):

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/modules/audio_pro...
webrtc/modules/audio_processing/audio_processing.gypi:16:
'intelligibility_enhancer': 0,
This won't be useful as it'll always be set to 0. If it's used in more than one
file (as it is here), you should declare it it in webrtc/build/common.gypi and
use the % format to make it possible to override it:

'intelligibility_enhancer%': 0,

[peah-webrtc, 2016-08-26 13:09:43]

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/media/engine/webr...
File webrtc/media/engine/webrtcvoiceengine.cc (right):

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvoiceengine.cc:69: #if
!defined(WEBRTC_INTELLIGIBILITY_ENHANCER) || \
On 2016/08/26 12:08:56, the sun wrote:
> Why not just
> #if WEBRTC_INTELLIGIBILITY_ENHANCER
> ?
> (unset defaults to 0)

The reason behind that is that this is a define that spans several subunits in
the build system, and doing the check like this ensures that it is explicitly
defined everywhere. 

This is to avoid previous errors where the define was set to 1 in some places
but was not specified in other places without any warning thereof.

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvoiceengine.cc:69: #if
!defined(WEBRTC_INTELLIGIBILITY_ENHANCER) || \
On 2016/08/26 12:52:01, kjellander_webrtc wrote:
> On 2016/08/26 12:08:56, the sun wrote:
> > Why not just
> > #if WEBRTC_INTELLIGIBILITY_ENHANCER
> > ?
> > (unset defaults to 0)
> 
> I think there was a similar discussion for an AEC define and there was a good
> reason for having it set to 0 or 1. I can't recall exactly why it was safer
> though.

The reason was it is up to the build system to distribute the defines. And if
that is missed in some place, that will be a silent error as the define then
defaults to 0.

The construct here is not perfect either, but it at least requires the define to
be explicitly set.

In this case, I think nothing can go bad at runtime during this, but the code
may evolve and therefore I'd prefer to have it like this.

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/media/media.gyp
File webrtc/media/media.gyp (right):

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/media/media.gyp#n...
webrtc/media/media.gyp:137: 
On 2016/08/26 12:08:56, the sun wrote:
> dd

Done.

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/modules/audio_pro...
File webrtc/modules/audio_processing/audio_processing.gypi (right):

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/modules/audio_pro...
webrtc/modules/audio_processing/audio_processing.gypi:16:
'intelligibility_enhancer': 0,
On 2016/08/26 12:52:01, kjellander_webrtc wrote:
> This won't be useful as it'll always be set to 0. If it's used in more than
one
> file (as it is here), you should declare it it in webrtc/build/common.gypi and
> use the % format to make it possible to override it:
> 
> 'intelligibility_enhancer%': 0,

Great point!

Done.

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/modules/audio_pro...
File webrtc/modules/audio_processing/audio_processing_impl.cc (right):

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/modules/audio_pro...
webrtc/modules/audio_processing/audio_processing_impl.cc:33: #if
(WEBRTC_INTELLIGIBILITY_ENHANCER == 1)
On 2016/08/26 12:08:56, the sun wrote:
> #if WEBRTC_INTELLIGIBILITY_ENHANCER
> should be enough

Done, + in the other places in this file.

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/modules/audio_pro...
webrtc/modules/audio_processing/audio_processing_impl.cc:57: #if
!defined(WEBRTC_INTELLIGIBILITY_ENHANCER) || \
On 2016/08/26 12:08:56, the sun wrote:
> Don't think you need this

You mean because this is a source file and not a header file?

The purpose is to enforce this to be explicitly set everywhere, so that it by
mistake is not specified in the build system. 

E.g., omitting specifying it properly in the build system should not cause a
silent error by causing the functionality to be turned off.

[the sun, 2016-08-26 13:17:48]

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/media/engine/webr...
File webrtc/media/engine/webrtcvoiceengine.cc (right):

https://codereview.webrtc.org/2272423003/diff/220001/webrtc/media/engine/webr...
webrtc/media/engine/webrtcvoiceengine.cc:69: #if
!defined(WEBRTC_INTELLIGIBILITY_ENHANCER) || \
On 2016/08/26 13:09:43, peah-webrtc wrote:
> On 2016/08/26 12:08:56, the sun wrote:
> > Why not just
> > #if WEBRTC_INTELLIGIBILITY_ENHANCER
> > ?
> > (unset defaults to 0)
> 
> The reason behind that is that this is a define that spans several subunits in
> the build system, and doing the check like this ensures that it is explicitly
> defined everywhere. 
> 
> This is to avoid previous errors where the define was set to 1 in some places
> but was not specified in other places without any warning thereof.

Does it have to be like that then? Could it be set up in one place in the build
files?

Even if you make sure it is set explicitly, it may still be set differently.

[kjellander_webrtc, 2016-08-26 13:22:12]

One more round needed.

https://codereview.webrtc.org/2272423003/diff/240001/webrtc/build/common.gypi
File webrtc/build/common.gypi (right):

https://codereview.webrtc.org/2272423003/diff/240001/webrtc/build/common.gypi...
webrtc/build/common.gypi:122: 'intelligibility_enhancer%': 0,
After comparing with other variables in this file,  I think this one should be
named enable_intelligibility_enhancer

https://codereview.webrtc.org/2272423003/diff/240001/webrtc/build/webrtc.gni
File webrtc/build/webrtc.gni (right):

https://codereview.webrtc.org/2272423003/diff/240001/webrtc/build/webrtc.gni#...
webrtc/build/webrtc.gni:40: intelligibility_enhancer = false
...and this rtc_enable_intelligibility_enhancer

rtc_ is a prefix mandated by the GN folks in Chromium, for project-specific
variables, since they all share the same global namespace.

[peah-webrtc, 2016-08-26 13:26:26]

On 2016/08/26 13:17:48, the sun wrote:
>
https://codereview.webrtc.org/2272423003/diff/220001/webrtc/media/engine/webr...
> File webrtc/media/engine/webrtcvoiceengine.cc (right):
> 
>
https://codereview.webrtc.org/2272423003/diff/220001/webrtc/media/engine/webr...
> webrtc/media/engine/webrtcvoiceengine.cc:69: #if
> !defined(WEBRTC_INTELLIGIBILITY_ENHANCER) || \
> On 2016/08/26 13:09:43, peah-webrtc wrote:
> > On 2016/08/26 12:08:56, the sun wrote:
> > > Why not just
> > > #if WEBRTC_INTELLIGIBILITY_ENHANCER
> > > ?
> > > (unset defaults to 0)
> > 
> > The reason behind that is that this is a define that spans several subunits
in
> > the build system, and doing the check like this ensures that it is
explicitly
> > defined everywhere. 
> > 
> > This is to avoid previous errors where the define was set to 1 in some
places
> > but was not specified in other places without any warning thereof.
> 
> Does it have to be like that then? Could it be set up in one place in the
build
> files?
> 
> Even if you make sure it is set explicitly, it may still be set differently.

I agree. but that is at least explicit, and the error can then be easily tracked
down. If it is implicit, the error may be silent, and is harder to find. 

I don't see any other way to ensure this from a source code point of view for
defines that are not local to files, or passed via header file inclusion.

[the sun, 2016-08-26 13:33:42]

On 2016/08/26 13:26:26, peah-webrtc wrote:
> On 2016/08/26 13:17:48, the sun wrote:
> >
>
https://codereview.webrtc.org/2272423003/diff/220001/webrtc/media/engine/webr...
> > File webrtc/media/engine/webrtcvoiceengine.cc (right):
> > 
> >
>
https://codereview.webrtc.org/2272423003/diff/220001/webrtc/media/engine/webr...
> > webrtc/media/engine/webrtcvoiceengine.cc:69: #if
> > !defined(WEBRTC_INTELLIGIBILITY_ENHANCER) || \
> > On 2016/08/26 13:09:43, peah-webrtc wrote:
> > > On 2016/08/26 12:08:56, the sun wrote:
> > > > Why not just
> > > > #if WEBRTC_INTELLIGIBILITY_ENHANCER
> > > > ?
> > > > (unset defaults to 0)
> > > 
> > > The reason behind that is that this is a define that spans several
subunits
> in
> > > the build system, and doing the check like this ensures that it is
> explicitly
> > > defined everywhere. 
> > > 
> > > This is to avoid previous errors where the define was set to 1 in some
> places
> > > but was not specified in other places without any warning thereof.
> > 
> > Does it have to be like that then? Could it be set up in one place in the
> build
> > files?
> > 
> > Even if you make sure it is set explicitly, it may still be set differently.
> 
> I agree. but that is at least explicit, and the error can then be easily
tracked
> down. If it is implicit, the error may be silent, and is harder to find. 
> 
> I don't see any other way to ensure this from a source code point of view for
> defines that are not local to files, or passed via header file inclusion.

I'll give you an l-g-t-m and let kjellander@ have final say here.

Is it really not possible to set up the WEBRTC_INTELLIGIBILITY_ENHANCER #define
in just one of the build files?

[the sun, 2016-08-26 13:33:58]

lgtm

[peah-webrtc, 2016-08-26 13:35:06]

https://codereview.webrtc.org/2272423003/diff/240001/webrtc/build/common.gypi
File webrtc/build/common.gypi (right):

https://codereview.webrtc.org/2272423003/diff/240001/webrtc/build/common.gypi...
webrtc/build/common.gypi:122: 'intelligibility_enhancer%': 0,
On 2016/08/26 13:22:12, kjellander_webrtc wrote:
> After comparing with other variables in this file,  I think this one should be
> named enable_intelligibility_enhancer

Done.

https://codereview.webrtc.org/2272423003/diff/240001/webrtc/build/webrtc.gni
File webrtc/build/webrtc.gni (right):

https://codereview.webrtc.org/2272423003/diff/240001/webrtc/build/webrtc.gni#...
webrtc/build/webrtc.gni:40: intelligibility_enhancer = false
On 2016/08/26 13:22:12, kjellander_webrtc wrote:
> ...and this rtc_enable_intelligibility_enhancer
> 
> rtc_ is a prefix mandated by the GN folks in Chromium, for project-specific
> variables, since they all share the same global namespace.

Done.

[kjellander_webrtc, 2016-08-26 13:38:57]

lgtm, assuming trybots are happy. I guess compile trybots are sufficient for a
change like this.

[peah-webrtc, 2016-08-26 13:47:39]

On 2016/08/26 13:38:57, kjellander_webrtc wrote:
> lgtm, assuming trybots are happy. I guess compile trybots are sufficient for a
> change like this.

Makes sense! I'll run the compile bots, and then add NOTRY=true.

[peah-webrtc, 2016-08-26 13:48:33]

On 2016/08/26 13:33:42, the sun wrote:
> On 2016/08/26 13:26:26, peah-webrtc wrote:
> > On 2016/08/26 13:17:48, the sun wrote:
> > >
> >
>
https://codereview.webrtc.org/2272423003/diff/220001/webrtc/media/engine/webr...
> > > File webrtc/media/engine/webrtcvoiceengine.cc (right):
> > > 
> > >
> >
>
https://codereview.webrtc.org/2272423003/diff/220001/webrtc/media/engine/webr...
> > > webrtc/media/engine/webrtcvoiceengine.cc:69: #if
> > > !defined(WEBRTC_INTELLIGIBILITY_ENHANCER) || \
> > > On 2016/08/26 13:09:43, peah-webrtc wrote:
> > > > On 2016/08/26 12:08:56, the sun wrote:
> > > > > Why not just
> > > > > #if WEBRTC_INTELLIGIBILITY_ENHANCER
> > > > > ?
> > > > > (unset defaults to 0)
> > > > 
> > > > The reason behind that is that this is a define that spans several
> subunits
> > in
> > > > the build system, and doing the check like this ensures that it is
> > explicitly
> > > > defined everywhere. 
> > > > 
> > > > This is to avoid previous errors where the define was set to 1 in some
> > places
> > > > but was not specified in other places without any warning thereof.
> > > 
> > > Does it have to be like that then? Could it be set up in one place in the
> > build
> > > files?
> > > 
> > > Even if you make sure it is set explicitly, it may still be set
differently.
> > 
> > I agree. but that is at least explicit, and the error can then be easily
> tracked
> > down. If it is implicit, the error may be silent, and is harder to find. 
> > 
> > I don't see any other way to ensure this from a source code point of view
for
> > defines that are not local to files, or passed via header file inclusion.
> 
> I'll give you an l-g-t-m and let kjellander@ have final say here.
> 
> Is it really not possible to set up the WEBRTC_INTELLIGIBILITY_ENHANCER
#define
> in just one of the build files?

I guess it should be possible if the define is set in the root build file. Lets
discuss this further.

[peah-webrtc, 2016-08-26 13:49:39]

Description was changed from

==========
Deactivated the intelligibility enhancement functionality by default

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

to

==========
Deactivated the intelligibility enhancement functionality by default

NOTRY=true
BUG=
==========

[peah-webrtc, 2016-08-26 14:13:00]

The CQ bit was checked by peah@webrtc.org

[peah-webrtc, 2016-08-26 14:13:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
henrik.lundin@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2272423003/#ps260001 (title:
"Changed name of build file flags")

[commit-bot: I haz the power, 2016-08-26 14:13:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-08-26 14:16:08]

Description was changed from

==========
Deactivated the intelligibility enhancement functionality by default

NOTRY=true
BUG=
==========

to

==========
Deactivated the intelligibility enhancement functionality by default

NOTRY=true
BUG=
==========

[commit-bot: I haz the power, 2016-08-26 14:16:09]

Committed patchset #11 (id:260001)

[commit-bot: I haz the power, 2016-08-26 14:16:18]

Description was changed from

==========
Deactivated the intelligibility enhancement functionality by default

NOTRY=true
BUG=
==========

to

==========
Deactivated the intelligibility enhancement functionality by default

NOTRY=true
BUG=

Committed: https://crrev.com/1bcfce5ff2cda67aeaf796cbb4779afaace2847e
Cr-Commit-Position: refs/heads/master@{#13937}
==========

[commit-bot: I haz the power, 2016-08-26 14:16:19]

Patchset 11 (id:??) landed as
https://crrev.com/1bcfce5ff2cda67aeaf796cbb4779afaace2847e
Cr-Commit-Position: refs/heads/master@{#13937}

[aluebs-webrtc, 2016-08-31 16:25:50]

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

[aluebs-webrtc, 2016-08-31 16:25:51]

Was a study done to assess the impact of this CL on memory footprint and
complexity? Because I could imagine that it just brings marginal gains at the
cost of code bloating and forcing an additional flag. Or is there another
motivation for this CL that I am missing?
Also, why did you remove all CCs? It would have been nice to get this in my
inbox, specially since I am working on this feature.

[peah-webrtc, 2016-08-31 18:29:12]

On 2016/08/31 16:25:51, aluebs-webrtc wrote:
> Was a study done to assess the impact of this CL on memory footprint and
> complexity? Because I could imagine that it just brings marginal gains at the
> cost of code bloating and forcing an additional flag. Or is there another
> motivation for this CL that I am missing?
> Also, why did you remove all CCs? It would have been nice to get this in my
> inbox, specially since I am working on this feature.

Sorry for the CCs, that was my mistake! The CL was first indented not to be
landed and just something to try on the trybots whether this would be feasible
and at that point I removed the CCs. But then I missed adding the CCs.
You should, however, have been informed in advance about this as it has been
discussed and communicated offline.

It was not my decision to deactivate this, and there is more than one reason for
the deactivation but in my mind the biggest reason is to ensure proper
performance of the WebRTC code. There are a couple of severe issues with the IE
functionality/integration and as it can be activated via a mediaconstraint there
is nothing preventing WebRTC users to use it and being hit by these.
Deactivating the code until those issues have been resolved seems to me like the
safest and easiest way to handle this.