Non-constraint interfaces for all constrainable interfaces

webrtc/api/peerconnectioninterface.h

 /*
  *  Copyright 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 225 matching lines @@
     RtcpMuxPolicy rtcp_mux_policy;
     TcpCandidatePolicy tcp_candidate_policy;
     int audio_jitter_buffer_max_packets;
     bool audio_jitter_buffer_fast_accelerate;
     int ice_connection_receiving_timeout;         // ms
     int ice_backup_candidate_pair_ping_interval;  // ms
     ContinualGatheringPolicy continual_gathering_policy;
     std::vector<rtc::scoped_refptr<rtc::RTCCertificate>> certificates;
     bool disable_prerenderer_smoothing;
     bool prioritize_most_likely_ice_candidate_pairs;
+    // Flags corresponding to values set by constraint flags.
+    // rtc::Optional flags can be "missing", in which case the webrtc
+    // default applies.
+    bool disable_ipv6;
+    rtc::Optional<bool> enable_dscp;
+    bool enable_rtp_data_channel;

[pthatcher1, 2016/03/03 17:51:17]

Why are two of these optional, but the rest not?

[hta-webrtc, 2016/03/03 17:57:50]

The DSCP one was the one where I found that it didn't have a hardwired default,
so that I had to preserve the information about whether or not the client had
asked for a specific value. Thinking more about it, I decided that if we were
ever to change a default on one of the other ones, we'd likely use a Finch
experiment, which would need exactly the same thing. But I couldn't imagine
changing our minds on defaults for IPv6 and RTP data channels, which is why I
left them as boolean.

So there's a certain logic to it. If a consistent treatment is important, I can
make them all optional. (Nisse pushed back on the use of optional, btw)

[nisse-webrtc, 2016/03/04 08:23:30]

For the ones that map to MediaConfig flags, I think the right way is to embed a
MediaConfig in RTCConfiguration, without any rtc::Optional. And refactor the
code so that values are assigned to MediaConfig flags in the order

1. Webrtc compiled-in defaults, set in the MediaConfig default constructor.

2. Overriding defaults set by the application, e.g., configuration of chrome
experiments.

3. Constraints (or other means of configuration) specific to a single
PeerConnection.

And I'd expect similar logic can be applied to other settings too. But I've
agreed with Harald that investigation of that path can be a later cl.

+    rtc::Optional<bool> cpu_overuse_detection;
+    rtc::Optional<bool> suspend_below_min_bitrate;
+    rtc::Optional<int> screencast_min_bitrate;
+    rtc::Optional<bool> combined_audio_video_bwe;
+    rtc::Optional<bool> enable_dtls_srtp;
     RTCConfiguration()
         : type(kAll),
           bundle_policy(kBundlePolicyBalanced),
           rtcp_mux_policy(kRtcpMuxPolicyNegotiate),
           tcp_candidate_policy(kTcpCandidatePolicyEnabled),
           audio_jitter_buffer_max_packets(kAudioJitterBufferMaxPackets),
           audio_jitter_buffer_fast_accelerate(false),
           ice_connection_receiving_timeout(kUndefined),
           ice_backup_candidate_pair_ping_interval(kUndefined),
           continual_gathering_policy(GATHER_ONCE),
           disable_prerenderer_smoothing(false),
-          prioritize_most_likely_ice_candidate_pairs(false) {}
+          prioritize_most_likely_ice_candidate_pairs(false),
+          disable_ipv6(false),
+          enable_rtp_data_channel(false) {}
   };
 
   struct RTCOfferAnswerOptions {
     static const int kUndefined = -1;
     static const int kMaxOfferToReceiveMedia = 1;
 
     // The default value for constraint offerToReceiveX:true.
     static const int kOfferToReceiveMediaTrue = 1;
 
     int offer_to_receive_video;
@@ skipping 107 matching lines @@
                            const MediaConstraintsInterface* constraints) {}
 
   // TODO(jiayl): remove the default impl and the old interface when chromium
   // code is updated.
   virtual void CreateOffer(CreateSessionDescriptionObserver* observer,
                            const RTCOfferAnswerOptions& options) {}
 
   // Create an answer to an offer.
   // The CreateSessionDescriptionObserver callback will be called when done.
   virtual void CreateAnswer(CreateSessionDescriptionObserver* observer,
-                            const MediaConstraintsInterface* constraints) = 0;
+                            const RTCOfferAnswerOptions& options) {}
+  // Deprecated - use version above.
+  // TODO(hta): Remove and remove default implementations when all callers
+  // are updated.
+  virtual void CreateAnswer(CreateSessionDescriptionObserver* observer,
+                            const MediaConstraintsInterface* constraints) {}
+
   // Sets the local session description.
   // JsepInterface takes the ownership of |desc| even if it fails.
   // The |observer| callback will be called when done.
   virtual void SetLocalDescription(SetSessionDescriptionObserver* observer,
                                    SessionDescriptionInterface* desc) = 0;
   // Sets the remote session description.
   // JsepInterface takes the ownership of |desc| even if it fails.
   // The |observer| callback will be called when done.
   virtual void SetRemoteDescription(SetSessionDescriptionObserver* observer,
                                     SessionDescriptionInterface* desc) = 0;
   // Restarts or updates the ICE Agent process of gathering local candidates
   // and pinging remote candidates.
   // TODO(deadbeef): Remove once Chrome is moved over to SetConfiguration.
   virtual bool UpdateIce(const IceServers& configuration,
                          const MediaConstraintsInterface* constraints) {
     return false;
   }
+  virtual bool UpdateIce(const IceServers& configuration) { return false; }

[perkj_webrtc, 2016/03/04 08:44:36]

nit: Is this really needed? 

   // Sets the PeerConnection's global configuration to |config|.
   // Any changes to STUN/TURN servers or ICE candidate policy will affect the
   // next gathering phase, and cause the next call to createOffer to generate
   // new ICE credentials. Note that the BUNDLE and RTCP-multiplexing policies
   // cannot be changed with this method.
   // TODO(deadbeef): Make this pure virtual once all Chrome subclasses of
   // PeerConnectionInterface implement it.
   virtual bool SetConfiguration(
       const PeerConnectionInterface::RTCConfiguration& config) {
     return false;
@@ skipping 106 matching lines @@
 
   virtual void SetOptions(const Options& options) = 0;
 
   virtual rtc::scoped_refptr<PeerConnectionInterface> CreatePeerConnection(
       const PeerConnectionInterface::RTCConfiguration& configuration,
       const MediaConstraintsInterface* constraints,
       rtc::scoped_ptr<cricket::PortAllocator> allocator,
       rtc::scoped_ptr<DtlsIdentityStoreInterface> dtls_identity_store,
       PeerConnectionObserver* observer) = 0;
 
+  virtual rtc::scoped_refptr<PeerConnectionInterface> CreatePeerConnection(
+      const PeerConnectionInterface::RTCConfiguration& configuration,
+      rtc::scoped_ptr<cricket::PortAllocator> allocator,
+      rtc::scoped_ptr<DtlsIdentityStoreInterface> dtls_identity_store,
+      PeerConnectionObserver* observer) = 0;
+
   virtual rtc::scoped_refptr<MediaStreamInterface>
       CreateLocalMediaStream(const std::string& label) = 0;
 
   // Creates a AudioSourceInterface.
   // |constraints| decides audio processing settings but can be NULL.
   virtual rtc::scoped_refptr<AudioSourceInterface> CreateAudioSource(
+      const cricket::AudioOptions& options) = 0;
+  // Deprecated - use version above.
+  virtual rtc::scoped_refptr<AudioSourceInterface> CreateAudioSource(
       const MediaConstraintsInterface* constraints) = 0;
 
   // Creates a VideoSourceInterface. The new source take ownership of
-  // |capturer|. |constraints| decides video resolution and frame rate but can
+  // |capturer|.
+  virtual rtc::scoped_refptr<VideoSourceInterface> CreateVideoSource(
+      cricket::VideoCapturer* capturer) = 0;
+  // A video source creator that allows selection of resolution and frame rate.
+  // |constraints| decides video resolution and frame rate but can
   // be NULL.
+  // In the NULL case, use the version above.
   virtual rtc::scoped_refptr<VideoSourceInterface> CreateVideoSource(
       cricket::VideoCapturer* capturer,
       const MediaConstraintsInterface* constraints) = 0;
 
   // Creates a new local VideoTrack. The same |source| can be used in several
   // tracks.
   virtual rtc::scoped_refptr<VideoTrackInterface>
       CreateVideoTrack(const std::string& label,
                        VideoSourceInterface* source) = 0;
 
@@ skipping 48 matching lines @@
 CreatePeerConnectionFactory(
     rtc::Thread* worker_thread,
     rtc::Thread* signaling_thread,
     AudioDeviceModule* default_adm,
     cricket::WebRtcVideoEncoderFactory* encoder_factory,
     cricket::WebRtcVideoDecoderFactory* decoder_factory);
 
 }  // namespace webrtc
 
 #endif  // WEBRTC_API_PEERCONNECTIONINTERFACE_H_