Refactor the relationship between BaseChannel and MediaChannel so that we send over all the paramet…

talk/session/media/channel.cc

Index: talk/session/media/channel.cc
diff --git a/talk/session/media/channel.cc b/talk/session/media/channel.cc
index d30972db06f13ef94bc5ca347d01ae4796034b87..d3d63d1e466adc41629e69b01ad74e4edcbdc656 100644
--- a/talk/session/media/channel.cc
+++ b/talk/session/media/channel.cc
@@ -899,6 +899,23 @@ void BaseChannel::ChannelNotWritable_w() {
   ChangeState();
 }
 
+bool BaseChannel::SetRtpTransportParameters_w(
+    const MediaContentDescription* content,
+    ContentAction action,
+    ContentSource src,
+    std::string* error_desc) {
+  // Cache secure_required_ for belt and suspenders check on SendPacket
+  set_secure_required(content->crypto_required() != CT_NONE);
+  if (!SetSrtp_w(content->cryptos(), action, src, error_desc)) {
+    return false;

[pbos-webrtc, 2015/07/14 08:25:01]

Don't you need SafeSetError anymore? (I never understood what that one did.)

[pthatcher1, 2015/07/16 10:47:11]

SetSrtp_w call SafeSetError.

+  }
+
+  if (!SetRtcpMux_w(content->rtcp_mux(), action, src, error_desc)) {
+    return false;
+  }
+  return true;
+}
+
 // |dtls| will be set to true if DTLS is active for transport channel and
 // crypto is empty.
 bool BaseChannel::CheckSrtpConfig(const std::vector<CryptoParams>& cryptos,
@@ -913,42 +930,6 @@ bool BaseChannel::CheckSrtpConfig(const std::vector<CryptoParams>& cryptos,
   return true;
 }
 
-bool BaseChannel::SetRecvRtpHeaderExtensions_w(
-    const MediaContentDescription* content,
-    MediaChannel* media_channel,
-    std::string* error_desc) {
-  if (content->rtp_header_extensions_set()) {
-    if (!media_channel->SetRecvRtpHeaderExtensions(
-            content->rtp_header_extensions())) {
-      std::ostringstream desc;
-      desc << "Failed to set receive rtp header extensions for "
-           << MediaTypeToString(content->type()) << " content.";
-      SafeSetError(desc.str(), error_desc);
-      return false;
-    }
-  }
-  return true;
-}
-
-bool BaseChannel::SetSendRtpHeaderExtensions_w(
-    const MediaContentDescription* content,
-    MediaChannel* media_channel,
-    std::string* error_desc) {
-  if (content->rtp_header_extensions_set()) {
-    if (!media_channel->SetSendRtpHeaderExtensions(
-            content->rtp_header_extensions())) {
-      std::ostringstream desc;
-      desc << "Failed to set send rtp header extensions for "
-           << MediaTypeToString(content->type()) << " content.";
-      SafeSetError(desc.str(), error_desc);
-      return false;
-    } else {
-      MaybeCacheRtpAbsSendTimeHeaderExtension(content->rtp_header_extensions());
-    }
-  }
-  return true;
-}
-
 bool BaseChannel::SetSrtp_w(const std::vector<CryptoParams>& cryptos,
                             ContentAction action,
                             ContentSource src,
@@ -1210,49 +1191,6 @@ bool BaseChannel::UpdateRemoteStreams_w(
   return ret;
 }
 
-bool BaseChannel::SetBaseLocalContent_w(const MediaContentDescription* content,
-                                        ContentAction action,
-                                        std::string* error_desc) {
-  // Cache secure_required_ for belt and suspenders check on SendPacket
-  secure_required_ = content->crypto_required() != CT_NONE;
-  // Set local RTP header extensions.
-  bool ret = SetRecvRtpHeaderExtensions_w(content, media_channel(), error_desc);
-  // Set local SRTP parameters (what we will encrypt with).
-  ret &= SetSrtp_w(content->cryptos(), action, CS_LOCAL, error_desc);
-  // Set local RTCP mux parameters.
-  ret &= SetRtcpMux_w(content->rtcp_mux(), action, CS_LOCAL, error_desc);
-
-  // Call UpdateLocalStreams_w last to make sure as many settings as possible
-  // are already set when creating streams.
-  ret &= UpdateLocalStreams_w(content->streams(), action, error_desc);
-  set_local_content_direction(content->direction());
-  return ret;
-}
-
-bool BaseChannel::SetBaseRemoteContent_w(const MediaContentDescription* content,
-                                         ContentAction action,
-                                         std::string* error_desc) {
-  // Set remote RTP header extensions.
-  bool ret = SetSendRtpHeaderExtensions_w(content, media_channel(), error_desc);
-  // Set remote SRTP parameters (what the other side will encrypt with).
-  ret &= SetSrtp_w(content->cryptos(), action, CS_REMOTE, error_desc);
-  // Set remote RTCP mux parameters.
-  ret &= SetRtcpMux_w(content->rtcp_mux(), action, CS_REMOTE, error_desc);
-  if (!media_channel()->SetMaxSendBandwidth(content->bandwidth())) {
-    std::ostringstream desc;
-    desc << "Failed to set max send bandwidth for "
-         << MediaTypeToString(content->type()) << " content.";
-    SafeSetError(desc.str(), error_desc);
-    ret = false;
-  }
-
-  // Call UpdateRemoteStreams_w last to make sure as many settings as possible
-  // are already set when creating streams.
-  ret &= UpdateRemoteStreams_w(content->streams(), action, error_desc);
-  set_remote_content_direction(content->direction());
-  return ret;
-}
-
 void BaseChannel::MaybeCacheRtpAbsSendTimeHeaderExtension(
     const std::vector<RtpHeaderExtension>& extensions) {
   const RtpHeaderExtension* send_time_extension =
@@ -1502,28 +1440,54 @@ bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content,
     return false;
   }
 
-  bool ret = SetBaseLocalContent_w(content, action, error_desc);
-  // Set local audio codecs (what we want to receive).
-  // TODO(whyuan): Change action != CA_UPDATE to !audio->partial() when partial
-  // is set properly.
-  if (action != CA_UPDATE || audio->has_codecs()) {
-    if (!media_channel()->SetRecvCodecs(audio->codecs())) {
-      SafeSetError("Failed to set audio receive codecs.", error_desc);
-      ret = false;
-    }
+  // CA_UPDATE is only used by the Harmony library to update send
+  // streams.
+  // TODO(pthatcher): Move this logic up into the Harmony library and
+  // remove CA_UPDATE.
+  if (action == CA_UPDATE &&
+      !UpdateLocalStreams_w(content->streams(), action, error_desc)) {
+    SafeSetError("Failed to set local audio description send streams.",
+                 error_desc);
+    return false;
   }
 
-  // If everything worked, see if we can start receiving.
-  if (ret) {
-    std::vector<AudioCodec>::const_iterator it = audio->codecs().begin();
-    for (; it != audio->codecs().end(); ++it) {
-      bundle_filter()->AddPayloadType(it->id);
-    }
-    ChangeState();
+  if (!SetRtpTransportParameters_w(content, action, CS_LOCAL, error_desc)) {
+    return false;
+  }
+
+  AudioRecvParameters recv_params;
+  recv_params.codecs = audio->codecs();
+  // TODO(pthatcher): See if we really need
+  // rtp_header_extensions_set() and remove it if we don't.
+  if (audio->rtp_header_extensions_set()) {
+    recv_params.extensions = audio->rtp_header_extensions();
   } else {
-    LOG(LS_WARNING) << "Failed to set local voice description";
+    // Just maintain the last set of header extensions.
+    recv_params.extensions = last_recv_params_.extensions;
   }
-  return ret;
+
+  if (!media_channel()->SetRecvParameters(recv_params)) {
+    SafeSetError("Failed to set local video description recv parameters.",
+                 error_desc);
+    return false;
+  }
+
+  // TODO(pthatcher): Move local streams into AudioSendParameters, and
+  // only give it to the media channel once we have a remote
+  // description too (without a remote description, we won't be able
+  // to send them anyway).
+  if (!UpdateLocalStreams_w(audio->streams(), action, error_desc)) {
+    SafeSetError("Failed to set local audio description streams.", error_desc);
+    return false;
+  }
+
+  for (const AudioCodec& codec : audio->codecs()) {
+    bundle_filter()->AddPayloadType(codec.id);
+  }
+
+  set_local_content_direction(content->direction());
+  ChangeState();
+  return true;
 }
 
 bool VoiceChannel::SetRemoteContent_w(const MediaContentDescription* content,
@@ -1540,43 +1504,66 @@ bool VoiceChannel::SetRemoteContent_w(const MediaContentDescription* content,
     return false;
   }
 
-  bool ret = true;
-  // Set remote video codecs (what the other side wants to receive).
-  if (action != CA_UPDATE || audio->has_codecs()) {
-    if (!media_channel()->SetSendCodecs(audio->codecs())) {
-      SafeSetError("Failed to set audio send codecs.", error_desc);
-      ret = false;
+  // CA_UPDATE is only used by the Harmony library to update send
+  // codecs and receive streams.
+  // TODO(pthatcher): Move this logic up into the Harmony library and
+  // remove CA_UPDATE.
+  if (action == CA_UPDATE) {
+    if (audio->has_codecs() &&
+        !media_channel()->SetSendCodecs(audio->codecs())) {
+      SafeSetError("Failed to set remote audio description send codecs.",
+                   error_desc);
+      return false;
+    }
+    if (!UpdateRemoteStreams_w(content->streams(), action, error_desc)) {
+      SafeSetError("Failed to set remote audio description receive streams.",
+                   error_desc);
+      return false;
     }
+    return true;
   }
 
-  ret &= SetBaseRemoteContent_w(content, action, error_desc);
-
-  if (action != CA_UPDATE) {
-    // Tweak our audio processing settings, if needed.
-    AudioOptions audio_options;
-    if (!media_channel()->GetOptions(&audio_options)) {
-      LOG(LS_WARNING) << "Can not set audio options from on remote content.";
-    } else {
-      if (audio->conference_mode()) {
-        audio_options.conference_mode.Set(true);
-      }
-      if (audio->agc_minus_10db()) {
-        audio_options.adjust_agc_delta.Set(kAgcMinus10db);
-      }
-      if (!media_channel()->SetOptions(audio_options)) {
-        // Log an error on failure, but don't abort the call.
-        LOG(LS_ERROR) << "Failed to set voice channel options";
-      }
-    }
+  if (!SetRtpTransportParameters_w(content, action, CS_REMOTE, error_desc)) {
+    return false;
   }
 
-  // If everything worked, see if we can start sending.
-  if (ret) {
-    ChangeState();
+  AudioSendParameters send_params;
+  send_params.codecs = audio->codecs();
+  // TODO(pthatcher): See if we really need
+  // rtp_header_extensions_set() and remove it if we don't.
+  if (audio->rtp_header_extensions_set()) {
+    send_params.extensions = audio->rtp_header_extensions();
   } else {
-    LOG(LS_WARNING) << "Failed to set remote voice description";
+    // Just maintain the last set of header extensions.
+    send_params.extensions = last_send_params_.extensions;
   }
-  return ret;
+  send_params.max_bandwidth_bps = audio->bandwidth();
+  send_params.options = last_send_params_.options;
+  if (audio->conference_mode()) {
+    send_params.options.conference_mode.Set(true);
+  }
+  if (audio->agc_minus_10db()) {
+    send_params.options.adjust_agc_delta.Set(kAgcMinus10db);
+  }
+  if (!media_channel()->SetSendParameters(send_params)) {
+    SafeSetError("Failed to set remote audio description send parameters.",
+                 error_desc);
+    return false;
+  }
+  last_send_params_ = send_params;
+
+  // TODO(pthatcher): Move remote streams into AudioRecvParameters,
+  // and only give it to the media channel once we have a local
+  // description too (without a local description, we won't be able to
+  // recv them anyway).
+  if (!UpdateRemoteStreams_w(audio->streams(), action, error_desc)) {
+    LOG(LS_WARNING) << "Failed to set remote audio description receive streams";
+    return false;
+  }
+
+  set_remote_content_direction(content->direction());
+  ChangeState();
+  return true;
 }
 
 bool VoiceChannel::SetRingbackTone_w(const void* buf, int len) {
@@ -1846,37 +1833,54 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content,
     return false;
   }
 
-  bool ret = SetBaseLocalContent_w(content, action, error_desc);
-  // Set local video codecs (what we want to receive).
-  if (action != CA_UPDATE || video->has_codecs()) {
-    if (!media_channel()->SetRecvCodecs(video->codecs())) {
-      SafeSetError("Failed to set video receive codecs.", error_desc);
-      ret = false;
-    }
+  // CA_UPDATE is only used by the Harmony library to update send streams.
+  // TODO(pthatcher): Move this logic up into the Harmony library and
+  // remove CA_UPDATE.
+  if (action == CA_UPDATE &&
+      !UpdateLocalStreams_w(content->streams(), action, error_desc)) {
+    SafeSetError("Failed to set local audio description send streams.",
+                 error_desc);
+    return false;
   }
 
-  if (action != CA_UPDATE) {
-    VideoOptions video_options;
-    media_channel()->GetOptions(&video_options);
-    video_options.buffered_mode_latency.Set(video->buffered_mode_latency());
-
-    if (!media_channel()->SetOptions(video_options)) {
-      // Log an error on failure, but don't abort the call.
-      LOG(LS_ERROR) << "Failed to set video channel options";
-    }
+  if (!SetRtpTransportParameters_w(content, action, CS_LOCAL, error_desc)) {
+    return false;
   }
 
-  // If everything worked, see if we can start receiving.
-  if (ret) {
-    std::vector<VideoCodec>::const_iterator it = video->codecs().begin();
-    for (; it != video->codecs().end(); ++it) {
-      bundle_filter()->AddPayloadType(it->id);
-    }
-    ChangeState();
+  VideoRecvParameters recv_params;
+  recv_params.codecs = video->codecs();
+  // TODO(pthatcher): See if we really need
+  // rtp_header_extensions_set() and remove it if we don't.
+  if (video->rtp_header_extensions_set()) {
+    recv_params.extensions = video->rtp_header_extensions();
   } else {
-    LOG(LS_WARNING) << "Failed to set local video description";
+    // Just maintain the last set of header extensions.
+    recv_params.extensions = last_recv_params_.extensions;
   }
-  return ret;
+
+  if (!media_channel()->SetRecvParameters(recv_params)) {
+    SafeSetError("Failed to set local video description recv parameters.",
+                 error_desc);
+    return false;
+  }
+  last_recv_params_ = recv_params;
+
+  // TODO(pthatcher): Move local streams into VideoSendParameters, and
+  // only give it to the media channel once we have a remote
+  // description too (without a remote description, we won't be able
+  // to send them anyway).
+  if (!UpdateLocalStreams_w(video->streams(), action, error_desc)) {
+    SafeSetError("Failed to set local video description streams.", error_desc);
+    return false;
+  }
+
+  for (const VideoCodec& codec : video->codecs()) {
+    bundle_filter()->AddPayloadType(codec.id);
+  }
+
+  set_local_content_direction(content->direction());
+  ChangeState();
+  return true;
 }
 
 bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content,
@@ -1893,39 +1897,67 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content,
     return false;
   }
 
-  bool ret = true;
-  // Set remote video codecs (what the other side wants to receive).
-  if (action != CA_UPDATE || video->has_codecs()) {
-    if (!media_channel()->SetSendCodecs(video->codecs())) {
-      SafeSetError("Failed to set video send codecs.", error_desc);
-      ret = false;
+  // CA_UPDATE is only used by the Harmony library to update send
+  // codecs and receive streams.
+  // TODO(pthatcher): Move this logic up into the Harmony library and
+  // remove CA_UPDATE.
+  if (action == CA_UPDATE) {
+    if (video->has_codecs() &&
+        !media_channel()->SetSendCodecs(video->codecs())) {
+      SafeSetError("Failed to set remote video description send codecs.",
+                   error_desc);
+      return false;
     }
+    if (!UpdateRemoteStreams_w(content->streams(), action, error_desc)) {
+      SafeSetError("Failed to set remote video description receive streams.",
+                   error_desc);
+      return false;
+    }
+    return true;
   }
 
-  ret &= SetBaseRemoteContent_w(content, action, error_desc);
+  if (!SetRtpTransportParameters_w(content, action, CS_REMOTE, error_desc)) {
+    return false;
+  }
 
-  if (action != CA_UPDATE) {
-    // Tweak our video processing settings, if needed.
-    VideoOptions video_options;
-    media_channel()->GetOptions(&video_options);
-    if (video->conference_mode()) {
-      video_options.conference_mode.Set(true);
-    }
-    video_options.buffered_mode_latency.Set(video->buffered_mode_latency());
+  VideoSendParameters send_params;
+  send_params.codecs = video->codecs();
+  // TODO(pthatcher): See if we really need
+  // rtp_header_extensions_set() and remove it if we don't.
+  if (video->rtp_header_extensions_set()) {
+    send_params.extensions = video->rtp_header_extensions();
+  } else {
+    // Just maintain the last set of header extensions.
+    send_params.extensions = last_send_params_.extensions;
+  }
+  send_params.max_bandwidth_bps = video->bandwidth();
+  send_params.options = last_send_params_.options;
+  if (video->conference_mode()) {
+    send_params.options.conference_mode.Set(true);
+  }
+  if (!media_channel()->SetSendParameters(send_params)) {
+    SafeSetError("Failed to set remote video description send parameters.",
+                 error_desc);
+    return false;
+  }
+  last_send_params_ = send_params;
 
-    if (!media_channel()->SetOptions(video_options)) {
-      // Log an error on failure, but don't abort the call.
-      LOG(LS_ERROR) << "Failed to set video channel options";
-    }
+  if (video->rtp_header_extensions_set()) {
+    MaybeCacheRtpAbsSendTimeHeaderExtension(video->rtp_header_extensions());
   }
 
-  // If everything worked, see if we can start sending.
-  if (ret) {
-    ChangeState();
-  } else {
-    LOG(LS_WARNING) << "Failed to set remote video description";
+  // TODO(pthatcher): Move remote streams into VideoRecvParameters,
+  // and only give it to the media channel once we have a local
+  // description too (without a local description, we won't be able to
+  // recv them anyway).
+  if (!UpdateRemoteStreams_w(video->streams(), action, error_desc)) {
+    LOG(LS_WARNING) << "Failed to set remote video description receive streams";
+    return false;
   }
-  return ret;
+
+  set_remote_content_direction(content->direction());
+  ChangeState();
+  return true;
 }
 
 bool VideoChannel::ApplyViewRequest_w(const ViewRequest& request) {
@@ -2230,44 +2262,53 @@ bool DataChannel::SetLocalContent_w(const MediaContentDescription* content,
     return false;
   }
 
-  bool ret = false;
+  // CA_UPDATE is only used by the Harmony library to update send streams.
+  // TODO(pthatcher): Move this logic up into the Harmony library and
+  // remove CA_UPDATE.
+  if (action == CA_UPDATE &&
+      !UpdateLocalStreams_w(content->streams(), action, error_desc)) {
+    SafeSetError("Failed to set local audio description send streams.",
+                 error_desc);
+    return false;
+  }
+
   if (!SetDataChannelTypeFromContent(data, error_desc)) {
     return false;
   }
 
-  if (data_channel_type_ == DCT_SCTP) {
-    // SCTP data channels don't need the rest of the stuff.
-    ret = UpdateLocalStreams_w(data->streams(), action, error_desc);
-    if (ret) {
-      set_local_content_direction(content->direction());
-      // As in SetRemoteContent_w, make sure we set the local SCTP port
-      // number as specified in our DataContentDescription.
-      if (!media_channel()->SetRecvCodecs(data->codecs())) {
-        SafeSetError("Failed to set data receive codecs.", error_desc);
-        ret = false;
-      }
+  if (data_channel_type_ == DCT_RTP) {
+    if (!SetRtpTransportParameters_w(content, action, CS_LOCAL, error_desc)) {
+      return false;
     }
-  } else {
-    ret = SetBaseLocalContent_w(content, action, error_desc);
-    if (action != CA_UPDATE || data->has_codecs()) {
-      if (!media_channel()->SetRecvCodecs(data->codecs())) {
-        SafeSetError("Failed to set data receive codecs.", error_desc);
-        ret = false;
-      }
+
+    for (const DataCodec& codec : data->codecs()) {
+      bundle_filter()->AddPayloadType(codec.id);
     }
   }
 
-  // If everything worked, see if we can start receiving.
-  if (ret) {
-    std::vector<DataCodec>::const_iterator it = data->codecs().begin();
-    for (; it != data->codecs().end(); ++it) {
-      bundle_filter()->AddPayloadType(it->id);
-    }
-    ChangeState();
-  } else {
-    LOG(LS_WARNING) << "Failed to set local data description";
+  // FYI: We send the SCTP port number (not to be confused with the
+  // underlying UDP port number) as a codec parameter.  So even SCTP
+  // data channels need codecs.
+  DataRecvParameters recv_params;
+  recv_params.codecs = data->codecs();
+  if (!media_channel()->SetRecvParameters(recv_params)) {
+    SafeSetError("Failed to set remote data description recv parameters.",
+                 error_desc);
+    return false;
   }
-  return ret;
+
+  // TODO(pthatcher): Move local streams into DataSendParameters, and
+  // only give it to the media channel once we have a remote
+  // description too (without a remote description, we won't be able
+  // to send them anyway).
+  if (!UpdateLocalStreams_w(data->streams(), action, error_desc)) {
+    SafeSetError("Failed to set local data description streams.", error_desc);
+    return false;
+  }
+
+  set_local_content_direction(content->direction());
+  ChangeState();
+  return true;
 }
 
 bool DataChannel::SetRemoteContent_w(const MediaContentDescription* content,
@@ -2283,62 +2324,62 @@ bool DataChannel::SetRemoteContent_w(const MediaContentDescription* content,
     return false;
   }
 
-  bool ret = true;
-  if (!SetDataChannelTypeFromContent(data, error_desc)) {
-    return false;
-  }
-
-  if (data_channel_type_ == DCT_SCTP) {
-    LOG(LS_INFO) << "Setting SCTP remote data description";
-    // SCTP data channels don't need the rest of the stuff.
-    ret = UpdateRemoteStreams_w(content->streams(), action, error_desc);
-    if (ret) {
-      set_remote_content_direction(content->direction());
-      // We send the SCTP port number (not to be confused with the underlying
-      // UDP port number) as a codec parameter.  Make sure it gets there.
-      if (!media_channel()->SetSendCodecs(data->codecs())) {
-        SafeSetError("Failed to set data send codecs.", error_desc);
-        ret = false;
-      }
+  // CA_UPDATE is only used by the Harmony library to update send
+  // codecs and receive streams.
+  // TODO(pthatcher): Move this logic up into the Harmony library and
+  // remove CA_UPDATE.
+  if (action == CA_UPDATE) {
+    if (data->has_codecs() && !media_channel()->SetSendCodecs(data->codecs())) {
+      SafeSetError("Failed to set remote data description send codecs.",
+                   error_desc);
+      return false;
     }
-  } else {
-    // If the remote data doesn't have codecs and isn't an update, it
-    // must be empty, so ignore it.
-    if (action != CA_UPDATE && !data->has_codecs()) {
-      return true;
+    if (!UpdateRemoteStreams_w(content->streams(), action, error_desc)) {
+      SafeSetError("Failed to set remote data description receive streams.",
+                   error_desc);
+      return false;
     }
-    LOG(LS_INFO) << "Setting remote data description";
+    return true;
+  }
 
-    // Set remote video codecs (what the other side wants to receive).
-    if (action != CA_UPDATE || data->has_codecs()) {
-      if (!media_channel()->SetSendCodecs(data->codecs())) {
-        SafeSetError("Failed to set data send codecs.", error_desc);
-        ret = false;
-      }
-    }
+  // If the remote data doesn't have codecs and isn't an update, it
+  // must be empty, so ignore it.
+  if (!data->has_codecs()) {
+    return true;
+  }
 
-    if (ret) {
-      ret &= SetBaseRemoteContent_w(content, action, error_desc);
-    }
+  if (!SetDataChannelTypeFromContent(data, error_desc)) {
+    return false;
+  }
 
-    if (action != CA_UPDATE) {
-      int bandwidth_bps = data->bandwidth();
-      if (!media_channel()->SetMaxSendBandwidth(bandwidth_bps)) {
-        std::ostringstream desc;
-        desc << "Failed to set max send bandwidth for data content.";
-        SafeSetError(desc.str(), error_desc);
-        ret = false;
-      }
-    }
+  LOG(LS_INFO) << "Setting remote data description";
+  if (data_channel_type_ == DCT_RTP &&
+      !SetRtpTransportParameters_w(content, action, CS_REMOTE, error_desc)) {
+    return false;
   }
 
-  // If everything worked, see if we can start sending.
-  if (ret) {
-    ChangeState();
-  } else {
-    LOG(LS_WARNING) << "Failed to set remote data description";
+  DataSendParameters send_params;
+  send_params.codecs = data->codecs();
+  send_params.max_bandwidth_bps = data->bandwidth();
+  if (!media_channel()->SetSendParameters(send_params)) {
+    SafeSetError("Failed to set remote data description send parameters.",
+                 error_desc);
+    return false;
   }
-  return ret;
+
+  // TODO(pthatcher): Move remote streams into DataRecvParameters,
+  // and only give it to the media channel once we have a local
+  // description too (without a local description, we won't be able to
+  // recv them anyway).
+  if (!UpdateRemoteStreams_w(data->streams(), action, error_desc)) {
+    SafeSetError("Failed to set remote data description streams.",
+                 error_desc);
+    return false;
+  }
+
+  set_remote_content_direction(content->direction());
+  ChangeState();
+  return true;
 }
 
 void DataChannel::ChangeState() {