Revert Make the new jitter buffer the default jitter buffer.
Speculative revert of https://codereview.chromium.org/2656983002/ to see if it fixes a downstream bug.
BUG=webrtc:5514
Review-Url: https://codereview.webrtc.org/2682073003
Cr-Commit-Position: refs/heads/master@{#16492}
diff --git a/webrtc/modules/video_coding/frame_buffer2.cc b/webrtc/modules/video_coding/frame_buffer2.cc
index dcbcb1f..027b943 100644
--- 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 @@
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 @@
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,
@@ -173,8 +172,9 @@
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 =
@@ -388,22 +388,28 @@
}
void FrameBuffer::UpdateJitterDelay() {
- if (!stats_callback_)
- return;
+ int unused;
+ int delay;
+ timing_->GetTimings(&unused, &unused, &unused, &unused, &delay, &unused,
+ &unused);
- 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, ¤t_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);
+ 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
index b554f5b..529428d 100644
--- 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 @@
FrameBuffer(Clock* clock,
VCMJitterEstimator* jitter_estimator,
- VCMTiming* timing,
- VCMReceiveStatisticsCallback* stats_proxy);
+ VCMTiming* timing);
virtual ~FrameBuffer();
@@ -143,6 +141,8 @@
void UpdateJitterDelay() EXCLUSIVE_LOCKS_REQUIRED(crit_);
+ void UpdateHistograms() const;
+
void ClearFramesAndHistory() EXCLUSIVE_LOCKS_REQUIRED(crit_);
FrameMap frames_ GUARDED_BY(crit_);
@@ -161,9 +161,16 @@
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
index c3d8ce7..1761f58 100644
--- 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 @@
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 @@
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 @@
: 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 @@
FrameBuffer buffer_;
std::vector<std::unique_ptr<FrameObject>> frames_;
Random rand_;
- ::testing::NiceMock<VCMReceiveStatisticsCallbackMock> stats_callback_;
int64_t max_wait_time_;
bool tear_down_;
@@ -472,30 +437,5 @@
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
index dede5b6..122ddc6 100644
--- 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 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
index 429c282..e7d2b1f 100644
--- a/webrtc/modules/video_coding/timing.h
+++ b/webrtc/modules/video_coding/timing.h
@@ -94,13 +94,13 @@
// 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
index 14f1265..129a1b5 100644
--- a/webrtc/modules/video_coding/video_receiver.cc
+++ b/webrtc/modules/video_coding/video_receiver.cc
@@ -56,14 +56,31 @@
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, ¤t_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 @@
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) {