Don't use default aec_dump in webrtcvoiceengine when non is provided

Don't use default aec_dump in webrtcvoiceengine when non is provided

Without this change, it is not possible to do aec_dumps in AppRTCDemo app.

This change in behavior was introduced in : https://codereview.webrtc.org/1500633002/

[aluebs-webrtc, 2015-12-17 19:30:50]

Description was changed from

==========
Don't use default aec_dump in webrtcvoiceengine when non is provided
==========

to

==========
Don't use default aec_dump in webrtcvoiceengine when non is provided

Without this change, it is not possible to do aec_dumps in AppRTCDemo app.

This change in behavior was introduced in :
https://codereview.webrtc.org/1500633002/
==========

[aluebs-webrtc, 2015-12-17 19:30:50]

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

[aluebs-webrtc, 2015-12-17 19:31:57]

solenberg, could you please have a look if this change in behavior is what you
want, or if we should use some other work around. Thank you very much! :)

[the sun, 2015-12-20 22:58:32]

On 2015/12/17 19:31:57, aluebs-webrtc wrote:
> solenberg, could you please have a look if this change in behavior is what you
> want, or if we should use some other work around. Thank you very much! :)

Would you mind explaining to me what it is that now doesn't work, as it is
obviously a case that I overlooked?

[the sun, 2015-12-21 20:47:38]

On 2015/12/20 22:58:32, the sun OOO until jan 7th wrote:
> On 2015/12/17 19:31:57, aluebs-webrtc wrote:
> > solenberg, could you please have a look if this change in behavior is what
you
> > want, or if we should use some other work around. Thank you very much! :)
> 
> Would you mind explaining to me what it is that now doesn't work, as it is
> obviously a case that I overlooked?

So if I'm to take a guess, what happens is that the option is set on a channel,
but then a different channel is activated (or the first channel goes away),
causing the default to kick in a dumping to stop. Correct?

Can you use the Start/StopAecDump() interface on PeerConnectionInterface
instead? AFAICT this is exposed all the way out in the Java bindings (not sure
about iOS but it should be possible to do the same). I'd actually like to remove
the aec_dump option when possible.

[aluebs-webrtc, 2016-01-05 02:27:03]

On 2015/12/21 20:47:38, the sun OOO until jan 7th wrote:
> On 2015/12/20 22:58:32, the sun OOO until jan 7th wrote:
> > On 2015/12/17 19:31:57, aluebs-webrtc wrote:
> > > solenberg, could you please have a look if this change in behavior is what
> you
> > > want, or if we should use some other work around. Thank you very much! :)
> > 
> > Would you mind explaining to me what it is that now doesn't work, as it is
> > obviously a case that I overlooked?
> 
> So if I'm to take a guess, what happens is that the option is set on a
channel,
> but then a different channel is activated (or the first channel goes away),
> causing the default to kick in a dumping to stop. Correct?
> 
> Can you use the Start/StopAecDump() interface on PeerConnectionInterface
> instead? AFAICT this is exposed all the way out in the Java bindings (not sure
> about iOS but it should be possible to do the same). I'd actually like to
remove
> the aec_dump option when possible.

I am using the Start/StopAecDump() interface from the Java bindings in this CL
where I added aec_dump support for the AppRTCDemo app:
https://codereview.webrtc.org/1514473008/diff/20001/webrtc/examples/androidap...

What changed in your CL is that before, when calling ApplyOptions, only the
options that were explicitly set were applied, but now all options are
explicitly set to the defaults at the beginning:
https://codereview.webrtc.org/1500633002/diff/320001/talk/media/webrtc/webrtc...

I am not sure what calls ApplyOptions after StartAecDump, but this disables the
aecdump option every time. Please let me know if you have any other questions.

[the sun, 2016-01-07 13:51:25]

