diff --git a/webrtc/common_video/i420_buffer_pool.cc b/webrtc/common_video/i420_buffer_pool.cc index 8896260fc0..bdc0e821c6 100644 --- a/webrtc/common_video/i420_buffer_pool.cc +++ b/webrtc/common_video/i420_buffer_pool.cc @@ -12,60 +12,11 @@ #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* DataY() const override { return buffer_->DataY(); } - const uint8_t* DataU() const override { return buffer_->DataU(); } - const uint8_t* DataV() const override { return buffer_->DataV(); } - - bool IsMutable() override { return HasOneRef(); } - // Make the IsMutable() check here instead of in |buffer_|, because the pool - // also has a reference to |buffer_|. - uint8_t* MutableDataY() override { - RTC_DCHECK(IsMutable()); - return const_cast(buffer_->DataY()); - } - uint8_t* MutableDataU() override { - RTC_DCHECK(IsMutable()); - return const_cast(buffer_->DataU()); - } - uint8_t* MutableDataV() override { - RTC_DCHECK(IsMutable()); - return const_cast(buffer_->DataV()); - } - int StrideY() const override { return buffer_->StrideY(); } - int StrideU() const override { return buffer_->StrideU(); } - int StrideV() const override { return buffer_->StrideV(); } - void* native_handle() const override { return nullptr; } - - rtc::scoped_refptr NativeToI420Buffer() override { - RTC_NOTREACHED(); - return nullptr; - } - - friend class rtc::RefCountedObject; - rtc::scoped_refptr buffer_; -}; - -} // namespace - namespace webrtc { I420BufferPool::I420BufferPool(bool zero_initialize) : zero_initialize_(zero_initialize) { - Release(); + thread_checker_.DetachFromThread(); } void I420BufferPool::Release() { @@ -73,8 +24,8 @@ void I420BufferPool::Release() { buffers_.clear(); } -rtc::scoped_refptr I420BufferPool::CreateBuffer(int width, - int height) { +rtc::scoped_refptr I420BufferPool::CreateBuffer(int width, + int height) { RTC_DCHECK(thread_checker_.CalledOnValidThread()); // Release buffers with wrong resolution. for (auto it = buffers_.begin(); it != buffers_.end();) { @@ -84,22 +35,21 @@ rtc::scoped_refptr I420BufferPool::CreateBuffer(int width, ++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->IsMutable()) - return new rtc::RefCountedObject(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 the application. If the ref count is 1, + // then the list we are looping over holds the only reference and it's safe + // to reuse. + if (buffer->HasOneRef()) + return buffer; } // Allocate new buffer. - rtc::scoped_refptr buffer = new rtc::RefCountedObject( - width, height); + rtc::scoped_refptr buffer = + new PooledI420Buffer(width, height); if (zero_initialize_) buffer->InitializeData(); buffers_.push_back(buffer); - return new rtc::RefCountedObject(buffers_.back()); + return buffer; } } // namespace webrtc diff --git a/webrtc/common_video/i420_buffer_pool_unittest.cc b/webrtc/common_video/i420_buffer_pool_unittest.cc index 2110066a92..3e795dbdc4 100644 --- a/webrtc/common_video/i420_buffer_pool_unittest.cc +++ b/webrtc/common_video/i420_buffer_pool_unittest.cc @@ -51,20 +51,12 @@ TEST(TestI420BufferPool, FailToReuse) { EXPECT_NE(v_ptr, buffer->DataV()); } -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->IsMutable()); -} - TEST(TestI420BufferPool, FrameValidAfterPoolDestruction) { rtc::scoped_refptr buffer; { I420BufferPool pool; buffer = pool.CreateBuffer(16, 16); } - EXPECT_TRUE(buffer->IsMutable()); EXPECT_EQ(16, buffer->width()); EXPECT_EQ(16, buffer->height()); // Try to trigger use-after-free errors by writing to y-plane. diff --git a/webrtc/common_video/i420_video_frame_unittest.cc b/webrtc/common_video/i420_video_frame_unittest.cc index c942e4a800..5144c2715f 100644 --- a/webrtc/common_video/i420_video_frame_unittest.cc +++ b/webrtc/common_video/i420_video_frame_unittest.cc @@ -200,18 +200,6 @@ TEST(TestVideoFrame, CopyBuffer) { EXPECT_LE(kSizeUv, frame2.allocated_size(kVPlane)); } -TEST(TestVideoFrame, ReuseAllocation) { - VideoFrame frame; - frame.CreateEmptyFrame(640, 320, 640, 320, 320); - const uint8_t* y = frame.video_frame_buffer()->DataY(); - const uint8_t* u = frame.video_frame_buffer()->DataU(); - const uint8_t* v = frame.video_frame_buffer()->DataV(); - frame.CreateEmptyFrame(640, 320, 640, 320, 320); - EXPECT_EQ(y, frame.video_frame_buffer()->DataY()); - EXPECT_EQ(u, frame.video_frame_buffer()->DataU()); - EXPECT_EQ(v, frame.video_frame_buffer()->DataV()); -} - TEST(TestVideoFrame, FailToReuseAllocation) { VideoFrame frame1; frame1.CreateEmptyFrame(640, 320, 640, 320, 320); diff --git a/webrtc/common_video/include/i420_buffer_pool.h b/webrtc/common_video/include/i420_buffer_pool.h index 5c9ab06a09..1465ddf4ec 100644 --- a/webrtc/common_video/include/i420_buffer_pool.h +++ b/webrtc/common_video/include/i420_buffer_pool.h @@ -30,14 +30,18 @@ class 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); + rtc::scoped_refptr CreateBuffer(int width, int height); // Clears buffers_ and detaches the thread checker so that it can be reused // later from another thread. void Release(); private: + // Explicitly use a RefCountedObject to get access to HasOneRef, + // needed by the pool to check exclusive access. + using PooledI420Buffer = rtc::RefCountedObject; + rtc::ThreadChecker thread_checker_; - std::list> buffers_; + std::list> buffers_; // If true, newly allocated buffers are zero-initialized. Note that recycled // buffers are not zero'd before reuse. This is required of buffers used by // FFmpeg according to http://crbug.com/390941, which only requires it for the diff --git a/webrtc/common_video/include/video_frame_buffer.h b/webrtc/common_video/include/video_frame_buffer.h index 6f082dee9b..984bd50d67 100644 --- a/webrtc/common_video/include/video_frame_buffer.h +++ b/webrtc/common_video/include/video_frame_buffer.h @@ -33,18 +33,6 @@ enum PlaneType { // not contain any frame metadata such as rotation, timestamp, pixel_width, etc. class VideoFrameBuffer : public rtc::RefCountInterface { public: - // Returns true if the caller is exclusive owner, and allowed to - // call MutableData. - - // TODO(nisse): Delete default implementation when subclasses in - // Chrome are updated. - virtual bool IsMutable() { return false; } - - // Underlying refcount access, used to implement IsMutable. - // TODO(nisse): Demote to protected, as soon as Chrome is changed to - // use IsMutable. - virtual bool HasOneRef() const = 0; - // The resolution of the frame in pixels. For formats where some planes are // subsampled, this is the highest-resolution plane. virtual int width() const = 0; @@ -67,7 +55,8 @@ class VideoFrameBuffer : public rtc::RefCountInterface { // TODO(nisse): Delete after all users are updated. virtual const uint8_t* data(PlaneType type) const; - // Non-const data access is allowed only if HasOneRef() is true. + // TODO(nisse): Move MutableData methods to the I420Buffer subclass. + // Non-const data access. virtual uint8_t* MutableDataY(); virtual uint8_t* MutableDataU(); virtual uint8_t* MutableDataV(); @@ -105,9 +94,7 @@ class I420Buffer : public VideoFrameBuffer { const uint8_t* DataY() const override; const uint8_t* DataU() const override; const uint8_t* DataV() const override; - // Non-const data access is only allowed if IsMutable() is true, to protect - // against unexpected overwrites. - bool IsMutable() override; + uint8_t* MutableDataY() override; uint8_t* MutableDataU() override; uint8_t* MutableDataV() override; @@ -152,7 +139,6 @@ class NativeHandleBuffer : public VideoFrameBuffer { int StrideV() const override; void* native_handle() const override; - bool IsMutable() override; protected: void* native_handle_; @@ -174,8 +160,6 @@ class WrappedI420Buffer : public webrtc::VideoFrameBuffer { int width() const override; int height() const override; - bool IsMutable() override; - const uint8_t* DataY() const override; const uint8_t* DataU() const override; const uint8_t* DataV() const override; diff --git a/webrtc/common_video/video_frame.cc b/webrtc/common_video/video_frame.cc index 4bd23f98dc..978dde15b7 100644 --- a/webrtc/common_video/video_frame.cc +++ b/webrtc/common_video/video_frame.cc @@ -66,18 +66,7 @@ void VideoFrame::CreateEmptyFrame(int width, render_time_ms_ = 0; rotation_ = kVideoRotation_0; - // Check if it's safe to reuse allocation. - if (video_frame_buffer_ && video_frame_buffer_->IsMutable() && - !video_frame_buffer_->native_handle() && - width == video_frame_buffer_->width() && - height == video_frame_buffer_->height() && - stride_y == video_frame_buffer_->StrideY() && - stride_u == video_frame_buffer_->StrideU() && - stride_v == video_frame_buffer_->StrideV()) { - return; - } - - // Need to allocate new buffer. + // Allocate a new buffer. video_frame_buffer_ = new rtc::RefCountedObject( width, height, stride_y, stride_u, stride_v); } diff --git a/webrtc/common_video/video_frame_buffer.cc b/webrtc/common_video/video_frame_buffer.cc index 700dcaf02b..34213e4656 100644 --- a/webrtc/common_video/video_frame_buffer.cc +++ b/webrtc/common_video/video_frame_buffer.cc @@ -154,20 +154,13 @@ const uint8_t* I420Buffer::DataV() const { return data_.get() + stride_y_ * height_ + stride_u_ * ((height_ + 1) / 2); } -bool I420Buffer::IsMutable() { - return HasOneRef(); -} - uint8_t* I420Buffer::MutableDataY() { - RTC_DCHECK(IsMutable()); return const_cast(DataY()); } uint8_t* I420Buffer::MutableDataU() { - RTC_DCHECK(IsMutable()); return const_cast(DataU()); } uint8_t* I420Buffer::MutableDataV() { - RTC_DCHECK(IsMutable()); return const_cast(DataV()); } @@ -216,10 +209,6 @@ NativeHandleBuffer::NativeHandleBuffer(void* native_handle, RTC_DCHECK_GT(height, 0); } -bool NativeHandleBuffer::IsMutable() { - return false; -} - int NativeHandleBuffer::width() const { return width_; } @@ -282,11 +271,6 @@ WrappedI420Buffer::~WrappedI420Buffer() { no_longer_used_cb_(); } -// Data owned by creator; never mutable. -bool WrappedI420Buffer::IsMutable() { - return false; -} - int WrappedI420Buffer::width() const { return width_; } diff --git a/webrtc/media/base/videoframe.cc b/webrtc/media/base/videoframe.cc index d5c24adf3c..e876d74e31 100644 --- a/webrtc/media/base/videoframe.cc +++ b/webrtc/media/base/videoframe.cc @@ -148,17 +148,6 @@ void VideoFrame::StretchToFrame(VideoFrame* dst, dst->set_rotation(rotation()); } -VideoFrame* VideoFrame::Stretch(size_t dst_width, size_t dst_height, - bool interpolate, bool vert_crop) const { - VideoFrame* dest = CreateEmptyFrame(static_cast(dst_width), - static_cast(dst_height), - GetTimeStamp()); - if (dest) { - StretchToFrame(dest, interpolate, vert_crop); - } - return dest; -} - bool VideoFrame::SetToBlack() { return libyuv::I420Rect(video_frame_buffer()->MutableDataY(), video_frame_buffer()->StrideY(), diff --git a/webrtc/media/base/videoframe.h b/webrtc/media/base/videoframe.h index 950f88848f..a8e3e4d116 100644 --- a/webrtc/media/base/videoframe.h +++ b/webrtc/media/base/videoframe.h @@ -56,10 +56,6 @@ class VideoFrame { // frame buffer. virtual VideoFrame *Copy() const = 0; - // Since VideoFrame supports shallow copy and the internal frame buffer might - // be shared, this function can be used to check exclusive ownership. - virtual bool IsExclusive() const = 0; - // Return a copy of frame which has its pending rotation applied. The // ownership of the returned frame is held by this frame. virtual const VideoFrame* GetCopyWithRotationApplied() const = 0; @@ -95,13 +91,6 @@ class VideoFrame { virtual void StretchToFrame(VideoFrame *target, bool interpolate, bool crop) const; - // Stretches the frame to the given size, creating a new VideoFrame object to - // hold it. The parameter "interpolate" controls whether to interpolate or - // just take the nearest-point. The parameter "crop" controls whether to crop - // this frame to the aspect ratio of the given dimensions before stretching. - virtual VideoFrame *Stretch(size_t w, size_t h, bool interpolate, - bool crop) const; - // Sets the video frame to black. virtual bool SetToBlack(); @@ -128,6 +117,7 @@ class VideoFrame { virtual VideoFrame* CreateEmptyFrame(int w, int h, int64_t timestamp_us) const = 0; + virtual void set_rotation(webrtc::VideoRotation rotation) = 0; }; diff --git a/webrtc/media/base/videoframefactory.cc b/webrtc/media/base/videoframefactory.cc index 23925bda56..0a7dfc22d4 100644 --- a/webrtc/media/base/videoframefactory.cc +++ b/webrtc/media/base/videoframefactory.cc @@ -39,23 +39,13 @@ VideoFrame* VideoFrameFactory::CreateAliasedFrame( std::swap(output_width, output_height); } - // Create and stretch the output frame if it has not been created yet, is - // still in use by others, or its size is not same as the expected. - if (!output_frame_ || !output_frame_->IsExclusive() || - output_frame_->width() != output_width || - output_frame_->height() != output_height) { - output_frame_.reset( - cropped_input_frame->Stretch(output_width, output_height, true, true)); - if (!output_frame_) { - LOG(LS_WARNING) << "Failed to stretch frame to " << output_width << "x" - << output_height; - return NULL; - } - } else { - cropped_input_frame->StretchToFrame(output_frame_.get(), true, true); - output_frame_->SetTimeStamp(cropped_input_frame->GetTimeStamp()); - } - return output_frame_->Copy(); + std::unique_ptr output_frame(new WebRtcVideoFrame( + pool_.CreateBuffer(output_width, output_height), + cropped_input_frame->rotation(), + cropped_input_frame->timestamp_us())); + + cropped_input_frame->StretchToFrame(output_frame.get(), true, true); + return output_frame.release(); } } // namespace cricket diff --git a/webrtc/media/base/videoframefactory.h b/webrtc/media/base/videoframefactory.h index 6aa2ce5173..985199f646 100644 --- a/webrtc/media/base/videoframefactory.h +++ b/webrtc/media/base/videoframefactory.h @@ -13,6 +13,7 @@ #include +#include "webrtc/common_video/include/i420_buffer_pool.h" #include "webrtc/media/base/videoframe.h" namespace cricket { @@ -52,9 +53,9 @@ class VideoFrameFactory { bool apply_rotation_; private: - // An internal frame buffer to avoid reallocations. It is mutable because it + // An internal pool to avoid reallocations. It is mutable because it // does not affect behaviour, only performance. - mutable std::unique_ptr output_frame_; + mutable webrtc::I420BufferPool pool_; }; } // namespace cricket diff --git a/webrtc/media/engine/webrtcvideoframe.cc b/webrtc/media/engine/webrtcvideoframe.cc index 5fa69619de..3ca6db12d0 100644 --- a/webrtc/media/engine/webrtcvideoframe.cc +++ b/webrtc/media/engine/webrtcvideoframe.cc @@ -79,10 +79,6 @@ int WebRtcVideoFrame::height() const { return video_frame_buffer_ ? video_frame_buffer_->height() : 0; } -bool WebRtcVideoFrame::IsExclusive() const { - return video_frame_buffer_->IsMutable(); -} - const rtc::scoped_refptr& WebRtcVideoFrame::video_frame_buffer() const { return video_frame_buffer_; diff --git a/webrtc/media/engine/webrtcvideoframe.h b/webrtc/media/engine/webrtcvideoframe.h index 58ba6e83e0..da8f11b816 100644 --- a/webrtc/media/engine/webrtcvideoframe.h +++ b/webrtc/media/engine/webrtcvideoframe.h @@ -78,7 +78,7 @@ class WebRtcVideoFrame : public VideoFrame { webrtc::VideoRotation rotation() const override { return rotation_; } VideoFrame* Copy() const override; - bool IsExclusive() const override; + size_t ConvertToRgbBuffer(uint32_t to_fourcc, uint8_t* buffer, size_t size,