Rename flexfec AddAndProcessReceivedPacket --> OnRtpPacket.
Preparing for a media-independent RTP receive stream interface.
BUG=webrtc:7135
Review-Url: https://codereview.webrtc.org/2686273002
Cr-Commit-Position: refs/heads/master@{#16646}
diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc
index ec80537..bb13d55 100644
--- a/webrtc/call/call.cc
+++ b/webrtc/call/call.cc
@@ -1216,7 +1216,7 @@
// 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(*parsed_packet);
+ it->second->OnRtpPacket(*parsed_packet);
event_log_->LogRtpHeader(kIncomingPacket, media_type, packet, length);
return DELIVERY_OK;
@@ -1225,12 +1225,9 @@
if (media_type == MediaType::ANY || media_type == MediaType::VIDEO) {
auto it = flexfec_receive_ssrcs_protection_.find(ssrc);
if (it != flexfec_receive_ssrcs_protection_.end()) {
- auto status = it->second->AddAndProcessReceivedPacket(*parsed_packet)
- ? DELIVERY_OK
- : DELIVERY_PACKET_ERROR;
- if (status == DELIVERY_OK)
- event_log_->LogRtpHeader(kIncomingPacket, media_type, packet, length);
- return status;
+ it->second->OnRtpPacket(*parsed_packet);
+ event_log_->LogRtpHeader(kIncomingPacket, media_type, packet, length);
+ return DELIVERY_OK;
}
}
return DELIVERY_UNKNOWN_SSRC;
diff --git a/webrtc/call/flexfec_receive_stream_impl.cc b/webrtc/call/flexfec_receive_stream_impl.cc
index f5272c9..290b897 100644
--- a/webrtc/call/flexfec_receive_stream_impl.cc
+++ b/webrtc/call/flexfec_receive_stream_impl.cc
@@ -149,19 +149,17 @@
process_thread_->DeRegisterModule(rtp_rtcp_.get());
}
-bool FlexfecReceiveStreamImpl::AddAndProcessReceivedPacket(
- const RtpPacketReceived& packet) {
+void FlexfecReceiveStreamImpl::OnRtpPacket(const RtpPacketReceived& packet) {
{
rtc::CritScope cs(&crit_);
if (!started_)
- return false;
+ return;
}
if (!receiver_)
- return false;
+ return;
- if (!receiver_->AddAndProcessReceivedPacket(packet))
- return false;
+ receiver_->OnRtpPacket(packet);
// Do not report media packets in the RTCP RRs generated by |rtp_rtcp_|.
if (packet.Ssrc() == config_.remote_ssrc) {
@@ -172,8 +170,6 @@
rtp_receive_statistics_->IncomingPacket(header, packet.size(),
kNotRetransmitted);
}
-
- return true;
}
void FlexfecReceiveStreamImpl::Start() {
diff --git a/webrtc/call/flexfec_receive_stream_impl.h b/webrtc/call/flexfec_receive_stream_impl.h
index 36ea623..7051829 100644
--- a/webrtc/call/flexfec_receive_stream_impl.h
+++ b/webrtc/call/flexfec_receive_stream_impl.h
@@ -36,7 +36,8 @@
const Config& GetConfig() const { return config_; }
- bool AddAndProcessReceivedPacket(const RtpPacketReceived& packet);
+ // TODO(nisse): Intended to be part of an RtpPacketReceiver interface.
+ void OnRtpPacket(const RtpPacketReceived& packet);
// Implements FlexfecReceiveStream.
void Start() override;
diff --git a/webrtc/call/flexfec_receive_stream_unittest.cc b/webrtc/call/flexfec_receive_stream_unittest.cc
index 2365811..46bd7c3 100644
--- a/webrtc/call/flexfec_receive_stream_unittest.cc
+++ b/webrtc/call/flexfec_receive_stream_unittest.cc
@@ -138,14 +138,14 @@
&rtt_stats_, &process_thread_);
// Do not call back before being started.
- receive_stream.AddAndProcessReceivedPacket(ParsePacket(kFlexfecPacket));
+ receive_stream.OnRtpPacket(ParsePacket(kFlexfecPacket));
// 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.OnRtpPacket(ParsePacket(kFlexfecPacket));
}
} // namespace webrtc
diff --git a/webrtc/modules/rtp_rtcp/include/flexfec_receiver.h b/webrtc/modules/rtp_rtcp/include/flexfec_receiver.h
index a986fbb..67dc813 100644
--- a/webrtc/modules/rtp_rtcp/include/flexfec_receiver.h
+++ b/webrtc/modules/rtp_rtcp/include/flexfec_receiver.h
@@ -43,15 +43,17 @@
// 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(const RtpPacketReceived& packet);
+ void OnRtpPacket(const RtpPacketReceived& packet);
// Returns a counter describing the added and recovered packets.
FecPacketCounter GetPacketCounter() const;
- private:
+ // Protected to aid testing.
+ protected:
bool AddReceivedPacket(const RtpPacketReceived& packet);
bool ProcessReceivedPackets();
+ private:
// Config.
const uint32_t ssrc_;
const uint32_t protected_media_ssrc_;
diff --git a/webrtc/modules/rtp_rtcp/source/flexfec_receiver.cc b/webrtc/modules/rtp_rtcp/source/flexfec_receiver.cc
index b81a039..81c5785 100644
--- a/webrtc/modules/rtp_rtcp/source/flexfec_receiver.cc
+++ b/webrtc/modules/rtp_rtcp/source/flexfec_receiver.cc
@@ -45,13 +45,12 @@
FlexfecReceiver::~FlexfecReceiver() = default;
-bool FlexfecReceiver::AddAndProcessReceivedPacket(
- const RtpPacketReceived& packet) {
+void FlexfecReceiver::OnRtpPacket(const RtpPacketReceived& packet) {
RTC_DCHECK_CALLED_SEQUENTIALLY(&sequence_checker_);
- if (!AddReceivedPacket(std::move(packet))) {
- return false;
+ if (!AddReceivedPacket(packet)) {
+ return;
}
- return ProcessReceivedPackets();
+ ProcessReceivedPackets();
}
FecPacketCounter FlexfecReceiver::GetPacketCounter() const {
diff --git a/webrtc/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc
index 8bf0f37..01d9a59 100644
--- a/webrtc/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/flexfec_receiver_unittest.cc
@@ -45,6 +45,18 @@
} // namespace
+class FlexfecReceiverForTest : public FlexfecReceiver {
+ public:
+ FlexfecReceiverForTest(uint32_t ssrc,
+ uint32_t protected_media_ssrc,
+ RecoveredPacketReceiver* recovered_packet_receiver)
+ : FlexfecReceiver(ssrc, protected_media_ssrc, recovered_packet_receiver) {
+ }
+ // Expose methods for tests.
+ using FlexfecReceiver::AddReceivedPacket;
+ using FlexfecReceiver::ProcessReceivedPackets;
+};
+
class FlexfecReceiverTest : public ::testing::Test {
protected:
FlexfecReceiverTest()
@@ -61,7 +73,7 @@
std::list<Packet*> EncodeFec(const PacketList& media_packets,
size_t num_fec_packets);
- FlexfecReceiver receiver_;
+ FlexfecReceiverForTest receiver_;
std::unique_ptr<ForwardErrorCorrection> erasure_code_;
FlexfecPacketGenerator packet_generator_;
@@ -100,8 +112,8 @@
std::unique_ptr<Packet> media_packet(
packet_generator_.NextPacket(0, kPayloadLength));
- EXPECT_TRUE(
- receiver_.AddAndProcessReceivedPacket(ParsePacket(*media_packet)));
+ EXPECT_TRUE(receiver_.AddReceivedPacket(ParsePacket(*media_packet)));
+ EXPECT_TRUE(receiver_.ProcessReceivedPackets());
}
TEST_F(FlexfecReceiverTest, ReceivesMediaAndFecPackets) {
@@ -114,9 +126,10 @@
const auto& media_packet = media_packets.front();
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_.AddReceivedPacket(ParsePacket(*media_packet)));
+ EXPECT_TRUE(receiver_.ProcessReceivedPackets());
+ EXPECT_TRUE(receiver_.AddReceivedPacket(ParsePacket(*fec_packet)));
+ EXPECT_TRUE(receiver_.ProcessReceivedPackets());
}
TEST_F(FlexfecReceiverTest, FailsOnTruncatedFecPacket) {
@@ -131,9 +144,9 @@
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_.AddReceivedPacket(ParsePacket(*media_packet)));
+ EXPECT_TRUE(receiver_.ProcessReceivedPackets());
+ EXPECT_FALSE(receiver_.AddReceivedPacket(ParsePacket(*fec_packet)));
}
TEST_F(FlexfecReceiverTest, FailsOnUnknownMediaSsrc) {
@@ -148,8 +161,7 @@
media_packet->data[10] = 2;
media_packet->data[11] = 3;
- EXPECT_FALSE(
- receiver_.AddAndProcessReceivedPacket(ParsePacket(*media_packet)));
+ EXPECT_FALSE(receiver_.AddReceivedPacket(ParsePacket(*media_packet)));
}
TEST_F(FlexfecReceiverTest, FailsOnUnknownFecSsrc) {
@@ -167,9 +179,9 @@
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_.AddReceivedPacket(ParsePacket(*media_packet)));
+ EXPECT_TRUE(receiver_.ProcessReceivedPackets());
+ EXPECT_FALSE(receiver_.AddReceivedPacket(ParsePacket(*fec_packet)));
}
TEST_F(FlexfecReceiverTest, ReceivesMultiplePackets) {
@@ -182,16 +194,17 @@
// Receive all media packets.
for (const auto& media_packet : media_packets) {
- EXPECT_TRUE(
- receiver_.AddAndProcessReceivedPacket(ParsePacket(*media_packet)));
+ EXPECT_TRUE(receiver_.AddReceivedPacket(ParsePacket(*media_packet)));
+ EXPECT_TRUE(receiver_.ProcessReceivedPackets());
}
// Receive FEC packet.
auto fec_packet = fec_packets.front();
std::unique_ptr<Packet> packet_with_rtp_header =
packet_generator_.BuildFlexfecPacket(*fec_packet);
- EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
- ParsePacket(*packet_with_rtp_header)));
+ EXPECT_TRUE(
+ receiver_.AddReceivedPacket(ParsePacket(*packet_with_rtp_header)));
+ EXPECT_TRUE(receiver_.ProcessReceivedPackets());
}
TEST_F(FlexfecReceiverTest, RecoversFromSingleMediaLoss) {
@@ -204,7 +217,7 @@
// Receive first media packet but drop second.
auto media_it = media_packets.begin();
- EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it)));
+ receiver_.OnRtpPacket(ParsePacket(**media_it));
// Receive FEC packet and ensure recovery of lost media packet.
auto fec_it = fec_packets.begin();
@@ -216,8 +229,7 @@
.With(
Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length)))
.WillOnce(Return(true));
- EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
- ParsePacket(*packet_with_rtp_header)));
+ receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header));
}
TEST_F(FlexfecReceiverTest, RecoversFromDoubleMediaLoss) {
@@ -240,8 +252,7 @@
.With(
Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length)))
.WillOnce(Return(true));
- EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
- ParsePacket(*packet_with_rtp_header)));
+ receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header));
// Receive second FEC packet and recover second lost media packet.
fec_it++;
@@ -252,8 +263,7 @@
.With(
Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length)))
.WillOnce(Return(true));
- EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
- ParsePacket(*packet_with_rtp_header)));
+ receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header));
}
TEST_F(FlexfecReceiverTest, DoesNotRecoverFromMediaAndFecLoss) {
@@ -266,7 +276,7 @@
// Receive first media packet.
auto media_it = media_packets.begin();
- EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it)));
+ receiver_.OnRtpPacket(ParsePacket(**media_it));
// Drop second media packet and FEC packet. Do not expect call back.
}
@@ -281,7 +291,7 @@
// Receive first media packet but drop second.
auto media_it = media_packets.begin();
- EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it)));
+ receiver_.OnRtpPacket(ParsePacket(**media_it));
// Receive FEC packet and ensure recovery of lost media packet.
auto fec_it = fec_packets.begin();
@@ -293,12 +303,10 @@
.With(
Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length)))
.WillOnce(Return(true));
- EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
- ParsePacket(*packet_with_rtp_header)));
+ receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header));
// Receive FEC packet again.
- EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
- ParsePacket(*packet_with_rtp_header)));
+ receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header));
// Do not call back again.
}
@@ -320,7 +328,7 @@
// Drop every second media packet.
auto media_it = media_packets.begin();
while (media_it != media_packets.end()) {
- EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it)));
+ receiver_.OnRtpPacket(ParsePacket(**media_it));
++media_it;
if (media_it == media_packets.end()) {
break;
@@ -342,8 +350,7 @@
.With(Args<0, 1>(
ElementsAreArray((*media_it)->data, (*media_it)->length)))
.WillOnce(Return(true));
- EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
- ParsePacket(*fec_packet_with_rtp_header)));
+ receiver_.OnRtpPacket(ParsePacket(*fec_packet_with_rtp_header));
++media_it;
}
}
@@ -370,7 +377,7 @@
// Receive all other media packets.
while (media_it != media_packets.end()) {
- EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it)));
+ receiver_.OnRtpPacket(ParsePacket(**media_it));
++media_it;
}
@@ -384,8 +391,7 @@
.With(
Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length)))
.WillOnce(Return(true));
- EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
- ParsePacket(*packet_with_rtp_header)));
+ receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header));
}
TEST_F(FlexfecReceiverTest, TooDelayedFecPacketDoesNotHelp) {
@@ -410,7 +416,7 @@
// Receive all other media packets.
while (media_it != media_packets.end()) {
- EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it)));
+ receiver_.OnRtpPacket(ParsePacket(**media_it));
++media_it;
}
@@ -418,8 +424,7 @@
auto fec_it = fec_packets.begin();
std::unique_ptr<Packet> packet_with_rtp_header =
packet_generator_.BuildFlexfecPacket(**fec_it);
- EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
- ParsePacket(*packet_with_rtp_header)));
+ receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header));
// Do not expect a call back.
}
@@ -440,14 +445,10 @@
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)));
+ receiver_.OnRtpPacket(ParsePacket(**media_packet5));
+ receiver_.OnRtpPacket(ParsePacket(**media_packet2));
+ receiver_.OnRtpPacket(ParsePacket(**media_packet3));
+ receiver_.OnRtpPacket(ParsePacket(**media_packet0));
// Expect to recover lost media packets.
EXPECT_CALL(recovered_packet_receiver_,
@@ -466,8 +467,7 @@
std::unique_ptr<Packet> packet_with_rtp_header;
while (fec_it != fec_packets.end()) {
packet_with_rtp_header = packet_generator_.BuildFlexfecPacket(**fec_it);
- EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
- ParsePacket(*packet_with_rtp_header)));
+ receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header));
++fec_it;
}
}
@@ -482,7 +482,7 @@
// Receive first media packet but drop second.
auto media_it = media_packets.begin();
- EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(ParsePacket(**media_it)));
+ receiver_.OnRtpPacket(ParsePacket(**media_it));
// Receive FEC packet and ensure recovery of lost media packet.
auto fec_it = fec_packets.begin();
@@ -494,8 +494,7 @@
.With(
Args<0, 1>(ElementsAreArray((*media_it)->data, (*media_it)->length)))
.WillOnce(Return(true));
- EXPECT_TRUE(receiver_.AddAndProcessReceivedPacket(
- ParsePacket(*packet_with_rtp_header)));
+ receiver_.OnRtpPacket(ParsePacket(*packet_with_rtp_header));
// Check stats calculations.
FecPacketCounter packet_counter = receiver_.GetPacketCounter();
diff --git a/webrtc/test/fuzzers/flexfec_receiver_fuzzer.cc b/webrtc/test/fuzzers/flexfec_receiver_fuzzer.cc
index 533d0d1..58c1a8d 100644
--- a/webrtc/test/fuzzers/flexfec_receiver_fuzzer.cc
+++ b/webrtc/test/fuzzers/flexfec_receiver_fuzzer.cc
@@ -64,7 +64,7 @@
}
RtpPacketReceived parsed_packet;
if (parsed_packet.Parse(packet.get(), packet_length)) {
- receiver.AddAndProcessReceivedPacket(parsed_packet);
+ receiver.OnRtpPacket(parsed_packet);
}
}
}
