Remove usage of webrtc::NativeHandle since is just adds an extra level of indirection.
BUG=1128
R=magjed@webrtc.org, pbos@webrtc.org
TBR=mflodman@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/43999004
Cr-Commit-Position: refs/heads/master@{#8932}
diff --git a/talk/app/webrtc/java/jni/androidmediadecoder_jni.cc b/talk/app/webrtc/java/jni/androidmediadecoder_jni.cc
index da6921f..2c2c12d 100644
--- a/talk/app/webrtc/java/jni/androidmediadecoder_jni.cc
+++ b/talk/app/webrtc/java/jni/androidmediadecoder_jni.cc
@@ -654,8 +654,9 @@
int32_t callback_status = WEBRTC_VIDEO_CODEC_OK;
if (use_surface_) {
native_handle_.SetTextureObject(surface_texture_, texture_id);
- I420VideoFrame texture_image(
- &native_handle_, width, height, output_timestamp_, 0);
+ I420VideoFrame texture_image(&native_handle_, width, height,
+ output_timestamp_, 0, webrtc::kVideoRotation_0,
+ rtc::Callback0<void>());
texture_image.set_ntp_time_ms(output_ntp_time_ms_);
callback_status = callback_->Decoded(texture_image);
} else {
diff --git a/talk/app/webrtc/java/jni/native_handle_impl.h b/talk/app/webrtc/java/jni/native_handle_impl.h
index 7de29d1..a10a4f3 100644
--- a/talk/app/webrtc/java/jni/native_handle_impl.h
+++ b/talk/app/webrtc/java/jni/native_handle_impl.h
@@ -33,19 +33,12 @@
namespace webrtc_jni {
-// Wrapper for texture object in TextureBuffer.
-class NativeHandleImpl : public webrtc::NativeHandle {
+// Wrapper for texture object.
+class NativeHandleImpl {
public:
- NativeHandleImpl() :
- ref_count_(0), texture_object_(NULL), texture_id_(-1) {}
- virtual ~NativeHandleImpl() {}
- virtual int32_t AddRef() {
- return ++ref_count_;
- }
- virtual int32_t Release() {
- return --ref_count_;
- }
- virtual void* GetHandle() {
+ NativeHandleImpl() : texture_object_(NULL), texture_id_(-1) {}
+
+ void* GetHandle() {
return texture_object_;
}
int GetTextureId() {
@@ -55,12 +48,8 @@
texture_object_ = reinterpret_cast<jobject>(texture_object);
texture_id_ = texture_id;
}
- int32_t ref_count() {
- return ref_count_;
- }
private:
- int32_t ref_count_;
jobject texture_object_;
int32_t texture_id_;
};
diff --git a/talk/media/webrtc/webrtcvideoframe.cc b/talk/media/webrtc/webrtcvideoframe.cc
index e308a73..055e24d 100644
--- a/talk/media/webrtc/webrtcvideoframe.cc
+++ b/talk/media/webrtc/webrtcvideoframe.cc
@@ -71,23 +71,6 @@
rotation_(webrtc::kVideoRotation_0) {
}
-WebRtcVideoFrame::WebRtcVideoFrame(webrtc::NativeHandle* handle,
- int width,
- int height,
- int64_t elapsed_time_ns,
- int64_t time_stamp_ns,
- webrtc::VideoRotation rotation)
- : video_frame_buffer_(
- new rtc::RefCountedObject<webrtc::TextureBuffer>(handle,
- width,
- height)),
- pixel_width_(1),
- pixel_height_(1),
- elapsed_time_ns_(elapsed_time_ns),
- time_stamp_ns_(time_stamp_ns),
- rotation_(rotation) {
-}
-
WebRtcVideoFrame::~WebRtcVideoFrame() {}
bool WebRtcVideoFrame::Init(uint32 format,
diff --git a/talk/media/webrtc/webrtcvideoframe.h b/talk/media/webrtc/webrtcvideoframe.h
index a1872c5..06ac52b 100644
--- a/talk/media/webrtc/webrtcvideoframe.h
+++ b/talk/media/webrtc/webrtcvideoframe.h
@@ -52,12 +52,6 @@
int64_t elapsed_time_ns,
int64_t time_stamp_ns);
- WebRtcVideoFrame(webrtc::NativeHandle* handle,
- int width,
- int height,
- int64_t elapsed_time_ns,
- int64_t time_stamp_ns,
- webrtc::VideoRotation rotation);
~WebRtcVideoFrame();
// Creates a frame from a raw sample with FourCC "format" and size "w" x "h".
diff --git a/talk/media/webrtc/webrtcvideoframe_unittest.cc b/talk/media/webrtc/webrtcvideoframe_unittest.cc
index 8cbe479..e3c4311 100644
--- a/talk/media/webrtc/webrtcvideoframe_unittest.cc
+++ b/talk/media/webrtc/webrtcvideoframe_unittest.cc
@@ -30,19 +30,6 @@
#include "talk/media/base/videoframe_unittest.h"
#include "talk/media/webrtc/webrtcvideoframe.h"
-class NativeHandleImpl : public webrtc::NativeHandle {
- public:
- NativeHandleImpl() : ref_count_(0) {}
- virtual ~NativeHandleImpl() {}
- virtual int32_t AddRef() { return ++ref_count_; }
- virtual int32_t Release() { return --ref_count_; }
- virtual void* GetHandle() { return NULL; }
-
- int32_t ref_count() { return ref_count_; }
- private:
- int32_t ref_count_;
-};
-
namespace {
class WebRtcVideoTestFrame : public cricket::WebRtcVideoFrame {
@@ -339,10 +326,12 @@
}
TEST_F(WebRtcVideoFrameTest, TextureInitialValues) {
- NativeHandleImpl handle;
- cricket::WebRtcVideoFrame frame(&handle, 640, 480, 100, 200,
- webrtc::kVideoRotation_0);
- EXPECT_EQ(&handle, frame.GetNativeHandle());
+ void* dummy_handle = reinterpret_cast<void*>(0x1);
+ webrtc::TextureBuffer* buffer =
+ new rtc::RefCountedObject<webrtc::TextureBuffer>(dummy_handle, 640, 480,
+ rtc::Callback0<void>());
+ cricket::WebRtcVideoFrame frame(buffer, 100, 200, webrtc::kVideoRotation_0);
+ EXPECT_EQ(dummy_handle, frame.GetNativeHandle());
EXPECT_EQ(640u, frame.GetWidth());
EXPECT_EQ(480u, frame.GetHeight());
EXPECT_EQ(100, frame.GetElapsedTime());
@@ -354,9 +343,11 @@
}
TEST_F(WebRtcVideoFrameTest, CopyTextureFrame) {
- NativeHandleImpl handle;
- cricket::WebRtcVideoFrame frame1(&handle, 640, 480, 100, 200,
- webrtc::kVideoRotation_0);
+ void* dummy_handle = reinterpret_cast<void*>(0x1);
+ webrtc::TextureBuffer* buffer =
+ new rtc::RefCountedObject<webrtc::TextureBuffer>(dummy_handle, 640, 480,
+ rtc::Callback0<void>());
+ cricket::WebRtcVideoFrame frame1(buffer, 100, 200, webrtc::kVideoRotation_0);
cricket::VideoFrame* frame2 = frame1.Copy();
EXPECT_EQ(frame1.GetNativeHandle(), frame2->GetNativeHandle());
EXPECT_EQ(frame1.GetWidth(), frame2->GetWidth());
diff --git a/webrtc/common_video/i420_buffer_pool.cc b/webrtc/common_video/i420_buffer_pool.cc
index 04a0ab9..35a6c10 100644
--- a/webrtc/common_video/i420_buffer_pool.cc
+++ b/webrtc/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/webrtc/common_video/i420_video_frame.cc b/webrtc/common_video/i420_video_frame.cc
index 25e1bf7..7bbde28 100644
--- a/webrtc/common_video/i420_video_frame.cc
+++ b/webrtc/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 {
@@ -52,6 +53,22 @@
DCHECK_GT(height, 0);
}
+I420VideoFrame::I420VideoFrame(void* native_handle,
+ int width,
+ int height,
+ uint32_t timestamp,
+ 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,
int stride_y, int stride_u, int stride_v) {
const int half_width = (width + 1) / 2;
diff --git a/webrtc/common_video/i420_video_frame_unittest.cc b/webrtc/common_video/i420_video_frame_unittest.cc
index 8273afc..adbdb03 100644
--- a/webrtc/common_video/i420_video_frame_unittest.cc
+++ b/webrtc/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/webrtc/common_video/interface/native_handle.h b/webrtc/common_video/interface/native_handle.h
index da1feab..13c26bd 100644
--- a/webrtc/common_video/interface/native_handle.h
+++ b/webrtc/common_video/interface/native_handle.h
@@ -20,6 +20,9 @@
// 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.
+
+// TODO(perkj): Remove this class once Chrome uses TextureBuffer directly
+// instead. This is just an extra level of indirection.
class NativeHandle {
public:
virtual ~NativeHandle() {}
diff --git a/webrtc/common_video/interface/video_frame_buffer.h b/webrtc/common_video/interface/video_frame_buffer.h
index e7cae48..90beb5b 100644
--- a/webrtc/common_video/interface/video_frame_buffer.h
+++ b/webrtc/common_video/interface/video_frame_buffer.h
@@ -51,7 +51,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 +68,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,9 +82,17 @@
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(void* native_handle,
+ int width,
+ int height,
+ const rtc::Callback0<void>& no_longer_used);
+
+ // TODO(perkj): Remove once Chrome does not need it.
TextureBuffer(const rtc::scoped_refptr<NativeHandle>& native_handle,
int width,
int height);
@@ -94,15 +102,17 @@
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 {
diff --git a/webrtc/common_video/video_frame_buffer.cc b/webrtc/common_video/video_frame_buffer.cc
index cca685b..254183f 100644
--- a/webrtc/common_video/video_frame_buffer.cc
+++ b/webrtc/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,39 @@
}
}
-rtc::scoped_refptr<NativeHandle> I420Buffer::native_handle() const {
+void* I420Buffer::native_handle() const {
return nullptr;
}
+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);
+}
+
+static void ReleaseNativeHandle(
+ rtc::scoped_refptr<NativeHandle> native_handle) {
+}
+
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());
- DCHECK_GT(width, 0);
- DCHECK_GT(height, 0);
+ : TextureBuffer(native_handle->GetHandle(),
+ width,
+ height,
+ rtc::Bind(&ReleaseNativeHandle, native_handle)) {
}
TextureBuffer::~TextureBuffer() {
+ no_longer_used_cb_();
}
int TextureBuffer::width() const {
@@ -129,7 +148,7 @@
return 0;
}
-rtc::scoped_refptr<NativeHandle> TextureBuffer::native_handle() const {
+void* TextureBuffer::native_handle() const {
return native_handle_;
}
diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc
index 428a273..bc823b2 100644
--- a/webrtc/video/video_send_stream_tests.cc
+++ b/webrtc/video/video_send_stream_tests.cc
@@ -12,6 +12,7 @@
#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"
@@ -50,13 +51,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 +1078,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/webrtc/video_engine/vie_capturer_unittest.cc b/webrtc/video_engine/vie_capturer_unittest.cc
index 18aa496..389f0bb 100644
--- a/webrtc/video_engine/vie_capturer_unittest.cc
+++ b/webrtc/video_engine/vie_capturer_unittest.cc
@@ -48,13 +48,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 +174,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 +205,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 +225,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/webrtc/video_frame.h b/webrtc/video_frame.h
index bd3e2b8..b101102 100644
--- a/webrtc/video_frame.h
+++ b/webrtc/video_frame.h
@@ -26,11 +26,19 @@
uint32_t timestamp,
int64_t render_time_ms,
VideoRotation rotation);
+ // TODO(perkj): Remove this constructor once Chrome no longer use it.
I420VideoFrame(NativeHandle* handle,
int width,
int height,
uint32_t timestamp,
int64_t render_time_ms);
+ I420VideoFrame(void* native_handle,
+ int width,
+ int height,
+ uint32_t timestamp,
+ 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).