Remove deprecated RecoveredPacketReceiver::OnRecoveredPacket signature

Bug: webrtc:7135, webrtc:14795
Change-Id: Ib2f434b59542d6d8a2b8a287047417b784187602
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290567
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Auto-Submit: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39049}
This commit is contained in:
Per K 2023-01-05 12:01:52 +01:00 committed by WebRTC LUCI CQ
parent bc319027ae
commit 83c357f70a
5 changed files with 43 additions and 58 deletions

View file

@ -36,6 +36,8 @@ namespace webrtc {
namespace { namespace {
using ::testing::_; using ::testing::_;
using ::testing::Eq;
using ::testing::Property;
constexpr uint8_t kFlexfecPlType = 118; constexpr uint8_t kFlexfecPlType = 118;
constexpr uint8_t kFlexfecSsrc[] = {0x00, 0x00, 0x00, 0x01}; constexpr uint8_t kFlexfecSsrc[] = {0x00, 0x00, 0x00, 0x01};
@ -143,7 +145,8 @@ TEST_F(FlexfecReceiveStreamTest, RecoversPacket) {
// clang-format on // clang-format on
EXPECT_CALL(recovered_packet_receiver_, EXPECT_CALL(recovered_packet_receiver_,
OnRecoveredPacket(_, kRtpHeaderSize + kPayloadLength[1])); OnRecoveredPacket(Property(&RtpPacketReceived::payload_size,
Eq(kPayloadLength[1]))));
receive_stream_->OnRtpPacket(ParsePacket(kFlexfecPacket)); receive_stream_->OnRtpPacket(ParsePacket(kFlexfecPacket));

View file

@ -20,16 +20,7 @@ namespace webrtc {
// the recovered RTP packets based on SSRC. // the recovered RTP packets based on SSRC.
class RecoveredPacketReceiver { class RecoveredPacketReceiver {
public: public:
// TODO(bugs.webrtc.org/7135,perkj): Remove when all virtual void OnRecoveredPacket(const RtpPacketReceived& packet) = 0;
// 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: protected:
virtual ~RecoveredPacketReceiver() = default; virtual ~RecoveredPacketReceiver() = default;

View file

@ -12,6 +12,7 @@
#define MODULES_RTP_RTCP_MOCKS_MOCK_RECOVERED_PACKET_RECEIVER_H_ #define MODULES_RTP_RTCP_MOCKS_MOCK_RECOVERED_PACKET_RECEIVER_H_
#include "modules/rtp_rtcp/include/flexfec_receiver.h" #include "modules/rtp_rtcp/include/flexfec_receiver.h"
#include "modules/rtp_rtcp/source/rtp_packet_received.h"
#include "test/gmock.h" #include "test/gmock.h"
namespace webrtc { namespace webrtc {
@ -20,7 +21,7 @@ class MockRecoveredPacketReceiver : public RecoveredPacketReceiver {
public: public:
MOCK_METHOD(void, MOCK_METHOD(void,
OnRecoveredPacket, OnRecoveredPacket,
(const uint8_t* packet, size_t length), (const RtpPacketReceived& packet),
(override)); (override));
}; };

View file

@ -25,8 +25,8 @@ namespace webrtc {
namespace { namespace {
using ::testing::_; using ::testing::_;
using ::testing::Args; using ::testing::Eq;
using ::testing::ElementsAreArray; using ::testing::Property;
using test::fec::FlexfecPacketGenerator; using test::fec::FlexfecPacketGenerator;
using Packet = ForwardErrorCorrection::Packet; using Packet = ForwardErrorCorrection::Packet;
@ -239,9 +239,8 @@ TEST_F(FlexfecReceiverTest, RecoversFromSingleMediaLoss) {
packet_generator_.BuildFlexfecPacket(**fec_it); packet_generator_.BuildFlexfecPacket(**fec_it);
media_it++; media_it++;
EXPECT_CALL(recovered_packet_receiver_, EXPECT_CALL(recovered_packet_receiver_,
OnRecoveredPacket(_, (*media_it)->data.size())) OnRecoveredPacket(
.With(Args<0, 1>(ElementsAreArray((*media_it)->data.cdata(), Property(&RtpPacketReceived::Buffer, Eq((*media_it)->data))));
(*media_it)->data.size())));
receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header)); receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header));
} }
@ -261,9 +260,8 @@ TEST_F(FlexfecReceiverTest, RecoversFromDoubleMediaLoss) {
packet_generator_.BuildFlexfecPacket(**fec_it); packet_generator_.BuildFlexfecPacket(**fec_it);
auto media_it = media_packets.begin(); auto media_it = media_packets.begin();
EXPECT_CALL(recovered_packet_receiver_, EXPECT_CALL(recovered_packet_receiver_,
OnRecoveredPacket(_, (*media_it)->data.size())) OnRecoveredPacket(
.With(Args<0, 1>(ElementsAreArray((*media_it)->data.cdata(), Property(&RtpPacketReceived::Buffer, Eq((*media_it)->data))));
(*media_it)->data.size())));
receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header)); receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header));
// Receive second FEC packet and recover second lost media packet. // Receive second FEC packet and recover second lost media packet.
@ -271,9 +269,9 @@ TEST_F(FlexfecReceiverTest, RecoversFromDoubleMediaLoss) {
packet_with_rtp_header = packet_generator_.BuildFlexfecPacket(**fec_it); packet_with_rtp_header = packet_generator_.BuildFlexfecPacket(**fec_it);
media_it++; media_it++;
EXPECT_CALL(recovered_packet_receiver_, EXPECT_CALL(recovered_packet_receiver_,
OnRecoveredPacket(_, (*media_it)->data.size())) OnRecoveredPacket(
.With(Args<0, 1>(ElementsAreArray((*media_it)->data.cdata(), Property(&RtpPacketReceived::Buffer, Eq((*media_it)->data))));
(*media_it)->data.size())));
receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header)); receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header));
} }
@ -310,9 +308,8 @@ TEST_F(FlexfecReceiverTest, DoesNotCallbackTwice) {
packet_generator_.BuildFlexfecPacket(**fec_it); packet_generator_.BuildFlexfecPacket(**fec_it);
media_it++; media_it++;
EXPECT_CALL(recovered_packet_receiver_, EXPECT_CALL(recovered_packet_receiver_,
OnRecoveredPacket(_, (*media_it)->data.size())) OnRecoveredPacket(
.With(Args<0, 1>(ElementsAreArray((*media_it)->data.cdata(), Property(&RtpPacketReceived::Buffer, Eq((*media_it)->data))));
(*media_it)->data.size())));
receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header)); receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header));
// Receive the FEC packet again, but do not call back. // Receive the FEC packet again, but do not call back.
@ -363,9 +360,8 @@ TEST_F(FlexfecReceiverTest, RecoversFrom50PercentLoss) {
break; break;
} }
EXPECT_CALL(recovered_packet_receiver_, EXPECT_CALL(recovered_packet_receiver_,
OnRecoveredPacket(_, (*media_it)->data.size())) OnRecoveredPacket(Property(&RtpPacketReceived::Buffer,
.With(Args<0, 1>(ElementsAreArray((*media_it)->data.cdata(), Eq((*media_it)->data))));
(*media_it)->data.size())));
receiver_.OnRtpPacket(ParsePacket(*fec_packet_with_rtp_header)); receiver_.OnRtpPacket(ParsePacket(*fec_packet_with_rtp_header));
++media_it; ++media_it;
} }
@ -404,9 +400,8 @@ TEST_F(FlexfecReceiverTest, DelayedFecPacketDoesHelp) {
packet_generator_.BuildFlexfecPacket(**fec_it); packet_generator_.BuildFlexfecPacket(**fec_it);
media_it = media_packets.begin(); media_it = media_packets.begin();
EXPECT_CALL(recovered_packet_receiver_, EXPECT_CALL(recovered_packet_receiver_,
OnRecoveredPacket(_, (*media_it)->data.size())) OnRecoveredPacket(
.With(Args<0, 1>(ElementsAreArray((*media_it)->data.cdata(), Property(&RtpPacketReceived::Buffer, Eq((*media_it)->data))));
(*media_it)->data.size())));
receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header)); receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header));
} }
@ -532,14 +527,11 @@ TEST_F(FlexfecReceiverTest, RecoversWithMediaPacketsOutOfOrder) {
// Expect to recover lost media packets. // Expect to recover lost media packets.
EXPECT_CALL(recovered_packet_receiver_, EXPECT_CALL(recovered_packet_receiver_,
OnRecoveredPacket(_, (*media_packet1)->data.size())) OnRecoveredPacket(Property(&RtpPacketReceived::Buffer,
.With(Args<0, 1>(ElementsAreArray((*media_packet1)->data.cdata(), Eq((*media_packet1)->data))));
(*media_packet1)->data.size())));
EXPECT_CALL(recovered_packet_receiver_, EXPECT_CALL(recovered_packet_receiver_,
OnRecoveredPacket(_, (*media_packet4)->data.size())) OnRecoveredPacket(Property(&RtpPacketReceived::Buffer,
.With(Args<0, 1>(ElementsAreArray((*media_packet4)->data.cdata(), Eq((*media_packet4)->data))));
(*media_packet4)->data.size())));
// Add FEC packets. // Add FEC packets.
auto fec_it = fec_packets.begin(); auto fec_it = fec_packets.begin();
std::unique_ptr<Packet> packet_with_rtp_header; std::unique_ptr<Packet> packet_with_rtp_header;
@ -631,9 +623,8 @@ TEST_F(FlexfecReceiverTest, CalculatesNumberOfPackets) {
packet_generator_.BuildFlexfecPacket(**fec_it); packet_generator_.BuildFlexfecPacket(**fec_it);
media_it++; media_it++;
EXPECT_CALL(recovered_packet_receiver_, EXPECT_CALL(recovered_packet_receiver_,
OnRecoveredPacket(_, (*media_it)->data.size())) OnRecoveredPacket(
.With(Args<0, 1>(ElementsAreArray((*media_it)->data.cdata(), Property(&RtpPacketReceived::Buffer, Eq((*media_it)->data))));
(*media_it)->data.size())));
receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header)); receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header));
// Check stats calculations. // Check stats calculations.

