Remove now unused code in RtpHeaderExtensionMap
Remove functions to enumerate all extensions,
Remove concept of the inactive extension.
Decision if extension should be included into rtp header is done by rtp_sender
GetTotalLengthInBytes now calculates all extension, included or not.
That is used only for calculating how much space to reserve for fec.
Since extension might suddenly be included in the next packet (which still might belong to same fec group), it is safer to calculate all registered extension.
BUG=webrtc:5565, webrtc:1994
Review-Url: https://codereview.webrtc.org/2431253003
Cr-Commit-Position: refs/heads/master@{#14763}
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_header_extension.cc b/webrtc/modules/rtp_rtcp/source/rtp_header_extension.cc
index 2c2a0a1..27bbda4 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_header_extension.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_header_extension.cc
@@ -32,19 +32,7 @@
}
}
-int32_t RtpHeaderExtensionMap::Register(const RTPExtensionType type,
- const uint8_t id) {
- return Register(type, id, true);
-}
-
-int32_t RtpHeaderExtensionMap::RegisterInactive(const RTPExtensionType type,
- const uint8_t id) {
- return Register(type, id, false);
-}
-
-int32_t RtpHeaderExtensionMap::Register(const RTPExtensionType type,
- const uint8_t id,
- bool active) {
+int32_t RtpHeaderExtensionMap::Register(RTPExtensionType type, uint8_t id) {
if (id < 1 || id > 14) {
return -1;
}
@@ -58,24 +46,12 @@
}
// This extension type is already registered with this id,
// so return success.
- it->second->active = active;
return 0;
}
- extensionMap_[id] = new HeaderExtension(type, active);
+ extensionMap_[id] = new HeaderExtension(type);
return 0;
}
-bool RtpHeaderExtensionMap::SetActive(const RTPExtensionType type,
- bool active) {
- for (auto& kv : extensionMap_) {
- if (kv.second->type == type) {
- kv.second->active = active;
- return true;
- }
- }
- return false;
-}
-
int32_t RtpHeaderExtensionMap::Deregister(const RTPExtensionType type) {
uint8_t id;
if (GetId(type, &id) != 0) {
@@ -148,88 +124,18 @@
size_t RtpHeaderExtensionMap::GetTotalLengthInBytes() const {
// Get length for each extension block.
size_t length = 0;
- std::map<uint8_t, HeaderExtension*>::const_iterator it =
- extensionMap_.begin();
- while (it != extensionMap_.end()) {
- HeaderExtension* extension = it->second;
- if (extension->active) {
- length += extension->length;
- }
- it++;
- }
+ for (const auto& kv : extensionMap_)
+ length += kv.second->length;
// Add RTP extension header length.
- if (length > 0) {
+ if (length > 0)
length += kRtpOneByteHeaderLength;
- }
// Pad up to nearest 32bit word.
length = RtpUtility::Word32Align(length);
return length;
}
-int32_t RtpHeaderExtensionMap::GetLengthUntilBlockStartInBytes(
- const RTPExtensionType type) const {
- uint8_t id;
- if (GetId(type, &id) != 0) {
- // Not registered.
- return -1;
- }
- // Get length until start of extension block type.
- uint16_t length = kRtpOneByteHeaderLength;
-
- std::map<uint8_t, HeaderExtension*>::const_iterator it =
- extensionMap_.begin();
- while (it != extensionMap_.end()) {
- HeaderExtension* extension = it->second;
- if (extension->type == type) {
- if (!extension->active) {
- return -1;
- }
- break;
- } else if (extension->active) {
- length += extension->length;
- }
- it++;
- }
- return length;
-}
-
int32_t RtpHeaderExtensionMap::Size() const {
- int32_t count = 0;
- for (auto& kv : extensionMap_) {
- if (kv.second->active) {
- count++;
- }
- }
- return count;
-}
-
-RTPExtensionType RtpHeaderExtensionMap::First() const {
- for (auto& kv : extensionMap_) {
- if (kv.second->active) {
- return kv.second->type;
- }
- }
-
- return kRtpExtensionNone;
-}
-
-RTPExtensionType RtpHeaderExtensionMap::Next(RTPExtensionType type) const {
- uint8_t id;
- if (GetId(type, &id) != 0) {
- return kRtpExtensionNone;
- }
- std::map<uint8_t, HeaderExtension*>::const_iterator it =
- extensionMap_.find(id);
- if (it == extensionMap_.end() || !it->second->active) {
- return kRtpExtensionNone;
- }
- while ((++it) != extensionMap_.end()) {
- if (it->second->active) {
- return it->second->type;
- }
- }
-
- return kRtpExtensionNone;
+ return extensionMap_.size();
}
void RtpHeaderExtensionMap::GetCopy(RtpHeaderExtensionMap* map) const {
@@ -238,7 +144,7 @@
extensionMap_.begin();
while (it != extensionMap_.end()) {
HeaderExtension* extension = it->second;
- map->Register(extension->type, it->first, extension->active);
+ map->Register(extension->type, it->first);
it++;
}
}
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h b/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h
index f7df4bb..b053f7f 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h
+++ b/webrtc/modules/rtp_rtcp/source/rtp_header_extension.h
@@ -37,12 +37,7 @@
struct HeaderExtension {
explicit HeaderExtension(RTPExtensionType extension_type)
- : type(extension_type), length(0), active(true) {
- Init();
- }
-
- HeaderExtension(RTPExtensionType extension_type, bool active)
- : type(extension_type), length(0), active(active) {
+ : type(extension_type), length(0) {
Init();
}
@@ -76,7 +71,6 @@
const RTPExtensionType type;
uint8_t length;
- bool active;
};
class RtpHeaderExtensionMap {
@@ -88,44 +82,26 @@
void Erase();
- int32_t Register(const RTPExtensionType type, const uint8_t id);
+ int32_t Register(RTPExtensionType type, uint8_t id);
- // Active on an extension indicates whether it is currently being added on
- // on the RTP packets. The active/inactive status on an extension can change
- // dynamically depending on the need to convey new information.
- int32_t RegisterInactive(const RTPExtensionType type, const uint8_t id);
- bool SetActive(const RTPExtensionType type, bool active);
-
- int32_t Deregister(const RTPExtensionType type);
+ int32_t Deregister(RTPExtensionType type);
bool IsRegistered(RTPExtensionType type) const;
- int32_t GetType(const uint8_t id, RTPExtensionType* type) const;
+ int32_t GetType(uint8_t id, RTPExtensionType* type) const;
// Return kInvalidType if not found.
RTPExtensionType GetType(uint8_t id) const;
int32_t GetId(const RTPExtensionType type, uint8_t* id) const;
// Return kInvalidId if not found.
uint8_t GetId(RTPExtensionType type) const;
-
- //
- // Methods below ignore any inactive rtp header extensions.
- //
-
size_t GetTotalLengthInBytes() const;
- int32_t GetLengthUntilBlockStartInBytes(const RTPExtensionType type) const;
-
void GetCopy(RtpHeaderExtensionMap* map) const;
int32_t Size() const;
- RTPExtensionType First() const;
-
- RTPExtensionType Next(RTPExtensionType type) const;
-
private:
- int32_t Register(const RTPExtensionType type, const uint8_t id, bool active);
std::map<uint8_t, HeaderExtension*> extensionMap_;
};
} // namespace webrtc
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 9ced70d..50a913e 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_header_extension_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_header_extension_unittest.cc
@@ -33,12 +33,6 @@
EXPECT_EQ(1, map_.Size());
EXPECT_EQ(0, map_.Deregister(kRtpExtensionTransmissionTimeOffset));
EXPECT_EQ(0, map_.Size());
-
- EXPECT_EQ(0, map_.RegisterInactive(kRtpExtensionTransmissionTimeOffset, kId));
- EXPECT_EQ(0, map_.Size());
- EXPECT_TRUE(map_.IsRegistered(kRtpExtensionTransmissionTimeOffset));
- EXPECT_TRUE(map_.SetActive(kRtpExtensionTransmissionTimeOffset, true));
- EXPECT_EQ(1, map_.Size());
}
TEST_F(RtpHeaderExtensionTest, RegisterIllegalArg) {
@@ -57,32 +51,15 @@
TEST_F(RtpHeaderExtensionTest, NonUniqueId) {
EXPECT_EQ(0, map_.Register(kRtpExtensionTransmissionTimeOffset, kId));
EXPECT_EQ(-1, map_.Register(kRtpExtensionAudioLevel, kId));
- EXPECT_EQ(-1, map_.RegisterInactive(kRtpExtensionAudioLevel, kId));
}
TEST_F(RtpHeaderExtensionTest, GetTotalLength) {
EXPECT_EQ(0u, map_.GetTotalLengthInBytes());
- EXPECT_EQ(0, map_.RegisterInactive(kRtpExtensionTransmissionTimeOffset, kId));
- EXPECT_EQ(0u, map_.GetTotalLengthInBytes());
-
EXPECT_EQ(0, map_.Register(kRtpExtensionTransmissionTimeOffset, kId));
EXPECT_EQ(kRtpOneByteHeaderLength + kTransmissionTimeOffsetLength,
map_.GetTotalLengthInBytes());
}
-TEST_F(RtpHeaderExtensionTest, GetLengthUntilBlockStart) {
- EXPECT_EQ(-1, map_.GetLengthUntilBlockStartInBytes(
- kRtpExtensionTransmissionTimeOffset));
- EXPECT_EQ(0, map_.RegisterInactive(kRtpExtensionTransmissionTimeOffset, kId));
- EXPECT_EQ(-1, map_.GetLengthUntilBlockStartInBytes(
- kRtpExtensionTransmissionTimeOffset));
-
- EXPECT_TRUE(map_.SetActive(kRtpExtensionTransmissionTimeOffset, true));
- EXPECT_EQ(static_cast<int>(kRtpOneByteHeaderLength),
- map_.GetLengthUntilBlockStartInBytes(
- kRtpExtensionTransmissionTimeOffset));
-}
-
TEST_F(RtpHeaderExtensionTest, GetType) {
RTPExtensionType typeOut;
EXPECT_EQ(-1, map_.GetType(kId, &typeOut));
@@ -101,27 +78,13 @@
EXPECT_EQ(kId, idOut);
}
-TEST_F(RtpHeaderExtensionTest, IterateTypes) {
- EXPECT_EQ(kRtpExtensionNone, map_.First());
- EXPECT_EQ(kRtpExtensionNone, map_.Next(kRtpExtensionTransmissionTimeOffset));
-
- EXPECT_EQ(0, map_.RegisterInactive(kRtpExtensionTransmissionTimeOffset, kId));
-
- EXPECT_EQ(kRtpExtensionNone, map_.First());
-
- EXPECT_TRUE(map_.SetActive(kRtpExtensionTransmissionTimeOffset, true));
-
- EXPECT_EQ(kRtpExtensionTransmissionTimeOffset, map_.First());
- EXPECT_EQ(kRtpExtensionNone, map_.Next(kRtpExtensionTransmissionTimeOffset));
-}
-
TEST_F(RtpHeaderExtensionTest, GetCopy) {
EXPECT_EQ(0, map_.Register(kRtpExtensionTransmissionTimeOffset, kId));
RtpHeaderExtensionMap mapOut;
map_.GetCopy(&mapOut);
EXPECT_EQ(1, mapOut.Size());
- EXPECT_EQ(kRtpExtensionTransmissionTimeOffset, mapOut.First());
+ EXPECT_EQ(kId, mapOut.GetId(kRtpExtensionTransmissionTimeOffset));
}
TEST_F(RtpHeaderExtensionTest, Erase) {
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
index 3e36402..f3fac77 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
@@ -92,8 +92,6 @@
payload_type_(-1),
payload_type_map_(),
rtp_header_extension_map_(),
- video_rotation_active_(false),
- playout_delay_active_(false),
packet_history_(clock),
// Statistics
rtp_stats_callback_(nullptr),
@@ -185,11 +183,7 @@
rtc::CritScope lock(&send_critsect_);
switch (type) {
case kRtpExtensionVideoRotation:
- video_rotation_active_ = false;
- return rtp_header_extension_map_.RegisterInactive(type, id);
case kRtpExtensionPlayoutDelay:
- playout_delay_active_ = false;
- return rtp_header_extension_map_.RegisterInactive(type, id);
case kRtpExtensionTransmissionTimeOffset:
case kRtpExtensionAbsoluteSendTime:
case kRtpExtensionAudioLevel:
@@ -378,16 +372,6 @@
return 0;
}
-bool RTPSender::ActivateCVORtpHeaderExtension() {
- if (!video_rotation_active_) {
- rtc::CritScope lock(&send_critsect_);
- if (rtp_header_extension_map_.SetActive(kRtpExtensionVideoRotation, true)) {
- video_rotation_active_ = true;
- }
- }
- return video_rotation_active_;
-}
-
bool RTPSender::SendOutgoingData(FrameType frame_type,
int8_t payload_type,
uint32_t capture_timestamp,
@@ -440,18 +424,6 @@
sequence_number);
}
- // Update the active/inactive status of playout delay extension based
- // on what the oracle indicates.
- {
- rtc::CritScope lock(&send_critsect_);
- bool send_playout_delay = playout_delay_oracle_.send_playout_delay();
- if (playout_delay_active_ != send_playout_delay) {
- playout_delay_active_ = send_playout_delay;
- rtp_header_extension_map_.SetActive(kRtpExtensionPlayoutDelay,
- playout_delay_active_);
- }
- }
-
result = video_->SendVideo(video_type, frame_type, payload_type,
rtp_timestamp, capture_time_ms, payload_data,
payload_size, fragmentation, rtp_header);
@@ -1027,44 +999,6 @@
return true;
}
-bool RTPSender::FindHeaderExtensionPosition(RTPExtensionType type,
- const uint8_t* rtp_packet,
- size_t rtp_packet_length,
- const RTPHeader& rtp_header,
- size_t* position) const {
- // Get length until start of header extension block.
- int extension_block_pos =
- rtp_header_extension_map_.GetLengthUntilBlockStartInBytes(type);
- if (extension_block_pos < 0) {
- LOG(LS_WARNING) << "Failed to find extension position for " << type
- << " as it is not registered.";
- return false;
- }
-
- HeaderExtension header_extension(type);
-
- size_t extension_pos =
- kRtpHeaderLength + rtp_header.numCSRCs * sizeof(uint32_t);
- size_t block_pos = extension_pos + extension_block_pos;
- if (rtp_packet_length < block_pos + header_extension.length ||
- rtp_header.headerLength < block_pos + header_extension.length) {
- LOG(LS_WARNING) << "Failed to find extension position for " << type
- << " as the length is invalid.";
- return false;
- }
-
- // Verify that header contains extension.
- if (!(rtp_packet[extension_pos] == 0xBE &&
- rtp_packet[extension_pos + 1] == 0xDE)) {
- LOG(LS_WARNING) << "Failed to find extension position for " << type
- << "as hdr extension not found.";
- return false;
- }
-
- *position = block_pos;
- return true;
-}
-
bool RTPSender::UpdateTransportSequenceNumber(RtpPacketToSend* packet,
int* packet_id) const {
RTC_DCHECK(packet);
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.h b/webrtc/modules/rtp_rtcp/source/rtp_sender.h
index 6b5a6c7..f31dde9 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h
@@ -211,7 +211,6 @@
RtpState GetRtpState() const;
void SetRtxRtpState(const RtpState& rtp_state);
RtpState GetRtxRtpState() const;
- bool ActivateCVORtpHeaderExtension();
protected:
int32_t CheckPayloadType(int8_t payload_type, RtpVideoCodecTypes* video_type);
@@ -250,24 +249,9 @@
int64_t capture_time_ms,
uint32_t ssrc);
- // Find the byte position of the RTP extension as indicated by |type| in
- // |rtp_packet|. Return false if such extension doesn't exist.
- bool FindHeaderExtensionPosition(RTPExtensionType type,
- const uint8_t* rtp_packet,
- size_t rtp_packet_length,
- const RTPHeader& rtp_header,
- size_t* position) const
- EXCLUSIVE_LOCKS_REQUIRED(send_critsect_);
-
bool UpdateTransportSequenceNumber(RtpPacketToSend* packet,
int* packet_id) const;
- void UpdatePlayoutDelayLimits(uint8_t* rtp_packet,
- size_t rtp_packet_length,
- const RTPHeader& rtp_header,
- uint16_t min_playout_delay,
- uint16_t max_playout_delay) const;
-
void UpdateRtpStats(const RtpPacketToSend& packet,
bool is_rtx,
bool is_retransmit);
@@ -296,13 +280,11 @@
std::map<int8_t, RtpUtility::Payload*> payload_type_map_;
RtpHeaderExtensionMap rtp_header_extension_map_ GUARDED_BY(send_critsect_);
- bool video_rotation_active_;
// Tracks the current request for playout delay limits from application
// and decides whether the current RTP frame should include the playout
// delay extension on header.
PlayoutDelayOracle playout_delay_oracle_;
- bool playout_delay_active_ GUARDED_BY(send_critsect_);
RtpPacketHistory packet_history_;
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
index 62fd743..2f664d7 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -332,7 +332,6 @@
rtp_sender_->RtpHeaderExtensionLength());
EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(
kRtpExtensionVideoRotation, kVideoRotationExtensionId));
- EXPECT_TRUE(rtp_sender_->ActivateCVORtpHeaderExtension());
EXPECT_EQ(RtpUtility::Word32Align(kRtpOneByteHeaderLength +
kTransmissionTimeOffsetLength +
kAbsoluteSendTimeLength +
@@ -365,9 +364,7 @@
EXPECT_EQ(0u, rtp_sender_->RtpHeaderExtensionLength());
EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(
kRtpExtensionVideoRotation, kVideoRotationExtensionId));
- EXPECT_EQ(0u, rtp_sender_->RtpHeaderExtensionLength());
- EXPECT_TRUE(rtp_sender_->ActivateCVORtpHeaderExtension());
EXPECT_EQ(
RtpUtility::Word32Align(kRtpOneByteHeaderLength + kVideoRotationLength),
rtp_sender_->RtpHeaderExtensionLength());
@@ -1435,8 +1432,6 @@
EXPECT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(
kRtpExtensionVideoRotation, kVideoRotationExtensionId));
- EXPECT_TRUE(rtp_sender_->ActivateCVORtpHeaderExtension());
-
EXPECT_EQ(
RtpUtility::Word32Align(kRtpOneByteHeaderLength + kVideoRotationLength),
rtp_sender_->RtpHeaderExtensionLength());
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
index bb41b83..a4a2b15 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc
@@ -247,13 +247,8 @@
// (e.g. a P-Frame) only if the current value is different from the previous
// value sent.
// Here we are adding it to every packet of every frame at this point.
- if (video_header && video_header->rotation != kVideoRotation_0) {
- // TODO(danilchap): Remove next call together with concept
- // of inactive extension. Now it helps to calulate total maximum size
- // or rtp header extensions that is used in FECPacketOverhead() function.
- rtp_sender_->ActivateCVORtpHeaderExtension();
+ if (video_header && video_header->rotation != kVideoRotation_0)
rtp_header->SetExtension<VideoOrientation>(video_header->rotation);
- }
size_t packet_capacity = rtp_sender_->MaxPayloadLength() -
FecPacketOverhead() -
