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));
