Negotiate H264 profiles in SDP

webrtc/pc/mediasession.cc

Index: webrtc/pc/mediasession.cc
diff --git a/webrtc/pc/mediasession.cc b/webrtc/pc/mediasession.cc
index ebba9acbcbf5adfddabb45eb31fb00575936a85b..4eb2330aba01538b03a572dd34862b243462bc7d 100644
--- a/webrtc/pc/mediasession.cc
+++ b/webrtc/pc/mediasession.cc
@@ -22,6 +22,7 @@
 #include "webrtc/base/helpers.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/base/stringutils.h"
+#include "webrtc/common_video/h264/profile_level_id.h"
 #include "webrtc/media/base/cryptoparams.h"
 #include "webrtc/media/base/mediaconstants.h"
 #include "webrtc/p2p/base/p2pconstants.h"
@@ -769,6 +770,30 @@ static bool ReferencedCodecsMatch(const std::vector<C>& codecs1,
 }
 
 template <class C>
+static rtc::Optional<C> IntersectCodecs(const C& local, const C& remote) {
+  C intersected = local;
+
+  if (CodecNamesEq(local.name.c_str(), kH264CodecName)) {
+    const rtc::Optional<webrtc::H264::ProfileLevelId> intersected_profile =
+        webrtc::H264::NegotiateProfileLevelId(local.params, remote.params);

[hta-webrtc, 2016/11/08 13:59:53]

Illustrates my point about Negotiate being misnamed in the CL that introduced it
- "Intersect" would be a better name.

[magjed_webrtc, 2016/11/08 16:18:26]

Yeah, I'm not that happy with either the name intersect or negotiate here.

+    if (!intersected_profile)
+      return rtc::Optional<C>();
+    intersected.params["profile-level-id"] =
+        *webrtc::H264::ProfileLevelIdToString(*intersected_profile);
+  }
+
+  intersected.IntersectFeedbackParams(remote);
+  if (IsRtxCodec(remote)) {
+    const auto apt_it = remote.params.find(kCodecParamAssociatedPayloadType);
+    RTC_DCHECK(apt_it != remote.params.end());
+    intersected.SetParam(kCodecParamAssociatedPayloadType, apt_it->second);
+  }
+  intersected.id = remote.id;
+  intersected.name = remote.name;
+  return rtc::Optional<C>(std::move(intersected));
+}
+
+template <class C>
 static void NegotiateCodecs(const std::vector<C>& local_codecs,
                             const std::vector<C>& offered_codecs,
                             std::vector<C>* negotiated_codecs) {
@@ -777,19 +802,9 @@ static void NegotiateCodecs(const std::vector<C>& local_codecs,
     // Note that we intentionally only find one matching codec for each of our
     // local codecs, in case the remote offer contains duplicate codecs.
     if (FindMatchingCodec(local_codecs, offered_codecs, ours, &theirs)) {
-      C negotiated = ours;
-      negotiated.IntersectFeedbackParams(theirs);
-      if (IsRtxCodec(negotiated)) {
-        std::string offered_apt_value;
-        theirs.GetParam(kCodecParamAssociatedPayloadType, &offered_apt_value);
-        // FindMatchingCodec shouldn't return something with no apt value.
-        RTC_DCHECK(!offered_apt_value.empty());
-        negotiated.SetParam(kCodecParamAssociatedPayloadType,
-                            offered_apt_value);
-      }
-      negotiated.id = theirs.id;
-      negotiated.name = theirs.name;
-      negotiated_codecs->push_back(negotiated);
+      rtc::Optional<C> negotiated = IntersectCodecs(ours, theirs);
+      if (negotiated)
+        negotiated_codecs->push_back(std::move(*negotiated));
     }
   }
   // RFC3264: Although the answerer MAY list the formats in their desired