From 7cf740323033b50845eed0ffe1e9802ae327cd8b Mon Sep 17 00:00:00 2001 From: honghaiz Date: Thu, 23 Jun 2016 16:43:49 -0700 Subject: [PATCH] Revert of Cleanups in cricket::VideoFrame and cricket::WebRtcVideoFrame. (patchset #5 id:110001 of https://codereview.webrtc.org/2075983003/ ) Reason for revert: Breaking Chrome FYI bots. Original issue's description: > Cleanups in cricket::VideoFrame and cricket::WebRtcVideoFrame. > > Removed some protected virtual methods from VideoFrame that no longer > need to exist. Some minor cleanups in the tests. > > BUG=webrtc:5682 > > Committed: https://crrev.com/742d7b10b9720ec43de26e0faef52e5cb9c0daa8 > Cr-Commit-Position: refs/heads/master@{#13275} TBR=pbos@webrtc.org,nisse@webrtc.org,deadbeef@webrtc.org,sergeyu@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=webrtc:5682 Review-Url: https://codereview.webrtc.org/2091983002 Cr-Commit-Position: refs/heads/master@{#13277} --- webrtc/media/base/videoframe.h | 8 +++ webrtc/media/base/videoframe_unittest.h | 14 ++-- webrtc/media/engine/webrtcvideoframe.cc | 47 +++++++++---- webrtc/media/engine/webrtcvideoframe.h | 9 ++- .../media/engine/webrtcvideoframe_unittest.cc | 68 +++++++++++++------ 5 files changed, 100 insertions(+), 46 deletions(-) diff --git a/webrtc/media/base/videoframe.h b/webrtc/media/base/videoframe.h index 3d765ad5df..1f282c5f29 100644 --- a/webrtc/media/base/videoframe.h +++ b/webrtc/media/base/videoframe.h @@ -97,6 +97,14 @@ class VideoFrame { int h, const uint8_t* sample, size_t sample_size); + + protected: + // Creates an empty frame. + virtual VideoFrame* CreateEmptyFrame(int w, + int h, + int64_t timestamp_us) const = 0; + + virtual void set_rotation(webrtc::VideoRotation rotation) = 0; }; } // namespace cricket diff --git a/webrtc/media/base/videoframe_unittest.h b/webrtc/media/base/videoframe_unittest.h index f19d0325d6..39985617c5 100644 --- a/webrtc/media/base/videoframe_unittest.h +++ b/webrtc/media/base/videoframe_unittest.h @@ -835,7 +835,7 @@ class VideoFrameTest : public testing::Test { data_size, 0, webrtc::kVideoRotation_0)); \ int width_rotate = frame1.width(); \ int height_rotate = frame1.height(); \ - frame3.InitToEmptyBuffer(width_rotate, height_rotate); \ + frame3.InitToEmptyBuffer(width_rotate, height_rotate, 0); \ libyuv::I420Mirror(frame2.video_frame_buffer()->DataY(), \ frame2.video_frame_buffer()->StrideY(), \ frame2.video_frame_buffer()->DataU(), \ @@ -847,8 +847,8 @@ class VideoFrameTest : public testing::Test { frame3.video_frame_buffer()->MutableDataU(), \ frame3.video_frame_buffer()->StrideU(), \ frame3.video_frame_buffer()->MutableDataV(), \ - frame3.video_frame_buffer()->StrideV(), kWidth, \ - kHeight); \ + frame3.video_frame_buffer()->StrideV(), \ + kWidth, kHeight); \ EXPECT_TRUE(IsEqual(frame1, frame3, 0)); \ } @@ -873,7 +873,7 @@ class VideoFrameTest : public testing::Test { data_size, 0, webrtc::kVideoRotation_0)); \ int width_rotate = frame1.width(); \ int height_rotate = frame1.height(); \ - frame3.InitToEmptyBuffer(width_rotate, height_rotate); \ + frame3.InitToEmptyBuffer(width_rotate, height_rotate, 0); \ libyuv::I420Rotate(frame2.video_frame_buffer()->DataY(), \ frame2.video_frame_buffer()->StrideY(), \ frame2.video_frame_buffer()->DataU(), \ @@ -885,8 +885,8 @@ class VideoFrameTest : public testing::Test { frame3.video_frame_buffer()->MutableDataU(), \ frame3.video_frame_buffer()->StrideU(), \ frame3.video_frame_buffer()->MutableDataV(), \ - frame3.video_frame_buffer()->StrideV(), kWidth, \ - kHeight, libyuv::kRotate##ROTATE); \ + frame3.video_frame_buffer()->StrideV(), \ + kWidth, kHeight, libyuv::kRotate##ROTATE); \ EXPECT_TRUE(IsEqual(frame1, frame3, 0)); \ } @@ -1480,7 +1480,7 @@ class VideoFrameTest : public testing::Test { out, out_size, stride)); } - frame2.InitToEmptyBuffer(kWidth, kHeight); + frame2.InitToEmptyBuffer(kWidth, kHeight, 0); for (int i = 0; i < repeat_from; ++i) { EXPECT_EQ(0, RGBToI420(out, stride, frame2.video_frame_buffer()->MutableDataY(), diff --git a/webrtc/media/engine/webrtcvideoframe.cc b/webrtc/media/engine/webrtcvideoframe.cc index 2ff4042c52..4f89c8b85d 100644 --- a/webrtc/media/engine/webrtcvideoframe.cc +++ b/webrtc/media/engine/webrtcvideoframe.cc @@ -149,11 +149,26 @@ bool WebRtcVideoFrame::Reset(uint32_t format, return true; } +VideoFrame* WebRtcVideoFrame::CreateEmptyFrame(int w, + int h, + int64_t timestamp_us) const { + WebRtcVideoFrame* frame = new WebRtcVideoFrame(); + frame->InitToEmptyBuffer(w, h, rtc::kNumNanosecsPerMicrosec * timestamp_us); + return frame; +} + void WebRtcVideoFrame::InitToEmptyBuffer(int w, int h) { video_frame_buffer_ = new rtc::RefCountedObject(w, h); rotation_ = webrtc::kVideoRotation_0; } +void WebRtcVideoFrame::InitToEmptyBuffer(int w, int h, + int64_t time_stamp_ns) { + video_frame_buffer_ = new rtc::RefCountedObject(w, h); + SetTimeStamp(time_stamp_ns); + rotation_ = webrtc::kVideoRotation_0; +} + const VideoFrame* WebRtcVideoFrame::GetCopyWithRotationApplied() const { // If the frame is not rotated, the caller should reuse this frame instead of // making a redundant copy. @@ -170,19 +185,19 @@ const VideoFrame* WebRtcVideoFrame::GetCopyWithRotationApplied() const { return rotated_frame_.get(); } - int current_width = width(); - int current_height = height(); + int orig_width = width(); + int orig_height = height(); - int rotated_width = current_width; - int rotated_height = current_height; + int rotated_width = orig_width; + int rotated_height = orig_height; if (rotation() == webrtc::kVideoRotation_90 || rotation() == webrtc::kVideoRotation_270) { - std::swap(rotated_width, rotated_height); + rotated_width = orig_height; + rotated_height = orig_width; } - rtc::scoped_refptr buffer = - new rtc::RefCountedObject(rotated_width, - rotated_height); + rotated_frame_.reset( + CreateEmptyFrame(rotated_width, rotated_height, timestamp_us_)); // TODO(guoweis): Add a function in webrtc_libyuv.cc to convert from // VideoRotation to libyuv::RotationMode. @@ -190,16 +205,18 @@ const VideoFrame* WebRtcVideoFrame::GetCopyWithRotationApplied() const { video_frame_buffer_->DataY(), video_frame_buffer_->StrideY(), video_frame_buffer_->DataU(), video_frame_buffer_->StrideU(), video_frame_buffer_->DataV(), video_frame_buffer_->StrideV(), - buffer->MutableDataY(), buffer->StrideY(), buffer->MutableDataU(), - buffer->StrideU(), buffer->MutableDataV(), buffer->StrideV(), - current_width, current_height, + rotated_frame_->video_frame_buffer()->MutableDataY(), + rotated_frame_->video_frame_buffer()->StrideY(), + rotated_frame_->video_frame_buffer()->MutableDataU(), + rotated_frame_->video_frame_buffer()->StrideU(), + rotated_frame_->video_frame_buffer()->MutableDataV(), + rotated_frame_->video_frame_buffer()->StrideV(), + orig_width, orig_height, static_cast(rotation())); if (ret == 0) { - rotated_frame_.reset( - new WebRtcVideoFrame(buffer, webrtc::kVideoRotation_0, timestamp_us_)); + return rotated_frame_.get(); } - - return rotated_frame_.get(); + return nullptr; } } // namespace cricket diff --git a/webrtc/media/engine/webrtcvideoframe.h b/webrtc/media/engine/webrtcvideoframe.h index 07525e7f3e..29bdb32f2b 100644 --- a/webrtc/media/engine/webrtcvideoframe.h +++ b/webrtc/media/engine/webrtcvideoframe.h @@ -19,7 +19,6 @@ #include "webrtc/common_types.h" #include "webrtc/common_video/include/video_frame_buffer.h" #include "webrtc/media/base/videoframe.h" -#include "webrtc/base/gtest_prod_util.h" namespace cricket { @@ -73,6 +72,7 @@ class WebRtcVideoFrame : public VideoFrame { bool Init(const CapturedFrame* frame, int dw, int dh, bool apply_rotation); void InitToEmptyBuffer(int w, int h); + void InitToEmptyBuffer(int w, int h, int64_t time_stamp_ns); int width() const override; int height() const override; @@ -96,6 +96,9 @@ class WebRtcVideoFrame : public VideoFrame { const VideoFrame* GetCopyWithRotationApplied() const override; protected: + void set_rotation(webrtc::VideoRotation rotation) override { + rotation_ = rotation; + } // Creates a frame from a raw sample with FourCC |format| and size |w| x |h|. // |h| can be negative indicating a vertically flipped image. // |dw| is destination width; can be less than |w| if cropping is desired. @@ -113,8 +116,8 @@ class WebRtcVideoFrame : public VideoFrame { bool apply_rotation); private: - // The test mutates |rotation_|, so it needs to be a friend. - FRIEND_TEST_ALL_PREFIXES(WebRtcVideoFrameTest, ApplyRotationToFrame); + VideoFrame* CreateEmptyFrame(int w, int h, + int64_t time_stamp_ns) const override; // An opaque reference counted handle that stores the pixel data. rtc::scoped_refptr video_frame_buffer_; diff --git a/webrtc/media/engine/webrtcvideoframe_unittest.cc b/webrtc/media/engine/webrtcvideoframe_unittest.cc index fbb7ebd625..abfd0aea3c 100644 --- a/webrtc/media/engine/webrtcvideoframe_unittest.cc +++ b/webrtc/media/engine/webrtcvideoframe_unittest.cc @@ -16,11 +16,37 @@ #include "webrtc/media/engine/webrtcvideoframe.h" #include "webrtc/test/fake_texture_frame.h" -namespace cricket { +namespace { -class WebRtcVideoFrameTest : public VideoFrameTest { +class WebRtcVideoTestFrame : public cricket::WebRtcVideoFrame { public: - WebRtcVideoFrameTest() {} + WebRtcVideoTestFrame() {} + WebRtcVideoTestFrame( + const rtc::scoped_refptr& buffer, + int64_t time_stamp_ns, + webrtc::VideoRotation rotation) + : WebRtcVideoFrame(buffer, time_stamp_ns, rotation) {} + + // The ApplyRotationToFrame test needs this as a public method. + using cricket::WebRtcVideoFrame::set_rotation; + + virtual VideoFrame* CreateEmptyFrame(int w, + int h, + int64_t time_stamp) const override { + rtc::scoped_refptr buffer( + new rtc::RefCountedObject(w, h)); + buffer->SetToBlack(); + return new WebRtcVideoTestFrame( + buffer, time_stamp, webrtc::kVideoRotation_0); + } +}; + +} // namespace + +class WebRtcVideoFrameTest : public VideoFrameTest { + public: + WebRtcVideoFrameTest() { + } void TestInit(int cropped_width, int cropped_height, webrtc::VideoRotation frame_rotation, @@ -29,8 +55,8 @@ class WebRtcVideoFrameTest : public VideoFrameTest { const int frame_height = 1080; // Build the CapturedFrame. - CapturedFrame captured_frame; - captured_frame.fourcc = FOURCC_I420; + cricket::CapturedFrame captured_frame; + captured_frame.fourcc = cricket::FOURCC_I420; captured_frame.time_stamp = rtc::TimeNanos(); captured_frame.rotation = frame_rotation; captured_frame.width = frame_width; @@ -44,7 +70,7 @@ class WebRtcVideoFrameTest : public VideoFrameTest { captured_frame.data = captured_frame_buffer.get(); // Create the new frame from the CapturedFrame. - WebRtcVideoFrame frame; + cricket::WebRtcVideoFrame frame; EXPECT_TRUE( frame.Init(&captured_frame, cropped_width, cropped_height, apply_rotation)); @@ -69,8 +95,9 @@ class WebRtcVideoFrameTest : public VideoFrameTest { } }; -#define TEST_WEBRTCVIDEOFRAME(X) \ - TEST_F(WebRtcVideoFrameTest, X) { VideoFrameTest::X(); } +#define TEST_WEBRTCVIDEOFRAME(X) TEST_F(WebRtcVideoFrameTest, X) { \ + VideoFrameTest::X(); \ +} TEST_WEBRTCVIDEOFRAME(ConstructI420) TEST_WEBRTCVIDEOFRAME(ConstructI422) @@ -254,7 +281,7 @@ TEST_F(WebRtcVideoFrameTest, TextureInitialValues) { new rtc::RefCountedObject( dummy_handle, 640, 480); // Timestamp is converted from ns to us, so last three digits are lost. - WebRtcVideoFrame frame(buffer, 20000, webrtc::kVideoRotation_0); + cricket::WebRtcVideoFrame frame(buffer, 20000, webrtc::kVideoRotation_0); EXPECT_EQ(dummy_handle, frame.video_frame_buffer()->native_handle()); EXPECT_EQ(640, frame.width()); EXPECT_EQ(480, frame.height()); @@ -272,8 +299,8 @@ TEST_F(WebRtcVideoFrameTest, CopyTextureFrame) { new rtc::RefCountedObject( dummy_handle, 640, 480); // Timestamp is converted from ns to us, so last three digits are lost. - WebRtcVideoFrame frame1(buffer, 20000, webrtc::kVideoRotation_0); - VideoFrame* frame2 = frame1.Copy(); + cricket::WebRtcVideoFrame frame1(buffer, 20000, webrtc::kVideoRotation_0); + cricket::VideoFrame* frame2 = frame1.Copy(); EXPECT_EQ(frame1.video_frame_buffer()->native_handle(), frame2->video_frame_buffer()->native_handle()); EXPECT_EQ(frame1.width(), frame2->width()); @@ -284,17 +311,17 @@ TEST_F(WebRtcVideoFrameTest, CopyTextureFrame) { } TEST_F(WebRtcVideoFrameTest, ApplyRotationToFrame) { - WebRtcVideoFrame applied0; + WebRtcVideoTestFrame applied0; EXPECT_TRUE(IsNull(applied0)); - EXPECT_TRUE(LoadFrame(CreateYuvSample(kWidth, kHeight, 12).get(), FOURCC_I420, - kWidth, kHeight, &applied0)); + EXPECT_TRUE(LoadFrame(CreateYuvSample(kWidth, kHeight, 12).get(), + cricket::FOURCC_I420, kWidth, kHeight, &applied0)); // Claim that this frame needs to be rotated for 90 degree. - applied0.rotation_ = webrtc::kVideoRotation_90; + applied0.set_rotation(webrtc::kVideoRotation_90); // Apply rotation on frame 1. Output should be different from frame 1. - WebRtcVideoFrame* applied90 = - const_cast(static_cast( + WebRtcVideoTestFrame* applied90 = const_cast( + static_cast( applied0.GetCopyWithRotationApplied())); EXPECT_TRUE(applied90); EXPECT_EQ(applied90->rotation(), webrtc::kVideoRotation_0); @@ -302,11 +329,10 @@ TEST_F(WebRtcVideoFrameTest, ApplyRotationToFrame) { // Claim the frame 2 needs to be rotated for another 270 degree. The output // from frame 2 rotation should be the same as frame 1. - applied90->rotation_ = webrtc::kVideoRotation_270; - const VideoFrame* applied360 = applied90->GetCopyWithRotationApplied(); + applied90->set_rotation(webrtc::kVideoRotation_270); + const cricket::VideoFrame* applied360 = + applied90->GetCopyWithRotationApplied(); EXPECT_TRUE(applied360); EXPECT_EQ(applied360->rotation(), webrtc::kVideoRotation_0); EXPECT_TRUE(IsEqual(applied0, *applied360, 0)); } - -} // namespace cricket