On 2016/01/05 02:27:03, aluebs-webrtc wrote:
> On 2015/12/21 20:47:38, the sun OOO until jan 7th wrote:
> > On 2015/12/20 22:58:32, the sun OOO until jan 7th wrote:
> > > On 2015/12/17 19:31:57, aluebs-webrtc wrote:
> > > > solenberg, could you please have a look if this change in behavior is
what
> > you
> > > > want, or if we should use some other work around. Thank you very much!
:)
> > > 
> > > Would you mind explaining to me what it is that now doesn't work, as it is
> > > obviously a case that I overlooked?
> > 
> > So if I'm to take a guess, what happens is that the option is set on a
> channel,
> > but then a different channel is activated (or the first channel goes away),
> > causing the default to kick in a dumping to stop. Correct?
> > 
> > Can you use the Start/StopAecDump() interface on PeerConnectionInterface
> > instead? AFAICT this is exposed all the way out in the Java bindings (not
sure
> > about iOS but it should be possible to do the same). I'd actually like to
> remove
> > the aec_dump option when possible.
> 
> I am using the Start/StopAecDump() interface from the Java bindings in this CL
> where I added aec_dump support for the AppRTCDemo app:
>
https://codereview.webrtc.org/1514473008/diff/20001/webrtc/examples/androidap...
> 
> What changed in your CL is that before, when calling ApplyOptions, only the
> options that were explicitly set were applied, but now all options are
> explicitly set to the defaults at the beginning:
>
https://codereview.webrtc.org/1500633002/diff/320001/talk/media/webrtc/webrtc...
> 
> I am not sure what calls ApplyOptions after StartAecDump, but this disables
the
> aecdump option every time. Please let me know if you have any other questions.

Ah, good find, and thanks for explaining. Since this likely affects more
settings than the aec_dump flag, I've uploaded
https://codereview.webrtc.org/1568853002/ to revert the initial changes.

[aluebs-webrtc, 2016-01-07 16:47:55]

On 2016/01/07 13:51:25, the sun wrote:
> On 2016/01/05 02:27:03, aluebs-webrtc wrote:
> > On 2015/12/21 20:47:38, the sun OOO until jan 7th wrote:
> > > On 2015/12/20 22:58:32, the sun OOO until jan 7th wrote:
> > > > On 2015/12/17 19:31:57, aluebs-webrtc wrote:
> > > > > solenberg, could you please have a look if this change in behavior is
> what
> > > you
> > > > > want, or if we should use some other work around. Thank you very much!
> :)
> > > > 
> > > > Would you mind explaining to me what it is that now doesn't work, as it
is
> > > > obviously a case that I overlooked?
> > > 
> > > So if I'm to take a guess, what happens is that the option is set on a
> > channel,
> > > but then a different channel is activated (or the first channel goes
away),
> > > causing the default to kick in a dumping to stop. Correct?
> > > 
> > > Can you use the Start/StopAecDump() interface on PeerConnectionInterface
> > > instead? AFAICT this is exposed all the way out in the Java bindings (not
> sure
> > > about iOS but it should be possible to do the same). I'd actually like to
> > remove
> > > the aec_dump option when possible.
> > 
> > I am using the Start/StopAecDump() interface from the Java bindings in this
CL
> > where I added aec_dump support for the AppRTCDemo app:
> >
>
https://codereview.webrtc.org/1514473008/diff/20001/webrtc/examples/androidap...
> > 
> > What changed in your CL is that before, when calling ApplyOptions, only the
> > options that were explicitly set were applied, but now all options are
> > explicitly set to the defaults at the beginning:
> >
>
https://codereview.webrtc.org/1500633002/diff/320001/talk/media/webrtc/webrtc...
> > 
> > I am not sure what calls ApplyOptions after StartAecDump, but this disables
> the
> > aecdump option every time. Please let me know if you have any other
questions.
> 
> Ah, good find, and thanks for explaining. Since this likely affects more
> settings than the aec_dump flag, I've uploaded
> https://codereview.webrtc.org/1568853002/ to revert the initial changes.

Sounds reasonable. I just didn't know if this was a desired behavior or not of
your change. Closing this CL in favor of yours.