Add I420 buffer pool to avoid unnecessary allocations
Now when we don't use SwapFrame consistently anymore, we need to recycle allocations with a buffer pool instead. This CL adds a buffer pool class, and updates the vp8 decoder to use it. If this CL lands successfully I will update the other video producers as well.
BUG=1128
R=stefan@webrtc.org, tommi@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/41189004
Cr-Commit-Position: refs/heads/master@{#8748}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8748 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/common_video/BUILD.gn b/webrtc/common_video/BUILD.gn
index afe2be9..24423f5 100644
--- a/webrtc/common_video/BUILD.gn
+++ b/webrtc/common_video/BUILD.gn
@@ -17,8 +17,10 @@
source_set("common_video") {
sources = [
+ "i420_buffer_pool.cc",
"i420_video_frame.cc",
"interface/i420_video_frame.h",
+ "interface/i420_buffer_pool.h",
"interface/native_handle.h",
"interface/video_frame_buffer.h",
"libyuv/include/scaler.h",
diff --git a/webrtc/common_video/common_video.gyp b/webrtc/common_video/common_video.gyp
index 84cbc74..5a41201 100644
--- a/webrtc/common_video/common_video.gyp
+++ b/webrtc/common_video/common_video.gyp
@@ -39,9 +39,11 @@
}],
],
'sources': [
+ 'interface/i420_buffer_pool.h',
'interface/i420_video_frame.h',
'interface/native_handle.h',
'interface/video_frame_buffer.h',
+ 'i420_buffer_pool.cc',
'i420_video_frame.cc',
'libyuv/include/webrtc_libyuv.h',
'libyuv/include/scaler.h',
diff --git a/webrtc/common_video/common_video_unittests.gyp b/webrtc/common_video/common_video_unittests.gyp
index e79b063..beeab5d 100644
--- a/webrtc/common_video/common_video_unittests.gyp
+++ b/webrtc/common_video/common_video_unittests.gyp
@@ -19,6 +19,7 @@
'<(webrtc_root)/test/test.gyp:test_support_main',
],
'sources': [
+ 'i420_buffer_pool_unittest.cc',
'i420_video_frame_unittest.cc',
'libyuv/libyuv_unittest.cc',
'libyuv/scaler_unittest.cc',
diff --git a/webrtc/common_video/i420_buffer_pool.cc b/webrtc/common_video/i420_buffer_pool.cc
new file mode 100644
index 0000000..b6ad2ba
--- /dev/null
+++ b/webrtc/common_video/i420_buffer_pool.cc
@@ -0,0 +1,82 @@
+/*
+ * 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 "webrtc/common_video/interface/i420_buffer_pool.h"
+
+#include "webrtc/base/checks.h"
+
+namespace {
+
+// One extra indirection is needed to make |HasOneRef| work.
+class PooledI420Buffer : public webrtc::VideoFrameBuffer {
+ public:
+ explicit PooledI420Buffer(
+ const rtc::scoped_refptr<webrtc::I420Buffer>& buffer)
+ : buffer_(buffer) {}
+
+ private:
+ ~PooledI420Buffer() override {}
+
+ int width() const override { return buffer_->width(); }
+ int height() const override { return buffer_->height(); }
+ const uint8_t* data(webrtc::PlaneType type) const override {
+ const webrtc::I420Buffer* cbuffer = buffer_.get();
+ return cbuffer->data(type);
+ }
+ uint8_t* data(webrtc::PlaneType type) {
+ DCHECK(HasOneRef());
+ const webrtc::I420Buffer* cbuffer = buffer_.get();
+ return const_cast<uint8_t*>(cbuffer->data(type));
+ }
+ int stride(webrtc::PlaneType type) const override {
+ return buffer_->stride(type);
+ }
+ rtc::scoped_refptr<webrtc::NativeHandle> native_handle() const override {
+ return nullptr;
+ }
+
+ friend class rtc::RefCountedObject<PooledI420Buffer>;
+ rtc::scoped_refptr<webrtc::I420Buffer> buffer_;
+};
+
+} // namespace
+
+namespace webrtc {
+
+I420BufferPool::I420BufferPool() {
+ thread_checker_.DetachFromThread();
+}
+
+rtc::scoped_refptr<VideoFrameBuffer> I420BufferPool::CreateBuffer(int width,
+ int height) {
+ DCHECK(thread_checker_.CalledOnValidThread());
+ // Release buffers with wrong resolution.
+ for (auto it = buffers_.begin(); it != buffers_.end();) {
+ if ((*it)->width() != width || (*it)->height() != height)
+ it = buffers_.erase(it);
+ else
+ ++it;
+ }
+ // Look for a free buffer.
+ for (const rtc::scoped_refptr<I420Buffer>& buffer : buffers_) {
+ // If the buffer is in use, the ref count will be 2, one from the list we
+ // are looping over and one from a PooledI420Buffer returned from
+ // CreateBuffer that has not been released yet. If the ref count is 1
+ // (HasOneRef), then the list we are looping over holds the only reference
+ // and it's safe to reuse.
+ if (buffer->HasOneRef())
+ return new rtc::RefCountedObject<PooledI420Buffer>(buffer);
+ }
+ // Allocate new buffer.
+ buffers_.push_back(new rtc::RefCountedObject<I420Buffer>(width, height));
+ return new rtc::RefCountedObject<PooledI420Buffer>(buffers_.back());
+}
+
+} // namespace webrtc
diff --git a/webrtc/common_video/i420_buffer_pool_unittest.cc b/webrtc/common_video/i420_buffer_pool_unittest.cc
new file mode 100644
index 0000000..625160b
--- /dev/null
+++ b/webrtc/common_video/i420_buffer_pool_unittest.cc
@@ -0,0 +1,74 @@
+/*
+ * 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 <string>
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "webrtc/common_video/interface/i420_buffer_pool.h"
+
+namespace webrtc {
+
+TEST(TestI420BufferPool, SimpleFrameReuse) {
+ I420BufferPool pool;
+ rtc::scoped_refptr<VideoFrameBuffer> buffer = pool.CreateBuffer(16, 16);
+ EXPECT_EQ(16, buffer->width());
+ EXPECT_EQ(16, buffer->height());
+ // Extract non-refcounted pointers for testing.
+ const uint8_t* y_ptr = buffer->data(kYPlane);
+ const uint8_t* u_ptr = buffer->data(kUPlane);
+ const uint8_t* v_ptr = buffer->data(kVPlane);
+ // Release buffer so that it is returned to the pool.
+ buffer = nullptr;
+ // Check that the memory is resued.
+ buffer = pool.CreateBuffer(16, 16);
+ EXPECT_EQ(y_ptr, buffer->data(kYPlane));
+ EXPECT_EQ(u_ptr, buffer->data(kUPlane));
+ EXPECT_EQ(v_ptr, buffer->data(kVPlane));
+ EXPECT_EQ(16, buffer->width());
+ EXPECT_EQ(16, buffer->height());
+}
+
+TEST(TestI420BufferPool, FailToReuse) {
+ I420BufferPool pool;
+ rtc::scoped_refptr<VideoFrameBuffer> buffer = pool.CreateBuffer(16, 16);
+ // Extract non-refcounted pointers for testing.
+ const uint8_t* u_ptr = buffer->data(kUPlane);
+ const uint8_t* v_ptr = buffer->data(kVPlane);
+ // Release buffer so that it is returned to the pool.
+ buffer = nullptr;
+ // Check that the pool doesn't try to reuse buffers of incorrect size.
+ buffer = pool.CreateBuffer(32, 16);
+ EXPECT_EQ(32, buffer->width());
+ EXPECT_EQ(16, buffer->height());
+ EXPECT_NE(u_ptr, buffer->data(kUPlane));
+ EXPECT_NE(v_ptr, buffer->data(kVPlane));
+}
+
+TEST(TestI420BufferPool, ExclusiveOwner) {
+ // Check that created buffers are exclusive so that they can be written to.
+ I420BufferPool pool;
+ rtc::scoped_refptr<VideoFrameBuffer> buffer = pool.CreateBuffer(16, 16);
+ EXPECT_TRUE(buffer->HasOneRef());
+}
+
+TEST(TestI420BufferPool, FrameValidAfterPoolDestruction) {
+ rtc::scoped_refptr<VideoFrameBuffer> buffer;
+ {
+ I420BufferPool pool;
+ buffer = pool.CreateBuffer(16, 16);
+ }
+ EXPECT_TRUE(buffer->HasOneRef());
+ EXPECT_EQ(16, buffer->width());
+ EXPECT_EQ(16, buffer->height());
+ // Try to trigger use-after-free errors by writing to y-plane.
+ memset(buffer->data(kYPlane), 0xA5, 16 * buffer->stride(kYPlane));
+}
+
+} // namespace webrtc
diff --git a/webrtc/common_video/interface/i420_buffer_pool.h b/webrtc/common_video/interface/i420_buffer_pool.h
new file mode 100644
index 0000000..e9dff8c
--- /dev/null
+++ b/webrtc/common_video/interface/i420_buffer_pool.h
@@ -0,0 +1,40 @@
+/*
+ * 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.
+ */
+
+#ifndef WEBRTC_COMMON_VIDEO_INTERFACE_I420_BUFFER_POOL_H_
+#define WEBRTC_COMMON_VIDEO_INTERFACE_I420_BUFFER_POOL_H_
+
+#include <list>
+
+#include "webrtc/base/thread_checker.h"
+#include "webrtc/common_video/interface/video_frame_buffer.h"
+
+namespace webrtc {
+
+// Simple buffer pool to avoid unnecessary allocations of I420Buffer objects.
+// The pool manages the memory of the I420Buffer returned from CreateBuffer.
+// When the I420Buffer is destructed, the memory is returned to the pool for use
+// by subsequent calls to CreateBuffer. If the resolution passed to CreateBuffer
+// changes, old buffers will be purged from the pool.
+class I420BufferPool {
+ public:
+ I420BufferPool();
+ // Returns a buffer from the pool, or creates a new buffer if no suitable
+ // buffer exists in the pool.
+ rtc::scoped_refptr<VideoFrameBuffer> CreateBuffer(int width, int height);
+
+ private:
+ rtc::ThreadChecker thread_checker_;
+ std::list<rtc::scoped_refptr<I420Buffer>> buffers_;
+};
+
+} // namespace webrtc
+
+#endif // WEBRTC_COMMON_VIDEO_INTERFACE_I420_BUFFER_POOL_H_
diff --git a/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc b/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc
index f9fa703..9c9c22d 100644
--- a/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc
+++ b/webrtc/modules/video_coding/codecs/vp8/vp8_impl.cc
@@ -17,6 +17,7 @@
// NOTE(ajm): Path provided by gyp.
#include "libyuv/scale.h" // NOLINT
+#include "libyuv/convert.h" // NOLINT
#include "webrtc/common.h"
#include "webrtc/common_types.h"
@@ -1333,17 +1334,18 @@
last_frame_width_ = img->d_w;
last_frame_height_ = img->d_h;
// Allocate memory for decoded image.
- // TODO(mikhal): This does a copy - need to SwapBuffers.
- decoded_image_.CreateFrame(img->planes[VPX_PLANE_Y],
- img->planes[VPX_PLANE_U],
- img->planes[VPX_PLANE_V],
- img->d_w, img->d_h,
- img->stride[VPX_PLANE_Y],
- img->stride[VPX_PLANE_U],
- img->stride[VPX_PLANE_V]);
- decoded_image_.set_timestamp(timestamp);
- decoded_image_.set_ntp_time_ms(ntp_time_ms);
- int ret = decode_complete_callback_->Decoded(decoded_image_);
+ I420VideoFrame decoded_image(buffer_pool_.CreateBuffer(img->d_w, img->d_h),
+ timestamp, 0, kVideoRotation_0);
+ libyuv::I420Copy(
+ img->planes[VPX_PLANE_Y], img->stride[VPX_PLANE_Y],
+ img->planes[VPX_PLANE_U], img->stride[VPX_PLANE_U],
+ img->planes[VPX_PLANE_V], img->stride[VPX_PLANE_V],
+ decoded_image.buffer(kYPlane), decoded_image.stride(kYPlane),
+ decoded_image.buffer(kUPlane), decoded_image.stride(kUPlane),
+ decoded_image.buffer(kVPlane), decoded_image.stride(kVPlane),
+ img->d_w, img->d_h);
+ decoded_image.set_ntp_time_ms(ntp_time_ms);
+ int ret = decode_complete_callback_->Decoded(decoded_image);
if (ret != 0)
return ret;
@@ -1386,7 +1388,7 @@
assert(false);
return NULL;
}
- if (decoded_image_.IsZeroSize()) {
+ if (last_frame_width_ == 0 || last_frame_height_ == 0) {
// Nothing has been decoded before; cannot clone.
return NULL;
}
@@ -1409,13 +1411,13 @@
return NULL;
}
// Allocate memory for reference image copy
- assert(decoded_image_.width() > 0);
- assert(decoded_image_.height() > 0);
+ assert(last_frame_width_ > 0);
+ assert(last_frame_height_ > 0);
assert(image_format_ > VPX_IMG_FMT_NONE);
// Check if frame format has changed.
if (ref_frame_ &&
- (decoded_image_.width() != static_cast<int>(ref_frame_->img.d_w) ||
- decoded_image_.height() != static_cast<int>(ref_frame_->img.d_h) ||
+ (last_frame_width_ != static_cast<int>(ref_frame_->img.d_w) ||
+ last_frame_height_ != static_cast<int>(ref_frame_->img.d_h) ||
image_format_ != ref_frame_->img.fmt)) {
vpx_img_free(&ref_frame_->img);
delete ref_frame_;
@@ -1430,7 +1432,7 @@
// for the y plane, but only half of it to the u and v planes.
if (!vpx_img_alloc(&ref_frame_->img,
static_cast<vpx_img_fmt_t>(image_format_),
- decoded_image_.width(), decoded_image_.height(),
+ last_frame_width_, last_frame_height_,
kVp832ByteAlign)) {
assert(false);
delete copy;
diff --git a/webrtc/modules/video_coding/codecs/vp8/vp8_impl.h b/webrtc/modules/video_coding/codecs/vp8/vp8_impl.h
index d5d66fc..fe7cf43 100644
--- a/webrtc/modules/video_coding/codecs/vp8/vp8_impl.h
+++ b/webrtc/modules/video_coding/codecs/vp8/vp8_impl.h
@@ -22,6 +22,7 @@
#include "vpx/vp8cx.h"
#include "vpx/vp8dx.h"
+#include "webrtc/common_video/interface/i420_buffer_pool.h"
#include "webrtc/common_video/interface/i420_video_frame.h"
#include "webrtc/modules/video_coding/codecs/interface/video_codec_interface.h"
#include "webrtc/modules/video_coding/codecs/vp8/include/vp8.h"
@@ -154,7 +155,7 @@
uint32_t timeStamp,
int64_t ntp_time_ms);
- I420VideoFrame decoded_image_;
+ I420BufferPool buffer_pool_;
DecodedImageCallback* decode_complete_callback_;
bool inited_;
bool feedback_mode_;
