diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 902b5365d1..a313407f2d 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -811,6 +811,8 @@ std::vector WebRtcVideoEngine::recv_codecs(bool include_rtx) const { std::vector WebRtcVideoEngine::GetRtpHeaderExtensions() const { std::vector result; + // id is *not* incremented for non-default extensions, UsedIds needs to + // resolve conflicts. int id = 1; for (const auto& uri : {webrtc::RtpExtension::kTimestampOffsetUri, @@ -825,27 +827,26 @@ WebRtcVideoEngine::GetRtpHeaderExtensions() const { result.emplace_back(uri, id++, webrtc::RtpTransceiverDirection::kSendRecv); } for (const auto& uri : {webrtc::RtpExtension::kAbsoluteCaptureTimeUri}) { - result.emplace_back(uri, id++, webrtc::RtpTransceiverDirection::kStopped); + result.emplace_back(uri, id, webrtc::RtpTransceiverDirection::kStopped); } - result.emplace_back(webrtc::RtpExtension::kGenericFrameDescriptorUri00, id++, + result.emplace_back(webrtc::RtpExtension::kGenericFrameDescriptorUri00, id, IsEnabled(trials_, "WebRTC-GenericDescriptorAdvertised") ? webrtc::RtpTransceiverDirection::kSendRecv : webrtc::RtpTransceiverDirection::kStopped); result.emplace_back( - webrtc::RtpExtension::kDependencyDescriptorUri, id++, + webrtc::RtpExtension::kDependencyDescriptorUri, id, IsEnabled(trials_, "WebRTC-DependencyDescriptorAdvertised") ? webrtc::RtpTransceiverDirection::kSendRecv : webrtc::RtpTransceiverDirection::kStopped); - result.emplace_back( - webrtc::RtpExtension::kVideoLayersAllocationUri, id++, + webrtc::RtpExtension::kVideoLayersAllocationUri, id, IsEnabled(trials_, "WebRTC-VideoLayersAllocationAdvertised") ? webrtc::RtpTransceiverDirection::kSendRecv : webrtc::RtpTransceiverDirection::kStopped); // VideoFrameTrackingId is a test-only extension. if (IsEnabled(trials_, "WebRTC-VideoFrameTrackingIdAdvertised")) { - result.emplace_back(webrtc::RtpExtension::kVideoFrameTrackingIdUri, id++, + result.emplace_back(webrtc::RtpExtension::kVideoFrameTrackingIdUri, id, webrtc::RtpTransceiverDirection::kSendRecv); } return result; diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index fcc2703f98..23a1b1927c 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -643,6 +643,8 @@ std::vector WebRtcVoiceEngine::GetRtpHeaderExtensions() const { RTC_DCHECK(signal_thread_checker_.IsCurrent()); std::vector result; + // id is *not* incremented for non-default extensions, UsedIds needs to + // resolve conflicts. int id = 1; for (const auto& uri : {webrtc::RtpExtension::kAudioLevelUri, webrtc::RtpExtension::kAbsSendTimeUri, @@ -651,7 +653,7 @@ WebRtcVoiceEngine::GetRtpHeaderExtensions() const { result.emplace_back(uri, id++, webrtc::RtpTransceiverDirection::kSendRecv); } for (const auto& uri : {webrtc::RtpExtension::kAbsoluteCaptureTimeUri}) { - result.emplace_back(uri, id++, webrtc::RtpTransceiverDirection::kStopped); + result.emplace_back(uri, id, webrtc::RtpTransceiverDirection::kStopped); } return result; } diff --git a/pc/used_ids.h b/pc/used_ids.h index 6b342cbea8..42ef00a7c0 100644 --- a/pc/used_ids.h +++ b/pc/used_ids.h @@ -147,15 +147,15 @@ class UsedRtpHeaderExtensionIds : public UsedIds { 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 + // header extensions. This hopefully reduces 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. + // `extmap_allow_mixed_` is true), search for unused ids from 16 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. + // extensions (14). while (IsIdUsed(next_extension_id_) && next_extension_id_ >= min_allowed_id_) { --next_extension_id_; @@ -165,9 +165,10 @@ class UsedRtpHeaderExtensionIds : public UsedIds { 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. + // continue at the first two-byte ID (16; avoid 15 since it is somewhat + // special per https://www.rfc-editor.org/rfc/rfc8285#section-4.2 next_extension_id_ = - webrtc::RtpExtension::kOneByteHeaderExtensionMaxId + 1; + webrtc::RtpExtension::kOneByteHeaderExtensionMaxId + 2; } if (next_extension_id_ > diff --git a/pc/used_ids_unittest.cc b/pc/used_ids_unittest.cc index 6362f2773a..df3790b52c 100644 --- a/pc/used_ids_unittest.cc +++ b/pc/used_ids_unittest.cc @@ -119,7 +119,8 @@ TEST_F(UsedRtpHeaderExtensionIdsTest, TwoByteIdsAllowed) { UsedRtpHeaderExtensionIds::IdDomain::kTwoByteAllowed); // Fill all one byte IDs. - for (int i = 1; i < 15; ++i) { + for (int i = 1; i <= webrtc::RtpExtension::kOneByteHeaderExtensionMaxId; + ++i) { webrtc::RtpExtension id("", i); used_ids.FindAndSetIdUsed(&id); } @@ -131,11 +132,11 @@ TEST_F(UsedRtpHeaderExtensionIdsTest, TwoByteIdsAllowed) { // Expect to reassign to two-byte header extension IDs. used_ids.FindAndSetIdUsed(&id1_collision); - EXPECT_EQ(id1_collision.id, 15); + EXPECT_EQ(id1_collision.id, 16); used_ids.FindAndSetIdUsed(&id2_collision); - EXPECT_EQ(id2_collision.id, 16); + EXPECT_EQ(id2_collision.id, 17); used_ids.FindAndSetIdUsed(&id3_collision); - EXPECT_EQ(id3_collision.id, 17); + EXPECT_EQ(id3_collision.id, 18); } // Death tests.