diff --git a/modules/rtp_rtcp/include/ulpfec_receiver.h b/modules/rtp_rtcp/include/ulpfec_receiver.h index 6cbae52c99..38957c3f84 100644 --- a/modules/rtp_rtcp/include/ulpfec_receiver.h +++ b/modules/rtp_rtcp/include/ulpfec_receiver.h @@ -49,7 +49,7 @@ class UlpfecReceiver { // Sends the received packets to the FEC and returns all packets // (both original media and recovered) through the callback. - virtual int32_t ProcessReceivedFec() = 0; + virtual void ProcessReceivedFec() = 0; // Returns a counter describing the added and recovered packets. virtual FecPacketCounter GetPacketCounter() const = 0; diff --git a/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc b/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc index 736ccfbf6a..1e52822614 100644 --- a/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc +++ b/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc @@ -159,8 +159,7 @@ bool UlpfecReceiverImpl::AddReceivedRedPacket( return true; } -// TODO(nisse): Drop always-zero return value. -int32_t UlpfecReceiverImpl::ProcessReceivedFec() { +void UlpfecReceiverImpl::ProcessReceivedFec() { RTC_DCHECK_RUN_ON(&sequence_checker_); // If we iterate over `received_packets_` and it contains a packet that cause @@ -219,8 +218,6 @@ int32_t UlpfecReceiverImpl::ProcessReceivedFec() { recovered_packet_callback_->OnRecoveredPacket(packet->data.data(), packet->data.size()); } - - return 0; } } // namespace webrtc diff --git a/modules/rtp_rtcp/source/ulpfec_receiver_impl.h b/modules/rtp_rtcp/source/ulpfec_receiver_impl.h index 92e51530b8..133967c9aa 100644 --- a/modules/rtp_rtcp/source/ulpfec_receiver_impl.h +++ b/modules/rtp_rtcp/source/ulpfec_receiver_impl.h @@ -37,7 +37,7 @@ class UlpfecReceiverImpl : public UlpfecReceiver { bool AddReceivedRedPacket(const RtpPacketReceived& rtp_packet, uint8_t ulpfec_payload_type) override; - int32_t ProcessReceivedFec() override; + void ProcessReceivedFec() override; FecPacketCounter GetPacketCounter() const override; diff --git a/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc b/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc index b16ef3d6b5..4537d5d7aa 100644 --- a/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc +++ b/modules/rtp_rtcp/source/ulpfec_receiver_unittest.cc @@ -164,7 +164,7 @@ void UlpfecReceiverTest::InjectGarbagePacketLength(size_t fec_garbage_offset) { // packet to cause a recovery from the FEC packet. BuildAndAddRedMediaPacket(augmented_media_packets.front()); BuildAndAddRedFecPacket(fec_packets.front()); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); FecPacketCounter counter = receiver_fec_->GetPacketCounter(); EXPECT_EQ(2U, counter.num_packets); @@ -200,7 +200,7 @@ TEST_F(UlpfecReceiverTest, TwoMediaOneFec) { auto it = augmented_media_packets.begin(); BuildAndAddRedMediaPacket(*it); VerifyReconstructedMediaPacket(**it, 1); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); counter = receiver_fec_->GetPacketCounter(); EXPECT_EQ(1u, counter.num_packets); EXPECT_EQ(0u, counter.num_fec_packets); @@ -213,7 +213,7 @@ TEST_F(UlpfecReceiverTest, TwoMediaOneFec) { BuildAndAddRedFecPacket(*fec_it); ++it; VerifyReconstructedMediaPacket(**it, 1); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); counter = receiver_fec_->GetPacketCounter(); EXPECT_EQ(2u, counter.num_packets); @@ -238,7 +238,7 @@ TEST_F(UlpfecReceiverTest, TwoMediaOneFecNotUsesRecoveredPackets) { auto it = augmented_media_packets.begin(); BuildAndAddRedMediaPacket(*it, /*is_recovered=*/true); VerifyReconstructedMediaPacket(**it, 1); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); counter = receiver_fec_->GetPacketCounter(); EXPECT_EQ(1u, counter.num_packets); EXPECT_EQ(0u, counter.num_fec_packets); @@ -250,7 +250,7 @@ TEST_F(UlpfecReceiverTest, TwoMediaOneFecNotUsesRecoveredPackets) { auto fec_it = fec_packets.begin(); BuildAndAddRedFecPacket(*fec_it); ++it; - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); counter = receiver_fec_->GetPacketCounter(); EXPECT_EQ(2u, counter.num_packets); @@ -284,12 +284,12 @@ TEST_F(UlpfecReceiverTest, TwoMediaTwoFec) { auto fec_it = fec_packets.begin(); BuildAndAddRedFecPacket(*fec_it); VerifyReconstructedMediaPacket(**it, 1); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); ++fec_it; BuildAndAddRedFecPacket(*fec_it); ++it; VerifyReconstructedMediaPacket(**it, 1); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); } TEST_F(UlpfecReceiverTest, TwoFramesOneFec) { @@ -305,12 +305,12 @@ TEST_F(UlpfecReceiverTest, TwoFramesOneFec) { auto it = augmented_media_packets.begin(); BuildAndAddRedMediaPacket(augmented_media_packets.front()); VerifyReconstructedMediaPacket(**it, 1); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); // Drop one media packet. BuildAndAddRedFecPacket(fec_packets.front()); ++it; VerifyReconstructedMediaPacket(**it, 1); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); } TEST_F(UlpfecReceiverTest, OneCompleteOneUnrecoverableFrame) { @@ -327,11 +327,11 @@ TEST_F(UlpfecReceiverTest, OneCompleteOneUnrecoverableFrame) { auto it = augmented_media_packets.begin(); BuildAndAddRedMediaPacket(*it); // First frame: one packet. VerifyReconstructedMediaPacket(**it, 1); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); ++it; BuildAndAddRedMediaPacket(*it); // First packet of second frame. VerifyReconstructedMediaPacket(**it, 1); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); } TEST_F(UlpfecReceiverTest, MaxFramesOneFec) { @@ -351,12 +351,12 @@ TEST_F(UlpfecReceiverTest, MaxFramesOneFec) { for (; it != augmented_media_packets.end(); ++it) { BuildAndAddRedMediaPacket(*it); VerifyReconstructedMediaPacket(**it, 1); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); } BuildAndAddRedFecPacket(fec_packets.front()); it = augmented_media_packets.begin(); VerifyReconstructedMediaPacket(**it, 1); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); } TEST_F(UlpfecReceiverTest, TooManyFrames) { @@ -388,7 +388,7 @@ TEST_F(UlpfecReceiverTest, PacketNotDroppedTooEarly) { BuildAndAddRedMediaPacket(augmented_media_packets_batch1.front()); EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _)).Times(1); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); delayed_fec = fec_packets.front(); // Fill the FEC decoder. No packets should be dropped. @@ -403,13 +403,13 @@ TEST_F(UlpfecReceiverTest, PacketNotDroppedTooEarly) { it != augmented_media_packets_batch2.end(); ++it) { BuildAndAddRedMediaPacket(*it); EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _)).Times(1); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); } // Add the delayed FEC packet. One packet should be reconstructed. BuildAndAddRedFecPacket(delayed_fec); EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _)).Times(1); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); } TEST_F(UlpfecReceiverTest, PacketDroppedWhenTooOld) { @@ -427,7 +427,7 @@ TEST_F(UlpfecReceiverTest, PacketDroppedWhenTooOld) { BuildAndAddRedMediaPacket(augmented_media_packets_batch1.front()); EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _)).Times(1); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); delayed_fec = fec_packets.front(); // Fill the FEC decoder and force the last packet to be dropped. @@ -442,14 +442,14 @@ TEST_F(UlpfecReceiverTest, PacketDroppedWhenTooOld) { it != augmented_media_packets_batch2.end(); ++it) { BuildAndAddRedMediaPacket(*it); EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _)).Times(1); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); } // 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. BuildAndAddRedFecPacket(delayed_fec); EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _)).Times(0); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); } TEST_F(UlpfecReceiverTest, OldFecPacketDropped) { @@ -468,7 +468,7 @@ TEST_F(UlpfecReceiverTest, OldFecPacketDropped) { // Only FEC packets inserted. No packets recoverable at this time. BuildAndAddRedFecPacket(*it); EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _)).Times(0); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); } // Move unique_ptr's to media_packets for lifetime management. media_packets.insert(media_packets.end(), @@ -483,7 +483,7 @@ TEST_F(UlpfecReceiverTest, OldFecPacketDropped) { // returned. BuildAndAddRedMediaPacket(augmented_media_packets.front()); EXPECT_CALL(recovered_packet_receiver_, OnRecoveredPacket(_, _)).Times(1); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); } TEST_F(UlpfecReceiverTest, TruncatedPacketWithFBitSet) { @@ -531,12 +531,12 @@ TEST_F(UlpfecReceiverTest, MediaWithPadding) { BuildAndAddRedMediaPacket(augmented_media_packets.front()); VerifyReconstructedMediaPacket(**it, 1); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); BuildAndAddRedFecPacket(fec_packets.front()); ++it; VerifyReconstructedMediaPacket(**it, 1); - EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); + receiver_fec_->ProcessReceivedFec(); } } // namespace webrtc