Cleanup rtp utils in media/base
Remove unused functions GetRtpHeader/GetRtpHeaderLength
Replace usage of SetRtpHeader with webrtc::RtpPacket
Move SetRtpSsrc next to the only place it is used.
Bug: None
Change-Id: I3ecc244b1a2bdb2d68e0dbdb34dd60160a3101f8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/225547
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34447}
diff --git a/media/base/fake_network_interface.h b/media/base/fake_network_interface.h
index 4023037..45b7aa0 100644
--- a/media/base/fake_network_interface.h
+++ b/media/base/fake_network_interface.h
@@ -18,6 +18,7 @@
#include "media/base/media_channel.h"
#include "media/base/rtp_utils.h"
#include "rtc_base/byte_order.h"
+#include "rtc_base/checks.h"
#include "rtc_base/copy_on_write_buffer.h"
#include "rtc_base/dscp.h"
#include "rtc_base/message_handler.h"
@@ -127,10 +128,7 @@
rtp_packets_.push_back(*packet);
if (conf_) {
for (size_t i = 0; i < conf_sent_ssrcs_.size(); ++i) {
- if (!SetRtpSsrc(packet->MutableData(), packet->size(),
- conf_sent_ssrcs_[i])) {
- return false;
- }
+ SetRtpSsrc(conf_sent_ssrcs_[i], *packet);
PostMessage(ST_RTP, *packet);
}
} else {
@@ -182,6 +180,11 @@
}
private:
+ void SetRtpSsrc(uint32_t ssrc, rtc::CopyOnWriteBuffer& buffer) {
+ RTC_CHECK_GE(buffer.size(), 12);
+ rtc::SetBE32(buffer.MutableData() + 8, ssrc);
+ }
+
void GetNumRtpBytesAndPackets(uint32_t ssrc, int* bytes, int* packets) {
if (bytes) {
*bytes = 0;
diff --git a/media/base/rtp_utils.cc b/media/base/rtp_utils.cc
index 3002e2d..9f90c46 100644
--- a/media/base/rtp_utils.cc
+++ b/media/base/rtp_utils.cc
@@ -25,8 +25,6 @@
namespace cricket {
-static const uint8_t kRtpVersion = 2;
-static const size_t kRtpFlagsOffset = 0;
static const size_t kRtpPayloadTypeOffset = 1;
static const size_t kRtpSeqNumOffset = 2;
static const size_t kRtpTimestampOffset = 4;
@@ -120,8 +118,6 @@
memcpy(auth_tag, output, tag_length);
}
-} // namespace
-
bool GetUint8(const void* data, size_t offset, int* value) {
if (!data || !value) {
return false;
@@ -147,36 +143,7 @@
return true;
}
-bool SetUint8(void* data, size_t offset, uint8_t value) {
- if (!data) {
- return false;
- }
- rtc::Set8(data, offset, value);
- return true;
-}
-
-bool SetUint16(void* data, size_t offset, uint16_t value) {
- if (!data) {
- return false;
- }
- rtc::SetBE16(static_cast<uint8_t*>(data) + offset, value);
- return true;
-}
-
-bool SetUint32(void* data, size_t offset, uint32_t value) {
- if (!data) {
- return false;
- }
- rtc::SetBE32(static_cast<uint8_t*>(data) + offset, value);
- return true;
-}
-
-bool GetRtpFlags(const void* data, size_t len, int* value) {
- if (len < kMinRtpPacketLen) {
- return false;
- }
- return GetUint8(data, kRtpFlagsOffset, value);
-}
+} // namespace
bool GetRtpPayloadType(const void* data, size_t len, int* value) {
if (len < kMinRtpPacketLen) {
@@ -210,34 +177,6 @@
return GetUint32(data, kRtpSsrcOffset, value);
}
-bool GetRtpHeaderLen(const void* data, size_t len, size_t* value) {
- if (!data || len < kMinRtpPacketLen || !value)
- return false;
- const uint8_t* header = static_cast<const uint8_t*>(data);
- // Get base header size + length of CSRCs (not counting extension yet).
- size_t header_size = kMinRtpPacketLen + (header[0] & 0xF) * sizeof(uint32_t);
- if (len < header_size)
- return false;
- // If there's an extension, read and add in the extension size.
- if (header[0] & 0x10) {
- if (len < header_size + sizeof(uint32_t))
- return false;
- header_size +=
- ((rtc::GetBE16(header + header_size + 2) + 1) * sizeof(uint32_t));
- if (len < header_size)
- return false;
- }
- *value = header_size;
- return true;
-}
-
-bool GetRtpHeader(const void* data, size_t len, RtpHeader* header) {
- return (GetRtpPayloadType(data, len, &(header->payload_type)) &&
- GetRtpSeqNum(data, len, &(header->seq_num)) &&
- GetRtpTimestamp(data, len, &(header->timestamp)) &&
- GetRtpSsrc(data, len, &(header->ssrc)));
-}
-
bool GetRtcpType(const void* data, size_t len, int* value) {
if (len < kMinRtcpPacketLen) {
return false;
@@ -262,24 +201,6 @@
return true;
}
-bool SetRtpSsrc(void* data, size_t len, uint32_t value) {
- return SetUint32(data, kRtpSsrcOffset, value);
-}
-
-// Assumes version 2, no padding, no extensions, no csrcs.
-bool SetRtpHeader(void* data, size_t len, const RtpHeader& header) {
- if (!IsValidRtpPayloadType(header.payload_type) || header.seq_num < 0 ||
- header.seq_num > static_cast<int>(UINT16_MAX)) {
- return false;
- }
- return (SetUint8(data, kRtpFlagsOffset, kRtpVersion << 6) &&
- SetUint8(data, kRtpPayloadTypeOffset, header.payload_type & 0x7F) &&
- SetUint16(data, kRtpSeqNumOffset,
- static_cast<uint16_t>(header.seq_num)) &&
- SetUint32(data, kRtpTimestampOffset, header.timestamp) &&
- SetRtpSsrc(data, len, header.ssrc));
-}
-
bool IsValidRtpPayloadType(int payload_type) {
return payload_type >= 0 && payload_type <= 127;
}
diff --git a/media/base/rtp_utils.h b/media/base/rtp_utils.h
index 7010248..f6b5dbc 100644
--- a/media/base/rtp_utils.h
+++ b/media/base/rtp_utils.h
@@ -26,13 +26,6 @@
const size_t kMaxRtpPacketLen = 2048;
const size_t kMinRtcpPacketLen = 4;
-struct RtpHeader {
- int payload_type;
- int seq_num;
- uint32_t timestamp;
- uint32_t ssrc;
-};
-
enum RtcpTypes {
kRtcpTypeSR = 200, // Sender report payload type.
kRtcpTypeRR = 201, // Receiver report payload type.
@@ -53,14 +46,9 @@
bool GetRtpSeqNum(const void* data, size_t len, int* value);
bool GetRtpTimestamp(const void* data, size_t len, uint32_t* value);
bool GetRtpSsrc(const void* data, size_t len, uint32_t* value);
-bool GetRtpHeaderLen(const void* data, size_t len, size_t* value);
+
bool GetRtcpType(const void* data, size_t len, int* value);
bool GetRtcpSsrc(const void* data, size_t len, uint32_t* value);
-bool GetRtpHeader(const void* data, size_t len, RtpHeader* header);
-
-bool SetRtpSsrc(void* data, size_t len, uint32_t value);
-// Assumes version 2, no padding, no extensions, no csrcs.
-bool SetRtpHeader(void* data, size_t len, const RtpHeader& header);
// Checks the packet header to determine if it can be an RTP or RTCP packet.
RtpPacketType InferRtpPacketType(rtc::ArrayView<const char> packet);
diff --git a/media/base/rtp_utils_unittest.cc b/media/base/rtp_utils_unittest.cc
index 5674d18..14599ab 100644
--- a/media/base/rtp_utils_unittest.cc
+++ b/media/base/rtp_utils_unittest.cc
@@ -23,24 +23,7 @@
static const uint8_t kRtpPacketWithMarker[] = {
0x80, 0x80, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01};
-// 3 CSRCs (0x01020304, 0x12345678, 0xAABBCCDD)
-// Extension (0xBEDE, 0x1122334455667788)
-static const uint8_t kRtpPacketWithMarkerAndCsrcAndExtension[] = {
- 0x93, 0x80, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
- 0x01, 0x02, 0x03, 0x04, 0x12, 0x34, 0x56, 0x78, 0xAA, 0xBB, 0xCC, 0xDD,
- 0xBE, 0xDE, 0x00, 0x02, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88};
static const uint8_t kInvalidPacket[] = {0x80, 0x00};
-static const uint8_t kInvalidPacketWithCsrc[] = {
- 0x83, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
- 0x01, 0x02, 0x03, 0x04, 0x12, 0x34, 0x56, 0x78, 0xAA, 0xBB, 0xCC};
-static const uint8_t kInvalidPacketWithCsrcAndExtension1[] = {
- 0x93, 0x80, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x01, 0x01, 0x02, 0x03, 0x04, 0x12, 0x34,
- 0x56, 0x78, 0xAA, 0xBB, 0xCC, 0xDD, 0xBE, 0xDE, 0x00};
-static const uint8_t kInvalidPacketWithCsrcAndExtension2[] = {
- 0x93, 0x80, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
- 0x01, 0x02, 0x03, 0x04, 0x12, 0x34, 0x56, 0x78, 0xAA, 0xBB, 0xCC, 0xDD,
- 0xBE, 0xDE, 0x00, 0x02, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77};
// PT = 206, FMT = 1, Sender SSRC = 0x1111, Media SSRC = 0x1111
// No FCI information is needed for PLI.
@@ -121,59 +104,12 @@
EXPECT_TRUE(GetRtpSsrc(kPcmuFrame, sizeof(kPcmuFrame), &ssrc));
EXPECT_EQ(1u, ssrc);
- RtpHeader header;
- EXPECT_TRUE(GetRtpHeader(kPcmuFrame, sizeof(kPcmuFrame), &header));
- EXPECT_EQ(0, header.payload_type);
- EXPECT_EQ(1, header.seq_num);
- EXPECT_EQ(0u, header.timestamp);
- EXPECT_EQ(1u, header.ssrc);
-
EXPECT_FALSE(GetRtpPayloadType(kInvalidPacket, sizeof(kInvalidPacket), &pt));
EXPECT_FALSE(GetRtpSeqNum(kInvalidPacket, sizeof(kInvalidPacket), &seq_num));
EXPECT_FALSE(GetRtpTimestamp(kInvalidPacket, sizeof(kInvalidPacket), &ts));
EXPECT_FALSE(GetRtpSsrc(kInvalidPacket, sizeof(kInvalidPacket), &ssrc));
}
-TEST(RtpUtilsTest, SetRtpHeader) {
- uint8_t packet[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
-
- RtpHeader header = {9, 1111, 2222u, 3333u};
- EXPECT_TRUE(SetRtpHeader(packet, sizeof(packet), header));
-
- // Bits: 10 0 0 0000
- EXPECT_EQ(128u, packet[0]);
- size_t len;
- EXPECT_TRUE(GetRtpHeaderLen(packet, sizeof(packet), &len));
- EXPECT_EQ(12U, len);
- EXPECT_TRUE(GetRtpHeader(packet, sizeof(packet), &header));
- EXPECT_EQ(9, header.payload_type);
- EXPECT_EQ(1111, header.seq_num);
- EXPECT_EQ(2222u, header.timestamp);
- EXPECT_EQ(3333u, header.ssrc);
-}
-
-TEST(RtpUtilsTest, GetRtpHeaderLen) {
- size_t len;
- EXPECT_TRUE(GetRtpHeaderLen(kPcmuFrame, sizeof(kPcmuFrame), &len));
- EXPECT_EQ(12U, len);
-
- EXPECT_TRUE(GetRtpHeaderLen(kRtpPacketWithMarkerAndCsrcAndExtension,
- sizeof(kRtpPacketWithMarkerAndCsrcAndExtension),
- &len));
- EXPECT_EQ(sizeof(kRtpPacketWithMarkerAndCsrcAndExtension), len);
-
- EXPECT_FALSE(GetRtpHeaderLen(kInvalidPacket, sizeof(kInvalidPacket), &len));
- EXPECT_FALSE(GetRtpHeaderLen(kInvalidPacketWithCsrc,
- sizeof(kInvalidPacketWithCsrc), &len));
- EXPECT_FALSE(GetRtpHeaderLen(kInvalidPacketWithCsrcAndExtension1,
- sizeof(kInvalidPacketWithCsrcAndExtension1),
- &len));
- EXPECT_FALSE(GetRtpHeaderLen(kInvalidPacketWithCsrcAndExtension2,
- sizeof(kInvalidPacketWithCsrcAndExtension2),
- &len));
-}
-
TEST(RtpUtilsTest, GetRtcp) {
int pt;
EXPECT_TRUE(GetRtcpType(kRtcpReport, sizeof(kRtcpReport), &pt));
diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc
index 1cad35a..d0745e3 100644
--- a/media/engine/webrtc_video_engine_unittest.cc
+++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -7002,18 +7002,14 @@
channel_->SetDefaultSink(&renderer);
// Receive VP8 packet on first SSRC.
- uint8_t data[kMinRtpPacketLen];
- cricket::RtpHeader rtpHeader;
- rtpHeader.payload_type = GetEngineCodec("VP8").id;
- rtpHeader.seq_num = rtpHeader.timestamp = 0;
- rtpHeader.ssrc = kIncomingUnsignalledSsrc + 1;
- cricket::SetRtpHeader(data, sizeof(data), rtpHeader);
- rtc::CopyOnWriteBuffer packet(data, sizeof(data));
- ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
+ webrtc::RtpPacket rtp_packet;
+ rtp_packet.SetPayloadType(GetEngineCodec("VP8").id);
+ rtp_packet.SetSsrc(kIncomingUnsignalledSsrc + 1);
+ ReceivePacketAndAdvanceTime(rtp_packet.Buffer(), /* packet_time_us */ -1);
// VP8 packet should create default receive stream.
ASSERT_EQ(1u, fake_call_->GetVideoReceiveStreams().size());
FakeVideoReceiveStream* recv_stream = fake_call_->GetVideoReceiveStreams()[0];
- EXPECT_EQ(rtpHeader.ssrc, recv_stream->GetConfig().rtp.remote_ssrc);
+ EXPECT_EQ(rtp_packet.Ssrc(), recv_stream->GetConfig().rtp.remote_ssrc);
// Verify that the receive stream sinks to a renderer.
webrtc::VideoFrame video_frame =
webrtc::VideoFrame::Builder()
@@ -7026,15 +7022,13 @@
EXPECT_EQ(1, renderer.num_rendered_frames());
// Receive VP9 packet on second SSRC.
- rtpHeader.payload_type = GetEngineCodec("VP9").id;
- rtpHeader.ssrc = kIncomingUnsignalledSsrc + 2;
- cricket::SetRtpHeader(data, sizeof(data), rtpHeader);
- rtc::CopyOnWriteBuffer packet2(data, sizeof(data));
- ReceivePacketAndAdvanceTime(packet2, /* packet_time_us */ -1);
+ rtp_packet.SetPayloadType(GetEngineCodec("VP9").id);
+ rtp_packet.SetSsrc(kIncomingUnsignalledSsrc + 2);
+ ReceivePacketAndAdvanceTime(rtp_packet.Buffer(), /* packet_time_us */ -1);
// VP9 packet should replace the default receive SSRC.
ASSERT_EQ(1u, fake_call_->GetVideoReceiveStreams().size());
recv_stream = fake_call_->GetVideoReceiveStreams()[0];
- EXPECT_EQ(rtpHeader.ssrc, recv_stream->GetConfig().rtp.remote_ssrc);
+ EXPECT_EQ(rtp_packet.Ssrc(), recv_stream->GetConfig().rtp.remote_ssrc);
// Verify that the receive stream sinks to a renderer.
webrtc::VideoFrame video_frame2 =
webrtc::VideoFrame::Builder()
@@ -7048,15 +7042,13 @@
#if defined(WEBRTC_USE_H264)
// Receive H264 packet on third SSRC.
- rtpHeader.payload_type = 126;
- rtpHeader.ssrc = kIncomingUnsignalledSsrc + 3;
- cricket::SetRtpHeader(data, sizeof(data), rtpHeader);
- rtc::CopyOnWriteBuffer packet3(data, sizeof(data));
- ReceivePacketAndAdvanceTime(packet3, /* packet_time_us */ -1);
+ rtp_packet.SetPayloadType(126);
+ rtp_packet.SetSsrc(kIncomingUnsignalledSsrc + 3);
+ ReceivePacketAndAdvanceTime(rtp_packet.Buffer(), /* packet_time_us */ -1);
// H264 packet should replace the default receive SSRC.
ASSERT_EQ(1u, fake_call_->GetVideoReceiveStreams().size());
recv_stream = fake_call_->GetVideoReceiveStreams()[0];
- EXPECT_EQ(rtpHeader.ssrc, recv_stream->GetConfig().rtp.remote_ssrc);
+ EXPECT_EQ(rtp_packet.Ssrc(), recv_stream->GetConfig().rtp.remote_ssrc);
// Verify that the receive stream sinks to a renderer.
webrtc::VideoFrame video_frame3 =
webrtc::VideoFrame::Builder()
@@ -7084,14 +7076,10 @@
EXPECT_EQ(0u, fake_call_->GetVideoReceiveStreams().size());
// Receive packet on an unsignaled SSRC.
- uint8_t data[kMinRtpPacketLen];
- cricket::RtpHeader rtp_header;
- rtp_header.payload_type = GetEngineCodec("VP8").id;
- rtp_header.seq_num = rtp_header.timestamp = 0;
- rtp_header.ssrc = kSsrcs3[0];
- cricket::SetRtpHeader(data, sizeof(data), rtp_header);
- rtc::CopyOnWriteBuffer packet(data, sizeof(data));
- ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
+ webrtc::RtpPacket rtp_packet;
+ rtp_packet.SetPayloadType(GetEngineCodec("VP8").id);
+ rtp_packet.SetSsrc(kSsrcs3[0]);
+ ReceivePacketAndAdvanceTime(rtp_packet.Buffer(), /* packet_time_us */ -1);
// Default receive stream should be created.
ASSERT_EQ(1u, fake_call_->GetVideoReceiveStreams().size());
FakeVideoReceiveStream* recv_stream0 =
@@ -7106,10 +7094,8 @@
EXPECT_EQ(kSsrcs3[0], recv_stream0->GetConfig().rtp.remote_ssrc);
// Receive packet on a different unsignaled SSRC.
- rtp_header.ssrc = kSsrcs3[1];
- cricket::SetRtpHeader(data, sizeof(data), rtp_header);
- packet.SetData(data, sizeof(data));
- ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
+ rtp_packet.SetSsrc(kSsrcs3[1]);
+ ReceivePacketAndAdvanceTime(rtp_packet.Buffer(), /* packet_time_us */ -1);
// New default receive stream should be created, but old stream should remain.
ASSERT_EQ(2u, fake_call_->GetVideoReceiveStreams().size());
EXPECT_EQ(recv_stream0, fake_call_->GetVideoReceiveStreams()[0]);
@@ -8715,14 +8701,10 @@
EXPECT_FALSE(rtp_parameters.encodings[0].ssrc);
// Receive VP8 packet.
- uint8_t data[kMinRtpPacketLen];
- cricket::RtpHeader rtpHeader;
- rtpHeader.payload_type = GetEngineCodec("VP8").id;
- rtpHeader.seq_num = rtpHeader.timestamp = 0;
- rtpHeader.ssrc = kIncomingUnsignalledSsrc;
- cricket::SetRtpHeader(data, sizeof(data), rtpHeader);
- rtc::CopyOnWriteBuffer packet(data, sizeof(data));
- ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1);
+ webrtc::RtpPacket rtp_packet;
+ rtp_packet.SetPayloadType(GetEngineCodec("VP8").id);
+ rtp_packet.SetSsrc(kIncomingUnsignalledSsrc);
+ ReceivePacketAndAdvanceTime(rtp_packet.Buffer(), /* packet_time_us */ -1);
// The |ssrc| member should still be unset.
rtp_parameters = channel_->GetDefaultRtpReceiveParameters();
