Simplify book-keeping of lost packets
Update the |cumulative_lost_| counter per received packet. The rules
follow from RFC 3550 and are fairly simple: Decrement the counter by
one for every received packet. For every in-order packet, i.e., increasing
|received_seq_max_|, add that change to |cumulative_lost_|.
Net change is zero as long as packets are received in proper sequence.
This way, GetStats() always returns an up-to-date value, independent
of the timing of RTCP report blocks.
For RTCP reports, keep a workaround to never report negative cumulative loss.
Bug: webrtc:10679
Change-Id: I47ff3bf266ff2382f405ec9828d34f7fad7068b4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/150641
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29058}
diff --git a/modules/rtp_rtcp/source/receive_statistics_impl.h b/modules/rtp_rtcp/source/receive_statistics_impl.h
index b7b9be3..8b8dde0 100644
--- a/modules/rtp_rtcp/source/receive_statistics_impl.h
+++ b/modules/rtp_rtcp/source/receive_statistics_impl.h
@@ -24,8 +24,7 @@
namespace webrtc {
-class StreamStatisticianImpl : public StreamStatistician,
- public RtpPacketSinkInterface {
+class StreamStatisticianImpl : public StreamStatistician {
public:
StreamStatisticianImpl(uint32_t ssrc,
Clock* clock,
@@ -41,12 +40,12 @@
StreamDataCounters GetReceiveStreamDataCounters() const override;
uint32_t BitrateReceived() const override;
- // Implements RtpPacketSinkInterface
- void OnRtpPacket(const RtpPacketReceived& packet) override;
-
void SetMaxReorderingThreshold(int max_reordering_threshold);
void EnableRetransmitDetection(bool enable);
+ // Updates StreamStatistician for incoming packets.
+ void UpdateCounters(const RtpPacketReceived& packet);
+
private:
bool IsRetransmitOfOldPacket(const RtpPacketReceived& packet,
int64_t now_ms) const
@@ -61,11 +60,9 @@
int64_t sequence_number,
int64_t now_ms)
RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_);
- // Updates StreamStatistician for incoming packets.
- StreamDataCounters UpdateCounters(const RtpPacketReceived& packet);
// Checks if this StreamStatistician received any rtp packets.
bool ReceivedRtpPacket() const RTC_EXCLUSIVE_LOCKS_REQUIRED(stream_lock_) {
- return received_seq_max_ >= 0;
+ return received_seq_first_ >= 0;
}
const uint32_t ssrc_;
@@ -78,7 +75,13 @@
// Stats on received RTP packets.
uint32_t jitter_q4_ RTC_GUARDED_BY(&stream_lock_);
- uint32_t cumulative_loss_ RTC_GUARDED_BY(&stream_lock_);
+ // Cumulative loss according to RFC 3550, which may be negative (and often is,
+ // if packets are reordered and there are non-RTX retransmissions).
+ int32_t cumulative_loss_ RTC_GUARDED_BY(&stream_lock_);
+ // Offset added to outgoing rtcp reports, to make ensure that the reported
+ // cumulative loss is non-negative. Reports with negative values confuse some
+ // senders, in particular, our own loss-based bandwidth estimator.
+ int32_t cumulative_loss_rtcp_offset_ RTC_GUARDED_BY(&stream_lock_);
int64_t last_receive_time_ms_ RTC_GUARDED_BY(&stream_lock_);
uint32_t last_received_timestamp_ RTC_GUARDED_BY(&stream_lock_);
@@ -94,8 +97,7 @@
StreamDataCounters receive_counters_ RTC_GUARDED_BY(&stream_lock_);
// Counter values when we sent the last report.
- uint32_t last_report_inorder_packets_ RTC_GUARDED_BY(&stream_lock_);
- uint32_t last_report_old_packets_ RTC_GUARDED_BY(&stream_lock_);
+ int32_t last_report_cumulative_loss_ RTC_GUARDED_BY(&stream_lock_);
int64_t last_report_seq_max_ RTC_GUARDED_BY(&stream_lock_);
RtcpStatistics last_reported_statistics_ RTC_GUARDED_BY(&stream_lock_);
};