Split out dscp option from VideoOptions to new struct MediaChannelOptions.

talk/media/base/mediachannel.h

 /*
  * libjingle
  * Copyright 2004 Google Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  *  1. Redistributions of source code must retain the above copyright notice,
  *     this list of conditions and the following disclaimer.
  *  2. Redistributions in binary form must reproduce the above copyright notice,
@@ skipping 76 matching lines @@
     for (size_t i = 0; i < vals.size(); ++i) {
       if (i > 0) {
         ost << ", ";
       }
       ost << vals[i].ToString();
     }
     ost << "]";
     return ost.str();
 }
 
+struct MediaChannelOptions {
+  MediaChannelOptions() {}

[pbos-webrtc, 2016/01/29 16:22:56]

Drop the ctor unless you want to move it to a .cc file.

[nisse-webrtc, 2016/02/01 08:29:49]

I think I'm showing C++ ignorance here. The thing is, I want a default
constructor setting the options to false. If I define a plain struct in C++, do
I get a default constructor initializing all members properly? Or will new
instances get uninitialized values?

+  // Set DSCP value for packet sent from media channel. This flag
+  // comes from the PeerConnection constraint 'googDscp'.
+  bool dscp;  // TODO(nisse): More descriptive name?

[pbos-webrtc, 2016/01/29 16:22:56]

enable_dscp? or enable_dscp_41 or whatever it was called? 'bool enable_dscp =
false;'

[nisse-webrtc, 2016/02/01 08:29:49]

enable_dcsp sounds good. And the actual codepoint used when enabled is different
for audio and video channels. 

That's why I had to add the DscpValue method. I don't quite like the way it's
used, I would prefer to have a member of type rtc::DiffServCodePoint, and set it
on construction depending on the value of the enable_dscp option and the
subclass being constructed. But I haven't found any nice and easy way to do that
(the "obvious" way, to let the constructor call the virtual method DscpValue,
doesn't work).

+};
+
 // Options that can be applied to a VoiceMediaChannel or a VoiceMediaEngine.
 // Used to be flags, but that makes it hard to selectively apply options.
 // We are moving all of the setting of options to structs like this,
 // but some things currently still use flags.
 struct AudioOptions {
   void SetAll(const AudioOptions& change) {
     SetFrom(&echo_cancellation, change.echo_cancellation);
     SetFrom(&auto_gain_control, change.auto_gain_control);
     SetFrom(&noise_suppression, change.noise_suppression);
     SetFrom(&highpass_filter, change.highpass_filter);
@@ skipping 10 matching lines @@
     SetFrom(&extended_filter_aec, change.extended_filter_aec);
     SetFrom(&delay_agnostic_aec, change.delay_agnostic_aec);
     SetFrom(&experimental_ns, change.experimental_ns);
     SetFrom(&aec_dump, change.aec_dump);
     SetFrom(&tx_agc_target_dbov, change.tx_agc_target_dbov);
     SetFrom(&tx_agc_digital_compression_gain,
             change.tx_agc_digital_compression_gain);
     SetFrom(&tx_agc_limiter, change.tx_agc_limiter);
     SetFrom(&recording_sample_rate, change.recording_sample_rate);
     SetFrom(&playout_sample_rate, change.playout_sample_rate);
-    SetFrom(&dscp, change.dscp);
     SetFrom(&combined_audio_video_bwe, change.combined_audio_video_bwe);
   }
 
   bool operator==(const AudioOptions& o) const {
     return echo_cancellation == o.echo_cancellation &&
         auto_gain_control == o.auto_gain_control &&
         noise_suppression == o.noise_suppression &&
         highpass_filter == o.highpass_filter &&
         stereo_swapping == o.stereo_swapping &&
         audio_jitter_buffer_max_packets == o.audio_jitter_buffer_max_packets &&
         audio_jitter_buffer_fast_accelerate ==
             o.audio_jitter_buffer_fast_accelerate &&
         typing_detection == o.typing_detection &&
         aecm_generate_comfort_noise == o.aecm_generate_comfort_noise &&
         conference_mode == o.conference_mode &&
         experimental_agc == o.experimental_agc &&
         extended_filter_aec == o.extended_filter_aec &&
         delay_agnostic_aec == o.delay_agnostic_aec &&
         experimental_ns == o.experimental_ns &&
         adjust_agc_delta == o.adjust_agc_delta &&
         aec_dump == o.aec_dump &&
         tx_agc_target_dbov == o.tx_agc_target_dbov &&
         tx_agc_digital_compression_gain == o.tx_agc_digital_compression_gain &&
         tx_agc_limiter == o.tx_agc_limiter &&
         recording_sample_rate == o.recording_sample_rate &&
         playout_sample_rate == o.playout_sample_rate &&
-        dscp == o.dscp &&
         combined_audio_video_bwe == o.combined_audio_video_bwe;
   }
 
   std::string ToString() const {
     std::ostringstream ost;
     ost << "AudioOptions {";
     ost << ToStringIfSet("aec", echo_cancellation);
     ost << ToStringIfSet("agc", auto_gain_control);
     ost << ToStringIfSet("ns", noise_suppression);
     ost << ToStringIfSet("hf", highpass_filter);
@@ skipping 10 matching lines @@
     ost << ToStringIfSet("extended_filter_aec", extended_filter_aec);
     ost << ToStringIfSet("delay_agnostic_aec", delay_agnostic_aec);
     ost << ToStringIfSet("experimental_ns", experimental_ns);
     ost << ToStringIfSet("aec_dump", aec_dump);
     ost << ToStringIfSet("tx_agc_target_dbov", tx_agc_target_dbov);
     ost << ToStringIfSet("tx_agc_digital_compression_gain",
         tx_agc_digital_compression_gain);
     ost << ToStringIfSet("tx_agc_limiter", tx_agc_limiter);
     ost << ToStringIfSet("recording_sample_rate", recording_sample_rate);
     ost << ToStringIfSet("playout_sample_rate", playout_sample_rate);
-    ost << ToStringIfSet("dscp", dscp);
     ost << ToStringIfSet("combined_audio_video_bwe", combined_audio_video_bwe);
     ost << "}";
     return ost.str();
   }
 
   // Audio processing that attempts to filter away the output signal from
   // later inbound pickup.
   rtc::Optional<bool> echo_cancellation;
   // Audio processing to adjust the sensitivity of the local mic dynamically.
   rtc::Optional<bool> auto_gain_control;
@@ skipping 16 matching lines @@
   rtc::Optional<bool> extended_filter_aec;
   rtc::Optional<bool> delay_agnostic_aec;
   rtc::Optional<bool> experimental_ns;
   rtc::Optional<bool> aec_dump;
   // Note that tx_agc_* only applies to non-experimental AGC.
   rtc::Optional<uint16_t> tx_agc_target_dbov;
   rtc::Optional<uint16_t> tx_agc_digital_compression_gain;
   rtc::Optional<bool> tx_agc_limiter;
   rtc::Optional<uint32_t> recording_sample_rate;
   rtc::Optional<uint32_t> playout_sample_rate;
-  // Set DSCP value for packet sent from audio channel.
-  rtc::Optional<bool> dscp;
   // Enable combined audio+bandwidth BWE.
   rtc::Optional<bool> combined_audio_video_bwe;
 
  private:
   template <typename T>
   static void SetFrom(rtc::Optional<T>* s, const rtc::Optional<T>& o) {
     if (o) {
       *s = o;
     }
   }
 };
 
 // Options that can be applied to a VideoMediaChannel or a VideoMediaEngine.
 // Used to be flags, but that makes it hard to selectively apply options.
 // We are moving all of the setting of options to structs like this,
 // but some things currently still use flags.
 struct VideoOptions {
   void SetAll(const VideoOptions& change) {
     SetFrom(&video_noise_reduction, change.video_noise_reduction);
     SetFrom(&cpu_overuse_detection, change.cpu_overuse_detection);
     SetFrom(&conference_mode, change.conference_mode);
-    SetFrom(&dscp, change.dscp);
     SetFrom(&suspend_below_min_bitrate, change.suspend_below_min_bitrate);
     SetFrom(&screencast_min_bitrate_kbps, change.screencast_min_bitrate_kbps);
     SetFrom(&disable_prerenderer_smoothing,
             change.disable_prerenderer_smoothing);
   }
 
   bool operator==(const VideoOptions& o) const {
     return video_noise_reduction == o.video_noise_reduction &&
            cpu_overuse_detection == o.cpu_overuse_detection &&
            conference_mode == o.conference_mode &&
-           dscp == o.dscp &&
            suspend_below_min_bitrate == o.suspend_below_min_bitrate &&
            screencast_min_bitrate_kbps == o.screencast_min_bitrate_kbps &&
            disable_prerenderer_smoothing == o.disable_prerenderer_smoothing;
   }
 
   std::string ToString() const {
     std::ostringstream ost;
     ost << "VideoOptions {";
     ost << ToStringIfSet("noise reduction", video_noise_reduction);
     ost << ToStringIfSet("cpu overuse detection", cpu_overuse_detection);
     ost << ToStringIfSet("conference mode", conference_mode);
-    ost << ToStringIfSet("dscp", dscp);
     ost << ToStringIfSet("suspend below min bitrate",
                          suspend_below_min_bitrate);
     ost << ToStringIfSet("screencast min bitrate kbps",
                          screencast_min_bitrate_kbps);
     ost << "}";
     return ost.str();
   }
 
   // Enable denoising? This flag comes from the getUserMedia
   // constraint 'googNoiseReduction', and WebRtcVideoEngine2 passes it
   // on to the codec options. Disabled by default.
   rtc::Optional<bool> video_noise_reduction;
   // Enable WebRTC Cpu Overuse Detection. This flag comes from the
   // PeerConnection constraint 'googCpuOveruseDetection' and is
   // checked in WebRtcVideoChannel2::OnLoadUpdate, where it's passed
   // to VideoCapturer::video_adapter()->OnCpuResolutionRequest.
   rtc::Optional<bool> cpu_overuse_detection;
   // Use conference mode? This flag comes from the remote
   // description's SDP line 'a=x-google-flag:conference', copied over
   // by VideoChannel::SetRemoteContent_w, and ultimately used by
   // conference mode screencast logic in
   // WebRtcVideoChannel2::WebRtcVideoSendStream::CreateVideoEncoderConfig.
   // The special screencast behaviour is disabled by default.
   rtc::Optional<bool> conference_mode;
-  // Set DSCP value for packet sent from video channel. This flag
-  // comes from the PeerConnection constraint 'googDscp' and,
-  // WebRtcVideoChannel2::SetOptions checks it before calling
-  // MediaChannel::SetDscp. If enabled, rtc::DSCP_AF41 is used. If
-  // disabled, which is the default, rtc::DSCP_DEFAULT is used.
-  rtc::Optional<bool> dscp;
   // Enable WebRTC suspension of video. No video frames will be sent
   // when the bitrate is below the configured minimum bitrate. This
   // flag comes from the PeerConnection constraint
   // 'googSuspendBelowMinBitrate', and WebRtcVideoChannel2 copies it
   // to VideoSendStream::Config::suspend_below_min_bitrate.
   rtc::Optional<bool> suspend_below_min_bitrate;
   // Force screencast to use a minimum bitrate. This flag comes from
   // the PeerConnection constraint 'googScreencastMinBitrate'. It is
   // copied to the encoder config by WebRtcVideoChannel2.
   rtc::Optional<int> screencast_min_bitrate_kbps;
@@ skipping 68 matching lines @@
     enum SocketType { ST_RTP, ST_RTCP };
     virtual bool SendPacket(rtc::Buffer* packet,
                             const rtc::PacketOptions& options) = 0;
     virtual bool SendRtcp(rtc::Buffer* packet,
                           const rtc::PacketOptions& options) = 0;
     virtual int SetOption(SocketType type, rtc::Socket::Option opt,
                           int option) = 0;
     virtual ~NetworkInterface() {}
   };
 
-  MediaChannel() : network_interface_(NULL) {}
+  MediaChannel(const MediaChannelOptions& options)
+      : options_(options), network_interface_(NULL) {}
+  MediaChannel() : options_(), network_interface_(NULL) {}
   virtual ~MediaChannel() {}
 
   // Sets the abstract interface class for sending RTP/RTCP data.
   virtual void SetInterface(NetworkInterface *iface) {
     rtc::CritScope cs(&network_interface_crit_);
     network_interface_ = iface;
+    SetDscp(options_.dscp ? DscpValue() : rtc::DSCP_DEFAULT);
   }
-
+  virtual rtc::DiffServCodePoint DscpValue() const { return rtc::DSCP_DEFAULT; }
   // Called when a RTP packet is received.
   virtual void OnPacketReceived(rtc::Buffer* packet,
                                 const rtc::PacketTime& packet_time) = 0;
   // Called when a RTCP packet is received.
   virtual void OnRtcpReceived(rtc::Buffer* packet,
                               const rtc::PacketTime& packet_time) = 0;
   // Called when the socket's ability to send has changed.
   virtual void OnReadyToSend(bool ready) = 0;
   // Creates a new outgoing media stream with SSRCs and CNAME as described
   // by sp.
@@ skipping 27 matching lines @@
   int SetOption(NetworkInterface::SocketType type,
                 rtc::Socket::Option opt,
                 int option) {
     rtc::CritScope cs(&network_interface_crit_);
     if (!network_interface_)
       return -1;
 
     return network_interface_->SetOption(type, opt, option);
   }
 
- protected:
+ private:
   // This method sets DSCP |value| on both RTP and RTCP channels.
   int SetDscp(rtc::DiffServCodePoint value) {
     int ret;
     ret = SetOption(NetworkInterface::ST_RTP,
                     rtc::Socket::OPT_DSCP,
                     value);
     if (ret == 0) {
       ret = SetOption(NetworkInterface::ST_RTCP,
                       rtc::Socket::OPT_DSCP,
                       value);
     }
     return ret;
   }
 
- private:
   bool DoSendPacket(rtc::Buffer* packet,
                     bool rtcp,
                     const rtc::PacketOptions& options) {
     rtc::CritScope cs(&network_interface_crit_);
     if (!network_interface_)
       return false;
 
     return (!rtcp) ? network_interface_->SendPacket(packet, options)
                    : network_interface_->SendRtcp(packet, options);
   }
 
+  const MediaChannelOptions options_;
   // |network_interface_| can be accessed from the worker_thread and
   // from any MediaEngine threads. This critical section is to protect accessing
   // of network_interface_ object.
   rtc::CriticalSection network_interface_crit_;
   NetworkInterface* network_interface_;
 };
 
 enum SendFlags {
   SEND_NOTHING,
   SEND_MICROPHONE
@@ skipping 432 matching lines @@
     ERROR_REC_TYPING_NOISE_DETECTED,      // Typing noise is detected.
     ERROR_PLAY_DEVICE_OPEN_FAILED = 200,  // Could not open playout.
     ERROR_PLAY_DEVICE_MUTED,              // Playout muted by OS.
     ERROR_PLAY_DEVICE_REMOVED,            // Playout removed while active.
     ERROR_PLAY_RUNTIME_ERROR,             // Errors in voice processing.
     ERROR_PLAY_SRTP_ERROR,                // Generic SRTP failure.
     ERROR_PLAY_SRTP_AUTH_FAILED,          // Failed to authenticate packets.
     ERROR_PLAY_SRTP_REPLAY,               // Packet replay detected.
   };
 
+  VoiceMediaChannel(const MediaChannelOptions& options)
+      : MediaChannel(options) {}
   VoiceMediaChannel() {}
+
   virtual ~VoiceMediaChannel() {}
   virtual bool SetSendParameters(const AudioSendParameters& params) = 0;
   virtual bool SetRecvParameters(const AudioRecvParameters& params) = 0;
   // Starts or stops playout of received audio.
   virtual bool SetPlayout(bool playout) = 0;
   // Starts or stops sending (and potentially capture) of local audio.
   virtual bool SetSend(SendFlags flag) = 0;
   // Configure stream for sending.
   virtual bool SetAudioSend(uint32_t ssrc,
                             bool enable,
@@ skipping 42 matching lines @@
     ERROR_REC_DEVICE_IN_USE,              // Device is in already use.
     ERROR_REC_DEVICE_REMOVED,             // Device is removed.
     ERROR_REC_SRTP_ERROR,                 // Generic sender SRTP failure.
     ERROR_REC_SRTP_AUTH_FAILED,           // Failed to authenticate packets.
     ERROR_REC_CPU_MAX_CANT_DOWNGRADE,     // Can't downgrade capture anymore.
     ERROR_PLAY_SRTP_ERROR = 200,          // Generic receiver SRTP failure.
     ERROR_PLAY_SRTP_AUTH_FAILED,          // Failed to authenticate packets.
     ERROR_PLAY_SRTP_REPLAY,               // Packet replay detected.
   };
 
-  VideoMediaChannel() : renderer_(NULL) {}
+  VideoMediaChannel(const MediaChannelOptions& options)
+      : MediaChannel(options), renderer_(NULL) {}
+  VideoMediaChannel() : MediaChannel(), renderer_(NULL) {}
   virtual ~VideoMediaChannel() {}
 
   virtual bool SetSendParameters(const VideoSendParameters& params) = 0;
   virtual bool SetRecvParameters(const VideoRecvParameters& params) = 0;
   // Gets the currently set codecs/payload types to be used for outgoing media.
   virtual bool GetSendCodec(VideoCodec* send_codec) = 0;
   // Sets the format of a specified outgoing stream.
   virtual bool SetSendStreamFormat(uint32_t ssrc,
                                    const VideoFormat& format) = 0;
   // Starts or stops transmission (and potentially capture) of local video.
@@ skipping 112 matching lines @@
   enum Error {
     ERROR_NONE = 0,                       // No error.
     ERROR_OTHER,                          // Other errors.
     ERROR_SEND_SRTP_ERROR = 200,          // Generic SRTP failure.
     ERROR_SEND_SRTP_AUTH_FAILED,          // Failed to authenticate packets.
     ERROR_RECV_SRTP_ERROR,                // Generic SRTP failure.
     ERROR_RECV_SRTP_AUTH_FAILED,          // Failed to authenticate packets.
     ERROR_RECV_SRTP_REPLAY,               // Packet replay detected.
   };
 
+  DataMediaChannel() {}

[pbos-webrtc, 2016/01/29 16:22:56]

Do you need/want this ctor?

[nisse-webrtc, 2016/02/01 08:29:49]

Probably not, it's a leftover from an earlier version where MediaChannel lacked
a default constructor. I'll delete it.

+
   virtual ~DataMediaChannel() {}
 
   virtual bool SetSendParameters(const DataSendParameters& params) = 0;
   virtual bool SetRecvParameters(const DataRecvParameters& params) = 0;
 
   // TODO(pthatcher): Implement this.
   virtual bool GetStats(DataMediaInfo* info) { return true; }
 
   virtual bool SetSend(bool send) = 0;
   virtual bool SetReceive(bool receive) = 0;
 
   virtual bool SendData(
       const SendDataParams& params,
       const rtc::Buffer& payload,
       SendDataResult* result = NULL) = 0;
   // Signals when data is received (params, data, len)
   sigslot::signal3<const ReceiveDataParams&,
                    const char*,
                    size_t> SignalDataReceived;
   // Signal when the media channel is ready to send the stream. Arguments are:
   //     writable(bool)
   sigslot::signal1<bool> SignalReadyToSend;
   // Signal for notifying that the remote side has closed the DataChannel.
   sigslot::signal1<uint32_t> SignalStreamClosedRemotely;
 };
 
 }  // namespace cricket
 
 #endif  // TALK_MEDIA_BASE_MEDIACHANNEL_H_