Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(578)

Unified Diff: webrtc/modules/pacing/paced_sender.cc

Issue 1618333002: Temporary hack to avoid assert errors when time moves backwards. (Closed) Base URL: https://chromium.googlesource.com/external/webrtc.git@master
Patch Set: Added comment Created 4 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: webrtc/modules/pacing/paced_sender.cc
diff --git a/webrtc/modules/pacing/paced_sender.cc b/webrtc/modules/pacing/paced_sender.cc
index 121f860c7daa344f0854530ee18a9cfc0fdb5a84..25ca2cf4c51c80f6cc3da1e025dd7d21939d0d95 100644
--- a/webrtc/modules/pacing/paced_sender.cc
+++ b/webrtc/modules/pacing/paced_sender.cc
@@ -35,6 +35,15 @@ const int64_t kMaxIntervalTimeMs = 30;
// TODO(sprang): Move at least PacketQueue and MediaBudget out to separate
// files, so that we can more easily test them.
+// Note about the -1 enqueue times below:
+// This is a temporary hack to avoid crashes when the real-time clock is
+// adjusted backwards, which can happen on Android when the phone syncs the
+// clock to the network. See this bug:
+// https://bugs.chromium.org/p/webrtc/issues/detail?id=5452
+// We can't just comment out the DCHECK either, as that would lead to the sum
+// being wrong. Instead we just ignore packets from the past when we calculate
+// the average queue time.
+
namespace webrtc {
namespace paced_sender {
struct Packet {
@@ -50,7 +59,8 @@ struct Packet {
ssrc(ssrc),
sequence_number(seq_number),
capture_time_ms(capture_time_ms),
- enqueue_time_ms(enqueue_time_ms),
+ // TODO(sprang): Remove -1 option once we can guarantee monotonic clock.
+ enqueue_time_ms(enqueue_time_ms), // -1 = not valid; don't count.
bytes(length_in_bytes),
retransmission(retransmission),
enqueue_order(enqueue_order) {}
@@ -99,13 +109,16 @@ class PacketQueue {
if (!AddToDupeSet(packet))
return;
- UpdateQueueTime(packet.enqueue_time_ms);
+ if (packet.enqueue_time_ms >= time_last_updated_)
+ UpdateQueueTime(packet.enqueue_time_ms);
// Store packet in list, use pointers in priority queue for cheaper moves.
// Packets have a handle to its own iterator in the list, for easy removal
// when popping from queue.
packet_list_.push_front(packet);
std::list<Packet>::iterator it = packet_list_.begin();
+ if (packet.enqueue_time_ms < time_last_updated_)
+ it->enqueue_time_ms = -1;
it->this_it = it; // Handle for direct removal from list.
prio_queue_.push(&(*it)); // Pointer into list.
bytes_ += packet.bytes;
@@ -122,7 +135,8 @@ class PacketQueue {
void FinalizePop(const Packet& packet) {
RemoveFromDupeSet(packet);
bytes_ -= packet.bytes;
- queue_time_sum_ -= (time_last_updated_ - packet.enqueue_time_ms);
+ if (packet.enqueue_time_ms != -1)
+ queue_time_sum_ -= (time_last_updated_ - packet.enqueue_time_ms);
packet_list_.erase(packet.this_it);
RTC_DCHECK_EQ(packet_list_.size(), prio_queue_.size());
if (packet_list_.empty())
@@ -136,14 +150,18 @@ class PacketQueue {
uint64_t SizeInBytes() const { return bytes_; }
int64_t OldestEnqueueTimeMs() const {
- auto it = packet_list_.rbegin();
- if (it == packet_list_.rend())
- return 0;
- return it->enqueue_time_ms;
+ for (auto it = packet_list_.rbegin(); it != packet_list_.rend(); ++it) {
+ if (it->enqueue_time_ms != -1)
+ return it->enqueue_time_ms;
+ }
+ return 0;
}
void UpdateQueueTime(int64_t timestamp_ms) {
- RTC_DCHECK_GE(timestamp_ms, time_last_updated_);
+ // TODO(sprang): Remove this condition and reinstate a DCHECK once we have
+ // made sure all clocks are monotonic.
+ if (timestamp_ms < time_last_updated_)
+ return;
int64_t delta = timestamp_ms - time_last_updated_;
// Use packet packet_list_.size() not prio_queue_.size() here, as there
// might be an outstanding element popped from prio_queue_ currently in the
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698