Non-constraint interfaces for all constrainable interfaces

webrtc/api/peerconnection.cc

Index: webrtc/api/peerconnection.cc
diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc
index ee359271c2fc2573b2f209d9af2c365ed6a5970c..7cb5bc93d93743b6be9b14b95cc1334cd05ae1fc 100644
--- a/webrtc/api/peerconnection.cc
+++ b/webrtc/api/peerconnection.cc
@@ -452,6 +452,13 @@ bool ConvertRtcOptionsForOffer(
   return true;
 }
 
+bool ConvertRtcOptionsForAnswer(
+    const PeerConnectionInterface::RTCOfferAnswerOptions& rtc_options,
+    cricket::MediaSessionOptions* session_options) {
+  // TODO(hta): Figure out if there's a difference

[nisse-webrtc, 2016/02/26 08:53:45]

Will you figure it out before landing?

[hta-webrtc, 2016/02/29 11:09:48]

Yes, I've decided that there is no difference we care about.
Will delete.

+  return ConvertRtcOptionsForOffer(rtc_options, session_options);
+}
+
 bool ParseConstraintsForAnswer(const MediaConstraintsInterface* constraints,
                                cricket::MediaSessionOptions* session_options) {
   bool value = false;
@@ -565,8 +572,8 @@ PeerConnection::~PeerConnection() {
 }
 
 bool PeerConnection::Initialize(
+    const cricket::MediaConfig& media_config,
     const PeerConnectionInterface::RTCConfiguration& configuration,
-    const MediaConstraintsInterface* constraints,
     rtc::scoped_ptr<cricket::PortAllocator> allocator,
     rtc::scoped_ptr<DtlsIdentityStoreInterface> dtls_identity_store,
     PeerConnectionObserver* observer) {
@@ -591,13 +598,10 @@ bool PeerConnection::Initialize(
   int portallocator_flags = port_allocator_->flags();
   portallocator_flags |= cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET |
                          cricket::PORTALLOCATOR_ENABLE_IPV6;
-  bool value;
-  // If IPv6 flag was specified, we'll not override it by experiment.
-  if (FindConstraint(constraints, MediaConstraintsInterface::kEnableIPv6,
-                     &value, nullptr)) {
-    if (!value) {
-      portallocator_flags &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6);
-    }
+  // If the disable-IPv6 flag was specified, we'll not override it
+  // by experiment.
+  if (configuration.disable_ipv6) {
+    portallocator_flags &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6);
   } else if (webrtc::field_trial::FindFullName("WebRTC-IPv6Default") ==
              "Disabled") {
     portallocator_flags &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6);
@@ -612,19 +616,6 @@ bool PeerConnection::Initialize(
   // No step delay is used while allocating ports.
   port_allocator_->set_step_delay(cricket::kMinimumStepDelay);
 
-  // We rely on default values when constraints aren't found.
-  cricket::MediaConfig media_config;
-
-  media_config.disable_prerenderer_smoothing =
-      configuration.disable_prerenderer_smoothing;
-
-  // Find DSCP constraint.
-  FindConstraint(constraints, MediaConstraintsInterface::kEnableDscp,
-                 &media_config.enable_dscp, NULL);
-  // Find constraints for cpu overuse detection.
-  FindConstraint(constraints, MediaConstraintsInterface::kCpuOveruseDetection,
-                 &media_config.enable_cpu_overuse_detection, NULL);
-
   media_controller_.reset(factory_->CreateMediaController(media_config));
 
   remote_stream_factory_.reset(new RemoteMediaStreamFactory(
@@ -636,8 +627,8 @@ bool PeerConnection::Initialize(
   stats_.reset(new StatsCollector(this));
 
   // Initialize the WebRtcSession. It creates transport channels etc.
-  if (!session_->Initialize(factory_->options(), constraints,
-                            std::move(dtls_identity_store), configuration)) {
+  if (!session_->Initialize(factory_->options(), std::move(dtls_identity_store),
+                            configuration)) {
     return false;
   }
 
@@ -1001,7 +992,26 @@ void PeerConnection::CreateAnswer(
     return;
   }
 
-  session_->CreateAnswer(observer, constraints, session_options);
+  session_->CreateAnswer(observer, session_options);
+}
+
+void PeerConnection::CreateAnswer(CreateSessionDescriptionObserver* observer,
+                                  const RTCOfferAnswerOptions& options) {
+  TRACE_EVENT0("webrtc", "PeerConnection::CreateAnswer");
+  if (!VERIFY(observer != nullptr)) {
+    LOG(LS_ERROR) << "CreateAnswer - observer is NULL.";
+    return;
+  }
+
+  cricket::MediaSessionOptions session_options;
+  if (!GetOptionsForAnswer(options, &session_options)) {
+    std::string error = "CreateAnswer called with invalid options.";
+    LOG(LS_ERROR) << error;
+    PostCreateSessionDescriptionFailure(observer, error);
+    return;
+  }
+
+  session_->CreateAnswer(observer, session_options);
 }
 
 void PeerConnection::SetLocalDescription(
@@ -1533,11 +1543,8 @@ bool PeerConnection::GetOptionsForOffer(
   return true;
 }
 
-bool PeerConnection::GetOptionsForAnswer(
-    const MediaConstraintsInterface* constraints,
+void PeerConnection::FinishOptionsForAnswer(
     cricket::MediaSessionOptions* session_options) {
-  session_options->recv_audio = false;
-  session_options->recv_video = false;
   // TODO(deadbeef): Once we have transceivers, enumerate them here instead of
   // ContentInfos.
   if (session_->remote_description()) {
@@ -1548,10 +1555,6 @@ bool PeerConnection::GetOptionsForAnswer(
           cricket::TransportOptions();
     }
   }
-  if (!ParseConstraintsForAnswer(constraints, session_options)) {
-    return false;
-  }
-
   AddSendStreams(session_options, senders_, rtp_data_channels_);
   session_options->bundle_enabled =
       session_options->bundle_enabled &&
@@ -1564,6 +1567,29 @@ bool PeerConnection::GetOptionsForAnswer(
   if (session_->data_channel_type() == cricket::DCT_SCTP) {
     session_options->data_channel_type = cricket::DCT_SCTP;
   }
+}
+
+bool PeerConnection::GetOptionsForAnswer(
+    const MediaConstraintsInterface* constraints,
+    cricket::MediaSessionOptions* session_options) {
+  session_options->recv_audio = false;
+  session_options->recv_video = false;
+  if (!ParseConstraintsForAnswer(constraints, session_options)) {
+    return false;
+  }
+  FinishOptionsForAnswer(session_options);
+  return true;
+}
+
+bool PeerConnection::GetOptionsForAnswer(
+    const RTCOfferAnswerOptions& options,
+    cricket::MediaSessionOptions* session_options) {
+  session_options->recv_audio = false;
+  session_options->recv_video = false;
+  if (!ConvertRtcOptionsForAnswer(options, session_options)) {
+    return false;
+  }
+  FinishOptionsForAnswer(session_options);
   return true;
 }
 
@@ -2099,4 +2125,41 @@ DataChannel* PeerConnection::FindDataChannelBySid(int sid) const {
   return nullptr;
 }
 
+void CopyConstraintsIntoRtcConfiguration(
+    const MediaConstraintsInterface* constraints,
+    PeerConnectionInterface::RTCConfiguration* configuration) {
+  // Copy info from constraints into configuration, if present.
+  if (!constraints) {
+    return;
+  }
+
+  bool value;
+  if (FindConstraint(constraints, MediaConstraintsInterface::kEnableIPv6,
+                     &value, nullptr)) {
+    if (!value) {
+      configuration->disable_ipv6 = true;
+    }
+  }
+  configuration->enable_dscp = ConstraintToOptionalBool(

[nisse-webrtc, 2016/02/25 15:25:54]

I like this version better. Have you considered including a MediaConfig inside
RTCConfiguration, is there any reason that won't work (compatibility...)? Then
this function should just write the corresponding flags directly into that
struct (enable_dscp, enable_cpu_adaptation, disable_prerenderer_smoothing)
without any Optional.

The way it would work, an application which just creates a RTCConfigurations
gets the values determined by the default constructor, and possibly overridden
by constraints. And if the application sets specific values, they're used
instead of the compiled-in defaults (in the absense of constraints).

I'm also curious about how constraints will be applied without the
MediaConstraintsInterface? There will be code somewhere parsing the wire
protocol (is that sdp?), and storing values directly in an RTCConfiguration?

[hta-webrtc, 2016/02/25 16:06:07]

But then the logic for deciding what was the default, which is currently in
webrtc, moves to the application. That's the part I don't like.
Remember: There are (will be) no more constraints in webrtc. Either it's
optionals, or it's straight booleans. If we want webrtc to make the decision,
it's got to be optionals. If the client app makes the decision *always*,
booleans are good enough.

 And if the application sets specific values, they're used
The creation of the PeerConnection is done before SDP enters the picture - SDP
comes in with setLocalDescription and setRemoteDescription.

+      constraints, MediaConstraintsInterface::kEnableDscp);
+  configuration->cpu_overuse_detection = ConstraintToOptionalBool(
+      constraints, MediaConstraintsInterface::kCpuOveruseDetection);
+  if (FindConstraint(constraints,
+                     MediaConstraintsInterface::kEnableRtpDataChannels, &value,
+                     NULL) &&
+      value) {
+    configuration->enable_rtp_data_channel = true;
+  }
+  // Find Suspend Below Min Bitrate constraint.
+  configuration->suspend_below_min_bitrate = ConstraintToOptionalBool(
+      constraints,
+      MediaConstraintsInterface::kEnableVideoSuspendBelowMinBitrate);
+  configuration->screencast_min_bitrate = ConstraintToOptionalInt(
+      constraints, MediaConstraintsInterface::kScreencastMinBitrate);
+  configuration->combined_audio_video_bwe = ConstraintToOptionalBool(
+      constraints, MediaConstraintsInterface::kCombinedAudioVideoBwe);
+  configuration->enable_dtls_srtp = ConstraintToOptionalBool(
+      constraints, MediaConstraintsInterface::kEnableDtlsSrtp);
+}
+
 }  // namespace webrtc