Added a level controller initialization value to MediaConstraints.

Added a level controller initialization value to MediaConstraints.

An audio track with a level controller with the correct initialization
value can be created by a combination of
PeerConnectionFactory::CreateAudioTrack(..., audio_source) and
either
audio_source = PeerConnectionFactory::CreateAudioSource(constraints) or
audio_source = PeerConnectionFactory::CreateAudioSource(audio_options).

NOTRY=True
BUG=webrtc:6386
Committed: https://crrev.com/e33c5d918a213202321bde751226c4949644fe5e
Cr-Commit-Position: refs/heads/master@{#14693}

[aleloi, 2016-10-12 11:11:04]

The following describes the path of an initialization value to the
level controller. Points marked '*' are modified in this CL.

1. Either a MediaConstraintsInterface or an AudioOptions object is
   passed to PCFactory::CreateAudioSource.
2. The settings are passed to LocalAudioSource::Create(...,
   audio_options) or LocalAudioSource::Create(..., constraints).
3. LocalAudioSource::Initialize(..., audio_options) or
   LocalAudioSource::Initialize(..., constraints) is called on the
   newly constructed source.
4. * If the constraint path was taken, the constraints are transformed
   into audio_options with FromConstraints. Mandatory options
   overwrite ordinary with AudioOptions::SetAll.
5. the options are stored in a private member options_ in
   LocalAudioSource.

An AudioTrack is constructed by PCFactory::CreateAudioTrack. The
options are read from the AudioSource object in
AudioRptSender::SetAudioSend, and sent to
VoiceChannel::SetAudioSend. From there, the options are not modified
and are applied in WebrtcVoiceEngine::ApplyOptions.

This CL does the following:

1. Adds the options to MediaConstraintsInterface and AudioOptions.
2. Extends copying and comparison of AudioOptions.
3. Alters FromConstraints to transform constraints into audio options.
4. Applies the audio options in WebRtcVoiceEngine::ApplyOptions.

[aleloi, 2016-10-12 11:43:08]

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

[aleloi, 2016-10-12 11:43:09]

Can you take an initial look, peah@?

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

https://codereview.webrtc.org/2408143003/diff/20001/webrtc/api/localaudiosour...
webrtc/api/localaudiosource.cc:79: }
I'm not sure about this. Maybe we should change the conversion algorithm
completely and have one solution for both boolean and non-boolean options?

[aleloi, 2016-10-12 11:43:38]

The following describes the path of an initialization value to the
level controller. Points marked '*' are modified in this CL.

1. Either a MediaConstraintsInterface or an AudioOptions object is
   passed to PCFactory::CreateAudioSource.
2. The settings are passed to LocalAudioSource::Create(...,
   audio_options) or LocalAudioSource::Create(..., constraints).
3. LocalAudioSource::Initialize(..., audio_options) or
   LocalAudioSource::Initialize(..., constraints) is called on the
   newly constructed source.
4. * If the constraint path was taken, the constraints are transformed
   into audio_options with FromConstraints. Mandatory options
   overwrite ordinary with AudioOptions::SetAll.
5. the options are stored in a private member options_ in
   LocalAudioSource.

An AudioTrack is constructed by PCFactory::CreateAudioTrack. The
options are read from the AudioSource object in
AudioRptSender::SetAudioSend, and sent to
VoiceChannel::SetAudioSend. From there, the options are not modified
and are applied in WebrtcVoiceEngine::ApplyOptions.

This CL does the following:

1. Adds the options to MediaConstraintsInterface and AudioOptions.
2. Extends copying and comparison of AudioOptions.
3. Alters FromConstraints to transform constraints into audio options.
4. Applies the audio options in WebRtcVoiceEngine::ApplyOptions.

[peah-webrtc, 2016-10-13 10:23:48]

Great! 
Have you been able to test this in a real call?

lgtm

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

https://codereview.webrtc.org/2408143003/diff/20001/webrtc/api/localaudiosour...
webrtc/api/localaudiosource.cc:79: }
On 2016/10/12 11:43:09, aleloi wrote:
> I'm not sure about this. Maybe we should change the conversion algorithm
> completely and have one solution for both boolean and non-boolean options?

I think it is fine to have them separated like this, but the variables for the
bool constraints should be renamed to reflect that the sets of constraints are
handled in different places.

https://codereview.webrtc.org/2408143003/diff/20001/webrtc/api/mediaconstrain...
File webrtc/api/mediaconstraintsinterface.h (right):

