Revert of Make the new jitter buffer the default jitter buffer. (patc…

…hset #2 id:230001 of https://codereview.webrtc.org/2642753002/ ) Reason for revert: Breaks tests downstream. Original issue's description: > Reland of Make the new jitter buffer the default jitter buffer. (patchset #1 id:1 of https://codereview.chromium.org/2632123005/ ) > > Reason for revert: > Fix in this CL: https://codereview.chromium.org/2640793003/ > > Original issue's description: > > Revert of Make the new jitter buffer the default jitter buffer. (patchset #7 id:120001 of https://codereview.chromium.org/2627463004/ ) > > > > Reason for revert: > > Breaks android bots. > > > > Original issue's description: > > > Make the new jitter buffer the default jitter buffer. > > > > > > This CL contains only the changes necessary to make the switch to the new jitter > > > buffer, clean up will be done in follow up CLs. > > > > > > In this CL: > > > - Removed the WebRTC-NewVideoJitterBuffer experiment and made the > > > new video jitter buffer the default one. > > > - Moved WebRTC.Video.KeyFramesReceivedInPermille and > > > WebRTC.Video.JitterBufferDelayInMs to the ReceiveStatisticsProxy. > > > > > > BUG=webrtc:5514 > > > > > > Review-Url: https://codereview.webrtc.org/2627463004 > > > Cr-Commit-Position: refs/heads/master@{#16114} > > > Committed: https://chromium.googlesource.com/external/webrtc/+/0f0763d86d5d4e7f27e8dece02560e39c6da97d6 > > > > [email protected],[email protected] > > # Skipping CQ checks because original CL landed less than 1 days ago. > > NOPRESUBMIT=true > > NOTREECHECKS=true > > NOTRY=true > > BUG=webrtc:5514 > > > > Review-Url: https://codereview.webrtc.org/2632123005 > > Cr-Commit-Position: refs/heads/master@{#16117} > > Committed: https://chromium.googlesource.com/external/webrtc/+/c08c191f7d206dc0de945185370d18f29d556931 > > [email protected],[email protected] > # Not skipping CQ checks because original CL landed more than 1 days ago. > BUG=webrtc:5514 > > Review-Url: https://codereview.webrtc.org/2642753002 > Cr-Commit-Position: refs/heads/master@{#16149} > Committed: https://chromium.googlesource.com/external/webrtc/+/f20dd0014d1cfc8a2e859a9e177e7fe2b21274ca [email protected],[email protected],[email protected] # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=webrtc:5514 Review-Url: https://codereview.webrtc.org/2638423003 Cr-Commit-Position: refs/heads/master@{#16159}
webrtc-uwp · Jan 19, 2017 · 04926b8 · 04926b8
1 parent f847178
commit 04926b8
Show file tree

Hide file tree

Showing 16 changed files with 239 additions and 311 deletions.
diff --git a/webrtc/modules/video_coding/frame_buffer2.cc b/webrtc/modules/video_coding/frame_buffer2.cc
@@ -16,7 +16,6 @@
 
 #include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
-#include "webrtc/modules/video_coding/include/video_coding_defines.h"
 #include "webrtc/modules/video_coding/jitter_estimator.h"
 #include "webrtc/modules/video_coding/timing.h"
 #include "webrtc/system_wrappers/include/clock.h"
@@ -35,8 +34,7 @@ constexpr int kMaxFramesHistory = 50;
 
 FrameBuffer::FrameBuffer(Clock* clock,
                          VCMJitterEstimator* jitter_estimator,
-                         VCMTiming* timing,
-                         VCMReceiveStatisticsCallback* stats_callback)
+                         VCMTiming* timing)
     : clock_(clock),
       new_countinuous_frame_event_(false, false),
       jitter_estimator_(jitter_estimator),
