From 5e5d017c2b520bafa794e0dbb552be5f089eff91 Mon Sep 17 00:00:00 2001 From: Per K Date: Thu, 22 Dec 2022 13:43:41 +0100 Subject: [PATCH] Change RecoveredPacket::OnRecoveredPacket to produce webrtc::RtpPacketReceived Instead of getting header extension mapping from a receiver object, get the mapping from the received packet. The purpose is to be able to remove extension information from webrtc/call/receive_stream.h. Header extensions are negotiated per mid, not per receive stream. The goal is to reduce the number of places where packets are parsed and demuxed. Bug: webrtc:7135, webrtc:14795 Change-Id: I8944bc06a11dc572d9e14e7d7ee446a841096295 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/288968 Reviewed-by: Ilya Nikolaevskiy Commit-Queue: Per Kjellander Reviewed-by: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#38944} --- call/call.cc | 26 +---------- call/rtp_stream_receiver_controller.cc | 6 +++ call/rtp_stream_receiver_controller.h | 9 +++- modules/rtp_rtcp/BUILD.gn | 1 + modules/rtp_rtcp/include/flexfec_receiver.h | 2 +- .../include/recovered_packet_receiver.h | 39 ++++++++++++++++ modules/rtp_rtcp/include/rtp_rtcp_defines.h | 11 ----- modules/rtp_rtcp/source/flexfec_receiver.cc | 24 ++++++---- .../source/flexfec_receiver_unittest.cc | 16 ++----- .../source/forward_error_correction.h | 3 ++ modules/rtp_rtcp/source/rtp_packet.h | 3 ++ modules/rtp_rtcp/source/ulpfec_receiver.cc | 46 +++++++++---------- modules/rtp_rtcp/source/ulpfec_receiver.h | 4 +- .../source/ulpfec_receiver_unittest.cc | 3 +- test/fuzzers/flexfec_receiver_fuzzer.cc | 2 +- test/fuzzers/ulpfec_receiver_fuzzer.cc | 5 +- video/rtp_video_stream_receiver2.cc | 27 +++-------- video/rtp_video_stream_receiver2.h | 4 +- 18 files changed, 116 insertions(+), 115 deletions(-) create mode 100644 modules/rtp_rtcp/include/recovered_packet_receiver.h diff --git a/call/call.cc b/call/call.cc index fc9777c2f1..99d6dcd22a 100644 --- a/call/call.cc +++ b/call/call.cc @@ -195,7 +195,6 @@ class ResourceVideoSendStreamForwarder { class Call final : public webrtc::Call, public PacketReceiver, - public RecoveredPacketReceiver, public TargetTransferRateObserver, public BitrateAllocator::LimitObserver { public: @@ -255,9 +254,6 @@ class Call final : public webrtc::Call, rtc::CopyOnWriteBuffer packet, int64_t packet_time_us) override; - // Implements RecoveredPacketReceiver. - void OnRecoveredPacket(const uint8_t* packet, size_t length) override; - void SignalChannelNetworkState(MediaType media, NetworkState state) override; void OnAudioTransportOverheadChanged( @@ -1103,7 +1099,8 @@ FlexfecReceiveStream* Call::CreateFlexfecReceiveStream( // OnRtpPacket until the constructor is finished and the object is // in a valid state, since OnRtpPacket runs on the same thread. FlexfecReceiveStreamImpl* receive_stream = new FlexfecReceiveStreamImpl( - clock_, std::move(config), this, call_stats_->AsRtcpRttStats()); + clock_, std::move(config), &video_receiver_controller_, + call_stats_->AsRtcpRttStats()); // TODO(bugs.webrtc.org/11993): Set this up asynchronously on the network // thread. @@ -1515,25 +1512,6 @@ PacketReceiver::DeliveryStatus Call::DeliverPacket( return DeliverRtp(media_type, std::move(packet), packet_time_us); } -void Call::OnRecoveredPacket(const uint8_t* packet, size_t length) { - // TODO(bugs.webrtc.org/11993): Expect to be called on the network thread. - // This method is called synchronously via `OnRtpPacket()` (see DeliverRtp) - // on the same thread. - RTC_DCHECK_RUN_ON(worker_thread_); - RtpPacketReceived parsed_packet; - if (!parsed_packet.Parse(packet, length)) - return; - - parsed_packet.set_recovered(true); - - if (!IdentifyReceivedPacket(parsed_packet)) - return; - - // TODO(brandtr): Update here when we support protecting audio packets too. - parsed_packet.set_payload_type_frequency(kVideoPayloadTypeFrequency); - video_receiver_controller_.OnRtpPacket(parsed_packet); -} - void Call::NotifyBweOfReceivedPacket(const RtpPacketReceived& packet, MediaType media_type, bool use_send_side_bwe) { diff --git a/call/rtp_stream_receiver_controller.cc b/call/rtp_stream_receiver_controller.cc index 8cefa2dffc..993a4fc76e 100644 --- a/call/rtp_stream_receiver_controller.cc +++ b/call/rtp_stream_receiver_controller.cc @@ -50,6 +50,12 @@ bool RtpStreamReceiverController::OnRtpPacket(const RtpPacketReceived& packet) { return demuxer_.OnRtpPacket(packet); } +void RtpStreamReceiverController::OnRecoveredPacket( + const RtpPacketReceived& packet) { + RTC_DCHECK_RUN_ON(&demuxer_sequence_); + demuxer_.OnRtpPacket(packet); +} + bool RtpStreamReceiverController::AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink) { RTC_DCHECK_RUN_ON(&demuxer_sequence_); diff --git a/call/rtp_stream_receiver_controller.h b/call/rtp_stream_receiver_controller.h index 46d04f73f8..1040632639 100644 --- a/call/rtp_stream_receiver_controller.h +++ b/call/rtp_stream_receiver_controller.h @@ -15,6 +15,7 @@ #include "api/sequence_checker.h" #include "call/rtp_demuxer.h" #include "call/rtp_stream_receiver_controller_interface.h" +#include "modules/rtp_rtcp/include/recovered_packet_receiver.h" namespace webrtc { @@ -24,8 +25,8 @@ class RtpPacketReceived; // single RTP session. // TODO(bugs.webrtc.org/7135): Add RTCP processing, we should aim to terminate // RTCP and not leave any RTCP processing to individual receive streams. -class RtpStreamReceiverController - : public RtpStreamReceiverControllerInterface { +class RtpStreamReceiverController : public RtpStreamReceiverControllerInterface, + public RecoveredPacketReceiver { public: RtpStreamReceiverController(); ~RtpStreamReceiverController() override; @@ -38,6 +39,10 @@ class RtpStreamReceiverController // TODO(bugs.webrtc.org/7135): Not yet responsible for parsing. bool OnRtpPacket(const RtpPacketReceived& packet); + // Implements RecoveredPacketReceiver. + // Responsible for demuxing recovered FLEXFEC packets. + void OnRecoveredPacket(const RtpPacketReceived& packet) override; + private: class Receiver : public RtpStreamReceiverInterface { public: diff --git a/modules/rtp_rtcp/BUILD.gn b/modules/rtp_rtcp/BUILD.gn index abcdb619f4..2256317e70 100644 --- a/modules/rtp_rtcp/BUILD.gn +++ b/modules/rtp_rtcp/BUILD.gn @@ -11,6 +11,7 @@ import("../../webrtc.gni") rtc_library("rtp_rtcp_format") { visibility = [ "*" ] public = [ + "include/recovered_packet_receiver.h", "include/report_block_data.h", "include/rtcp_statistics.h", "include/rtp_cvo.h", diff --git a/modules/rtp_rtcp/include/flexfec_receiver.h b/modules/rtp_rtcp/include/flexfec_receiver.h index d34ba74620..3cf4c3845e 100644 --- a/modules/rtp_rtcp/include/flexfec_receiver.h +++ b/modules/rtp_rtcp/include/flexfec_receiver.h @@ -16,7 +16,7 @@ #include #include "api/sequence_checker.h" -#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" +#include "modules/rtp_rtcp/include/recovered_packet_receiver.h" #include "modules/rtp_rtcp/source/forward_error_correction.h" #include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "modules/rtp_rtcp/source/ulpfec_receiver.h" diff --git a/modules/rtp_rtcp/include/recovered_packet_receiver.h b/modules/rtp_rtcp/include/recovered_packet_receiver.h new file mode 100644 index 0000000000..0be539d772 --- /dev/null +++ b/modules/rtp_rtcp/include/recovered_packet_receiver.h @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2022 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ +#ifndef MODULES_RTP_RTCP_INCLUDE_RECOVERED_PACKET_RECEIVER_H_ +#define MODULES_RTP_RTCP_INCLUDE_RECOVERED_PACKET_RECEIVER_H_ + +#include "modules/rtp_rtcp/source/rtp_packet_received.h" +#include "rtc_base/checks.h" + +namespace webrtc { + +// Callback interface for packets recovered by FlexFEC or ULPFEC. In +// the FlexFEC case, the implementation should be able to demultiplex +// the recovered RTP packets based on SSRC. +class RecoveredPacketReceiver { + public: + // TODO(bugs.webrtc.org/7135,perkj): Remove when all + // implementations implement OnRecoveredPacket(const RtpPacketReceived&) + virtual void OnRecoveredPacket(const uint8_t* packet, size_t length) { + RTC_DCHECK_NOTREACHED(); + } + // TODO(bugs.webrtc.org/7135,perkj): Make pure virtual when all + // implementations are updated. + virtual void OnRecoveredPacket(const RtpPacketReceived& packet) { + OnRecoveredPacket(packet.Buffer().data(), packet.Buffer().size()); + } + + protected: + virtual ~RecoveredPacketReceiver() = default; +}; + +} // namespace webrtc +#endif // MODULES_RTP_RTCP_INCLUDE_RECOVERED_PACKET_RECEIVER_H_ diff --git a/modules/rtp_rtcp/include/rtp_rtcp_defines.h b/modules/rtp_rtcp/include/rtp_rtcp_defines.h index c482e0c7dc..1f2804815b 100644 --- a/modules/rtp_rtcp/include/rtp_rtcp_defines.h +++ b/modules/rtp_rtcp/include/rtp_rtcp_defines.h @@ -176,17 +176,6 @@ struct RtpState { bool ssrc_has_acked; }; -// Callback interface for packets recovered by FlexFEC or ULPFEC. In -// the FlexFEC case, the implementation should be able to demultiplex -// the recovered RTP packets based on SSRC. -class RecoveredPacketReceiver { - public: - virtual void OnRecoveredPacket(const uint8_t* packet, size_t length) = 0; - - protected: - virtual ~RecoveredPacketReceiver() = default; -}; - class RtcpIntraFrameObserver { public: virtual ~RtcpIntraFrameObserver() {} diff --git a/modules/rtp_rtcp/source/flexfec_receiver.cc b/modules/rtp_rtcp/source/flexfec_receiver.cc index b5198b53a6..3f345cd6d2 100644 --- a/modules/rtp_rtcp/source/flexfec_receiver.cc +++ b/modules/rtp_rtcp/source/flexfec_receiver.cc @@ -97,6 +97,7 @@ FlexfecReceiver::AddReceivedPacket(const RtpPacketReceived& packet) { new ForwardErrorCorrection::ReceivedPacket()); received_packet->seq_num = packet.SequenceNumber(); received_packet->ssrc = packet.Ssrc(); + received_packet->extensions = packet.extension_manager(); if (received_packet->ssrc == ssrc_) { // This is a FlexFEC packet. if (packet.payload_size() < kMinFlexfecHeaderSize) { @@ -160,13 +161,17 @@ void FlexfecReceiver::ProcessReceivedPacket( // again, with the same packet. recovered_packet->returned = true; RTC_CHECK_GE(recovered_packet->pkt->data.size(), kRtpHeaderSize); - recovered_packet_receiver_->OnRecoveredPacket( - recovered_packet->pkt->data.cdata(), - recovered_packet->pkt->data.size()); - uint32_t media_ssrc = - ForwardErrorCorrection::ParseSsrc(recovered_packet->pkt->data.data()); - uint16_t media_seq_num = ForwardErrorCorrection::ParseSequenceNumber( - recovered_packet->pkt->data.data()); + + RtpPacketReceived parsed_packet(&received_packet.extensions); + if (!parsed_packet.Parse(recovered_packet->pkt->data)) { + continue; + } + parsed_packet.set_recovered(true); + + // TODO(brandtr): Update here when we support protecting audio packets too. + parsed_packet.set_payload_type_frequency(kVideoPayloadTypeFrequency); + recovered_packet_receiver_->OnRecoveredPacket(parsed_packet); + // Periodically log the incoming packets at LS_INFO. int64_t now_ms = clock_->TimeInMilliseconds(); bool should_log_periodically = @@ -174,8 +179,9 @@ void FlexfecReceiver::ProcessReceivedPacket( if (RTC_LOG_CHECK_LEVEL(LS_VERBOSE) || should_log_periodically) { rtc::LoggingSeverity level = should_log_periodically ? rtc::LS_INFO : rtc::LS_VERBOSE; - RTC_LOG_V(level) << "Recovered media packet with SSRC: " << media_ssrc - << " seq " << media_seq_num << " recovered length " + RTC_LOG_V(level) << "Recovered media packet with SSRC: " + << parsed_packet.Ssrc() << " seq " + << parsed_packet.SequenceNumber() << " recovered length " << recovered_packet->pkt->data.size() << " from FlexFEC stream with SSRC: " << ssrc_; if (should_log_periodically) { diff --git a/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc b/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc index 54ed11d64c..5e21b973a0 100644 --- a/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc @@ -457,13 +457,10 @@ TEST_F(FlexfecReceiverTest, SurvivesOldRecoveredPacketBeingReinserted) { void SetReceiver(FlexfecReceiver* receiver) { receiver_ = receiver; } // Implements RecoveredPacketReceiver. - void OnRecoveredPacket(const uint8_t* packet, size_t length) override { - RtpPacketReceived parsed_packet; - EXPECT_TRUE(parsed_packet.Parse(packet, length)); - parsed_packet.set_recovered(true); - + void OnRecoveredPacket(const RtpPacketReceived& packet) override { + EXPECT_TRUE(packet.recovered()); RTC_DCHECK(receiver_); - receiver_->OnRtpPacket(parsed_packet); + receiver_->OnRtpPacket(packet); } private: @@ -571,10 +568,7 @@ TEST_F(FlexfecReceiverTest, RecoveryCallbackDoesNotLoopInfinitely) { bool DeepRecursion() const { return deep_recursion_; } // Implements RecoveredPacketReceiver. - void OnRecoveredPacket(const uint8_t* packet, size_t length) override { - RtpPacketReceived parsed_packet; - EXPECT_TRUE(parsed_packet.Parse(packet, length)); - + void OnRecoveredPacket(const RtpPacketReceived& packet) override { did_receive_call_back_ = true; if (recursion_depth_ > kMaxRecursionDepth) { @@ -583,7 +577,7 @@ TEST_F(FlexfecReceiverTest, RecoveryCallbackDoesNotLoopInfinitely) { } ++recursion_depth_; RTC_DCHECK(receiver_); - receiver_->OnRtpPacket(parsed_packet); + receiver_->OnRtpPacket(packet); --recursion_depth_; } diff --git a/modules/rtp_rtcp/source/forward_error_correction.h b/modules/rtp_rtcp/source/forward_error_correction.h index 0ebe1c5091..ac8f5361e4 100644 --- a/modules/rtp_rtcp/source/forward_error_correction.h +++ b/modules/rtp_rtcp/source/forward_error_correction.h @@ -19,7 +19,9 @@ #include #include "api/scoped_refptr.h" +#include "api/units/timestamp.h" #include "modules/include/module_fec_types.h" +#include "modules/rtp_rtcp/include/rtp_header_extension_map.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/forward_error_correction_internal.h" #include "rtc_base/copy_on_write_buffer.h" @@ -83,6 +85,7 @@ class ForwardErrorCorrection { bool is_fec; // Set to true if this is an FEC packet and false // otherwise. bool is_recovered; + RtpHeaderExtensionMap extensions; rtc::scoped_refptr pkt; // Pointer to the packet storage. }; diff --git a/modules/rtp_rtcp/source/rtp_packet.h b/modules/rtp_rtcp/source/rtp_packet.h index b87d213636..aba4975b7a 100644 --- a/modules/rtp_rtcp/source/rtp_packet.h +++ b/modules/rtp_rtcp/source/rtp_packet.h @@ -53,6 +53,9 @@ class RtpPacket { // Maps extensions id to their types. void IdentifyExtensions(ExtensionManager extensions); + // Returns the extension map used for identifying extensions in this packet. + const ExtensionManager& extension_manager() const { return extensions_; } + // Header. bool Marker() const { return marker_; } uint8_t PayloadType() const { return payload_type_; } diff --git a/modules/rtp_rtcp/source/ulpfec_receiver.cc b/modules/rtp_rtcp/source/ulpfec_receiver.cc index 4090d99e8d..4452bdbba5 100644 --- a/modules/rtp_rtcp/source/ulpfec_receiver.cc +++ b/modules/rtp_rtcp/source/ulpfec_receiver.cc @@ -24,12 +24,10 @@ namespace webrtc { UlpfecReceiver::UlpfecReceiver(uint32_t ssrc, int ulpfec_payload_type, RecoveredPacketReceiver* callback, - rtc::ArrayView extensions, Clock* clock) : ssrc_(ssrc), ulpfec_payload_type_(ulpfec_payload_type), clock_(clock), - extensions_(extensions), recovered_packet_callback_(callback), fec_(ForwardErrorCorrection::CreateUlpfec(ssrc_)) { // TODO(tommi, brandtr): Once considerations for red have been split @@ -75,12 +73,6 @@ FecPacketCounter UlpfecReceiver::GetPacketCounter() const { return packet_counter_; } -void UlpfecReceiver::SetRtpExtensions( - rtc::ArrayView extensions) { - RTC_DCHECK_RUN_ON(&sequence_checker_); - extensions_.Reset(extensions); -} - // 0 1 2 3 // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ @@ -143,6 +135,7 @@ bool UlpfecReceiver::AddReceivedRedPacket(const RtpPacketReceived& rtp_packet) { received_packet->is_recovered = rtp_packet.recovered(); received_packet->ssrc = rtp_packet.Ssrc(); received_packet->seq_num = rtp_packet.SequenceNumber(); + received_packet->extensions = rtp_packet.extension_manager(); if (rtp_packet.payload()[0] & 0x80) { // f bit set in RED header, i.e. there are more than one RED header blocks. @@ -198,28 +191,26 @@ void UlpfecReceiver::ProcessReceivedFec() { std::vector> received_packets; received_packets.swap(received_packets_); + RtpHeaderExtensionMap* last_recovered_extension_map = nullptr; for (const auto& received_packet : received_packets) { // Send received media packet to VCM. if (!received_packet->is_fec) { ForwardErrorCorrection::Packet* packet = received_packet->pkt.get(); - recovered_packet_callback_->OnRecoveredPacket(packet->data.data(), - packet->data.size()); - // Create a packet with the buffer to modify it. - RtpPacketReceived rtp_packet; - const uint8_t* const original_data = packet->data.cdata(); - if (!rtp_packet.Parse(packet->data)) { + + RtpPacketReceived rtp_packet(&received_packet->extensions); + if (!rtp_packet.Parse(std::move(packet->data))) { RTC_LOG(LS_WARNING) << "Corrupted media packet"; - } else { - rtp_packet.IdentifyExtensions(extensions_); - // Reset buffer reference, so zeroing would work on a buffer with a - // single reference. - packet->data = rtc::CopyOnWriteBuffer(0); - rtp_packet.ZeroMutableExtensions(); - packet->data = rtp_packet.Buffer(); - // Ensure that zeroing of extensions was done in place. - RTC_DCHECK_EQ(packet->data.cdata(), original_data); + continue; } + recovered_packet_callback_->OnRecoveredPacket(rtp_packet); + // Some header extensions need to be zeroed in `received_packet` since + // they are written to the packet after FEC encoding. We try to do it + // without a copy of the underlying Copy-On-Write buffer, but if a + // reference is held by `recovered_packet_callback_->OnRecoveredPacket` a + // copy will still be made in 'rtp_packet.ZeroMutableExtensions()'. + rtp_packet.ZeroMutableExtensions(); + packet->data = rtp_packet.Buffer(); } if (!received_packet->is_recovered) { // Do not pass recovered packets to FEC. Recovered packet might have @@ -227,6 +218,7 @@ void UlpfecReceiver::ProcessReceivedFec() { // representation than the original packet, That will corrupt // FEC calculation. fec_->DecodeFec(*received_packet, &recovered_packets_); + last_recovered_extension_map = &received_packet->extensions; } } @@ -241,8 +233,12 @@ void UlpfecReceiver::ProcessReceivedFec() { // Set this flag first; in case the recovered packet carries a RED // header, OnRecoveredPacket will recurse back here. recovered_packet->returned = true; - recovered_packet_callback_->OnRecoveredPacket(packet->data.data(), - packet->data.size()); + RtpPacketReceived parsed_packet(last_recovered_extension_map); + if (!parsed_packet.Parse(packet->data)) { + continue; + } + parsed_packet.set_recovered(true); + recovered_packet_callback_->OnRecoveredPacket(parsed_packet); } } diff --git a/modules/rtp_rtcp/source/ulpfec_receiver.h b/modules/rtp_rtcp/source/ulpfec_receiver.h index b8ac8d8c30..3ad1c75e09 100644 --- a/modules/rtp_rtcp/source/ulpfec_receiver.h +++ b/modules/rtp_rtcp/source/ulpfec_receiver.h @@ -18,8 +18,8 @@ #include #include "api/sequence_checker.h" +#include "modules/rtp_rtcp/include/recovered_packet_receiver.h" #include "modules/rtp_rtcp/include/rtp_header_extension_map.h" -#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/forward_error_correction.h" #include "modules/rtp_rtcp/source/rtp_packet_received.h" #include "rtc_base/system/no_unique_address.h" @@ -43,7 +43,6 @@ class UlpfecReceiver { UlpfecReceiver(uint32_t ssrc, int ulpfec_payload_type, RecoveredPacketReceiver* callback, - rtc::ArrayView extensions, Clock* clock); ~UlpfecReceiver(); @@ -61,7 +60,6 @@ class UlpfecReceiver { const uint32_t ssrc_; const int ulpfec_payload_type_; Clock* const clock_; - RtpHeaderExtensionMap extensions_ RTC_GUARDED_BY(&sequence_checker_); RTC_NO_UNIQUE_ADDRESS SequenceChecker sequence_checker_; RecoveredPacketReceiver* const recovered_packet_callback_; diff --git a/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc b/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc index 1b0b0daf56..81dd399988 100644 --- a/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc @@ -53,7 +53,6 @@ class UlpfecReceiverTest : public ::testing::Test { receiver_fec_(kMediaSsrc, kFecPayloadType, &recovered_packet_receiver_, - {}, Clock::GetRealTimeClock()), packet_generator_(kMediaSsrc) {} @@ -180,7 +179,7 @@ void UlpfecReceiverTest::SurvivesMaliciousPacket(const uint8_t* data, uint8_t ulpfec_payload_type) { NullRecoveredPacketReceiver null_callback; UlpfecReceiver receiver_fec(kMediaSsrc, ulpfec_payload_type, &null_callback, - {}, Clock::GetRealTimeClock()); + Clock::GetRealTimeClock()); RtpPacketReceived rtp_packet; ASSERT_TRUE(rtp_packet.Parse(data, length)); diff --git a/test/fuzzers/flexfec_receiver_fuzzer.cc b/test/fuzzers/flexfec_receiver_fuzzer.cc index c5034bb933..67d603d3fc 100644 --- a/test/fuzzers/flexfec_receiver_fuzzer.cc +++ b/test/fuzzers/flexfec_receiver_fuzzer.cc @@ -19,7 +19,7 @@ namespace webrtc { namespace { class DummyCallback : public RecoveredPacketReceiver { - void OnRecoveredPacket(const uint8_t* packet, size_t length) override {} + void OnRecoveredPacket(const RtpPacketReceived& packet) override {} }; } // namespace diff --git a/test/fuzzers/ulpfec_receiver_fuzzer.cc b/test/fuzzers/ulpfec_receiver_fuzzer.cc index d88bee2c1b..0a29ba3259 100644 --- a/test/fuzzers/ulpfec_receiver_fuzzer.cc +++ b/test/fuzzers/ulpfec_receiver_fuzzer.cc @@ -20,7 +20,7 @@ namespace webrtc { namespace { class DummyCallback : public RecoveredPacketReceiver { - void OnRecoveredPacket(const uint8_t* packet, size_t length) override {} + void OnRecoveredPacket(const RtpPacketReceived& packet) override {} }; } // namespace @@ -36,8 +36,7 @@ void FuzzOneInput(const uint8_t* data, size_t size) { uint16_t media_seq_num = ByteReader::ReadLittleEndian(data + 10); DummyCallback callback; - UlpfecReceiver receiver(ulpfec_ssrc, 0, &callback, {}, - Clock::GetRealTimeClock()); + UlpfecReceiver receiver(ulpfec_ssrc, 0, &callback, Clock::GetRealTimeClock()); test::FuzzDataHelper fuzz_data(rtc::MakeArrayView(data, size)); while (fuzz_data.CanReadBytes(kMinDataNeeded)) { diff --git a/video/rtp_video_stream_receiver2.cc b/video/rtp_video_stream_receiver2.cc index 169aeb4700..fe89e05f59 100644 --- a/video/rtp_video_stream_receiver2.cc +++ b/video/rtp_video_stream_receiver2.cc @@ -126,7 +126,6 @@ std::unique_ptr MaybeConstructUlpfecReceiver( uint32_t remote_ssrc, int red_payload_type, int ulpfec_payload_type, - rtc::ArrayView extensions, RecoveredPacketReceiver* callback, Clock* clock) { RTC_DCHECK_GE(red_payload_type, -1); @@ -141,7 +140,7 @@ std::unique_ptr MaybeConstructUlpfecReceiver( // return nullptr; return std::make_unique(remote_ssrc, ulpfec_payload_type, - callback, extensions, clock); + callback, clock); } static const int kPacketLogIntervalMs = 10000; @@ -262,7 +261,6 @@ RtpVideoStreamReceiver2::RtpVideoStreamReceiver2( MaybeConstructUlpfecReceiver(config->rtp.remote_ssrc, config->rtp.red_payload_type, config->rtp.ulpfec_payload_type, - config->rtp.extensions, this, clock_)), red_payload_type_(config_.rtp.red_payload_type), @@ -694,26 +692,13 @@ void RtpVideoStreamReceiver2::OnReceivedPayloadData( OnInsertedPacket(packet_buffer_.InsertPacket(std::move(packet))); } -void RtpVideoStreamReceiver2::OnRecoveredPacket(const uint8_t* rtp_packet, - size_t rtp_packet_length) { +void RtpVideoStreamReceiver2::OnRecoveredPacket( + const RtpPacketReceived& packet) { RTC_DCHECK_RUN_ON(&packet_sequence_checker_); - - RtpPacketReceived packet; - if (!packet.Parse(rtp_packet, rtp_packet_length)) - return; if (packet.PayloadType() == red_payload_type_) { RTC_LOG(LS_WARNING) << "Discarding recovered packet with RED encapsulation"; return; } - - packet.IdentifyExtensions(rtp_header_extensions_); - packet.set_payload_type_frequency(kVideoPayloadTypeFrequency); - // TODO(bugs.webrtc.org/7135): UlpfecReceiverImpl::ProcessReceivedFec passes - // both original (decapsulated) media packets and recovered packets to this - // callback. We need a way to distinguish, for setting packet.recovered() - // correctly. Ideally, move RED decapsulation out of the Ulpfec - // implementation. - ReceivePacket(packet); } @@ -1048,9 +1033,9 @@ void RtpVideoStreamReceiver2::SetProtectionPayloadTypes( RTC_DCHECK(red_payload_type >= -1 && red_payload_type < 0x80); RTC_DCHECK(ulpfec_payload_type >= -1 && ulpfec_payload_type < 0x80); red_payload_type_ = red_payload_type; - ulpfec_receiver_ = MaybeConstructUlpfecReceiver( - config_.rtp.remote_ssrc, red_payload_type, ulpfec_payload_type, - config_.rtp.extensions, this, clock_); + ulpfec_receiver_ = + MaybeConstructUlpfecReceiver(config_.rtp.remote_ssrc, red_payload_type, + ulpfec_payload_type, this, clock_); } absl::optional RtpVideoStreamReceiver2::LastReceivedPacketMs() const { diff --git a/video/rtp_video_stream_receiver2.h b/video/rtp_video_stream_receiver2.h index cead2a3147..782fa522f7 100644 --- a/video/rtp_video_stream_receiver2.h +++ b/video/rtp_video_stream_receiver2.h @@ -26,9 +26,9 @@ #include "call/syncable.h" #include "call/video_receive_stream.h" #include "modules/rtp_rtcp/include/receive_statistics.h" +#include "modules/rtp_rtcp/include/recovered_packet_receiver.h" #include "modules/rtp_rtcp/include/remote_ntp_time_estimator.h" #include "modules/rtp_rtcp/include/rtp_header_extension_map.h" -#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "modules/rtp_rtcp/source/absolute_capture_time_interpolator.h" #include "modules/rtp_rtcp/source/capture_clock_offset_updater.h" #include "modules/rtp_rtcp/source/rtp_dependency_descriptor_extension.h" @@ -138,7 +138,7 @@ class RtpVideoStreamReceiver2 : public LossNotificationSender, const RTPVideoHeader& video); // Implements RecoveredPacketReceiver. - void OnRecoveredPacket(const uint8_t* packet, size_t packet_length) override; + void OnRecoveredPacket(const RtpPacketReceived& packet) override; // Send an RTCP keyframe request. void RequestKeyFrame() override;