Add support for two-byte RTP header extensions to UsedRtpHeaderExtensionIds

Bug: webrtc:9985
Change-Id: I4c736cb920d1ac03e43c98a81218f037feebcd97
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/139601
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Commit-Queue: Johannes Kron <kron@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28346}

pc/BUILD.gn

@@ -71,6 +71,7 @@ rtc_static_library("rtc_pc_base") {
     "srtp_transport.h",
     "transport_stats.cc",
     "transport_stats.h",
+    "used_ids.h",
   ]
 
   deps = [
@@ -276,6 +277,7 @@ if (rtc_include_tests) {
       "srtp_transport_unittest.cc",
       "test/rtp_transport_test_util.h",
       "test/srtp_test_util.h",
+      "used_ids_unittest.cc",
     ]
 
     include_dirs = [ "//third_party/libsrtp/srtp" ]

pc/media_session.cc

@@ -31,6 +31,7 @@
 #include "pc/media_protocol_names.h"
 #include "pc/rtp_media_utils.h"
 #include "pc/srtp_filter.h"
+#include "pc/used_ids.h"
 #include "rtc_base/checks.h"
 #include "rtc_base/helpers.h"
 #include "rtc_base/logging.h"
@@ -281,94 +282,6 @@ void FilterDataCodecs(std::vector<DataCodec>* codecs, bool sctp) {
                 codecs->end());
 }
 
