diff --git a/webrtc/test/fuzzers/transport_feedback_packet_loss_tracker_fuzzer.cc b/webrtc/test/fuzzers/transport_feedback_packet_loss_tracker_fuzzer.cc index c8e261982d..bcb075d046 100644 --- a/webrtc/test/fuzzers/transport_feedback_packet_loss_tracker_fuzzer.cc +++ b/webrtc/test/fuzzers/transport_feedback_packet_loss_tracker_fuzzer.cc @@ -11,6 +11,7 @@ #include #include "webrtc/base/array_view.h" +#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "webrtc/modules/rtp_rtcp/source/byte_io.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include "webrtc/voice_engine/transport_feedback_packet_loss_tracker.h" @@ -47,22 +48,24 @@ class TransportFeedbackGenerator { explicit TransportFeedbackGenerator(const uint8_t** data, size_t* size) : data_(data), size_(size) {} - bool GetNextTransportFeedback(rtcp::TransportFeedback* feedback) { - uint16_t base_seq_num = 0; - if (!ReadData(&base_seq_num)) { - return false; - } - constexpr int64_t kBaseTimeUs = 1234; // Irrelevant to this test. - feedback->SetBase(base_seq_num, kBaseTimeUs); + bool GetNextTransportFeedbackVector( + std::vector* feedback_vector) { + RTC_CHECK(feedback_vector->empty()); uint16_t remaining_packets = 0; - if (!ReadData(&remaining_packets)) + if (!ReadData(&remaining_packets)) { return false; - // Range is [0x00001 : 0x10000], but we keep it 0x0000 to 0xffff for now, - // and add the last status as RECEIVED. That is because of a limitation - // that says that the last status cannot be LOST. + } + + if (remaining_packets == 0) { + return true; + } + + uint16_t seq_num; + if (!ReadData(&seq_num)) { // Fuzz base sequence number. + return false; + } - uint16_t seq_num = base_seq_num; while (remaining_packets > 0) { uint8_t status_byte = 0; if (!ReadData(&status_byte)) { @@ -70,18 +73,18 @@ class TransportFeedbackGenerator { } // Each status byte contains 8 statuses. for (size_t i = 0; i < 8 && remaining_packets > 0; ++i) { + // Any positive integer signals reception. kNotReceived signals loss. + // Other values are just illegal. + constexpr int64_t kArrivalTimeMs = 1234; + const bool received = (status_byte & (0x01 << i)); - if (received) { - feedback->AddReceivedPacket(seq_num, kBaseTimeUs); - } - ++seq_num; + feedback_vector->emplace_back(PacketFeedback( + received ? kArrivalTimeMs : PacketFeedback::kNotReceived, + seq_num++)); --remaining_packets; } } - // As mentioned above, all feedbacks must report with a received packet. - feedback->AddReceivedPacket(seq_num, kBaseTimeUs); - return true; } @@ -236,12 +239,13 @@ bool FuzzTransportFeedbackBlock( TransportFeedbackGenerator feedback_generator(data, size); for (size_t i = 0; i < feedbacks_num; i++) { - rtcp::TransportFeedback feedback; - bool may_continue = feedback_generator.GetNextTransportFeedback(&feedback); + std::vector feedback_vector; + bool may_continue = + feedback_generator.GetNextTransportFeedbackVector(&feedback_vector); if (!may_continue) { return false; } - tracker->OnReceivedTransportFeedback(feedback); + tracker->OnNewTransportFeedbackVector(feedback_vector); tracker->Validate(); } diff --git a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc b/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc index 637794398e..6cdeec2f1f 100644 --- a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc +++ b/webrtc/voice_engine/transport_feedback_packet_loss_tracker.cc @@ -15,6 +15,7 @@ #include "webrtc/base/checks.h" #include "webrtc/base/mod_ops.h" +#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" namespace { @@ -97,23 +98,17 @@ void TransportFeedbackPacketLossTracker::OnPacketAdded(uint16_t seq_num, } } -void TransportFeedbackPacketLossTracker::OnReceivedTransportFeedback( - const rtcp::TransportFeedback& feedback) { - const auto& status_vector = feedback.GetStatusVector(); - const uint16_t base_seq_num = feedback.GetBaseSequence(); - - uint16_t seq_num = base_seq_num; - for (size_t i = 0; i < status_vector.size(); ++i, ++seq_num) { - const auto& it = packet_status_window_.find(seq_num); +void TransportFeedbackPacketLossTracker::OnNewTransportFeedbackVector( + const std::vector& packet_feedback_vector) { + for (const PacketFeedback& packet : packet_feedback_vector) { + const auto& it = packet_status_window_.find(packet.sequence_number); // Packets which aren't at least marked as unacked either do not belong to // this media stream, or have been shifted out of window. if (it == packet_status_window_.end()) continue; - const bool lost = - status_vector[i] == - webrtc::rtcp::TransportFeedback::StatusSymbol::kNotReceived; + const bool lost = packet.arrival_time_ms == PacketFeedback::kNotReceived; const PacketStatus packet_status = lost ? PacketStatus::Lost : PacketStatus::Received; diff --git a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.h b/webrtc/voice_engine/transport_feedback_packet_loss_tracker.h index b8fd0cc4bb..231fb7070a 100644 --- a/webrtc/voice_engine/transport_feedback_packet_loss_tracker.h +++ b/webrtc/voice_engine/transport_feedback_packet_loss_tracker.h @@ -22,6 +22,8 @@ namespace rtcp { class TransportFeedback; } +struct PacketFeedback; + class TransportFeedbackPacketLossTracker final { public: // * We count up to |max_window_size_ms| from the sent @@ -36,7 +38,8 @@ class TransportFeedbackPacketLossTracker final { void OnPacketAdded(uint16_t seq_num, int64_t send_time_ms); - void OnReceivedTransportFeedback(const rtcp::TransportFeedback& feedback); + void OnNewTransportFeedbackVector( + const std::vector& packet_feedbacks_vector); // Returns the packet loss rate, if the window has enough packet statuses to // reliably compute it. Otherwise, returns empty. diff --git a/webrtc/voice_engine/transport_feedback_packet_loss_tracker_unittest.cc b/webrtc/voice_engine/transport_feedback_packet_loss_tracker_unittest.cc index cca1ff2b5a..1e463dd611 100644 --- a/webrtc/voice_engine/transport_feedback_packet_loss_tracker_unittest.cc +++ b/webrtc/voice_engine/transport_feedback_packet_loss_tracker_unittest.cc @@ -14,6 +14,7 @@ #include #include "webrtc/base/checks.h" +#include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include "webrtc/test/gmock.h" #include "webrtc/test/gtest.h" @@ -28,8 +29,6 @@ constexpr int64_t kDefaultMaxWindowSizeMs = 500 * kDefaultSendIntervalMs; class TransportFeedbackPacketLossTrackerTest : public ::testing::TestWithParam { - using TransportFeedback = webrtc::rtcp::TransportFeedback; - public: TransportFeedbackPacketLossTrackerTest() = default; virtual ~TransportFeedbackPacketLossTrackerTest() = default; @@ -75,28 +74,19 @@ class TransportFeedbackPacketLossTrackerTest TransportFeedbackPacketLossTracker* tracker, uint16_t base_sequence_num, const std::vector& reception_status_vec) { - const int64_t kBaseTimeUs = 1234; // Irrelevant to this test. - TransportFeedback test_feedback; - test_feedback.SetBase(base_sequence_num, kBaseTimeUs); - uint16_t sequence_num = base_sequence_num; - for (bool status : reception_status_vec) { - if (status) - test_feedback.AddReceivedPacket(sequence_num, kBaseTimeUs); - ++sequence_num; + // Any positive integer signals reception. kNotReceived signals loss. + // Other values are just illegal. + constexpr int64_t kArrivalTimeMs = 1234; + + std::vector packet_feedback_vector; + uint16_t seq_num = base_sequence_num; + for (bool received : reception_status_vec) { + packet_feedback_vector.emplace_back(PacketFeedback( + received ? kArrivalTimeMs : PacketFeedback::kNotReceived, seq_num)); + ++seq_num; } - // TransportFeedback imposes some limitations on what constitutes a legal - // status vector. For instance, the vector cannot terminate in a lost - // packet. Make sure all limitations are abided by. - RTC_CHECK_EQ(base_sequence_num, test_feedback.GetBaseSequence()); - const auto& vec = test_feedback.GetStatusVector(); - RTC_CHECK_EQ(reception_status_vec.size(), vec.size()); - for (size_t i = 0; i < reception_status_vec.size(); i++) { - RTC_CHECK_EQ(reception_status_vec[i], - vec[i] != TransportFeedback::StatusSymbol::kNotReceived); - } - - tracker->OnReceivedTransportFeedback(test_feedback); + tracker->OnNewTransportFeedbackVector(packet_feedback_vector); tracker->Validate(); }