From 70e4053844d06377aaa2142a64b90037d74bbcf9 Mon Sep 17 00:00:00 2001 From: brandtr Date: Wed, 21 Dec 2016 00:22:03 -0800 Subject: [PATCH] Revert of Parse FlexFEC RTP headers in Call and add integration with BWE. (patchset #17 id:460001 of https://codereview.webrtc.org/2553863003/ ) Reason for revert: Unexpected perf regressions. Original issue's description: > Parse FlexFEC RTP headers in Call and add integration with BWE. > > BUG=webrtc:5654 > > Review-Url: https://codereview.webrtc.org/2553863003 > Cr-Commit-Position: refs/heads/master@{#15709} > Committed: https://chromium.googlesource.com/external/webrtc/+/ab2ffa3b28b55ef359232723049fb88b2dcd807a TBR=philipel@webrtc.org,stefan@webrtc.org,danilchap@webrtc.org,nisse@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=webrtc:5654 Review-Url: https://codereview.webrtc.org/2589393002 Cr-Commit-Position: refs/heads/master@{#15727} --- webrtc/call/call.cc | 121 +++--------------- webrtc/call/flexfec_receive_stream.h | 2 +- webrtc/call/flexfec_receive_stream_impl.cc | 25 ++-- webrtc/call/flexfec_receive_stream_impl.h | 7 +- .../call/flexfec_receive_stream_unittest.cc | 41 +++--- webrtc/media/engine/webrtcvideoengine2.cc | 2 +- .../congestion_controller.cc | 17 --- .../congestion_controller_unittest.cc | 40 ------ .../include/congestion_controller.h | 4 - .../include/mock/mock_congestion_controller.h | 5 - .../rtp_rtcp/include/flexfec_receiver.h | 10 +- .../rtp_rtcp/source/flexfec_receiver.cc | 56 ++++---- .../source/flexfec_receiver_unittest.cc | 120 +++++++++-------- webrtc/modules/rtp_rtcp/source/rtp_packet.cc | 4 - webrtc/modules/rtp_rtcp/source/rtp_packet.h | 1 - .../test/fuzzers/flexfec_receiver_fuzzer.cc | 6 +- 16 files changed, 166 insertions(+), 295 deletions(-) diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc index c7998b2cd8..1c4b7b2a71 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc @@ -24,7 +24,6 @@ #include "webrtc/base/checks.h" #include "webrtc/base/constructormagic.h" #include "webrtc/base/logging.h" -#include "webrtc/base/optional.h" #include "webrtc/base/task_queue.h" #include "webrtc/base/thread_annotations.h" #include "webrtc/base/thread_checker.h" @@ -40,8 +39,6 @@ #include "webrtc/modules/rtp_rtcp/include/flexfec_receiver.h" #include "webrtc/modules/rtp_rtcp/include/rtp_header_parser.h" #include "webrtc/modules/rtp_rtcp/source/byte_io.h" -#include "webrtc/modules/rtp_rtcp/source/rtp_header_extension.h" -#include "webrtc/modules/rtp_rtcp/source/rtp_packet_received.h" #include "webrtc/modules/utility/include/process_thread.h" #include "webrtc/system_wrappers/include/clock.h" #include "webrtc/system_wrappers/include/cpu_info.h" @@ -110,8 +107,6 @@ class Call : public webrtc::Call, // Implements RecoveredPacketReceiver. bool OnRecoveredPacket(const uint8_t* packet, size_t length) override; - void NotifyBweOfReceivedPacket(const RtpPacketReceived& packet); - void SetBitrateConfig( const webrtc::Call::Config::BitrateConfig& bitrate_config) override; @@ -159,11 +154,6 @@ class Call : public webrtc::Call, return nullptr; } - rtc::Optional ParseRtpPacket(const uint8_t* packet, - size_t length, - const PacketTime& packet_time) - SHARED_LOCKS_REQUIRED(receive_crit_); - void UpdateSendHistograms() EXCLUSIVE_LOCKS_REQUIRED(&bitrate_crit_); void UpdateReceiveHistograms(); void UpdateHistograms(); @@ -202,14 +192,6 @@ class Call : public webrtc::Call, std::map sync_stream_mapping_ GUARDED_BY(receive_crit_); - // Registered RTP header extensions for each stream. - // Note that RTP header extensions are negotiated per track ("m= line") in the - // SDP, but we have no notion of tracks at the Call level. We therefore store - // the RTP header extensions per SSRC instead, which leads to some storage - // overhead. - std::map received_rtp_header_extensions_ - GUARDED_BY(receive_crit_); - std::unique_ptr send_crit_; // Audio and Video send streams are owned by the client that creates them. std::map audio_send_ssrcs_ GUARDED_BY(send_crit_); @@ -363,29 +345,6 @@ Call::~Call() { Trace::ReturnTrace(); } -rtc::Optional Call::ParseRtpPacket( - const uint8_t* packet, - size_t length, - const PacketTime& packet_time) { - RtpPacketReceived parsed_packet; - if (!parsed_packet.Parse(packet, length)) - return rtc::Optional(); - - auto it = received_rtp_header_extensions_.find(parsed_packet.Ssrc()); - if (it != received_rtp_header_extensions_.end()) - parsed_packet.IdentifyExtensions(it->second); - - int64_t arrival_time_ms; - if (packet_time.timestamp != -1) { - arrival_time_ms = (packet_time.timestamp + 500) / 1000; - } else { - arrival_time_ms = clock_->TimeInMilliseconds(); - } - parsed_packet.set_arrival_time_ms(arrival_time_ms); - - return rtc::Optional(std::move(parsed_packet)); -} - void Call::UpdateHistograms() { RTC_HISTOGRAM_COUNTS_100000( "WebRTC.Call.LifetimeInSeconds", @@ -700,40 +659,25 @@ FlexfecReceiveStream* Call::CreateFlexfecReceiveStream( const FlexfecReceiveStream::Config& config) { TRACE_EVENT0("webrtc", "Call::CreateFlexfecReceiveStream"); RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread()); - - RecoveredPacketReceiver* recovered_packet_receiver = this; FlexfecReceiveStreamImpl* receive_stream = - new FlexfecReceiveStreamImpl(config, recovered_packet_receiver); + new FlexfecReceiveStreamImpl(config, this); { WriteLockScoped write_lock(*receive_crit_); - - RTC_DCHECK(flexfec_receive_streams_.find(receive_stream) == - flexfec_receive_streams_.end()); - flexfec_receive_streams_.insert(receive_stream); - for (auto ssrc : config.protected_media_ssrcs) flexfec_receive_ssrcs_media_.insert(std::make_pair(ssrc, receive_stream)); - RTC_DCHECK(flexfec_receive_ssrcs_protection_.find(config.remote_ssrc) == flexfec_receive_ssrcs_protection_.end()); flexfec_receive_ssrcs_protection_[config.remote_ssrc] = receive_stream; - - RTC_DCHECK(received_rtp_header_extensions_.find(config.remote_ssrc) == - received_rtp_header_extensions_.end()); - RtpHeaderExtensionMap rtp_header_extensions(config.rtp_header_extensions); - received_rtp_header_extensions_[config.remote_ssrc] = rtp_header_extensions; + flexfec_receive_streams_.insert(receive_stream); } - // TODO(brandtr): Store config in RtcEventLog here. - return receive_stream; } void Call::DestroyFlexfecReceiveStream(FlexfecReceiveStream* receive_stream) { TRACE_EVENT0("webrtc", "Call::DestroyFlexfecReceiveStream"); RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread()); - RTC_DCHECK(receive_stream != nullptr); // There exist no other derived classes of FlexfecReceiveStream, // so this downcast is safe. @@ -741,19 +685,8 @@ void Call::DestroyFlexfecReceiveStream(FlexfecReceiveStream* receive_stream) { static_cast(receive_stream); { WriteLockScoped write_lock(*receive_crit_); - - uint32_t ssrc = receive_stream_impl->GetConfig().remote_ssrc; - received_rtp_header_extensions_.erase(ssrc); - // Remove all SSRCs pointing to the FlexfecReceiveStreamImpl to be // destroyed. - auto prot_it = flexfec_receive_ssrcs_protection_.begin(); - while (prot_it != flexfec_receive_ssrcs_protection_.end()) { - if (prot_it->second == receive_stream_impl) - prot_it = flexfec_receive_ssrcs_protection_.erase(prot_it); - else - ++prot_it; - } auto media_it = flexfec_receive_ssrcs_media_.begin(); while (media_it != flexfec_receive_ssrcs_media_.end()) { if (media_it->second == receive_stream_impl) @@ -761,10 +694,15 @@ void Call::DestroyFlexfecReceiveStream(FlexfecReceiveStream* receive_stream) { else ++media_it; } - + auto prot_it = flexfec_receive_ssrcs_protection_.begin(); + while (prot_it != flexfec_receive_ssrcs_protection_.end()) { + if (prot_it->second == receive_stream_impl) + prot_it = flexfec_receive_ssrcs_protection_.erase(prot_it); + else + ++prot_it; + } flexfec_receive_streams_.erase(receive_stream_impl); } - delete receive_stream_impl; } @@ -1138,21 +1076,13 @@ PacketReceiver::DeliveryStatus Call::DeliverRtp(MediaType media_type, if (it != video_receive_ssrcs_.end()) { received_bytes_per_second_counter_.Add(static_cast(length)); received_video_bytes_per_second_counter_.Add(static_cast(length)); - // TODO(brandtr): Notify the BWE of received media packets here. auto status = it->second->DeliverRtp(packet, length, packet_time) ? DELIVERY_OK : DELIVERY_PACKET_ERROR; - // Deliver media packets to FlexFEC subsystem. RTP header extensions need - // not be parsed, as FlexFEC is oblivious to the semantic meaning of the - // packet contents beyond the 12 byte RTP base header. The BWE is fed - // information about these media packets from the regular media pipeline. - rtc::Optional parsed_packet = - ParseRtpPacket(packet, length, packet_time); - if (parsed_packet) { - auto it_bounds = flexfec_receive_ssrcs_media_.equal_range(ssrc); - for (auto it = it_bounds.first; it != it_bounds.second; ++it) - it->second->AddAndProcessReceivedPacket(*parsed_packet); - } + // Deliver media packets to FlexFEC subsystem. + auto it_bounds = flexfec_receive_ssrcs_media_.equal_range(ssrc); + for (auto it = it_bounds.first; it != it_bounds.second; ++it) + it->second->AddAndProcessReceivedPacket(packet, length); if (status == DELIVERY_OK) event_log_->LogRtpHeader(kIncomingPacket, media_type, packet, length); return status; @@ -1161,18 +1091,12 @@ PacketReceiver::DeliveryStatus Call::DeliverRtp(MediaType media_type, if (media_type == MediaType::ANY || media_type == MediaType::VIDEO) { auto it = flexfec_receive_ssrcs_protection_.find(ssrc); if (it != flexfec_receive_ssrcs_protection_.end()) { - rtc::Optional parsed_packet = - ParseRtpPacket(packet, length, packet_time); - if (parsed_packet) { - NotifyBweOfReceivedPacket(*parsed_packet); - auto status = - it->second->AddAndProcessReceivedPacket(std::move(*parsed_packet)) - ? DELIVERY_OK - : DELIVERY_PACKET_ERROR; - if (status == DELIVERY_OK) - event_log_->LogRtpHeader(kIncomingPacket, media_type, packet, length); - return status; - } + auto status = it->second->AddAndProcessReceivedPacket(packet, length) + ? DELIVERY_OK + : DELIVERY_PACKET_ERROR; + if (status == DELIVERY_OK) + event_log_->LogRtpHeader(kIncomingPacket, media_type, packet, length); + return status; } } return DELIVERY_UNKNOWN_SSRC; @@ -1204,12 +1128,5 @@ bool Call::OnRecoveredPacket(const uint8_t* packet, size_t length) { return it->second->OnRecoveredPacket(packet, length); } -void Call::NotifyBweOfReceivedPacket(const RtpPacketReceived& packet) { - RTPHeader header; - packet.GetHeader(&header); - congestion_controller_->OnReceivedPacket(packet.arrival_time_ms(), - packet.payload_size(), header); -} - } // namespace internal } // namespace webrtc diff --git a/webrtc/call/flexfec_receive_stream.h b/webrtc/call/flexfec_receive_stream.h index 20a57f2c15..83b212bad6 100644 --- a/webrtc/call/flexfec_receive_stream.h +++ b/webrtc/call/flexfec_receive_stream.h @@ -59,7 +59,7 @@ class FlexfecReceiveStream { bool transport_cc = false; // RTP header extensions that have been negotiated for this track. - std::vector rtp_header_extensions; + std::vector extensions; }; // Starts stream activity. diff --git a/webrtc/call/flexfec_receive_stream_impl.cc b/webrtc/call/flexfec_receive_stream_impl.cc index 13390a12c0..e0e49c1a2e 100644 --- a/webrtc/call/flexfec_receive_stream_impl.cc +++ b/webrtc/call/flexfec_receive_stream_impl.cc @@ -10,8 +10,6 @@ #include "webrtc/call/flexfec_receive_stream_impl.h" -#include - #include "webrtc/base/checks.h" #include "webrtc/base/logging.h" @@ -36,12 +34,12 @@ std::string FlexfecReceiveStream::Config::ToString() const { if (!protected_media_ssrcs.empty()) ss << protected_media_ssrcs[i]; ss << "], transport_cc: " << (transport_cc ? "on" : "off"); - ss << ", rtp_header_extensions: ["; + ss << ", extensions: ["; i = 0; - for (; i + 1 < rtp_header_extensions.size(); ++i) - ss << rtp_header_extensions[i].ToString() << ", "; - if (!rtp_header_extensions.empty()) - ss << rtp_header_extensions[i].ToString(); + for (; i + 1 < extensions.size(); ++i) + ss << extensions[i].ToString() << ", "; + if (!extensions.empty()) + ss << extensions[i].ToString(); ss << "]}"; return ss.str(); } @@ -51,7 +49,7 @@ namespace { // TODO(brandtr): Update this function when we support multistream protection. std::unique_ptr MaybeCreateFlexfecReceiver( const FlexfecReceiveStream::Config& config, - RecoveredPacketReceiver* recovered_packet_receiver) { + RecoveredPacketReceiver* recovered_packet_callback) { if (config.payload_type < 0) { LOG(LS_WARNING) << "Invalid FlexFEC payload type given. " << "This FlexfecReceiveStream will therefore be useless."; @@ -81,18 +79,18 @@ std::unique_ptr MaybeCreateFlexfecReceiver( RTC_DCHECK_EQ(1U, config.protected_media_ssrcs.size()); return std::unique_ptr( new FlexfecReceiver(config.remote_ssrc, config.protected_media_ssrcs[0], - recovered_packet_receiver)); + recovered_packet_callback)); } } // namespace FlexfecReceiveStreamImpl::FlexfecReceiveStreamImpl( const Config& config, - RecoveredPacketReceiver* recovered_packet_receiver) + RecoveredPacketReceiver* recovered_packet_callback) : started_(false), config_(config), receiver_( - MaybeCreateFlexfecReceiver(config_, recovered_packet_receiver)) { + MaybeCreateFlexfecReceiver(config_, recovered_packet_callback)) { LOG(LS_INFO) << "FlexfecReceiveStreamImpl: " << config_.ToString(); } @@ -102,7 +100,8 @@ FlexfecReceiveStreamImpl::~FlexfecReceiveStreamImpl() { } bool FlexfecReceiveStreamImpl::AddAndProcessReceivedPacket( - RtpPacketReceived packet) { + const uint8_t* packet, + size_t packet_length) { { rtc::CritScope cs(&crit_); if (!started_) @@ -110,7 +109,7 @@ bool FlexfecReceiveStreamImpl::AddAndProcessReceivedPacket( } if (!receiver_) return false; - return receiver_->AddAndProcessReceivedPacket(std::move(packet)); + return receiver_->AddAndProcessReceivedPacket(packet, packet_length); } void FlexfecReceiveStreamImpl::Start() { diff --git a/webrtc/call/flexfec_receive_stream_impl.h b/webrtc/call/flexfec_receive_stream_impl.h index 7267dc0e45..79c6cc0687 100644 --- a/webrtc/call/flexfec_receive_stream_impl.h +++ b/webrtc/call/flexfec_receive_stream_impl.h @@ -18,19 +18,16 @@ #include "webrtc/base/criticalsection.h" #include "webrtc/call/flexfec_receive_stream.h" #include "webrtc/modules/rtp_rtcp/include/flexfec_receiver.h" -#include "webrtc/modules/rtp_rtcp/source/rtp_packet_received.h" namespace webrtc { class FlexfecReceiveStreamImpl : public FlexfecReceiveStream { public: FlexfecReceiveStreamImpl(const Config& config, - RecoveredPacketReceiver* recovered_packet_receiver); + RecoveredPacketReceiver* recovered_packet_callback); ~FlexfecReceiveStreamImpl() override; - const Config& GetConfig() const { return config_; } - - bool AddAndProcessReceivedPacket(RtpPacketReceived packet); + bool AddAndProcessReceivedPacket(const uint8_t* packet, size_t length); // Implements FlexfecReceiveStream. void Start() override; diff --git a/webrtc/call/flexfec_receive_stream_unittest.cc b/webrtc/call/flexfec_receive_stream_unittest.cc index 07d3943257..314d9c0efe 100644 --- a/webrtc/call/flexfec_receive_stream_unittest.cc +++ b/webrtc/call/flexfec_receive_stream_unittest.cc @@ -8,36 +8,24 @@ * be found in the AUTHORS file in the root of the source tree. */ -#include "webrtc/base/array_view.h" #include "webrtc/base/basictypes.h" #include "webrtc/call/flexfec_receive_stream_impl.h" #include "webrtc/modules/rtp_rtcp/include/flexfec_receiver.h" #include "webrtc/modules/rtp_rtcp/source/byte_io.h" -#include "webrtc/modules/rtp_rtcp/source/rtp_packet_received.h" #include "webrtc/modules/rtp_rtcp/mocks/mock_recovered_packet_receiver.h" #include "webrtc/test/gmock.h" #include "webrtc/test/gtest.h" namespace webrtc { -namespace { - -RtpPacketReceived ParsePacket(rtc::ArrayView packet) { - RtpPacketReceived parsed_packet(nullptr); - EXPECT_TRUE(parsed_packet.Parse(packet)); - return parsed_packet; -} - -} // namespace - TEST(FlexfecReceiveStreamTest, ConstructDestruct) { FlexfecReceiveStream::Config config; config.payload_type = 118; config.remote_ssrc = 424223; config.protected_media_ssrcs = {912512}; - MockRecoveredPacketReceiver recovered_packet_receiver; + MockRecoveredPacketReceiver callback; - FlexfecReceiveStreamImpl receive_stream(config, &recovered_packet_receiver); + FlexfecReceiveStreamImpl receive_stream(config, &callback); } TEST(FlexfecReceiveStreamTest, StartStop) { @@ -45,13 +33,27 @@ TEST(FlexfecReceiveStreamTest, StartStop) { config.payload_type = 118; config.remote_ssrc = 1652392; config.protected_media_ssrcs = {23300443}; - MockRecoveredPacketReceiver recovered_packet_receiver; - FlexfecReceiveStreamImpl receive_stream(config, &recovered_packet_receiver); + MockRecoveredPacketReceiver callback; + FlexfecReceiveStreamImpl receive_stream(config, &callback); receive_stream.Start(); receive_stream.Stop(); } +TEST(FlexfecReceiveStreamTest, DoesNotProcessPacketWhenNoMediaSsrcGiven) { + FlexfecReceiveStream::Config config; + config.payload_type = 118; + config.remote_ssrc = 424223; + config.protected_media_ssrcs = {}; + MockRecoveredPacketReceiver callback; + FlexfecReceiveStreamImpl receive_stream(config, &callback); + const uint8_t packet[] = {0x00, 0x11, 0x22, 0x33}; + const size_t packet_length = sizeof(packet); + + EXPECT_FALSE( + receive_stream.AddAndProcessReceivedPacket(packet, packet_length)); +} + // Create a FlexFEC packet that protects a single media packet and ensure // that the callback is called. Correctness of recovery is checked in the // FlexfecReceiver unit tests. @@ -89,6 +91,7 @@ TEST(FlexfecReceiveStreamTest, RecoversPacketWhenStarted) { // FEC payload. kPayloadBits, kPayloadBits, kPayloadBits, kPayloadBits}; // clang-format on + constexpr size_t kFlexfecPacketLength = sizeof(kFlexfecPacket); FlexfecReceiveStream::Config config; config.payload_type = kFlexfecPlType; @@ -99,14 +102,16 @@ TEST(FlexfecReceiveStreamTest, RecoversPacketWhenStarted) { FlexfecReceiveStreamImpl receive_stream(config, &recovered_packet_receiver); // Do not call back before being started. - receive_stream.AddAndProcessReceivedPacket(ParsePacket(kFlexfecPacket)); + receive_stream.AddAndProcessReceivedPacket(kFlexfecPacket, + kFlexfecPacketLength); // Call back after being started. receive_stream.Start(); EXPECT_CALL( recovered_packet_receiver, OnRecoveredPacket(::testing::_, kRtpHeaderSize + kPayloadLength[1])); - receive_stream.AddAndProcessReceivedPacket(ParsePacket(kFlexfecPacket)); + receive_stream.AddAndProcessReceivedPacket(kFlexfecPacket, + kFlexfecPacketLength); } } // namespace webrtc diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index 129eb3288a..9d9f14120f 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -2349,7 +2349,7 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::RecreateWebRtcStream() { // RTCP messages and RTP header extensions apply to the entire track // in the SDP. config.transport_cc = config_.rtp.transport_cc; - config.rtp_header_extensions = config_.rtp.extensions; + config.extensions = config_.rtp.extensions; flexfec_stream_ = call_->CreateFlexfecReceiveStream(config); flexfec_stream_->Start(); } diff --git a/webrtc/modules/congestion_controller/congestion_controller.cc b/webrtc/modules/congestion_controller/congestion_controller.cc index 2fae264156..bbdd968dc6 100644 --- a/webrtc/modules/congestion_controller/congestion_controller.cc +++ b/webrtc/modules/congestion_controller/congestion_controller.cc @@ -197,23 +197,6 @@ CongestionController::CongestionController( CongestionController::~CongestionController() {} -void CongestionController::OnReceivedPacket(int64_t arrival_time_ms, - size_t payload_size, - const RTPHeader& header) { - // Send-side BWE. - if (header.extension.hasTransportSequenceNumber) { - remote_estimator_proxy_.IncomingPacket(arrival_time_ms, payload_size, - header); - return; - } - - // Receive-side BWE. - if (remote_bitrate_estimator_) { - remote_bitrate_estimator_->IncomingPacket(arrival_time_ms, payload_size, - header); - } -} - void CongestionController::SetBweBitrates(int min_bitrate_bps, int start_bitrate_bps, int max_bitrate_bps) { diff --git a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc index 3850797b9e..bdbaaf1cce 100644 --- a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc +++ b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc @@ -26,18 +26,6 @@ using testing::Return; using testing::SaveArg; using testing::StrictMock; -namespace { - -// Helper to convert some time format to resolution used in absolute send time -// header extension, rounded upwards. |t| is the time to convert, in some -// resolution. |denom| is the value to divide |t| by to get whole seconds, -// e.g. |denom| = 1000 if |t| is in milliseconds. -uint32_t AbsSendTime(int64_t t, int64_t denom) { - return (((t << 18) + (denom >> 1)) / denom) & 0x00fffffful; -} - -} // namespace - namespace webrtc { namespace test { @@ -213,33 +201,5 @@ TEST_F(CongestionControllerTest, GetProbingInterval) { controller_->Process(); } -TEST_F(CongestionControllerTest, OnReceivedPacketWithAbsSendTime) { - NiceMock observer; - StrictMock remote_bitrate_observer; - std::unique_ptr pacer(new NiceMock()); - controller_.reset( - new CongestionController(&clock_, &observer, &remote_bitrate_observer, - &event_log_, &packet_router_, std::move(pacer))); - - size_t payload_size = 1000; - RTPHeader header; - header.ssrc = 0x11eb21c; - header.extension.hasAbsoluteSendTime = true; - - std::vector ssrcs; - EXPECT_CALL(remote_bitrate_observer, OnReceiveBitrateChanged(_, _)) - .WillRepeatedly(SaveArg<0>(&ssrcs)); - - for (int i = 0; i < 10; ++i) { - clock_.AdvanceTimeMilliseconds((1000 * payload_size) / kInitialBitrateBps); - int64_t now_ms = clock_.TimeInMilliseconds(); - header.extension.absoluteSendTime = AbsSendTime(now_ms, 1000); - controller_->OnReceivedPacket(now_ms, payload_size, header); - } - - ASSERT_EQ(1u, ssrcs.size()); - EXPECT_EQ(header.ssrc, ssrcs[0]); -} - } // namespace test } // namespace webrtc diff --git a/webrtc/modules/congestion_controller/include/congestion_controller.h b/webrtc/modules/congestion_controller/include/congestion_controller.h index 48b9531260..004a84752e 100644 --- a/webrtc/modules/congestion_controller/include/congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/congestion_controller.h @@ -68,10 +68,6 @@ class CongestionController : public CallStatsObserver, public Module { std::unique_ptr pacer); virtual ~CongestionController(); - virtual void OnReceivedPacket(int64_t arrival_time_ms, - size_t payload_size, - const RTPHeader& header); - virtual void SetBweBitrates(int min_bitrate_bps, int start_bitrate_bps, int max_bitrate_bps); diff --git a/webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h b/webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h index aefb2be7dc..1b932dc867 100644 --- a/webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/mock/mock_congestion_controller.h @@ -13,7 +13,6 @@ #include "webrtc/base/constructormagic.h" #include "webrtc/base/socket.h" -#include "webrtc/common_types.h" #include "webrtc/modules/congestion_controller/include/congestion_controller.h" #include "webrtc/test/gmock.h" @@ -45,10 +44,6 @@ class MockCongestionController : public CongestionController { remote_bitrate_observer, event_log, packet_router) {} - MOCK_METHOD3(OnReceivedPacket, - void(int64_t arrival_time_ms, - size_t payload_size, - const RTPHeader& header)); MOCK_METHOD3(SetBweBitrates, void(int min_bitrate_bps, int start_bitrate_bps, diff --git a/webrtc/modules/rtp_rtcp/include/flexfec_receiver.h b/webrtc/modules/rtp_rtcp/include/flexfec_receiver.h index 9ad093133c..721d6c592f 100644 --- a/webrtc/modules/rtp_rtcp/include/flexfec_receiver.h +++ b/webrtc/modules/rtp_rtcp/include/flexfec_receiver.h @@ -16,9 +16,9 @@ #include "webrtc/base/basictypes.h" #include "webrtc/base/sequenced_task_checker.h" #include "webrtc/call/call.h" +#include "webrtc/modules/rtp_rtcp/include/flexfec_receiver.h" #include "webrtc/modules/rtp_rtcp/include/ulpfec_receiver.h" #include "webrtc/modules/rtp_rtcp/source/forward_error_correction.h" -#include "webrtc/modules/rtp_rtcp/source/rtp_packet_received.h" #include "webrtc/system_wrappers/include/clock.h" namespace webrtc { @@ -37,19 +37,19 @@ class FlexfecReceiver { public: FlexfecReceiver(uint32_t ssrc, uint32_t protected_media_ssrc, - RecoveredPacketReceiver* recovered_packet_receiver); + RecoveredPacketReceiver* callback); ~FlexfecReceiver(); // Inserts a received packet (can be either media or FlexFEC) into the // internal buffer, and sends the received packets to the erasure code. // All newly recovered packets are sent back through the callback. - bool AddAndProcessReceivedPacket(RtpPacketReceived packet); + bool AddAndProcessReceivedPacket(const uint8_t* packet, size_t packet_length); // Returns a counter describing the added and recovered packets. FecPacketCounter GetPacketCounter() const; private: - bool AddReceivedPacket(RtpPacketReceived packet); + bool AddReceivedPacket(const uint8_t* packet, size_t packet_length); bool ProcessReceivedPackets(); // Config. @@ -63,7 +63,7 @@ class FlexfecReceiver { GUARDED_BY(sequence_checker_); ForwardErrorCorrection::RecoveredPacketList recovered_packets_ GUARDED_BY(sequence_checker_); - RecoveredPacketReceiver* const recovered_packet_receiver_; + RecoveredPacketReceiver* const callback_ GUARDED_BY(sequence_checker_); // Logging and stats. Clock* const clock_; diff --git a/webrtc/modules/rtp_rtcp/source/flexfec_receiver.cc b/webrtc/modules/rtp_rtcp/source/flexfec_receiver.cc index 204ce84d2a..369dd31ae1 100644 --- a/webrtc/modules/rtp_rtcp/source/flexfec_receiver.cc +++ b/webrtc/modules/rtp_rtcp/source/flexfec_receiver.cc @@ -10,8 +10,11 @@ #include "webrtc/modules/rtp_rtcp/include/flexfec_receiver.h" +#include + #include "webrtc/base/logging.h" #include "webrtc/base/scoped_ref_ptr.h" +#include "webrtc/modules/rtp_rtcp/source/rtp_packet_received.h" namespace webrtc { @@ -28,14 +31,13 @@ constexpr int kPacketLogIntervalMs = 10000; } // namespace -FlexfecReceiver::FlexfecReceiver( - uint32_t ssrc, - uint32_t protected_media_ssrc, - RecoveredPacketReceiver* recovered_packet_receiver) +FlexfecReceiver::FlexfecReceiver(uint32_t ssrc, + uint32_t protected_media_ssrc, + RecoveredPacketReceiver* callback) : ssrc_(ssrc), protected_media_ssrc_(protected_media_ssrc), erasure_code_(ForwardErrorCorrection::CreateFlexfec()), - recovered_packet_receiver_(recovered_packet_receiver), + callback_(callback), clock_(Clock::GetRealTimeClock()), last_recovered_packet_ms_(-1) { // It's OK to create this object on a different thread/task queue than @@ -45,9 +47,10 @@ FlexfecReceiver::FlexfecReceiver( FlexfecReceiver::~FlexfecReceiver() = default; -bool FlexfecReceiver::AddAndProcessReceivedPacket(RtpPacketReceived packet) { +bool FlexfecReceiver::AddAndProcessReceivedPacket(const uint8_t* packet, + size_t packet_length) { RTC_DCHECK_CALLED_SEQUENTIALLY(&sequence_checker_); - if (!AddReceivedPacket(std::move(packet))) { + if (!AddReceivedPacket(packet, packet_length)) { return false; } return ProcessReceivedPackets(); @@ -58,32 +61,42 @@ FecPacketCounter FlexfecReceiver::GetPacketCounter() const { return packet_counter_; } -bool FlexfecReceiver::AddReceivedPacket(RtpPacketReceived packet) { +bool FlexfecReceiver::AddReceivedPacket(const uint8_t* packet, + size_t packet_length) { RTC_DCHECK_CALLED_SEQUENTIALLY(&sequence_checker_); // RTP packets with a full base header (12 bytes), but without payload, // could conceivably be useful in the decoding. Therefore we check - // with a non-strict inequality here. - RTC_DCHECK_GE(packet.size(), kRtpHeaderSize); + // with a strict inequality here. + if (packet_length < kRtpHeaderSize) { + LOG(LS_WARNING) << "Truncated packet, discarding."; + return false; + } + + // TODO(brandtr): Consider how to handle received FlexFEC packets and + // the bandwidth estimator. + RtpPacketReceived parsed_packet; + if (!parsed_packet.Parse(packet, packet_length)) { + return false; + } // Demultiplex based on SSRC, and insert into erasure code decoder. std::unique_ptr received_packet(new ReceivedPacket()); - received_packet->seq_num = packet.SequenceNumber(); - received_packet->ssrc = packet.Ssrc(); + received_packet->seq_num = parsed_packet.SequenceNumber(); + received_packet->ssrc = parsed_packet.Ssrc(); if (received_packet->ssrc == ssrc_) { - // This is a FlexFEC packet. - if (packet.payload_size() < kMinFlexfecHeaderSize) { + // This is a FEC packet belonging to this FlexFEC stream. + if (parsed_packet.payload_size() < kMinFlexfecHeaderSize) { LOG(LS_WARNING) << "Truncated FlexFEC packet, discarding."; return false; } received_packet->is_fec = true; ++packet_counter_.num_fec_packets; - // Insert packet payload into erasure code. // TODO(brandtr): Remove this memcpy when the FEC packet classes // are using COW buffers internally. received_packet->pkt = rtc::scoped_refptr(new Packet()); - auto payload = packet.payload(); + auto payload = parsed_packet.payload(); memcpy(received_packet->pkt->data, payload.data(), payload.size()); received_packet->pkt->length = payload.size(); } else { @@ -93,14 +106,13 @@ bool FlexfecReceiver::AddReceivedPacket(RtpPacketReceived packet) { return false; } received_packet->is_fec = false; - // Insert entire packet into erasure code. // TODO(brandtr): Remove this memcpy too. received_packet->pkt = rtc::scoped_refptr(new Packet()); - memcpy(received_packet->pkt->data, packet.data(), packet.size()); - received_packet->pkt->length = packet.size(); + memcpy(received_packet->pkt->data, parsed_packet.data(), + parsed_packet.size()); + received_packet->pkt->length = parsed_packet.size(); } - received_packets_.push_back(std::move(received_packet)); ++packet_counter_.num_packets; @@ -132,8 +144,8 @@ bool FlexfecReceiver::ProcessReceivedPackets() { continue; } ++packet_counter_.num_recovered_packets; - if (!recovered_packet_receiver_->OnRecoveredPacket( - recovered_packet->pkt->data, recovered_packet->pkt->length)) { + if (!callback_->OnRecoveredPacket(recovered_packet->pkt->data, + recovered_packet->pkt->length)) { return false; } recovered_packet->returned = true; diff --git a/webrtc/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc index 8bf0f3701f..8b46878e2d 100644 --- a/webrtc/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc @@ -16,7 +16,6 @@ #include "webrtc/modules/rtp_rtcp/mocks/mock_recovered_packet_receiver.h" #include "webrtc/modules/rtp_rtcp/source/fec_test_helper.h" #include "webrtc/modules/rtp_rtcp/source/forward_error_correction.h" -#include "webrtc/modules/rtp_rtcp/source/rtp_packet_received.h" #include "webrtc/test/gmock.h" #include "webrtc/test/gtest.h" @@ -37,12 +36,6 @@ constexpr size_t kPayloadLength = 500; constexpr uint32_t kFlexfecSsrc = 42984; constexpr uint32_t kMediaSsrc = 8353; -RtpPacketReceived ParsePacket(const Packet& packet) { - RtpPacketReceived parsed_packet; - EXPECT_TRUE(parsed_packet.Parse(packet.data, packet.length)); - return parsed_packet; -} - } // namespace class FlexfecReceiverTest : public ::testing::Test { @@ -100,8 +93,21 @@ TEST_F(FlexfecReceiverTest, ReceivesMediaPacket) { std::unique_ptr media_packet( packet_generator_.NextPacket(0, kPayloadLength)); - EXPECT_TRUE( - receiver_.AddAndProcessReceivedPacket(ParsePacket(*media_packet))); + EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(media_packet->data, + media_packet->length)); +} + +TEST_F(FlexfecReceiverTest, FailsOnTruncatedMediaPacket) { + const size_t kNoPayload = 0; + + packet_generator_.NewFrame(1); + std::unique_ptr media_packet( + packet_generator_.NextPacket(0, kNoPayload)); + // Simulate truncated media packet. + media_packet->length = kRtpHeaderSize - 1; + + EXPECT_FALSE(receiver_.AddAndProcessReceivedPacket(media_packet->data, + media_packet->length)); } TEST_F(FlexfecReceiverTest, ReceivesMediaAndFecPackets) { @@ -111,12 +117,13 @@ TEST_F(FlexfecReceiverTest, ReceivesMediaAndFecPackets) { PacketList media_packets; PacketizeFrame(kNumMediaPackets, 0, &media_packets); std::list fec_packets = EncodeFec(media_packets, kNumFecPackets); - const auto& media_packet = media_packets.front(); + auto media_packet = media_packets.front().get(); auto fec_packet = packet_generator_.BuildFlexfecPacket(*fec_packets.front()); - EXPECT_TRUE( - receiver_.AddAndProcessReceivedPacket(ParsePacket(*media_packet))); - EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(*fec_packet))); + EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(media_packet->data, + media_packet->length)); + EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(fec_packet->data, + fec_packet->length)); } TEST_F(FlexfecReceiverTest, FailsOnTruncatedFecPacket) { @@ -126,14 +133,15 @@ TEST_F(FlexfecReceiverTest, FailsOnTruncatedFecPacket) { PacketList media_packets; PacketizeFrame(kNumMediaPackets, 0, &media_packets); std::list fec_packets = EncodeFec(media_packets, kNumFecPackets); - const auto& media_packet = media_packets.front(); + auto media_packet = media_packets.front().get(); // Simulate truncated FlexFEC payload. fec_packets.front()->length = 1; auto fec_packet = packet_generator_.BuildFlexfecPacket(*fec_packets.front()); - EXPECT_TRUE( - receiver_.AddAndProcessReceivedPacket(ParsePacket(*media_packet))); - EXPECT_FALSE(receiver_.AddAndProcessReceivedPacket(ParsePacket(*fec_packet))); + EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(media_packet->data, + media_packet->length)); + EXPECT_FALSE(receiver_.AddAndProcessReceivedPacket(fec_packet->data, + fec_packet->length)); } TEST_F(FlexfecReceiverTest, FailsOnUnknownMediaSsrc) { @@ -141,15 +149,15 @@ TEST_F(FlexfecReceiverTest, FailsOnUnknownMediaSsrc) { PacketList media_packets; PacketizeFrame(kNumMediaPackets, 0, &media_packets); - auto& media_packet = media_packets.front(); + auto media_packet = media_packets.front().get(); // Corrupt the SSRC. media_packet->data[8] = 0; media_packet->data[9] = 1; media_packet->data[10] = 2; media_packet->data[11] = 3; - EXPECT_FALSE( - receiver_.AddAndProcessReceivedPacket(ParsePacket(*media_packet))); + EXPECT_FALSE(receiver_.AddAndProcessReceivedPacket(media_packet->data, + media_packet->length)); } TEST_F(FlexfecReceiverTest, FailsOnUnknownFecSsrc) { @@ -159,7 +167,7 @@ TEST_F(FlexfecReceiverTest, FailsOnUnknownFecSsrc) { PacketList media_packets; PacketizeFrame(kNumMediaPackets, 0, &media_packets); std::list fec_packets = EncodeFec(media_packets, kNumFecPackets); - const auto& media_packet = media_packets.front(); + auto media_packet = media_packets.front().get(); auto fec_packet = packet_generator_.BuildFlexfecPacket(*fec_packets.front()); // Corrupt the SSRC. fec_packet->data[8] = 4; @@ -167,9 +175,10 @@ TEST_F(FlexfecReceiverTest, FailsOnUnknownFecSsrc) { fec_packet->data[10] = 6; fec_packet->data[11] = 7; - EXPECT_TRUE( - receiver_.AddAndProcessReceivedPacket(ParsePacket(*media_packet))); - EXPECT_FALSE(receiver_.AddAndProcessReceivedPacket(ParsePacket(*fec_packet))); + EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(media_packet->data, + media_packet->length)); + EXPECT_FALSE(receiver_.AddAndProcessReceivedPacket(fec_packet->data, + fec_packet->length)); } TEST_F(FlexfecReceiverTest, ReceivesMultiplePackets) { @@ -182,8 +191,8 @@ TEST_F(FlexfecReceiverTest, ReceivesMultiplePackets) { // Receive all media packets. for (const auto& media_packet : media_packets) { - EXPECT_TRUE( - receiver_.AddAndProcessReceivedPacket(ParsePacket(*media_packet))); + EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(media_packet->data, + media_packet->length)); } // Receive FEC packet. @@ -191,7 +200,7 @@ TEST_F(FlexfecReceiverTest, ReceivesMultiplePackets) { std::unique_ptr packet_with_rtp_header = packet_generator_.BuildFlexfecPacket(*fec_packet); EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket( - ParsePacket(*packet_with_rtp_header))); + packet_with_rtp_header->data, packet_with_rtp_header->length)); } TEST_F(FlexfecReceiverTest, RecoversFromSingleMediaLoss) { @@ -204,7 +213,8 @@ TEST_F(FlexfecReceiverTest, RecoversFromSingleMediaLoss) { // Receive first media packet but drop second. auto media_it = media_packets.begin(); - EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it))); + EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_it)->data, + (*media_it)->length)); // Receive FEC packet and ensure recovery of lost media packet. auto fec_it = fec_packets.begin(); @@ -217,7 +227,7 @@ TEST_F(FlexfecReceiverTest, RecoversFromSingleMediaLoss) { Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length))) .WillOnce(Return(true)); EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket( - ParsePacket(*packet_with_rtp_header))); + packet_with_rtp_header->data, packet_with_rtp_header->length)); } TEST_F(FlexfecReceiverTest, RecoversFromDoubleMediaLoss) { @@ -241,7 +251,7 @@ TEST_F(FlexfecReceiverTest, RecoversFromDoubleMediaLoss) { Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length))) .WillOnce(Return(true)); EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket( - ParsePacket(*packet_with_rtp_header))); + packet_with_rtp_header->data, packet_with_rtp_header->length)); // Receive second FEC packet and recover second lost media packet. fec_it++; @@ -253,7 +263,7 @@ TEST_F(FlexfecReceiverTest, RecoversFromDoubleMediaLoss) { Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length))) .WillOnce(Return(true)); EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket( - ParsePacket(*packet_with_rtp_header))); + packet_with_rtp_header->data, packet_with_rtp_header->length)); } TEST_F(FlexfecReceiverTest, DoesNotRecoverFromMediaAndFecLoss) { @@ -266,7 +276,8 @@ TEST_F(FlexfecReceiverTest, DoesNotRecoverFromMediaAndFecLoss) { // Receive first media packet. auto media_it = media_packets.begin(); - EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it))); + EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_it)->data, + (*media_it)->length)); // Drop second media packet and FEC packet. Do not expect call back. } @@ -281,7 +292,8 @@ TEST_F(FlexfecReceiverTest, DoesNotCallbackTwice) { // Receive first media packet but drop second. auto media_it = media_packets.begin(); - EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it))); + EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_it)->data, + (*media_it)->length)); // Receive FEC packet and ensure recovery of lost media packet. auto fec_it = fec_packets.begin(); @@ -294,11 +306,11 @@ TEST_F(FlexfecReceiverTest, DoesNotCallbackTwice) { Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length))) .WillOnce(Return(true)); EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket( - ParsePacket(*packet_with_rtp_header))); + packet_with_rtp_header->data, packet_with_rtp_header->length)); // Receive FEC packet again. EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket( - ParsePacket(*packet_with_rtp_header))); + packet_with_rtp_header->data, packet_with_rtp_header->length)); // Do not call back again. } @@ -320,7 +332,8 @@ TEST_F(FlexfecReceiverTest, RecoversFrom50PercentLoss) { // Drop every second media packet. auto media_it = media_packets.begin(); while (media_it != media_packets.end()) { - EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it))); + EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_it)->data, + (*media_it)->length)); ++media_it; if (media_it == media_packets.end()) { break; @@ -343,7 +356,7 @@ TEST_F(FlexfecReceiverTest, RecoversFrom50PercentLoss) { ElementsAreArray((*media_it)->data, (*media_it)->length))) .WillOnce(Return(true)); EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket( - ParsePacket(*fec_packet_with_rtp_header))); + fec_packet_with_rtp_header->data, fec_packet_with_rtp_header->length)); ++media_it; } } @@ -370,7 +383,8 @@ TEST_F(FlexfecReceiverTest, DelayedFecPacketDoesHelp) { // Receive all other media packets. while (media_it != media_packets.end()) { - EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it))); + EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_it)->data, + (*media_it)->length)); ++media_it; } @@ -385,7 +399,7 @@ TEST_F(FlexfecReceiverTest, DelayedFecPacketDoesHelp) { Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length))) .WillOnce(Return(true)); EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket( - ParsePacket(*packet_with_rtp_header))); + packet_with_rtp_header->data, packet_with_rtp_header->length)); } TEST_F(FlexfecReceiverTest, TooDelayedFecPacketDoesNotHelp) { @@ -410,7 +424,8 @@ TEST_F(FlexfecReceiverTest, TooDelayedFecPacketDoesNotHelp) { // Receive all other media packets. while (media_it != media_packets.end()) { - EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it))); + EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_it)->data, + (*media_it)->length)); ++media_it; } @@ -419,7 +434,7 @@ TEST_F(FlexfecReceiverTest, TooDelayedFecPacketDoesNotHelp) { std::unique_ptr packet_with_rtp_header = packet_generator_.BuildFlexfecPacket(**fec_it); EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket( - ParsePacket(*packet_with_rtp_header))); + packet_with_rtp_header->data, packet_with_rtp_header->length)); // Do not expect a call back. } @@ -440,14 +455,14 @@ TEST_F(FlexfecReceiverTest, RecoversWithMediaPacketsOutOfOrder) { auto media_packet3 = media_it++; auto media_packet4 = media_it++; auto media_packet5 = media_it++; - EXPECT_TRUE( - receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_packet5))); - EXPECT_TRUE( - receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_packet2))); - EXPECT_TRUE( - receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_packet3))); - EXPECT_TRUE( - receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_packet0))); + EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_packet5)->data, + (*media_packet5)->length)); + EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_packet2)->data, + (*media_packet2)->length)); + EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_packet3)->data, + (*media_packet3)->length)); + EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_packet0)->data, + (*media_packet0)->length)); // Expect to recover lost media packets. EXPECT_CALL(recovered_packet_receiver_, @@ -467,7 +482,7 @@ TEST_F(FlexfecReceiverTest, RecoversWithMediaPacketsOutOfOrder) { while (fec_it != fec_packets.end()) { packet_with_rtp_header = packet_generator_.BuildFlexfecPacket(**fec_it); EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket( - ParsePacket(*packet_with_rtp_header))); + packet_with_rtp_header->data, packet_with_rtp_header->length)); ++fec_it; } } @@ -482,7 +497,8 @@ TEST_F(FlexfecReceiverTest, CalculatesNumberOfPackets) { // Receive first media packet but drop second. auto media_it = media_packets.begin(); - EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it))); + EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket((*media_it)->data, + (*media_it)->length)); // Receive FEC packet and ensure recovery of lost media packet. auto fec_it = fec_packets.begin(); @@ -495,7 +511,7 @@ TEST_F(FlexfecReceiverTest, CalculatesNumberOfPackets) { Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length))) .WillOnce(Return(true)); EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket( - ParsePacket(*packet_with_rtp_header))); + packet_with_rtp_header->data, packet_with_rtp_header->length)); // Check stats calculations. FecPacketCounter packet_counter = receiver_.GetPacketCounter(); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_packet.cc b/webrtc/modules/rtp_rtcp/source/rtp_packet.cc index e720eebc4a..b7d71c2789 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_packet.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_packet.cc @@ -86,10 +86,6 @@ bool Packet::Parse(const uint8_t* buffer, size_t buffer_size) { return true; } -bool Packet::Parse(rtc::ArrayView packet) { - return Parse(packet.data(), packet.size()); -} - bool Packet::Parse(rtc::CopyOnWriteBuffer buffer) { if (!ParseBuffer(buffer.cdata(), buffer.size())) { Clear(); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_packet.h b/webrtc/modules/rtp_rtcp/source/rtp_packet.h index 3c02125e08..2b3d38ec49 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_packet.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_packet.h @@ -31,7 +31,6 @@ class Packet { // Parse and copy given buffer into Packet. bool Parse(const uint8_t* buffer, size_t size); - bool Parse(rtc::ArrayView packet); // Parse and move given buffer into Packet. bool Parse(rtc::CopyOnWriteBuffer packet); diff --git a/webrtc/test/fuzzers/flexfec_receiver_fuzzer.cc b/webrtc/test/fuzzers/flexfec_receiver_fuzzer.cc index 533d0d16de..89ad4944ce 100644 --- a/webrtc/test/fuzzers/flexfec_receiver_fuzzer.cc +++ b/webrtc/test/fuzzers/flexfec_receiver_fuzzer.cc @@ -14,7 +14,6 @@ #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "webrtc/modules/rtp_rtcp/include/flexfec_receiver.h" #include "webrtc/modules/rtp_rtcp/source/byte_io.h" -#include "webrtc/modules/rtp_rtcp/source/rtp_packet_received.h" namespace webrtc { @@ -62,10 +61,7 @@ void FuzzOneInput(const uint8_t* data, size_t size) { ByteWriter::WriteBigEndian(packet.get() + 2, media_seq_num++); ByteWriter::WriteBigEndian(packet.get() + 8, media_ssrc); } - RtpPacketReceived parsed_packet; - if (parsed_packet.Parse(packet.get(), packet_length)) { - receiver.AddAndProcessReceivedPacket(parsed_packet); - } + receiver.AddAndProcessReceivedPacket(packet.get(), packet_length); } }