Skip to content

Commit

Permalink
Implement new video encoder performance tests, for VP9/VP8 comparison.
Browse files Browse the repository at this point in the history
The old encoder perf tests (in VideoEncoderVpxTest) were not providing
very realistic results because:
 - They were measuring performance on frames with random data.
 - They were testing the case when the whole frame is changing every
   time.
 - They were using real clock, which affects results.

This change implements new tests that provide closer approximation to
how the codec is being used.

CyclicFrameGenerator added in this test will also be used to test
performance of the new WebRTC-based protocol.

Review URL: https://codereview.chromium.org/1601303002

Cr-Commit-Position: refs/heads/master@{#370491}
  • Loading branch information
SergeyUlanov authored and Commit bot committed Jan 20, 2016
1 parent 7f7f154 commit 2e0319b
Show file tree
Hide file tree
Showing 12 changed files with 409 additions and 118 deletions.
4 changes: 1 addition & 3 deletions remoting/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,7 @@ if (!is_mac) {
if (enable_remoting_host) {
test("remoting_perftests") {
sources = [
"codec/codec_test.cc",
"codec/codec_test.h",
"codec/video_encoder_vpx_perftest.cc",
"test/codec_perftest.cc",
"test/protocol_perftest.cc",
]

Expand Down
44 changes: 0 additions & 44 deletions remoting/codec/codec_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -343,48 +343,4 @@ void TestVideoEncoderDecoderGradient(VideoEncoder* encoder,
decoder_tester.VerifyResultsApprox(max_error_limit, mean_error_limit);
}

float MeasureVideoEncoderFpsWithSize(VideoEncoder* encoder,
const DesktopSize& size) {
scoped_ptr<DesktopFrame> frame(PrepareFrame(size));
frame->mutable_updated_region()->SetRect(DesktopRect::MakeSize(size));
std::list<DesktopFrame*> frames;
frames.push_back(frame.get());
return MeasureVideoEncoderFpsWithFrames(encoder, frames);
}

float MeasureVideoEncoderFpsWithFrames(VideoEncoder* encoder,
const std::list<DesktopFrame*>& frames) {
const base::TimeDelta kTestTime = base::TimeDelta::FromSeconds(1);

// Encode some frames to "warm up" the encoder (i.e. to let it set up initial
// structures, establish a stable working set, etc), then encode at least
// kMinimumFrameCount frames to measure the encoder's performance.
const int kWarmUpFrameCount = 10;
const int kMinimumFrameCount = 10;
base::TimeTicks start_time;
base::TimeDelta elapsed;
std::list<DesktopFrame*> test_frames;
int frame_count;
for (frame_count = 0;
(frame_count < kMinimumFrameCount + kWarmUpFrameCount ||
elapsed < kTestTime);
++frame_count) {
if (frame_count == kWarmUpFrameCount) {
start_time = base::TimeTicks::Now();
}

if (test_frames.empty()) {
test_frames = frames;
}
scoped_ptr<VideoPacket> packet = encoder->Encode(*test_frames.front());
test_frames.pop_front();

if (frame_count >= kWarmUpFrameCount) {
elapsed = base::TimeTicks::Now() - start_time;
}
}

return (frame_count * base::TimeDelta::FromSeconds(1)) / elapsed;
}

} // namespace remoting
13 changes: 9 additions & 4 deletions remoting/codec/video_encoder_vpx.cc
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,10 @@ scoped_ptr<VideoEncoderVpx> VideoEncoderVpx::CreateForVP9() {

VideoEncoderVpx::~VideoEncoderVpx() {}

void VideoEncoderVpx::SetTickClockForTests(base::TickClock* tick_clock) {
clock_ = tick_clock;
}

void VideoEncoderVpx::SetLosslessEncode(bool want_lossless) {
if (use_vp9_ && (want_lossless != lossless_encode_)) {
lossless_encode_ = want_lossless;
Expand Down Expand Up @@ -296,7 +300,7 @@ scoped_ptr<VideoPacket> VideoEncoderVpx::Encode(
}

// Do the actual encoding.
int timestamp = (base::TimeTicks::Now() - timestamp_base_).InMilliseconds();
int timestamp = (clock_->NowTicks() - timestamp_base_).InMilliseconds();
vpx_codec_err_t ret = vpx_codec_encode(
codec_.get(), image_.get(), timestamp, 1, 0, VPX_DL_REALTIME);
DCHECK_EQ(ret, VPX_CODEC_OK)
Expand Down Expand Up @@ -345,8 +349,9 @@ scoped_ptr<VideoPacket> VideoEncoderVpx::Encode(
}

VideoEncoderVpx::VideoEncoderVpx(bool use_vp9)
: use_vp9_(use_vp9), encode_unchanged_frame_(false) {
}
: use_vp9_(use_vp9),
encode_unchanged_frame_(false),
clock_(&default_tick_clock_) {}

void VideoEncoderVpx::Configure(const webrtc::DesktopSize& size) {
DCHECK(use_vp9_ || !lossless_color_);
Expand Down Expand Up @@ -376,7 +381,7 @@ void VideoEncoderVpx::Configure(const webrtc::DesktopSize& size) {

// (Re)Set the base for frame timestamps if the codec is being (re)created.
if (!codec_) {
timestamp_base_ = base::TimeTicks::Now();
timestamp_base_ = clock_->NowTicks();
}

// Fetch a default configuration for the desired codec.
Expand Down
6 changes: 6 additions & 0 deletions remoting/codec/video_encoder_vpx.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "base/callback.h"
#include "base/macros.h"
#include "base/time/default_tick_clock.h"
#include "base/time/time.h"
#include "remoting/codec/scoped_vpx_codec.h"
#include "remoting/codec/video_encoder.h"
Expand All @@ -31,6 +32,8 @@ class VideoEncoderVpx : public VideoEncoder {

~VideoEncoderVpx() override;

void SetTickClockForTests(base::TickClock* tick_clock);

// VideoEncoder interface.
void SetLosslessEncode(bool want_lossless) override;
void SetLosslessColor(bool want_lossless) override;
Expand Down Expand Up @@ -84,6 +87,9 @@ class VideoEncoderVpx : public VideoEncoder {
// Used to help initialize VideoPackets from DesktopFrames.
VideoEncoderHelper helper_;

base::DefaultTickClock default_tick_clock_;
base::TickClock* clock_;

DISALLOW_COPY_AND_ASSIGN(VideoEncoderVpx);
};

Expand Down
64 changes: 0 additions & 64 deletions remoting/codec/video_encoder_vpx_perftest.cc

This file was deleted.

7 changes: 4 additions & 3 deletions remoting/remoting_test.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
'dependencies': [
'../base/base.gyp:base',
'../net/net.gyp:net_test_support',
'../skia/skia.gyp:skia',
'../testing/gmock.gyp:gmock',
'../testing/gtest.gyp:gtest',
'remoting_base',
Expand Down Expand Up @@ -65,6 +66,8 @@
'test/connection_setup_info.h',
'test/connection_time_observer.cc',
'test/connection_time_observer.h',
'test/cyclic_frame_generator.cc',
'test/cyclic_frame_generator.h',
'test/fake_access_token_fetcher.cc',
'test/fake_access_token_fetcher.h',
'test/fake_app_remoting_report_issue_request.cc',
Expand Down Expand Up @@ -557,9 +560,7 @@
],
'sources': [
'base/run_all_unittests.cc',
'codec/codec_test.cc',
'codec/codec_test.h',
'codec/video_encoder_vpx_perftest.cc',
'test/codec_perftest.cc',
'test/protocol_perftest.cc',
],
'conditions': [
Expand Down
3 changes: 3 additions & 0 deletions remoting/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ source_set("test_support") {
"connection_setup_info.h",
"connection_time_observer.cc",
"connection_time_observer.h",
"cyclic_frame_generator.cc",
"cyclic_frame_generator.h",
"fake_access_token_fetcher.cc",
"fake_access_token_fetcher.h",
"fake_app_remoting_report_issue_request.cc",
Expand Down Expand Up @@ -76,6 +78,7 @@ source_set("test_support") {

deps = [
"//google_apis",
"//skia",
"//testing/gmock",
"//testing/gtest",
"//third_party/libjingle",
Expand Down
1 change: 1 addition & 0 deletions remoting/test/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ include_rules = [
"+remoting/signaling",
"+ui/gfx",
"+ui/events/keycodes/dom",
"+third_party/skia",
]
Loading

0 comments on commit 2e0319b

Please sign in to comment.