Adding "adapter" ORTC objects on top of ChannelManager/BaseChannel/etc.

webrtc/ortc/rtptransportcontrollershim.cc

+/*
+ *  Copyright 2017 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.
+ */
+
+#include "webrtc/ortc/rtptransportcontrollershim.h"
+
+#include <algorithm>  // For "remove", "find".
+#include <set>
+#include <utility>  // For std::move.
+
+#include "webrtc/api/proxy.h"
+#include "webrtc/base/checks.h"
+#include "webrtc/media/base/mediaconstants.h"
+#include "webrtc/ortc/rtptransportshim.h"
+
+namespace {
+
+static const int kVideoClockrate = 90000;

[pthatcher1, 2017/02/10 22:36:53]

It's already in mediaconstants.h :).

[Taylor Brandstetter, 2017/02/14 06:55:05]

I was slightly confused because it was already defined in multiple places. I'll
fix that.

+
+// Returns false on invalid input. Certain message types are only valid for
+// certain feedback types.
+webrtc::RTCError ValidateAndConvertRtcpFeedback(
+    const webrtc::RtcpFeedback& feedback,
+    cricket::Codec* codec) {

[pthatcher1, 2017/02/10 22:36:53]

Seem like it should be called AddCricketCodecFeedbackParam or something like
that.

[Taylor Brandstetter, 2017/02/14 06:55:05]

That doesn't communicate that these parameters are validated and an error
returned. Same goes for other comments.

+  switch (feedback.type) {
+    case webrtc::RtcpFeedbackType::ACK:
+      if (feedback.message_type) {
+        return CreateAndLogError(
+            webrtc::RTCErrorType::INVALID_PARAMETER,
+            "Didn't expect message type in ACK RtcpFeedback.");
+      }
+      codec->AddFeedbackParam(cricket::FeedbackParam("ack"));
+      return webrtc::RTCError();
+    case webrtc::RtcpFeedbackType::CCM:
+      if (!feedback.message_type) {
+        return CreateAndLogError(webrtc::RTCErrorType::INVALID_PARAMETER,
+                                 "Missing message type in CCM RtcpFeedback.");
+      } else if (*feedback.message_type !=
+                 webrtc::RtcpFeedbackMessageType::FIR) {
+        return CreateAndLogError(webrtc::RTCErrorType::INVALID_PARAMETER,
+                                 "Invalid message type in CCM RtcpFeedback.");
+      }
+      codec->AddFeedbackParam(cricket::FeedbackParam("ccm", "fir"));

[pthatcher1, 2017/02/10 22:36:53]

You can use the strings in mediaconstants.h.

[Taylor Brandstetter, 2017/02/14 06:55:05]

Done.

+      return webrtc::RTCError();
+    case webrtc::RtcpFeedbackType::NACK:
+      if (!feedback.message_type) {
+        return CreateAndLogError(webrtc::RTCErrorType::INVALID_PARAMETER,
+                                 "Missing message type in NACK RtcpFeedback.");
+      }
+      switch (*feedback.message_type) {
+        case webrtc::RtcpFeedbackMessageType::GENERIC_NACK:
+          codec->AddFeedbackParam(cricket::FeedbackParam("nack"));
+          return webrtc::RTCError();
+        case webrtc::RtcpFeedbackMessageType::PLI:
+          codec->AddFeedbackParam(cricket::FeedbackParam("nack", "pli"));
+          return webrtc::RTCError();
+        default:
+          return CreateAndLogError(
+              webrtc::RTCErrorType::INVALID_PARAMETER,
+              "Invalid message type in NACK RtcpFeedback.");
+      }
+    case webrtc::RtcpFeedbackType::REMB:
+      if (feedback.message_type) {
+        return CreateAndLogError(
+            webrtc::RTCErrorType::INVALID_PARAMETER,
+            "Didn't expect message type in REMB RtcpFeedback.");
+      }
+      codec->AddFeedbackParam(cricket::FeedbackParam("goog-remb"));
+      return webrtc::RTCError();
+    case webrtc::RtcpFeedbackType::TRANSPORT_CC:
+      if (feedback.message_type) {
+        return CreateAndLogError(
+            webrtc::RTCErrorType::INVALID_PARAMETER,
+            "Didn't expect message type in transport-cc RtcpFeedback.");
+      }
+      codec->AddFeedbackParam(cricket::FeedbackParam("transport-cc"));
+      return webrtc::RTCError();
+  }
+}
+
+template <typename C>
+webrtc::RTCError CodecSpecificConversion(
+    const webrtc::RtpCodecParameters& codec,
+    C* cricket_codec) {}
+
+template <>
+webrtc::RTCError CodecSpecificConversion<cricket::AudioCodec>(
+    const webrtc::RtpCodecParameters& codec,
+    cricket::AudioCodec* cricket_codec) {

[pthatcher1, 2017/02/10 22:36:53]

Seems like this could be called ToCricketAudioCodec or something similar.

+  if (codec.kind != cricket::MEDIA_TYPE_AUDIO) {
+    return CreateAndLogError(
+        webrtc::RTCErrorType::INVALID_PARAMETER,
+        "Can't use video codec with audio sender or receiver.");
+  }
+  if (!codec.num_channels) {
+    // A default value for number of channels should have been filled already.
+    RTC_NOTREACHED();
+    return CreateAndLogError(webrtc::RTCErrorType::INVALID_PARAMETER,
+                             "Missing number of channels for audio codec.");
+  }
+  if (*codec.num_channels <= 0) {
+    return CreateAndLogError(webrtc::RTCErrorType::INVALID_RANGE,
+                             "Number of channels must be positive.");
+  }
+  cricket_codec->channels = *codec.num_channels;
+  if (!codec.clock_rate) {
+    return CreateAndLogError(webrtc::RTCErrorType::INVALID_PARAMETER,
+                             "Missing codec clock rate.");
+  }
+  if (*codec.clock_rate <= 0) {
+    return CreateAndLogError(webrtc::RTCErrorType::INVALID_RANGE,
+                             "Clock rate must be positive.");
+  }
+  cricket_codec->clockrate = *codec.clock_rate;
+  return webrtc::RTCError();
+}
+
+// Video codec doesn't use num_channels or clock_rate, but they should at least
+// be validated.
+template <>
+webrtc::RTCError CodecSpecificConversion<cricket::VideoCodec>(
+    const webrtc::RtpCodecParameters& codec,

[pthatcher1, 2017/02/10 22:36:53]

And this could be called ToCricketVideoCodec.

+    cricket::VideoCodec*) {
+  if (codec.kind != cricket::MEDIA_TYPE_VIDEO) {
+    return CreateAndLogError(
+        webrtc::RTCErrorType::INVALID_PARAMETER,
+        "Can't use video codec with video sender or receiver.");
+  }
+  if (codec.num_channels) {
+    return CreateAndLogError(webrtc::RTCErrorType::INVALID_PARAMETER,
+                             "Video codec shouldn't have num_channels.");
+  }
+  if (!codec.clock_rate) {
+    // A default value should have been filled already.
+    RTC_NOTREACHED();
+    return CreateAndLogError(webrtc::RTCErrorType::INVALID_PARAMETER,
+                             "Missing codec clock rate.");
+  }
+  if (*codec.clock_rate != kVideoClockrate) {
+    return CreateAndLogError(webrtc::RTCErrorType::INVALID_PARAMETER,
+                             "Video clock rate must be 90000.");
+  }
+  return webrtc::RTCError();
+}
+
+template <typename C>
+webrtc::RTCError ValidateAndConvertCodecs(
+    const std::vector<webrtc::RtpCodecParameters>& codecs,
+    std::vector<C>* cricket_codecs) {

[pthatcher1, 2017/02/10 22:36:53]

Similarly, this could be called ToCricketCodecs.

+  std::ostringstream err_writer;
+  std::set<int> seen_payload_types;
+  for (const webrtc::RtpCodecParameters& codec : codecs) {
+    C cricket_codec;
+    // Start with audio/video specific conversion.
+    webrtc::RTCError err = CodecSpecificConversion(codec, &cricket_codec);
+    if (!err.ok()) {
+      return err;
+    }
+    cricket_codec.name = codec.name;
+    if (codec.payload_type < 0 || codec.payload_type > 127) {

[pthatcher1, 2017/02/10 22:36:53]

127 could use a name constant somewhere.  Perhaps just at the top of this file.

[Taylor Brandstetter, 2017/02/14 06:55:05]

Done.

+      err_writer << "Invalid payload type: " << codec.payload_type;
+      return CreateAndLogError(webrtc::RTCErrorType::INVALID_RANGE,
+                               err_writer.str());
+    }
+    if (!seen_payload_types.insert(codec.payload_type).second) {
+      err_writer << "Duplicate payload type: " << codec.payload_type;
+      return CreateAndLogError(webrtc::RTCErrorType::INVALID_PARAMETER,
+                               err_writer.str());
+    }
+    cricket_codec.id = codec.payload_type;
+    for (const webrtc::RtcpFeedback& feedback : codec.rtcp_feedback) {
+      webrtc::RTCError err =
+          ValidateAndConvertRtcpFeedback(feedback, &cricket_codec);
+      if (!err.ok()) {
+        return err;
+      }
+    }
+    cricket_codec.params.insert(codec.parameters.begin(),
+                                codec.parameters.end());
+    cricket_codecs->push_back(std::move(cricket_codec));
+  }
+  return webrtc::RTCError();
+}
+
+webrtc::RTCError ValidateAndConvertHeaderExtensions(
+    const std::vector<webrtc::RtpHeaderExtensionParameters>& extensions,
+    cricket::RtpHeaderExtensions* cricket_extensions) {

[pthatcher1, 2017/02/10 22:36:53]

Similarly, this could be called ToCricketRtpHeaderExtensions.

+  std::ostringstream err_writer;
+  std::set<int> seen_header_extension_ids;
+  for (const webrtc::RtpHeaderExtensionParameters& extension : extensions) {
+    if (extension.id < 1 || extension.id > 14) {
+      err_writer << "Invalid header extension id: " << extension.id;
+      return CreateAndLogError(webrtc::RTCErrorType::INVALID_RANGE,
+                               err_writer.str());
+    }
+    if (!seen_header_extension_ids.insert(extension.id).second) {
+      err_writer << "Duplicate header extension id: " << extension.id;
+      return CreateAndLogError(webrtc::RTCErrorType::INVALID_PARAMETER,
+                               err_writer.str());
+    }
+    cricket_extensions->emplace_back(extension.uri, extension.id);
+  }
+  return webrtc::RTCError();
+}
+
+// Missing SSRC is treated differently for receiver encodings; this means
+// SSRCs are unsignaled.
+webrtc::RTCError ValidateAndConvertReceiverEncodings(
+    const std::vector<webrtc::RtpEncodingParameters> encodings,
+    cricket::StreamParamsVec* cricket_streams,
+    bool* receiving) {
+  if (encodings.size() > 1u) {
+    return CreateAndLogError(webrtc::RTCErrorType::UNSUPPORTED_PARAMETER,
+                             "ORTC API implementation doesn't currently "
+                             "support simulcast or layered encodings.");
+  }
+  if (encodings.size() == 1u) {

[pthatcher1, 2017/02/10 22:36:53]

Doing the encodings.size() == 0 first and return an RTCError() would be nice to
move the main block against the left side.

[Taylor Brandstetter, 2017/02/14 06:55:05]

Done.

+    const webrtc::RtpEncodingParameters& encoding = encodings[0];
+    *receiving = encoding.active;
+    if (encoding.ssrc) {
+      cricket::StreamParams stream_params;
+      stream_params.add_ssrc(*encoding.ssrc);
+      if (encoding.rtx && encoding.rtx->ssrc) {
+        stream_params.AddFidSsrc(*encoding.ssrc, *encoding.rtx->ssrc);
+      }
+      cricket_streams->push_back(std::move(stream_params));
+    }

[pthatcher1, 2017/02/10 22:36:53]

Similarly for !encoding.ssrc: return RTCError early, pushing the main code to
the right.

[Taylor Brandstetter, 2017/02/14 06:55:05]

Done

+  } else {
+    *receiving = false;
+  }
+  return webrtc::RTCError();
+}
+
+// Codecs that will never be used, but will at least not be rejected by
+// BaseChannel/MediaChannel classes.
+void AddDefaultAudioCodec(cricket::AudioContentDescription* description) {

[pthatcher1, 2017/02/10 22:36:53]

Should this be AddDummyAudioCodec and AddDummyVideoCodec?

[Taylor Brandstetter, 2017/02/14 06:55:05]

Done.

+  description->AddCodec(
+      cricket::AudioCodec(0, cricket::kPcmuCodecName, 8000, 0, 1));
+}
+
+void AddDefaultVideoCodec(cricket::VideoContentDescription* description) {
+  description->AddCodec(cricket::VideoCodec(96, cricket::kVp8CodecName));
+}
+
+}  // namespace
+
+namespace webrtc {
+
+BEGIN_OWNED_PROXY_MAP(RtpTransportController)
+PROXY_SIGNALING_THREAD_DESTRUCTOR()
+PROXY_CONSTMETHOD0(std::vector<RtpTransportInterface*>, GetTransports)
+protected:
+RtpTransportControllerShim* GetInternal() override {
+  return internal();
+}
+END_PROXY_MAP()
+
+// static
+std::unique_ptr<RtpTransportControllerInterface>
+RtpTransportControllerShim::CreateProxied(
+    const cricket::MediaConfig& config,
+    cricket::ChannelManager* channel_manager,
+    webrtc::RtcEventLog* event_log,
+    rtc::Thread* signaling_thread,
+    rtc::Thread* worker_thread) {
+  return RtpTransportControllerProxyWithInternal<
+      RtpTransportControllerShim>::Create(signaling_thread, worker_thread,
+                                          new RtpTransportControllerShim(
+                                              config, channel_manager,
+                                              event_log, signaling_thread,
+                                              worker_thread));
+}
+
+RtpTransportControllerShim::~RtpTransportControllerShim() {
+  RTC_DCHECK_RUN_ON(signaling_thread_);
+  if (!transport_proxies_.empty()) {
+    LOG(LS_ERROR)
+        << "Destroying RtpTransportControllerShim while RtpTransports "
+           "are still using it; this is unsafe.";
+  }
+  if (voice_channel_) {
+    // This would mean audio RTP senders/receivers are still using us.

[pthatcher1, 2017/02/10 22:36:53]

It could be more clear that you mean that the RtpSenders/Recievers have not been
deleted, and thus not called Detach yet.  

But what does that mean for them?  What happens if code tries to use them still?

[Taylor Brandstetter, 2017/02/14 06:55:05]

Probably a crash. This is what we get if things aren't ref-counted and can be
destroyed at will by the application.

+    DestroyVoiceChannel();
+  }
+  if (voice_channel_) {
+    // This would mean video RTP senders/receivers are still using us.
+    DestroyVideoChannel();
+  }
+}
+
+std::vector<RtpTransportInterface*> RtpTransportControllerShim::GetTransports()
+    const {
+  RTC_DCHECK_RUN_ON(signaling_thread_);
+  return transport_proxies_;
+}
+
+void RtpTransportControllerShim::AddTransport(
+    RtpTransportInterface* transport_proxy) {
+  RTC_DCHECK_RUN_ON(signaling_thread_);
+  transport_proxies_.push_back(transport_proxy);
+}
+
+void RtpTransportControllerShim::RemoveTransport(
+    RtpTransportInterface* inner_transport) {
+  RTC_DCHECK_RUN_ON(signaling_thread_);
+  auto it = std::find_if(transport_proxies_.begin(), transport_proxies_.end(),
+                         [inner_transport](RtpTransportInterface* proxy) {
+                           return proxy->GetInternal() == inner_transport;
+                         });
+  if (it == transport_proxies_.end()) {
+    RTC_NOTREACHED();
+    return;
+  }
+  transport_proxies_.erase(it);

[pthatcher1, 2017/02/10 22:36:53]

It seems like it would be easier to have Add and Remove take the same value. 
Why did you make RemoveTransport different?

[Taylor Brandstetter, 2017/02/14 06:55:05]

Because AddTransport needs to take a proxy, because GetTransport needs to return
a proxy.

And RemoveTransport is called by the transport implementation underneath the
proxy, which doesn't have a pointer to the proxy itself.

+}
+
+RTCError RtpTransportControllerShim::SetRtcpParameters(
+    const RtcpParameters& parameters,
+    RtpTransportInterface* inner_transport) {
+  if (inner_transport == inner_audio_transport_) {
+    CopyRtcpParametersToDescriptions(parameters, &local_audio_description_,
+                                     &remote_audio_description_);
+    if (!voice_channel_->SetLocalContent(&local_audio_description_,
+                                         cricket::CA_OFFER, nullptr)) {
+      return CreateAndLogError(RTCErrorType::INTERNAL_ERROR,
+                               "Failed to apply new RTCP parameters.");
+    }
+    if (!voice_channel_->SetRemoteContent(&remote_audio_description_,
+                                          cricket::CA_ANSWER, nullptr)) {
+      return CreateAndLogError(RTCErrorType::INTERNAL_ERROR,
+                               "Failed to apply new RTCP parameters.");
+    }
+  } else if (inner_transport == inner_video_transport_) {
+    CopyRtcpParametersToDescriptions(parameters, &local_video_description_,
+                                     &remote_video_description_);
+    if (!video_channel_->SetLocalContent(&local_video_description_,
+                                         cricket::CA_OFFER, nullptr)) {
+      return CreateAndLogError(RTCErrorType::INTERNAL_ERROR,
+                               "Failed to apply new RTCP parameters.");
+    }
+    if (!video_channel_->SetRemoteContent(&remote_video_description_,
+                                          cricket::CA_ANSWER, nullptr)) {
+      return CreateAndLogError(RTCErrorType::INTERNAL_ERROR,
+                               "Failed to apply new RTCP parameters.");
+    }
+  }
+  return RTCError();
+}
+
+RTCError RtpTransportControllerShim::AttachAudioSender(
+    RtpTransportInterface* inner_transport) {
+  if (have_audio_sender_) {
+    return CreateAndLogError(RTCErrorType::UNSUPPORTED_OPERATION,
+                             "Using two RtpSenders with the same "
+                             "RtpTransportControllerShim is not currently "
+                             "supported.");
+  }
+  if (inner_audio_transport_ && inner_audio_transport_ != inner_transport) {
+    return CreateAndLogError(RTCErrorType::UNSUPPORTED_PARAMETER,
+                             "Using different transports for the audio "
+                             "RtpSender and RtpReceiver is not currently "
+                             "supported.");
+  }
+  // If setting new transport, extract its RTCP parameters and create voice
+  // channel.
+  if (!inner_audio_transport_) {
+    CopyRtcpParametersToDescriptions(inner_transport->GetRtcpParameters(),
+                                     &local_audio_description_,
+                                     &remote_audio_description_);
+    inner_audio_transport_ = inner_transport;
+    CreateVoiceChannel();
+  }
+  have_audio_sender_ = true;
+  return RTCError();
+}
+
+RTCError RtpTransportControllerShim::AttachVideoSender(
+    RtpTransportInterface* inner_transport) {
+  if (have_video_sender_) {
+    return CreateAndLogError(RTCErrorType::UNSUPPORTED_OPERATION,
+                             "Using two RtpSenders with the same "
+                             "RtpTransportControllerShim is not currently "
+                             "supported.");
+  }
+  if (inner_video_transport_ && inner_video_transport_ != inner_transport) {
+    return CreateAndLogError(RTCErrorType::UNSUPPORTED_PARAMETER,
+                             "Using different transports for the video "
+                             "RtpSender and RtpReceiver is not currently "
+                             "supported.");
+  }
+  // If setting new transport, extract its RTCP parameters and create video
+  // channel.
+  if (!inner_video_transport_) {
+    CopyRtcpParametersToDescriptions(inner_transport->GetRtcpParameters(),
+                                     &local_video_description_,
+                                     &remote_video_description_);
+    inner_video_transport_ = inner_transport;
+    CreateVideoChannel();
+  }
+  have_video_sender_ = true;
+  return RTCError();
+}
+
+RTCError RtpTransportControllerShim::AttachAudioReceiver(
+    RtpTransportInterface* inner_transport) {
+  if (have_audio_receiver_) {
+    return CreateAndLogError(RTCErrorType::UNSUPPORTED_OPERATION,
+                             "Using two RtpReceivers with the same "
+                             "RtpTransportControllerShim is not currently "
+                             "supported.");
+  }
+  if (inner_audio_transport_ && inner_audio_transport_ != inner_transport) {
+    return CreateAndLogError(RTCErrorType::UNSUPPORTED_PARAMETER,
+                             "Using different transports for the audio "
+                             "RtpReceiver and RtpReceiver is not currently "
+                             "supported.");
+  }
+  // If setting new transport, extract its RTCP parameters and create voice
+  // channel.
+  if (!inner_audio_transport_) {
+    CopyRtcpParametersToDescriptions(inner_transport->GetRtcpParameters(),
+                                     &local_audio_description_,
+                                     &remote_audio_description_);
+    inner_audio_transport_ = inner_transport;
+    CreateVoiceChannel();
+  }
+  have_audio_receiver_ = true;
+  return RTCError();
+}
+
+RTCError RtpTransportControllerShim::AttachVideoReceiver(
+    RtpTransportInterface* inner_transport) {
+  if (have_video_receiver_) {
+    return CreateAndLogError(RTCErrorType::UNSUPPORTED_OPERATION,
+                             "Using two RtpReceivers with the same "
+                             "RtpTransportControllerShim is not currently "
+                             "supported.");
+  }
+  if (inner_video_transport_ && inner_video_transport_ != inner_transport) {
+    return CreateAndLogError(RTCErrorType::UNSUPPORTED_PARAMETER,
+                             "Using different transports for the video "
+                             "RtpReceiver and RtpReceiver is not currently "
+                             "supported.");
+  }
+  // If setting new transport, extract its RTCP parameters and create video
+  // channel.
+  if (!inner_video_transport_) {
+    CopyRtcpParametersToDescriptions(inner_transport->GetRtcpParameters(),
+                                     &local_video_description_,
+                                     &remote_video_description_);
+    inner_video_transport_ = inner_transport;
+    CreateVideoChannel();
+  }
+  have_video_receiver_ = true;
+  return RTCError();
+}
+
+void RtpTransportControllerShim::DetachAudioSender() {
+  if (!have_audio_sender_) {
+    // Should be impossible unless RtpSenderShim is doing something wrong.
+    RTC_NOTREACHED();
+    return;
+  }
+  // Empty parameters should result in sending being stopped.
+  RTCError err =
+      ValidateAndApplyAudioSenderParameters(RtpParameters(), nullptr);
+  RTC_DCHECK(err.ok());
+  have_audio_sender_ = false;
+  if (!have_audio_receiver_) {
+    DestroyVoiceChannel();
+  }
+}
+
+void RtpTransportControllerShim::DetachVideoSender() {
+  if (!have_video_sender_) {
+    // Should be impossible unless RtpSenderShim is doing something wrong.
+    RTC_NOTREACHED();
+    return;
+  }
+  // Empty parameters should result in sending being stopped.
+  RTCError err =
+      ValidateAndApplyVideoSenderParameters(RtpParameters(), nullptr);
+  RTC_DCHECK(err.ok());
+  have_video_sender_ = false;
+  if (!have_video_receiver_) {
+    DestroyVideoChannel();
+  }
+}
+
+void RtpTransportControllerShim::DetachAudioReceiver() {
+  if (!have_audio_receiver_) {
+    // Should be impossible unless RtpReceiverShim is doing something wrong.
+    RTC_NOTREACHED();
+    return;
+  }
+  // Empty parameters should result in receiving being stopped.
+  RTCError err = ValidateAndApplyAudioReceiverParameters(RtpParameters());
+  RTC_DCHECK(err.ok());
+  have_audio_receiver_ = false;
+  if (!have_audio_sender_) {
+    DestroyVoiceChannel();
+  }
+}
+
+void RtpTransportControllerShim::DetachVideoReceiver() {
+  if (!have_video_receiver_) {
+    // Should be impossible unless RtpReceiverShim is doing something wrong.
+    RTC_NOTREACHED();
+    return;
+  }
+  // Empty parameters should result in receiving being stopped.
+  RTCError err = ValidateAndApplyVideoReceiverParameters(RtpParameters());
+  RTC_DCHECK(err.ok());
+  have_video_receiver_ = false;
+  if (!have_video_sender_) {
+    DestroyVideoChannel();
+  }
+}
+
+RTCError RtpTransportControllerShim::ValidateAndApplyAudioSenderParameters(
+    const RtpParameters& parameters,
+    uint32_t* primary_ssrc) {
+  RTC_DCHECK(voice_channel_);
+  RTC_DCHECK(have_audio_sender_);
+
+  std::vector<cricket::AudioCodec> cricket_codecs;
+  RTCError err = ValidateAndConvertCodecs(parameters.codecs, &cricket_codecs);
+  if (!err.ok()) {
+    return err;
+  }
+
+  cricket::RtpHeaderExtensions cricket_extensions;
+  err = ValidateAndConvertHeaderExtensions(parameters.header_extensions,
+                                           &cricket_extensions);
+  if (!err.ok()) {
+    return err;
+  }
+
+  cricket::StreamParamsVec cricket_streams;
+  cricket::RtpTransceiverDirection local_direction =
+      cricket::RtpTransceiverDirection::FromMediaContentDirection(
+          local_audio_description_.direction());
+  int bandwidth = cricket::kAutoBandwidth;
+  err = ValidateAndConvertSenderEncodings(
+      parameters.encodings, inner_audio_transport_->GetRtcpParameters().cname,
+      local_audio_description_, &cricket_streams, &local_direction.send,
+      &bandwidth);
+  if (!err.ok()) {
+    return err;
+  }
+  if (primary_ssrc && !cricket_streams.empty()) {
+    *primary_ssrc = cricket_streams[0].first_ssrc();
+  }
+
+  // Validation is done, so we can attempt applying the descriptions. Sent
+  // codecs and header extensions go in remote description, streams go in
+  // local.
+  remote_audio_description_.set_codecs(cricket_codecs);
+  remote_audio_description_.set_rtp_header_extensions(cricket_extensions);
+  remote_audio_description_.set_bandwidth(bandwidth);
+  local_audio_description_.mutable_streams() = cricket_streams;
+  // Direction set based on encoding "active" flag.
+  local_audio_description_.set_direction(
+      local_direction.ToMediaContentDirection());
+  remote_audio_description_.set_direction(
+      local_direction.MakeReversed().ToMediaContentDirection());
+  // Add default codec if there are no encodings; no codec and no encodings
+  // should be legal, but the media engine code doesn't support that.
+  if (local_audio_description_.streams().empty() &&
+      remote_audio_description_.codecs().empty()) {
+    AddDefaultAudioCodec(&remote_audio_description_);
+  }
+
+  // Set remote content first, to ensure the stream is created with the correct
+  // codec.
+  if (!voice_channel_->SetRemoteContent(&remote_audio_description_,
+                                        cricket::CA_OFFER, nullptr)) {
+    return CreateAndLogError(
+        RTCErrorType::INTERNAL_ERROR,
+        "Failed to apply remote parameters to media channel.");
+  }
+  if (!voice_channel_->SetLocalContent(&local_audio_description_,
+                                       cricket::CA_ANSWER, nullptr)) {
+    return CreateAndLogError(
+        RTCErrorType::INTERNAL_ERROR,
+        "Failed to apply local parameters to media channel.");
+  }
+  return RTCError();
+}
+
+RTCError RtpTransportControllerShim::ValidateAndApplyVideoSenderParameters(
+    const RtpParameters& parameters,
+    uint32_t* primary_ssrc) {
+  RTC_DCHECK(video_channel_);
+  RTC_DCHECK(have_video_sender_);
+
+  std::vector<cricket::VideoCodec> cricket_codecs;
+  RTCError err = ValidateAndConvertCodecs(parameters.codecs, &cricket_codecs);
+  if (!err.ok()) {
+    return err;
+  }
+
+  cricket::RtpHeaderExtensions cricket_extensions;
+  err = ValidateAndConvertHeaderExtensions(parameters.header_extensions,
+                                           &cricket_extensions);
+  if (!err.ok()) {
+    return err;
+  }
+
+  cricket::StreamParamsVec cricket_streams;
+  cricket::RtpTransceiverDirection local_direction =
+      cricket::RtpTransceiverDirection::FromMediaContentDirection(
+          local_video_description_.direction());
+  int bandwidth = cricket::kAutoBandwidth;
+  err = ValidateAndConvertSenderEncodings(
+      parameters.encodings, inner_video_transport_->GetRtcpParameters().cname,
+      local_video_description_, &cricket_streams, &local_direction.send,
+      &bandwidth);
+  if (!err.ok()) {
+    return err;
+  }
+  if (primary_ssrc && !cricket_streams.empty()) {
+    *primary_ssrc = cricket_streams[0].first_ssrc();
+  }
+
+  // Validation is done, so we can attempt applying the descriptions. Sent
+  // codecs and header extensions go in remote description, streams go in
+  // local.
+  remote_video_description_.set_codecs(cricket_codecs);
+  remote_video_description_.set_rtp_header_extensions(cricket_extensions);
+  remote_video_description_.set_bandwidth(bandwidth);
+  local_video_description_.mutable_streams() = cricket_streams;
+  // Direction set based on encoding "active" flag.
+  local_video_description_.set_direction(
+      local_direction.ToMediaContentDirection());
+  remote_video_description_.set_direction(
+      local_direction.MakeReversed().ToMediaContentDirection());
+  // Add default codec if there are no encodings; no codec and no encodings
+  // should be legal, but the media engine code doesn't support that.
+  if (local_video_description_.streams().empty() &&
+      remote_video_description_.codecs().empty()) {
+    AddDefaultVideoCodec(&remote_video_description_);
+  }
+
+  // Set remote content first, to ensure the stream is created with the correct
+  // codec.
+  if (!video_channel_->SetRemoteContent(&remote_video_description_,
+                                        cricket::CA_OFFER, nullptr)) {
+    return CreateAndLogError(
+        RTCErrorType::INTERNAL_ERROR,
+        "Failed to apply remote parameters to media channel.");
+  }
+  if (!video_channel_->SetLocalContent(&local_video_description_,
+                                       cricket::CA_ANSWER, nullptr)) {
+    return CreateAndLogError(
+        RTCErrorType::INTERNAL_ERROR,
+        "Failed to apply local parameters to media channel.");
+  }
+  return RTCError();
+}
+
+RTCError RtpTransportControllerShim::ValidateAndApplyAudioReceiverParameters(
+    const RtpParameters& parameters) {
+  RTC_DCHECK(voice_channel_);
+  RTC_DCHECK(have_audio_receiver_);
+
+  std::vector<cricket::AudioCodec> cricket_codecs;
+  RTCError err = ValidateAndConvertCodecs(parameters.codecs, &cricket_codecs);
+  if (!err.ok()) {
+    return err;
+  }
+
+  cricket::RtpHeaderExtensions cricket_extensions;
+  err = ValidateAndConvertHeaderExtensions(parameters.header_extensions,
+                                           &cricket_extensions);
+  if (!err.ok()) {
+    return err;
+  }
+
+  cricket::StreamParamsVec cricket_streams;
+  cricket::RtpTransceiverDirection local_direction =
+      cricket::RtpTransceiverDirection::FromMediaContentDirection(
+          local_audio_description_.direction());
+  int bandwidth = cricket::kAutoBandwidth;
+  err = ValidateAndConvertReceiverEncodings(
+      parameters.encodings, &cricket_streams, &local_direction.recv);
+  if (!err.ok()) {
+    return err;
+  }
+
+  // Validation is done, so we can attempt applying the descriptions. Received
+  // codecs and header extensions go in local description, streams go in
+  // remote.
+  local_audio_description_.set_codecs(cricket_codecs);
+  local_audio_description_.set_rtp_header_extensions(cricket_extensions);
+  local_audio_description_.set_bandwidth(bandwidth);
+  remote_audio_description_.mutable_streams() = cricket_streams;
+  // Direction set based on encoding "active" flag.
+  local_audio_description_.set_direction(
+      local_direction.ToMediaContentDirection());
+  remote_audio_description_.set_direction(
+      local_direction.MakeReversed().ToMediaContentDirection());
+  // Add default codec if there are no encodings; no codec and no encodings
+  // should be legal, but the media engine code doesn't support that.
+  if (remote_audio_description_.streams().empty() &&
+      local_audio_description_.codecs().empty()) {
+    AddDefaultAudioCodec(&local_audio_description_);
+  }
+
+  if (!voice_channel_->SetLocalContent(&local_audio_description_,
+                                       cricket::CA_OFFER, nullptr)) {
+    return CreateAndLogError(
+        RTCErrorType::INTERNAL_ERROR,
+        "Failed to apply local parameters to media channel.");
+  }
+  if (!voice_channel_->SetRemoteContent(&remote_audio_description_,
+                                        cricket::CA_ANSWER, nullptr)) {
+    return CreateAndLogError(
+        RTCErrorType::INTERNAL_ERROR,
+        "Failed to apply remote parameters to media channel.");
+  }
+  return RTCError();
+}
+
+RTCError RtpTransportControllerShim::ValidateAndApplyVideoReceiverParameters(
+    const RtpParameters& parameters) {
+  RTC_DCHECK(video_channel_);
+  RTC_DCHECK(have_video_receiver_);
+
+  std::vector<cricket::VideoCodec> cricket_codecs;
+  RTCError err = ValidateAndConvertCodecs(parameters.codecs, &cricket_codecs);
+  if (!err.ok()) {
+    return err;
+  }
+
+  cricket::RtpHeaderExtensions cricket_extensions;
+  err = ValidateAndConvertHeaderExtensions(parameters.header_extensions,
+                                           &cricket_extensions);
+  if (!err.ok()) {
+    return err;
+  }
+
+  cricket::StreamParamsVec cricket_streams;
+  cricket::RtpTransceiverDirection local_direction =
+      cricket::RtpTransceiverDirection::FromMediaContentDirection(
+          local_video_description_.direction());
+  int bandwidth = cricket::kAutoBandwidth;
+  err = ValidateAndConvertReceiverEncodings(
+      parameters.encodings, &cricket_streams, &local_direction.recv);
+  if (!err.ok()) {
+    return err;
+  }
+
+  // Validation is done, so we can attempt applying the descriptions. Received
+  // codecs and header extensions go in local description, streams go in
+  // remote.
+  local_video_description_.set_codecs(cricket_codecs);
+  local_video_description_.set_rtp_header_extensions(cricket_extensions);
+  local_video_description_.set_bandwidth(bandwidth);
+  remote_video_description_.mutable_streams() = cricket_streams;
+  // Direction set based on encoding "active" flag.
+  local_video_description_.set_direction(
+      local_direction.ToMediaContentDirection());
+  remote_video_description_.set_direction(
+      local_direction.MakeReversed().ToMediaContentDirection());
+  // Add default codec if there are no encodings; no codec and no encodings
+  // should be legal, but the media engine code doesn't support that.
+  if (remote_video_description_.streams().empty() &&
+      local_video_description_.codecs().empty()) {
+    AddDefaultVideoCodec(&local_video_description_);
+  }
+
+  if (!video_channel_->SetLocalContent(&local_video_description_,
+                                       cricket::CA_OFFER, nullptr)) {
+    return CreateAndLogError(
+        RTCErrorType::INTERNAL_ERROR,
+        "Failed to apply local parameters to media channel.");
+  }
+  if (!video_channel_->SetRemoteContent(&remote_video_description_,
+                                        cricket::CA_ANSWER, nullptr)) {
+    return CreateAndLogError(
+        RTCErrorType::INTERNAL_ERROR,
+        "Failed to apply remote parameters to media channel.");
+  }
+  return RTCError();
+}
+
+RtpTransportControllerShim::RtpTransportControllerShim(
+    const cricket::MediaConfig& config,
+    cricket::ChannelManager* channel_manager,
+    webrtc::RtcEventLog* event_log,
+    rtc::Thread* signaling_thread,
+    rtc::Thread* worker_thread)
+    : signaling_thread_(signaling_thread),
+      worker_thread_(worker_thread),
+      media_controller_(MediaControllerInterface::Create(config,
+                                                         worker_thread,
+                                                         channel_manager,
+                                                         event_log)) {
+  RTC_DCHECK_RUN_ON(signaling_thread_);
+  RTC_DCHECK(channel_manager);
+  // MediaControllerInterface::Create should never fail.
+  RTC_DCHECK(media_controller_);
+  AddDefaultAudioCodec(&local_audio_description_);
+  AddDefaultAudioCodec(&remote_audio_description_);
+  AddDefaultVideoCodec(&local_video_description_);
+  AddDefaultVideoCodec(&remote_video_description_);

[pthatcher1, 2017/02/10 22:36:53]

This could use a comment as to why you do it.

+}
+
+void RtpTransportControllerShim::CreateVoiceChannel() {
+  voice_channel_ = media_controller_->channel_manager()->CreateVoiceChannel(
+      media_controller_.get(),
+      inner_audio_transport_->GetRtpPacketTransport()->GetInternal(),
+      inner_audio_transport_->GetRtcpPacketTransport()
+          ? inner_audio_transport_->GetRtcpPacketTransport()->GetInternal()
+          : nullptr,
+      signaling_thread_, "audio", false, cricket::AudioOptions());
+  RTC_DCHECK(voice_channel_);
+  voice_channel_->Enable(true);
+}
+
+void RtpTransportControllerShim::CreateVideoChannel() {
+  video_channel_ = media_controller_->channel_manager()->CreateVideoChannel(
+      media_controller_.get(),
+      inner_video_transport_->GetRtpPacketTransport()->GetInternal(),
+      inner_video_transport_->GetRtcpPacketTransport()
+          ? inner_video_transport_->GetRtcpPacketTransport()->GetInternal()
+          : nullptr,
+      signaling_thread_, "audio", false, cricket::VideoOptions());
+  RTC_DCHECK(video_channel_);
+  video_channel_->Enable(true);
+}
+
+void RtpTransportControllerShim::DestroyVoiceChannel() {
+  RTC_DCHECK(voice_channel_);
+  media_controller_->channel_manager()->DestroyVoiceChannel(voice_channel_);
+  voice_channel_ = nullptr;
+}
+
+void RtpTransportControllerShim::DestroyVideoChannel() {
+  RTC_DCHECK(video_channel_);
+  media_controller_->channel_manager()->DestroyVideoChannel(video_channel_);
+  video_channel_ = nullptr;
+}
+
+void RtpTransportControllerShim::CopyRtcpParametersToDescriptions(
+    const RtcpParameters& params,
+    cricket::MediaContentDescription* local,
+    cricket::MediaContentDescription* remote) {
+  local->set_rtcp_mux(params.mux);
+  remote->set_rtcp_mux(params.mux);
+  local->set_rtcp_reduced_size(params.reduced_size);
+  remote->set_rtcp_reduced_size(params.reduced_size);
+  for (cricket::StreamParams& stream_params : local->mutable_streams()) {
+    stream_params.cname = params.cname;
+  }
+}
+
+uint32_t RtpTransportControllerShim::GenerateUnusedSsrc(
+    const cricket::StreamParams& new_params) const {
+  uint32_t ssrc;
+  do {
+    ssrc = rtc::CreateRandomNonZeroId();
+  } while (
+      cricket::GetStreamBySsrc(local_audio_description_.streams(), ssrc) ||
+      cricket::GetStreamBySsrc(remote_audio_description_.streams(), ssrc) ||
+      cricket::GetStreamBySsrc(local_video_description_.streams(), ssrc) ||
+      cricket::GetStreamBySsrc(remote_video_description_.streams(), ssrc) ||
+      new_params.has_ssrc(ssrc));
+  return ssrc;
+}
+
+RTCError RtpTransportControllerShim::ValidateAndConvertSenderEncodings(
+    const std::vector<RtpEncodingParameters> encodings,
+    const std::string& cname,
+    const cricket::MediaContentDescription& description,
+    cricket::StreamParamsVec* cricket_streams,
+    bool* sending,
+    int* bandwidth) const {
+  if (encodings.size() > 1u) {
+    return CreateAndLogError(RTCErrorType::UNSUPPORTED_PARAMETER,
+                             "ORTC API implementation doesn't currently "
+                             "support simulcast or layered encodings.");
+  }
+  if (encodings.size() == 1u) {

[pthatcher1, 2017/02/10 22:36:53]

This could use some early returns to push code to the left to be more readable. 
For example, take care of the encodings.size() == 0 case.

+    const RtpEncodingParameters& encoding = encodings[0];
+    cricket::StreamParams stream_params;
+    stream_params.cname = cname;
+    if (encoding.ssrc) {
+      stream_params.add_ssrc(*encoding.ssrc);
+    } else {
+      // SSRC not provided; generate it or use the existing one.
+      if (!description.streams().empty()) {
+        stream_params.add_ssrc(description.streams()[0].first_ssrc());
+      } else {
+        stream_params.add_ssrc(GenerateUnusedSsrc(stream_params));
+      }
+    }
+    if (encoding.rtx) {
+      if (encoding.rtx->ssrc) {
+        stream_params.AddFidSsrc(stream_params.first_ssrc(),
+                                 *encoding.rtx->ssrc);
+      } else {
+        // SSRC not provided; generate it or use the existing one.
+        if (!description.streams().empty() &&
+            description.streams()[0].has_ssrc_group(
+                cricket::kFidSsrcGroupSemantics)) {
+          stream_params.AddFidSsrc(
+              stream_params.first_ssrc(),
+              description.streams()[0]
+                  .get_ssrc_group(cricket::kFidSsrcGroupSemantics)
+                  ->ssrcs[1]);
+        } else {
+          stream_params.AddFidSsrc(stream_params.first_ssrc(),
+                                   GenerateUnusedSsrc(stream_params));
+        }
+      }
+    }
+    cricket_streams->push_back(std::move(stream_params));
+    if (encoding.max_bitrate_bps) {
+      *bandwidth = *encoding.max_bitrate_bps;
+    }
+    *sending = encoding.active;
+  } else {
+    *sending = false;
+  }
+  return RTCError();
+}
+
+}  // namespace webrtc