Fixed bug in test frame generator, causing incorrect reuse of frame
object, in turn causing performance regression.
Plus a small optimization.
BUG=chromium:460954, 4329
R=pbos@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/42499004
Cr-Commit-Position: refs/heads/master@{#8552}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8552 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/test/frame_generator.cc b/webrtc/test/frame_generator.cc
index 0841c74..fe6b1b1 100644
--- a/webrtc/test/frame_generator.cc
+++ b/webrtc/test/frame_generator.cc
@@ -69,7 +69,7 @@
current_display_count_(0) {
assert(width > 0);
assert(height > 0);
- ReadNextFrame();
+ assert(frame_repeat_count > 0);
}
virtual ~YuvFileGenerator() {
@@ -78,17 +78,18 @@
}
virtual I420VideoFrame* NextFrame() OVERRIDE {
- if (frame_display_count_ > 0) {
- if (current_display_count_ < frame_display_count_) {
- ++current_display_count_;
- } else {
- ReadNextFrame();
- current_display_count_ = 0;
- }
- }
+ if (current_display_count_ == 0)
+ ReadNextFrame();
+ if (++current_display_count_ >= frame_display_count_)
+ current_display_count_ = 0;
- current_frame_.CopyFrame(last_read_frame_);
- return ¤t_frame_;
+ // If this is the last repeatition of this frame, it's OK to use the
+ // original instance, otherwise use a copy.
+ if (current_display_count_ == frame_display_count_)
+ return &last_read_frame_;
+
+ temp_frame_copy_.CopyFrame(last_read_frame_);
+ return &temp_frame_copy_;
}
void ReadNextFrame() {
@@ -121,8 +122,8 @@
const rtc::scoped_ptr<uint8_t[]> frame_buffer_;
const int frame_display_count_;
int current_display_count_;
- I420VideoFrame current_frame_;
I420VideoFrame last_read_frame_;
+ I420VideoFrame temp_frame_copy_;
};
} // namespace
diff --git a/webrtc/test/frame_generator.h b/webrtc/test/frame_generator.h
index 4598de8..969a358 100644
--- a/webrtc/test/frame_generator.h
+++ b/webrtc/test/frame_generator.h
@@ -33,7 +33,7 @@
// Creates a frame generator that repeatedly plays a set of yuv files.
// The frame_repeat_count determines how many times each frame is shown,
- // with 0 = show the first frame indefinitely, 1 = show each frame once, etc.
+ // with 1 = show each frame once, etc.
static FrameGenerator* CreateFromYuvFile(std::vector<std::string> files,
size_t width,
size_t height,
diff --git a/webrtc/test/frame_generator_unittest.cc b/webrtc/test/frame_generator_unittest.cc
new file mode 100644
index 0000000..7d9a3a4
--- /dev/null
+++ b/webrtc/test/frame_generator_unittest.cc
@@ -0,0 +1,152 @@
+/*
+ * Copyright (c) 2015 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include <stdio.h>
+#include <string>
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "webrtc/base/scoped_ptr.h"
+#include "webrtc/test/frame_generator.h"
+#include "webrtc/test/testsupport/fileutils.h"
+
+namespace webrtc {
+namespace test {
+
+static const int kFrameWidth = 4;
+static const int kFrameHeight = 4;
+
+class FrameGeneratorTest : public ::testing::Test {
+ public:
+ void SetUp() override {
+ two_frame_filename_ =
+ test::TempFilename(test::OutputPath(), "2_frame_yuv_file");
+ one_frame_filename_ =
+ test::TempFilename(test::OutputPath(), "1_frame_yuv_file");
+
+ FILE* file = fopen(two_frame_filename_.c_str(), "wb");
+ WriteYuvFile(file, 0, 0, 0);
+ WriteYuvFile(file, 127, 127, 127);
+ fclose(file);
+ file = fopen(one_frame_filename_.c_str(), "wb");
+ WriteYuvFile(file, 255, 255, 255);
+ fclose(file);
+ }
+ void TearDown() override {
+ remove(one_frame_filename_.c_str());
+ remove(two_frame_filename_.c_str());
+ }
+
+ protected:
+ void WriteYuvFile(FILE* file, uint8_t y, uint8_t u, uint8_t v) {
+ assert(file);
+ rtc::scoped_ptr<uint8_t[]> plane_buffer(new uint8_t[y_size]);
+ memset(plane_buffer.get(), y, y_size);
+ fwrite(plane_buffer.get(), 1, y_size, file);
+ memset(plane_buffer.get(), u, uv_size);
+ fwrite(plane_buffer.get(), 1, uv_size, file);
+ memset(plane_buffer.get(), v, uv_size);
+ fwrite(plane_buffer.get(), 1, uv_size, file);
+ }
+
+ void CheckFrameAndMutate(I420VideoFrame* frame,
+ uint8_t y,
+ uint8_t u,
+ uint8_t v) {
+ // Check that frame is valid, has the correct color and timestamp are clean.
+ ASSERT_NE(nullptr, frame);
+ uint8_t* buffer;
+ ASSERT_EQ(y_size, frame->allocated_size(PlaneType::kYPlane));
+ buffer = frame->buffer(PlaneType::kYPlane);
+ for (int i = 0; i < y_size; ++i)
+ ASSERT_EQ(y, buffer[i]);
+ ASSERT_EQ(uv_size, frame->allocated_size(PlaneType::kUPlane));
+ buffer = frame->buffer(PlaneType::kUPlane);
+ for (int i = 0; i < uv_size; ++i)
+ ASSERT_EQ(u, buffer[i]);
+ ASSERT_EQ(uv_size, frame->allocated_size(PlaneType::kVPlane));
+ buffer = frame->buffer(PlaneType::kVPlane);
+ for (int i = 0; i < uv_size; ++i)
+ ASSERT_EQ(v, buffer[i]);
+ EXPECT_EQ(0, frame->ntp_time_ms());
+ EXPECT_EQ(0, frame->render_time_ms());
+ EXPECT_EQ(0u, frame->timestamp());
+
+ // Mutate to something arbitrary non-zero.
+ frame->set_ntp_time_ms(11);
+ frame->set_render_time_ms(12);
+ frame->set_timestamp(13);
+ }
+
+ std::string two_frame_filename_;
+ std::string one_frame_filename_;
+ const int y_size = kFrameWidth * kFrameHeight;
+ const int uv_size = ((kFrameHeight + 1) / 2) * ((kFrameWidth + 1) / 2);
+};
+
+TEST_F(FrameGeneratorTest, SingleFrameFile) {
+ rtc::scoped_ptr<FrameGenerator> generator(FrameGenerator::CreateFromYuvFile(
+ std::vector<std::string>(1, one_frame_filename_), kFrameWidth,
+ kFrameHeight, 1));
+ CheckFrameAndMutate(generator->NextFrame(), 255, 255, 255);
+ CheckFrameAndMutate(generator->NextFrame(), 255, 255, 255);
+}
+
+TEST_F(FrameGeneratorTest, TwoFrameFile) {
+ rtc::scoped_ptr<FrameGenerator> generator(FrameGenerator::CreateFromYuvFile(
+ std::vector<std::string>(1, two_frame_filename_), kFrameWidth,
+ kFrameHeight, 1));
+ CheckFrameAndMutate(generator->NextFrame(), 0, 0, 0);
+ CheckFrameAndMutate(generator->NextFrame(), 127, 127, 127);
+ CheckFrameAndMutate(generator->NextFrame(), 0, 0, 0);
+}
+
+TEST_F(FrameGeneratorTest, MultipleFrameFiles) {
+ std::vector<std::string> files;
+ files.push_back(two_frame_filename_);
+ files.push_back(one_frame_filename_);
+
+ rtc::scoped_ptr<FrameGenerator> generator(
+ FrameGenerator::CreateFromYuvFile(files, kFrameWidth, kFrameHeight, 1));
+ CheckFrameAndMutate(generator->NextFrame(), 0, 0, 0);
+ CheckFrameAndMutate(generator->NextFrame(), 127, 127, 127);
+ CheckFrameAndMutate(generator->NextFrame(), 255, 255, 255);
+ CheckFrameAndMutate(generator->NextFrame(), 0, 0, 0);
+}
+
+TEST_F(FrameGeneratorTest, TwoFrameFileWithRepeat) {
+ const int kRepeatCount = 3;
+ rtc::scoped_ptr<FrameGenerator> generator(FrameGenerator::CreateFromYuvFile(
+ std::vector<std::string>(1, two_frame_filename_), kFrameWidth,
+ kFrameHeight, kRepeatCount));
+ for (int i = 0; i < kRepeatCount; ++i)
+ CheckFrameAndMutate(generator->NextFrame(), 0, 0, 0);
+ for (int i = 0; i < kRepeatCount; ++i)
+ CheckFrameAndMutate(generator->NextFrame(), 127, 127, 127);
+ CheckFrameAndMutate(generator->NextFrame(), 0, 0, 0);
+}
+
+TEST_F(FrameGeneratorTest, MultipleFrameFilesWithRepeat) {
+ const int kRepeatCount = 3;
+ std::vector<std::string> files;
+ files.push_back(two_frame_filename_);
+ files.push_back(one_frame_filename_);
+ rtc::scoped_ptr<FrameGenerator> generator(FrameGenerator::CreateFromYuvFile(
+ files, kFrameWidth, kFrameHeight, kRepeatCount));
+ for (int i = 0; i < kRepeatCount; ++i)
+ CheckFrameAndMutate(generator->NextFrame(), 0, 0, 0);
+ for (int i = 0; i < kRepeatCount; ++i)
+ CheckFrameAndMutate(generator->NextFrame(), 127, 127, 127);
+ for (int i = 0; i < kRepeatCount; ++i)
+ CheckFrameAndMutate(generator->NextFrame(), 255, 255, 255);
+ CheckFrameAndMutate(generator->NextFrame(), 0, 0, 0);
+}
+
+} // namespace test
+} // namespace webrtc
diff --git a/webrtc/test/webrtc_test_common.gyp b/webrtc/test/webrtc_test_common.gyp
index bb6224e..ea6e405 100644
--- a/webrtc/test/webrtc_test_common.gyp
+++ b/webrtc/test/webrtc_test_common.gyp
@@ -157,6 +157,7 @@
],
'sources': [
'fake_network_pipe_unittest.cc',
+ 'frame_generator_unittest.cc',
'rtp_file_reader_unittest.cc',
'rtp_file_writer_unittest.cc',
],
