Move pacer to fully use webrtc::Clock instead of webrtc::TickTime.

This required rewriting the send-side delay stats api to be callback based, as otherwise the SuspendBelowMinBitrate test started flaking much more frequently since it had lock order inversion problems.

R=pbos@webrtc.org, tommi@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/21869005

git-svn-id: http://webrtc.googlecode.com/svn/trunk@6664 4adac7df-926f-26a2-2b94-8c16560cd09d

webrtc/build/tsan_suppressions.cc

@@ -71,7 +71,6 @@ char kTSanDefaultSuppressions[] =
 "deadlock:webrtc::RTPSenderAudio::RegisterAudioPayload\n"
 "deadlock:webrtc::test::UdpSocketManagerPosixImpl::RemoveSocket\n"
 "deadlock:webrtc::vcm::VideoReceiver::RegisterPacketRequestCallback\n"
-"deadlock:webrtc::VideoSendStreamTest_SuspendBelowMinBitrate_Test::TestBody\n"
 "deadlock:webrtc::ViECaptureImpl::ConnectCaptureDevice\n"
 "deadlock:webrtc::ViEChannel::StartSend\n"
 "deadlock:webrtc::ViECodecImpl::GetSendSideDelay\n"

webrtc/common_types.h

@@ -269,6 +269,15 @@ class FrameCountObserver {
                                  const unsigned int ssrc) = 0;
 };
 
+// Callback, used to notify an observer whenever the send-side delay is updated.
+class SendSideDelayObserver {
+ public:
+  virtual ~SendSideDelayObserver() {}
+  virtual void SendSideDelayUpdated(int avg_delay_ms,
+                                    int max_delay_ms,
+                                    uint32_t ssrc) = 0;
+};
+
 // ==================================================================
 // Voice specific types
 // ==================================================================

webrtc/config.h

@@ -34,10 +34,17 @@ struct RtpStatistics {
 };
 
 struct StreamStats {
-  StreamStats() : key_frames(0), delta_frames(0), bitrate_bps(0) {}
+  StreamStats()
+      : key_frames(0),
+        delta_frames(0),
+        bitrate_bps(0),
+        avg_delay_ms(0),
+        max_delay_ms(0) {}
   uint32_t key_frames;
   uint32_t delta_frames;
   int32_t bitrate_bps;
+  int avg_delay_ms;
+  int max_delay_ms;
   StreamDataCounters rtp_stats;
   RtcpStatistics rtcp_stats;
 };

webrtc/modules/pacing/include/paced_sender.h

@@ -17,7 +17,6 @@
 #include "webrtc/modules/interface/module.h"
 #include "webrtc/system_wrappers/interface/scoped_ptr.h"
 #include "webrtc/system_wrappers/interface/thread_annotations.h"
