From eaf4a1e10372e1a5a09f8669fca6ae61b156837f Mon Sep 17 00:00:00 2001 From: magjed Date: Tue, 30 May 2017 01:21:59 -0700 Subject: [PATCH] Add separate base classes for I420 and I444 buffers Previously, the base class PlanarYuvBuffer was used directly. Having separate base classes will allow us to improve type safety in some places. BUG=webrtc:7632 TBR=stefan@webrtc.org Review-Url: https://codereview.webrtc.org/2914463002 Cr-Commit-Position: refs/heads/master@{#18317} --- webrtc/api/video/i420_buffer.cc | 4 - webrtc/api/video/i420_buffer.h | 3 +- webrtc/api/video/video_frame_buffer.cc | 102 ++++++++---------- webrtc/api/video/video_frame_buffer.h | 41 +++++-- .../common_video/include/video_frame_buffer.h | 4 +- webrtc/common_video/video_frame_buffer.cc | 4 - 6 files changed, 82 insertions(+), 76 deletions(-) diff --git a/webrtc/api/video/i420_buffer.cc b/webrtc/api/video/i420_buffer.cc index 51d83c9df5..ae770c6039 100644 --- a/webrtc/api/video/i420_buffer.cc +++ b/webrtc/api/video/i420_buffer.cc @@ -136,10 +136,6 @@ void I420Buffer::InitializeData() { I420DataSize(height_, stride_y_, stride_u_, stride_v_)); } -VideoFrameBuffer::Type I420Buffer::type() const { - return Type::kI420; -} - int I420Buffer::width() const { return width_; } diff --git a/webrtc/api/video/i420_buffer.h b/webrtc/api/video/i420_buffer.h index 20a84a1a53..c849ab22c0 100644 --- a/webrtc/api/video/i420_buffer.h +++ b/webrtc/api/video/i420_buffer.h @@ -20,7 +20,7 @@ namespace webrtc { // Plain I420 buffer in standard memory. -class I420Buffer : public PlanarYuvBuffer { +class I420Buffer : public I420BufferInterface { public: static rtc::scoped_refptr Create(int width, int height); static rtc::scoped_refptr Create(int width, @@ -53,7 +53,6 @@ class I420Buffer : public PlanarYuvBuffer { // are resolved in a better way. Or in the mean time, use SetBlack. void InitializeData(); - Type type() const override; int width() const override; int height() const override; const uint8_t* DataY() const override; diff --git a/webrtc/api/video/video_frame_buffer.cc b/webrtc/api/video/video_frame_buffer.cc index b4b30a1268..bb3171768c 100644 --- a/webrtc/api/video/video_frame_buffer.cc +++ b/webrtc/api/video/video_frame_buffer.cc @@ -10,7 +10,7 @@ #include "webrtc/api/video/video_frame_buffer.h" -#include "libyuv/convert_from.h" +#include "libyuv/convert.h" #include "webrtc/api/video/i420_buffer.h" #include "webrtc/base/checks.h" @@ -21,16 +21,14 @@ namespace { // TODO(magjed): Remove this class. It is only used for providing a default // implementation of ToI420() until external clients are updated. ToI420() will // then be made pure virtual. This adapter adapts a VideoFrameBuffer (which is -// expected to be in I420 format) to the PlanarYuvBuffer interface. The reason -// this is needed is because of the return type mismatch in NativeToI420Buffer -// (returns VideoFrameBuffer) vs ToI420 (returns PlanarYuvBuffer). -class PlanarYuvBufferAdapter : public PlanarYuvBuffer { +// expected to be in I420 format) to I420BufferInterface. The reason this is +// needed is because of the return type mismatch in NativeToI420Buffer (returns +// VideoFrameBuffer) vs ToI420 (returns I420BufferInterface). +class I420InterfaceAdapter : public I420BufferInterface { public: - explicit PlanarYuvBufferAdapter(rtc::scoped_refptr buffer) + explicit I420InterfaceAdapter(rtc::scoped_refptr buffer) : buffer_(buffer) {} - Type type() const override { return Type::kI420; } - int width() const override { return buffer_->width(); } int height() const override { return buffer_->height(); } @@ -89,65 +87,59 @@ rtc::scoped_refptr VideoFrameBuffer::NativeToI420Buffer() { return ToI420(); } -rtc::scoped_refptr VideoFrameBuffer::ToI420() { - return new rtc::RefCountedObject( - NativeToI420Buffer()); +rtc::scoped_refptr VideoFrameBuffer::ToI420() { + return new rtc::RefCountedObject(NativeToI420Buffer()); } -rtc::scoped_refptr VideoFrameBuffer::GetI420() { +rtc::scoped_refptr VideoFrameBuffer::GetI420() { RTC_CHECK(type() == Type::kI420); - // TODO(magjed): static_cast to PlanarYuvBuffer instead once external clients - // are updated. - return new rtc::RefCountedObject(this); + // TODO(magjed): static_cast to I420BufferInterface instead once external + // clients are updated. + return new rtc::RefCountedObject(this); } -rtc::scoped_refptr VideoFrameBuffer::GetI444() { +rtc::scoped_refptr VideoFrameBuffer::GetI444() { RTC_CHECK(type() == Type::kI444); - return static_cast(this); + return static_cast(this); } -rtc::scoped_refptr PlanarYuvBuffer::ToI420() { - switch (type()) { - case Type::kI420: - return this; - case Type::kI444: { - rtc::scoped_refptr i420_buffer = - I420Buffer::Create(width(), height()); - libyuv::I420ToI444(DataY(), StrideY(), DataU(), StrideU(), DataV(), - StrideV(), i420_buffer->MutableDataY(), - i420_buffer->StrideY(), i420_buffer->MutableDataU(), - i420_buffer->StrideU(), i420_buffer->MutableDataV(), - i420_buffer->StrideV(), width(), height()); - return i420_buffer; - } - default: - RTC_NOTREACHED(); - return nullptr; - } +VideoFrameBuffer::Type I420BufferInterface::type() const { + return Type::kI420; } -int PlanarYuvBuffer::ChromaWidth() const { - switch (type()) { - case Type::kI420: - return (width() + 1) / 2; - case Type::kI444: - return width(); - default: - RTC_NOTREACHED(); - return 0; - } +int I420BufferInterface::ChromaWidth() const { + return (width() + 1) / 2; } -int PlanarYuvBuffer::ChromaHeight() const { - switch (type()) { - case Type::kI420: - return (height() + 1) / 2; - case Type::kI444: - return height(); - default: - RTC_NOTREACHED(); - return 0; - } +int I420BufferInterface::ChromaHeight() const { + return (height() + 1) / 2; +} + +rtc::scoped_refptr I420BufferInterface::ToI420() { + return this; +} + +VideoFrameBuffer::Type I444BufferInterface::type() const { + return Type::kI444; +} + +int I444BufferInterface::ChromaWidth() const { + return width(); +} + +int I444BufferInterface::ChromaHeight() const { + return height(); +} + +rtc::scoped_refptr I444BufferInterface::ToI420() { + rtc::scoped_refptr i420_buffer = + I420Buffer::Create(width(), height()); + libyuv::I444ToI420(DataY(), StrideY(), DataU(), StrideU(), DataV(), StrideV(), + i420_buffer->MutableDataY(), i420_buffer->StrideY(), + i420_buffer->MutableDataU(), i420_buffer->StrideU(), + i420_buffer->MutableDataV(), i420_buffer->StrideV(), + width(), height()); + return i420_buffer; } } // namespace webrtc diff --git a/webrtc/api/video/video_frame_buffer.h b/webrtc/api/video/video_frame_buffer.h index 6d0c99107c..13005afc8a 100644 --- a/webrtc/api/video/video_frame_buffer.h +++ b/webrtc/api/video/video_frame_buffer.h @@ -18,7 +18,8 @@ namespace webrtc { -class PlanarYuvBuffer; +class I420BufferInterface; +class I444BufferInterface; // Base class for frame buffers of different types of pixel format and storage. // The tag in type() indicates how the data is represented, and each type is @@ -58,12 +59,12 @@ class VideoFrameBuffer : public rtc::RefCountInterface { // in another format, a conversion will take place. All implementations must // provide a fallback to I420 for compatibility with e.g. the internal WebRTC // software encoders. - virtual rtc::scoped_refptr ToI420(); + virtual rtc::scoped_refptr ToI420(); // These functions should only be called if type() is of the correct type. // Calling with a different type will result in a crash. - rtc::scoped_refptr GetI420(); - rtc::scoped_refptr GetI444(); + rtc::scoped_refptr GetI420(); + rtc::scoped_refptr GetI444(); // Deprecated - use ToI420() first instead. // Returns pointer to the pixel data for a given plane. The memory is owned by @@ -92,8 +93,8 @@ class VideoFrameBuffer : public rtc::RefCountInterface { // This interface represents Type::kI420 and Type::kI444. class PlanarYuvBuffer : public VideoFrameBuffer { public: - int ChromaWidth() const; - int ChromaHeight() const; + virtual int ChromaWidth() const = 0; + virtual int ChromaHeight() const = 0; // 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. @@ -106,12 +107,36 @@ class PlanarYuvBuffer : public VideoFrameBuffer { int StrideU() const override = 0; int StrideV() const override = 0; - rtc::scoped_refptr ToI420() override; - protected: ~PlanarYuvBuffer() override {} }; +class I420BufferInterface : public PlanarYuvBuffer { + public: + Type type() const final; + + int ChromaWidth() const final; + int ChromaHeight() const final; + + rtc::scoped_refptr ToI420() final; + + protected: + ~I420BufferInterface() override {} +}; + +class I444BufferInterface : public PlanarYuvBuffer { + public: + Type type() const final; + + int ChromaWidth() const final; + int ChromaHeight() const final; + + rtc::scoped_refptr ToI420() final; + + protected: + ~I444BufferInterface() override {} +}; + } // namespace webrtc #endif // WEBRTC_API_VIDEO_VIDEO_FRAME_BUFFER_H_ diff --git a/webrtc/common_video/include/video_frame_buffer.h b/webrtc/common_video/include/video_frame_buffer.h index 7e2fcf99e1..6b578bce49 100644 --- a/webrtc/common_video/include/video_frame_buffer.h +++ b/webrtc/common_video/include/video_frame_buffer.h @@ -45,7 +45,7 @@ class NativeHandleBuffer : public VideoFrameBuffer { const int height_; }; -class WrappedI420Buffer : public PlanarYuvBuffer { +class WrappedI420Buffer : public I420BufferInterface { public: WrappedI420Buffer(int width, int height, @@ -56,8 +56,6 @@ class WrappedI420Buffer : public PlanarYuvBuffer { const uint8_t* v_plane, int v_stride, const rtc::Callback0& no_longer_used); - Type type() const override; - int width() const override; int height() const override; diff --git a/webrtc/common_video/video_frame_buffer.cc b/webrtc/common_video/video_frame_buffer.cc index 0322dd1fd5..5f339a5f35 100644 --- a/webrtc/common_video/video_frame_buffer.cc +++ b/webrtc/common_video/video_frame_buffer.cc @@ -96,10 +96,6 @@ WrappedI420Buffer::~WrappedI420Buffer() { no_longer_used_cb_(); } -VideoFrameBuffer::Type WrappedI420Buffer::type() const { - return Type::kI420; -} - int WrappedI420Buffer::width() const { return width_; }