Bug 1146529 - Part 1: Allow codecs to be "strongly preferred", and set this bit on H264 hardware codecs. r=jesup

--HG--
extra : rebase_source : 91215aff24d46112eca1f38c22d693c67e74e752

media/webrtc/signaling/src/jsep/JsepCodecDescription.h

@@ -30,7 +30,8 @@ struct JsepCodecDescription {
         mName(name),
         mClock(clock),
         mChannels(channels),
-        mEnabled(enabled)
+        mEnabled(enabled),
+        mStronglyPreferred(false)
   {
   }
   virtual ~JsepCodecDescription() {}
@@ -92,12 +93,11 @@ struct JsepCodecDescription {
   }
 
   UniquePtr<JsepCodecDescription>
-  MakeNegotiatedCodec(const std::string& pt,
-                      const SdpMediaSection& remoteMsection) const
+  MakeNegotiatedCodec(const SdpMediaSection& remoteMsection) const
   {
     UniquePtr<JsepCodecDescription> negotiated(Clone());
 
-    if (!negotiated->Negotiate(pt, remoteMsection)) {
+    if (!negotiated->Negotiate(remoteMsection)) {
       negotiated.reset();
     }
 
@@ -105,9 +105,8 @@ struct JsepCodecDescription {
   }
 
   virtual bool
-  Negotiate(const std::string& pt, const SdpMediaSection& remoteMsection)
+  Negotiate(const SdpMediaSection& remoteMsection)
   {
-    mDefaultPt = pt;
     return true;
   }
 
@@ -131,13 +130,11 @@ struct JsepCodecDescription {
   }
 
   UniquePtr<JsepCodecDescription>
-  MakeSendCodec(const mozilla::SdpMediaSection& remoteMsection,
-                const std::string& pt) const
+  MakeSendCodec(const mozilla::SdpMediaSection& remoteMsection) const
   {
     UniquePtr<JsepCodecDescription> sendCodec(Clone());
-    sendCodec->mDefaultPt = pt;
 
-    if (!sendCodec->LoadSendParameters(remoteMsection, pt)) {
+    if (!sendCodec->LoadSendParameters(remoteMsection)) {
       sendCodec.reset();
     }
 
@@ -145,13 +142,11 @@ struct JsepCodecDescription {
   }
 
   UniquePtr<JsepCodecDescription>
-  MakeRecvCodec(const mozilla::SdpMediaSection& remoteMsection,
-                const std::string& pt) const
+  MakeRecvCodec(const mozilla::SdpMediaSection& remoteMsection) const
   {
     UniquePtr<JsepCodecDescription> recvCodec(Clone());
-    recvCodec->mDefaultPt = pt;
 
-    if (!recvCodec->LoadRecvParameters(remoteMsection, pt)) {
+    if (!recvCodec->LoadRecvParameters(remoteMsection)) {
       recvCodec.reset();
     }
 
@@ -159,15 +154,13 @@ struct JsepCodecDescription {
   }
 
   virtual bool LoadSendParameters(
-      const mozilla::SdpMediaSection& remoteMsection,
-      const std::string& pt)
+      const mozilla::SdpMediaSection& remoteMsection)
   {
     return true;
   }
 
   virtual bool LoadRecvParameters(
-      const mozilla::SdpMediaSection& remoteMsection,
-      const std::string& pt)
+      const mozilla::SdpMediaSection& remoteMsection)
   {
     return true;
   }
@@ -233,6 +226,7 @@ struct JsepCodecDescription {
   uint32_t mClock;
   uint32_t mChannels;
   bool mEnabled;
+  bool mStronglyPreferred;
 };
 
 struct JsepAudioCodecDescription : public JsepCodecDescription {
@@ -368,12 +362,11 @@ struct JsepVideoCodecDescription : public JsepCodecDescription {
   }
 
   virtual bool
-  Negotiate(const std::string& pt,
-            const SdpMediaSection& remoteMsection) override
+  Negotiate(const SdpMediaSection& remoteMsection) override
   {
     if (mName == "H264") {
       SdpFmtpAttributeList::H264Parameters h264Params(
-          GetH264Parameters(pt, remoteMsection));
+          GetH264Parameters(mDefaultPt, remoteMsection));
       if (!h264Params.level_asymmetry_allowed) {
         SetSaneH264Level(std::min(GetSaneH264Level(h264Params.profile_level_id),
                                   GetSaneH264Level(mProfileLevelId)),
@@ -383,7 +376,7 @@ struct JsepVideoCodecDescription : public JsepCodecDescription {
       // TODO(bug 1143709): max-recv-level support
     }
 
-    return JsepCodecDescription::Negotiate(pt, remoteMsection);
+    return JsepCodecDescription::Negotiate(remoteMsection);
   }
 
   // Maps the not-so-sane encoding of H264 level into something that is
@@ -439,13 +432,12 @@ struct JsepVideoCodecDescription : public JsepCodecDescription {
   }
 
   virtual bool
-  LoadSendParameters(const mozilla::SdpMediaSection& remoteMsection,
-                     const std::string& pt) override
+  LoadSendParameters(const mozilla::SdpMediaSection& remoteMsection) override
   {
 
     if (mName == "H264") {
       SdpFmtpAttributeList::H264Parameters h264Params(
-          GetH264Parameters(pt, remoteMsection));
+          GetH264Parameters(mDefaultPt, remoteMsection));
 
       if (!h264Params.level_asymmetry_allowed) {
         SetSaneH264Level(std::min(GetSaneH264Level(h264Params.profile_level_id),
@@ -464,7 +456,7 @@ struct JsepVideoCodecDescription : public JsepCodecDescription {
       mSpropParameterSets = h264Params.sprop_parameter_sets;
     } else if (mName == "VP8" || mName == "VP9") {
       SdpFmtpAttributeList::VP8Parameters vp8Params(
-          GetVP8Parameters(pt, remoteMsection));
+          GetVP8Parameters(mDefaultPt, remoteMsection));
 
       mMaxFs = vp8Params.max_fs;
       mMaxFr = vp8Params.max_fr;
@@ -473,8 +465,7 @@ struct JsepVideoCodecDescription : public JsepCodecDescription {
   }
 
   virtual bool
-  LoadRecvParameters(const mozilla::SdpMediaSection& remoteMsection,
-                     const std::string& pt) override
+  LoadRecvParameters(const mozilla::SdpMediaSection& remoteMsection) override
   {
     const SdpAttributeList& attrs(remoteMsection.GetAttributeList());
 
@@ -503,7 +494,7 @@ struct JsepVideoCodecDescription : public JsepCodecDescription {
 
     if (mName == "H264") {
       SdpFmtpAttributeList::H264Parameters h264Params(
-          GetH264Parameters(pt, remoteMsection));
+          GetH264Parameters(mDefaultPt, remoteMsection));
       if (!h264Params.level_asymmetry_allowed) {
         SetSaneH264Level(std::min(GetSaneH264Level(h264Params.profile_level_id),
                                   GetSaneH264Level(mProfileLevelId)),

media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp

@@ -904,26 +904,28 @@ JsepSessionImpl::GetRtpExtensions(SdpMediaSection::MediaType type) const
   }
 }
 
-void
-JsepSessionImpl::AddCommonCodecs(const SdpMediaSection& remoteMsection,
-                                 SdpMediaSection* msection)
+static bool
+CompareCodec(const UniquePtr<JsepCodecDescription>& lhs,
+             const UniquePtr<JsepCodecDescription>& rhs)
 {
-  const std::vector<std::string>& formats = remoteMsection.GetFormats();
+  return lhs->mStronglyPreferred && !rhs->mStronglyPreferred;
+}
 
-  for (auto fmt = formats.begin(); fmt != formats.end(); ++fmt) {
-    JsepCodecDescription* codec = FindMatchingCodec(*fmt, remoteMsection);
+std::vector<UniquePtr<JsepCodecDescription>>
+JsepSessionImpl::GetCommonCodecs(const SdpMediaSection& remoteMsection)
+{
+  std::vector<UniquePtr<JsepCodecDescription>> matchingCodecs;
+  for (const std::string& fmt : remoteMsection.GetFormats()) {
+    JsepCodecDescription* codec = FindMatchingCodec(fmt, remoteMsection);
     if (codec) {
-      UniquePtr<JsepCodecDescription> negotiated(
-          codec->MakeNegotiatedCodec(*fmt, remoteMsection));
-      if (negotiated) {
-        negotiated->AddToMediaSection(*msection);
-        codec->mDefaultPt = *fmt; // Remember the other side's PT
-      }
-      // TODO(bug 1099351): Once bug 1073475 is fixed on all supported
-      // versions, we can remove this limitation.
-      break;
+      codec->mDefaultPt = fmt; // Remember the other side's PT
+      matchingCodecs.push_back(UniquePtr<JsepCodecDescription>(codec->Clone()));
     }
   }
+
+  std::stable_sort(matchingCodecs.begin(), matchingCodecs.end(), CompareCodec);
+
+  return matchingCodecs;
 }
 
 void
@@ -1177,7 +1179,19 @@ JsepSessionImpl::CreateAnswerMSection(const JsepAnswerOptions& options,
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Now add the codecs.
-  AddCommonCodecs(remoteMsection, msection);
+  std::vector<UniquePtr<JsepCodecDescription>> matchingCodecs(
+      GetCommonCodecs(remoteMsection));
+
+  for (const UniquePtr<JsepCodecDescription>& codec : matchingCodecs) {
+    UniquePtr<JsepCodecDescription> negotiated(
+        codec->MakeNegotiatedCodec(remoteMsection));
+    if (negotiated) {
+      negotiated->AddToMediaSection(*msection);
+      // TODO(bug 1099351): Once bug 1073475 is fixed on all supported
+      // versions, we can remove this limitation.
+      break;
+    }
+  }
 
   // Add extmap attributes.
   AddCommonExtmaps(remoteMsection, msection);
@@ -1668,15 +1682,10 @@ JsepSessionImpl::NegotiateTrack(const SdpMediaSection& remoteMsection,
   negotiatedDetails->mProtocol = remoteMsection.GetProtocol();
 
   // Insert all the codecs we jointly support.
-  const std::vector<std::string>& formats = remoteMsection.GetFormats();
-
-  for (auto fmt = formats.begin(); fmt != formats.end(); ++fmt) {
-    JsepCodecDescription* codec = FindMatchingCodec(*fmt, remoteMsection);
-
-    if (!codec) {
-      continue;
-    }
+  std::vector<UniquePtr<JsepCodecDescription>> commonCodecs(
+      GetCommonCodecs(remoteMsection));
 
+  for (const UniquePtr<JsepCodecDescription>& codec : commonCodecs) {
     bool sending = (direction == JsepTrack::kJsepTrackSending);
 
     // Everywhere else in JsepSessionImpl, a JsepCodecDescription describes
@@ -1690,11 +1699,9 @@ JsepSessionImpl::NegotiateTrack(const SdpMediaSection& remoteMsection,
     UniquePtr<JsepCodecDescription> sendOrReceiveCodec;
 
     if (sending) {
-      sendOrReceiveCodec =
-        Move(codec->MakeSendCodec(remoteMsection, *fmt));
+      sendOrReceiveCodec = Move(codec->MakeSendCodec(remoteMsection));
     } else {
-      sendOrReceiveCodec =
-        Move(codec->MakeRecvCodec(remoteMsection, *fmt));
+      sendOrReceiveCodec = Move(codec->MakeRecvCodec(remoteMsection));
     }
 
     if (!sendOrReceiveCodec) {
@@ -1708,7 +1715,7 @@ JsepSessionImpl::NegotiateTrack(const SdpMediaSection& remoteMsection,
       if (!sendOrReceiveCodec->GetPtAsInt(&payloadType) ||
           payloadType > UINT8_MAX) {
         JSEP_SET_ERROR("audio/video payload type is not an 8 bit unsigned int: "
-                       << *fmt);
+                       << codec->mDefaultPt);
         return NS_ERROR_INVALID_ARG;
       }
     }

media/webrtc/signaling/src/jsep/JsepSessionImpl.h

@@ -183,8 +183,8 @@ private:
       const SdpMediaSection& msection) const;
   const std::vector<SdpExtmapAttributeList::Extmap>* GetRtpExtensions(
       SdpMediaSection::MediaType type) const;
-  void AddCommonCodecs(const SdpMediaSection& remoteMsection,
-                       SdpMediaSection* msection);
+  std::vector<UniquePtr<JsepCodecDescription>> GetCommonCodecs(
+      const SdpMediaSection& remoteMsection);
   void AddCommonExtmaps(const SdpMediaSection& remoteMsection,
                         SdpMediaSection* msection);
   nsresult SetupIds();

media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp

@@ -818,10 +818,6 @@ class CompareCodecPriority {
       mPreferredCodec = os.str();
     }
 
-    void AddHardwareCodec(const std::string& codec) {
-      mHardwareCodecs.insert(codec);
-    }
-
     bool operator()(JsepCodecDescription* lhs,
                     JsepCodecDescription* rhs) const {
       if (!mPreferredCodec.empty() &&
@@ -830,8 +826,7 @@ class CompareCodecPriority {
         return true;
       }
 
-      if (mHardwareCodecs.count(lhs->mDefaultPt) &&
-          !mHardwareCodecs.count(rhs->mDefaultPt)) {
+      if (lhs->mStronglyPreferred && !rhs->mStronglyPreferred) {
         return true;
       }
 
@@ -840,7 +835,6 @@ class CompareCodecPriority {
 
   private:
     std::string mPreferredCodec;
-    std::set<std::string> mHardwareCodecs;
 };
 
 nsresult
@@ -907,7 +901,7 @@ PeerConnectionImpl::ConfigureJsepSessionCodecs() {
   bool vp9Enabled = false;
   branch->GetBoolPref("media.peerconnection.video.vp9_enabled",
                       &vp9Enabled);
-  
+
   auto& codecs = mJsepSession->Codecs();
 
   // We use this to sort the list of codecs once everything is configured
@@ -955,7 +949,7 @@ PeerConnectionImpl::ConfigureJsepSessionCodecs() {
             }
 
             if (hardwareH264Supported) {
-              comparator.AddHardwareCodec(videoCodec.mDefaultPt);
+              videoCodec.mStronglyPreferred = true;
             }
           } else if (codec.mName == "VP8" || codec.mName == "VP9") {
             if (videoCodec.mName == "VP9" && !vp9Enabled) {

media/webrtc/signaling/test/jsep_session_unittest.cpp

@@ -3099,6 +3099,29 @@ TEST(H264ProfileLevelIdTest, TestLevelSetting)
   ASSERT_EQ((uint32_t)0x6E100B, profileLevelId);
 }
 
+TEST_F(JsepSessionTest, StronglyPreferredCodec)
+{
+  for (JsepCodecDescription* codec : mSessionAns.Codecs()) {
+    if (codec->mName == "H264") {
+      codec->mStronglyPreferred = true;
+    }
+  }
+
+  types.push_back(SdpMediaSection::kVideo);
+  AddTracks(mSessionOff, "video");
+  AddTracks(mSessionAns, "video");
+
+  OfferAnswer();
+
+  const JsepCodecDescription* codec;
+  GetCodec(mSessionAns, 0, true, 0, &codec); // sending
+  ASSERT_TRUE(codec);
+  ASSERT_EQ("H264", codec->mName);
+  GetCodec(mSessionAns, 0, false, 0, &codec); // receiving
+  ASSERT_TRUE(codec);
+  ASSERT_EQ("H264", codec->mName);
+}
+
 } // namespace mozilla
 
 int