Adding googAudioNetworkAdaptorConfig to MediaConstraintsInterface.

Adding googAudioNetworkAdaptorConfig to MediaConstraintsInterface.

This is to allow application to pass an audio network adaptor config string to WebRTC.

BUG=webrtc:6303
Committed: https://crrev.com/ba41428b1202621237d15feceab715e64593bc26
Cr-Commit-Position: refs/heads/master@{#15532}

[minyue-webrtc, 2016-10-31 13:24:47]

Patchset #1 (id:1) has been deleted

[minyue-webrtc, 2016-11-01 10:50:33]

Description was changed from

==========
format


parsing


final change


more to continue

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

to

==========
Adding AudioNetworkAdaptorConfig to MediaConstraintsInterface.

This is to allow application to pass an audio network adaptor config string to
WebRTC.

BUG=webrtc:6303
==========

[minyue-webrtc, 2016-11-01 10:50:34]

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

[minyue-webrtc, 2016-11-01 10:50:42]

Patchset #1 (id:20001) has been deleted

[minyue-webrtc, 2016-11-01 10:55:09]

Hi Peter,

Would you help review this CL? Thanks!

https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/localaudiosour...
File webrtc/api/localaudiosource.cc (left):

https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/localaudiosour...
webrtc/api/localaudiosource.cc:70: // Set non-boolean constraints.
This is a refactoring. This part is moved into the loop since FindFirst makes
another loop, which can be omitted.

[minyue-webrtc, 2016-11-03 14:03:05]

On 2016/11/01 10:55:09, minyue-webrtc wrote:
> Hi Peter,
> 
> Would you help review this CL? Thanks!
> 
>
https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/localaudiosour...
> File webrtc/api/localaudiosource.cc (left):
> 
>
https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/localaudiosour...
> webrtc/api/localaudiosource.cc:70: // Set non-boolean constraints.
> This is a refactoring. This part is moved into the loop since FindFirst makes
> another loop, which can be omitted.

Hi Peter,

I kindly ping for review.

[minyue-webrtc, 2016-11-08 07:31:30]

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

[minyue-webrtc, 2016-11-08 07:31:53]

+Fredrik to take a look

[the sun, 2016-11-09 10:14:25]

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

[the sun, 2016-11-09 10:14:25]

lgtm % comments

Also, do get Harald's input on naming the constraint.

https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/localaudiosour...
File webrtc/api/localaudiosource.cc (right):

https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/localaudiosour...
webrtc/api/localaudiosource.cc:63: float level_control_initial_peak_level_dbfs;
= 0.0f;

*always* initialize. Code may be changed and lines moved around. It is very easy
to forget a thing like this and end up in an unexpected state.

https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/localaudiosour...
webrtc/api/localaudiosource.cc:77: rtc::Optional<std::string>(constraint.value);
Is there any way we can validate the string at this stage?

https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/mediaconstrain...
File webrtc/api/mediaconstraintsinterface.cc (right):

https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/mediaconstrain...
webrtc/api/mediaconstraintsinterface.cc:60: "audioNetworkAdaptorConfig";
I don't know whether this should be a "goog" constraint or not - adding hta@ for
a comment.

[hta-webrtc, 2016-11-09 11:11:01]

Short version:

Please don't.

Long version:

Chrome is no longer using the constraints interface in WebRTC, and won't ever
return to using it. If you want to have this control available, please expose it
on a conventional API; it is then your business whether you also want to make it
settable via the constraints mechanism for C++ users of WebRTC that still use
that API. If you still want it (although I don't understand why), please use a
goog name for the constraint, so that it will not collide with standardized
names and confuse the users.

If you want help on publishing a spec for the new constraint at the Javascript
API and then implementing it in Blink, contact me.

[minyue-webrtc, 2016-11-09 11:38:33]

On 2016/11/09 11:11:01, hta-webrtc wrote:
> Short version:
> 
> Please don't.
> 
> Long version:
> 
> Chrome is no longer using the constraints interface in WebRTC, and won't ever
> return to using it. If you want to have this control available, please expose
it
> on a conventional API; it is then your business whether you also want to make
it
> settable via the constraints mechanism for C++ users of WebRTC that still use
> that API. If you still want it (although I don't understand why), please use a
> goog name for the constraint, so that it will not collide with standardized
> names and confuse the users.
> 
> If you want help on publishing a spec for the new constraint at the Javascript
> API and then implementing it in Blink, contact me.


Hi Harald,

Thanks for the comments!

I notice that we recently added some new constraints like
kLevelControlInitialPeakLevelDBFS etc, and there has been many others that
WebRTC rely a lot, like googEchoCancellation etc. 

What other means of signaling things like that would you suggest?

[hta-webrtc, 2016-11-09 15:51:01]

On 2016/11/09 11:38:33, minyue-webrtc wrote:
> On 2016/11/09 11:11:01, hta-webrtc wrote:
> > Short version:
> > 
> > Please don't.
> > 
> > Long version:
> > 
> > Chrome is no longer using the constraints interface in WebRTC, and won't
ever
> > return to using it. If you want to have this control available, please
expose
> it
> > on a conventional API; it is then your business whether you also want to
make
> it
> > settable via the constraints mechanism for C++ users of WebRTC that still
use
> > that API. If you still want it (although I don't understand why), please use
a
> > goog name for the constraint, so that it will not collide with standardized
> > names and confuse the users.
> > 
> > If you want help on publishing a spec for the new constraint at the
Javascript
> > API and then implementing it in Blink, contact me.
> 
> 
> Hi Harald,
> 
> Thanks for the comments!
> 
> I notice that we recently added some new constraints like
> kLevelControlInitialPeakLevelDBFS etc, and there has been many others that
> WebRTC rely a lot, like googEchoCancellation etc. 
> 
> What other means of signaling things like that would you suggest?

Nobody asked me about those.... my recommendation would have been the same.

This parameter is already exposed in cricket::AudioOptions; I think the caller
should use cricket::AudioOptions (second version of Initialize in
localaudiosource.cc) and never call the first version at all.
My preference would be to deprecate the version of Initialize that takes
constraints.

[pthatcher1, 2016-11-09 16:50:02]

On 2016/11/09 15:51:01, hta-webrtc wrote:
> On 2016/11/09 11:38:33, minyue-webrtc wrote:
> > On 2016/11/09 11:11:01, hta-webrtc wrote:
> > > Short version:
> > > 
> > > Please don't.
> > > 
> > > Long version:
> > > 
> > > Chrome is no longer using the constraints interface in WebRTC, and won't
> ever
> > > return to using it. If you want to have this control available, please
> expose
> > it
> > > on a conventional API; it is then your business whether you also want to
> make
> > it
> > > settable via the constraints mechanism for C++ users of WebRTC that still
> use
> > > that API. If you still want it (although I don't understand why), please
use
> a
> > > goog name for the constraint, so that it will not collide with
standardized
> > > names and confuse the users.
> > > 
> > > If you want help on publishing a spec for the new constraint at the
> Javascript
> > > API and then implementing it in Blink, contact me.
> > 
> > 
> > Hi Harald,
> > 
> > Thanks for the comments!
> > 
> > I notice that we recently added some new constraints like
> > kLevelControlInitialPeakLevelDBFS etc, and there has been many others that
> > WebRTC rely a lot, like googEchoCancellation etc. 
> > 
> > What other means of signaling things like that would you suggest?
> 
> Nobody asked me about those.... my recommendation would have been the same.
> 
> This parameter is already exposed in cricket::AudioOptions; I think the caller
> should use cricket::AudioOptions (second version of Initialize in
> localaudiosource.cc) and never call the first version at all.
> My preference would be to deprecate the version of Initialize that takes
> constraints.

I agree with Harald.  We should be using cricket::AudioOptions.

[minyue-webrtc, 2016-11-10 15:15:09]

On 2016/11/09 16:50:02, pthatcher1 wrote:
> On 2016/11/09 15:51:01, hta-webrtc wrote:
> > On 2016/11/09 11:38:33, minyue-webrtc wrote:
> > > On 2016/11/09 11:11:01, hta-webrtc wrote:
> > > > Short version:
> > > > 
> > > > Please don't.
> > > > 
> > > > Long version:
> > > > 
> > > > Chrome is no longer using the constraints interface in WebRTC, and won't
> > ever
> > > > return to using it. If you want to have this control available, please
> > expose
> > > it
> > > > on a conventional API; it is then your business whether you also want to
> > make
> > > it
> > > > settable via the constraints mechanism for C++ users of WebRTC that
still
> > use
> > > > that API. If you still want it (although I don't understand why), please
> use
> > a
> > > > goog name for the constraint, so that it will not collide with
> standardized
> > > > names and confuse the users.
> > > > 
> > > > If you want help on publishing a spec for the new constraint at the
> > Javascript
> > > > API and then implementing it in Blink, contact me.
> > > 
> > > 
> > > Hi Harald,
> > > 
> > > Thanks for the comments!
> > > 
> > > I notice that we recently added some new constraints like
> > > kLevelControlInitialPeakLevelDBFS etc, and there has been many others that
> > > WebRTC rely a lot, like googEchoCancellation etc. 
> > > 
> > > What other means of signaling things like that would you suggest?
> > 
> > Nobody asked me about those.... my recommendation would have been the same.
> > 
> > This parameter is already exposed in cricket::AudioOptions; I think the
caller
> > should use cricket::AudioOptions (second version of Initialize in
> > localaudiosource.cc) and never call the first version at all.
> > My preference would be to deprecate the version of Initialize that takes
> > constraints.
> 
> I agree with Harald.  We should be using cricket::AudioOptions.

Ok. Will revert this CL.

[minyue-webrtc, 2016-11-23 10:39:20]

On 2016/11/10 15:15:09, minyue-webrtc wrote:
> On 2016/11/09 16:50:02, pthatcher1 wrote:
> > On 2016/11/09 15:51:01, hta-webrtc wrote:
> > > On 2016/11/09 11:38:33, minyue-webrtc wrote:
> > > > On 2016/11/09 11:11:01, hta-webrtc wrote:
> > > > > Short version:
> > > > > 
> > > > > Please don't.
> > > > > 
> > > > > Long version:
> > > > > 
> > > > > Chrome is no longer using the constraints interface in WebRTC, and
won't
> > > ever
> > > > > return to using it. If you want to have this control available, please
> > > expose
> > > > it
> > > > > on a conventional API; it is then your business whether you also want
to
> > > make
> > > > it
> > > > > settable via the constraints mechanism for C++ users of WebRTC that
> still
> > > use
> > > > > that API. If you still want it (although I don't understand why),
please
> > use
> > > a
> > > > > goog name for the constraint, so that it will not collide with
> > standardized
> > > > > names and confuse the users.
> > > > > 
> > > > > If you want help on publishing a spec for the new constraint at the
> > > Javascript
> > > > > API and then implementing it in Blink, contact me.
> > > > 
> > > > 
> > > > Hi Harald,
> > > > 
> > > > Thanks for the comments!
> > > > 
> > > > I notice that we recently added some new constraints like
> > > > kLevelControlInitialPeakLevelDBFS etc, and there has been many others
that
> > > > WebRTC rely a lot, like googEchoCancellation etc. 
> > > > 
> > > > What other means of signaling things like that would you suggest?
> > > 
> > > Nobody asked me about those.... my recommendation would have been the
same.
> > > 
> > > This parameter is already exposed in cricket::AudioOptions; I think the
> caller
> > > should use cricket::AudioOptions (second version of Initialize in
> > > localaudiosource.cc) and never call the first version at all.
> > > My preference would be to deprecate the version of Initialize that takes
> > > constraints.
> > 
> > I agree with Harald.  We should be using cricket::AudioOptions.
> 
> Ok. Will revert this CL.

I have to re-start this CL since the JNI method of CreateAudioSource has not
wired up to the AudioOptions.

Since this work is fairly more urgent than changing the JNI, I would hope we can
allow this MediaConstraintsInterface for a moment.

And I have created a bug and will try to work on it in near future:
https://bugs.chromium.org/p/webrtc/issues/detail?id=6752

[hta-webrtc, 2016-11-24 14:29:53]

lgtm, with strong reservations.

"This is not about starting on the slippery slope, this is how far we are
sliding into the muck at the bottom".

Besides the fact that this is using an outdated interface, it also illustrates
exactly why this interface is outdated: I spent a bit of time trying to find out
what the syntax and semantics of the audio network adapter config string is -
and I couldn't do it.

Since it's just a string, it's not possible to trace it by type. And it's passed
down through so many levels of indirection that it's very hard to verify that
it's discarded on every path except the one you're looking for.

As far as I can tell, it's passed to some function that is passed into the Opus
audio creator. That's all I can say.

This is not a good design.

Note: The bug doesn't give any means of evaluating whether this is urgent or
not. No context.

https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/mediaconstrain...
File webrtc/api/mediaconstraintsinterface.cc (right):

https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/mediaconstrain...
webrtc/api/mediaconstraintsinterface.cc:60: "audioNetworkAdaptorConfig";
On 2016/11/09 10:14:25, the sun wrote:
> I don't know whether this should be a "goog" constraint or not - adding hta@
for
> a comment.

This should be a "goog" constraint.

[minyue-webrtc, 2016-11-25 08:53:02]

On 2016/11/24 14:29:53, hta-webrtc wrote:
> lgtm, with strong reservations.
> 
> "This is not about starting on the slippery slope, this is how far we are
> sliding into the muck at the bottom".
> 
> Besides the fact that this is using an outdated interface, it also illustrates
> exactly why this interface is outdated: I spent a bit of time trying to find
out
> what the syntax and semantics of the audio network adapter config string is -
> and I couldn't do it.
> 
> Since it's just a string, it's not possible to trace it by type. And it's
passed
> down through so many levels of indirection that it's very hard to verify that
> it's discarded on every path except the one you're looking for.
> 
> As far as I can tell, it's passed to some function that is passed into the
Opus
> audio creator. That's all I can say.
> 
> This is not a good design.
> 
> Note: The bug doesn't give any means of evaluating whether this is urgent or
> not. No context.
> 
>
https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/mediaconstrain...
> File webrtc/api/mediaconstraintsinterface.cc (right):
> 
>
https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/mediaconstrain...
> webrtc/api/mediaconstraintsinterface.cc:60: "audioNetworkAdaptorConfig";
> On 2016/11/09 10:14:25, the sun wrote:
> > I don't know whether this should be a "goog" constraint or not - adding hta@
> for
> > a comment.
> 
> This should be a "goog" constraint.

Hi Harald,

Thanks for the comments! I will try to switch JNI to new interface as soon as I
can.

Regarding the string, there have been some discussions. The reason for using a
string is that it is a protobuf serialized string and can easily go cross
languages. With this, different implementations of application layer can set it
up. The reason for not parsing it earlier than it arrives at the Opus encoder is
because, the middle layers do not actually care about the content. I am open to
other opinions, and if needed, we can modify the design.

[the sun, 2016-11-25 08:59:24]

On 2016/11/25 08:53:02, minyue-webrtc wrote:
> On 2016/11/24 14:29:53, hta-webrtc wrote:
> > lgtm, with strong reservations.
> > 
> > "This is not about starting on the slippery slope, this is how far we are
> > sliding into the muck at the bottom".
> > 
> > Besides the fact that this is using an outdated interface, it also
illustrates
> > exactly why this interface is outdated: I spent a bit of time trying to find
> out
> > what the syntax and semantics of the audio network adapter config string is
-
> > and I couldn't do it.
> > 
> > Since it's just a string, it's not possible to trace it by type. And it's
> passed
> > down through so many levels of indirection that it's very hard to verify
that
> > it's discarded on every path except the one you're looking for.
> > 
> > As far as I can tell, it's passed to some function that is passed into the
> Opus
> > audio creator. That's all I can say.
> > 
> > This is not a good design.
> > 
> > Note: The bug doesn't give any means of evaluating whether this is urgent or
> > not. No context.
> > 
> >
>
https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/mediaconstrain...
> > File webrtc/api/mediaconstraintsinterface.cc (right):
> > 
> >
>
https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/mediaconstrain...
> > webrtc/api/mediaconstraintsinterface.cc:60: "audioNetworkAdaptorConfig";
> > On 2016/11/09 10:14:25, the sun wrote:
> > > I don't know whether this should be a "goog" constraint or not - adding
hta@
> > for
> > > a comment.
> > 
> > This should be a "goog" constraint.
> 
> Hi Harald,
> 
> Thanks for the comments! I will try to switch JNI to new interface as soon as
I
> can.
> 
> Regarding the string, there have been some discussions. The reason for using a
> string is that it is a protobuf serialized string and can easily go cross
> languages. With this, different implementations of application layer can set
it
> up. The reason for not parsing it earlier than it arrives at the Opus encoder
is
> because, the middle layers do not actually care about the content. I am open
to
> other opinions, and if needed, we can modify the design.

Does this mean that the Opus/ANA code is now dependent on the protobuf lib?
There have been discussions about the need to be able to build WebRTC without
protobufs.

[minyue-webrtc, 2016-11-25 10:15:17]

On 2016/11/25 08:59:24, the sun wrote:
> On 2016/11/25 08:53:02, minyue-webrtc wrote:
> > On 2016/11/24 14:29:53, hta-webrtc wrote:
> > > lgtm, with strong reservations.
> > > 
> > > "This is not about starting on the slippery slope, this is how far we are
> > > sliding into the muck at the bottom".
> > > 
> > > Besides the fact that this is using an outdated interface, it also
> illustrates
> > > exactly why this interface is outdated: I spent a bit of time trying to
find
> > out
> > > what the syntax and semantics of the audio network adapter config string
is
> -
> > > and I couldn't do it.
> > > 
> > > Since it's just a string, it's not possible to trace it by type. And it's
> > passed
> > > down through so many levels of indirection that it's very hard to verify
> that
> > > it's discarded on every path except the one you're looking for.
> > > 
> > > As far as I can tell, it's passed to some function that is passed into the
> > Opus
> > > audio creator. That's all I can say.
> > > 
> > > This is not a good design.
> > > 
> > > Note: The bug doesn't give any means of evaluating whether this is urgent
or
> > > not. No context.
> > > 
> > >
> >
>
https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/mediaconstrain...
> > > File webrtc/api/mediaconstraintsinterface.cc (right):
> > > 
> > >
> >
>
https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/mediaconstrain...
> > > webrtc/api/mediaconstraintsinterface.cc:60: "audioNetworkAdaptorConfig";
> > > On 2016/11/09 10:14:25, the sun wrote:
> > > > I don't know whether this should be a "goog" constraint or not - adding
> hta@
> > > for
> > > > a comment.
> > > 
> > > This should be a "goog" constraint.
> > 
> > Hi Harald,
> > 
> > Thanks for the comments! I will try to switch JNI to new interface as soon
as
> I
> > can.
> > 
> > Regarding the string, there have been some discussions. The reason for using
a
> > string is that it is a protobuf serialized string and can easily go cross
> > languages. With this, different implementations of application layer can set
> it
> > up. The reason for not parsing it earlier than it arrives at the Opus
encoder
> is
> > because, the middle layers do not actually care about the content. I am open
> to
> > other opinions, and if needed, we can modify the design.
> 
> Does this mean that the Opus/ANA code is now dependent on the protobuf lib?
> There have been discussions about the need to be able to build WebRTC without
> protobufs.

Hi Fredrik,

That is true for now. but can be made so that Opus-ANA uses a default setting
when protobuf is not available.

[hta-webrtc, 2016-11-28 08:56:36]

On 2016/11/25 10:15:17, minyue-webrtc wrote:
> On 2016/11/25 08:59:24, the sun wrote:
> > On 2016/11/25 08:53:02, minyue-webrtc wrote:
> > > On 2016/11/24 14:29:53, hta-webrtc wrote:
> > > > lgtm, with strong reservations.
> > > > 
> > > > "This is not about starting on the slippery slope, this is how far we
are
> > > > sliding into the muck at the bottom".
> > > > 
> > > > Besides the fact that this is using an outdated interface, it also
> > illustrates
> > > > exactly why this interface is outdated: I spent a bit of time trying to
> find
> > > out
> > > > what the syntax and semantics of the audio network adapter config string
> is
> > -
> > > > and I couldn't do it.
> > > > 
> > > > Since it's just a string, it's not possible to trace it by type. And
it's
> > > passed
> > > > down through so many levels of indirection that it's very hard to verify
> > that
> > > > it's discarded on every path except the one you're looking for.
> > > > 
> > > > As far as I can tell, it's passed to some function that is passed into
the
> > > Opus
> > > > audio creator. That's all I can say.
> > > > 
> > > > This is not a good design.
> > > > 
> > > > Note: The bug doesn't give any means of evaluating whether this is
urgent
> or
> > > > not. No context.
> > > > 
> > > >
> > >
> >
>
https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/mediaconstrain...
> > > > File webrtc/api/mediaconstraintsinterface.cc (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/mediaconstrain...
> > > > webrtc/api/mediaconstraintsinterface.cc:60: "audioNetworkAdaptorConfig";
> > > > On 2016/11/09 10:14:25, the sun wrote:
> > > > > I don't know whether this should be a "goog" constraint or not -
adding
> > hta@
> > > > for
> > > > > a comment.
> > > > 
> > > > This should be a "goog" constraint.
> > > 
> > > Hi Harald,
> > > 
> > > Thanks for the comments! I will try to switch JNI to new interface as soon
> as
> > I
> > > can.
> > > 
> > > Regarding the string, there have been some discussions. The reason for
using
> a
> > > string is that it is a protobuf serialized string and can easily go cross
> > > languages. With this, different implementations of application layer can
set
> > it
> > > up. The reason for not parsing it earlier than it arrives at the Opus
> encoder
> > is
> > > because, the middle layers do not actually care about the content. I am
open
> > to
> > > other opinions, and if needed, we can modify the design.
> > 
> > Does this mean that the Opus/ANA code is now dependent on the protobuf lib?
> > There have been discussions about the need to be able to build WebRTC
without
> > protobufs.
> 
> Hi Fredrik,
> 
> That is true for now. but can be made so that Opus-ANA uses a default setting
> when protobuf is not available.

So the string is Opus-specific, and its syntax is Google-specific. The correct
name seems to be "googOpusAudioEncoderConfiguration".

When the string is not given as a constraint, the code has to do *something*, so
a default setting is available anyway.

[minyue-webrtc, 2016-11-28 14:58:48]

On 2016/11/28 08:56:36, hta-webrtc wrote:
> On 2016/11/25 10:15:17, minyue-webrtc wrote:
> > On 2016/11/25 08:59:24, the sun wrote:
> > > On 2016/11/25 08:53:02, minyue-webrtc wrote:
> > > > On 2016/11/24 14:29:53, hta-webrtc wrote:
> > > > > lgtm, with strong reservations.
> > > > > 
> > > > > "This is not about starting on the slippery slope, this is how far we
> are
> > > > > sliding into the muck at the bottom".
> > > > > 
> > > > > Besides the fact that this is using an outdated interface, it also
> > > illustrates
> > > > > exactly why this interface is outdated: I spent a bit of time trying
to
> > find
> > > > out
> > > > > what the syntax and semantics of the audio network adapter config
string
> > is
> > > -
> > > > > and I couldn't do it.
> > > > > 
> > > > > Since it's just a string, it's not possible to trace it by type. And
> it's
> > > > passed
> > > > > down through so many levels of indirection that it's very hard to
verify
> > > that
> > > > > it's discarded on every path except the one you're looking for.
> > > > > 
> > > > > As far as I can tell, it's passed to some function that is passed into
> the
> > > > Opus
> > > > > audio creator. That's all I can say.
> > > > > 
> > > > > This is not a good design.
> > > > > 
> > > > > Note: The bug doesn't give any means of evaluating whether this is
> urgent
> > or
> > > > > not. No context.
> > > > > 
> > > > >
> > > >
> > >
> >
>
https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/mediaconstrain...
> > > > > File webrtc/api/mediaconstraintsinterface.cc (right):
> > > > > 
> > > > >
> > > >
> > >
> >
>
https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/mediaconstrain...
> > > > > webrtc/api/mediaconstraintsinterface.cc:60:
"audioNetworkAdaptorConfig";
> > > > > On 2016/11/09 10:14:25, the sun wrote:
> > > > > > I don't know whether this should be a "goog" constraint or not -
> adding
> > > hta@
> > > > > for
> > > > > > a comment.
> > > > > 
> > > > > This should be a "goog" constraint.
> > > > 
> > > > Hi Harald,
> > > > 
> > > > Thanks for the comments! I will try to switch JNI to new interface as
soon
> > as
> > > I
> > > > can.
> > > > 
> > > > Regarding the string, there have been some discussions. The reason for
> using
> > a
> > > > string is that it is a protobuf serialized string and can easily go
cross
> > > > languages. With this, different implementations of application layer can
> set
> > > it
> > > > up. The reason for not parsing it earlier than it arrives at the Opus
> > encoder
> > > is
> > > > because, the middle layers do not actually care about the content. I am
> open
> > > to
> > > > other opinions, and if needed, we can modify the design.
> > > 
> > > Does this mean that the Opus/ANA code is now dependent on the protobuf
lib?
> > > There have been discussions about the need to be able to build WebRTC
> without
> > > protobufs.
> > 
> > Hi Fredrik,
> > 
> > That is true for now. but can be made so that Opus-ANA uses a default
setting
> > when protobuf is not available.
> 
> So the string is Opus-specific, and its syntax is Google-specific. The correct
> name seems to be "googOpusAudioEncoderConfiguration".
> 
> When the string is not given as a constraint, the code has to do *something*,
so
> a default setting is available anyway.

It is currently Opus specific but can be applied to other audio codecs in the
future. I hope we can make the name a bit more general, how does 
googAudioNetworkAdaptorConfig sound?

[hta-webrtc, 2016-11-28 15:57:38]

> It is currently Opus specific but can be applied to other audio codecs in the
> future. I hope we can make the name a bit more general, how does 
> googAudioNetworkAdaptorConfig sound?

Quite bad, actually.

If you're aiming to have something that can have an externally visible spec in
the end, you should aim for having an externally visible spec as soon as
possible, and the name you choose for your temporary measure until that external
spec is available should clearly mark that it's temporary/internal ("goog") and
delimit the use case you 're aiming for ("Opus").

I doubt we can standardize stuff that depends on the protobuf serialization (hm
- is this the ASCII or binary encoding?); OTOH, JSON-encoded stuff has been
standardized any number of times.

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

Description was changed from

==========
Adding AudioNetworkAdaptorConfig to MediaConstraintsInterface.

This is to allow application to pass an audio network adaptor config string to
WebRTC.

BUG=webrtc:6303
==========

to

==========
Adding googAudioNetworkAdaptorConfig to MediaConstraintsInterface.

This is to allow application to pass an audio network adaptor config string to
WebRTC.

BUG=webrtc:6303
==========

[minyue-webrtc, 2016-12-02 10:34:19]

Sorry for coming back to this after a while, since I had other work to finish
first. Thank you for the comments and discussions! I now propose a renaming as
discussed.

This might be somehow temporal in many senses, further improvements can be made
in future, e.g. when we deprecate MediaConstraintInterface.

https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/localaudiosour...
File webrtc/api/localaudiosource.cc (right):

https://codereview.webrtc.org/2437803004/diff/40001/webrtc/api/localaudiosour...
webrtc/api/localaudiosource.cc:77: rtc::Optional<std::string>(constraint.value);
On 2016/11/09 10:14:25, the sun wrote:
> Is there any way we can validate the string at this stage?

The interpretation and validity check of the string are in the audio network
adaptor. We may need to instantiate it to see if the string is valid. Therefore,
we may better just pass it down at this stage.

[hta-webrtc, 2016-12-06 08:09:00]

lgtm

[minyue-webrtc, 2016-12-06 11:23:09]

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

[commit-bot: I haz the power, 2016-12-06 11:23:17]

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-12-06 11:40:33]

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

[commit-bot: I haz the power, 2016-12-06 11:40:34]

Dry run: This issue passed the CQ dry run.

[minyue-webrtc, 2016-12-08 08:31:58]

The CQ bit was checked by minyue@webrtc.org

[minyue-webrtc, 2016-12-08 08:31:58]

The patchset sent to the CQ was uploaded after l-g-t-m from solenberg@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2437803004/#ps60001 (title:
"renaming")

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

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

[minyue-webrtc, 2016-12-08 08:32:38]

The CQ bit was unchecked by minyue@webrtc.org

[minyue-webrtc, 2016-12-11 10:01:50]

The CQ bit was checked by minyue@webrtc.org

[commit-bot: I haz the power, 2016-12-11 10:01:58]

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-12-11 10:17:54]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481450510811950,
"parent_rev": "7ad57975175b13a702005d516b1d07446ded94b9", "commit_rev":
"3103c470bf60ec873fbda9a795795fa7e4d9b11f"}

[commit-bot: I haz the power, 2016-12-11 10:17:57]

Description was changed from

==========
Adding googAudioNetworkAdaptorConfig to MediaConstraintsInterface.

This is to allow application to pass an audio network adaptor config string to
WebRTC.

BUG=webrtc:6303
==========

to

==========
Adding googAudioNetworkAdaptorConfig to MediaConstraintsInterface.

This is to allow application to pass an audio network adaptor config string to
WebRTC.

BUG=webrtc:6303

Review-Url: https://codereview.webrtc.org/2437803004
==========

[commit-bot: I haz the power, 2016-12-11 10:17:58]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2016-12-11 10:18:07]

Description was changed from

==========
Adding googAudioNetworkAdaptorConfig to MediaConstraintsInterface.

This is to allow application to pass an audio network adaptor config string to
WebRTC.

BUG=webrtc:6303

Review-Url: https://codereview.webrtc.org/2437803004
==========

to

==========
Adding googAudioNetworkAdaptorConfig to MediaConstraintsInterface.

This is to allow application to pass an audio network adaptor config string to
WebRTC.

BUG=webrtc:6303

Committed: https://crrev.com/ba41428b1202621237d15feceab715e64593bc26
Cr-Commit-Position: refs/heads/master@{#15532}
==========

[commit-bot: I haz the power, 2016-12-11 10:18:08]

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