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 {
+  // Set DSCP value for packet sent from media channel. This flag
+  // comes from the PeerConnection constraint 'googDscp'.
+  bool enable_dscp = false;

[pbos-webrtc, 2016/02/01 10:57:54]

To me it sounds like DSCP_DEFAULT is also some form of DSCP? If so this flag
name doesn't make sense (but it didn't before either). pthatcher@ can you help
name this?

If DSCP_DEFAULT means that DSCP is off, please comment that in the DscpValue()
method below, or rename to DSCP_OFF if possible.

[nisse-webrtc, 2016/02/01 11:26:34]

We're talking about is the diffserv bits in the IPv4 header, right? If so, it's
not optional, they're present in every packet, with some default if you don't
tell the IP stack otherwise.

In this piece of the code, my intention is to just mirror the constraint on the
wire. I hope we don't need three states, i.e., that a missing "googDscp"
constraint should be in all respects equivalent to a "googDscp" constraint with
the value false.  

+};
+
 // 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_.enable_dscp ? DscpValue() : rtc::DSCP_DEFAULT);
   }
-
+  virtual rtc::DiffServCodePoint DscpValue() const { return rtc::DSCP_DEFAULT; }

[pbos-webrtc, 2016/02/01 10:57:54]

This method name confuses me, should this be DscpValueForMediaTypeWhenEnabled()
or something? pthatcher@ how do we make this more readable?

[nisse-webrtc, 2016/02/01 11:26:34]

The intention is to return the diffserve codepoint appropriate for this media
type. It's very static. The videoengine and voiceengine report different values,


For video AF41, "assured forwarding", see
https://en.wikipedia.org/wiki/Differentiated_services#Assured_Forwarding_.28A...

For audio, it's EF, high priority expedited forwarding.

For data, I just return DSCP_DEFAULT, which means that enable_dscp has no
effect.

Personally, I think diffserv is pretty useless.

[pbos-webrtc, 2016/02/01 11:27:21]

Name this MediaTypeDscpValue() then perhaps?

[nisse-webrtc, 2016/02/01 12:09:45]

Done.

   // 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 141 matching lines @@
   // 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_