From 73d763e71f5c77cac82aa65d737e64d893f190a0 Mon Sep 17 00:00:00 2001 From: "magjed@webrtc.org" Date: Tue, 17 Mar 2015 11:40:45 +0000 Subject: 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 --- webrtc/common_video/i420_buffer_pool.cc | 82 +++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 webrtc/common_video/i420_buffer_pool.cc (limited to 'webrtc/common_video/i420_buffer_pool.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 -- cgit v1.2.3 From a3209a2b27b7bf2059f8119a126a1b1be9f0377f Mon Sep 17 00:00:00 2001 From: "pbos@webrtc.org" Date: Fri, 20 Mar 2015 13:35:56 +0000 Subject: Release buffer pool in Vp8DecoderImpl::Release(). Permits reusing an external VP8DecoderImpl instance from another VideoReceiveStream without a thread-checker DCHECK blowing up. Also releases buffers that would've been kept in memory even though the decoder isn't configured. BUG= R=magjed@webrtc.org, stefan@webrtc.org Review URL: https://webrtc-codereview.appspot.com/50449004 Cr-Commit-Position: refs/heads/master@{#8807} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8807 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc/common_video/i420_buffer_pool.cc | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'webrtc/common_video/i420_buffer_pool.cc') diff --git a/webrtc/common_video/i420_buffer_pool.cc b/webrtc/common_video/i420_buffer_pool.cc index b6ad2ba2e4..04a0ab9b7f 100644 --- a/webrtc/common_video/i420_buffer_pool.cc +++ b/webrtc/common_video/i420_buffer_pool.cc @@ -51,7 +51,12 @@ class PooledI420Buffer : public webrtc::VideoFrameBuffer { namespace webrtc { I420BufferPool::I420BufferPool() { + Release(); +} + +void I420BufferPool::Release() { thread_checker_.DetachFromThread(); + buffers_.clear(); } rtc::scoped_refptr I420BufferPool::CreateBuffer(int width, -- cgit v1.2.3 From 75db8612588b4fabdf1b05f4ab145f7737093b45 Mon Sep 17 00:00:00 2001 From: Per Date: Tue, 7 Apr 2015 12:50:40 +0200 Subject: Remove usage of webrtc::NativeHandle since is just adds an extra level of indirection. BUG=1128 R=magjed@webrtc.org, pbos@webrtc.org TBR=mflodman@webrtc.org Review URL: https://webrtc-codereview.appspot.com/43999004 Cr-Commit-Position: refs/heads/master@{#8932} --- webrtc/common_video/i420_buffer_pool.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'webrtc/common_video/i420_buffer_pool.cc') diff --git a/webrtc/common_video/i420_buffer_pool.cc b/webrtc/common_video/i420_buffer_pool.cc index 04a0ab9b7f..35a6c10953 100644 --- a/webrtc/common_video/i420_buffer_pool.cc +++ b/webrtc/common_video/i420_buffer_pool.cc @@ -38,9 +38,7 @@ class PooledI420Buffer : public webrtc::VideoFrameBuffer { int stride(webrtc::PlaneType type) const override { return buffer_->stride(type); } - rtc::scoped_refptr native_handle() const override { - return nullptr; - } + void* native_handle() const override { return nullptr; } friend class rtc::RefCountedObject; rtc::scoped_refptr buffer_; -- cgit v1.2.3 From e41d774c4d0a60066866fc2d0ae48dd0e839ff23 Mon Sep 17 00:00:00 2001 From: Per Date: Tue, 7 Apr 2015 17:20:48 +0200 Subject: Revert "Remove usage of webrtc::NativeHandle since is just adds an extra level of indirection." This reverts commit 75db8612588b4fabdf1b05f4ab145f7737093b45. Revert "Fix build breakage in WrappedI420Buffer::native_handle()" This reverts commit 3211934ebf7cac3e6df2cb4aacb6e47cc1cffe2b. Reason for revert: Breaks chrome build and tests on clank, See https://codereview.chromium.org/1067803002/ BUG=1128 TBR=magjed@webrtc.org Review URL: https://webrtc-codereview.appspot.com/43079004 Cr-Commit-Position: refs/heads/master@{#8940} --- webrtc/common_video/i420_buffer_pool.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'webrtc/common_video/i420_buffer_pool.cc') diff --git a/webrtc/common_video/i420_buffer_pool.cc b/webrtc/common_video/i420_buffer_pool.cc index 35a6c10953..04a0ab9b7f 100644 --- a/webrtc/common_video/i420_buffer_pool.cc +++ b/webrtc/common_video/i420_buffer_pool.cc @@ -38,7 +38,9 @@ class PooledI420Buffer : public webrtc::VideoFrameBuffer { int stride(webrtc::PlaneType type) const override { return buffer_->stride(type); } - void* native_handle() const override { return nullptr; } + rtc::scoped_refptr native_handle() const override { + return nullptr; + } friend class rtc::RefCountedObject; rtc::scoped_refptr buffer_; -- cgit v1.2.3 From 9b3f56ea055934a5d5416db0386c857494410acc Mon Sep 17 00:00:00 2001 From: Per Date: Thu, 9 Apr 2015 13:44:16 +0200 Subject: Reland "Remove usage of webrtc::NativeHandle since is just adds an extra level of indirection."" This reverts commit e41d774c4d0a60066866fc2d0ae48dd0e839ff23. Original code review: https://webrtc-codereview.appspot.com/43999004/ Reason for reland: There was nothing wrong with this cl as is, but it breaks chrome compatibility. We will now reland this and fix Chrome during roll. Patset 1: Original cl. Patchset 2: Removed more code that is no longer needed. R=magjed@webrtc.org, pbos@webrtc.org TBR=mflodman@webrtc.org BUG=1128 Review URL: https://webrtc-codereview.appspot.com/45049004 Cr-Commit-Position: refs/heads/master@{#8956} --- webrtc/common_video/i420_buffer_pool.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'webrtc/common_video/i420_buffer_pool.cc') diff --git a/webrtc/common_video/i420_buffer_pool.cc b/webrtc/common_video/i420_buffer_pool.cc index 04a0ab9b7f..35a6c10953 100644 --- a/webrtc/common_video/i420_buffer_pool.cc +++ b/webrtc/common_video/i420_buffer_pool.cc @@ -38,9 +38,7 @@ class PooledI420Buffer : public webrtc::VideoFrameBuffer { int stride(webrtc::PlaneType type) const override { return buffer_->stride(type); } - rtc::scoped_refptr native_handle() const override { - return nullptr; - } + void* native_handle() const override { return nullptr; } friend class rtc::RefCountedObject; rtc::scoped_refptr buffer_; -- cgit v1.2.3 From a831dc3a7d10a1fbaa258ee6b1ca6cfc7e91c5ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Bostr=C3=B6m?= Date: Mon, 1 Jun 2015 20:06:42 +0200 Subject: Convert native handles to buffers before encoding. Required to permit conversion of NV12 handles on iOS to I420 for VP8 software encoding, which blocks texture-based capture. This change enforces that all texture-based input provides a method for converting native handles to I420 if they are ever used with software encoders that do not understand the native handles. BUG=4081 R=emircan@chromium.org, glaznev@webrtc.org, hbos@webrtc.org, magjed@webrtc.org, mflodman@webrtc.org, stefan@webrtc.org, tkchin@webrtc.org Review URL: https://webrtc-codereview.appspot.com/50909005 Cr-Commit-Position: refs/heads/master@{#9347} --- webrtc/common_video/i420_buffer_pool.cc | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'webrtc/common_video/i420_buffer_pool.cc') diff --git a/webrtc/common_video/i420_buffer_pool.cc b/webrtc/common_video/i420_buffer_pool.cc index 35a6c10953..4b8695824c 100644 --- a/webrtc/common_video/i420_buffer_pool.cc +++ b/webrtc/common_video/i420_buffer_pool.cc @@ -40,6 +40,11 @@ class PooledI420Buffer : public webrtc::VideoFrameBuffer { } void* native_handle() const override { return nullptr; } + rtc::scoped_refptr NativeToI420Buffer() override { + RTC_NOTREACHED(); + return nullptr; + } + friend class rtc::RefCountedObject; rtc::scoped_refptr buffer_; }; -- cgit v1.2.3 From 308d163c715df7b4348a1e00bf2a6761c0adb689 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Bostr=C3=B6m?= Date: Tue, 2 Jun 2015 15:04:23 +0200 Subject: Revert "Convert native handles to buffers before encoding." This reverts commit a831dc3a7d10a1fbaa258ee6b1ca6cfc7e91c5ca to unblock rolling into Chromium. BUG=4081 TBR=magjed@webrtc.org Review URL: https://webrtc-codereview.appspot.com/55549004 Cr-Commit-Position: refs/heads/master@{#9354} --- webrtc/common_video/i420_buffer_pool.cc | 5 ----- 1 file changed, 5 deletions(-) (limited to 'webrtc/common_video/i420_buffer_pool.cc') diff --git a/webrtc/common_video/i420_buffer_pool.cc b/webrtc/common_video/i420_buffer_pool.cc index 4b8695824c..35a6c10953 100644 --- a/webrtc/common_video/i420_buffer_pool.cc +++ b/webrtc/common_video/i420_buffer_pool.cc @@ -40,11 +40,6 @@ class PooledI420Buffer : public webrtc::VideoFrameBuffer { } void* native_handle() const override { return nullptr; } - rtc::scoped_refptr NativeToI420Buffer() override { - RTC_NOTREACHED(); - return nullptr; - } - friend class rtc::RefCountedObject; rtc::scoped_refptr buffer_; }; -- cgit v1.2.3 From eb66e800d1f5f74ab366715d2618fbede8cf3e12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20Bostr=C3=B6m?= Date: Fri, 5 Jun 2015 11:08:03 +0200 Subject: Re-land "Convert native handles to buffers before encoding." This reverts commit a67675506c9057bd9ffd4d76aae8b743343d434d. BUG=webrtc:4081 TBR=magjed@webrtc.org Review URL: https://codereview.webrtc.org/1158273010 Cr-Commit-Position: refs/heads/master@{#9381} --- webrtc/common_video/i420_buffer_pool.cc | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'webrtc/common_video/i420_buffer_pool.cc') diff --git a/webrtc/common_video/i420_buffer_pool.cc b/webrtc/common_video/i420_buffer_pool.cc index 35a6c10953..4b8695824c 100644 --- a/webrtc/common_video/i420_buffer_pool.cc +++ b/webrtc/common_video/i420_buffer_pool.cc @@ -40,6 +40,11 @@ class PooledI420Buffer : public webrtc::VideoFrameBuffer { } void* native_handle() const override { return nullptr; } + rtc::scoped_refptr NativeToI420Buffer() override { + RTC_NOTREACHED(); + return nullptr; + } + friend class rtc::RefCountedObject; rtc::scoped_refptr buffer_; }; -- cgit v1.2.3 From 3318f984cd7f51d24da4726665c05f5f06f82e6d Mon Sep 17 00:00:00 2001 From: Magnus Jedvert Date: Wed, 26 Aug 2015 16:06:21 +0200 Subject: VideoFrameBuffer: Make non-const data access explicit VideoFrameBuffer currently has two overloaded data() functions for pixel access, one for const and one for non-const. Unfortunately, it will default to the non-const version, even when 'const scoped_refptr&' is used. This is a problem, because many subclasses use RTC_NOTREACHED() in the non-const version. This CL makes the non-const version of data() explicit with a different, longer function name MutableData(). R=tommi@webrtc.org Review URL: https://codereview.webrtc.org/1304143003 . Cr-Commit-Position: refs/heads/master@{#9787} --- webrtc/common_video/i420_buffer_pool.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'webrtc/common_video/i420_buffer_pool.cc') diff --git a/webrtc/common_video/i420_buffer_pool.cc b/webrtc/common_video/i420_buffer_pool.cc index 4b8695824c..cb1f4d4022 100644 --- a/webrtc/common_video/i420_buffer_pool.cc +++ b/webrtc/common_video/i420_buffer_pool.cc @@ -27,13 +27,13 @@ class PooledI420Buffer : public webrtc::VideoFrameBuffer { 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); + return buffer_->data(type); } - uint8_t* data(webrtc::PlaneType type) { + uint8_t* MutableData(webrtc::PlaneType type) override { + // Make the HasOneRef() check here instead of in |buffer_|, because the pool + // also has a reference to |buffer_|. DCHECK(HasOneRef()); - const webrtc::I420Buffer* cbuffer = buffer_.get(); - return const_cast(cbuffer->data(type)); + return const_cast(buffer_->data(type)); } int stride(webrtc::PlaneType type) const override { return buffer_->stride(type); -- cgit v1.2.3 From 91d6edef35e7275879c30ce16ecb8b6dc73c6e4a Mon Sep 17 00:00:00 2001 From: henrikg Date: Thu, 17 Sep 2015 00:24:34 -0700 Subject: Add RTC_ prefix to (D)CHECKs and related macros. We must remove dependency on Chromium, i.e. we can't use Chromium's base/logging.h. That means we need to define these macros in WebRTC also when doing Chromium builds. And this causes redefinition. Alternative solutions: * Check if we already have defined e.g. CHECK, and don't define them in that case. This makes us depend on include order in Chromium, which is not acceptable. * Don't allow using the macros in WebRTC headers. Error prone since if someone adds it there by mistake it may compile fine, but later break if a header in added or order is changed in Chromium. That will be confusing and hard to enforce. * Ensure that headers that are included by an embedder don't include our macros. This would require some heavy refactoring to be maintainable and enforcable. * Changes in Chromium for this is obviously not an option. BUG=chromium:468375 NOTRY=true Review URL: https://codereview.webrtc.org/1335923002 Cr-Commit-Position: refs/heads/master@{#9964} --- webrtc/common_video/i420_buffer_pool.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'webrtc/common_video/i420_buffer_pool.cc') diff --git a/webrtc/common_video/i420_buffer_pool.cc b/webrtc/common_video/i420_buffer_pool.cc index cb1f4d4022..c746666a16 100644 --- a/webrtc/common_video/i420_buffer_pool.cc +++ b/webrtc/common_video/i420_buffer_pool.cc @@ -32,7 +32,7 @@ class PooledI420Buffer : public webrtc::VideoFrameBuffer { uint8_t* MutableData(webrtc::PlaneType type) override { // Make the HasOneRef() check here instead of in |buffer_|, because the pool // also has a reference to |buffer_|. - DCHECK(HasOneRef()); + RTC_DCHECK(HasOneRef()); return const_cast(buffer_->data(type)); } int stride(webrtc::PlaneType type) const override { @@ -64,7 +64,7 @@ void I420BufferPool::Release() { rtc::scoped_refptr I420BufferPool::CreateBuffer(int width, int height) { - DCHECK(thread_checker_.CalledOnValidThread()); + RTC_DCHECK(thread_checker_.CalledOnValidThread()); // Release buffers with wrong resolution. for (auto it = buffers_.begin(); it != buffers_.end();) { if ((*it)->width() != width || (*it)->height() != height) -- cgit v1.2.3