Added new VideoFrameBuffer methods Data[YUV]() etc.
Eliminate most uses of the old methods.
To continue on this path, once we agree the new methods make sense,
the next step is to rename cricket::VideoFrame::GetVideoFrameBuffer
--> video_frame_buffer, to match the name in webrtc::VideoFrame (if we
think that name is ok?). And then start updating all code to access
planes via the VideoFrameBuffer, and delete corresponding methods in
both cricket::VideoFrame and webrtc::VideoFrame.
BUG=webrtc:5682
Review URL: https://codereview.webrtc.org/1878623002
Cr-Original-Commit-Position: refs/heads/master@{#12407}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: 06176e49e276d290209676c832f17d34329d9ecd
diff --git a/common_video/i420_buffer_pool.cc b/common_video/i420_buffer_pool.cc
index d00ab78..c382e93 100644
--- a/common_video/i420_buffer_pool.cc
+++ b/common_video/i420_buffer_pool.cc
@@ -26,19 +26,28 @@
int width() const override { return buffer_->width(); }
int height() const override { return buffer_->height(); }
- const uint8_t* data(webrtc::PlaneType type) const override {
- return buffer_->data(type);
- }
+ const uint8_t* DataY() const override { return buffer_->DataY(); }
+ const uint8_t* DataU() const override { return buffer_->DataU(); }
+ const uint8_t* DataV() const override { return buffer_->DataV(); }
+
bool IsMutable() { return HasOneRef(); }
- uint8_t* MutableData(webrtc::PlaneType type) override {
- // Make the HasOneRef() check here instead of in |buffer_|, because the pool
- // also has a reference to |buffer_|.
+ // Make the IsMutable() check here instead of in |buffer_|, because the pool
+ // also has a reference to |buffer_|.
+ uint8_t* MutableDataY() override {
RTC_DCHECK(IsMutable());
- return const_cast<uint8_t*>(buffer_->data(type));
+ return const_cast<uint8_t*>(buffer_->DataY());
}
- int stride(webrtc::PlaneType type) const override {
- return buffer_->stride(type);
+ uint8_t* MutableDataU() override {
+ RTC_DCHECK(IsMutable());
+ return const_cast<uint8_t*>(buffer_->DataU());
}
+ uint8_t* MutableDataV() override {
+ RTC_DCHECK(IsMutable());
+ return const_cast<uint8_t*>(buffer_->DataV());
+ }
+ int StrideY() const override { return buffer_->StrideY(); }
+ int StrideU() const override { return buffer_->StrideU(); }
+ int StrideV() const override { return buffer_->StrideV(); }
void* native_handle() const override { return nullptr; }
rtc::scoped_refptr<VideoFrameBuffer> NativeToI420Buffer() override {
diff --git a/common_video/i420_buffer_pool_unittest.cc b/common_video/i420_buffer_pool_unittest.cc
index 273b72d..2110066 100644
--- a/common_video/i420_buffer_pool_unittest.cc
+++ b/common_video/i420_buffer_pool_unittest.cc
@@ -21,16 +21,16 @@
EXPECT_EQ(16, buffer->width());
EXPECT_EQ(16, buffer->height());
// Extract non-refcounted pointers for testing.
- const uint8_t* y_ptr = buffer->data(kYPlane);
- const uint8_t* u_ptr = buffer->data(kUPlane);
- const uint8_t* v_ptr = buffer->data(kVPlane);
+ const uint8_t* y_ptr = buffer->DataY();
+ const uint8_t* u_ptr = buffer->DataU();
+ const uint8_t* v_ptr = buffer->DataV();
// Release buffer so that it is returned to the pool.
buffer = nullptr;
// Check that the memory is resued.
buffer = pool.CreateBuffer(16, 16);
- EXPECT_EQ(y_ptr, buffer->data(kYPlane));
- EXPECT_EQ(u_ptr, buffer->data(kUPlane));
- EXPECT_EQ(v_ptr, buffer->data(kVPlane));
+ EXPECT_EQ(y_ptr, buffer->DataY());
+ EXPECT_EQ(u_ptr, buffer->DataU());
+ EXPECT_EQ(v_ptr, buffer->DataV());
EXPECT_EQ(16, buffer->width());
EXPECT_EQ(16, buffer->height());
}
@@ -39,16 +39,16 @@
I420BufferPool pool;
rtc::scoped_refptr<VideoFrameBuffer> buffer = pool.CreateBuffer(16, 16);
// Extract non-refcounted pointers for testing.
- const uint8_t* u_ptr = buffer->data(kUPlane);
- const uint8_t* v_ptr = buffer->data(kVPlane);
+ const uint8_t* u_ptr = buffer->DataU();
+ const uint8_t* v_ptr = buffer->DataV();
// Release buffer so that it is returned to the pool.
buffer = nullptr;
// Check that the pool doesn't try to reuse buffers of incorrect size.
buffer = pool.CreateBuffer(32, 16);
EXPECT_EQ(32, buffer->width());
EXPECT_EQ(16, buffer->height());
- EXPECT_NE(u_ptr, buffer->data(kUPlane));
- EXPECT_NE(v_ptr, buffer->data(kVPlane));
+ EXPECT_NE(u_ptr, buffer->DataU());
+ EXPECT_NE(v_ptr, buffer->DataV());
}
TEST(TestI420BufferPool, ExclusiveOwner) {
@@ -68,7 +68,7 @@
EXPECT_EQ(16, buffer->width());
EXPECT_EQ(16, buffer->height());
// Try to trigger use-after-free errors by writing to y-plane.
- memset(buffer->MutableData(kYPlane), 0xA5, 16 * buffer->stride(kYPlane));
+ memset(buffer->MutableDataY(), 0xA5, 16 * buffer->StrideY());
}
} // namespace webrtc
diff --git a/common_video/i420_video_frame_unittest.cc b/common_video/i420_video_frame_unittest.cc
index b2f59ea..8368436 100644
--- a/common_video/i420_video_frame_unittest.cc
+++ b/common_video/i420_video_frame_unittest.cc
@@ -258,9 +258,9 @@
TEST(TestI420FrameBuffer, Copy) {
rtc::scoped_refptr<I420Buffer> buf1(
new rtc::RefCountedObject<I420Buffer>(20, 10));
- memset(buf1->MutableData(kYPlane), 1, 200);
- memset(buf1->MutableData(kUPlane), 2, 50);
- memset(buf1->MutableData(kVPlane), 3, 50);
+ memset(buf1->MutableDataY(), 1, 200);
+ memset(buf1->MutableDataU(), 2, 50);
+ memset(buf1->MutableDataV(), 3, 50);
rtc::scoped_refptr<I420Buffer> buf2 = I420Buffer::Copy(buf1);
EXPECT_TRUE(test::FrameBufsEqual(buf1, buf2));
}
diff --git a/common_video/include/video_frame_buffer.h b/common_video/include/video_frame_buffer.h
index e78a1d6..6f082de 100644
--- a/common_video/include/video_frame_buffer.h
+++ b/common_video/include/video_frame_buffer.h
@@ -50,16 +50,36 @@
virtual int width() const = 0;
virtual int height() const = 0;
+ // TODO(nisse): For the transition, we use default implementations
+ // of the stride and data methods where the new methods calls the
+ // old method, and the old method calls the new methods. Subclasses
+ // must override either the new methods or the old method, to break
+ // infinite recursion. And similarly for the strides. When
+ // applications, in particular Chrome, are updated, delete the old
+ // method and delete the default implementation of the new methods.
+
// Returns pointer to the pixel data for a given plane. The memory is owned by
// the VideoFrameBuffer object and must not be freed by the caller.
- virtual const uint8_t* data(PlaneType type) const = 0;
+ virtual const uint8_t* DataY() const;
+ virtual const uint8_t* DataU() const;
+ virtual const uint8_t* DataV() const;
+ // Deprecated method.
+ // TODO(nisse): Delete after all users are updated.
+ virtual const uint8_t* data(PlaneType type) const;
- // Non-const data access is disallowed by default. You need to make sure you
- // have exclusive access and a writable buffer before calling this function.
+ // Non-const data access is allowed only if HasOneRef() is true.
+ virtual uint8_t* MutableDataY();
+ virtual uint8_t* MutableDataU();
+ virtual uint8_t* MutableDataV();
+ // Deprecated method. TODO(nisse): Delete after all users are updated.
virtual uint8_t* MutableData(PlaneType type);
// Returns the number of bytes between successive rows for a given plane.
- virtual int stride(PlaneType type) const = 0;
+ virtual int StrideY() const;
+ virtual int StrideU() const;
+ virtual int StrideV() const;
+ // Deprecated method. TODO(nisse): Delete after all users are updated.
+ virtual int stride(PlaneType type) const;
// Return the handle of the underlying video frame. This is used when the
// frame is backed by a texture.
@@ -82,12 +102,19 @@
int width() const override;
int height() const override;
- const uint8_t* data(PlaneType type) const override;
- // Non-const data access is only allowed if HasOneRef() is true to protect
+ const uint8_t* DataY() const override;
+ const uint8_t* DataU() const override;
+ const uint8_t* DataV() const override;
+ // Non-const data access is only allowed if IsMutable() is true, to protect
// against unexpected overwrites.
bool IsMutable() override;
- uint8_t* MutableData(PlaneType type) override;
- int stride(PlaneType type) const override;
+ uint8_t* MutableDataY() override;
+ uint8_t* MutableDataU() override;
+ uint8_t* MutableDataV() override;
+ int StrideY() const override;
+ int StrideU() const override;
+ int StrideV() const override;
+
void* native_handle() const override;
rtc::scoped_refptr<VideoFrameBuffer> NativeToI420Buffer() override;
@@ -117,8 +144,13 @@
int width() const override;
int height() const override;
- const uint8_t* data(PlaneType type) const override;
- int stride(PlaneType type) const override;
+ const uint8_t* DataY() const override;
+ const uint8_t* DataU() const override;
+ const uint8_t* DataV() const override;
+ int StrideY() const override;
+ int StrideU() const override;
+ int StrideV() const override;
+
void* native_handle() const override;
bool IsMutable() override;
@@ -144,9 +176,13 @@
bool IsMutable() override;
- const uint8_t* data(PlaneType type) const override;
+ const uint8_t* DataY() const override;
+ const uint8_t* DataU() const override;
+ const uint8_t* DataV() const override;
+ int StrideY() const override;
+ int StrideU() const override;
+ int StrideV() const override;
- int stride(PlaneType type) const override;
void* native_handle() const override;
rtc::scoped_refptr<VideoFrameBuffer> NativeToI420Buffer() override;
diff --git a/common_video/video_frame_buffer.cc b/common_video/video_frame_buffer.cc
index 6ce806e..700dcaf 100644
--- a/common_video/video_frame_buffer.cc
+++ b/common_video/video_frame_buffer.cc
@@ -27,10 +27,80 @@
} // namespace
-uint8_t* VideoFrameBuffer::MutableData(PlaneType type) {
+const uint8_t* VideoFrameBuffer::data(PlaneType type) const {
+ switch (type) {
+ case kYPlane:
+ return DataY();
+ case kUPlane:
+ return DataU();
+ case kVPlane:
+ return DataV();
+ default:
+ RTC_NOTREACHED();
+ return nullptr;
+ }
+}
+
+const uint8_t* VideoFrameBuffer::DataY() const {
+ return data(kYPlane);
+}
+const uint8_t* VideoFrameBuffer::DataU() const {
+ return data(kUPlane);
+}
+const uint8_t* VideoFrameBuffer::DataV() const {
+ return data(kVPlane);
+}
+
+int VideoFrameBuffer::stride(PlaneType type) const {
+ switch (type) {
+ case kYPlane:
+ return StrideY();
+ case kUPlane:
+ return StrideU();
+ case kVPlane:
+ return StrideV();
+ default:
+ RTC_NOTREACHED();
+ return 0;
+ }
+}
+
+int VideoFrameBuffer::StrideY() const {
+ return stride(kYPlane);
+}
+int VideoFrameBuffer::StrideU() const {
+ return stride(kUPlane);
+}
+int VideoFrameBuffer::StrideV() const {
+ return stride(kVPlane);
+}
+
+uint8_t* VideoFrameBuffer::MutableDataY() {
RTC_NOTREACHED();
return nullptr;
}
+uint8_t* VideoFrameBuffer::MutableDataU() {
+ RTC_NOTREACHED();
+ return nullptr;
+}
+uint8_t* VideoFrameBuffer::MutableDataV() {
+ RTC_NOTREACHED();
+ return nullptr;
+}
+
+uint8_t* VideoFrameBuffer::MutableData(PlaneType type) {
+ switch (type) {
+ case kYPlane:
+ return MutableDataY();
+ case kUPlane:
+ return MutableDataU();
+ case kVPlane:
+ return MutableDataV();
+ default:
+ RTC_NOTREACHED();
+ return nullptr;
+ }
+}
VideoFrameBuffer::~VideoFrameBuffer() {}
@@ -74,43 +144,41 @@
return height_;
}
-const uint8_t* I420Buffer::data(PlaneType type) const {
- switch (type) {
- case kYPlane:
- return data_.get();
- case kUPlane:
- return data_.get() + stride_y_ * height_;
- case kVPlane:
- return data_.get() + stride_y_ * height_ +
- stride_u_ * ((height_ + 1) / 2);
- default:
- RTC_NOTREACHED();
- return nullptr;
- }
+const uint8_t* I420Buffer::DataY() const {
+ return data_.get();
+}
+const uint8_t* I420Buffer::DataU() const {
+ return data_.get() + stride_y_ * height_;
+}
+const uint8_t* I420Buffer::DataV() const {
+ return data_.get() + stride_y_ * height_ + stride_u_ * ((height_ + 1) / 2);
}
bool I420Buffer::IsMutable() {
return HasOneRef();
}
-uint8_t* I420Buffer::MutableData(PlaneType type) {
+uint8_t* I420Buffer::MutableDataY() {
RTC_DCHECK(IsMutable());
- return const_cast<uint8_t*>(
- static_cast<const VideoFrameBuffer*>(this)->data(type));
+ return const_cast<uint8_t*>(DataY());
+}
+uint8_t* I420Buffer::MutableDataU() {
+ RTC_DCHECK(IsMutable());
+ return const_cast<uint8_t*>(DataU());
+}
+uint8_t* I420Buffer::MutableDataV() {
+ RTC_DCHECK(IsMutable());
+ return const_cast<uint8_t*>(DataV());
}
-int I420Buffer::stride(PlaneType type) const {
- switch (type) {
- case kYPlane:
- return stride_y_;
- case kUPlane:
- return stride_u_;
- case kVPlane:
- return stride_v_;
- default:
- RTC_NOTREACHED();
- return 0;
- }
+int I420Buffer::StrideY() const {
+ return stride_y_;
+}
+int I420Buffer::StrideU() const {
+ return stride_u_;
+}
+int I420Buffer::StrideV() const {
+ return stride_v_;
}
void* I420Buffer::native_handle() const {
@@ -128,12 +196,12 @@
int height = buffer->height();
rtc::scoped_refptr<I420Buffer> copy =
new rtc::RefCountedObject<I420Buffer>(width, height);
- RTC_CHECK(libyuv::I420Copy(buffer->data(kYPlane), buffer->stride(kYPlane),
- buffer->data(kUPlane), buffer->stride(kUPlane),
- buffer->data(kVPlane), buffer->stride(kVPlane),
- copy->MutableData(kYPlane), copy->stride(kYPlane),
- copy->MutableData(kUPlane), copy->stride(kUPlane),
- copy->MutableData(kVPlane), copy->stride(kVPlane),
+ RTC_CHECK(libyuv::I420Copy(buffer->DataY(), buffer->StrideY(),
+ buffer->DataU(), buffer->StrideU(),
+ buffer->DataV(), buffer->StrideV(),
+ copy->MutableDataY(), copy->StrideY(),
+ copy->MutableDataU(), copy->StrideU(),
+ copy->MutableDataV(), copy->StrideV(),
width, height) == 0);
return copy;
@@ -160,12 +228,28 @@
return height_;
}
-const uint8_t* NativeHandleBuffer::data(PlaneType type) const {
+const uint8_t* NativeHandleBuffer::DataY() const {
+ RTC_NOTREACHED(); // Should not be called.
+ return nullptr;
+}
+const uint8_t* NativeHandleBuffer::DataU() const {
+ RTC_NOTREACHED(); // Should not be called.
+ return nullptr;
+}
+const uint8_t* NativeHandleBuffer::DataV() const {
RTC_NOTREACHED(); // Should not be called.
return nullptr;
}
-int NativeHandleBuffer::stride(PlaneType type) const {
+int NativeHandleBuffer::StrideY() const {
+ RTC_NOTREACHED(); // Should not be called.
+ return 0;
+}
+int NativeHandleBuffer::StrideU() const {
+ RTC_NOTREACHED(); // Should not be called.
+ return 0;
+}
+int NativeHandleBuffer::StrideV() const {
RTC_NOTREACHED(); // Should not be called.
return 0;
}
@@ -211,32 +295,24 @@
return height_;
}
-const uint8_t* WrappedI420Buffer::data(PlaneType type) const {
- switch (type) {
- case kYPlane:
- return y_plane_;
- case kUPlane:
- return u_plane_;
- case kVPlane:
- return v_plane_;
- default:
- RTC_NOTREACHED();
- return nullptr;
- }
+const uint8_t* WrappedI420Buffer::DataY() const {
+ return y_plane_;
+}
+const uint8_t* WrappedI420Buffer::DataU() const {
+ return u_plane_;
+}
+const uint8_t* WrappedI420Buffer::DataV() const {
+ return v_plane_;
}
-int WrappedI420Buffer::stride(PlaneType type) const {
- switch (type) {
- case kYPlane:
- return y_stride_;
- case kUPlane:
- return u_stride_;
- case kVPlane:
- return v_stride_;
- default:
- RTC_NOTREACHED();
- return 0;
- }
+int WrappedI420Buffer::StrideY() const {
+ return y_stride_;
+}
+int WrappedI420Buffer::StrideU() const {
+ return u_stride_;
+}
+int WrappedI420Buffer::StrideV() const {
+ return v_stride_;
}
void* WrappedI420Buffer::native_handle() const {
@@ -265,17 +341,17 @@
const int offset_x = uv_offset_x * 2;
const int offset_y = uv_offset_y * 2;
- const uint8_t* y_plane = buffer->data(kYPlane) +
- buffer->stride(kYPlane) * offset_y + offset_x;
- const uint8_t* u_plane = buffer->data(kUPlane) +
- buffer->stride(kUPlane) * uv_offset_y + uv_offset_x;
- const uint8_t* v_plane = buffer->data(kVPlane) +
- buffer->stride(kVPlane) * uv_offset_y + uv_offset_x;
+ const uint8_t* y_plane = buffer->DataY() +
+ buffer->StrideY() * offset_y + offset_x;
+ const uint8_t* u_plane = buffer->DataU() +
+ buffer->StrideU() * uv_offset_y + uv_offset_x;
+ const uint8_t* v_plane = buffer->DataV() +
+ buffer->StrideV() * uv_offset_y + uv_offset_x;
return new rtc::RefCountedObject<WrappedI420Buffer>(
cropped_width, cropped_height,
- y_plane, buffer->stride(kYPlane),
- u_plane, buffer->stride(kUPlane),
- v_plane, buffer->stride(kVPlane),
+ y_plane, buffer->StrideY(),
+ u_plane, buffer->StrideU(),
+ v_plane, buffer->StrideV(),
rtc::KeepRefUntilDone(buffer));
}
diff --git a/media/engine/webrtcvideoframe.cc b/media/engine/webrtcvideoframe.cc
index 9983db9..145f265 100644
--- a/media/engine/webrtcvideoframe.cc
+++ b/media/engine/webrtcvideoframe.cc
@@ -80,15 +80,15 @@
}
const uint8_t* WebRtcVideoFrame::GetYPlane() const {
- return video_frame_buffer_ ? video_frame_buffer_->data(kYPlane) : nullptr;
+ return video_frame_buffer_ ? video_frame_buffer_->DataY() : nullptr;
}
const uint8_t* WebRtcVideoFrame::GetUPlane() const {
- return video_frame_buffer_ ? video_frame_buffer_->data(kUPlane) : nullptr;
+ return video_frame_buffer_ ? video_frame_buffer_->DataU() : nullptr;
}
const uint8_t* WebRtcVideoFrame::GetVPlane() const {
- return video_frame_buffer_ ? video_frame_buffer_->data(kVPlane) : nullptr;
+ return video_frame_buffer_ ? video_frame_buffer_->DataV() : nullptr;
}
uint8_t* WebRtcVideoFrame::GetYPlane() {
@@ -107,15 +107,15 @@
}
int32_t WebRtcVideoFrame::GetYPitch() const {
- return video_frame_buffer_ ? video_frame_buffer_->stride(kYPlane) : 0;
+ return video_frame_buffer_ ? video_frame_buffer_->StrideY() : 0;
}
int32_t WebRtcVideoFrame::GetUPitch() const {
- return video_frame_buffer_ ? video_frame_buffer_->stride(kUPlane) : 0;
+ return video_frame_buffer_ ? video_frame_buffer_->StrideU() : 0;
}
int32_t WebRtcVideoFrame::GetVPitch() const {
- return video_frame_buffer_ ? video_frame_buffer_->stride(kVPlane) : 0;
+ return video_frame_buffer_ ? video_frame_buffer_->StrideV() : 0;
}
bool WebRtcVideoFrame::IsExclusive() const {
diff --git a/test/fake_texture_frame.h b/test/fake_texture_frame.h
index 9575fae..15b70d8 100644
--- a/test/fake_texture_frame.h
+++ b/test/fake_texture_frame.h
@@ -42,9 +42,9 @@
new rtc::RefCountedObject<I420Buffer>(width_, height_));
int half_height = (height_ + 1) / 2;
int half_width = (width_ + 1) / 2;
- memset(buffer->MutableData(kYPlane), 0, height_ * width_);
- memset(buffer->MutableData(kUPlane), 0, half_height * half_width);
- memset(buffer->MutableData(kVPlane), 0, half_height * half_width);
+ memset(buffer->MutableDataY(), 0, height_ * width_);
+ memset(buffer->MutableDataU(), 0, half_height * half_width);
+ memset(buffer->MutableDataV(), 0, half_height * half_width);
return buffer;
}
};
diff --git a/test/frame_utils.cc b/test/frame_utils.cc
index 21daa44..0fad3ad 100644
--- a/test/frame_utils.cc
+++ b/test/frame_utils.cc
@@ -61,14 +61,14 @@
}
const int half_width = (f1->width() + 1) / 2;
const int half_height = (f1->height() + 1) / 2;
- return EqualPlane(f1->data(webrtc::kYPlane), f2->data(webrtc::kYPlane),
- f1->stride(webrtc::kYPlane), f2->stride(webrtc::kYPlane),
+ return EqualPlane(f1->DataY(), f2->DataY(),
+ f1->StrideY(), f2->StrideY(),
f1->width(), f1->height()) &&
- EqualPlane(f1->data(webrtc::kUPlane), f2->data(webrtc::kUPlane),
- f1->stride(webrtc::kUPlane), f2->stride(webrtc::kUPlane),
+ EqualPlane(f1->DataU(), f2->DataU(),
+ f1->StrideU(), f2->StrideU(),
half_width, half_height) &&
- EqualPlane(f1->data(webrtc::kVPlane), f2->data(webrtc::kVPlane),
- f1->stride(webrtc::kVPlane), f2->stride(webrtc::kVPlane),
+ EqualPlane(f1->DataV(), f2->DataV(),
+ f1->StrideV(), f2->StrideV(),
half_width, half_height);
}