@@ -47,10 +45,11 @@ FrameBuffer::FrameBuffer(Clock* clock,
       num_frames_history_(0),
       num_frames_buffered_(0),
       stopped_(false),
-      protection_mode_(kProtectionNack),
-      stats_callback_(stats_callback) {}
+      protection_mode_(kProtectionNack) {}
 
-FrameBuffer::~FrameBuffer() {}
+FrameBuffer::~FrameBuffer() {
+  UpdateHistograms();
+}
 
 FrameBuffer::ReturnReason FrameBuffer::NextFrame(
     int64_t max_wait_time_ms,
@@ -164,8 +163,9 @@ int FrameBuffer::InsertFrame(std::unique_ptr<FrameObject> frame) {
   rtc::CritScope lock(&crit_);
   RTC_DCHECK(frame);
 
-  if (stats_callback_)
-    stats_callback_->OnCompleteFrame(frame->num_references == 0, frame->size());
+  ++num_total_frames_;
+  if (frame->num_references == 0)
+    ++num_key_frames_;
 
   FrameKey key(frame->picture_id, frame->spatial_layer);
   int last_continuous_picture_id =
@@ -379,22 +379,28 @@ bool FrameBuffer::UpdateFrameInfoWithIncomingFrame(const FrameObject& frame,
 }
 
 void FrameBuffer::UpdateJitterDelay() {
-  if (!stats_callback_)
-    return;
-
-  int decode_ms;
-  int max_decode_ms;
-  int current_delay_ms;
-  int target_delay_ms;
-  int jitter_buffer_ms;
-  int min_playout_delay_ms;
-  int render_delay_ms;
-  if (timing_->GetTimings(&decode_ms, &max_decode_ms, &current_delay_ms,
-                          &target_delay_ms, &jitter_buffer_ms,
-                          &min_playout_delay_ms, &render_delay_ms)) {
-    stats_callback_->OnFrameBufferTimingsUpdated(
-        decode_ms, max_decode_ms, current_delay_ms, target_delay_ms,
-        jitter_buffer_ms, min_playout_delay_ms, render_delay_ms);
+  int unused;
+  int delay;
+  timing_->GetTimings(&unused, &unused, &unused, &unused, &delay, &unused,
+                      &unused);
+
+  accumulated_delay_ += delay;
+  ++accumulated_delay_samples_;
+}
+
+void FrameBuffer::UpdateHistograms() const {
+  rtc::CritScope lock(&crit_);
+  if (num_total_frames_ > 0) {
+    int key_frames_permille = (static_cast<float>(num_key_frames_) * 1000.0f /
+                                   static_cast<float>(num_total_frames_) +
+                               0.5f);
+    RTC_HISTOGRAM_COUNTS_1000("WebRTC.Video.KeyFramesReceivedInPermille",
+                              key_frames_permille);
+  }
+
+  if (accumulated_delay_samples_ > 0) {
+    RTC_HISTOGRAM_COUNTS_10000("WebRTC.Video.JitterBufferDelayInMs",
+                               accumulated_delay_ / accumulated_delay_samples_);
   }
 }
 

diff --git a/webrtc/modules/video_coding/frame_buffer2.h b/webrtc/modules/video_coding/frame_buffer2.h
@@ -28,7 +28,6 @@
 namespace webrtc {
 
 class Clock;
-class VCMReceiveStatisticsCallback;
 class VCMJitterEstimator;
 class VCMTiming;
 
@@ -40,8 +39,7 @@ class FrameBuffer {
 
   FrameBuffer(Clock* clock,
               VCMJitterEstimator* jitter_estimator,
-              VCMTiming* timing,
-              VCMReceiveStatisticsCallback* stats_proxy);
+              VCMTiming* timing);
 
   virtual ~FrameBuffer();
 
@@ -143,6 +141,8 @@ class FrameBuffer {
 
   void UpdateJitterDelay() EXCLUSIVE_LOCKS_REQUIRED(crit_);
 
+  void UpdateHistograms() const;
+
   void ClearFramesAndHistory() EXCLUSIVE_LOCKS_REQUIRED(crit_);
 
   FrameMap frames_ GUARDED_BY(crit_);
@@ -160,9 +160,16 @@ class FrameBuffer {
   int num_frames_buffered_ GUARDED_BY(crit_);
   bool stopped_ GUARDED_BY(crit_);
   VCMVideoProtection protection_mode_ GUARDED_BY(crit_);
-  VCMReceiveStatisticsCallback* const stats_callback_;
 
   RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(FrameBuffer);
+
+  // For WebRTC.Video.JitterBufferDelayInMs metric.
+  int64_t accumulated_delay_ = 0;
+  int64_t accumulated_delay_samples_ = 0;
+
+  // For WebRTC.Video.KeyFramesReceivedInPermille metric.
+  int64_t num_total_frames_ = 0;
+  int64_t num_key_frames_ = 0;
 };
 
 }  // namespace video_coding

diff --git a/webrtc/modules/video_coding/frame_buffer2_unittest.cc b/webrtc/modules/video_coding/frame_buffer2_unittest.cc
@@ -25,9 +25,6 @@
 #include "webrtc/test/gmock.h"
 #include "webrtc/test/gtest.h"
 
-using testing::_;
-using testing::Return;
-
 namespace webrtc {
 namespace video_coding {
 
@@ -57,16 +54,6 @@ class VCMTimingFake : public VCMTiming {
     return std::max<int>(0, render_time_ms - now_ms - kDecodeTime);
   }
 
-  bool GetTimings(int* decode_ms,
-                  int* max_decode_ms,
-                  int* current_delay_ms,
-                  int* target_delay_ms,
-                  int* jitter_buffer_ms,
-                  int* min_playout_delay_ms,
-                  int* render_delay_ms) const override {
-    return true;
-  }
-
  private:
   static constexpr int kDelayMs = 50;
   static constexpr int kDecodeTime = kDelayMs / 2;
@@ -95,27 +82,6 @@ class FrameObjectFake : public FrameObject {
   int64_t ReceivedTime() const override { return 0; }
 
   int64_t RenderTime() const override { return _renderTimeMs; }
-
-  // In EncodedImage |_length| is used to descibe its size and |_size| to
-  // describe its capacity.
-  void SetSize(int size) { _length = size; }
-};
-
-class VCMReceiveStatisticsCallbackMock : public VCMReceiveStatisticsCallback {
- public:
-  MOCK_METHOD2(OnReceiveRatesUpdated,
-               void(uint32_t bitRate, uint32_t frameRate));
-  MOCK_METHOD2(OnCompleteFrame, void(bool is_keyframe, size_t size_bytes));
-  MOCK_METHOD1(OnDiscardedPacketsUpdated, void(int discarded_packets));
-  MOCK_METHOD1(OnFrameCountsUpdated, void(const FrameCounts& frame_counts));
-  MOCK_METHOD7(OnFrameBufferTimingsUpdated,
-               void(int decode_ms,
-                    int max_decode_ms,
-                    int current_delay_ms,
-                    int target_delay_ms,
-                    int jitter_buffer_ms,
-                    int min_playout_delay_ms,
-                    int render_delay_ms));
 };
 
 class TestFrameBuffer2 : public ::testing::Test {
@@ -129,7 +95,7 @@ class TestFrameBuffer2 : public ::testing::Test {
       : clock_(0),
         timing_(&clock_),
         jitter_estimator_(&clock_),
-        buffer_(&clock_, &jitter_estimator_, &timing_, &stats_callback_),
+        buffer_(&clock_, &jitter_estimator_, &timing_),
         rand_(0x34678213),
         tear_down_(false),
         extract_thread_(&ExtractLoop, this, "Extract Thread"),
@@ -224,7 +190,6 @@ class TestFrameBuffer2 : public ::testing::Test {
   FrameBuffer buffer_;
   std::vector<std::unique_ptr<FrameObject>> frames_;
   Random rand_;
-  ::testing::NiceMock<VCMReceiveStatisticsCallbackMock> stats_callback_;
 
   int64_t max_wait_time_;
   bool tear_down_;
@@ -471,30 +436,5 @@ TEST_F(TestFrameBuffer2, PictureIdJumpBack) {
   CheckNoFrame(2);
 }
 
-TEST_F(TestFrameBuffer2, StatsCallback) {
-  uint16_t pid = Rand();
-  uint32_t ts = Rand();
-  const int kFrameSize = 5000;
-
-  EXPECT_CALL(stats_callback_, OnCompleteFrame(true, kFrameSize));
-  EXPECT_CALL(stats_callback_,
-              OnFrameBufferTimingsUpdated(_, _, _, _, _, _, _));
-
-  {
-    std::unique_ptr<FrameObjectFake> frame(new FrameObjectFake());
-    frame->SetSize(kFrameSize);
-    frame->picture_id = pid;
-    frame->spatial_layer = 0;
-    frame->timestamp = ts;
-    frame->num_references = 0;
-    frame->inter_layer_predicted = false;
-
-    EXPECT_EQ(buffer_.InsertFrame(std::move(frame)), pid);
-  }
-
-  ExtractFrame();
-  CheckFrame(0, pid, 0);
-}
-
 }  // namespace video_coding
 }  // namespace webrtc
diff --git a/webrtc/modules/video_coding/include/video_coding_defines.h b/webrtc/modules/video_coding/include/video_coding_defines.h
@@ -90,16 +90,8 @@ class VCMSendStatisticsCallback {
 class VCMReceiveStatisticsCallback {
  public:
   virtual void OnReceiveRatesUpdated(uint32_t bitRate, uint32_t frameRate) = 0;
-  virtual void OnCompleteFrame(bool is_keyframe, size_t size_bytes) = 0;
   virtual void OnDiscardedPacketsUpdated(int discarded_packets) = 0;
   virtual void OnFrameCountsUpdated(const FrameCounts& frame_counts) = 0;
-  virtual void OnFrameBufferTimingsUpdated(int decode_ms,
-                                           int max_decode_ms,
-                                           int current_delay_ms,
-                                           int target_delay_ms,
-                                           int jitter_buffer_ms,
-                                           int min_playout_delay_ms,
-                                           int render_delay_ms) = 0;
 
  protected:
   virtual ~VCMReceiveStatisticsCallback() {}

diff --git a/webrtc/modules/video_coding/timing.h b/webrtc/modules/video_coding/timing.h
@@ -94,13 +94,13 @@ class VCMTiming {
 
   // Return current timing information. Returns true if the first frame has been
   // decoded, false otherwise.
-  virtual bool GetTimings(int* decode_ms,
-                          int* max_decode_ms,
-                          int* current_delay_ms,
-                          int* target_delay_ms,
-                          int* jitter_buffer_ms,
-                          int* min_playout_delay_ms,
-                          int* render_delay_ms) const;
+  bool GetTimings(int* decode_ms,
+                  int* max_decode_ms,
+                  int* current_delay_ms,
+                  int* target_delay_ms,
+                  int* jitter_buffer_ms,
+                  int* min_playout_delay_ms,
+                  int* render_delay_ms) const;
 
   enum { kDefaultRenderDelayMs = 10 };
   enum { kDelayMaxChangeMsPerS = 100 };

diff --git a/webrtc/modules/video_coding/video_receiver.cc b/webrtc/modules/video_coding/video_receiver.cc
@@ -56,14 +56,31 @@ VideoReceiver::~VideoReceiver() {}
 
 void VideoReceiver::Process() {
   // Receive-side statistics
-
-  // TODO(philipel): Remove this if block when we know what to do with
-  //                 ReceiveStatisticsProxy::QualitySample.
   if (_receiveStatsTimer.TimeUntilProcess() == 0) {
     _receiveStatsTimer.Processed();
     rtc::CritScope cs(&process_crit_);
     if (_receiveStatsCallback != nullptr) {
-      _receiveStatsCallback->OnReceiveRatesUpdated(0, 0);
+      uint32_t bitRate;
+      uint32_t frameRate;
+      _receiver.ReceiveStatistics(&bitRate, &frameRate);
+      _receiveStatsCallback->OnReceiveRatesUpdated(bitRate, frameRate);
+    }
+
+    if (_decoderTimingCallback != nullptr) {
+      int decode_ms;
+      int max_decode_ms;
+      int current_delay_ms;
+      int target_delay_ms;
+      int jitter_buffer_ms;
+      int min_playout_delay_ms;
+      int render_delay_ms;
+      if (_timing->GetTimings(&decode_ms, &max_decode_ms, &current_delay_ms,
+                              &target_delay_ms, &jitter_buffer_ms,
+                              &min_playout_delay_ms, &render_delay_ms)) {
+        _decoderTimingCallback->OnDecoderTiming(
+            decode_ms, max_decode_ms, current_delay_ms, target_delay_ms,
+            jitter_buffer_ms, min_playout_delay_ms, render_delay_ms);
+      }
     }
   }
 
@@ -275,7 +292,7 @@ int32_t VideoReceiver::Decode(uint16_t maxWaitTimeMs) {
   return ret;
 }
 
-// Used for the new jitter buffer.
+// Used for the WebRTC-NewVideoJitterBuffer experiment.
 // TODO(philipel): Clean up among the Decode functions as we replace
 //                 VCMEncodedFrame with FrameObject.
 int32_t VideoReceiver::Decode(const webrtc::VCMEncodedFrame* frame) {

diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc
@@ -1228,6 +1228,9 @@ TEST_P(EndToEndTest, ReceivesPliAndRecoversWithNack) {
 }
 
 TEST_P(EndToEndTest, ReceivesPliAndRecoversWithoutNack) {
+  // This test makes no sense for the new video jitter buffer.
+  if (GetParam() == new_jb_enabled)
+    return;
   ReceivesPliAndRecovers(0);
 }
 
@@ -3027,6 +3030,10 @@ TEST_P(EndToEndTest, GetStats) {
     ReceiveStreamRenderer receive_stream_renderer_;
   } test;
 
+  // TODO(philipel): Implement statistics for the new video jitter buffer.
+  if (GetParam() == new_jb_enabled)
+    return;
+
   RunBaseTest(&test);
 }