From b5b5bcee722c8b646e545ca6b85518aa8aaa1eda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrik=20H=C3=B6glund?= Date: Mon, 13 Nov 2017 10:19:58 +0100 Subject: [PATCH] Separate i420 and i444 implementations to separate targets. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This means we can properly declare the dependency between libjingle_peerconnection_api and video_frame_api. i420 pulls in system_wrappers, which can't be a dependency of the public API. Plan: 1) Land this CL + send out PSA 2) Make all direct users of i420_buffer depend on the new video_frame_api_i420 target 3) Move i420_buffer.cc to the new target 4) Make libjingle_peerconnection_api depend on video_frame_api, since it no longer contains i420 code Bug: webrtc:7504 Change-Id: I30d90f2ac7af53748859bbde8aed92386d5501f9 Reviewed-on: https://webrtc-review.googlesource.com/9382 Reviewed-by: Niels Moller Reviewed-by: Karl Wiberg Commit-Queue: Patrik Höglund Cr-Commit-Position: refs/heads/master@{#20656} --- api/BUILD.gn | 22 ++++- api/video/video_frame_buffer.cc | 13 --- api/video/video_frame_buffer.h | 2 - common_video/BUILD.gn | 2 + common_video/video_frame_buffer.cc | 125 +++++++++++++++++------------ examples/BUILD.gn | 4 +- media/BUILD.gn | 3 + modules/BUILD.gn | 1 + modules/video_capture/BUILD.gn | 2 + modules/video_coding/BUILD.gn | 8 ++ test/BUILD.gn | 4 + video/BUILD.gn | 2 + 12 files changed, 116 insertions(+), 72 deletions(-) diff --git a/api/BUILD.gn b/api/BUILD.gn index bad3a51b30..05b8902c0f 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -81,15 +81,16 @@ rtc_static_library("libjingle_peerconnection_api") { ] deps = [ - # Basically, don't add stuff here. You might break sensitive downstream - # targets like pnacl. API should not depend on anything, really. All these - # should go away, in time. ":optional", ":rtc_stats_api", + "audio_codecs:audio_codecs_api", + + # Basically, don't add stuff here. You might break sensitive downstream + # targets like pnacl. API should not depend on anything outside of this + # file, really. All these should arguably go away in time. "..:webrtc_common", "../rtc_base:rtc_base", "../rtc_base:rtc_base_approved", - "audio_codecs:audio_codecs_api", ] # This is needed until bugs.webrtc.org/7504 is removed so this target can @@ -214,6 +215,8 @@ rtc_source_set("transport_api") { rtc_source_set("video_frame_api") { sources = [ + # TODO(phoglund): move i420 files to video_frame_api_i420 after updating + # downstream. See bugs.webrtc.org/7504. "video/i420_buffer.cc", "video/i420_buffer.h", "video/video_content_type.cc", @@ -245,6 +248,17 @@ rtc_source_set("video_frame_api") { } } +rtc_source_set("video_frame_api_i420") { + sources = [ + "video/i420_buffer.h", + ] + deps = [ + ":video_frame_api", + "../rtc_base:rtc_base_approved", + "../system_wrappers", + ] +} + rtc_source_set("array_view") { sources = [ "array_view.h", diff --git a/api/video/video_frame_buffer.cc b/api/video/video_frame_buffer.cc index b231745e11..867f249fe6 100644 --- a/api/video/video_frame_buffer.cc +++ b/api/video/video_frame_buffer.cc @@ -10,8 +10,6 @@ #include "api/video/video_frame_buffer.h" -#include "libyuv/convert.h" -#include "api/video/i420_buffer.h" #include "rtc_base/checks.h" namespace webrtc { @@ -79,15 +77,4 @@ 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/api/video/video_frame_buffer.h b/api/video/video_frame_buffer.h index e656edd7e6..2be7e0bb9f 100644 --- a/api/video/video_frame_buffer.h +++ b/api/video/video_frame_buffer.h @@ -129,8 +129,6 @@ class I444BufferInterface : public PlanarYuvBuffer { int ChromaWidth() const final; int ChromaHeight() const final; - rtc::scoped_refptr ToI420() final; - protected: ~I444BufferInterface() override {} }; diff --git a/common_video/BUILD.gn b/common_video/BUILD.gn index c7da8d6588..669fa7a541 100644 --- a/common_video/BUILD.gn +++ b/common_video/BUILD.gn @@ -58,6 +58,7 @@ rtc_static_library("common_video") { deps = [ "..:webrtc_common", "../api:optional", + "../api:video_frame_api_i420", "../media:rtc_h264_profile_id", "../modules:module_api", "../rtc_base:rtc_base", @@ -115,6 +116,7 @@ if (rtc_include_tests) { deps = [ ":common_video", + "../api:video_frame_api_i420", "../modules/video_capture:video_capture", "../rtc_base:rtc_base", "../rtc_base:rtc_base_approved", diff --git a/common_video/video_frame_buffer.cc b/common_video/video_frame_buffer.cc index 9c8955cbd7..88c8ca4a5d 100644 --- a/common_video/video_frame_buffer.cc +++ b/common_video/video_frame_buffer.cc @@ -13,6 +13,7 @@ #include +#include "api/video/i420_buffer.h" #include "libyuv/convert.h" #include "libyuv/planar_functions.h" #include "libyuv/scale.h" @@ -21,57 +22,7 @@ namespace webrtc { -WrappedI420Buffer::WrappedI420Buffer(int width, - int height, - const uint8_t* y_plane, - int y_stride, - const uint8_t* u_plane, - int u_stride, - const uint8_t* v_plane, - int v_stride, - const rtc::Callback0& no_longer_used) - : width_(width), - height_(height), - y_plane_(y_plane), - u_plane_(u_plane), - v_plane_(v_plane), - y_stride_(y_stride), - u_stride_(u_stride), - v_stride_(v_stride), - no_longer_used_cb_(no_longer_used) { -} - -WrappedI420Buffer::~WrappedI420Buffer() { - no_longer_used_cb_(); -} - -int WrappedI420Buffer::width() const { - return width_; -} - -int WrappedI420Buffer::height() const { - return height_; -} - -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::StrideY() const { - return y_stride_; -} -int WrappedI420Buffer::StrideU() const { - return u_stride_; -} -int WrappedI420Buffer::StrideV() const { - return v_stride_; -} +namespace { // Template to implement a wrapped buffer for a I4??BufferInterface. template @@ -163,6 +114,76 @@ class WrappedYuvaBuffer : public WrappedYuvBuffer { const int a_stride_; }; +class I444BufferBase : public I444BufferInterface { + public: + rtc::scoped_refptr ToI420() final; +}; + +rtc::scoped_refptr I444BufferBase::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 + +WrappedI420Buffer::WrappedI420Buffer(int width, + int height, + const uint8_t* y_plane, + int y_stride, + const uint8_t* u_plane, + int u_stride, + const uint8_t* v_plane, + int v_stride, + const rtc::Callback0& no_longer_used) + : width_(width), + height_(height), + y_plane_(y_plane), + u_plane_(u_plane), + v_plane_(v_plane), + y_stride_(y_stride), + u_stride_(u_stride), + v_stride_(v_stride), + no_longer_used_cb_(no_longer_used) { +} + +WrappedI420Buffer::~WrappedI420Buffer() { + no_longer_used_cb_(); +} + +int WrappedI420Buffer::width() const { + return width_; +} + +int WrappedI420Buffer::height() const { + return height_; +} + +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::StrideY() const { + return y_stride_; +} +int WrappedI420Buffer::StrideU() const { + return u_stride_; +} +int WrappedI420Buffer::StrideV() const { + return v_stride_; +} + rtc::scoped_refptr WrapI420Buffer( int width, int height, @@ -208,7 +229,7 @@ rtc::scoped_refptr WrapI444Buffer( int v_stride, const rtc::Callback0& no_longer_used) { return rtc::scoped_refptr( - new rtc::RefCountedObject>( + new rtc::RefCountedObject>( width, height, y_plane, y_stride, u_plane, u_stride, v_plane, v_stride, no_longer_used)); } diff --git a/examples/BUILD.gn b/examples/BUILD.gn index 01d73a48dc..740ebe5e90 100644 --- a/examples/BUILD.gn +++ b/examples/BUILD.gn @@ -492,7 +492,9 @@ if (is_linux || is_win) { # Suppress warnings from the Chromium Clang plugin (bugs.webrtc.org/163). suppressed_configs += [ "//build/config/clang:find_bad_constructs" ] } - deps = [] + deps = [ + "../api:video_frame_api_i420", + ] if (is_win) { sources += [ "peerconnection/client/flagdefs.h", diff --git a/media/BUILD.gn b/media/BUILD.gn index 2af6121785..5370791930 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -135,6 +135,7 @@ rtc_static_library("rtc_audio_video") { defines = [] libs = [] deps = [ + "../api:video_frame_api_i420", "../modules/video_coding:video_coding_utility", ] sources = [ @@ -339,6 +340,7 @@ if (rtc_include_tests) { include_dirs = [] public_deps = [] deps = [ + "../api:video_frame_api_i420", "../call:video_stream_api", "../modules/audio_coding:rent_a_codec", "../modules/audio_processing:audio_processing", @@ -440,6 +442,7 @@ if (rtc_include_tests) { defines = [] deps = [ + "../api:video_frame_api_i420", "../pc:rtc_pc", "../test:field_trial", ] diff --git a/modules/BUILD.gn b/modules/BUILD.gn index 5d4987e8c7..d534f569be 100644 --- a/modules/BUILD.gn +++ b/modules/BUILD.gn @@ -37,6 +37,7 @@ rtc_source_set("module_api") { "..:webrtc_common", "../api:optional", "../api:video_frame_api", + "../api:video_frame_api_i420", "../rtc_base:rtc_base_approved", "video_coding:codec_globals_headers", ] diff --git a/modules/video_capture/BUILD.gn b/modules/video_capture/BUILD.gn index 5176862c42..31a4b65a3a 100644 --- a/modules/video_capture/BUILD.gn +++ b/modules/video_capture/BUILD.gn @@ -28,6 +28,7 @@ rtc_static_library("video_capture_module") { deps = [ "..:module_api", "../..:webrtc_common", + "../../api:video_frame_api_i420", "../../common_video", "../../rtc_base:rtc_base_approved", "../../system_wrappers", @@ -200,6 +201,7 @@ if (!build_with_chromium) { deps = [ ":video_capture_internal_impl", ":video_capture_module", + "../../api:video_frame_api_i420", "../../common_video:common_video", "../../rtc_base:rtc_base_approved", "../../system_wrappers:system_wrappers", diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn index d561bf60f9..728ed80bd1 100644 --- a/modules/video_coding/BUILD.gn +++ b/modules/video_coding/BUILD.gn @@ -99,6 +99,7 @@ rtc_static_library("video_coding") { "..:module_api", "../..:webrtc_common", "../../api:optional", + "../../api:video_frame_api_i420", "../../call:video_stream_api", "../../common_video", "../../rtc_base:rtc_base", @@ -189,6 +190,7 @@ rtc_static_library("webrtc_h264") { defines = [] deps = [ ":video_coding_utility", + "../../api:video_frame_api_i420", "../../api/video_codecs:video_codecs_api", "../../media:rtc_media_base", "../../rtc_base:rtc_base_approved", @@ -229,6 +231,7 @@ rtc_static_library("webrtc_i420") { deps = [ ":video_coding_utility", "../..:webrtc_common", + "../../api:video_frame_api_i420", "../../common_video:common_video", "../../rtc_base:rtc_base_approved", "../../system_wrappers", @@ -252,6 +255,7 @@ rtc_static_library("webrtc_stereo") { ":video_coding_utility", "..:module_api", "../..:webrtc_common", + "../../api:video_frame_api_i420", "../../api/video_codecs:video_codecs_api", "../../common_video:common_video", "../../rtc_base:rtc_base_approved", @@ -376,6 +380,7 @@ if (rtc_include_tests) { ":video_coding", ":webrtc_vp8", "../../api:video_frame_api", + "../../api:video_frame_api_i420", "../../common_video:common_video", "../../rtc_base:rtc_base_approved", "../../test:test_support", @@ -409,6 +414,7 @@ if (rtc_include_tests) { ":video_coding_utility", ":webrtc_vp8", "../..:webrtc_common", + "../../api:video_frame_api_i420", "../../api/video_codecs:video_codecs_api", "../../common_video:common_video", "../../rtc_base:rtc_base_approved", @@ -471,6 +477,7 @@ if (rtc_include_tests) { "../../api:mock_video_codec_factory", "../../api:optional", "../../api:video_frame_api", + "../../api:video_frame_api_i420", "../../common_video", "../../media:rtc_audio_video", "../../media:rtc_media_base", @@ -586,6 +593,7 @@ if (rtc_include_tests) { "..:module_api", "../..:webrtc_common", "../../api:video_frame_api", + "../../api:video_frame_api_i420", "../../api/video_codecs:video_codecs_api", "../../common_video:common_video", "../../rtc_base:rtc_base", diff --git a/test/BUILD.gn b/test/BUILD.gn index e00bbef145..8a1c823928 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -61,6 +61,7 @@ rtc_source_set("video_test_common") { deps = [ "..:webrtc_common", "../api:optional", + "../api:video_frame_api_i420", "../api/video_codecs:video_codecs_api", "../call:video_stream_api", "../common_video", @@ -212,6 +213,7 @@ if (rtc_include_tests) { ":video_test_common", "..:webrtc_common", "../api:video_frame_api", + "../api:video_frame_api_i420", "../common_video", "../rtc_base:rtc_base_approved", "../system_wrappers", @@ -285,6 +287,7 @@ if (rtc_include_tests) { ":fake_audio_device", ":rtp_test_utils", "../api:video_frame_api", + "../api:video_frame_api_i420", "../call:call_interfaces", "../common_audio", "../modules/rtp_rtcp", @@ -553,6 +556,7 @@ rtc_source_set("test_common") { "..:webrtc_common", "../api:transport_api", "../api:video_frame_api", + "../api:video_frame_api_i420", "../api/audio_codecs:builtin_audio_decoder_factory", "../api/audio_codecs:builtin_audio_encoder_factory", "../api/video_codecs:video_codecs_api", diff --git a/video/BUILD.gn b/video/BUILD.gn index 8799a8467e..e75a446fd4 100644 --- a/video/BUILD.gn +++ b/video/BUILD.gn @@ -57,6 +57,7 @@ rtc_static_library("video") { "..:webrtc_common", "../api:optional", "../api:transport_api", + "../api:video_frame_api_i420", "../api/video_codecs:video_codecs_api", "../call:bitrate_allocator", "../call:call_interfaces", @@ -273,6 +274,7 @@ if (rtc_include_tests) { ":video", "../api:optional", "../api:video_frame_api", + "../api:video_frame_api_i420", "../api/video_codecs:video_codecs_api", "../call:call_interfaces", "../call:mock_rtp_interfaces",