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

…hset #1 id:1 of https://codereview.webrtc.org/2638423003/ ) Reason for revert: Bugfixes related to the new jitter buffer has landed. Original issue's description: > Revert of Make the new jitter buffer the default jitter buffer. (patchset #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} > Committed: https://chromium.googlesource.com/external/webrtc/+/04926b82641c426d764aa6e013e133db519129db [email protected],[email protected],[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/2652043005 Cr-Commit-Position: refs/heads/master@{#16293}
webrtc-uwp · Jan 26, 2017 · 09d6ef0 · 09d6ef0
1 parent ac9c211
commit 09d6ef0
Show file tree

Hide file tree

Showing 17 changed files with 313 additions and 246 deletions.
diff --git a/webrtc/modules/video_coding/frame_buffer2.cc b/webrtc/modules/video_coding/frame_buffer2.cc
@@ -16,6 +16,7 @@
 
 #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"
@@ -34,7 +35,8 @@ constexpr int kMaxFramesHistory = 50;
 
 FrameBuffer::FrameBuffer(Clock* clock,
                          VCMJitterEstimator* jitter_estimator,
-                         VCMTiming* timing)
+                         VCMTiming* timing,
+                         VCMReceiveStatisticsCallback* stats_callback)
     : clock_(clock),
       new_countinuous_frame_event_(false, false),
       jitter_estimator_(jitter_estimator),
@@ -45,11 +47,10 @@ FrameBuffer::FrameBuffer(Clock* clock,
       num_frames_history_(0),
       num_frames_buffered_(0),
       stopped_(false),
-      protection_mode_(kProtectionNack) {}
+      protection_mode_(kProtectionNack),
+      stats_callback_(stats_callback) {}
 
-FrameBuffer::~FrameBuffer() {
-  UpdateHistograms();
-}
+FrameBuffer::~FrameBuffer() {}
 
 FrameBuffer::ReturnReason FrameBuffer::NextFrame(
     int64_t max_wait_time_ms,
@@ -165,9 +166,8 @@ int FrameBuffer::InsertFrame(std::unique_ptr<FrameObject> frame) {
   rtc::CritScope lock(&crit_);
   RTC_DCHECK(frame);
 
-  ++num_total_frames_;
-  if (frame->num_references == 0)
-    ++num_key_frames_;
+  if (stats_callback_)
+    stats_callback_->OnCompleteFrame(frame->num_references == 0, frame->size());
 
   FrameKey key(frame->picture_id, frame->spatial_layer);
   int last_continuous_picture_id =
@@ -381,28 +381,22 @@ bool FrameBuffer::UpdateFrameInfoWithIncomingFrame(const FrameObject& frame,
 }
 
 void FrameBuffer::UpdateJitterDelay() {
-  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_);
+  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);
   }
 }
 

diff --git a/webrtc/modules/video_coding/frame_buffer2.h b/webrtc/modules/video_coding/frame_buffer2.h
@@ -28,6 +28,7 @@
 namespace webrtc {
 
 class Clock;
+class VCMReceiveStatisticsCallback;
 class VCMJitterEstimator;
 class VCMTiming;
 
@@ -39,7 +40,8 @@ class FrameBuffer {
 
   FrameBuffer(Clock* clock,
               VCMJitterEstimator* jitter_estimator,
-              VCMTiming* timing);
+              VCMTiming* timing,
+              VCMReceiveStatisticsCallback* stats_proxy);
 
   virtual ~FrameBuffer();
 
@@ -141,8 +143,6 @@ class FrameBuffer {
 
   void UpdateJitterDelay() EXCLUSIVE_LOCKS_REQUIRED(crit_);
 
-  void UpdateHistograms() const;
-
   void ClearFramesAndHistory() EXCLUSIVE_LOCKS_REQUIRED(crit_);
 
   FrameMap frames_ GUARDED_BY(crit_);
@@ -160,16 +160,9 @@ 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,6 +25,9 @@
 #include "webrtc/test/gmock.h"
 #include "webrtc/test/gtest.h"
 
+using testing::_;
+using testing::Return;
+
 namespace webrtc {
 namespace video_coding {
 
@@ -54,6 +57,16 @@ 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;
@@ -82,6 +95,27 @@ 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 {
@@ -95,7 +129,7 @@ class TestFrameBuffer2 : public ::testing::Test {
       : clock_(0),
         timing_(&clock_),
         jitter_estimator_(&clock_),
-        buffer_(&clock_, &jitter_estimator_, &timing_),
+        buffer_(&clock_, &jitter_estimator_, &timing_, &stats_callback_),
         rand_(0x34678213),
         tear_down_(false),
         extract_thread_(&ExtractLoop, this, "Extract Thread"),
@@ -190,6 +224,7 @@ 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_;
@@ -436,5 +471,30 @@ 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,8 +90,16 @@ 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.
-  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;
+  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;
 
   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,31 +56,14 @@ 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) {
-      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);
-      }
+      _receiveStatsCallback->OnReceiveRatesUpdated(0, 0);
     }
   }
 
@@ -292,7 +275,7 @@ int32_t VideoReceiver::Decode(uint16_t maxWaitTimeMs) {
   return ret;
 }
 
-// Used for the WebRTC-NewVideoJitterBuffer experiment.
+// Used for the new jitter buffer.
 // 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
@@ -1234,9 +1234,6 @@ 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);
 }
 
@@ -3036,10 +3033,6 @@ 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);
 }