Reland "Remove usage of webrtc::NativeHandle since is just adds an extra level of indirection.""
This reverts commit e41d774c4d0a60066866fc2d0ae48dd0e839ff23.
Original code review: https://webrtc-codereview.appspot.com/43999004/
Reason for reland: There was nothing wrong with this cl as is, but it breaks chrome compatibility. We will now reland this and fix Chrome during roll.
Patset 1: Original cl.
Patchset 2: Removed more code that is no longer needed.
R=magjed@webrtc.org, pbos@webrtc.org
TBR=mflodman@webrtc.org
BUG=1128
Review URL: https://webrtc-codereview.appspot.com/45049004
Cr-Original-Commit-Position: refs/heads/master@{#8956}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: 9b3f56ea055934a5d5416db0386c857494410acc
diff --git a/common_video/BUILD.gn b/common_video/BUILD.gn
index 24423f5..ca196de 100644
--- a/common_video/BUILD.gn
+++ b/common_video/BUILD.gn
@@ -21,7 +21,6 @@
"i420_video_frame.cc",
"interface/i420_video_frame.h",
"interface/i420_buffer_pool.h",
- "interface/native_handle.h",
"interface/video_frame_buffer.h",
"libyuv/include/scaler.h",
"libyuv/include/webrtc_libyuv.h",
diff --git a/common_video/common_video.gyp b/common_video/common_video.gyp
index 5a41201..4577a80 100644
--- a/common_video/common_video.gyp
+++ b/common_video/common_video.gyp
@@ -41,7 +41,6 @@
'sources': [
'interface/i420_buffer_pool.h',
'interface/i420_video_frame.h',
- 'interface/native_handle.h',
'interface/video_frame_buffer.h',
'i420_buffer_pool.cc',
'i420_video_frame.cc',
diff --git a/common_video/i420_buffer_pool.cc b/common_video/i420_buffer_pool.cc
index 04a0ab9..35a6c10 100644
--- a/common_video/i420_buffer_pool.cc
+++ b/common_video/i420_buffer_pool.cc
@@ -38,9 +38,7 @@
int stride(webrtc::PlaneType type) const override {
return buffer_->stride(type);
}
- rtc::scoped_refptr<webrtc::NativeHandle> native_handle() const override {
- return nullptr;
- }
+ void* native_handle() const override { return nullptr; }
friend class rtc::RefCountedObject<PooledI420Buffer>;
rtc::scoped_refptr<webrtc::I420Buffer> buffer_;
diff --git a/common_video/i420_video_frame.cc b/common_video/i420_video_frame.cc
index 25e1bf7..6fd5f3e 100644
--- a/common_video/i420_video_frame.cc
+++ b/common_video/i420_video_frame.cc
@@ -14,6 +14,7 @@
#include <algorithm> // swap
+#include "webrtc/base/bind.h"
#include "webrtc/base/checks.h"
namespace webrtc {
@@ -36,20 +37,20 @@
rotation_(rotation) {
}
-I420VideoFrame::I420VideoFrame(NativeHandle* handle,
+I420VideoFrame::I420VideoFrame(void* native_handle,
int width,
int height,
uint32_t timestamp,
- int64_t render_time_ms)
- : video_frame_buffer_(
- new rtc::RefCountedObject<TextureBuffer>(handle, width, height)),
- timestamp_(timestamp),
- ntp_time_ms_(0),
- render_time_ms_(render_time_ms),
- rotation_(kVideoRotation_0) {
- DCHECK(handle != nullptr);
- DCHECK_GT(width, 0);
- DCHECK_GT(height, 0);
+ int64_t render_time_ms,
+ VideoRotation rotation,
+ const rtc::Callback0<void>& no_longer_used)
+ : I420VideoFrame(new rtc::RefCountedObject<TextureBuffer>(native_handle,
+ width,
+ height,
+ no_longer_used),
+ timestamp,
+ render_time_ms,
+ rotation) {
}
int I420VideoFrame::CreateEmptyFrame(int width, int height,
diff --git a/common_video/i420_video_frame_unittest.cc b/common_video/i420_video_frame_unittest.cc
index 8273afc..adbdb03 100644
--- a/common_video/i420_video_frame_unittest.cc
+++ b/common_video/i420_video_frame_unittest.cc
@@ -12,22 +12,21 @@
#include <string.h>
#include "testing/gtest/include/gtest/gtest.h"
+#include "webrtc/base/bind.h"
#include "webrtc/base/scoped_ptr.h"
#include "webrtc/common_video/interface/i420_video_frame.h"
namespace webrtc {
-class NativeHandleImpl : public NativeHandle {
+class NativeHandleImpl {
public:
- NativeHandleImpl() : ref_count_(0) {}
+ NativeHandleImpl() : no_longer_needed_(false) {}
virtual ~NativeHandleImpl() {}
- virtual int32_t AddRef() { return ++ref_count_; }
- virtual int32_t Release() { return --ref_count_; }
- virtual void* GetHandle() { return NULL; }
+ bool no_longer_needed() const { return no_longer_needed_; }
+ void SetNoLongerNeeded() { no_longer_needed_ = true; }
- int32_t ref_count() { return ref_count_; }
private:
- int32_t ref_count_;
+ bool no_longer_needed_;
};
bool EqualPlane(const uint8_t* data1,
@@ -256,7 +255,8 @@
TEST(TestI420VideoFrame, TextureInitialValues) {
NativeHandleImpl handle;
- I420VideoFrame frame(&handle, 640, 480, 100, 10);
+ I420VideoFrame frame(&handle, 640, 480, 100, 10, webrtc::kVideoRotation_0,
+ rtc::Callback0<void>());
EXPECT_EQ(640, frame.width());
EXPECT_EQ(480, frame.height());
EXPECT_EQ(100u, frame.timestamp());
@@ -269,13 +269,15 @@
EXPECT_EQ(20, frame.render_time_ms());
}
-TEST(TestI420VideoFrame, RefCount) {
+TEST(TestI420VideoFrame, NoLongerNeeded) {
NativeHandleImpl handle;
- EXPECT_EQ(0, handle.ref_count());
- I420VideoFrame *frame = new I420VideoFrame(&handle, 640, 480, 100, 200);
- EXPECT_EQ(1, handle.ref_count());
+ ASSERT_FALSE(handle.no_longer_needed());
+ I420VideoFrame* frame = new I420VideoFrame(
+ &handle, 640, 480, 100, 200, webrtc::kVideoRotation_0,
+ rtc::Bind(&NativeHandleImpl::SetNoLongerNeeded, &handle));
+ EXPECT_FALSE(handle.no_longer_needed());
delete frame;
- EXPECT_EQ(0, handle.ref_count());
+ EXPECT_TRUE(handle.no_longer_needed());
}
bool EqualPlane(const uint8_t* data1,
diff --git a/common_video/interface/native_handle.h b/common_video/interface/native_handle.h
deleted file mode 100644
index da1feab..0000000
--- a/common_video/interface/native_handle.h
+++ /dev/null
@@ -1,36 +0,0 @@
-/*
- * Copyright (c) 2013 The WebRTC project authors. All Rights Reserved.
- *
- * Use of this source code is governed by a BSD-style license
- * that can be found in the LICENSE file in the root of the source
- * tree. An additional intellectual property rights grant can be found
- * in the file PATENTS. All contributing project authors may
- * be found in the AUTHORS file in the root of the source tree.
- */
-
-#ifndef COMMON_VIDEO_INTERFACE_NATIVEHANDLE_H_
-#define COMMON_VIDEO_INTERFACE_NATIVEHANDLE_H_
-
-#include "webrtc/typedefs.h"
-
-namespace webrtc {
-
-// A class to store an opaque handle of the underlying video frame. This is used
-// when the frame is backed by a texture. WebRTC carries the handle in
-// TextureBuffer. This object keeps a reference to the handle. The reference
-// is cleared when the object is destroyed. It is important to destroy the
-// object as soon as possible so the texture can be recycled.
-class NativeHandle {
- public:
- virtual ~NativeHandle() {}
- // For scoped_refptr
- virtual int32_t AddRef() = 0;
- virtual int32_t Release() = 0;
-
- // Gets the handle.
- virtual void* GetHandle() = 0;
-};
-
-} // namespace webrtc
-
-#endif // COMMON_VIDEO_INTERFACE_NATIVEHANDLE_H_
diff --git a/common_video/interface/video_frame_buffer.h b/common_video/interface/video_frame_buffer.h
index e7cae48..1aefa6c 100644
--- a/common_video/interface/video_frame_buffer.h
+++ b/common_video/interface/video_frame_buffer.h
@@ -15,7 +15,6 @@
#include "webrtc/base/refcount.h"
#include "webrtc/base/scoped_ptr.h"
#include "webrtc/base/scoped_ref_ptr.h"
-#include "webrtc/common_video/interface/native_handle.h"
#include "webrtc/system_wrappers/interface/aligned_malloc.h"
namespace webrtc {
@@ -51,7 +50,7 @@
// Return the handle of the underlying video frame. This is used when the
// frame is backed by a texture.
- virtual rtc::scoped_refptr<NativeHandle> native_handle() const = 0;
+ virtual void* native_handle() const = 0;
protected:
virtual ~VideoFrameBuffer();
@@ -68,7 +67,7 @@
const uint8_t* data(PlaneType type) const override;
uint8_t* data(PlaneType type) override;
int stride(PlaneType type) const override;
- rtc::scoped_refptr<NativeHandle> native_handle() const override;
+ void* native_handle() const override;
protected:
~I420Buffer() override;
@@ -82,27 +81,31 @@
const rtc::scoped_ptr<uint8_t, AlignedFreeDeleter> data_;
};
-// Texture buffer around a NativeHandle.
+// Texture buffer is a VideoFrameBuffer wrapper around a |native_handle|.
+// |native_handle| must be valid for the lifetime of an instance of this object.
+// |no_longer_used| can be used to manage the lifetime of |native_handle|.
class TextureBuffer : public VideoFrameBuffer {
public:
- TextureBuffer(const rtc::scoped_refptr<NativeHandle>& native_handle,
+ TextureBuffer(void* native_handle,
int width,
- int height);
-
+ int height,
+ const rtc::Callback0<void>& no_longer_used);
int width() const override;
int height() const override;
const uint8_t* data(PlaneType type) const override;
uint8_t* data(PlaneType type) override;
int stride(PlaneType type) const override;
- rtc::scoped_refptr<NativeHandle> native_handle() const override;
+ void* native_handle() const override;
private:
friend class rtc::RefCountedObject<TextureBuffer>;
~TextureBuffer() override;
- const rtc::scoped_refptr<NativeHandle> native_handle_;
+ // |native_handle_| is a raw pointer and not owned by TextureBuffer.
+ void* native_handle_;
const int width_;
const int height_;
+ rtc::Callback0<void> no_longer_used_cb_;
};
class WrappedI420Buffer : public webrtc::VideoFrameBuffer {
@@ -125,7 +128,7 @@
uint8_t* data(PlaneType type) override;
int stride(PlaneType type) const override;
- rtc::scoped_refptr<NativeHandle> native_handle() const override;
+ void* native_handle() const override;
private:
friend class rtc::RefCountedObject<WrappedI420Buffer>;
diff --git a/common_video/video_frame_buffer.cc b/common_video/video_frame_buffer.cc
index cca685b..908c972 100644
--- a/common_video/video_frame_buffer.cc
+++ b/common_video/video_frame_buffer.cc
@@ -10,6 +10,7 @@
#include "webrtc/common_video/interface/video_frame_buffer.h"
+#include "webrtc/base/bind.h"
#include "webrtc/base/checks.h"
// Aligning pointer to 64 bytes for improved performance, e.g. use SIMD.
@@ -89,21 +90,25 @@
}
}
-rtc::scoped_refptr<NativeHandle> I420Buffer::native_handle() const {
+void* I420Buffer::native_handle() const {
return nullptr;
}
-TextureBuffer::TextureBuffer(
- const rtc::scoped_refptr<NativeHandle>& native_handle,
- int width,
- int height)
- : native_handle_(native_handle), width_(width), height_(height) {
- DCHECK(native_handle.get());
+TextureBuffer::TextureBuffer(void* native_handle,
+ int width,
+ int height,
+ const rtc::Callback0<void>& no_longer_used)
+ : native_handle_(native_handle),
+ width_(width),
+ height_(height),
+ no_longer_used_cb_(no_longer_used) {
+ DCHECK(native_handle != nullptr);
DCHECK_GT(width, 0);
DCHECK_GT(height, 0);
}
TextureBuffer::~TextureBuffer() {
+ no_longer_used_cb_();
}
int TextureBuffer::width() const {
@@ -129,7 +134,7 @@
return 0;
}
-rtc::scoped_refptr<NativeHandle> TextureBuffer::native_handle() const {
+void* TextureBuffer::native_handle() const {
return native_handle_;
}
@@ -211,7 +216,7 @@
}
}
-rtc::scoped_refptr<NativeHandle> WrappedI420Buffer::native_handle() const {
+void* WrappedI420Buffer::native_handle() const {
return nullptr;
}
diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc
index 428a273..61e3d5b 100644
--- a/video/video_send_stream_tests.cc
+++ b/video/video_send_stream_tests.cc
@@ -12,11 +12,11 @@
#include "testing/gtest/include/gtest/gtest.h"
+#include "webrtc/base/bind.h"
#include "webrtc/base/checks.h"
#include "webrtc/base/scoped_ptr.h"
#include "webrtc/call.h"
#include "webrtc/common_video/interface/i420_video_frame.h"
-#include "webrtc/common_video/interface/native_handle.h"
#include "webrtc/frame_callback.h"
#include "webrtc/modules/rtp_rtcp/interface/rtp_header_parser.h"
#include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h"
@@ -50,13 +50,16 @@
const std::vector<I420VideoFrame>& frames2);
I420VideoFrame CreateI420VideoFrame(int width, int height, uint8_t data);
-class FakeNativeHandle : public NativeHandle {
+class FakeNativeHandle {
public:
FakeNativeHandle() {}
- virtual ~FakeNativeHandle() {}
- virtual void* GetHandle() { return nullptr; }
+ ~FakeNativeHandle() {}
};
+void DeleteNativeHandle(FakeNativeHandle* handle) {
+ delete handle;
+}
+
class VideoSendStreamTest : public test::CallTest {
protected:
void TestNackRetransmission(uint32_t retransmit_ssrc,
@@ -1074,17 +1077,20 @@
std::vector<I420VideoFrame> input_frames;
int width = static_cast<int>(encoder_config_.streams[0].width);
int height = static_cast<int>(encoder_config_.streams[0].height);
- webrtc::RefCountImpl<FakeNativeHandle>* handle1 =
- new webrtc::RefCountImpl<FakeNativeHandle>();
- webrtc::RefCountImpl<FakeNativeHandle>* handle2 =
- new webrtc::RefCountImpl<FakeNativeHandle>();
- webrtc::RefCountImpl<FakeNativeHandle>* handle3 =
- new webrtc::RefCountImpl<FakeNativeHandle>();
- input_frames.push_back(I420VideoFrame(handle1, width, height, 1, 1));
- input_frames.push_back(I420VideoFrame(handle2, width, height, 2, 2));
+ FakeNativeHandle* handle1 = new FakeNativeHandle();
+ FakeNativeHandle* handle2 = new FakeNativeHandle();
+ FakeNativeHandle* handle3 = new FakeNativeHandle();
+ input_frames.push_back(
+ I420VideoFrame(handle1, width, height, 1, 1, kVideoRotation_0,
+ rtc::Bind(&DeleteNativeHandle, handle1)));
+ input_frames.push_back(
+ I420VideoFrame(handle2, width, height, 2, 2, kVideoRotation_0,
+ rtc::Bind(&DeleteNativeHandle, handle2)));
input_frames.push_back(CreateI420VideoFrame(width, height, 3));
input_frames.push_back(CreateI420VideoFrame(width, height, 4));
- input_frames.push_back(I420VideoFrame(handle3, width, height, 5, 5));
+ input_frames.push_back(
+ I420VideoFrame(handle3, width, height, 5, 5, kVideoRotation_0,
+ rtc::Bind(&DeleteNativeHandle, handle3)));
send_stream_->Start();
for (size_t i = 0; i < input_frames.size(); i++) {
diff --git a/video_engine/vie_capturer_unittest.cc b/video_engine/vie_capturer_unittest.cc
index de83577..0f20afc 100644
--- a/video_engine/vie_capturer_unittest.cc
+++ b/video_engine/vie_capturer_unittest.cc
@@ -18,7 +18,6 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "webrtc/base/scoped_ptr.h"
#include "webrtc/common.h"
-#include "webrtc/common_video/interface/native_handle.h"
#include "webrtc/modules/utility/interface/mock/mock_process_thread.h"
#include "webrtc/modules/video_capture/include/mock/mock_video_capture.h"
#include "webrtc/system_wrappers/interface/critical_section_wrapper.h"
@@ -48,13 +47,6 @@
const ScopedVector<I420VideoFrame>& frames2);
I420VideoFrame* CreateI420VideoFrame(uint8_t length);
-class FakeNativeHandle : public NativeHandle {
- public:
- FakeNativeHandle() {}
- virtual ~FakeNativeHandle() {}
- virtual void* GetHandle() { return NULL; }
-};
-
class ViECapturerTest : public ::testing::Test {
protected:
ViECapturerTest()
@@ -181,13 +173,14 @@
TEST_F(ViECapturerTest, TestTextureFrames) {
const int kNumFrame = 3;
for (int i = 0 ; i < kNumFrame; ++i) {
- webrtc::RefCountImpl<FakeNativeHandle>* handle =
- new webrtc::RefCountImpl<FakeNativeHandle>();
+ void* dummy_handle = reinterpret_cast<void*>(i+1);
// Add one to |i| so that width/height > 0.
input_frames_.push_back(
- new I420VideoFrame(handle, i + 1, i + 1, i + 1, i + 1));
+ new I420VideoFrame(dummy_handle, i + 1, i + 1, i + 1, i + 1,
+ webrtc::kVideoRotation_0, rtc::Callback0<void>()));
AddInputFrame(input_frames_[i]);
WaitOutputFrame();
+ EXPECT_EQ(dummy_handle, output_frames_[i]->native_handle());
}
EXPECT_TRUE(EqualFramesVector(input_frames_, output_frames_));
@@ -211,11 +204,13 @@
}
TEST_F(ViECapturerTest, TestI420FrameAfterTextureFrame) {
- webrtc::RefCountImpl<FakeNativeHandle>* handle =
- new webrtc::RefCountImpl<FakeNativeHandle>();
- input_frames_.push_back(new I420VideoFrame(handle, 1, 1, 1, 1));
+ void* dummy_handle = &input_frames_;
+ input_frames_.push_back(new I420VideoFrame(dummy_handle, 1, 1, 1, 1,
+ webrtc::kVideoRotation_0,
+ rtc::Callback0<void>()));
AddInputFrame(input_frames_[0]);
WaitOutputFrame();
+ EXPECT_EQ(dummy_handle, output_frames_[0]->native_handle());
input_frames_.push_back(CreateI420VideoFrame(2));
AddInputFrame(input_frames_[1]);
@@ -229,9 +224,10 @@
AddInputFrame(input_frames_[0]);
WaitOutputFrame();
- webrtc::RefCountImpl<FakeNativeHandle>* handle =
- new webrtc::RefCountImpl<FakeNativeHandle>();
- input_frames_.push_back(new I420VideoFrame(handle, 1, 1, 2, 2));
+ void* dummy_handle = &input_frames_;
+ input_frames_.push_back(new I420VideoFrame(dummy_handle, 1, 1, 2, 2,
+ webrtc::kVideoRotation_0,
+ rtc::Callback0<void>()));
AddInputFrame(input_frames_[1]);
WaitOutputFrame();
diff --git a/video_frame.h b/video_frame.h
index bd3e2b8..db8f051 100644
--- a/video_frame.h
+++ b/video_frame.h
@@ -12,7 +12,6 @@
#define WEBRTC_VIDEO_FRAME_H_
#include "webrtc/base/scoped_ref_ptr.h"
-#include "webrtc/common_video/interface/native_handle.h"
#include "webrtc/common_video/interface/video_frame_buffer.h"
#include "webrtc/common_video/rotation.h"
#include "webrtc/typedefs.h"
@@ -26,11 +25,13 @@
uint32_t timestamp,
int64_t render_time_ms,
VideoRotation rotation);
- I420VideoFrame(NativeHandle* handle,
+ I420VideoFrame(void* native_handle,
int width,
int height,
uint32_t timestamp,
- int64_t render_time_ms);
+ int64_t render_time_ms,
+ VideoRotation rotation,
+ const rtc::Callback0<void>& no_longer_used);
// TODO(pbos): Make all create/copy functions void, they should not be able to
// fail (which should be DCHECK/CHECKed instead).
