Bug 1667319 - Move duplicate payload type checking to JsepCodecDescription; r=bwc

This moves most of the JsepTrack functionality for dealing with duplicate
payload types to a new EnsurePayloadTypeNotDuplicate method in
JsepCodecDescription. It also adds a virtual EnsureNoDuplicatePayloadTypes
method that checks the appropriate payload types for each codec type.

Differential Revision: https://phabricator.services.mozilla.com/D92112

dom/media/webrtc/jsep/JsepCodecDescription.h

@@ -87,6 +87,38 @@ class JsepCodecDescription {
 
   virtual void AddParametersToMSection(SdpMediaSection& msection) const {}
 
+  virtual void EnsureNoDuplicatePayloadTypes(std::set<std::string>& aUsedPts) {
+    mEnabled = EnsurePayloadTypeNotDuplicate(aUsedPts, mDefaultPt);
+  }
+
+  bool EnsurePayloadTypeNotDuplicate(std::set<std::string>& aUsedPts,
+                                     std::string& aPtToCheck) {
+    if (!mEnabled) {
+      return false;
+    }
+
+    if (!aUsedPts.count(aPtToCheck)) {
+      aUsedPts.insert(aPtToCheck);
+      return true;
+    }
+
+    // |codec| cannot use its current payload type. Try to find another.
+    for (uint16_t freePt = 96; freePt <= 127; ++freePt) {
+      // Not super efficient, but readability is probably more important.
+      std::ostringstream os;
+      os << freePt;
+      std::string freePtAsString = os.str();
+
+      if (!aUsedPts.count(freePtAsString)) {
+        aUsedPts.insert(freePtAsString);
+        aPtToCheck = freePtAsString;
+        return true;
+      }
+    }
+
+    return false;
+  }
+
   mozilla::SdpMediaSection::MediaType mType;
   std::string mDefaultPt;
   std::string mName;
@@ -787,6 +819,17 @@ class JsepVideoCodecDescription : public JsepCodecDescription {
     }
   }
 
+  void EnsureNoDuplicatePayloadTypes(std::set<std::string>& aUsedPts) override {
+    JsepCodecDescription::EnsureNoDuplicatePayloadTypes(aUsedPts);
+    if (mFECEnabled) {
+      mFECEnabled = EnsurePayloadTypeNotDuplicate(aUsedPts, mREDPayloadType) &&
+                    EnsurePayloadTypeNotDuplicate(aUsedPts, mULPFECPayloadType);
+    }
+    if (mRtxEnabled) {
+      mRtxEnabled = EnsurePayloadTypeNotDuplicate(aUsedPts, mRtxPayloadType);
+    }
+  }
+
   JSEP_CODEC_CLONE(JsepVideoCodecDescription)
 
   std::vector<std::string> mAckFbTypes;
@@ -892,6 +935,10 @@ class JsepApplicationCodecDescription : public JsepCodecDescription {
     return false;
   }
 
+  // We only support one datachannel per m-section
+  void EnsureNoDuplicatePayloadTypes(std::set<std::string>& aUsedPts) override {
+  }
+
   uint16_t mLocalPort;
   uint32_t mLocalMaxMessageSize;
   uint16_t mRemotePort;

dom/media/webrtc/jsep/JsepTrack.cpp

@@ -41,44 +41,9 @@ void JsepTrack::GetPayloadTypes(
 
 void JsepTrack::EnsureNoDuplicatePayloadTypes(
     std::vector<UniquePtr<JsepCodecDescription>>* codecs) {
-  std::set<uint16_t> uniquePayloadTypes;
-
+  std::set<std::string> uniquePayloadTypes;
   for (auto& codec : *codecs) {
-    // We assume there are no dupes in negotiated codecs; unnegotiated codecs
-    // need to change if there is a clash.
-    if (!codec->mEnabled ||
-        // We only support one datachannel per m-section
-        !codec->mName.compare("webrtc-datachannel")) {
-      continue;
-    }
-
-    // Disable, and only re-enable if we can ensure it has a unique pt.
-    codec->mEnabled = false;
-
-    uint16_t currentPt;
-    if (!codec->GetPtAsInt(&currentPt)) {
-      MOZ_ASSERT(false);
-      continue;
-    }
-
-    if (!uniquePayloadTypes.count(currentPt)) {
-      codec->mEnabled = true;
-      uniquePayloadTypes.insert(currentPt);
-      continue;
-    }
-
-    // |codec| cannot use its current payload type. Try to find another.
-    for (uint16_t freePt = 96; freePt <= 127; ++freePt) {
-      // Not super efficient, but readability is probably more important.
-      if (!uniquePayloadTypes.count(freePt)) {
-        uniquePayloadTypes.insert(freePt);
-        codec->mEnabled = true;
-        std::ostringstream os;
-        os << freePt;
-        codec->mDefaultPt = os.str();
-        break;
-      }
-    }
+    codec->EnsureNoDuplicatePayloadTypes(uniquePayloadTypes);
   }
 }
 

media/webrtc/signaling/gtest/jsep_session_unittest.cpp

@@ -6990,4 +6990,46 @@ TEST_F(JsepSessionTest, TestOfferRtxNoMsid) {
   ASSERT_EQ(std::string::npos, offer.find("FID")) << offer;
 }
 
+TEST_F(JsepSessionTest, TestDuplicatePayloadTypes) {
+  for (auto& codec : mSessionOff->Codecs()) {
+    if (codec->mType == SdpMediaSection::kVideo) {
+      JsepVideoCodecDescription* videoCodec =
+          static_cast<JsepVideoCodecDescription*>(codec.get());
+      videoCodec->mRtxPayloadType = "97";
+      videoCodec->EnableFec("97", "97");
+    }
+  }
+
+  types.push_back(SdpMediaSection::kVideo);
+  AddTracks(*mSessionOff, "video");
+  AddTracks(*mSessionAns, "video");
+
+  OfferAnswer();
+
+  std::vector<sdp::Direction> directions = {sdp::kSend, sdp::kRecv};
+  for (auto direction : directions) {
+    UniquePtr<JsepCodecDescription> codec;
+    std::set<std::string> payloadTypes;
+    std::string redPt, ulpfecPt;
+    for (size_t i = 0; i < 4; ++i) {
+      GetCodec(*mSessionOff, 0, direction, 0, i, &codec);
+      ASSERT_TRUE(codec);
+      JsepVideoCodecDescription* videoCodec =
+          static_cast<JsepVideoCodecDescription*>(codec.get());
+      ASSERT_TRUE(payloadTypes.insert(videoCodec->mDefaultPt).second);
+      ASSERT_TRUE(payloadTypes.insert(videoCodec->mRtxPayloadType).second);
+      // ULPFEC and RED payload types are the same for each codec, so we only
+      // check them for the first one.
+      if (i == 0) {
+        ASSERT_TRUE(payloadTypes.insert(videoCodec->mREDPayloadType).second);
+        ASSERT_TRUE(payloadTypes.insert(videoCodec->mULPFECPayloadType).second);
+        redPt = videoCodec->mREDPayloadType;
+        ulpfecPt = videoCodec->mULPFECPayloadType;
+      } else {
+        ASSERT_TRUE(redPt == videoCodec->mREDPayloadType);
+        ASSERT_TRUE(ulpfecPt == videoCodec->mULPFECPayloadType);
+      }
+    }
+  }
+}
 }  // namespace mozilla