From 99a71f49c03a3148e29ba5e815ffd5a64afa6e90 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Mon, 19 Jul 2021 13:20:46 +0000 Subject: [PATCH] Move helpers to parse base rtp packet fields to rtp_rtcp module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rtp_rtcp_format is lighter build target than rtc_media_base and a more natural place to keep rtp parsing functions. Bug: None Change-Id: Ibcb5661cc65edbdc89a63f3e411d7ad1218353cc Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/226330 Commit-Queue: Danil Chapovalov Reviewed-by: Niels Moller Reviewed-by: Henrik Boström Cr-Commit-Position: refs/heads/master@{#34504} --- media/BUILD.gn | 1 + media/base/fake_network_interface.h | 17 +- media/base/rtp_utils.cc | 53 ----- media/base/rtp_utils.h | 5 - media/base/rtp_utils_unittest.cc | 28 --- media/engine/fake_webrtc_call.cc | 10 +- media/engine/webrtc_video_engine.cc | 14 +- media/engine/webrtc_video_engine_unittest.cc | 226 +++++++------------ media/engine/webrtc_voice_engine.cc | 8 +- modules/rtp_rtcp/source/rtp_util.cc | 17 ++ modules/rtp_rtcp/source/rtp_util.h | 6 + modules/rtp_rtcp/source/rtp_util_unittest.cc | 31 ++- pc/srtp_session.cc | 9 +- pc/srtp_transport.cc | 15 +- 14 files changed, 167 insertions(+), 273 deletions(-) diff --git a/media/BUILD.gn b/media/BUILD.gn index 5f0f527b8f..0e6b340d4c 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -292,6 +292,7 @@ rtc_library("rtc_audio_video") { "../modules/audio_processing:api", "../modules/audio_processing/aec_dump", "../modules/audio_processing/agc:gain_control_interface", + "../modules/rtp_rtcp:rtp_rtcp_format", "../modules/video_coding", "../modules/video_coding:video_codec_interface", "../modules/video_coding:video_coding_utility", diff --git a/media/base/fake_network_interface.h b/media/base/fake_network_interface.h index 45b7aa0fc0..043e559f28 100644 --- a/media/base/fake_network_interface.h +++ b/media/base/fake_network_interface.h @@ -17,6 +17,7 @@ #include "media/base/media_channel.h" #include "media/base/rtp_utils.h" +#include "modules/rtp_rtcp/source/rtp_util.h" #include "rtc_base/byte_order.h" #include "rtc_base/checks.h" #include "rtc_base/copy_on_write_buffer.h" @@ -116,13 +117,12 @@ class FakeNetworkInterface : public MediaChannel::NetworkInterface, virtual bool SendPacket(rtc::CopyOnWriteBuffer* packet, const rtc::PacketOptions& options) RTC_LOCKS_EXCLUDED(mutex_) { - webrtc::MutexLock lock(&mutex_); - - uint32_t cur_ssrc = 0; - if (!GetRtpSsrc(packet->data(), packet->size(), &cur_ssrc)) { + if (!webrtc::IsRtpPacket(*packet)) { return false; } - sent_ssrcs_[cur_ssrc]++; + + webrtc::MutexLock lock(&mutex_); + sent_ssrcs_[webrtc::ParseRtpSsrc(*packet)]++; options_ = options; rtp_packets_.push_back(*packet); @@ -192,13 +192,8 @@ class FakeNetworkInterface : public MediaChannel::NetworkInterface, if (packets) { *packets = 0; } - uint32_t cur_ssrc = 0; for (size_t i = 0; i < rtp_packets_.size(); ++i) { - if (!GetRtpSsrc(rtp_packets_[i].data(), rtp_packets_[i].size(), - &cur_ssrc)) { - return; - } - if (ssrc == cur_ssrc) { + if (ssrc == webrtc::ParseRtpSsrc(rtp_packets_[i])) { if (bytes) { *bytes += static_cast(rtp_packets_[i].size()); } diff --git a/media/base/rtp_utils.cc b/media/base/rtp_utils.cc index 9f90c468f7..e796482cbb 100644 --- a/media/base/rtp_utils.cc +++ b/media/base/rtp_utils.cc @@ -25,10 +25,6 @@ namespace cricket { -static const size_t kRtpPayloadTypeOffset = 1; -static const size_t kRtpSeqNumOffset = 2; -static const size_t kRtpTimestampOffset = 4; -static const size_t kRtpSsrcOffset = 8; static const size_t kRtcpPayloadTypeOffset = 1; static const size_t kRtpExtensionHeaderLen = 4; static const size_t kAbsSendTimeExtensionLen = 3; @@ -126,57 +122,8 @@ bool GetUint8(const void* data, size_t offset, int* value) { return true; } -bool GetUint16(const void* data, size_t offset, int* value) { - if (!data || !value) { - return false; - } - *value = static_cast( - rtc::GetBE16(static_cast(data) + offset)); - return true; -} - -bool GetUint32(const void* data, size_t offset, uint32_t* value) { - if (!data || !value) { - return false; - } - *value = rtc::GetBE32(static_cast(data) + offset); - return true; -} - } // namespace -bool GetRtpPayloadType(const void* data, size_t len, int* value) { - if (len < kMinRtpPacketLen) { - return false; - } - if (!GetUint8(data, kRtpPayloadTypeOffset, value)) { - return false; - } - *value &= 0x7F; - return true; -} - -bool GetRtpSeqNum(const void* data, size_t len, int* value) { - if (len < kMinRtpPacketLen) { - return false; - } - return GetUint16(data, kRtpSeqNumOffset, value); -} - -bool GetRtpTimestamp(const void* data, size_t len, uint32_t* value) { - if (len < kMinRtpPacketLen) { - return false; - } - return GetUint32(data, kRtpTimestampOffset, value); -} - -bool GetRtpSsrc(const void* data, size_t len, uint32_t* value) { - if (len < kMinRtpPacketLen) { - return false; - } - return GetUint32(data, kRtpSsrcOffset, value); -} - bool GetRtcpType(const void* data, size_t len, int* value) { if (len < kMinRtcpPacketLen) { return false; diff --git a/media/base/rtp_utils.h b/media/base/rtp_utils.h index f6b5dbc9f0..e10403c756 100644 --- a/media/base/rtp_utils.h +++ b/media/base/rtp_utils.h @@ -42,11 +42,6 @@ enum class RtpPacketType { kUnknown, }; -bool GetRtpPayloadType(const void* data, size_t len, int* value); -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 GetRtcpType(const void* data, size_t len, int* value); bool GetRtcpSsrc(const void* data, size_t len, uint32_t* value); diff --git a/media/base/rtp_utils_unittest.cc b/media/base/rtp_utils_unittest.cc index 14599abca2..543babee35 100644 --- a/media/base/rtp_utils_unittest.cc +++ b/media/base/rtp_utils_unittest.cc @@ -21,8 +21,6 @@ namespace cricket { -static const uint8_t kRtpPacketWithMarker[] = { - 0x80, 0x80, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}; static const uint8_t kInvalidPacket[] = {0x80, 0x00}; // PT = 206, FMT = 1, Sender SSRC = 0x1111, Media SSRC = 0x1111 @@ -84,32 +82,6 @@ static const rtc::ArrayView kInvalidPacketArrayView = rtc::MakeArrayView(reinterpret_cast(kInvalidPacket), sizeof(kInvalidPacket)); -TEST(RtpUtilsTest, GetRtp) { - int pt; - EXPECT_TRUE(GetRtpPayloadType(kPcmuFrame, sizeof(kPcmuFrame), &pt)); - EXPECT_EQ(0, pt); - EXPECT_TRUE(GetRtpPayloadType(kRtpPacketWithMarker, - sizeof(kRtpPacketWithMarker), &pt)); - EXPECT_EQ(0, pt); - - int seq_num; - EXPECT_TRUE(GetRtpSeqNum(kPcmuFrame, sizeof(kPcmuFrame), &seq_num)); - EXPECT_EQ(1, seq_num); - - uint32_t ts; - EXPECT_TRUE(GetRtpTimestamp(kPcmuFrame, sizeof(kPcmuFrame), &ts)); - EXPECT_EQ(0u, ts); - - uint32_t ssrc; - EXPECT_TRUE(GetRtpSsrc(kPcmuFrame, sizeof(kPcmuFrame), &ssrc)); - EXPECT_EQ(1u, 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, GetRtcp) { int pt; EXPECT_TRUE(GetRtcpType(kRtcpReport, sizeof(kRtcpReport), &pt)); diff --git a/media/engine/fake_webrtc_call.cc b/media/engine/fake_webrtc_call.cc index e8c7f6e0c9..13401514bd 100644 --- a/media/engine/fake_webrtc_call.cc +++ b/media/engine/fake_webrtc_call.cc @@ -14,12 +14,15 @@ #include "absl/algorithm/container.h" #include "api/call/audio_sink.h" -#include "media/base/rtp_utils.h" +#include "modules/rtp_rtcp/source/rtp_util.h" #include "rtc_base/checks.h" #include "rtc_base/gunit.h" #include "rtc_base/thread.h" namespace cricket { + +using ::webrtc::ParseRtpSsrc; + FakeAudioSendStream::FakeAudioSendStream( int id, const webrtc::AudioSendStream::Config& config) @@ -601,10 +604,11 @@ FakeCall::DeliveryStatus FakeCall::DeliverPacket(webrtc::MediaType media_type, RTC_DCHECK(media_type == webrtc::MediaType::AUDIO || media_type == webrtc::MediaType::VIDEO); - uint32_t ssrc; - if (!GetRtpSsrc(packet.cdata(), packet.size(), &ssrc)) + if (!webrtc::IsRtpPacket(packet)) { return DELIVERY_PACKET_ERROR; + } + uint32_t ssrc = ParseRtpSsrc(packet); if (media_type == webrtc::MediaType::VIDEO) { for (auto receiver : video_receive_streams_) { if (receiver->GetConfig().rtp.remote_ssrc == ssrc) { diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 38a210ee7d..017ce53b52 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -31,6 +31,7 @@ #include "media/engine/simulcast.h" #include "media/engine/webrtc_media_engine.h" #include "media/engine/webrtc_voice_engine.h" +#include "modules/rtp_rtcp/source/rtp_util.h" #include "rtc_base/copy_on_write_buffer.h" #include "rtc_base/experiments/field_trial_parser.h" #include "rtc_base/experiments/field_trial_units.h" @@ -46,6 +47,9 @@ namespace cricket { namespace { +using ::webrtc::ParseRtpPayloadType; +using ::webrtc::ParseRtpSsrc; + const int kMinLayerSize = 16; constexpr int64_t kUnsignaledSsrcCooldownMs = rtc::kNumMillisecsPerSec / 2; @@ -1727,10 +1731,7 @@ void WebRtcVideoChannel::OnPacketReceived(rtc::CopyOnWriteBuffer packet, break; } - uint32_t ssrc = 0; - if (!GetRtpSsrc(packet.cdata(), packet.size(), &ssrc)) { - return; - } + uint32_t ssrc = ParseRtpSsrc(packet); if (unknown_ssrc_packet_buffer_) { unknown_ssrc_packet_buffer_->AddPacket(ssrc, packet_time_us, packet); @@ -1741,10 +1742,7 @@ void WebRtcVideoChannel::OnPacketReceived(rtc::CopyOnWriteBuffer packet, return; } - int payload_type = 0; - if (!GetRtpPayloadType(packet.cdata(), packet.size(), &payload_type)) { - return; - } + int payload_type = ParseRtpPayloadType(packet); // See if this payload_type is registered as one that usually gets its // own SSRC (RTX) or at least is safe to drop either way (FEC). If it diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index d0745e35f5..97764f8583 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -77,8 +77,9 @@ using ::testing::Return; using ::testing::SizeIs; using ::testing::StrNe; using ::testing::Values; -using webrtc::BitrateConstraints; -using webrtc::RtpExtension; +using ::webrtc::BitrateConstraints; +using ::webrtc::RtpExtension; +using ::webrtc::RtpPacket; namespace { static const int kDefaultQpMax = 56; @@ -1425,7 +1426,7 @@ class WebRtcVideoChannelEncodedFrameCallbackTest : public ::testing::Test { } void DeliverKeyFrame(uint32_t ssrc) { - webrtc::RtpPacket packet; + RtpPacket packet; packet.SetMarker(true); packet.SetPayloadType(96); // VP8 packet.SetSsrc(ssrc); @@ -1666,7 +1667,7 @@ class WebRtcVideoChannelBaseTest : public ::testing::Test { return network_interface_.GetRtpPacket(index); } static int GetPayloadType(rtc::CopyOnWriteBuffer p) { - webrtc::RtpPacket header; + RtpPacket header; EXPECT_TRUE(header.Parse(std::move(p))); return header.PayloadType(); } @@ -2060,7 +2061,7 @@ TEST_F(WebRtcVideoChannelBaseTest, SetSendSsrc) { EXPECT_TRUE(SetSend(true)); SendFrame(); EXPECT_TRUE_WAIT(NumRtpPackets() > 0, kTimeout); - webrtc::RtpPacket header; + RtpPacket header; EXPECT_TRUE(header.Parse(GetRtpPacket(0))); EXPECT_EQ(kSsrc, header.Ssrc()); @@ -2084,7 +2085,7 @@ TEST_F(WebRtcVideoChannelBaseTest, SetSendSsrcAfterSetCodecs) { EXPECT_TRUE(SetSend(true)); EXPECT_TRUE(WaitAndSendFrame(0)); EXPECT_TRUE_WAIT(NumRtpPackets() > 0, kTimeout); - webrtc::RtpPacket header; + RtpPacket header; EXPECT_TRUE(header.Parse(GetRtpPacket(0))); EXPECT_EQ(999u, header.Ssrc()); // Packets are being paced out, so these can mismatch between the first and @@ -2099,16 +2100,13 @@ TEST_F(WebRtcVideoChannelBaseTest, SetSendSsrcAfterSetCodecs) { // Test that we can set the default video renderer before and after // media is received. TEST_F(WebRtcVideoChannelBaseTest, SetSink) { - uint8_t data1[] = {0x80, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; - - rtc::CopyOnWriteBuffer packet1(data1, sizeof(data1)); - rtc::SetBE32(packet1.MutableData() + 8, kSsrc); + RtpPacket packet; + packet.SetSsrc(kSsrc); channel_->SetDefaultSink(NULL); EXPECT_TRUE(SetDefaultCodec()); EXPECT_TRUE(SetSend(true)); EXPECT_EQ(0, renderer_.num_rendered_frames()); - channel_->OnPacketReceived(packet1, /* packet_time_us */ -1); + channel_->OnPacketReceived(packet.Buffer(), /* packet_time_us */ -1); channel_->SetDefaultSink(&renderer_); SendFrame(); EXPECT_FRAME_WAIT(1, kVideoWidth, kVideoHeight, kTimeout); @@ -2122,7 +2120,7 @@ TEST_F(WebRtcVideoChannelBaseTest, AddRemoveSendStreams) { SendFrame(); EXPECT_FRAME_WAIT(1, kVideoWidth, kVideoHeight, kTimeout); EXPECT_GT(NumRtpPackets(), 0); - webrtc::RtpPacket header; + RtpPacket header; size_t last_packet = NumRtpPackets() - 1; EXPECT_TRUE(header.Parse(GetRtpPacket(static_cast(last_packet)))); EXPECT_EQ(kSsrc, header.Ssrc()); @@ -6301,12 +6299,9 @@ TEST_F(WebRtcVideoChannelTest, DefaultReceiveStreamReconfiguresToUseRtx) { const std::vector rtx_ssrcs = MAKE_VECTOR(kRtxSsrcs1); ASSERT_EQ(0u, fake_call_->GetVideoReceiveStreams().size()); - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - rtc::SetBE32(&data[8], ssrcs[0]); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetSsrc(ssrcs[0]); + ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1); ASSERT_EQ(1u, fake_call_->GetVideoReceiveStreams().size()) << "No default receive stream created."; @@ -6462,12 +6457,9 @@ TEST_F(WebRtcVideoChannelTest, RecvUnsignaledSsrcWithSignaledStreamId) { EXPECT_EQ(0u, fake_call_->GetVideoReceiveStreams().size()); // Create and deliver packet. - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - rtc::SetBE32(&data[8], kIncomingUnsignalledSsrc); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetSsrc(kIncomingUnsignalledSsrc); + ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1); // The stream should now be created with the appropriate sync label. EXPECT_EQ(1u, fake_call_->GetVideoReceiveStreams().size()); @@ -6482,14 +6474,14 @@ TEST_F(WebRtcVideoChannelTest, RecvUnsignaledSsrcWithSignaledStreamId) { // Until the demuxer criteria has been updated, we ignore in-flight ssrcs of // the recently removed unsignaled receive stream. - ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1); + ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1); EXPECT_EQ(0u, fake_call_->GetVideoReceiveStreams().size()); // After the demuxer criteria has been updated, we should proceed to create // unsignalled receive streams. This time when a default video receive stream // is created it won't have a sync_group. channel_->OnDemuxerCriteriaUpdateComplete(); - ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1); + ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1); EXPECT_EQ(1u, fake_call_->GetVideoReceiveStreams().size()); EXPECT_TRUE( fake_call_->GetVideoReceiveStreams()[0]->GetConfig().sync_group.empty()); @@ -6501,12 +6493,9 @@ TEST_F(WebRtcVideoChannelTest, EXPECT_TRUE(fake_call_->GetVideoReceiveStreams().empty()); // Packet with unsignaled SSRC is received. - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - rtc::SetBE32(&data[8], kIncomingUnsignalledSsrc); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetSsrc(kIncomingUnsignalledSsrc); + ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1); // Default receive stream created. const auto& receivers1 = fake_call_->GetVideoReceiveStreams(); @@ -6551,21 +6540,15 @@ TEST_F(WebRtcVideoChannelTest, // the demuxer is updated. { // Receive a packet for kSsrc1. - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - rtc::SetBE32(&data[8], kSsrc1); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetSsrc(kSsrc1); + ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1); } { // Receive a packet for kSsrc2. - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - rtc::SetBE32(&data[8], kSsrc2); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetSsrc(kSsrc2); + ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1); } // No unsignaled ssrc for kSsrc2 should have been created, but kSsrc1 should @@ -6583,21 +6566,15 @@ TEST_F(WebRtcVideoChannelTest, // Receive packets for kSsrc1 and kSsrc2 again. { // Receive a packet for kSsrc1. - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - rtc::SetBE32(&data[8], kSsrc1); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetSsrc(kSsrc1); + ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1); } { // Receive a packet for kSsrc2. - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - rtc::SetBE32(&data[8], kSsrc2); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetSsrc(kSsrc2); + ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1); } // An unsignalled ssrc for kSsrc2 should be created and the packet counter @@ -6634,21 +6611,15 @@ TEST_F(WebRtcVideoChannelTest, // the demuxer is updated. { // Receive a packet for kSsrc1. - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - rtc::SetBE32(&data[8], kSsrc1); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetSsrc(kSsrc1); + ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1); } { // Receive a packet for kSsrc2. - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - rtc::SetBE32(&data[8], kSsrc2); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetSsrc(kSsrc2); + ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1); } // No unsignaled ssrc for kSsrc1 should have been created, but the packet @@ -6665,21 +6636,15 @@ TEST_F(WebRtcVideoChannelTest, // Receive packets for kSsrc1 and kSsrc2 again. { // Receive a packet for kSsrc1. - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - rtc::SetBE32(&data[8], kSsrc1); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetSsrc(kSsrc1); + ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1); } { // Receive a packet for kSsrc2. - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - rtc::SetBE32(&data[8], kSsrc2); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetSsrc(kSsrc2); + ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1); } // An unsignalled ssrc for kSsrc1 should be created and the packet counter @@ -6712,12 +6677,9 @@ TEST_F(WebRtcVideoChannelTest, MultiplePendingDemuxerCriteriaUpdates) { // In-flight packets should arrive because the stream was recreated, even // though demuxer criteria updates are pending... { - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - rtc::SetBE32(&data[8], kSsrc); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetSsrc(kSsrc); + ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1); } EXPECT_EQ(fake_call_->GetDeliveredPacketsForSsrc(kSsrc), 1u); @@ -6728,12 +6690,9 @@ TEST_F(WebRtcVideoChannelTest, MultiplePendingDemuxerCriteriaUpdates) { // This still should not prevent in-flight packets from arriving because we // have a receive stream for it. { - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - rtc::SetBE32(&data[8], kSsrc); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetSsrc(kSsrc); + ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1); } EXPECT_EQ(fake_call_->GetDeliveredPacketsForSsrc(kSsrc), 2u); @@ -6745,12 +6704,9 @@ TEST_F(WebRtcVideoChannelTest, MultiplePendingDemuxerCriteriaUpdates) { // Now the packet should be dropped and not create an unsignalled receive // stream. { - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - rtc::SetBE32(&data[8], kSsrc); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetSsrc(kSsrc); + ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1); } EXPECT_EQ(fake_call_->GetVideoReceiveStreams().size(), 0u); EXPECT_EQ(fake_call_->GetDeliveredPacketsForSsrc(kSsrc), 2u); @@ -6762,12 +6718,9 @@ TEST_F(WebRtcVideoChannelTest, MultiplePendingDemuxerCriteriaUpdates) { // The packets should continue to be dropped because removal happened after // the most recently completed demuxer update. { - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - rtc::SetBE32(&data[8], kSsrc); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetSsrc(kSsrc); + ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1); } EXPECT_EQ(fake_call_->GetVideoReceiveStreams().size(), 0u); EXPECT_EQ(fake_call_->GetDeliveredPacketsForSsrc(kSsrc), 2u); @@ -6779,12 +6732,9 @@ TEST_F(WebRtcVideoChannelTest, MultiplePendingDemuxerCriteriaUpdates) { // If packets still arrive after the demuxer knows about the latest removal we // should finally create an unsignalled receive stream. { - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - rtc::SetBE32(&data[8], kSsrc); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetSsrc(kSsrc); + ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1); } EXPECT_EQ(fake_call_->GetVideoReceiveStreams().size(), 1u); EXPECT_EQ(fake_call_->GetDeliveredPacketsForSsrc(kSsrc), 3u); @@ -6797,12 +6747,9 @@ TEST_F(WebRtcVideoChannelTest, UnsignalledSsrcHasACooldown) { // Send packets for kSsrc1, creating an unsignalled receive stream. { // Receive a packet for kSsrc1. - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - rtc::SetBE32(&data[8], kSsrc1); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - channel_->OnPacketReceived(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetSsrc(kSsrc1); + channel_->OnPacketReceived(packet.Buffer(), /* packet_time_us */ -1); } rtc::Thread::Current()->ProcessMessages(0); fake_clock_.AdvanceTime( @@ -6815,12 +6762,9 @@ TEST_F(WebRtcVideoChannelTest, UnsignalledSsrcHasACooldown) { { // Receive a packet for kSsrc2. - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - rtc::SetBE32(&data[8], kSsrc2); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - channel_->OnPacketReceived(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetSsrc(kSsrc2); + channel_->OnPacketReceived(packet.Buffer(), /* packet_time_us */ -1); } rtc::Thread::Current()->ProcessMessages(0); @@ -6835,12 +6779,9 @@ TEST_F(WebRtcVideoChannelTest, UnsignalledSsrcHasACooldown) { fake_clock_.AdvanceTime(webrtc::TimeDelta::Millis(1)); { // Receive a packet for kSsrc2. - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - rtc::SetBE32(&data[8], kSsrc2); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - channel_->OnPacketReceived(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetSsrc(kSsrc2); + channel_->OnPacketReceived(packet.Buffer(), /* packet_time_us */ -1); } rtc::Thread::Current()->ProcessMessages(0); @@ -6880,12 +6821,9 @@ TEST_F(WebRtcVideoChannelTest, BaseMinimumPlayoutDelayMsUnsignaledRecvStream) { // Spawn an unsignaled stream by sending a packet, it should inherit // default delay 200. - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - rtc::SetBE32(&data[8], kIncomingUnsignalledSsrc); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetSsrc(kIncomingUnsignalledSsrc); + ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1); recv_stream = fake_call_->GetVideoReceiveStream(kIncomingUnsignalledSsrc); EXPECT_EQ(recv_stream->base_mininum_playout_delay_ms(), 200); @@ -6915,14 +6853,10 @@ void WebRtcVideoChannelTest::TestReceiveUnsignaledSsrcPacket( EXPECT_TRUE(channel_->SetRecvParameters(recv_parameters_)); ASSERT_EQ(0u, fake_call_->GetVideoReceiveStreams().size()); - const size_t kDataLength = 12; - uint8_t data[kDataLength]; - memset(data, 0, sizeof(data)); - - rtc::Set8(data, 1, payload_type); - rtc::SetBE32(&data[8], kIncomingUnsignalledSsrc); - rtc::CopyOnWriteBuffer packet(data, kDataLength); - ReceivePacketAndAdvanceTime(packet, /* packet_time_us */ -1); + RtpPacket packet; + packet.SetPayloadType(payload_type); + packet.SetSsrc(kIncomingUnsignalledSsrc); + ReceivePacketAndAdvanceTime(packet.Buffer(), /* packet_time_us */ -1); if (expect_created_receive_stream) { EXPECT_EQ(1u, fake_call_->GetVideoReceiveStreams().size()) @@ -7002,7 +6936,7 @@ TEST_F(WebRtcVideoChannelTest, ReceiveDifferentUnsignaledSsrc) { channel_->SetDefaultSink(&renderer); // Receive VP8 packet on first SSRC. - webrtc::RtpPacket rtp_packet; + RtpPacket rtp_packet; rtp_packet.SetPayloadType(GetEngineCodec("VP8").id); rtp_packet.SetSsrc(kIncomingUnsignalledSsrc + 1); ReceivePacketAndAdvanceTime(rtp_packet.Buffer(), /* packet_time_us */ -1); @@ -7076,7 +7010,7 @@ TEST_F(WebRtcVideoChannelTest, EXPECT_EQ(0u, fake_call_->GetVideoReceiveStreams().size()); // Receive packet on an unsignaled SSRC. - webrtc::RtpPacket rtp_packet; + RtpPacket rtp_packet; rtp_packet.SetPayloadType(GetEngineCodec("VP8").id); rtp_packet.SetSsrc(kSsrcs3[0]); ReceivePacketAndAdvanceTime(rtp_packet.Buffer(), /* packet_time_us */ -1); @@ -8701,7 +8635,7 @@ TEST_F(WebRtcVideoChannelTest, EXPECT_FALSE(rtp_parameters.encodings[0].ssrc); // Receive VP8 packet. - webrtc::RtpPacket rtp_packet; + RtpPacket rtp_packet; rtp_packet.SetPayloadType(GetEngineCodec("VP8").id); rtp_packet.SetSsrc(kIncomingUnsignalledSsrc); ReceivePacketAndAdvanceTime(rtp_packet.Buffer(), /* packet_time_us */ -1); diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index 7ce61b01f9..a2741f7a7b 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -35,6 +35,7 @@ #include "modules/audio_mixer/audio_mixer_impl.h" #include "modules/audio_processing/aec_dump/aec_dump_factory.h" #include "modules/audio_processing/include/audio_processing.h" +#include "modules/rtp_rtcp/source/rtp_util.h" #include "rtc_base/arraysize.h" #include "rtc_base/byte_order.h" #include "rtc_base/experiments/field_trial_parser.h" @@ -66,6 +67,8 @@ RTC_POP_IGNORING_WUNDEF() namespace cricket { namespace { +using ::webrtc::ParseRtpSsrc; + constexpr size_t kMaxUnsignaledRecvStreams = 4; constexpr int kNackRtpHistoryMs = 5000; @@ -2179,10 +2182,7 @@ void WebRtcVoiceMediaChannel::OnPacketReceived(rtc::CopyOnWriteBuffer packet, // Create an unsignaled receive stream for this previously not received // ssrc. If there already is N unsignaled receive streams, delete the // oldest. See: https://bugs.chromium.org/p/webrtc/issues/detail?id=5208 - uint32_t ssrc = 0; - if (!GetRtpSsrc(packet.cdata(), packet.size(), &ssrc)) { - return; - } + uint32_t ssrc = ParseRtpSsrc(packet); RTC_DCHECK(!absl::c_linear_search(unsignaled_recv_ssrcs_, ssrc)); // Add new stream. diff --git a/modules/rtp_rtcp/source/rtp_util.cc b/modules/rtp_rtcp/source/rtp_util.cc index 46c641ea2f..cf1e54254a 100644 --- a/modules/rtp_rtcp/source/rtp_util.cc +++ b/modules/rtp_rtcp/source/rtp_util.cc @@ -14,6 +14,8 @@ #include #include "api/array_view.h" +#include "modules/rtp_rtcp/source/byte_io.h" +#include "rtc_base/checks.h" namespace webrtc { namespace { @@ -43,4 +45,19 @@ bool IsRtcpPacket(rtc::ArrayView packet) { PayloadTypeIsReservedForRtcp(packet[1] & 0x7F); } +int ParseRtpPayloadType(rtc::ArrayView rtp_packet) { + RTC_DCHECK(IsRtpPacket(rtp_packet)); + return rtp_packet[1] & 0x7F; +} + +uint16_t ParseRtpSequenceNumber(rtc::ArrayView rtp_packet) { + RTC_DCHECK(IsRtpPacket(rtp_packet)); + return ByteReader::ReadBigEndian(rtp_packet.data() + 2); +} + +uint32_t ParseRtpSsrc(rtc::ArrayView rtp_packet) { + RTC_DCHECK(IsRtpPacket(rtp_packet)); + return ByteReader::ReadBigEndian(rtp_packet.data() + 8); +} + } // namespace webrtc diff --git a/modules/rtp_rtcp/source/rtp_util.h b/modules/rtp_rtcp/source/rtp_util.h index b85727bf47..835cfcd6c8 100644 --- a/modules/rtp_rtcp/source/rtp_util.h +++ b/modules/rtp_rtcp/source/rtp_util.h @@ -20,6 +20,12 @@ namespace webrtc { bool IsRtcpPacket(rtc::ArrayView packet); bool IsRtpPacket(rtc::ArrayView packet); +// Returns base rtp header fields of the rtp packet. +// Behaviour is undefined when `!IsRtpPacket(rtp_packet)`. +int ParseRtpPayloadType(rtc::ArrayView rtp_packet); +uint16_t ParseRtpSequenceNumber(rtc::ArrayView rtp_packet); +uint32_t ParseRtpSsrc(rtc::ArrayView rtp_packet); + } // namespace webrtc #endif // MODULES_RTP_RTCP_SOURCE_RTP_UTIL_H_ diff --git a/modules/rtp_rtcp/source/rtp_util_unittest.cc b/modules/rtp_rtcp/source/rtp_util_unittest.cc index 8f980ecff1..3e23416ff4 100644 --- a/modules/rtp_rtcp/source/rtp_util_unittest.cc +++ b/modules/rtp_rtcp/source/rtp_util_unittest.cc @@ -15,7 +15,7 @@ namespace webrtc { namespace { -TEST(RtpUtil, IsRtpPacket) { +TEST(RtpUtilTest, IsRtpPacket) { constexpr uint8_t kMinimalisticRtpPacket[] = {0x80, 97, 0, 0, // 0, 0, 0, 0, // 0, 0, 0, 0}; @@ -39,7 +39,7 @@ TEST(RtpUtil, IsRtpPacket) { EXPECT_FALSE(IsRtpPacket({})); } -TEST(RtpUtil, IsRtcpPacket) { +TEST(RtpUtilTest, IsRtcpPacket) { constexpr uint8_t kMinimalisticRtcpPacket[] = {0x80, 202, 0, 0}; EXPECT_TRUE(IsRtcpPacket(kMinimalisticRtcpPacket)); @@ -55,5 +55,32 @@ TEST(RtpUtil, IsRtcpPacket) { EXPECT_FALSE(IsRtcpPacket({})); } +TEST(RtpUtilTest, ParseRtpPayloadType) { + constexpr uint8_t kMinimalisticRtpPacket[] = {0x80, 97, 0, 0, // + 0, 0, 0, 0, // + 0x12, 0x34, 0x56, 0x78}; + EXPECT_EQ(ParseRtpPayloadType(kMinimalisticRtpPacket), 97); + + constexpr uint8_t kMinimalisticRtpPacketWithMarker[] = { + 0x80, 0x80 | 97, 0, 0, // + 0, 0, 0, 0, // + 0x12, 0x34, 0x56, 0x78}; + EXPECT_EQ(ParseRtpPayloadType(kMinimalisticRtpPacketWithMarker), 97); +} + +TEST(RtpUtilTest, ParseRtpSequenceNumber) { + constexpr uint8_t kMinimalisticRtpPacket[] = {0x80, 97, 0x12, 0x34, // + 0, 0, 0, 0, // + 0, 0, 0, 0}; + EXPECT_EQ(ParseRtpSequenceNumber(kMinimalisticRtpPacket), 0x1234); +} + +TEST(RtpUtilTest, ParseRtpSsrc) { + constexpr uint8_t kMinimalisticRtpPacket[] = {0x80, 97, 0, 0, // + 0, 0, 0, 0, // + 0x12, 0x34, 0x56, 0x78}; + EXPECT_EQ(ParseRtpSsrc(kMinimalisticRtpPacket), 0x12345678u); +} + } // namespace } // namespace webrtc diff --git a/pc/srtp_session.cc b/pc/srtp_session.cc index 45f6b67d12..76ab3a8fe8 100644 --- a/pc/srtp_session.cc +++ b/pc/srtp_session.cc @@ -13,7 +13,8 @@ #include #include "absl/base/attributes.h" -#include "media/base/rtp_utils.h" +#include "api/array_view.h" +#include "modules/rtp_rtcp/source/rtp_util.h" #include "pc/external_hmac.h" #include "rtc_base/logging.h" #include "rtc_base/ssl_stream_adapter.h" @@ -26,6 +27,8 @@ namespace cricket { +using ::webrtc::ParseRtpSequenceNumber; + // One more than the maximum libsrtp error code. Required by // RTC_HISTOGRAM_ENUMERATION. Keep this in sync with srtp_error_status_t defined // in srtp.h. @@ -96,8 +99,8 @@ bool SrtpSession::ProtectRtp(void* p, int in_len, int max_len, int* out_len) { *out_len = in_len; int err = srtp_protect(session_, p, out_len); - int seq_num; - GetRtpSeqNum(p, in_len, &seq_num); + int seq_num = ParseRtpSequenceNumber( + rtc::MakeArrayView(reinterpret_cast(p), in_len)); if (err != srtp_err_status_ok) { RTC_LOG(LS_WARNING) << "Failed to protect SRTP packet, seqnum=" << seq_num << ", err=" << err diff --git a/pc/srtp_transport.cc b/pc/srtp_transport.cc index c90b3fa227..47ba6d673f 100644 --- a/pc/srtp_transport.cc +++ b/pc/srtp_transport.cc @@ -18,6 +18,7 @@ #include "absl/strings/match.h" #include "media/base/rtp_utils.h" +#include "modules/rtp_rtcp/source/rtp_util.h" #include "pc/rtp_transport.h" #include "pc/srtp_session.h" #include "rtc_base/async_packet_socket.h" @@ -160,10 +161,8 @@ bool SrtpTransport::SendRtpPacket(rtc::CopyOnWriteBuffer* packet, } #endif if (!res) { - int seq_num = -1; - uint32_t ssrc = 0; - cricket::GetRtpSeqNum(data, len, &seq_num); - cricket::GetRtpSsrc(data, len, &ssrc); + uint16_t seq_num = ParseRtpSequenceNumber(*packet); + uint32_t ssrc = ParseRtpSsrc(*packet); RTC_LOG(LS_ERROR) << "Failed to protect RTP packet: size=" << len << ", seqnum=" << seq_num << ", SSRC=" << ssrc; return false; @@ -210,17 +209,13 @@ void SrtpTransport::OnRtpPacketReceived(rtc::CopyOnWriteBuffer packet, char* data = packet.MutableData(); int len = rtc::checked_cast(packet.size()); if (!UnprotectRtp(data, len, &len)) { - int seq_num = -1; - uint32_t ssrc = 0; - cricket::GetRtpSeqNum(data, len, &seq_num); - cricket::GetRtpSsrc(data, len, &ssrc); - // Limit the error logging to avoid excessive logs when there are lots of // bad packets. const int kFailureLogThrottleCount = 100; if (decryption_failure_count_ % kFailureLogThrottleCount == 0) { RTC_LOG(LS_ERROR) << "Failed to unprotect RTP packet: size=" << len - << ", seqnum=" << seq_num << ", SSRC=" << ssrc + << ", seqnum=" << ParseRtpSequenceNumber(packet) + << ", SSRC=" << ParseRtpSsrc(packet) << ", previous failure count: " << decryption_failure_count_; }