https://codereview.webrtc.org/2408143003/diff/20001/webrtc/api/mediaconstrain...
webrtc/api/mediaconstraintsinterface.h:79: kLevelControlInitialPeakLevelDBFS[]; 
// levelControlInitialPeakLevelDBFS
I'm not sure that DBFS should be in all capitals. Compare to kUseRtpMux below.

Maybe dbfs would be better? Or dBfs?

[aleloi, 2016-10-13 10:30:47]

Yes, I modified peerconnection_client locally to check that the options were
applied correctly.

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

https://codereview.webrtc.org/2408143003/diff/20001/webrtc/api/localaudiosour...
webrtc/api/localaudiosource.cc:79: }
On 2016/10/13 10:23:48, peah-webrtc wrote:
> On 2016/10/12 11:43:09, aleloi wrote:
> > I'm not sure about this. Maybe we should change the conversion algorithm
> > completely and have one solution for both boolean and non-boolean options?
> 
> I think it is fine to have them separated like this, but the variables for the
> bool constraints should be renamed to reflect that the sets of constraints are
> handled in different places.

Acknowledged.

https://codereview.webrtc.org/2408143003/diff/20001/webrtc/api/mediaconstrain...
File webrtc/api/mediaconstraintsinterface.h (right):

https://codereview.webrtc.org/2408143003/diff/20001/webrtc/api/mediaconstrain...
webrtc/api/mediaconstraintsinterface.h:79: kLevelControlInitialPeakLevelDBFS[]; 
// levelControlInitialPeakLevelDBFS
On 2016/10/13 10:23:48, peah-webrtc wrote:
> I'm not sure that DBFS should be in all capitals. Compare to kUseRtpMux below.
> 
> Maybe dbfs would be better? Or dBfs?

Acknowledged.

[aleloi, 2016-10-17 11:41:41]

Description was changed from

==========
Added a level controller initialization value to MediaConstraints.

An audio track with a level controller with the correct initialization
value can be created by a combination of
PeerConnectionFactory::CreateAudioTrack(..., audio_source) and
either
audio_source = PeerConnectionFactory::CreateAudioSource(constraints) or
audio_source = PeerConnectionFactory::CreateAudioSource(audio_options).

BUG=webrtc:6386
==========

to

==========
Added a level controller initialization value to MediaConstraints.

An audio track with a level controller with the correct initialization
value can be created by a combination of
PeerConnectionFactory::CreateAudioTrack(..., audio_source) and
either
audio_source = PeerConnectionFactory::CreateAudioSource(constraints) or
audio_source = PeerConnectionFactory::CreateAudioSource(audio_options).

BUG=webrtc:6386
==========

[aleloi, 2016-10-17 11:41:41]

aleloi@webrtc.org changed reviewers:
+ tommi@webrtc.org

[aleloi, 2016-10-17 11:45:31]

tommi@, could you take a look at this? This CL is similar to this one reviewed
by you previously https://codereview.webrtc.org/2095563002/

[aleloi, 2016-10-17 14:11:09]

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