-template <typename IdStruct>
-class UsedIds {
- public:
-  UsedIds(int min_allowed_id, int max_allowed_id)
-      : min_allowed_id_(min_allowed_id),
-        max_allowed_id_(max_allowed_id),
-        next_id_(max_allowed_id) {}
-
-  // Loops through all Id in |ids| and changes its id if it is
-  // already in use by another IdStruct. Call this methods with all Id
-  // in a session description to make sure no duplicate ids exists.
-  // Note that typename Id must be a type of IdStruct.
-  template <typename Id>
-  void FindAndSetIdUsed(std::vector<Id>* ids) {
-    for (const Id& id : *ids) {
-      FindAndSetIdUsed(&id);
-    }
-  }
-
-  // Finds and sets an unused id if the |idstruct| id is already in use.
-  void FindAndSetIdUsed(IdStruct* idstruct) {
-    const int original_id = idstruct->id;
-    int new_id = idstruct->id;
-
-    if (original_id > max_allowed_id_ || original_id < min_allowed_id_) {
-      // If the original id is not in range - this is an id that can't be
-      // dynamically changed.
-      return;
-    }
-
-    if (IsIdUsed(original_id)) {
-      new_id = FindUnusedId();
-      RTC_LOG(LS_WARNING) << "Duplicate id found. Reassigning from "
-                          << original_id << " to " << new_id;
-      idstruct->id = new_id;
-    }
-    SetIdUsed(new_id);
-  }
-
- private:
-  // Returns the first unused id in reverse order.
-  // This hopefully reduce the risk of more collisions. We want to change the
-  // default ids as little as possible.
-  int FindUnusedId() {
-    while (IsIdUsed(next_id_) && next_id_ >= min_allowed_id_) {
-      --next_id_;
-    }
-    RTC_DCHECK(next_id_ >= min_allowed_id_);
-    return next_id_;
-  }
-
-  bool IsIdUsed(int new_id) { return id_set_.find(new_id) != id_set_.end(); }
-
-  void SetIdUsed(int new_id) { id_set_.insert(new_id); }
-
-  const int min_allowed_id_;
-  const int max_allowed_id_;
-  int next_id_;
-  std::set<int> id_set_;
-};
-
-// Helper class used for finding duplicate RTP payload types among audio, video
-// and data codecs. When bundle is used the payload types may not collide.
-class UsedPayloadTypes : public UsedIds<Codec> {
- public:
-  UsedPayloadTypes()
-      : UsedIds<Codec>(kDynamicPayloadTypeMin, kDynamicPayloadTypeMax) {}
-
- private:
-  static const int kDynamicPayloadTypeMin = 96;
-  static const int kDynamicPayloadTypeMax = 127;
-};
-
-// Helper class used for finding duplicate RTP Header extension ids among
-// audio and video extensions. Only applies to one-byte header extensions at the
-// moment. ids > 14 will always be reported as available.
-// TODO(kron): This class needs to be refactored when we start to send two-byte
-// header extensions.
-class UsedRtpHeaderExtensionIds : public UsedIds<webrtc::RtpExtension> {
- public:
-  UsedRtpHeaderExtensionIds()
-      : UsedIds<webrtc::RtpExtension>(
-            webrtc::RtpExtension::kMinId,
-            webrtc::RtpExtension::kOneByteHeaderExtensionMaxId) {}
-
- private:
-};
-
 static StreamParams CreateStreamParamsForNewSenderWithSsrcs(
     const SenderOptions& sender,
     const std::string& rtcp_cname,
@@ -1512,8 +1425,9 @@ std::unique_ptr<SessionDescription> MediaSessionDescriptionFactory::CreateOffer(
 
   RtpHeaderExtensions audio_rtp_extensions;
   RtpHeaderExtensions video_rtp_extensions;
-  GetRtpHdrExtsToOffer(current_active_contents, &audio_rtp_extensions,
+  GetRtpHdrExtsToOffer(current_active_contents,
-                       &video_rtp_extensions);
+                       session_options.offer_extmap_allow_mixed,
+                       &audio_rtp_extensions, &video_rtp_extensions);
 
   auto offer = absl::make_unique<SessionDescription>();
 
@@ -1958,11 +1872,19 @@ void MediaSessionDescriptionFactory::GetCodecsForAnswer(
 
 void MediaSessionDescriptionFactory::GetRtpHdrExtsToOffer(
     const std::vector<const ContentInfo*>& current_active_contents,
+    bool extmap_allow_mixed,
     RtpHeaderExtensions* offer_audio_extensions,
     RtpHeaderExtensions* offer_video_extensions) const {
   // All header extensions allocated from the same range to avoid potential
   // issues when using BUNDLE.
-  UsedRtpHeaderExtensionIds used_ids;
+
+  // Strictly speaking the SDP attribute extmap_allow_mixed signals that the
+  // receiver supports an RTP stream where one- and two-byte RTP header
+  // extensions are mixed. For backwards compatibility reasons it's used in
+  // WebRTC to signal that two-byte RTP header extensions are supported.
+  UsedRtpHeaderExtensionIds used_ids(
+      extmap_allow_mixed ? UsedRtpHeaderExtensionIds::IdDomain::kTwoByteAllowed
+                         : UsedRtpHeaderExtensionIds::IdDomain::kOneByteOnly);
   RtpHeaderExtensions all_regular_extensions;
   RtpHeaderExtensions all_encrypted_extensions;
 

pc/media_session.h

@@ -203,6 +203,7 @@ class MediaSessionDescriptionFactory {
       RtpDataCodecs* rtp_data_codecs) const;
   void GetRtpHdrExtsToOffer(
       const std::vector<const ContentInfo*>& current_active_contents,
+      bool extmap_allow_mixed,
       RtpHeaderExtensions* audio_extensions,
       RtpHeaderExtensions* video_extensions) const;
   bool AddTransportOffer(const std::string& content_name,

pc/used_ids.h

@@ -0,0 +1,168 @@
+/*
+ *  Copyright 2019 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.
+ */
+#ifndef PC_USED_IDS_H_
+#define PC_USED_IDS_H_
+
+#include <set>
+#include <vector>
+
+#include "api/rtp_parameters.h"
+#include "media/base/codec.h"
+#include "rtc_base/checks.h"
+#include "rtc_base/logging.h"
+
+namespace cricket {
+template <typename IdStruct>
+class UsedIds {
+ public:
+  UsedIds(int min_allowed_id, int max_allowed_id)
+      : min_allowed_id_(min_allowed_id),
+        max_allowed_id_(max_allowed_id),
+        next_id_(max_allowed_id) {}
+  virtual ~UsedIds() {}
+
+  // Loops through all Id in |ids| and changes its id if it is
+  // already in use by another IdStruct. Call this methods with all Id
+  // in a session description to make sure no duplicate ids exists.
+  // Note that typename Id must be a type of IdStruct.
+  template <typename Id>
+  void FindAndSetIdUsed(std::vector<Id>* ids) {
+    for (const Id& id : *ids) {
+      FindAndSetIdUsed(&id);
+    }
+  }
+
+  // Finds and sets an unused id if the |idstruct| id is already in use.
+  void FindAndSetIdUsed(IdStruct* idstruct) {
+    const int original_id = idstruct->id;
+    int new_id = idstruct->id;
+
+    if (original_id > max_allowed_id_ || original_id < min_allowed_id_) {
+      // If the original id is not in range - this is an id that can't be
+      // dynamically changed.
+      return;
+    }
+
+    if (IsIdUsed(original_id)) {
+      new_id = FindUnusedId();
+      RTC_LOG(LS_WARNING) << "Duplicate id found. Reassigning from "
+                          << original_id << " to " << new_id;
+      idstruct->id = new_id;
+    }
+    SetIdUsed(new_id);
+  }
+
+ protected:
+  bool IsIdUsed(int new_id) { return id_set_.find(new_id) != id_set_.end(); }
+  const int min_allowed_id_;
+  const int max_allowed_id_;
+
+ private:
+  // Returns the first unused id in reverse order.
+  // This hopefully reduces the risk of more collisions. We want to change the
+  // default ids as little as possible. This function is virtual and can be
+  // overriden if the search for unused IDs should follow a specific pattern.
+  virtual int FindUnusedId() {
+    while (IsIdUsed(next_id_) && next_id_ >= min_allowed_id_) {
+      --next_id_;
+    }
+    RTC_DCHECK(next_id_ >= min_allowed_id_);
+    return next_id_;
+  }
+
+  void SetIdUsed(int new_id) {
+    RTC_DCHECK(new_id >= min_allowed_id_);
+    RTC_DCHECK(new_id <= max_allowed_id_);
+    RTC_DCHECK(!IsIdUsed(new_id));
+    id_set_.insert(new_id);
+  }
+  int next_id_;
+  std::set<int> id_set_;
+};
+
+// Helper class used for finding duplicate RTP payload types among audio, video
+// and data codecs. When bundle is used the payload types may not collide.
+class UsedPayloadTypes : public UsedIds<Codec> {
+ public:
+  UsedPayloadTypes()
+      : UsedIds<Codec>(kDynamicPayloadTypeMin, kDynamicPayloadTypeMax) {}
+
+ private:
+  static const int kDynamicPayloadTypeMin = 96;
+  static const int kDynamicPayloadTypeMax = 127;
+};
+
+// Helper class used for finding duplicate RTP Header extension ids among
+// audio and video extensions.
+class UsedRtpHeaderExtensionIds : public UsedIds<webrtc::RtpExtension> {
+ public:
+  enum class IdDomain {
+    // Only allocate IDs that fit in one-byte header extensions.
+    kOneByteOnly,
+    // Prefer to allocate one-byte header extension IDs, but overflow to
+    // two-byte if none are left.
+    kTwoByteAllowed,
+  };
+
+  explicit UsedRtpHeaderExtensionIds(IdDomain id_domain)
+      : UsedIds<webrtc::RtpExtension>(
+            webrtc::RtpExtension::kMinId,
+            id_domain == IdDomain::kTwoByteAllowed
+                ? webrtc::RtpExtension::kMaxId
+                : webrtc::RtpExtension::kOneByteHeaderExtensionMaxId),
+        id_domain_(id_domain),
+        next_extension_id_(webrtc::RtpExtension::kOneByteHeaderExtensionMaxId) {
+  }
+
+ private:
+  // Returns the first unused id in reverse order from the max id of one byte
+  // header extensions. This hopefully reduce the risk of more collisions. We
+  // want to change the default ids as little as possible. If no unused id is
+  // found and two byte header extensions are enabled (i.e.,
+  // |extmap_allow_mixed_| is true), search for unused ids from 15 to 255.
+  int FindUnusedId() override {
+    if (next_extension_id_ <=
+        webrtc::RtpExtension::kOneByteHeaderExtensionMaxId) {
+      // First search in reverse order from the max id of one byte header
+      // extensions.
+      while (IsIdUsed(next_extension_id_) &&
+             next_extension_id_ >= min_allowed_id_) {
+        --next_extension_id_;
+      }
+    }
+
+    if (id_domain_ == IdDomain::kTwoByteAllowed) {
+      if (next_extension_id_ < min_allowed_id_) {
+        // We have searched among all one-byte IDs without finding an unused ID,
+        // continue at the first two-byte ID.
+        next_extension_id_ =
+            webrtc::RtpExtension::kOneByteHeaderExtensionMaxId + 1;
+      }
+
+      if (next_extension_id_ >
+          webrtc::RtpExtension::kOneByteHeaderExtensionMaxId) {
+        while (IsIdUsed(next_extension_id_) &&
+               next_extension_id_ <= max_allowed_id_) {
+          ++next_extension_id_;
+        }
+      }
+    }
+    RTC_DCHECK(next_extension_id_ >= min_allowed_id_);
+    RTC_DCHECK(next_extension_id_ <= max_allowed_id_);
+    return next_extension_id_;
+  }
+
+  const IdDomain id_domain_;
+  int next_extension_id_;
+};
+
+}  // namespace cricket
+
+#endif  // PC_USED_IDS_H_

pc/used_ids_unittest.cc

@@ -0,0 +1,176 @@
+/*
+ *  Copyright 2019 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 "pc/used_ids.h"
+#include "test/gtest.h"
+
+using cricket::UsedIds;
+using cricket::UsedRtpHeaderExtensionIds;
+
+struct Foo {
+  int id;
+};
+
+TEST(UsedIdsTest, UniqueIdsAreUnchanged) {
+  UsedIds<Foo> used_ids(1, 5);
+  for (int i = 1; i <= 5; ++i) {
+    Foo id = {i};
+    used_ids.FindAndSetIdUsed(&id);
+    EXPECT_EQ(id.id, i);
+  }
+}
+
+TEST(UsedIdsTest, IdsOutsideRangeAreUnchanged) {
+  UsedIds<Foo> used_ids(1, 5);
+
+  Foo id_11 = {11};
+  Foo id_12 = {12};
+  Foo id_12_collision = {12};
+  Foo id_13 = {13};
+  Foo id_13_collision = {13};
+
+  used_ids.FindAndSetIdUsed(&id_11);
+  EXPECT_EQ(id_11.id, 11);
+  used_ids.FindAndSetIdUsed(&id_12);
+  EXPECT_EQ(id_12.id, 12);
+  used_ids.FindAndSetIdUsed(&id_12_collision);
+  EXPECT_EQ(id_12_collision.id, 12);
+  used_ids.FindAndSetIdUsed(&id_13);
+  EXPECT_EQ(id_13.id, 13);
+  used_ids.FindAndSetIdUsed(&id_13_collision);
+  EXPECT_EQ(id_13_collision.id, 13);
+}
+
+TEST(UsedIdsTest, CollisionsAreReassignedIdsInReverseOrder) {
+  UsedIds<Foo> used_ids(1, 10);
+  Foo id_1 = {1};
+  Foo id_2 = {2};
+  Foo id_2_collision = {2};
+  Foo id_3 = {3};
+  Foo id_3_collision = {3};
+
+  used_ids.FindAndSetIdUsed(&id_1);
+  used_ids.FindAndSetIdUsed(&id_2);
+  used_ids.FindAndSetIdUsed(&id_2_collision);
+  EXPECT_EQ(id_2_collision.id, 10);
+  used_ids.FindAndSetIdUsed(&id_3);
+  used_ids.FindAndSetIdUsed(&id_3_collision);
+  EXPECT_EQ(id_3_collision.id, 9);
+}
+
+struct TestParams {
+  UsedRtpHeaderExtensionIds::IdDomain id_domain;
+  int max_id;
+};
+
+class UsedRtpHeaderExtensionIdsTest
+    : public ::testing::TestWithParam<TestParams> {};
+
+constexpr TestParams kOneByteTestParams = {
+    UsedRtpHeaderExtensionIds::IdDomain::kOneByteOnly, 14};
+constexpr TestParams kTwoByteTestParams = {
+    UsedRtpHeaderExtensionIds::IdDomain::kTwoByteAllowed, 255};
+
+INSTANTIATE_TEST_SUITE_P(,
+                         UsedRtpHeaderExtensionIdsTest,
+                         ::testing::Values(kOneByteTestParams,
+                                           kTwoByteTestParams));
+
+TEST_P(UsedRtpHeaderExtensionIdsTest, UniqueIdsAreUnchanged) {
+  UsedRtpHeaderExtensionIds used_ids(GetParam().id_domain);
+
+  // Fill all IDs.
+  for (int j = 1; j <= GetParam().max_id; ++j) {
+    webrtc::RtpExtension extension("", j);
+    used_ids.FindAndSetIdUsed(&extension);
+    EXPECT_EQ(extension.id, j);
+  }
+}
+
+TEST_P(UsedRtpHeaderExtensionIdsTest, PrioritizeReassignmentToOneByteIds) {
+  UsedRtpHeaderExtensionIds used_ids(GetParam().id_domain);
+  webrtc::RtpExtension id_1("", 1);
+  webrtc::RtpExtension id_2("", 2);
+  webrtc::RtpExtension id_2_collision("", 2);
+  webrtc::RtpExtension id_3("", 3);
+  webrtc::RtpExtension id_3_collision("", 3);
+
+  // Expect that colliding IDs are reassigned to one-byte IDs.
+  used_ids.FindAndSetIdUsed(&id_1);
+  used_ids.FindAndSetIdUsed(&id_2);
+  used_ids.FindAndSetIdUsed(&id_2_collision);
+  EXPECT_EQ(id_2_collision.id, 14);
+  used_ids.FindAndSetIdUsed(&id_3);
+  used_ids.FindAndSetIdUsed(&id_3_collision);
+  EXPECT_EQ(id_3_collision.id, 13);
+}
+
+TEST_F(UsedRtpHeaderExtensionIdsTest, TwoByteIdsAllowed) {
+  UsedRtpHeaderExtensionIds used_ids(
+      UsedRtpHeaderExtensionIds::IdDomain::kTwoByteAllowed);
+
+  // Fill all one byte IDs.
+  for (int i = 1; i < 15; ++i) {
+    webrtc::RtpExtension id("", i);
+    used_ids.FindAndSetIdUsed(&id);
+  }
+
+  // Add new extensions with colliding IDs.
+  webrtc::RtpExtension id1_collision("", 1);
+  webrtc::RtpExtension id2_collision("", 2);
+  webrtc::RtpExtension id3_collision("", 3);
+
+  // Expect to reassign to two-byte header extension IDs.
+  used_ids.FindAndSetIdUsed(&id1_collision);
+  EXPECT_EQ(id1_collision.id, 15);
+  used_ids.FindAndSetIdUsed(&id2_collision);
+  EXPECT_EQ(id2_collision.id, 16);
+  used_ids.FindAndSetIdUsed(&id3_collision);
+  EXPECT_EQ(id3_collision.id, 17);
+}
+
+// Death tests.
+// Disabled on Android because death tests misbehave on Android, see
+// base/test/gtest_util.h.
+#if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)
+TEST(UsedIdsDeathTest, DieWhenAllIdsAreOccupied) {
+  UsedIds<Foo> used_ids(1, 5);
+  for (int i = 1; i <= 5; ++i) {
+    Foo id = {i};
+    used_ids.FindAndSetIdUsed(&id);
+  }
+  Foo id_collision = {3};
+  EXPECT_DEATH(used_ids.FindAndSetIdUsed(&id_collision), "");
+}
+
+using UsedRtpHeaderExtensionIdsDeathTest = UsedRtpHeaderExtensionIdsTest;
+INSTANTIATE_TEST_SUITE_P(,
+                         UsedRtpHeaderExtensionIdsDeathTest,
+                         ::testing::Values(kOneByteTestParams,
+                                           kTwoByteTestParams));
+
+TEST_P(UsedRtpHeaderExtensionIdsDeathTest, DieWhenAllIdsAreOccupied) {
+  UsedRtpHeaderExtensionIds used_ids(GetParam().id_domain);
+
+  // Fill all IDs.
+  for (int j = 1; j <= GetParam().max_id; ++j) {
+    webrtc::RtpExtension id("", j);
+    used_ids.FindAndSetIdUsed(&id);
+  }
+
+  webrtc::RtpExtension id1_collision("", 1);
+  webrtc::RtpExtension id2_collision("", 2);
+  webrtc::RtpExtension id3_collision("", GetParam().max_id);
+
+  EXPECT_DEATH(used_ids.FindAndSetIdUsed(&id1_collision), "");
+  EXPECT_DEATH(used_ids.FindAndSetIdUsed(&id2_collision), "");
+  EXPECT_DEATH(used_ids.FindAndSetIdUsed(&id3_collision), "");
+}
+#endif  // RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID)