Revert of Delete method cricket::VideoFrame::Copy. (patchset #3 id:210001 of https://codereview.webrtc.org/2275243002/ ) Reason for revert: Again, brakes a downstream build by removing VideoFrame::Copy method. Original issue's description: > Reland of Delete method cricket::VideoFrame::Copy. (patchset #1 id:1 of https://codereview.webrtc.org/2087923004/ ) > > Reason for revert: > Downstream issue now fixed. > > Original issue's description: > > Revert of Delete method cricket::VideoFrame::Copy. (patchset #7 id:120001 of https://codereview.webrtc.org/2080253002/ ) > > > > Reason for revert: > > It broke a downstream build by removing VideoFrame::Copy method. > > > > Original issue's description: > > > Delete method cricket::VideoFrame::Copy. > > > > > > Should be unused in Chrome since cl > > > https://codereview.chromium.org/2068703002/ > > > > > > TBR=tkchin@webrtc.org,magjed@webrtc.org > > > BUG=webrtc:5682 > > > > > > Committed: https://crrev.com/9c00f646f0b3cd33506a1944c7bc6724af041237 > > > Committed: https://crrev.com/7e4e00d189a5dfac2b463a5100ee65ee2f11ed79 > > > Cr-Original-Commit-Position: refs/heads/master@{#13236} > > > Cr-Commit-Position: refs/heads/master@{#13244} > > > > TBR=pbos@webrtc.org,tkchin@webrtc.org,magjed@webrtc.org,sergeyu@chromium.org,nisse@webrtc.org > > # Skipping CQ checks because original CL landed less than 1 days ago. > > NOPRESUBMIT=true > > NOTREECHECKS=true > > NOTRY=true > > BUG=webrtc:5682 > > > > Committed: https://crrev.com/123f33cd009606d22cca8b0f4756812406d4580f > > Cr-Commit-Position: refs/heads/master@{#13246} > > TBR=pbos@webrtc.org,tkchin@webrtc.org,magjed@webrtc.org,sergeyu@chromium.org,honghaiz@webrtc.org > # Not skipping CQ checks because original CL landed more than 1 days ago. > BUG=webrtc:5682 > > Committed: https://crrev.com/f715f983f1b33208ab2d2434f8b36ad5271f680f > Cr-Commit-Position: refs/heads/master@{#13924} TBR=pbos@webrtc.org,tkchin@webrtc.org,magjed@webrtc.org,sergeyu@chromium.org,honghaiz@webrtc.org,nisse@webrtc.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/2275313003 Cr-Commit-Position: refs/heads/master@{#13925}
diff --git a/talk/app/webrtc/objc/RTCI420Frame.mm b/talk/app/webrtc/objc/RTCI420Frame.mm index 1646510..6c6c564 100644 --- a/talk/app/webrtc/objc/RTCI420Frame.mm +++ b/talk/app/webrtc/objc/RTCI420Frame.mm
@@ -29,7 +29,7 @@ #include <memory> -#include "webrtc/media/engine/webrtcvideoframe.h" +#include "webrtc/media/base/videoframe.h" @implementation RTCI420Frame { std::unique_ptr<cricket::VideoFrame> _videoFrame; @@ -98,10 +98,7 @@ if (self = [super init]) { // Keep a shallow copy of the video frame. The underlying frame buffer is // not copied. - _videoFrame.reset(new cricket::WebRtcVideoFrame( - videoFrame->video_frame_buffer(), - videoFrame->rotation(), - videoFrame->timestamp_us())); + _videoFrame.reset(videoFrame->Copy()); } return self; }
diff --git a/webrtc/api/android/jni/peerconnection_jni.cc b/webrtc/api/android/jni/peerconnection_jni.cc index 5095b15..9efe037 100644 --- a/webrtc/api/android/jni/peerconnection_jni.cc +++ b/webrtc/api/android/jni/peerconnection_jni.cc
@@ -770,8 +770,7 @@ // ownership of the frame, and the frame should be released with // VideoRenderer.releaseNativeFrame(). static jlong javaShallowCopy(const cricket::VideoFrame* frame) { - return jlongFromPointer(new cricket::WebRtcVideoFrame( - frame->video_frame_buffer(), frame->rotation(), frame->timestamp_us())); + return jlongFromPointer(frame->Copy()); } // Return a VideoRenderer.I420Frame referring to the data in |frame|.
diff --git a/webrtc/media/base/videoframe.h b/webrtc/media/base/videoframe.h index 3dcbb4e..a139349 100644 --- a/webrtc/media/base/videoframe.h +++ b/webrtc/media/base/videoframe.h
@@ -58,6 +58,16 @@ // Indicates the rotation angle in degrees. virtual webrtc::VideoRotation rotation() const = 0; + // Make a shallow copy of the frame. The frame buffer itself is not copied. + // Both the current and new VideoFrame will share a single reference-counted + // frame buffer. + + // TODO(nisse): Deprecated, to be deleted in the cricket::VideoFrame and + // webrtc::VideoFrame merge. To make a copy, use the cricket::WebRtcVideoFrame + // constructor passing video_frame_buffer(), rotation() and timestamp_us() as + // arguments. + virtual VideoFrame *Copy() const = 0; + // Return a copy of frame which has its pending rotation applied. The // ownership of the returned frame is held by this frame.
diff --git a/webrtc/media/base/videoframe_unittest.h b/webrtc/media/base/videoframe_unittest.h index 13fb790..f19d032 100644 --- a/webrtc/media/base/videoframe_unittest.h +++ b/webrtc/media/base/videoframe_unittest.h
@@ -1808,6 +1808,31 @@ EXPECT_TRUE(IsEqual(frame1, frame2, 1)); } + /////////////////// + // General tests // + /////////////////// + + void Copy() { + std::unique_ptr<T> source(new T); + std::unique_ptr<cricket::VideoFrame> target; + ASSERT_TRUE(LoadFrameNoRepeat(source.get())); + target.reset(source->Copy()); + EXPECT_TRUE(IsEqual(*source, *target, 0)); + source.reset(); + ASSERT_TRUE(target->video_frame_buffer() != NULL); + EXPECT_TRUE(target->video_frame_buffer()->DataY() != NULL); + } + + void CopyIsRef() { + std::unique_ptr<T> source(new T); + std::unique_ptr<const cricket::VideoFrame> target; + ASSERT_TRUE(LoadFrameNoRepeat(source.get())); + target.reset(source->Copy()); + EXPECT_TRUE(IsEqual(*source, *target, 0)); + const T* const_source = source.get(); + EXPECT_EQ(const_source->video_frame_buffer(), target->video_frame_buffer()); + } + int repeat_; };
diff --git a/webrtc/media/engine/webrtcvideoframe.cc b/webrtc/media/engine/webrtcvideoframe.cc index 3102b05..71219d1 100644 --- a/webrtc/media/engine/webrtcvideoframe.cc +++ b/webrtc/media/engine/webrtcvideoframe.cc
@@ -103,6 +103,11 @@ return rotation_; } +VideoFrame* WebRtcVideoFrame::Copy() const { + return new WebRtcVideoFrame(video_frame_buffer_, rotation_, timestamp_us_, + transport_frame_id_); +} + size_t WebRtcVideoFrame::ConvertToRgbBuffer(uint32_t to_fourcc, uint8_t* buffer, size_t size,
diff --git a/webrtc/media/engine/webrtcvideoframe.h b/webrtc/media/engine/webrtcvideoframe.h index cfbef51..bdf5a02 100644 --- a/webrtc/media/engine/webrtcvideoframe.h +++ b/webrtc/media/engine/webrtcvideoframe.h
@@ -93,6 +93,8 @@ webrtc::VideoRotation rotation() const override; + VideoFrame* Copy() const override; + size_t ConvertToRgbBuffer(uint32_t to_fourcc, uint8_t* buffer, size_t size,
diff --git a/webrtc/media/engine/webrtcvideoframe_unittest.cc b/webrtc/media/engine/webrtcvideoframe_unittest.cc index 9d6bd5f..fb7be22 100644 --- a/webrtc/media/engine/webrtcvideoframe_unittest.cc +++ b/webrtc/media/engine/webrtcvideoframe_unittest.cc
@@ -223,6 +223,9 @@ // TEST_WEBRTCVIDEOFRAME(ConvertToI422Buffer) // TEST_WEBRTCVIDEOFRAME(ConstructARGBBlackWhitePixel) +TEST_WEBRTCVIDEOFRAME(Copy) +TEST_WEBRTCVIDEOFRAME(CopyIsRef) + // These functions test implementation-specific details. // Tests the Init function with different cropped size. TEST_F(WebRtcVideoFrameTest, InitEvenSize) { @@ -267,6 +270,24 @@ EXPECT_EQ(40, frame.timestamp_us()); } +TEST_F(WebRtcVideoFrameTest, CopyTextureFrame) { + webrtc::test::FakeNativeHandle* dummy_handle = + new webrtc::test::FakeNativeHandle(); + webrtc::NativeHandleBuffer* buffer = + new rtc::RefCountedObject<webrtc::test::FakeNativeHandleBuffer>( + 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(); + EXPECT_EQ(frame1.video_frame_buffer()->native_handle(), + frame2->video_frame_buffer()->native_handle()); + EXPECT_EQ(frame1.width(), frame2->width()); + EXPECT_EQ(frame1.height(), frame2->height()); + EXPECT_EQ(frame1.GetTimeStamp(), frame2->GetTimeStamp()); + EXPECT_EQ(frame1.timestamp_us(), frame2->timestamp_us()); + delete frame2; +} + TEST_F(WebRtcVideoFrameTest, ApplyRotationToFrame) { WebRtcVideoFrame applied0; EXPECT_TRUE(IsNull(applied0));