https://codereview.webrtc.org/2408143003/diff/40001/webrtc/api/mediaconstrain...
webrtc/api/mediaconstraintsinterface.h:79: kLevelControlInitialPeakLevelDBFS[]; 
// levelControlInitialPeakLevelDBFS
I wonder how the deprecation of media constraints (see
https://bugs.chromium.org/p/webrtc/issues/detail?id=5617) relates to this. I
think it's still currently the right approach, because 
webrtc/api/android/jni/peerconnection_jni.cc seems to only support creating peer
connections with media constraints.

[tommi, 2016-10-19 11:25:50]

Lgtm with two comments

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

https://codereview.webrtc.org/2408143003/diff/40001/webrtc/api/localaudiosour...
webrtc/api/localaudiosource.cc:29: // Set boolean constarints first.
Fix typo

https://codereview.webrtc.org/2408143003/diff/40001/webrtc/api/localaudiosour...
webrtc/api/localaudiosource.cc:33: } boolean_constraint_key_to_value[] = {
Are these changes relevant to the cl?
Maybe it's just fine to leave this part as it was, with the comment etc. As is,
having the new qualifier yet no other map, might just make it more confusing.

[aleloi, 2016-10-19 13:59:21]

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

https://codereview.webrtc.org/2408143003/diff/40001/webrtc/api/localaudiosour...
webrtc/api/localaudiosource.cc:29: // Set boolean constarints first.
On 2016/10/19 11:25:50, tommi (webrtc) wrote:
> Fix typo

Done.

https://codereview.webrtc.org/2408143003/diff/40001/webrtc/api/localaudiosour...
webrtc/api/localaudiosource.cc:33: } boolean_constraint_key_to_value[] = {
On 2016/10/19 11:25:50, tommi (webrtc) wrote:
> Are these changes relevant to the cl?
> Maybe it's just fine to leave this part as it was, with the comment etc. As
is,
> having the new qualifier yet no other map, might just make it more confusing. 

I agree, lets leave it as is.

[aleloi, 2016-10-19 13:59:56]

Description was changed from

==========
Added a level controller initialization value to MediaConstraints.

An audio track with a level controller with the correct initialization
value can be created by a combination of
PeerConnectionFactory::CreateAudioTrack(..., audio_source) and
either
audio_source = PeerConnectionFactory::CreateAudioSource(constraints) or
audio_source = PeerConnectionFactory::CreateAudioSource(audio_options).

BUG=webrtc:6386
==========

to

==========
Added a level controller initialization value to MediaConstraints.

An audio track with a level controller with the correct initialization
value can be created by a combination of
PeerConnectionFactory::CreateAudioTrack(..., audio_source) and
either
audio_source = PeerConnectionFactory::CreateAudioSource(constraints) or
audio_source = PeerConnectionFactory::CreateAudioSource(audio_options).

NOTRY=True
BUG=webrtc:6386
==========

[aleloi, 2016-10-19 14:00:11]

The CQ bit was checked by aleloi@webrtc.org

[aleloi, 2016-10-19 14:00:11]

The patchset sent to the CQ was uploaded after l-g-t-m from peah@webrtc.org,
tommi@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2408143003/#ps60001 (title:
"Changed the boolean constraints variable name and comment back to its original
form.")

[commit-bot: I haz the power, 2016-10-19 14:00:13]

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-10-19 16:00:34]

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

[commit-bot: I haz the power, 2016-10-19 16:00:35]

Try jobs failed on following builders:
  presubmit on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[aleloi, 2016-10-20 08:22:21]

The CQ bit was checked by aleloi@webrtc.org

[aleloi, 2016-10-20 08:22:22]

The patchset sent to the CQ was uploaded after l-g-t-m from tommi@webrtc.org,
peah@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2408143003/#ps80001 (title:
"Rebase.")

[commit-bot: I haz the power, 2016-10-20 08:22:28]

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-10-20 08:53:32]

Description was changed from

==========
Added a level controller initialization value to MediaConstraints.

An audio track with a level controller with the correct initialization
value can be created by a combination of
PeerConnectionFactory::CreateAudioTrack(..., audio_source) and
either
audio_source = PeerConnectionFactory::CreateAudioSource(constraints) or
audio_source = PeerConnectionFactory::CreateAudioSource(audio_options).

NOTRY=True
BUG=webrtc:6386
==========

to

==========
Added a level controller initialization value to MediaConstraints.

An audio track with a level controller with the correct initialization
value can be created by a combination of
PeerConnectionFactory::CreateAudioTrack(..., audio_source) and
either
audio_source = PeerConnectionFactory::CreateAudioSource(constraints) or
audio_source = PeerConnectionFactory::CreateAudioSource(audio_options).

NOTRY=True
BUG=webrtc:6386
==========

[commit-bot: I haz the power, 2016-10-20 08:53:33]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-10-20 08:53:37]

Description was changed from

==========
Added a level controller initialization value to MediaConstraints.

An audio track with a level controller with the correct initialization
value can be created by a combination of
PeerConnectionFactory::CreateAudioTrack(..., audio_source) and
either
audio_source = PeerConnectionFactory::CreateAudioSource(constraints) or
audio_source = PeerConnectionFactory::CreateAudioSource(audio_options).

NOTRY=True
BUG=webrtc:6386
==========

to

==========
Added a level controller initialization value to MediaConstraints.

An audio track with a level controller with the correct initialization
value can be created by a combination of
PeerConnectionFactory::CreateAudioTrack(..., audio_source) and
either
audio_source = PeerConnectionFactory::CreateAudioSource(constraints) or
audio_source = PeerConnectionFactory::CreateAudioSource(audio_options).

NOTRY=True
BUG=webrtc:6386

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

[commit-bot: I haz the power, 2016-10-20 08:53:38]

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