Initial asymmetric codec support in MediaSessionDescription

webrtc/pc/mediasession.cc

Index: webrtc/pc/mediasession.cc
diff --git a/webrtc/pc/mediasession.cc b/webrtc/pc/mediasession.cc
index 52abfe855f6aef47fa83d7e1ce51321f090a2417..d48404b151ecabd89849a50d9ac5b14768b094da 100644
--- a/webrtc/pc/mediasession.cc
+++ b/webrtc/pc/mediasession.cc
@@ -50,7 +50,6 @@ void GetSupportedCryptoSuiteNames(void (*func)(std::vector<int>*),
 
 namespace cricket {
 
-
 // RTP Profile names
 // http://www.iana.org/assignments/rtp-parameters/rtp-parameters.xml
 // RFC4585
@@ -69,6 +68,32 @@ const char kMediaProtocolDtlsSctp[] = "DTLS/SCTP";
 const char kMediaProtocolUdpDtlsSctp[] = "UDP/DTLS/SCTP";
 const char kMediaProtocolTcpDtlsSctp[] = "TCP/DTLS/SCTP";
 
+RtpTransceiverDirection RtpTransceiverDirection::FromMediaContentDirection(
+    MediaContentDirection md) {
+  const bool send = (md == MD_SENDRECV || md == MD_SENDONLY);
+  const bool recv = (md == MD_SENDRECV || md == MD_RECVONLY);
+  return RtpTransceiverDirection(send, recv);
+}
+
+MediaContentDirection RtpTransceiverDirection::ToMediaContentDirection() const {
+  if (send && recv) {
+    return MD_SENDRECV;
+  } else if (send) {
+    return MD_SENDONLY;
+  } else if (recv) {
+    return MD_RECVONLY;
+  }
+
+  return MD_INACTIVE;
+}
+
+RtpTransceiverDirection
+NegotiateRtpTransceiverDirection(RtpTransceiverDirection offer,
+                                 RtpTransceiverDirection wants) {
+  return RtpTransceiverDirection(offer.recv && wants.send,
+                                 offer.send && wants.recv);
+}
+
 static bool IsMediaContentOfType(const ContentInfo* content,
                                  MediaType media_type) {
   if (!IsMediaContent(content)) {
@@ -1030,30 +1055,22 @@ static bool CreateMediaContentAnswer(
 
   // Make sure the answer media content direction is per default set as
   // described in RFC3264 section 6.1.
-  switch (offer->direction()) {
-    case MD_INACTIVE:
-      answer->set_direction(MD_INACTIVE);
-      break;
-    case MD_SENDONLY:
-      answer->set_direction(MD_RECVONLY);
-      break;
-    case MD_RECVONLY:
-      answer->set_direction(IsRtpProtocol(answer->protocol()) &&
-                                    answer->streams().empty()
-                                ? MD_INACTIVE
-                                : MD_SENDONLY);
-      break;
-    case MD_SENDRECV:
-      answer->set_direction(IsRtpProtocol(answer->protocol()) &&
-                                    answer->streams().empty()
-                                ? MD_RECVONLY
-                                : MD_SENDRECV);
-      break;
-    default:
-      RTC_DCHECK(false && "MediaContentDescription has unexpected direction.");
-      break;
-  }
-
+  const bool is_data = !IsRtpProtocol(answer->protocol());
+  const bool has_send_streams = !answer->streams().empty();
+  const bool wants_send = has_send_streams || is_data;
+  const bool recv_audio =
+      answer->type() == cricket::MEDIA_TYPE_AUDIO && options.recv_audio;
+  const bool recv_video =
+      answer->type() == cricket::MEDIA_TYPE_VIDEO && options.recv_video;
+  const bool recv_data =
+      answer->type() == cricket::MEDIA_TYPE_DATA;
+  const bool wants_receive = recv_audio || recv_video || recv_data;
+
+  auto offer_rtd =
+      RtpTransceiverDirection::FromMediaContentDirection(offer->direction());
+  auto wants_rtd = RtpTransceiverDirection(wants_send, wants_receive);
+  answer->set_direction(NegotiateRtpTransceiverDirection(offer_rtd, wants_rtd)
+                        .ToMediaContentDirection());
   return true;
 }
 
@@ -1173,6 +1190,29 @@ std::string MediaTypeToString(MediaType type) {
   return type_str;
 }
 
+std::string MediaContentDirectionToString(MediaContentDirection direction) {
+  std::string dir_str;
+  switch (direction) {
+    case MD_INACTIVE:
+      dir_str = "inactive";
+      break;
+    case MD_SENDONLY:
+      dir_str = "sendonly";
+      break;
+    case MD_RECVONLY:
+      dir_str = "recvonly";
+      break;
+    case MD_SENDRECV:
+      dir_str = "sendrecv";
+      break;
+    default:
+      ASSERT(false);
+      break;
+  }
+
+  return dir_str;
+}
+
 void MediaSessionOptions::AddSendStream(MediaType type,
                                     const std::string& id,
                                     const std::string& sync_label) {
@@ -1234,11 +1274,57 @@ MediaSessionDescriptionFactory::MediaSessionDescriptionFactory(
     : secure_(SEC_DISABLED),
       add_legacy_(true),
       transport_desc_factory_(transport_desc_factory) {
-  channel_manager->GetSupportedAudioCodecs(&audio_codecs_);
+  channel_manager->GetSupportedAudioCodecs(&audio_sendrecv_codecs_);
   channel_manager->GetSupportedAudioRtpHeaderExtensions(&audio_rtp_extensions_);
   channel_manager->GetSupportedVideoCodecs(&video_codecs_);
   channel_manager->GetSupportedVideoRtpHeaderExtensions(&video_rtp_extensions_);
   channel_manager->GetSupportedDataCodecs(&data_codecs_);
+  audio_send_codecs_ = audio_sendrecv_codecs_;
+  audio_recv_codecs_ = audio_sendrecv_codecs_;
+}
+
+const AudioCodecs& MediaSessionDescriptionFactory::audio_codecs() const {
+  return audio_sendrecv_codecs_;
+}
+
+const AudioCodecs& MediaSessionDescriptionFactory::audio_send_codecs() const {
+  return audio_send_codecs_;
+}
+
+const AudioCodecs& MediaSessionDescriptionFactory::audio_recv_codecs() const {
+  return audio_recv_codecs_;
+}
+
+void MediaSessionDescriptionFactory::set_audio_codecs(
+    const AudioCodecs& send_codecs, const AudioCodecs& recv_codecs) {
+  audio_send_codecs_ = send_codecs;
+  audio_recv_codecs_ = recv_codecs;
+  audio_sendrecv_codecs_.clear();
+
+  // Intersect the two lists of codecs, preserving the order of the send codecs.
+  // If there's any difference in priorities, chances are encoding is more
+  // expensive than decoding, and high-priority send codecs are likely handled
+  // better (e.g. through hardware encoder support) than low-priority ones.
+  // TODO(ossu): This is O(n^2). However, each list should generally be small
+  // enough for this to not be a problem. They are also nicely local in memory
+  // like this.
+  for (const auto& sc : audio_send_codecs_) {
+    for (const auto& rc : audio_recv_codecs_) {
+      const size_t send_channels = (sc.channels == 0) ? 1 : sc.channels;
+      const size_t recv_channels = (rc.channels == 0) ? 1 : rc.channels;
+      if (sc.clockrate == rc.clockrate &&
+          sc.bitrate == rc.bitrate &&
+          send_channels == recv_channels &&
+          (_stricmp(sc.name.c_str(), rc.name.c_str()) == 0) &&
+          sc.params == rc.params &&
+          sc.feedback_params == rc.feedback_params) {

[Taylor Brandstetter, 2016/06/03 17:13:20]

Can this comparison be replaced by a comparison method on AudioCodec?

[ossu, 2016/06/10 09:10:04]

There are currently two ways to compare AudioCodecs: operator== and
AudioCodec.Matches(). The former does not do case insensitive matching of the
names, and does require the payload types (ids) be the same.
Matches, on the other hand, compares payload types if they are one of the static
payload types, or names (case-insensitively) if they are not. It also allows
zeroes in several of the parameters to match anything.
I don't think I could modify any of them to work like this code, without
breaking something, but I'll take a look at it. Just adding a third one doesn't
seem right. It's already unclear enough why there are two, IMO.

[Taylor Brandstetter, 2016/06/10 17:39:13]

Understood. My only worry is that an additional attribute may be added to
AudioCodec and this comparison would break. Though I guess that's unlikely; new
things will probably just go in `params`.

[ossu, 2016/06/13 12:47:19]

Ah, yes. Well, I guess it's a moot point now that I've changed to using
NegotiateCodecs instead.

+        AudioCodec out_codec = sc;
+        out_codec.channels = send_channels;
+        audio_sendrecv_codecs_.push_back(out_codec);
+        break;
+      }
+    }
+  }
 }
 
 SessionDescription* MediaSessionDescriptionFactory::CreateOffer(
@@ -1249,11 +1335,17 @@ SessionDescription* MediaSessionDescriptionFactory::CreateOffer(
   StreamParamsVec current_streams;
   GetCurrentStreamParams(current_description, &current_streams);
 
+  const bool wants_send =
+      options.HasSendMediaStream(MEDIA_TYPE_AUDIO) || add_legacy_;
+  const AudioCodecs& supported_audio_codecs =
+      GetAudioCodecsForDirection({wants_send, options.recv_audio});
+
   AudioCodecs audio_codecs;
   VideoCodecs video_codecs;
   DataCodecs data_codecs;
-  GetCodecsToOffer(current_description, &audio_codecs, &video_codecs,
-                   &data_codecs);
+  GetCodecsToOffer(current_description, supported_audio_codecs,
+                   video_codecs_, data_codecs_,
+                   &audio_codecs, &video_codecs, &data_codecs);
 
   if (!options.vad_enabled) {
     // If application doesn't want CN codecs in offer.
@@ -1412,8 +1504,24 @@ SessionDescription* MediaSessionDescriptionFactory::CreateAnswer(
   return answer.release();
 }
 
+const AudioCodecs& MediaSessionDescriptionFactory::GetAudioCodecsForDirection(
+    const RtpTransceiverDirection& direction) const {
+  // If stream is inactive - generate list as if sendrecv.
+  // See RFC 3264 Section 6.1.
+  if (direction.send == direction.recv) {
+    return audio_sendrecv_codecs_;

[Taylor Brandstetter, 2016/06/03 17:13:20]

The rule in rfc 3264 for "inactive" is a bit more complex; it takes into account
both the offer and answer direction. I think this function should take the offer
and answer direction and do something like the following:

RtpTransceiverDirection effective_direction = answer_direction;
if (!effective_direction.send && !effective_direction.recv) {
  effective_direction.send = offer_direction.recv;
  effective_direction.recv = offer_direction.send;
}

[ossu, 2016/06/10 09:10:04]

Ah, that's correct! I missed that when reading the RFC, but I see that now. I
will revise.

+  } else if (direction.send) {
+    return audio_send_codecs_;
+  } else {
+    return audio_recv_codecs_;
+  }
+}
+
 void MediaSessionDescriptionFactory::GetCodecsToOffer(
     const SessionDescription* current_description,
+    const AudioCodecs& supported_audio_codecs,
+    const VideoCodecs& supported_video_codecs,
+    const DataCodecs& supported_data_codecs,
     AudioCodecs* audio_codecs,
     VideoCodecs* video_codecs,
     DataCodecs* data_codecs) const {
@@ -1449,9 +1557,12 @@ void MediaSessionDescriptionFactory::GetCodecsToOffer(
   }
 
   // Add our codecs that are not in |current_description|.
-  FindCodecsToOffer<AudioCodec>(audio_codecs_, audio_codecs, &used_pltypes);
-  FindCodecsToOffer<VideoCodec>(video_codecs_, video_codecs, &used_pltypes);
-  FindCodecsToOffer<DataCodec>(data_codecs_, data_codecs, &used_pltypes);
+  FindCodecsToOffer<AudioCodec>(supported_audio_codecs, audio_codecs,
+                                &used_pltypes);
+  FindCodecsToOffer<VideoCodec>(supported_video_codecs, video_codecs,
+                                &used_pltypes);
+  FindCodecsToOffer<DataCodec>(supported_data_codecs, data_codecs,
+                               &used_pltypes);
 }
 
 void MediaSessionDescriptionFactory::GetRtpHdrExtsToOffer(
@@ -1577,19 +1688,9 @@ bool MediaSessionDescriptionFactory::AddAudioContentForOffer(
   bool secure_transport = (transport_desc_factory_->secure() != SEC_DISABLED);
   SetMediaProtocol(secure_transport, audio.get());
 
-  if (!audio->streams().empty()) {
-    if (options.recv_audio) {
-      audio->set_direction(MD_SENDRECV);
-    } else {
-      audio->set_direction(MD_SENDONLY);
-    }
-  } else {
-    if (options.recv_audio) {
-      audio->set_direction(MD_RECVONLY);
-    } else {
-      audio->set_direction(MD_INACTIVE);
-    }
-  }
+  auto offer_rtd =
+      RtpTransceiverDirection(!audio->streams().empty(), options.recv_audio);
+  audio->set_direction(offer_rtd.ToMediaContentDirection());
 
   desc->AddContent(content_name, NS_JINGLE_RTP, audio.release());
   if (!AddTransportOffer(content_name,
@@ -1733,6 +1834,8 @@ bool MediaSessionDescriptionFactory::AddAudioContentForAnswer(
     StreamParamsVec* current_streams,
     SessionDescription* answer) const {
   const ContentInfo* audio_content = GetFirstAudioContent(offer);
+  const AudioContentDescription* offer_audio =
+      static_cast<const AudioContentDescription*>(audio_content->description);
 
   std::unique_ptr<TransportDescription> audio_transport(CreateTransportAnswer(
       audio_content->name, offer,
@@ -1741,7 +1844,15 @@ bool MediaSessionDescriptionFactory::AddAudioContentForAnswer(
     return false;
   }
 
-  AudioCodecs audio_codecs = audio_codecs_;
+  // Pick codecs based on the requested communications direction in the offer.
+  const bool wants_send =
+      options.HasSendMediaStream(MEDIA_TYPE_AUDIO) || add_legacy_;
+  auto wants_rtd = RtpTransceiverDirection(wants_send, options.recv_audio);
+  auto offer_rtd =
+      RtpTransceiverDirection::FromMediaContentDirection(
+          offer_audio->direction());
+  auto answer_rtd = NegotiateRtpTransceiverDirection(offer_rtd, wants_rtd);
+  AudioCodecs audio_codecs = GetAudioCodecsForDirection(answer_rtd);
   if (!options.vad_enabled) {
     StripCNCodecs(&audio_codecs);
   }
@@ -1754,8 +1865,7 @@ bool MediaSessionDescriptionFactory::AddAudioContentForAnswer(
   cricket::SecurePolicy sdes_policy =
       audio_transport->secure() ? cricket::SEC_DISABLED : secure();
   if (!CreateMediaContentAnswer(
-          static_cast<const AudioContentDescription*>(
-              audio_content->description),
+          offer_audio,
           options,
           audio_codecs,
           sdes_policy,