admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove redundant feedback_packet_seq_num_set_ in RtpVideoSender

Browse Source 
The state this set tracks (whether this is new feedback for a packet
belonging to a media ssrc) can already be inferred from data provided
by the SendTimeHistory: if packet send time is not populated in the
feedback it's either because:
1. The feedback has already been processed
2. The receiver is sending feedback for bogus non-existent packets

If the first case, this maps to |feedback_packet_seq_num_set_|
containing the packet, if the ssrc (present in the feedback) is part
of the media ssrcs.

In the second case, this data should be ignored anyway.

Bug: webrtc:10604
Change-Id: If4828091142d68baa8dbb62be9d0b24ccaaa9546
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/135163
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27882}
This commit is contained in:
Erik Språng 2019-05-07 14:54:29 -07:00 committed by Commit Bot
parent 8f119ca0a7
commit bd7046c524
3 changed files with 15 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
call/rtp_video_sender.cc
View File

@ -49,10 +49,6 @@ RtpStreamSender::~RtpStreamSender() = default;
namespace {
static const int kMinSendSidePacketHistorySize = 600;
// Assume an average video stream has around 3 packets per frame (1 mbps / 30
// fps / 1400B) A sequence number set with size 5500 will be able to store
// packet sequence number for at least last 60 seconds.
static const int kSendSideSeqNumSetMaxSize = 5500;
// We don't do MTU discovery, so assume that we have the standard ethernet MTU.
static const size_t kPathMTU = 1500;
@ -780,30 +776,22 @@ int RtpVideoSender::ProtectionRequest(const FecProtectionParams* delta_params,
return 0;
}
void RtpVideoSender::OnPacketAdded(uint32_t ssrc, uint16_t seq_num) {
const auto ssrcs = rtp_config_.ssrcs;
if (std::find(ssrcs.begin(), ssrcs.end(), ssrc) != ssrcs.end()) {
feedback_packet_seq_num_set_.insert(seq_num);
if (feedback_packet_seq_num_set_.size() > kSendSideSeqNumSetMaxSize) {
RTC_LOG(LS_WARNING) << "Feedback packet sequence number set exceed it's "
"max size', will get reset.";
feedback_packet_seq_num_set_.clear();
}
}
}
void RtpVideoSender::OnPacketFeedbackVector(
const std::vector<PacketFeedback>& packet_feedback_vector) {
if (fec_controller_->UseLossVectorMask()) {
rtc::CritScope cs(&crit_);
for (const PacketFeedback& packet : packet_feedback_vector) {
const bool lost = packet.arrival_time_ms == PacketFeedback::kNotReceived;
// Lost feedbacks are not considered to be lost packets.
auto it = feedback_packet_seq_num_set_.find(packet.sequence_number);
if (it != feedback_packet_seq_num_set_.end()) {
loss_mask_vector_.push_back(lost);
feedback_packet_seq_num_set_.erase(it);
if (packet.send_time_ms == PacketFeedback::kNoSendTime || !packet.ssrc ||
std::find(rtp_config_.ssrcs.begin(), rtp_config_.ssrcs.end(),
*packet.ssrc) == rtp_config_.ssrcs.end()) {
// If packet send time is missing, the feedback for this packet has
// probably already been processed, so ignore it.
// If packet does not belong to a registered media ssrc, we are also
// not interested in it.
continue;
}
loss_mask_vector_.push_back(packet.arrival_time_ms ==
PacketFeedback::kNotReceived);
}
}

3
call/rtp_video_sender.h
View File

@ -146,7 +146,7 @@ class RtpVideoSender : public RtpVideoSenderInterface,
rtc::ArrayView<const uint16_t> sequence_numbers) const override;
// From PacketFeedbackObserver.
void OnPacketAdded(uint32_t ssrc, uint16_t seq_num) override;
void OnPacketAdded(uint32_t ssrc, uint16_t seq_num) override {}
void OnPacketFeedbackVector(
const std::vector<PacketFeedback>& packet_feedback_vector) override;
@ -191,7 +191,6 @@ class RtpVideoSender : public RtpVideoSenderInterface,
uint32_t protection_bitrate_bps_;
uint32_t encoder_target_rate_bps_;
std::unordered_set<uint16_t> feedback_packet_seq_num_set_;
std::vector<bool> loss_mask_vector_ RTC_GUARDED_BY(crit_);
std::vector<FrameCounts> frame_counts_ RTC_GUARDED_BY(crit_);

4
call/rtp_video_sender_unittest.cc
View File

@ -433,6 +433,8 @@ TEST(RtpVideoSenderTest, DoesNotRetrasmitAckedPackets) {
EXPECT_EQ(
EncodedImageCallback::Result::OK,
test.router()->OnEncodedImage(encoded_image, nullptr, nullptr).error);
const int64_t send_time_ms = test.clock().TimeInMilliseconds();
test.clock().AdvanceTimeMilliseconds(33);
ASSERT_TRUE(event.Wait(kTimeoutMs));
@ -473,11 +475,13 @@ TEST(RtpVideoSenderTest, DoesNotRetrasmitAckedPackets) {
transport_sequence_numbers[0]);
received_packet_feedback.rtp_sequence_number = rtp_sequence_numbers[0];
received_packet_feedback.ssrc = kSsrc1;
received_packet_feedback.send_time_ms = send_time_ms;
PacketFeedback lost_packet_feedback(PacketFeedback::kNotReceived,
transport_sequence_numbers[1]);
lost_packet_feedback.rtp_sequence_number = rtp_sequence_numbers[1];
lost_packet_feedback.ssrc = kSsrc1;
lost_packet_feedback.send_time_ms = send_time_ms;
std::vector<PacketFeedback> feedback_vector = {received_packet_feedback,
lost_packet_feedback};