Updated structures and functions for setting the max bitrate limit to take rtc::Optional<int>

webrtc/media/engine/webrtcvideoengine2.h

Index: webrtc/media/engine/webrtcvideoengine2.h
diff --git a/webrtc/media/engine/webrtcvideoengine2.h b/webrtc/media/engine/webrtcvideoengine2.h
index 850bcaef63e161542e7b7267a302e8638d8fe9dd..1f602f9824518d96a1cbbbf0b24c7cb13eb07751 100644
--- a/webrtc/media/engine/webrtcvideoengine2.h
+++ b/webrtc/media/engine/webrtcvideoengine2.h
@@ -191,7 +191,7 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport {
     // These optionals are unset if not changed.
     rtc::Optional<VideoCodecSettings> codec;
     rtc::Optional<std::vector<webrtc::RtpExtension>> rtp_header_extensions;
-    rtc::Optional<int> max_bandwidth_bps;
+    rtc::Optional<rtc::Optional<int>> max_bandwidth_bps;

[stefan-webrtc, 2016/03/18 08:28:40]

What is an optional optional? This looks super weird to me.

[skvlad, 2016/03/18 18:01:52]

As Peter said, this structure represents the diff from the previous values - and
it uses rtc::Optional<> with no value to indicate that there was no change.
The bandwidth limit parameter itself is optional - so it can not change at all
(outer rtc::Optional has no value), or it can change to a new limit
(rtc::Optional with rtc::Optional with a new value), or it can become unset
after it was set before - in which case it will be an rtc::Optional
(representing the diff) with an rtc::Optional (representing the setting) with no
value (no limit).

Do you think if we add a type alias - like rtc::Change<T> - for this particular
case, it will make it clearer? "rtc::Change<rtc::Optional<int>>"? Or will the
overhead of the new type make it harder to read?

[pthatcher1, 2016/03/21 17:41:13]

I think that would make it less clear about what's going on.  

[Taylor Brandstetter, 2016/03/29 02:26:58]

I think just a comment above this line explaining the semantics of each
"Optional" would suffice.

[skvlad, 2016/03/30 19:40:44]

Done.

     rtc::Optional<bool> conference_mode;
     rtc::Optional<webrtc::RtcpMode> rtcp_mode;
   };
@@ -239,7 +239,7 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport {
         const VideoOptions& options,
         WebRtcVideoEncoderFactory* external_encoder_factory,
         bool enable_cpu_overuse_detection,
-        int max_bitrate_bps,
+        rtc::Optional<int> max_bitrate_bps,
         const rtc::Optional<VideoCodecSettings>& codec_settings,
         const std::vector<webrtc::RtpExtension>& rtp_extensions,
         const VideoSendParameters& send_params);
@@ -276,11 +276,11 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport {
       VideoSendStreamParameters(
           const webrtc::VideoSendStream::Config& config,
           const VideoOptions& options,
-          int max_bitrate_bps,
+          rtc::Optional<int> max_bitrate_bps,
           const rtc::Optional<VideoCodecSettings>& codec_settings);
       webrtc::VideoSendStream::Config config;
       VideoOptions options;
-      int max_bitrate_bps;
+      rtc::Optional<int> max_bitrate_bps;
       bool conference_mode;
       rtc::Optional<VideoCodecSettings> codec_settings;
       // Sent resolutions + bitrates etc. by the underlying VideoSendStream,
@@ -321,12 +321,12 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport {
     static std::vector<webrtc::VideoStream> CreateVideoStreams(
         const VideoCodec& codec,
         const VideoOptions& options,
-        int max_bitrate_bps,
+        rtc::Optional<int> max_bitrate_bps,
         size_t num_streams);
     static std::vector<webrtc::VideoStream> CreateSimulcastVideoStreams(
         const VideoCodec& codec,
         const VideoOptions& options,
-        int max_bitrate_bps,
+        rtc::Optional<int> max_bitrate_bps,
         size_t num_streams);
 
     void* ConfigureVideoEncoderSettings(const VideoCodec& codec)