From 603ae3ece2d3b4167eb0b88362866b4fa0eb0f4f Mon Sep 17 00:00:00 2001 From: "bemasc@google.com" Date: Fri, 1 Mar 2013 17:03:02 +0000 Subject: [PATCH] Make RtpHeaderExtensionMap::Register and ::Deregister idempotent. This CL changes the return code of these methods to indicate success instead of failure when there is nothing to change. This change appears to resolve an issue where enabling the timestamp offset extension via SDP would result in a failure if that extension had already been enabled. Review URL: https://webrtc-codereview.appspot.com/1118008 git-svn-id: http://webrtc.googlecode.com/svn/trunk@3588 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../modules/rtp_rtcp/source/rtp_header_extension.cc | 13 +++++++++---- .../source/rtp_header_extension_unittest.cc | 9 ++++++++- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/rtp_header_extension.cc b/webrtc/modules/rtp_rtcp/source/rtp_header_extension.cc index 58b01bf410..64455ff3db 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_header_extension.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_header_extension.cc @@ -39,7 +39,14 @@ int32_t RtpHeaderExtensionMap::Register(const RTPExtensionType type, std::map::iterator it = extensionMap_.find(id); if (it != extensionMap_.end()) { - return -1; + if (it->second->type != type) { + // An extension is already registered with the same id + // but a different type, so return failure. + return -1; + } + // This extension type is already registered with this id, + // so return success. + return 0; } extensionMap_[id] = new HeaderExtension(type); return 0; @@ -52,9 +59,7 @@ int32_t RtpHeaderExtensionMap::Deregister(const RTPExtensionType type) { } std::map::iterator it = extensionMap_.find(id); - if (it == extensionMap_.end()) { - return -1; - } + assert(it != extensionMap_.end()); delete it->second; extensionMap_.erase(it); return 0; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_header_extension_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_header_extension_unittest.cc index e4160c7c51..e47902e5c5 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_header_extension_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_header_extension_unittest.cc @@ -44,9 +44,16 @@ TEST_F(RtpHeaderExtensionTest, RegisterIllegalArg) { EXPECT_EQ(-1, map_.Register(kRtpExtensionTransmissionTimeOffset, 15)); } +TEST_F(RtpHeaderExtensionTest, Idempotent) { + EXPECT_EQ(0, map_.Register(kRtpExtensionTransmissionTimeOffset, kId)); + EXPECT_EQ(0, map_.Register(kRtpExtensionTransmissionTimeOffset, kId)); + EXPECT_EQ(0, map_.Deregister(kRtpExtensionTransmissionTimeOffset)); + EXPECT_EQ(0, map_.Deregister(kRtpExtensionTransmissionTimeOffset)); +} + TEST_F(RtpHeaderExtensionTest, NonUniqueId) { EXPECT_EQ(0, map_.Register(kRtpExtensionTransmissionTimeOffset, kId)); - EXPECT_EQ(-1, map_.Register(kRtpExtensionTransmissionTimeOffset, kId)); + EXPECT_EQ(-1, map_.Register(kRtpExtensionAudioLevel, kId)); } TEST_F(RtpHeaderExtensionTest, GetTotalLength) {