-#include "webrtc/system_wrappers/interface/tick_util.h"
 #include "webrtc/typedefs.h"
 
 namespace webrtc {
@@ -147,8 +146,8 @@ class PacedSender : public Module {
   scoped_ptr<paced_sender::IntervalBudget> padding_budget_
       GUARDED_BY(critsect_);
 
-  TickTime time_last_update_ GUARDED_BY(critsect_);
+  int64_t time_last_update_ GUARDED_BY(critsect_);
-  TickTime time_last_send_ GUARDED_BY(critsect_);
+  int64_t time_last_send_ GUARDED_BY(critsect_);
   int64_t capture_time_ms_last_queued_ GUARDED_BY(critsect_);
   int64_t capture_time_ms_last_sent_ GUARDED_BY(critsect_);
 

webrtc/modules/pacing/paced_sender.cc

@@ -142,7 +142,7 @@ PacedSender::PacedSender(Clock* clock,
       max_queue_length_ms_(kDefaultMaxQueueLengthMs),
       media_budget_(new paced_sender::IntervalBudget(max_bitrate_kbps)),
       padding_budget_(new paced_sender::IntervalBudget(min_bitrate_kbps)),
-      time_last_update_(TickTime::Now()),
+      time_last_update_(clock->TimeInMilliseconds()),
       capture_time_ms_last_queued_(0),
       capture_time_ms_last_sent_(0),
       high_priority_packets_(new paced_sender::PacketList),
@@ -248,8 +248,7 @@ int PacedSender::QueueInMs() const {
 
 int32_t PacedSender::TimeUntilNextProcess() {
   CriticalSectionScoped cs(critsect_.get());
-  int64_t elapsed_time_ms =
+  int64_t elapsed_time_ms = clock_->TimeInMilliseconds() - time_last_update_;
-      (TickTime::Now() - time_last_update_).Milliseconds();
   if (elapsed_time_ms <= 0) {
     return kMinPacketLimitMs;
   }
@@ -260,9 +259,9 @@ int32_t PacedSender::TimeUntilNextProcess() {
 }
 
 int32_t PacedSender::Process() {
-  TickTime now = TickTime::Now();
+  int64_t now = clock_->TimeInMilliseconds();
   CriticalSectionScoped cs(critsect_.get());
-  int elapsed_time_ms = (now - time_last_update_).Milliseconds();
+  int elapsed_time_ms = now - time_last_update_;
   time_last_update_ = now;
   if (!enabled_) {
     return 0;
@@ -335,7 +334,7 @@ bool PacedSender::ShouldSendNextPacket(paced_sender::PacketList** packet_list) {
   if (media_budget_->bytes_remaining() <= 0) {
     // All bytes consumed for this interval.
     // Check if we have not sent in a too long time.
-    if ((TickTime::Now() - time_last_send_).Milliseconds() >
+    if (clock_->TimeInMilliseconds() - time_last_send_ >
         kMaxQueueTimeWithoutSendingMs) {
       if (!high_priority_packets_->empty()) {
         *packet_list = high_priority_packets_.get();
@@ -389,7 +388,7 @@ paced_sender::Packet PacedSender::GetNextPacketFromList(
 
 // MUST have critsect_ when calling.
 void PacedSender::UpdateMediaBytesSent(int num_bytes) {
-  time_last_send_ = TickTime::Now();
+  time_last_send_ = clock_->TimeInMilliseconds();
   media_budget_->UseBudget(num_bytes);
   padding_budget_->UseBudget(num_bytes);
 }

webrtc/modules/pacing/paced_sender_unittest.cc

@@ -58,7 +58,6 @@ class PacedSenderTest : public ::testing::Test {
  protected:
   PacedSenderTest() : clock_(123456) {
     srand(0);
-    TickTime::UseFakeClock(123456);
     // Need to initialize PacedSender after we initialize clock.
     send_bucket_.reset(
         new PacedSender(
@@ -99,10 +98,8 @@ TEST_F(PacedSenderTest, QueuePacket) {
   EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
   EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0);
   clock_.AdvanceTimeMilliseconds(4);
-  TickTime::AdvanceFakeClock(4);
   EXPECT_EQ(1, send_bucket_->TimeUntilNextProcess());
   clock_.AdvanceTimeMilliseconds(1);
-  TickTime::AdvanceFakeClock(1);
   EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
   EXPECT_CALL(callback_, TimeToSendPacket(
       ssrc, sequence_number++, queued_packet_timestamp, false))
@@ -137,7 +134,6 @@ TEST_F(PacedSenderTest, PaceQueuedPackets) {
   for (int k = 0; k < 10; ++k) {
     EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
     clock_.AdvanceTimeMilliseconds(5);
-    TickTime::AdvanceFakeClock(5);
     EXPECT_CALL(callback_,
         TimeToSendPacket(ssrc, _, _, false))
         .Times(3)
@@ -147,7 +143,6 @@ TEST_F(PacedSenderTest, PaceQueuedPackets) {
   }
   EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
   clock_.AdvanceTimeMilliseconds(5);
-  TickTime::AdvanceFakeClock(5);
   EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
   EXPECT_EQ(0, send_bucket_->Process());
   SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++,
@@ -185,7 +180,6 @@ TEST_F(PacedSenderTest, PaceQueuedPacketsWithDuplicates) {
   for (int k = 0; k < 10; ++k) {
     EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
     clock_.AdvanceTimeMilliseconds(5);
-    TickTime::AdvanceFakeClock(5);
 
     for (int i = 0; i < 3; ++i) {
       EXPECT_CALL(callback_, TimeToSendPacket(ssrc, queued_sequence_number++,
@@ -199,7 +193,6 @@ TEST_F(PacedSenderTest, PaceQueuedPacketsWithDuplicates) {
   }
   EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
   clock_.AdvanceTimeMilliseconds(5);
-  TickTime::AdvanceFakeClock(5);
   EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
   EXPECT_EQ(0, send_bucket_->Process());
   SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++,
@@ -233,7 +226,6 @@ TEST_F(PacedSenderTest, CanQueuePacketsWithSameSequenceNumberOnDifferentSsrcs) {
                       false);
 
   clock_.AdvanceTimeMilliseconds(1000);
-  TickTime::AdvanceFakeClock(1000);
   send_bucket_->Process();
 }
 
@@ -253,7 +245,6 @@ TEST_F(PacedSenderTest, Padding) {
   EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0);
   EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
   clock_.AdvanceTimeMilliseconds(5);
-  TickTime::AdvanceFakeClock(5);
   EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
   EXPECT_EQ(0, send_bucket_->Process());
 
@@ -262,7 +253,6 @@ TEST_F(PacedSenderTest, Padding) {
       WillOnce(Return(250));
   EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
   clock_.AdvanceTimeMilliseconds(5);
-  TickTime::AdvanceFakeClock(5);
   EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
   EXPECT_EQ(0, send_bucket_->Process());
 }
@@ -274,13 +264,11 @@ TEST_F(PacedSenderTest, NoPaddingWhenDisabled) {
   EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0);
   EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
   clock_.AdvanceTimeMilliseconds(5);
-  TickTime::AdvanceFakeClock(5);
   EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
   EXPECT_EQ(0, send_bucket_->Process());
   EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0);
   EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
   clock_.AdvanceTimeMilliseconds(5);
-  TickTime::AdvanceFakeClock(5);
   EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
   EXPECT_EQ(0, send_bucket_->Process());
 }
@@ -297,7 +285,6 @@ TEST_F(PacedSenderTest, VerifyPaddingUpToBitrate) {
     SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++,
                         capture_time_ms, 250, false);
     clock_.AdvanceTimeMilliseconds(kTimeStep);
-    TickTime::AdvanceFakeClock(kTimeStep);
     EXPECT_CALL(callback_, TimeToSendPadding(250)).Times(1).
         WillOnce(Return(250));
     send_bucket_->Process();
@@ -323,7 +310,6 @@ TEST_F(PacedSenderTest, VerifyAverageBitrateVaryingMediaPayload) {
                                           media_payload, false));
     media_bytes += media_payload;
     clock_.AdvanceTimeMilliseconds(kTimeStep);
-    TickTime::AdvanceFakeClock(kTimeStep);
     send_bucket_->Process();
   }
   EXPECT_NEAR(kTargetBitrate, 8 * (media_bytes + callback.padding_sent()) /
@@ -365,7 +351,6 @@ TEST_F(PacedSenderTest, Priority) {
 
   EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
   clock_.AdvanceTimeMilliseconds(5);
-  TickTime::AdvanceFakeClock(5);
   EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
   EXPECT_EQ(0, send_bucket_->Process());
 
@@ -376,7 +361,6 @@ TEST_F(PacedSenderTest, Priority) {
 
   EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
   clock_.AdvanceTimeMilliseconds(5);
-  TickTime::AdvanceFakeClock(5);
   EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
   EXPECT_EQ(0, send_bucket_->Process());
 }
@@ -408,7 +392,6 @@ TEST_F(PacedSenderTest, Pause) {
       ssrc, sequence_number++, capture_time_ms, 250, false));
 
   clock_.AdvanceTimeMilliseconds(10000);
-  TickTime::AdvanceFakeClock(10000);
   int64_t second_capture_time_ms = clock_.TimeInMilliseconds();
 
   // Expect everything to be queued.
@@ -425,7 +408,6 @@ TEST_F(PacedSenderTest, Pause) {
 
   for (int i = 0; i < 10; ++i) {
     clock_.AdvanceTimeMilliseconds(5);
-    TickTime::AdvanceFakeClock(5);
     EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
     EXPECT_EQ(0, send_bucket_->Process());
   }
@@ -438,7 +420,6 @@ TEST_F(PacedSenderTest, Pause) {
 
   EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
   clock_.AdvanceTimeMilliseconds(5);
-  TickTime::AdvanceFakeClock(5);
   EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
   EXPECT_EQ(0, send_bucket_->Process());
 
@@ -448,7 +429,6 @@ TEST_F(PacedSenderTest, Pause) {
       .WillRepeatedly(Return(true));
   EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
   clock_.AdvanceTimeMilliseconds(5);
-  TickTime::AdvanceFakeClock(5);
   EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
   EXPECT_EQ(0, send_bucket_->Process());
   EXPECT_EQ(0, send_bucket_->QueueInMs());
@@ -467,7 +447,6 @@ TEST_F(PacedSenderTest, ResendPacket) {
                                         250,
                                         false));
   clock_.AdvanceTimeMilliseconds(1);
-  TickTime::AdvanceFakeClock(1);
   EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority,
                                         ssrc,
                                         sequence_number + 1,
@@ -475,7 +454,6 @@ TEST_F(PacedSenderTest, ResendPacket) {
                                         250,
                                         false));
   clock_.AdvanceTimeMilliseconds(9999);
-  TickTime::AdvanceFakeClock(9999);
   EXPECT_EQ(clock_.TimeInMilliseconds() - capture_time_ms,
             send_bucket_->QueueInMs());
   // Fails to send first packet so only one call.
@@ -484,7 +462,6 @@ TEST_F(PacedSenderTest, ResendPacket) {
       .Times(1)
       .WillOnce(Return(false));
   clock_.AdvanceTimeMilliseconds(10000);
-  TickTime::AdvanceFakeClock(10000);
   send_bucket_->Process();
 
   // Queue remains unchanged.
@@ -501,7 +478,6 @@ TEST_F(PacedSenderTest, ResendPacket) {
       .Times(1)
       .WillOnce(Return(false));
   clock_.AdvanceTimeMilliseconds(10000);
-  TickTime::AdvanceFakeClock(10000);
   send_bucket_->Process();
 
   // Queue is reduced by 1 packet.
@@ -514,7 +490,6 @@ TEST_F(PacedSenderTest, ResendPacket) {
       .Times(1)
       .WillOnce(Return(true));
   clock_.AdvanceTimeMilliseconds(10000);
-  TickTime::AdvanceFakeClock(10000);
   send_bucket_->Process();
   EXPECT_EQ(0, send_bucket_->QueueInMs());
 }
@@ -535,7 +510,6 @@ TEST_F(PacedSenderTest, MaxQueueLength) {
   }
 
   clock_.AdvanceTimeMilliseconds(2001);
-  TickTime::AdvanceFakeClock(2001);
   SendAndExpectPacket(PacedSender::kNormalPriority,
                       ssrc,
                       sequence_number++,
@@ -546,7 +520,7 @@ TEST_F(PacedSenderTest, MaxQueueLength) {
   send_bucket_->Process();
   EXPECT_EQ(0, send_bucket_->QueueInMs());
   clock_.AdvanceTimeMilliseconds(31);
-  TickTime::AdvanceFakeClock(31);
+
   send_bucket_->Process();
 }
 
@@ -564,7 +538,6 @@ TEST_F(PacedSenderTest, QueueTimeGrowsOverTime) {
                       false);
 
   clock_.AdvanceTimeMilliseconds(500);
-  TickTime::AdvanceFakeClock(500);
   EXPECT_EQ(500, send_bucket_->QueueInMs());
   send_bucket_->Process();
   EXPECT_EQ(0, send_bucket_->QueueInMs());

webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h

@@ -47,8 +47,8 @@ class RtpRtcp : public Module {
     *  intra_frame_callback - Called when the receiver request a intra frame.
     *  bandwidth_callback   - Called when we receive a changed estimate from
     *                         the receiver of out stream.
-    *  audio_messages       - Telehone events. May not be NULL; default callback
+    *  audio_messages       - Telephone events. May not be NULL; default
-    *                         will do nothing.
+    *                         callback will do nothing.
     *  remote_bitrate_estimator - Estimates the bandwidth available for a set of
     *                             streams from the same client.
     *  paced_sender             - Spread any bursts of packets into smaller
@@ -69,6 +69,7 @@ class RtpRtcp : public Module {
     PacedSender* paced_sender;
     BitrateStatisticsObserver* send_bitrate_observer;
     FrameCountObserver* send_frame_count_observer;
+    SendSideDelayObserver* send_side_delay_observer;
   };
 
   /*

webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc

@@ -39,7 +39,8 @@ RtpRtcp::Configuration::Configuration()
       remote_bitrate_estimator(NULL),
       paced_sender(NULL),
       send_bitrate_observer(NULL),
-      send_frame_count_observer(NULL) {
+      send_frame_count_observer(NULL),
+      send_side_delay_observer(NULL) {
 }
 
 RtpRtcp* RtpRtcp::CreateRtpRtcp(const RtpRtcp::Configuration& configuration) {
@@ -64,7 +65,8 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration)
                   configuration.audio_messages,
                   configuration.paced_sender,
                   configuration.send_bitrate_observer,
-                  configuration.send_frame_count_observer),
+                  configuration.send_frame_count_observer,
+                  configuration.send_side_delay_observer),
       rtcp_sender_(configuration.id,
                    configuration.audio,
                    configuration.clock,

webrtc/modules/rtp_rtcp/source/rtp_sender.cc

@@ -47,7 +47,8 @@ RTPSender::RTPSender(const int32_t id,
                      RtpAudioFeedback* audio_feedback,
                      PacedSender* paced_sender,
                      BitrateStatisticsObserver* bitrate_callback,
-                     FrameCountObserver* frame_count_observer)
+                     FrameCountObserver* frame_count_observer,
+                     SendSideDelayObserver* send_side_delay_observer)
     : clock_(clock),
       bitrate_sent_(clock, this),
       id_(id),
@@ -75,6 +76,7 @@ RTPSender::RTPSender(const int32_t id,
       rtp_stats_callback_(NULL),
       bitrate_callback_(bitrate_callback),
       frame_count_observer_(frame_count_observer),
+      send_side_delay_observer_(send_side_delay_observer),
       // RTP variables
       start_timestamp_forced_(false),
       start_timestamp_(0),
@@ -164,9 +166,7 @@ uint32_t RTPSender::NackOverheadRate() const {
 
 bool RTPSender::GetSendSideDelay(int* avg_send_delay_ms,
                                  int* max_send_delay_ms) const {
-  if (!SendingMedia())
+  CriticalSectionScoped lock(statistics_crit_.get());
-    return false;
-  CriticalSectionScoped cs(statistics_crit_.get());
   SendDelayMap::const_iterator it = send_delays_.upper_bound(
       clock_->TimeInMilliseconds() - kSendSideDelayWindowMs);
   if (it == send_delays_.end())
@@ -997,10 +997,26 @@ int32_t RTPSender::SendToNetwork(
 }
 
 void RTPSender::UpdateDelayStatistics(int64_t capture_time_ms, int64_t now_ms) {
+  uint32_t ssrc;
+  int avg_delay_ms = 0;
+  int max_delay_ms = 0;
+  {
+    CriticalSectionScoped lock(send_critsect_);
+    ssrc = ssrc_;
+  }
+  {
     CriticalSectionScoped cs(statistics_crit_.get());
+    // TODO(holmer): Compute this iteratively instead.
     send_delays_[now_ms] = now_ms - capture_time_ms;
     send_delays_.erase(send_delays_.begin(),
-                     send_delays_.lower_bound(now_ms - kSendSideDelayWindowMs));
+                       send_delays_.lower_bound(now_ms -
+                       kSendSideDelayWindowMs));
+  }
+  if (send_side_delay_observer_ &&
+      GetSendSideDelay(&avg_delay_ms, &max_delay_ms)) {
+    send_side_delay_observer_->SendSideDelayUpdated(avg_delay_ms,
+        max_delay_ms, ssrc);
+  }
 }
 
 void RTPSender::ProcessBitrate() {

webrtc/modules/rtp_rtcp/source/rtp_sender.h

@@ -71,7 +71,8 @@ class RTPSender : public RTPSenderInterface, public Bitrate::Observer {
             Transport *transport, RtpAudioFeedback *audio_feedback,
             PacedSender *paced_sender,
             BitrateStatisticsObserver* bitrate_callback,
-            FrameCountObserver* frame_count_observer);
+            FrameCountObserver* frame_count_observer,
+            SendSideDelayObserver* send_side_delay_observer);
   virtual ~RTPSender();
 
   void ProcessBitrate();
@@ -379,6 +380,7 @@ class RTPSender : public RTPSenderInterface, public Bitrate::Observer {
   StreamDataCountersCallback* rtp_stats_callback_ GUARDED_BY(statistics_crit_);
   BitrateStatisticsObserver* const bitrate_callback_;
   FrameCountObserver* const frame_count_observer_;
+  SendSideDelayObserver* const send_side_delay_observer_;
 
   // RTP variables
   bool start_timestamp_forced_ GUARDED_BY(send_critsect_);

webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc

@@ -94,7 +94,7 @@ class RtpSenderTest : public ::testing::Test {
 
   virtual void SetUp() {
     rtp_sender_.reset(new RTPSender(0, false, &fake_clock_, &transport_, NULL,
-                                    &mock_paced_sender_, NULL, NULL));
+                                    &mock_paced_sender_, NULL, NULL, NULL));
     rtp_sender_->SetSequenceNumber(kSeqNum);
   }
 
@@ -672,7 +672,7 @@ TEST_F(RtpSenderTest, SendPadding) {
 TEST_F(RtpSenderTest, SendRedundantPayloads) {
   MockTransport transport;
   rtp_sender_.reset(new RTPSender(0, false, &fake_clock_, &transport, NULL,
-                                  &mock_paced_sender_, NULL, NULL));
+                                  &mock_paced_sender_, NULL, NULL, NULL));
   rtp_sender_->SetSequenceNumber(kSeqNum);
   // Make all packets go through the pacer.
   EXPECT_CALL(mock_paced_sender_,
@@ -818,7 +818,7 @@ TEST_F(RtpSenderTest, FrameCountCallbacks) {
   } callback;
 
   rtp_sender_.reset(new RTPSender(0, false, &fake_clock_, &transport_, NULL,
-                                  &mock_paced_sender_, NULL, &callback));
+                                  &mock_paced_sender_, NULL, &callback, NULL));
 
   char payload_name[RTP_PAYLOAD_NAME_SIZE] = "GENERIC";
   const uint8_t payload_type = 127;
@@ -867,7 +867,7 @@ TEST_F(RtpSenderTest, BitrateCallbacks) {
     BitrateStatistics bitrate_;
   } callback;
   rtp_sender_.reset(new RTPSender(0, false, &fake_clock_, &transport_, NULL,
-                                  &mock_paced_sender_, &callback, NULL));
+                                  &mock_paced_sender_, &callback, NULL, NULL));
 
   // Simulate kNumPackets sent with kPacketInterval ms intervals.
   const uint32_t kNumPackets = 15;
@@ -923,7 +923,7 @@ class RtpSenderAudioTest : public RtpSenderTest {
   virtual void SetUp() {
     payload_ = kAudioPayload;
     rtp_sender_.reset(new RTPSender(0, true, &fake_clock_, &transport_, NULL,
-                                    &mock_paced_sender_, NULL, NULL));
+                                    &mock_paced_sender_, NULL, NULL, NULL));
     rtp_sender_->SetSequenceNumber(kSeqNum);
   }
 };

webrtc/video/end_to_end_tests.cc

@@ -1345,9 +1345,6 @@ TEST_F(EndToEndTest, GetStats) {
       send_stats_filled_["NumStreams"] |=
           stats.substreams.size() == expected_send_ssrcs_.size();
 
-      send_stats_filled_["Delay"] |=
-          stats.avg_delay_ms != 0 || stats.max_delay_ms != 0;
-
       for (std::map<uint32_t, StreamStats>::const_iterator it =
                stats.substreams.begin();
            it != stats.substreams.end();
@@ -1380,6 +1377,9 @@ TEST_F(EndToEndTest, GetStats) {
 
         send_stats_filled_[CompoundKey("OutgoingRate", it->first)] |=
             stats.encode_frame_rate != 0;
+
+        send_stats_filled_[CompoundKey("Delay", it->first)] |=
+            stream_stats.avg_delay_ms != 0 || stream_stats.max_delay_ms != 0;
       }
 
       return AllStatsFilled(send_stats_filled_);

webrtc/video/send_statistics_proxy.cc

@@ -17,10 +17,8 @@
 namespace webrtc {
 
 SendStatisticsProxy::SendStatisticsProxy(
-    const VideoSendStream::Config& config,
+    const VideoSendStream::Config& config)
-    SendStatisticsProxy::StatsProvider* stats_provider)
     : config_(config),
-      stats_provider_(stats_provider),
       crit_(CriticalSectionWrapper::CreateCriticalSection()) {
 }
 
@@ -45,13 +43,8 @@ void SendStatisticsProxy::CapturedFrameRate(const int capture_id,
 }
 
 VideoSendStream::Stats SendStatisticsProxy::GetStats() const {
-  VideoSendStream::Stats stats;
-  {
   CriticalSectionScoped lock(crit_.get());
-    stats = stats_;
+  return stats_;
-  }
-  stats_provider_->GetSendSideDelay(&stats);
-  return stats;
 }
 
 StreamStats* SendStatisticsProxy::GetStatsEntry(uint32_t ssrc) {
@@ -119,4 +112,15 @@ void SendStatisticsProxy::FrameCountUpdated(FrameType frame_type,
   }
 }
 
+void SendStatisticsProxy::SendSideDelayUpdated(int avg_delay_ms,
+                                               int max_delay_ms,
+                                               uint32_t ssrc) {
+  CriticalSectionScoped lock(crit_.get());
+  StreamStats* stats = GetStatsEntry(ssrc);
+  if (stats == NULL)
+    return;
+  stats->avg_delay_ms = avg_delay_ms;
+  stats->max_delay_ms = max_delay_ms;
+}
+
 }  // namespace webrtc

webrtc/video/send_statistics_proxy.h

@@ -29,19 +29,10 @@ class SendStatisticsProxy : public RtcpStatisticsCallback,
                             public BitrateStatisticsObserver,
                             public FrameCountObserver,
                             public ViEEncoderObserver,
-                            public ViECaptureObserver {
+                            public ViECaptureObserver,
+                            public SendSideDelayObserver {
  public:
-  class StatsProvider {
+  explicit SendStatisticsProxy(const VideoSendStream::Config& config);
-   protected:
-    StatsProvider() {}
-    virtual ~StatsProvider() {}
-
-   public:
-    virtual bool GetSendSideDelay(VideoSendStream::Stats* stats) = 0;
-  };
-
-  SendStatisticsProxy(const VideoSendStream::Config& config,
-                      StatsProvider* stats_provider);
   virtual ~SendStatisticsProxy();
 
   VideoSendStream::Stats GetStats() const;
@@ -79,11 +70,14 @@ class SendStatisticsProxy : public RtcpStatisticsCallback,
   virtual void NoPictureAlarm(const int capture_id,
                               const CaptureAlarm alarm) OVERRIDE {}
 
+  virtual void SendSideDelayUpdated(int avg_delay_ms,
+                                    int max_delay_ms,
+                                    uint32_t ssrc) OVERRIDE;
+
  private:
   StreamStats* GetStatsEntry(uint32_t ssrc) EXCLUSIVE_LOCKS_REQUIRED(crit_);
 
   const VideoSendStream::Config config_;
-  StatsProvider* const stats_provider_;
   scoped_ptr<CriticalSectionWrapper> crit_;
   VideoSendStream::Stats stats_ GUARDED_BY(crit_);
 };

webrtc/video/send_statistics_proxy_unittest.cc

@@ -19,8 +19,7 @@
 
 namespace webrtc {
 
-class SendStatisticsProxyTest : public ::testing::Test,
+class SendStatisticsProxyTest : public ::testing::Test {
-                                protected SendStatisticsProxy::StatsProvider {
  public:
   SendStatisticsProxyTest() : avg_delay_ms_(0), max_delay_ms_(0) {}
   virtual ~SendStatisticsProxyTest() {}
@@ -28,7 +27,7 @@ class SendStatisticsProxyTest : public ::testing::Test,
  protected:
   virtual void SetUp() {
     statistics_proxy_.reset(
-        new SendStatisticsProxy(GetTestConfig(), this));
+        new SendStatisticsProxy(GetTestConfig()));
     config_ = GetTestConfig();
     expected_ = VideoSendStream::Stats();
   }
@@ -40,18 +39,9 @@ class SendStatisticsProxyTest : public ::testing::Test,
     return config;
   }
 
-  virtual bool GetSendSideDelay(VideoSendStream::Stats* stats) OVERRIDE {
-    stats->avg_delay_ms = avg_delay_ms_;
-    stats->max_delay_ms = max_delay_ms_;
-    return true;
-  }
-
   void ExpectEqual(VideoSendStream::Stats one, VideoSendStream::Stats other) {
-    EXPECT_EQ(one.avg_delay_ms, other.avg_delay_ms);
     EXPECT_EQ(one.input_frame_rate, other.input_frame_rate);
     EXPECT_EQ(one.encode_frame_rate, other.encode_frame_rate);
-    EXPECT_EQ(one.avg_delay_ms, other.avg_delay_ms);
-    EXPECT_EQ(one.max_delay_ms, other.max_delay_ms);
     EXPECT_EQ(one.suspended, other.suspended);
 
     EXPECT_EQ(one.substreams.size(), other.substreams.size());
@@ -68,6 +58,8 @@ class SendStatisticsProxyTest : public ::testing::Test,
       EXPECT_EQ(a.key_frames, b.key_frames);
       EXPECT_EQ(a.delta_frames, b.delta_frames);
       EXPECT_EQ(a.bitrate_bps, b.bitrate_bps);
+      EXPECT_EQ(a.avg_delay_ms, b.avg_delay_ms);
+      EXPECT_EQ(a.max_delay_ms, b.max_delay_ms);
 
       EXPECT_EQ(a.rtp_stats.bytes, b.rtp_stats.bytes);
       EXPECT_EQ(a.rtp_stats.header_bytes, b.rtp_stats.header_bytes);
@@ -190,6 +182,7 @@ TEST_F(SendStatisticsProxyTest, Bitrate) {
        ++it) {
     const uint32_t ssrc = *it;
     BitrateStatistics bitrate;
+    // Use ssrc as bitrate_bps to get a unique value for each stream.
     bitrate.bitrate_bps = ssrc;
     observer->Notify(bitrate, ssrc);
     expected_.substreams[ssrc].bitrate_bps = ssrc;
@@ -199,14 +192,23 @@ TEST_F(SendStatisticsProxyTest, Bitrate) {
   ExpectEqual(expected_, stats);
 }
 
-TEST_F(SendStatisticsProxyTest, StreamStats) {
+TEST_F(SendStatisticsProxyTest, SendSideDelay) {
-  avg_delay_ms_ = 1;
+  SendSideDelayObserver* observer = statistics_proxy_.get();
-  max_delay_ms_ = 2;
+  for (std::vector<uint32_t>::const_iterator it = config_.rtp.ssrcs.begin();
+       it != config_.rtp.ssrcs.end();
+       ++it) {
+    const uint32_t ssrc = *it;
+    // Use ssrc as avg_delay_ms and max_delay_ms to get a unique value for each
+    // stream.
+    int avg_delay_ms = ssrc;
+    int max_delay_ms = ssrc + 1;
+    observer->SendSideDelayUpdated(avg_delay_ms, max_delay_ms, ssrc);
+    expected_.substreams[ssrc].avg_delay_ms = avg_delay_ms;
+    expected_.substreams[ssrc].max_delay_ms = max_delay_ms;
+  }
 
   VideoSendStream::Stats stats = statistics_proxy_->GetStats();
-
+  ExpectEqual(expected_, stats);
-  EXPECT_EQ(avg_delay_ms_, stats.avg_delay_ms);
-  EXPECT_EQ(max_delay_ms_, stats.max_delay_ms);
 }
 
 TEST_F(SendStatisticsProxyTest, NoSubstreams) {

webrtc/video/video_send_stream.cc

@@ -125,7 +125,7 @@ VideoSendStream::VideoSendStream(
       suspended_ssrcs_(suspended_ssrcs),
       external_codec_(NULL),
       channel_(-1),
-      stats_proxy_(new SendStatisticsProxy(config, this)) {
+      stats_proxy_(config) {
   video_engine_base_ = ViEBase::GetInterface(video_engine);
   video_engine_base_->CreateChannel(channel_, base_channel);
   assert(channel_ != -1);
@@ -216,6 +216,8 @@ VideoSendStream::VideoSendStream(
   if (overuse_observer)
     video_engine_base_->RegisterCpuOveruseObserver(channel_, overuse_observer);
 
+  video_engine_base_->RegisterSendSideDelayObserver(channel_, &stats_proxy_);
+
   image_process_ = ViEImageProcess::GetInterface(video_engine);
   image_process_->RegisterPreEncodeCallback(channel_,
                                             config_.pre_encode_callback);
@@ -228,26 +230,26 @@ VideoSendStream::VideoSendStream(
     codec_->SuspendBelowMinBitrate(channel_);
 
   rtp_rtcp_->RegisterSendChannelRtcpStatisticsCallback(channel_,
-                                                       stats_proxy_.get());
+                                                       &stats_proxy_);
   rtp_rtcp_->RegisterSendChannelRtpStatisticsCallback(channel_,
-                                                      stats_proxy_.get());
+                                                      &stats_proxy_);
-  rtp_rtcp_->RegisterSendBitrateObserver(channel_, stats_proxy_.get());
+  rtp_rtcp_->RegisterSendBitrateObserver(channel_, &stats_proxy_);
-  rtp_rtcp_->RegisterSendFrameCountObserver(channel_, stats_proxy_.get());
+  rtp_rtcp_->RegisterSendFrameCountObserver(channel_, &stats_proxy_);
 
-  codec_->RegisterEncoderObserver(channel_, *stats_proxy_);
+  codec_->RegisterEncoderObserver(channel_, stats_proxy_);
-  capture_->RegisterObserver(capture_id_, *stats_proxy_);
+  capture_->RegisterObserver(capture_id_, stats_proxy_);
 }
 
 VideoSendStream::~VideoSendStream() {
   capture_->DeregisterObserver(capture_id_);
   codec_->DeregisterEncoderObserver(channel_);
 
-  rtp_rtcp_->DeregisterSendFrameCountObserver(channel_, stats_proxy_.get());
+  rtp_rtcp_->DeregisterSendFrameCountObserver(channel_, &stats_proxy_);
-  rtp_rtcp_->DeregisterSendBitrateObserver(channel_, stats_proxy_.get());
+  rtp_rtcp_->DeregisterSendBitrateObserver(channel_, &stats_proxy_);
   rtp_rtcp_->DeregisterSendChannelRtpStatisticsCallback(channel_,
-                                                        stats_proxy_.get());
+                                                        &stats_proxy_);
   rtp_rtcp_->DeregisterSendChannelRtcpStatisticsCallback(channel_,
-                                                         stats_proxy_.get());
+                                                         &stats_proxy_);
 
   image_process_->DeRegisterPreEncodeCallback(channel_);
 
@@ -395,12 +397,7 @@ bool VideoSendStream::DeliverRtcp(const uint8_t* packet, size_t length) {
 }
 
 VideoSendStream::Stats VideoSendStream::GetStats() const {
-  return stats_proxy_->GetStats();
+  return stats_proxy_.GetStats();
-}
-
-bool VideoSendStream::GetSendSideDelay(VideoSendStream::Stats* stats) {
-  return codec_->GetSendSideDelay(
-      channel_, &stats->avg_delay_ms, &stats->max_delay_ms);
 }
 
 void VideoSendStream::ConfigureSsrcs() {

webrtc/video/video_send_stream.h

@@ -38,8 +38,7 @@ class ViERTP_RTCP;
 namespace internal {
 
 class VideoSendStream : public webrtc::VideoSendStream,
-                        public VideoSendStreamInput,
+                        public VideoSendStreamInput {
-                        public SendStatisticsProxy::StatsProvider {
  public:
   VideoSendStream(newapi::Transport* transport,
                   CpuOveruseObserver* overuse_observer,
@@ -72,10 +71,6 @@ class VideoSendStream : public webrtc::VideoSendStream,
   typedef std::map<uint32_t, RtpState> RtpStateMap;
   RtpStateMap GetRtpStates() const;
 
- protected:
-  // From SendStatisticsProxy::StreamStatsProvider.
-  virtual bool GetSendSideDelay(VideoSendStream::Stats* stats) OVERRIDE;
-
  private:
   void ConfigureSsrcs();
   TransportAdapter transport_adapter_;
@@ -96,7 +91,7 @@ class VideoSendStream : public webrtc::VideoSendStream,
   int channel_;
   int capture_id_;
 
-  const scoped_ptr<SendStatisticsProxy> stats_proxy_;
+  SendStatisticsProxy stats_proxy_;
 };
 }  // namespace internal
 }  // namespace webrtc

webrtc/video/video_send_stream_tests.cc

@@ -27,6 +27,7 @@
 #include "webrtc/system_wrappers/interface/scoped_vector.h"
 #include "webrtc/system_wrappers/interface/sleep.h"
 #include "webrtc/system_wrappers/interface/thread_wrapper.h"
+#include "webrtc/system_wrappers/interface/logging.h"
 #include "webrtc/test/call_test.h"
 #include "webrtc/test/configurable_frame_size_encoder.h"
 #include "webrtc/test/null_transport.h"
@@ -956,8 +957,8 @@ TEST_F(VideoSendStreamTest, ProducesStats) {
     bool CheckStats() {
       VideoSendStream::Stats stats = stream_->GetStats();
       // Check that all applicable data sources have been used.
-      if (stats.input_frame_rate > 0 && stats.encode_frame_rate > 0 &&
+      if (stats.input_frame_rate > 0 && stats.encode_frame_rate > 0
-          stats.avg_delay_ms > 0 && !stats.substreams.empty()) {
+          && !stats.substreams.empty()) {
         uint32_t ssrc = stats.substreams.begin()->first;
         EXPECT_NE(
             config_.rtp.ssrcs.end(),
@@ -967,7 +968,8 @@ TEST_F(VideoSendStreamTest, ProducesStats) {
         // data is received from remote side. Tested in call tests instead.
         const StreamStats& entry = stats.substreams[ssrc];
         if (entry.key_frames > 0u && entry.bitrate_bps > 0 &&
-            entry.rtp_stats.packets > 0u) {
+            entry.rtp_stats.packets > 0u && entry.avg_delay_ms > 0 &&
+            entry.max_delay_ms > 0) {
           return true;
         }
       }

webrtc/video_engine/include/vie_base.h

@@ -204,6 +204,13 @@ class WEBRTC_DLLEXPORT ViEBase {
   // Gets cpu overuse measures.
   virtual int GetCpuOveruseMetrics(int channel, CpuOveruseMetrics* metrics) = 0;
 
+  // Registers a callback which is called when send-side delay statistics has
+  // been updated.
+  // TODO(holmer): Remove the default implementation when fakevideoengine.h has
+  // been updated.
+  virtual void RegisterSendSideDelayObserver(
+      int channel, SendSideDelayObserver* observer) {}
+
   // Specifies the VoiceEngine and VideoEngine channel pair to use for
   // audio/video synchronization.
   virtual int ConnectAudioChannel(const int video_channel,

webrtc/video_engine/vie_base_impl.cc

@@ -144,6 +144,14 @@ int ViEBaseImpl::GetCpuOveruseMetrics(int video_channel,
   return -1;
 }
 
+void ViEBaseImpl::RegisterSendSideDelayObserver(
+    int channel, SendSideDelayObserver* observer) {
+  ViEChannelManagerScoped cs(*(shared_data_.channel_manager()));
+  ViEChannel* vie_channel = cs.Channel(channel);
+  assert(vie_channel);
+  vie_channel->RegisterSendSideDelayObserver(observer);
+}
+
 int ViEBaseImpl::CreateChannel(int& video_channel) {  // NOLINT
   return CreateChannel(video_channel, static_cast<const Config*>(NULL));
 }

webrtc/video_engine/vie_base_impl.h

@@ -37,6 +37,8 @@ class ViEBaseImpl
                                    const CpuOveruseOptions& options);
   virtual int GetCpuOveruseMetrics(int channel,
                                    CpuOveruseMetrics* metrics);
+  virtual void RegisterSendSideDelayObserver(int channel,
+      SendSideDelayObserver* observer) OVERRIDE;
   virtual int CreateChannel(int& video_channel);  // NOLINT
   virtual int CreateChannel(int& video_channel,  // NOLINT
                             const Config* config);

webrtc/video_engine/vie_channel.cc

@@ -118,6 +118,7 @@ ViEChannel::ViEChannel(int32_t channel_id,
   configuration.receive_statistics = vie_receiver_.GetReceiveStatistics();
   configuration.send_bitrate_observer = &send_bitrate_observer_;
   configuration.send_frame_count_observer = &send_frame_count_observer_;
+  configuration.send_side_delay_observer = &send_side_delay_observer_;
 
   rtp_rtcp_.reset(RtpRtcp::CreateRtpRtcp(configuration));
   vie_receiver_.SetRtpRtcpModule(rtp_rtcp_.get());
@@ -1200,6 +1201,11 @@ bool ViEChannel::GetSendSideDelay(int* avg_send_delay,
   return valid_estimate;
 }
 
+void ViEChannel::RegisterSendSideDelayObserver(
+    SendSideDelayObserver* observer) {
+  send_side_delay_observer_.Set(observer);
+}
+
 void ViEChannel::RegisterSendBitrateObserver(
     BitrateStatisticsObserver* observer) {
   send_bitrate_observer_.Set(observer);
@@ -1558,6 +1564,7 @@ RtpRtcp* ViEChannel::CreateRtpRtcpModule() {
   configuration.bandwidth_callback = bandwidth_observer_.get();
   configuration.rtt_stats = rtt_stats_;
   configuration.paced_sender = paced_sender_;
+  configuration.send_side_delay_observer = &send_side_delay_observer_;
 
   return RtpRtcp::CreateRtpRtcp(configuration);
 }

webrtc/video_engine/vie_channel.h

@@ -212,7 +212,10 @@ class ViEChannel
                          uint32_t* video_bitrate_sent,
                          uint32_t* fec_bitrate_sent,
                          uint32_t* nackBitrateSent) const;
+  // TODO(holmer): Deprecated. We should use the SendSideDelayObserver instead
+  // to avoid deadlocks.
   bool GetSendSideDelay(int* avg_send_delay, int* max_send_delay) const;
+  void RegisterSendSideDelayObserver(SendSideDelayObserver* observer);
   void GetReceiveBandwidthEstimatorStats(
       ReceiveBandwidthEstimatorStats* output) const;
 
@@ -429,6 +432,17 @@ class ViEChannel
     }
   } send_frame_count_observer_;
 
+  class RegisterableSendSideDelayObserver :
+      public RegisterableCallback<SendSideDelayObserver> {
+    virtual void SendSideDelayUpdated(int avg_delay_ms,
+                                      int max_delay_ms,
+                                      uint32_t ssrc) OVERRIDE {
+      CriticalSectionScoped cs(critsect_.get());
+      if (callback_)
+        callback_->SendSideDelayUpdated(avg_delay_ms, max_delay_ms, ssrc);
+    }
+  } send_side_delay_observer_;
+
   int32_t channel_id_;
   int32_t engine_id_;
   uint32_t number_of_cores_;

webrtc/video_send_stream.h

@@ -41,14 +41,9 @@ class VideoSendStream {
     Stats()
         : input_frame_rate(0),
           encode_frame_rate(0),
-          avg_delay_ms(0),
-          max_delay_ms(0),
           suspended(false) {}
-
     int input_frame_rate;
     int encode_frame_rate;
-    int avg_delay_ms;
-    int max_delay_ms;
     bool suspended;
     std::map<uint32_t, StreamStats> substreams;
   };