From f54573bd3b9774db6782af8de0f22242598cb974 Mon Sep 17 00:00:00 2001 From: nisse Date: Wed, 13 Sep 2017 07:13:57 -0700 Subject: [PATCH] Reland of Delete Rtx-related methods from RTPPayloadRegistry. (patchset #1 id:1 of https://codereview.webrtc.org/3011093002/ ) Reason for revert: The cl this change depended on has now been successfully relanded. Original issue's description: > Revert of Delete Rtx-related methods from RTPPayloadRegistry. (patchset #3 id:40001 of https://codereview.webrtc.org/3006993002/ ) > > Reason for revert: > This has to be reverted to enable reverting cl https://codereview.webrtc.org/3006063002/, which seems to have broken ulpfec. > > Original issue's description: > > Delete Rtx-related methods from RTPPayloadRegistry. > > > > Delete methods IsRtx, IsEncapsulated and RestoreOriginalPacket. > > > > BUG=webrtc:7135 > > > > Review-Url: https://codereview.webrtc.org/3006993002 > > Cr-Commit-Position: refs/heads/master@{#19739} > > Committed: https://chromium.googlesource.com/external/webrtc/+/5b4b52264132eefba10bc6880871715f9692da90 > > TBR=stefan@webrtc.org,danilchap@webrtc.org > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG=webrtc:7135 > > Review-Url: https://codereview.webrtc.org/3011093002 > Cr-Commit-Position: refs/heads/master@{#19742} > Committed: https://chromium.googlesource.com/external/webrtc/+/a64685325c2f8f51873b67ae8a91f94ffb19d70b TBR=stefan@webrtc.org,danilchap@webrtc.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=webrtc:7135 Review-Url: https://codereview.webrtc.org/3012253002 Cr-Commit-Position: refs/heads/master@{#19821} --- webrtc/modules/rtp_rtcp/BUILD.gn | 1 + .../rtp_rtcp/include/rtp_payload_registry.h | 12 -- .../rtp_rtcp/source/nack_rtx_unittest.cc | 166 ++++++------------ .../rtp_rtcp/source/rtp_payload_registry.cc | 60 ------- .../source/rtp_payload_registry_unittest.cc | 100 ----------- .../modules/rtp_rtcp/test/testAPI/test_api.cc | 19 -- webrtc/video/rtp_video_stream_receiver.cc | 4 +- 7 files changed, 58 insertions(+), 304 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/BUILD.gn b/webrtc/modules/rtp_rtcp/BUILD.gn index 82bd16d0eb..811dca6a6d 100644 --- a/webrtc/modules/rtp_rtcp/BUILD.gn +++ b/webrtc/modules/rtp_rtcp/BUILD.gn @@ -368,6 +368,7 @@ if (rtc_include_tests) { "../../api:array_view", "../../api:libjingle_peerconnection_api", "../../api:transport_api", + "../../call:rtp_receiver", "../../common_video:common_video", "../../rtc_base:rtc_base_approved", "../../system_wrappers:system_wrappers", diff --git a/webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h b/webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h index a84e2d35cf..4eda28d2ce 100644 --- a/webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h +++ b/webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h @@ -53,20 +53,8 @@ class RTPPayloadRegistry { void SetRtxPayloadType(int payload_type, int associated_payload_type); - bool IsRtx(const RTPHeader& header) const; - - bool RestoreOriginalPacket(uint8_t* restored_packet, - const uint8_t* packet, - size_t* packet_length, - uint32_t original_ssrc, - const RTPHeader& header); - bool IsRed(const RTPHeader& header) const; - // Returns true if the media of this RTP packet is encapsulated within an - // extra header, such as RTX or RED. - bool IsEncapsulated(const RTPHeader& header) const; - bool GetPayloadSpecifics(uint8_t payload_type, PayloadUnion* payload) const; int GetPayloadTypeFrequency(uint8_t payload_type) const; diff --git a/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc b/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc index 32c9d5b9e8..1a68726b4a 100644 --- a/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/nack_rtx_unittest.cc @@ -15,13 +15,13 @@ #include #include "webrtc/api/call/transport.h" +#include "webrtc/call/rtp_stream_receiver_controller.h" +#include "webrtc/call/rtx_receive_stream.h" #include "webrtc/common_types.h" #include "webrtc/modules/rtp_rtcp/include/receive_statistics.h" -#include "webrtc/modules/rtp_rtcp/include/rtp_header_parser.h" -#include "webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h" -#include "webrtc/modules/rtp_rtcp/include/rtp_receiver.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp.h" #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" +#include "webrtc/modules/rtp_rtcp/source/rtp_packet_received.h" #include "webrtc/rtc_base/rate_limiter.h" #include "webrtc/test/gtest.h" @@ -29,6 +29,7 @@ namespace webrtc { const int kVideoNackListSize = 30; const uint32_t kTestSsrc = 3456; +const uint32_t kTestRtxSsrc = kTestSsrc + 1; const uint16_t kTestSequenceNumber = 2345; const uint32_t kTestNumberOfPackets = 1350; const int kTestNumberOfRtxPackets = 149; @@ -37,35 +38,19 @@ const int kPayloadType = 123; const int kRtxPayloadType = 98; const int64_t kMaxRttMs = 1000; -class VerifyingRtxReceiver : public RtpData { +class VerifyingMediaStream : public RtpPacketSinkInterface { public: - VerifyingRtxReceiver() {} + VerifyingMediaStream() {} - int32_t OnReceivedPayloadData( - const uint8_t* data, - size_t size, - const webrtc::WebRtcRTPHeader* rtp_header) override { + void OnRtpPacket(const RtpPacketReceived& packet) override { if (!sequence_numbers_.empty()) - EXPECT_EQ(kTestSsrc, rtp_header->header.ssrc); - sequence_numbers_.push_back(rtp_header->header.sequenceNumber); - return 0; + EXPECT_EQ(kTestSsrc, packet.Ssrc()); + + sequence_numbers_.push_back(packet.SequenceNumber()); } std::list sequence_numbers_; }; -class TestRtpFeedback : public NullRtpFeedback { - public: - explicit TestRtpFeedback(RtpRtcp* rtp_rtcp) : rtp_rtcp_(rtp_rtcp) {} - virtual ~TestRtpFeedback() {} - - void OnIncomingSSRCChanged(uint32_t ssrc) override { - rtp_rtcp_->SetRemoteSSRC(ssrc); - } - - private: - RtpRtcp* rtp_rtcp_; -}; - class RtxLoopBackTransport : public webrtc::Transport { public: explicit RtxLoopBackTransport(uint32_t rtx_ssrc) @@ -75,16 +60,10 @@ class RtxLoopBackTransport : public webrtc::Transport { consecutive_drop_end_(0), rtx_ssrc_(rtx_ssrc), count_rtx_ssrc_(0), - rtp_payload_registry_(NULL), - rtp_receiver_(NULL), module_(NULL) {} - void SetSendModule(RtpRtcp* rtpRtcpModule, - RTPPayloadRegistry* rtp_payload_registry, - RtpReceiver* receiver) { + void SetSendModule(RtpRtcp* rtpRtcpModule) { module_ = rtpRtcpModule; - rtp_payload_registry_ = rtp_payload_registry; - rtp_receiver_ = receiver; } void DropEveryNthPacket(int n) { packet_loss_ = n; } @@ -99,24 +78,15 @@ class RtxLoopBackTransport : public webrtc::Transport { size_t len, const PacketOptions& options) override { count_++; - const unsigned char* ptr = static_cast(data); - uint32_t ssrc = (ptr[8] << 24) + (ptr[9] << 16) + (ptr[10] << 8) + ptr[11]; - if (ssrc == rtx_ssrc_) - count_rtx_ssrc_++; - uint16_t sequence_number = (ptr[2] << 8) + ptr[3]; - size_t packet_length = len; - uint8_t restored_packet[1500]; - RTPHeader header; - std::unique_ptr parser(RtpHeaderParser::Create()); - if (!parser->Parse(ptr, len, &header)) { + RtpPacketReceived packet; + if (!packet.Parse(data, len)) return false; - } - - if (!rtp_payload_registry_->IsRtx(header)) { - // Don't store retransmitted packets since we compare it to the list - // created by the receiver. + if (packet.Ssrc() == rtx_ssrc_) { + count_rtx_ssrc_++; + } else { + // For non-RTX packets only. expected_sequence_numbers_.insert(expected_sequence_numbers_.end(), - sequence_number); + packet.SequenceNumber()); } if (packet_loss_ > 0) { if ((count_ % packet_loss_) == 0) { @@ -126,28 +96,7 @@ class RtxLoopBackTransport : public webrtc::Transport { count_ < consecutive_drop_end_) { return true; } - if (rtp_payload_registry_->IsRtx(header)) { - // Remove the RTX header and parse the original RTP header. - EXPECT_TRUE(rtp_payload_registry_->RestoreOriginalPacket( - restored_packet, ptr, &packet_length, rtp_receiver_->SSRC(), header)); - if (!parser->Parse(restored_packet, packet_length, &header)) { - return false; - } - ptr = restored_packet; - } else { - rtp_payload_registry_->SetIncomingPayloadType(header); - } - - PayloadUnion payload_specific; - if (!rtp_payload_registry_->GetPayloadSpecifics(header.payloadType, - &payload_specific)) { - return false; - } - if (!rtp_receiver_->IncomingRtpPacket(header, ptr + header.headerLength, - packet_length - header.headerLength, - payload_specific, true)) { - return false; - } + EXPECT_TRUE(stream_receiver_controller_.OnRtpPacket(packet)); return true; } @@ -160,9 +109,8 @@ class RtxLoopBackTransport : public webrtc::Transport { int consecutive_drop_end_; uint32_t rtx_ssrc_; int count_rtx_ssrc_; - RTPPayloadRegistry* rtp_payload_registry_; - RtpReceiver* rtp_receiver_; RtpRtcp* module_; + RtpStreamReceiverController stream_receiver_controller_; std::set expected_sequence_numbers_; }; @@ -170,8 +118,10 @@ class RtpRtcpRtxNackTest : public ::testing::Test { protected: RtpRtcpRtxNackTest() : rtp_rtcp_module_(nullptr), - transport_(kTestSsrc + 1), - receiver_(), + transport_(kTestRtxSsrc), + rtx_stream_(&media_stream_, + rtx_associated_payload_types_, + kTestSsrc), payload_data_length(sizeof(payload_data)), fake_clock(123456), retransmission_rate_limiter_(&fake_clock, kMaxRttMs) {} @@ -187,11 +137,6 @@ class RtpRtcpRtxNackTest : public ::testing::Test { configuration.retransmission_rate_limiter = &retransmission_rate_limiter_; rtp_rtcp_module_ = RtpRtcp::CreateRtpRtcp(configuration); - rtp_feedback_.reset(new TestRtpFeedback(rtp_rtcp_module_)); - - rtp_receiver_.reset(RtpReceiver::CreateVideoReceiver( - &fake_clock, &receiver_, rtp_feedback_.get(), &rtp_payload_registry_)); - rtp_rtcp_module_->SetSSRC(kTestSsrc); rtp_rtcp_module_->SetRTCPStatus(RtcpMode::kCompound); rtp_rtcp_module_->SetStorePacketsStatus(true, 600); @@ -199,18 +144,16 @@ class RtpRtcpRtxNackTest : public ::testing::Test { rtp_rtcp_module_->SetSequenceNumber(kTestSequenceNumber); rtp_rtcp_module_->SetStartTimestamp(111111); - transport_.SetSendModule(rtp_rtcp_module_, &rtp_payload_registry_, - rtp_receiver_.get()); + // Used for NACK processing. + // TODO(nisse): Unclear on which side? It's confusing to use a + // single rtp_rtcp module for both send and receive side. + rtp_rtcp_module_->SetRemoteSSRC(kTestSsrc); - VideoCodec video_codec; - memset(&video_codec, 0, sizeof(video_codec)); - video_codec.plType = kPayloadType; - memcpy(video_codec.plName, "I420", 5); - - EXPECT_EQ(0, rtp_rtcp_module_->RegisterSendPayload(video_codec)); + rtp_rtcp_module_->RegisterVideoSendPayload(kPayloadType, "video"); rtp_rtcp_module_->SetRtxSendPayloadType(kRtxPayloadType, kPayloadType); - EXPECT_EQ(0, rtp_payload_registry_.RegisterReceivePayload(video_codec)); - rtp_payload_registry_.SetRtxPayloadType(kRtxPayloadType, kPayloadType); + transport_.SetSendModule(rtp_rtcp_module_); + media_receiver_ = transport_.stream_receiver_controller_.CreateReceiver( + kTestSsrc, &media_stream_); for (size_t n = 0; n < payload_data_length; n++) { payload_data[n] = n % 10; @@ -218,14 +161,14 @@ class RtpRtcpRtxNackTest : public ::testing::Test { } int BuildNackList(uint16_t* nack_list) { - receiver_.sequence_numbers_.sort(); + media_stream_.sequence_numbers_.sort(); std::list missing_sequence_numbers; - std::list::iterator it = receiver_.sequence_numbers_.begin(); + std::list::iterator it = media_stream_.sequence_numbers_.begin(); - while (it != receiver_.sequence_numbers_.end()) { + while (it != media_stream_.sequence_numbers_.end()) { uint16_t sequence_number_1 = *it; ++it; - if (it != receiver_.sequence_numbers_.end()) { + if (it != media_stream_.sequence_numbers_.end()) { uint16_t sequence_number_2 = *it; // Add all missing sequence numbers to list for (uint16_t i = sequence_number_1 + 1; i < sequence_number_2; ++i) { @@ -243,8 +186,8 @@ class RtpRtcpRtxNackTest : public ::testing::Test { bool ExpectedPacketsReceived() { std::list received_sorted; - std::copy(receiver_.sequence_numbers_.begin(), - receiver_.sequence_numbers_.end(), + std::copy(media_stream_.sequence_numbers_.begin(), + media_stream_.sequence_numbers_.end(), std::back_inserter(received_sorted)); received_sorted.sort(); return received_sorted.size() == @@ -254,9 +197,10 @@ class RtpRtcpRtxNackTest : public ::testing::Test { } void RunRtxTest(RtxMode rtx_method, int loss) { - rtp_payload_registry_.SetRtxSsrc(kTestSsrc + 1); + rtx_receiver_ = transport_.stream_receiver_controller_.CreateReceiver( + kTestRtxSsrc, &rtx_stream_); rtp_rtcp_module_->SetRtxSendStatus(rtx_method); - rtp_rtcp_module_->SetRtxSsrc(kTestSsrc + 1); + rtp_rtcp_module_->SetRtxSsrc(kTestRtxSsrc); transport_.DropEveryNthPacket(loss); uint32_t timestamp = 3000; uint16_t nack_list[kVideoNackListSize]; @@ -274,22 +218,24 @@ class RtpRtcpRtxNackTest : public ::testing::Test { // Prepare next frame. timestamp += 3000; } - receiver_.sequence_numbers_.sort(); + media_stream_.sequence_numbers_.sort(); } void TearDown() override { delete rtp_rtcp_module_; } std::unique_ptr receive_statistics_; - RTPPayloadRegistry rtp_payload_registry_; - std::unique_ptr rtp_receiver_; RtpRtcp* rtp_rtcp_module_; - std::unique_ptr rtp_feedback_; RtxLoopBackTransport transport_; - VerifyingRtxReceiver receiver_; + const std::map rtx_associated_payload_types_ = + {{kRtxPayloadType, kPayloadType}}; + VerifyingMediaStream media_stream_; + RtxReceiveStream rtx_stream_; uint8_t payload_data[65000]; size_t payload_data_length; SimulatedClock fake_clock; RateLimiter retransmission_rate_limiter_; + std::unique_ptr media_receiver_; + std::unique_ptr rtx_receiver_; }; TEST_F(RtpRtcpRtxNackTest, LongNackList) { @@ -316,26 +262,26 @@ TEST_F(RtpRtcpRtxNackTest, LongNackList) { rtp_rtcp_module_->Process(); } EXPECT_FALSE(transport_.expected_sequence_numbers_.empty()); - EXPECT_FALSE(receiver_.sequence_numbers_.empty()); - size_t last_receive_count = receiver_.sequence_numbers_.size(); + EXPECT_FALSE(media_stream_.sequence_numbers_.empty()); + size_t last_receive_count = media_stream_.sequence_numbers_.size(); int length = BuildNackList(nack_list); for (int i = 0; i < kNumRequiredRtcp - 1; ++i) { rtp_rtcp_module_->SendNACK(nack_list, length); - EXPECT_GT(receiver_.sequence_numbers_.size(), last_receive_count); - last_receive_count = receiver_.sequence_numbers_.size(); + EXPECT_GT(media_stream_.sequence_numbers_.size(), last_receive_count); + last_receive_count = media_stream_.sequence_numbers_.size(); EXPECT_FALSE(ExpectedPacketsReceived()); } rtp_rtcp_module_->SendNACK(nack_list, length); - EXPECT_GT(receiver_.sequence_numbers_.size(), last_receive_count); + EXPECT_GT(media_stream_.sequence_numbers_.size(), last_receive_count); EXPECT_TRUE(ExpectedPacketsReceived()); } TEST_F(RtpRtcpRtxNackTest, RtxNack) { RunRtxTest(kRtxRetransmitted, 10); - EXPECT_EQ(kTestSequenceNumber, *(receiver_.sequence_numbers_.begin())); + EXPECT_EQ(kTestSequenceNumber, *(media_stream_.sequence_numbers_.begin())); EXPECT_EQ(kTestSequenceNumber + kTestNumberOfPackets - 1, - *(receiver_.sequence_numbers_.rbegin())); - EXPECT_EQ(kTestNumberOfPackets, receiver_.sequence_numbers_.size()); + *(media_stream_.sequence_numbers_.rbegin())); + EXPECT_EQ(kTestNumberOfPackets, media_stream_.sequence_numbers_.size()); EXPECT_EQ(kTestNumberOfRtxPackets, transport_.count_rtx_ssrc_); EXPECT_TRUE(ExpectedPacketsReceived()); } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc index 35616b6122..fe2bc804e4 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry.cc @@ -14,7 +14,6 @@ #include "webrtc/common_types.h" #include "webrtc/modules/audio_coding/codecs/audio_format_conversion.h" -#include "webrtc/modules/rtp_rtcp/source/byte_io.h" #include "webrtc/rtc_base/checks.h" #include "webrtc/rtc_base/logging.h" #include "webrtc/rtc_base/stringutils.h" @@ -270,65 +269,10 @@ bool RTPPayloadRegistry::RtxEnabled() const { return rtx_; } -bool RTPPayloadRegistry::IsRtx(const RTPHeader& header) const { - rtc::CritScope cs(&crit_sect_); - return IsRtxInternal(header); -} - bool RTPPayloadRegistry::IsRtxInternal(const RTPHeader& header) const { return rtx_ && ssrc_rtx_ == header.ssrc; } -bool RTPPayloadRegistry::RestoreOriginalPacket(uint8_t* restored_packet, - const uint8_t* packet, - size_t* packet_length, - uint32_t original_ssrc, - const RTPHeader& header) { - if (kRtxHeaderSize + header.headerLength + header.paddingLength > - *packet_length) { - return false; - } - const uint8_t* rtx_header = packet + header.headerLength; - uint16_t original_sequence_number = (rtx_header[0] << 8) + rtx_header[1]; - - // Copy the packet into the restored packet, except for the RTX header. - memcpy(restored_packet, packet, header.headerLength); - memcpy(restored_packet + header.headerLength, - packet + header.headerLength + kRtxHeaderSize, - *packet_length - header.headerLength - kRtxHeaderSize); - *packet_length -= kRtxHeaderSize; - - // Replace the SSRC and the sequence number with the originals. - ByteWriter::WriteBigEndian(restored_packet + 2, - original_sequence_number); - ByteWriter::WriteBigEndian(restored_packet + 8, original_ssrc); - - rtc::CritScope cs(&crit_sect_); - if (!rtx_) - return true; - - auto apt_mapping = rtx_payload_type_map_.find(header.payloadType); - if (apt_mapping == rtx_payload_type_map_.end()) { - // No associated payload type found. Warn, unless we have already done so. - if (payload_types_with_suppressed_warnings_.find(header.payloadType) == - payload_types_with_suppressed_warnings_.end()) { - LOG(LS_WARNING) - << "No RTX associated payload type mapping was available; " - "not able to restore original packet from RTX packet " - "with payload type: " - << static_cast(header.payloadType) << ". " - << "Suppressing further warnings for this payload type."; - payload_types_with_suppressed_warnings_.insert(header.payloadType); - } - return false; - } - restored_packet[1] = static_cast(apt_mapping->second); - if (header.markerBit) { - restored_packet[1] |= kRtpMarkerBitMask; // Marker bit is set. - } - return true; -} - void RTPPayloadRegistry::SetRtxSsrc(uint32_t ssrc) { rtc::CritScope cs(&crit_sect_); ssrc_rtx_ = ssrc; @@ -359,10 +303,6 @@ bool RTPPayloadRegistry::IsRed(const RTPHeader& header) const { return it != payload_type_map_.end() && _stricmp(it->second.name, "red") == 0; } -bool RTPPayloadRegistry::IsEncapsulated(const RTPHeader& header) const { - return IsRed(header) || IsRtx(header); -} - bool RTPPayloadRegistry::GetPayloadSpecifics(uint8_t payload_type, PayloadUnion* payload) const { rtc::CritScope cs(&crit_sect_); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc index f5707d226c..6e17eb9aca 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_payload_registry_unittest.cc @@ -13,7 +13,6 @@ #include "webrtc/common_types.h" #include "webrtc/modules/rtp_rtcp/include/rtp_header_parser.h" #include "webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h" -#include "webrtc/modules/rtp_rtcp/source/byte_io.h" #include "webrtc/modules/rtp_rtcp/source/rtp_utility.h" #include "webrtc/test/gmock.h" #include "webrtc/test/gtest.h" @@ -259,105 +258,6 @@ TEST_P(RtpPayloadRegistryGenericTest, RegisterGenericReceivePayloadType) { rtp_payload_registry.RegisterReceivePayload(audio_codec, &ignored)); } -// Generates an RTX packet for the given length and original sequence number. -// The RTX sequence number and ssrc will use the default value of 9999. The -// caller takes ownership of the returned buffer. -const uint8_t* GenerateRtxPacket(size_t header_length, - size_t payload_length, - uint16_t original_sequence_number) { - uint8_t* packet = - new uint8_t[kRtxHeaderSize + header_length + payload_length](); - // Write the RTP version to the first byte, so the resulting header can be - // parsed. - static const int kRtpExpectedVersion = 2; - packet[0] = static_cast(kRtpExpectedVersion << 6); - // Write a junk sequence number. It should be thrown away when the packet is - // restored. - ByteWriter::WriteBigEndian(packet + 2, 9999); - // Write a junk ssrc. It should also be thrown away when the packet is - // restored. - ByteWriter::WriteBigEndian(packet + 8, 9999); - - // Now write the RTX header. It occurs at the start of the payload block, and - // contains just the sequence number. - ByteWriter::WriteBigEndian(packet + header_length, - original_sequence_number); - return packet; -} - -void TestRtxPacket(RTPPayloadRegistry* rtp_payload_registry, - int rtx_payload_type, - int expected_payload_type, - bool should_succeed) { - size_t header_length = 100; - size_t payload_length = 200; - size_t original_length = header_length + payload_length + kRtxHeaderSize; - - RTPHeader header; - header.ssrc = 1000; - header.sequenceNumber = 100; - header.payloadType = rtx_payload_type; - header.headerLength = header_length; - - uint16_t original_sequence_number = 1234; - uint32_t original_ssrc = 500; - - std::unique_ptr packet(GenerateRtxPacket( - header_length, payload_length, original_sequence_number)); - std::unique_ptr restored_packet( - new uint8_t[header_length + payload_length]); - size_t length = original_length; - bool success = rtp_payload_registry->RestoreOriginalPacket( - restored_packet.get(), packet.get(), &length, original_ssrc, header); - EXPECT_EQ(should_succeed, success) - << "Test success should match should_succeed."; - if (!success) { - return; - } - - EXPECT_EQ(original_length - kRtxHeaderSize, length) - << "The restored packet should be exactly kRtxHeaderSize smaller."; - - std::unique_ptr header_parser(RtpHeaderParser::Create()); - RTPHeader restored_header; - ASSERT_TRUE( - header_parser->Parse(restored_packet.get(), length, &restored_header)); - EXPECT_EQ(original_sequence_number, restored_header.sequenceNumber) - << "The restored packet should have the original sequence number " - << "in the correct location in the RTP header."; - EXPECT_EQ(expected_payload_type, restored_header.payloadType) - << "The restored packet should have the correct payload type."; - EXPECT_EQ(original_ssrc, restored_header.ssrc) - << "The restored packet should have the correct ssrc."; -} - -TEST(RtpPayloadRegistryTest, MultipleRtxPayloadTypes) { - RTPPayloadRegistry rtp_payload_registry; - // Set the incoming payload type to 90. - RTPHeader header; - header.payloadType = 90; - header.ssrc = 1; - rtp_payload_registry.SetIncomingPayloadType(header); - rtp_payload_registry.SetRtxSsrc(100); - // Map two RTX payload types. - rtp_payload_registry.SetRtxPayloadType(105, 95); - rtp_payload_registry.SetRtxPayloadType(106, 96); - - TestRtxPacket(&rtp_payload_registry, 105, 95, true); - TestRtxPacket(&rtp_payload_registry, 106, 96, true); -} - -TEST(RtpPayloadRegistryTest, InvalidRtxConfiguration) { - RTPPayloadRegistry rtp_payload_registry; - rtp_payload_registry.SetRtxSsrc(100); - // Fails because no mappings exist and the incoming payload type isn't known. - TestRtxPacket(&rtp_payload_registry, 105, 0, false); - // Succeeds when the mapping is used, but fails for the implicit fallback. - rtp_payload_registry.SetRtxPayloadType(105, 95); - TestRtxPacket(&rtp_payload_registry, 105, 95, true); - TestRtxPacket(&rtp_payload_registry, 106, 0, false); -} - INSTANTIATE_TEST_CASE_P(TestDynamicRange, RtpPayloadRegistryGenericTest, testing::Range(96, 127 + 1)); diff --git a/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc b/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc index b39c8d72da..28be2225b2 100644 --- a/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc +++ b/webrtc/modules/rtp_rtcp/test/testAPI/test_api.cc @@ -162,23 +162,4 @@ TEST_F(RtpRtcpAPITest, RtxSender) { EXPECT_EQ(kRtxRetransmitted, module_->RtxSendStatus()); } -TEST_F(RtpRtcpAPITest, RtxReceiver) { - const uint32_t kRtxSsrc = 1; - const int kRtxPayloadType = 119; - const int kPayloadType = 100; - EXPECT_FALSE(rtp_payload_registry_->RtxEnabled()); - rtp_payload_registry_->SetRtxSsrc(kRtxSsrc); - rtp_payload_registry_->SetRtxPayloadType(kRtxPayloadType, kPayloadType); - EXPECT_TRUE(rtp_payload_registry_->RtxEnabled()); - RTPHeader rtx_header; - rtx_header.ssrc = kRtxSsrc; - rtx_header.payloadType = kRtxPayloadType; - EXPECT_TRUE(rtp_payload_registry_->IsRtx(rtx_header)); - rtx_header.ssrc = 0; - EXPECT_FALSE(rtp_payload_registry_->IsRtx(rtx_header)); - rtx_header.ssrc = kRtxSsrc; - rtx_header.payloadType = 0; - EXPECT_TRUE(rtp_payload_registry_->IsRtx(rtx_header)); -} - } // namespace webrtc diff --git a/webrtc/video/rtp_video_stream_receiver.cc b/webrtc/video/rtp_video_stream_receiver.cc index 1dbd86988d..5aff22633b 100644 --- a/webrtc/video/rtp_video_stream_receiver.cc +++ b/webrtc/video/rtp_video_stream_receiver.cc @@ -453,7 +453,7 @@ void RtpVideoStreamReceiver::ReceivePacket(const uint8_t* packet, size_t packet_length, const RTPHeader& header, bool in_order) { - if (rtp_payload_registry_.IsEncapsulated(header)) { + if (rtp_payload_registry_.IsRed(header)) { ParseAndHandleEncapsulatingHeader(packet, packet_length, header); return; } @@ -485,8 +485,6 @@ void RtpVideoStreamReceiver::ParseAndHandleEncapsulatingHeader( return; } ulpfec_receiver_->ProcessReceivedFec(); - } else if (rtp_payload_registry_.IsRtx(header)) { - LOG(LS_WARNING) << "Unexpected RTX packet on media ssrc"; } }