From e3fe4a7c2d4abd2dfde575b8e42c635f98703b9e Mon Sep 17 00:00:00 2001 From: nisse Date: Thu, 10 Nov 2016 08:44:38 -0800 Subject: [PATCH] Update VideoFrameBuffer-related methods to not use references to scoped_refptr. Chrome coding standard now discourages use of references to smart pointers. This cl updates some recent methods to the new conventions. BUG=webrtc:6672 Review-Url: https://codereview.webrtc.org/2477233004 Cr-Commit-Position: refs/heads/master@{#15028} --- .../common_video/i420_video_frame_unittest.cc | 29 ++++++----- .../common_video/include/video_frame_buffer.h | 31 +++++++++--- webrtc/common_video/video_frame.cc | 3 +- webrtc/common_video/video_frame_buffer.cc | 49 ++++++++++--------- .../codecs/test/videoprocessor.cc | 2 +- .../codecs/vp8/test/vp8_impl_unittest.cc | 2 +- .../video_coding/utility/quality_scaler.cc | 2 +- .../video_processing/spatial_resampler.cc | 2 +- webrtc/video_frame.h | 5 +- 9 files changed, 72 insertions(+), 53 deletions(-) diff --git a/webrtc/common_video/i420_video_frame_unittest.cc b/webrtc/common_video/i420_video_frame_unittest.cc index 420ae4e152..2b000a709c 100644 --- a/webrtc/common_video/i420_video_frame_unittest.cc +++ b/webrtc/common_video/i420_video_frame_unittest.cc @@ -151,13 +151,16 @@ TEST(TestVideoFrame, ShallowCopy) { memset(buffer_y, 16, kSizeY); memset(buffer_u, 8, kSizeU); memset(buffer_v, 4, kSizeV); + // TODO(nisse): This new + Copy looks quite awkward. Consider adding + // an alternative I420Buffer::Create method. VideoFrame frame1( - I420Buffer::Copy(new rtc::RefCountedObject( - width, height, - buffer_y, stride_y, - buffer_u, stride_u, - buffer_v, stride_v, - rtc::Callback0([](){}))), + I420Buffer::Copy(*rtc::scoped_refptr( + new rtc::RefCountedObject( + width, height, + buffer_y, stride_y, + buffer_u, stride_u, + buffer_v, stride_v, + rtc::Callback0([](){})))), kRotation, 0); frame1.set_timestamp(timestamp); frame1.set_ntp_time_ms(ntp_time_ms); @@ -211,7 +214,7 @@ TEST(TestI420FrameBuffer, Copy) { memset(buf1->MutableDataY(), 1, 200); memset(buf1->MutableDataU(), 2, 50); memset(buf1->MutableDataV(), 3, 50); - rtc::scoped_refptr buf2 = I420Buffer::Copy(buf1); + rtc::scoped_refptr buf2 = I420Buffer::Copy(*buf1); EXPECT_TRUE(test::FrameBufsEqual(buf1, buf2)); } @@ -222,7 +225,7 @@ TEST(TestI420FrameBuffer, Scale) { rtc::scoped_refptr scaled_buffer( I420Buffer::Create(150, 75)); - scaled_buffer->ScaleFrom(buf); + scaled_buffer->ScaleFrom(*buf); CheckCrop(*scaled_buffer, 0.0, 0.0, 1.0, 1.0); } @@ -233,7 +236,7 @@ TEST(TestI420FrameBuffer, CropXCenter) { rtc::scoped_refptr scaled_buffer( I420Buffer::Create(100, 100)); - scaled_buffer->CropAndScaleFrom(buf, 50, 0, 100, 100); + scaled_buffer->CropAndScaleFrom(*buf, 50, 0, 100, 100); CheckCrop(*scaled_buffer, 0.25, 0.0, 0.5, 1.0); } @@ -244,7 +247,7 @@ TEST(TestI420FrameBuffer, CropXNotCenter) { rtc::scoped_refptr scaled_buffer( I420Buffer::Create(100, 100)); - scaled_buffer->CropAndScaleFrom(buf, 25, 0, 100, 100); + scaled_buffer->CropAndScaleFrom(*buf, 25, 0, 100, 100); CheckCrop(*scaled_buffer, 0.125, 0.0, 0.5, 1.0); } @@ -255,7 +258,7 @@ TEST(TestI420FrameBuffer, CropYCenter) { rtc::scoped_refptr scaled_buffer( I420Buffer::Create(100, 100)); - scaled_buffer->CropAndScaleFrom(buf, 0, 50, 100, 100); + scaled_buffer->CropAndScaleFrom(*buf, 0, 50, 100, 100); CheckCrop(*scaled_buffer, 0.0, 0.25, 1.0, 0.5); } @@ -266,7 +269,7 @@ TEST(TestI420FrameBuffer, CropYNotCenter) { rtc::scoped_refptr scaled_buffer( I420Buffer::Create(100, 100)); - scaled_buffer->CropAndScaleFrom(buf, 0, 25, 100, 100); + scaled_buffer->CropAndScaleFrom(*buf, 0, 25, 100, 100); CheckCrop(*scaled_buffer, 0.0, 0.125, 1.0, 0.5); } @@ -277,7 +280,7 @@ TEST(TestI420FrameBuffer, CropAndScale16x9) { rtc::scoped_refptr scaled_buffer( I420Buffer::Create(320, 180)); - scaled_buffer->CropAndScaleFrom(buf); + scaled_buffer->CropAndScaleFrom(*buf); CheckCrop(*scaled_buffer, 0.0, 0.125, 1.0, 0.75); } diff --git a/webrtc/common_video/include/video_frame_buffer.h b/webrtc/common_video/include/video_frame_buffer.h index 0b5d863f28..287624e59e 100644 --- a/webrtc/common_video/include/video_frame_buffer.h +++ b/webrtc/common_video/include/video_frame_buffer.h @@ -96,12 +96,11 @@ class I420Buffer : public VideoFrameBuffer { rtc::scoped_refptr NativeToI420Buffer() override; // Create a new buffer and copy the pixel data. - static rtc::scoped_refptr Copy( - const rtc::scoped_refptr& buffer); + static rtc::scoped_refptr Copy(const VideoFrameBuffer& buffer); // Scale the cropped area of |src| to the size of |this| buffer, and // write the result into |this|. - void CropAndScaleFrom(const rtc::scoped_refptr& src, + void CropAndScaleFrom(const VideoFrameBuffer& src, int offset_x, int offset_y, int crop_width, @@ -109,16 +108,36 @@ class I420Buffer : public VideoFrameBuffer { // The common case of a center crop, when needed to adjust the // aspect ratio without distorting the image. - void CropAndScaleFrom(const rtc::scoped_refptr& src); + void CropAndScaleFrom(const VideoFrameBuffer& src); // Scale all of |src| to the size of |this| buffer, with no cropping. - void ScaleFrom(const rtc::scoped_refptr& src); + void ScaleFrom(const VideoFrameBuffer& src); + + // Deprecated methods, using smart pointer references. + // TODO(nisse): Delete once downstream applications are updated. + static rtc::scoped_refptr Copy( + const rtc::scoped_refptr& buffer) { + return Copy(*buffer); + } + void CropAndScaleFrom(const rtc::scoped_refptr& src, + int offset_x, + int offset_y, + int crop_width, + int crop_height) { + CropAndScaleFrom(*src, offset_x, offset_y, crop_width, crop_height); + } + void CropAndScaleFrom(const rtc::scoped_refptr& src) { + CropAndScaleFrom(*src); + } + void ScaleFrom(const rtc::scoped_refptr& src) { + ScaleFrom(*src); + } // Returns a rotated versions of |src|. Native buffers are not // supported. The reason this function doesn't return an I420Buffer, // is that it returns |src| unchanged in case |rotation| is zero. static rtc::scoped_refptr Rotate( - const rtc::scoped_refptr& src, + rtc::scoped_refptr src, VideoRotation rotation); protected: diff --git a/webrtc/common_video/video_frame.cc b/webrtc/common_video/video_frame.cc index 4c35552d33..ab1a6bef40 100644 --- a/webrtc/common_video/video_frame.cc +++ b/webrtc/common_video/video_frame.cc @@ -63,8 +63,7 @@ bool VideoFrame::IsZeroSize() const { return !video_frame_buffer_; } -const rtc::scoped_refptr& VideoFrame::video_frame_buffer() - const { +rtc::scoped_refptr VideoFrame::video_frame_buffer() const { return video_frame_buffer_; } diff --git a/webrtc/common_video/video_frame_buffer.cc b/webrtc/common_video/video_frame_buffer.cc index 869eb4f87c..1ee97531b0 100644 --- a/webrtc/common_video/video_frame_buffer.cc +++ b/webrtc/common_video/video_frame_buffer.cc @@ -126,14 +126,15 @@ rtc::scoped_refptr I420Buffer::NativeToI420Buffer() { return nullptr; } +// static rtc::scoped_refptr I420Buffer::Copy( - const rtc::scoped_refptr& source) { - int width = source->width(); - int height = source->height(); + const VideoFrameBuffer& source) { + int width = source.width(); + int height = source.height(); rtc::scoped_refptr target = I420Buffer::Create(width, height); - RTC_CHECK(libyuv::I420Copy(source->DataY(), source->StrideY(), - source->DataU(), source->StrideU(), - source->DataV(), source->StrideV(), + RTC_CHECK(libyuv::I420Copy(source.DataY(), source.StrideY(), + source.DataU(), source.StrideU(), + source.DataV(), source.StrideV(), target->MutableDataY(), target->StrideY(), target->MutableDataU(), target->StrideU(), target->MutableDataV(), target->StrideV(), @@ -151,15 +152,15 @@ void I420Buffer::SetToBlack() { } void I420Buffer::CropAndScaleFrom( - const rtc::scoped_refptr& src, + const VideoFrameBuffer& src, int offset_x, int offset_y, int crop_width, int crop_height) { - RTC_CHECK_LE(crop_width, src->width()); - RTC_CHECK_LE(crop_height, src->height()); - RTC_CHECK_LE(crop_width + offset_x, src->width()); - RTC_CHECK_LE(crop_height + offset_y, src->height()); + RTC_CHECK_LE(crop_width, src.width()); + RTC_CHECK_LE(crop_height, src.height()); + RTC_CHECK_LE(crop_width + offset_x, src.width()); + RTC_CHECK_LE(crop_height + offset_y, src.height()); RTC_CHECK_GE(offset_x, 0); RTC_CHECK_GE(offset_y, 0); @@ -170,14 +171,14 @@ void I420Buffer::CropAndScaleFrom( offset_y = uv_offset_y * 2; const uint8_t* y_plane = - src->DataY() + src->StrideY() * offset_y + offset_x; + src.DataY() + src.StrideY() * offset_y + offset_x; const uint8_t* u_plane = - src->DataU() + src->StrideU() * uv_offset_y + uv_offset_x; + src.DataU() + src.StrideU() * uv_offset_y + uv_offset_x; const uint8_t* v_plane = - src->DataV() + src->StrideV() * uv_offset_y + uv_offset_x; - int res = libyuv::I420Scale(y_plane, src->StrideY(), - u_plane, src->StrideU(), - v_plane, src->StrideV(), + src.DataV() + src.StrideV() * uv_offset_y + uv_offset_x; + int res = libyuv::I420Scale(y_plane, src.StrideY(), + u_plane, src.StrideU(), + v_plane, src.StrideV(), crop_width, crop_height, MutableDataY(), StrideY(), MutableDataU(), StrideU(), @@ -188,25 +189,25 @@ void I420Buffer::CropAndScaleFrom( } void I420Buffer::CropAndScaleFrom( - const rtc::scoped_refptr& src) { + const VideoFrameBuffer& src) { const int crop_width = - std::min(src->width(), width() * src->height() / height()); + std::min(src.width(), width() * src.height() / height()); const int crop_height = - std::min(src->height(), height() * src->width() / width()); + std::min(src.height(), height() * src.width() / width()); CropAndScaleFrom( src, - (src->width() - crop_width) / 2, (src->height() - crop_height) / 2, + (src.width() - crop_width) / 2, (src.height() - crop_height) / 2, crop_width, crop_height); } -void I420Buffer::ScaleFrom(const rtc::scoped_refptr& src) { - CropAndScaleFrom(src, 0, 0, src->width(), src->height()); +void I420Buffer::ScaleFrom(const VideoFrameBuffer& src) { + CropAndScaleFrom(src, 0, 0, src.width(), src.height()); } // static rtc::scoped_refptr I420Buffer::Rotate( - const rtc::scoped_refptr& src, + rtc::scoped_refptr src, VideoRotation rotation) { RTC_DCHECK(src->DataY()); RTC_DCHECK(src->DataU()); diff --git a/webrtc/modules/video_coding/codecs/test/videoprocessor.cc b/webrtc/modules/video_coding/codecs/test/videoprocessor.cc index 8436eb0042..3f1a4371d8 100644 --- a/webrtc/modules/video_coding/codecs/test/videoprocessor.cc +++ b/webrtc/modules/video_coding/codecs/test/videoprocessor.cc @@ -334,7 +334,7 @@ void VideoProcessorImpl::FrameDecoded(const VideoFrame& image) { config_.codec_settings->height)); // Should be the same aspect ratio, no cropping needed. - up_image->ScaleFrom(image.video_frame_buffer()); + up_image->ScaleFrom(*image.video_frame_buffer()); // TODO(mikhal): Extracting the buffer for now - need to update test. size_t length = diff --git a/webrtc/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc b/webrtc/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc index d636b14f50..2dafbf96b4 100644 --- a/webrtc/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc +++ b/webrtc/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc @@ -149,7 +149,7 @@ class TestVp8Impl : public ::testing::Test { I420Buffer::Create(kWidth, kHeight, stride_y, stride_uv, stride_uv)); // No scaling in our case, just a copy, to add stride to the image. - stride_buffer->ScaleFrom(compact_buffer); + stride_buffer->ScaleFrom(*compact_buffer); input_frame_.reset( new VideoFrame(stride_buffer, kVideoRotation_0, 0)); diff --git a/webrtc/modules/video_coding/utility/quality_scaler.cc b/webrtc/modules/video_coding/utility/quality_scaler.cc index 59bc605966..0f63edca7d 100644 --- a/webrtc/modules/video_coding/utility/quality_scaler.cc +++ b/webrtc/modules/video_coding/utility/quality_scaler.cc @@ -187,7 +187,7 @@ rtc::scoped_refptr QualityScaler::GetScaledBuffer( rtc::scoped_refptr scaled_buffer = pool_.CreateBuffer(res.width, res.height); - scaled_buffer->ScaleFrom(frame); + scaled_buffer->ScaleFrom(*frame); return scaled_buffer; } diff --git a/webrtc/modules/video_processing/spatial_resampler.cc b/webrtc/modules/video_processing/spatial_resampler.cc index 7c4aae2088..4d98605b2c 100644 --- a/webrtc/modules/video_processing/spatial_resampler.cc +++ b/webrtc/modules/video_processing/spatial_resampler.cc @@ -56,7 +56,7 @@ int32_t VPMSimpleSpatialResampler::ResampleFrame(const VideoFrame& inFrame, rtc::scoped_refptr scaled_buffer( buffer_pool_.CreateBuffer(target_width_, target_height_)); - scaled_buffer->CropAndScaleFrom(inFrame.video_frame_buffer()); + scaled_buffer->CropAndScaleFrom(*inFrame.video_frame_buffer()); *outFrame = VideoFrame(scaled_buffer, inFrame.timestamp(), diff --git a/webrtc/video_frame.h b/webrtc/video_frame.h index 0569278897..bd6a85791f 100644 --- a/webrtc/video_frame.h +++ b/webrtc/video_frame.h @@ -120,10 +120,7 @@ class VideoFrame { // Return the underlying buffer. Never nullptr for a properly // initialized VideoFrame. - // Creating a new reference breaks the HasOneRef and IsMutable - // logic. So return a const ref to our reference. - const rtc::scoped_refptr& video_frame_buffer() - const; + rtc::scoped_refptr video_frame_buffer() const; // Return true if the frame is stored in a texture. bool is_texture() const {