diff --git a/webrtc/common_video/BUILD.gn b/webrtc/common_video/BUILD.gn index afe2be9bb1..24423f5d33 100644 --- a/webrtc/common_video/BUILD.gn +++ b/webrtc/common_video/BUILD.gn @@ -17,8 +17,10 @@ config("common_video_config") { 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 84cbc7491b..5a412014ea 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 e79b063c5c..beeab5ddca 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 0000000000..b6ad2ba2e4 --- /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& 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(cbuffer->data(type)); + } + int stride(webrtc::PlaneType type) const override { + return buffer_->stride(type); + } + rtc::scoped_refptr native_handle() const override { + return nullptr; + } + + friend class rtc::RefCountedObject; + rtc::scoped_refptr buffer_; +}; + +} // namespace + +namespace webrtc { + +I420BufferPool::I420BufferPool() { + thread_checker_.DetachFromThread(); +} + +rtc::scoped_refptr 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& 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(buffer); + } + // Allocate new buffer. + buffers_.push_back(new rtc::RefCountedObject(width, height)); + return new rtc::RefCountedObject(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 0000000000..625160be11 --- /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 + +#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 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 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 buffer = pool.CreateBuffer(16, 16); + EXPECT_TRUE(buffer->HasOneRef()); +} + +TEST(TestI420BufferPool, FrameValidAfterPoolDestruction) { + rtc::scoped_refptr 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 0000000000..e9dff8ca5e --- /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 + +#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 CreateBuffer(int width, int height); + + private: + rtc::ThreadChecker thread_checker_; + std::list> 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 f9fa7034b4..9c9c22d21d 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 @@ int VP8DecoderImpl::ReturnFrame(const vpx_image_t* img, 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 @@ VideoDecoder* VP8DecoderImpl::Copy() { 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 @@ VideoDecoder* VP8DecoderImpl::Copy() { 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(ref_frame_->img.d_w) || - decoded_image_.height() != static_cast(ref_frame_->img.d_h) || + (last_frame_width_ != static_cast(ref_frame_->img.d_w) || + last_frame_height_ != static_cast(ref_frame_->img.d_h) || image_format_ != ref_frame_->img.fmt)) { vpx_img_free(&ref_frame_->img); delete ref_frame_; @@ -1430,7 +1432,7 @@ VideoDecoder* VP8DecoderImpl::Copy() { // for the y plane, but only half of it to the u and v planes. if (!vpx_img_alloc(&ref_frame_->img, static_cast(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 d5d66fca5d..fe7cf43342 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 @@ class VP8DecoderImpl : public VP8Decoder { uint32_t timeStamp, int64_t ntp_time_ms); - I420VideoFrame decoded_image_; + I420BufferPool buffer_pool_; DecodedImageCallback* decode_complete_callback_; bool inited_; bool feedback_mode_;