View file

@ -29,8 +29,8 @@ namespace webrtc {
namespace { namespace {
using ::testing::_; using ::testing::_;
using ::testing::Args; using ::testing::Eq;
using ::testing::ElementsAreArray; using ::testing::Property;
using test::fec::AugmentedPacket; using test::fec::AugmentedPacket;
using Packet = ForwardErrorCorrection::Packet; using Packet = ForwardErrorCorrection::Packet;
@ -41,7 +41,7 @@ constexpr uint32_t kMediaSsrc = 835424;
class NullRecoveredPacketReceiver : public RecoveredPacketReceiver { class NullRecoveredPacketReceiver : public RecoveredPacketReceiver {
public: public:
void OnRecoveredPacket(const uint8_t* packet, size_t length) override {} void OnRecoveredPacket(const RtpPacketReceived& packet) override {}
}; };
} // namespace } // namespace
@ -143,15 +143,14 @@ void UlpfecReceiverTest::VerifyReconstructedMediaPacket(
// Verify that the content of the reconstructed packet is equal to the // Verify that the content of the reconstructed packet is equal to the
// content of `packet`, and that the same content is received `times` number // content of `packet`, and that the same content is received `times` number
// of times in a row. // of times in a row.
EXPECT_CALL(recovered_packet_receiver_, EXPECT_CALL(
OnRecoveredPacket(_, packet.data.size())) recovered_packet_receiver_,
.With( OnRecoveredPacket(Property(&RtpPacketReceived::Buffer, Eq(packet.data))))
Args<0, 1>(ElementsAreArray(packet.data.cdata(), packet.data.size())))
.Times(times); .Times(times);
} }
void UlpfecReceiverTest::InjectGarbagePacketLength(size_t fec_garbage_offset) { void UlpfecReceiverTest::InjectGarbagePacketLength(size_t fec_garbage_offset) {
EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _)); EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_));
const size_t kNumFecPackets = 1; const size_t kNumFecPackets = 1;
std::list<AugmentedPacket*> augmented_media_packets; std::list<AugmentedPacket*> augmented_media_packets;
@ -389,7 +388,7 @@ TEST_F(UlpfecReceiverTest, PacketNotDroppedTooEarly) {
EncodeFec(media_packets_batch1, kNumFecPacketsBatch1, &fec_packets); EncodeFec(media_packets_batch1, kNumFecPacketsBatch1, &fec_packets);
BuildAndAddRedMediaPacket(augmented_media_packets_batch1.front()); BuildAndAddRedMediaPacket(augmented_media_packets_batch1.front());
EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _)).Times(1); EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_)).Times(1);
receiver_fec_.ProcessReceivedFec(); receiver_fec_.ProcessReceivedFec();
delayed_fec = fec_packets.front(); delayed_fec = fec_packets.front();
@ -404,13 +403,13 @@ TEST_F(UlpfecReceiverTest, PacketNotDroppedTooEarly) {
for (auto it = augmented_media_packets_batch2.begin(); for (auto it = augmented_media_packets_batch2.begin();
it != augmented_media_packets_batch2.end(); ++it) { it != augmented_media_packets_batch2.end(); ++it) {
BuildAndAddRedMediaPacket(*it); BuildAndAddRedMediaPacket(*it);
EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _)).Times(1); EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_)).Times(1);
receiver_fec_.ProcessReceivedFec(); receiver_fec_.ProcessReceivedFec();
} }
// Add the delayed FEC packet. One packet should be reconstructed. // Add the delayed FEC packet. One packet should be reconstructed.
BuildAndAddRedFecPacket(delayed_fec); BuildAndAddRedFecPacket(delayed_fec);
EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _)).Times(1); EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_)).Times(1);
receiver_fec_.ProcessReceivedFec(); receiver_fec_.ProcessReceivedFec();
} }
@ -428,7 +427,7 @@ TEST_F(UlpfecReceiverTest, PacketDroppedWhenTooOld) {
EncodeFec(media_packets_batch1, kNumFecPacketsBatch1, &fec_packets); EncodeFec(media_packets_batch1, kNumFecPacketsBatch1, &fec_packets);
BuildAndAddRedMediaPacket(augmented_media_packets_batch1.front()); BuildAndAddRedMediaPacket(augmented_media_packets_batch1.front());
EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _)).Times(1); EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_)).Times(1);
receiver_fec_.ProcessReceivedFec(); receiver_fec_.ProcessReceivedFec();
delayed_fec = fec_packets.front(); delayed_fec = fec_packets.front();
@ -443,14 +442,14 @@ TEST_F(UlpfecReceiverTest, PacketDroppedWhenTooOld) {
for (auto it = augmented_media_packets_batch2.begin(); for (auto it = augmented_media_packets_batch2.begin();
it != augmented_media_packets_batch2.end(); ++it) { it != augmented_media_packets_batch2.end(); ++it) {
BuildAndAddRedMediaPacket(*it); BuildAndAddRedMediaPacket(*it);
EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _)).Times(1); EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_)).Times(1);
receiver_fec_.ProcessReceivedFec(); receiver_fec_.ProcessReceivedFec();
} }
// Add the delayed FEC packet. No packet should be reconstructed since the // Add the delayed FEC packet. No packet should be reconstructed since the
// first media packet of that frame has been dropped due to being too old. // first media packet of that frame has been dropped due to being too old.
BuildAndAddRedFecPacket(delayed_fec); BuildAndAddRedFecPacket(delayed_fec);
EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _)).Times(0); EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_)).Times(0);
receiver_fec_.ProcessReceivedFec(); receiver_fec_.ProcessReceivedFec();
} }
@ -469,7 +468,7 @@ TEST_F(UlpfecReceiverTest, OldFecPacketDropped) {
for (auto it = fec_packets.begin(); it != fec_packets.end(); ++it) { for (auto it = fec_packets.begin(); it != fec_packets.end(); ++it) {
// Only FEC packets inserted. No packets recoverable at this time. // Only FEC packets inserted. No packets recoverable at this time.
BuildAndAddRedFecPacket(*it); BuildAndAddRedFecPacket(*it);
EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _)).Times(0); EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_)).Times(0);
receiver_fec_.ProcessReceivedFec(); receiver_fec_.ProcessReceivedFec();
} }
// Move unique_ptr's to media_packets for lifetime management. // Move unique_ptr's to media_packets for lifetime management.
@ -484,7 +483,7 @@ TEST_F(UlpfecReceiverTest, OldFecPacketDropped) {
// and should have been dropped. Only the media packet we inserted will be // and should have been dropped. Only the media packet we inserted will be
// returned. // returned.
BuildAndAddRedMediaPacket(augmented_media_packets.front()); BuildAndAddRedMediaPacket(augmented_media_packets.front());
EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _)).Times(1); EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_)).Times(1);
receiver_fec_.ProcessReceivedFec(); receiver_fec_.ProcessReceivedFec();
} }