From fa0aa39fba04f392c6046ca4d52d82a5a83a3f01 Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Fri, 16 Nov 2018 09:54:32 +0100 Subject: [PATCH] Removes templating from CompositeMediaEngine. Usage of templates makes it harder for tooling to help the user. This can be experienced when trying to investigate compile failures and using editor tools to browse the code. This CL replaces usage of templates with injection of unique pointers to interfaces that implements the behavior that previously was assumed by the templated implementation. Bug: webrtc:9883 Change-Id: Ica17af9646f68a9b063988f9e85d6acc8ca37c10 Reviewed-on: https://webrtc-review.googlesource.com/c/106703 Commit-Queue: Sebastian Jansson Reviewed-by: Fredrik Solenberg Cr-Commit-Position: refs/heads/master@{#25668} --- media/BUILD.gn | 1 + media/base/fakemediaengine.cc | 9 ++- media/base/fakemediaengine.h | 3 +- media/base/mediaengine.cc | 81 +++++++++++++++++++ media/base/mediaengine.h | 72 ++++++----------- .../engine/nullwebrtcvideoengine_unittest.cc | 18 ++--- media/engine/webrtcmediaengine.cc | 52 +++++------- 7 files changed, 142 insertions(+), 94 deletions(-) diff --git a/media/BUILD.gn b/media/BUILD.gn index 3b13d09e18..e2d7704b44 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -374,6 +374,7 @@ rtc_static_library("rtc_audio_video") { "../rtc_base:stringutils", "../rtc_base/experiments:normalize_simulcast_size_experiment", "../system_wrappers", + "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/strings", "//third_party/abseil-cpp/absl/types:optional", ] diff --git a/media/base/fakemediaengine.cc b/media/base/fakemediaengine.cc index 55e38c6c4f..b48206fd77 100644 --- a/media/base/fakemediaengine.cc +++ b/media/base/fakemediaengine.cc @@ -12,6 +12,7 @@ #include +#include "absl/memory/memory.h" #include "absl/strings/match.h" #include "rtc_base/checks.h" @@ -552,10 +553,10 @@ bool FakeVideoEngine::SetCapture(bool capture) { } FakeMediaEngine::FakeMediaEngine() - : CompositeMediaEngine(std::tuple<>(), - std::tuple<>()), - voice_(&voice()), - video_(&video()) {} + : CompositeMediaEngine(absl::make_unique(), + absl::make_unique()), + voice_(static_cast(&voice())), + video_(static_cast(&video())) {} FakeMediaEngine::~FakeMediaEngine() {} void FakeMediaEngine::SetAudioCodecs(const std::vector& codecs) { voice_->SetCodecs(codecs); diff --git a/media/base/fakemediaengine.h b/media/base/fakemediaengine.h index 4821ad4395..d7b44c598f 100644 --- a/media/base/fakemediaengine.h +++ b/media/base/fakemediaengine.h @@ -554,8 +554,7 @@ class FakeVideoEngine : public VideoEngineInterface { friend class FakeMediaEngine; }; -class FakeMediaEngine - : public CompositeMediaEngine { +class FakeMediaEngine : public CompositeMediaEngine { public: FakeMediaEngine(); diff --git a/media/base/mediaengine.cc b/media/base/mediaengine.cc index 76cac4b438..94ebcad81b 100644 --- a/media/base/mediaengine.cc +++ b/media/base/mediaengine.cc @@ -10,6 +10,8 @@ #include "media/base/mediaengine.h" +#include + #include "api/video/video_bitrate_allocation.h" #include "rtc_base/stringencode.h" @@ -104,4 +106,83 @@ webrtc::RTCError ValidateRtpParameters( return webrtc::RTCError::OK(); } +CompositeMediaEngine::CompositeMediaEngine( + std::unique_ptr voice_engine, + std::unique_ptr video_engine) + : voice_engine_(std::move(voice_engine)), + video_engine_(std::move(video_engine)) {} + +CompositeMediaEngine::~CompositeMediaEngine() = default; + +bool CompositeMediaEngine::Init() { + voice().Init(); + return true; +} + +rtc::scoped_refptr CompositeMediaEngine::GetAudioState() + const { + return voice().GetAudioState(); +} + +VoiceMediaChannel* CompositeMediaEngine::CreateChannel( + webrtc::Call* call, + const MediaConfig& config, + const AudioOptions& options, + const webrtc::CryptoOptions& crypto_options) { + return voice().CreateMediaChannel(call, config, options, crypto_options); +} + +VideoMediaChannel* CompositeMediaEngine::CreateVideoChannel( + webrtc::Call* call, + const MediaConfig& config, + const VideoOptions& options, + const webrtc::CryptoOptions& crypto_options) { + return video().CreateMediaChannel(call, config, options, crypto_options); +} + +const std::vector& CompositeMediaEngine::audio_send_codecs() { + return voice().send_codecs(); +} + +const std::vector& CompositeMediaEngine::audio_recv_codecs() { + return voice().recv_codecs(); +} + +RtpCapabilities CompositeMediaEngine::GetAudioCapabilities() { + return voice().GetCapabilities(); +} + +std::vector CompositeMediaEngine::video_codecs() { + return video().codecs(); +} + +RtpCapabilities CompositeMediaEngine::GetVideoCapabilities() { + return video().GetCapabilities(); +} + +bool CompositeMediaEngine::StartAecDump(rtc::PlatformFile file, + int64_t max_size_bytes) { + return voice().StartAecDump(file, max_size_bytes); +} + +void CompositeMediaEngine::StopAecDump() { + voice().StopAecDump(); +} + +VoiceEngineInterface& CompositeMediaEngine::voice() { + return *voice_engine_.get(); +} + +VideoEngineInterface& CompositeMediaEngine::video() { + return *video_engine_.get(); +} + +const VoiceEngineInterface& CompositeMediaEngine::voice() const { + return *voice_engine_.get(); +} + +const VideoEngineInterface& CompositeMediaEngine::video() const { + return *video_engine_.get(); +} + }; // namespace cricket diff --git a/media/base/mediaengine.h b/media/base/mediaengine.h index eede3af624..1ddb36c9cd 100644 --- a/media/base/mediaengine.h +++ b/media/base/mediaengine.h @@ -15,9 +15,8 @@ #include #endif +#include #include -#include -#include #include #include "api/audio_codecs/audio_decoder_factory.h" @@ -147,68 +146,45 @@ class MediaEngineInterface { // CompositeMediaEngine constructs a MediaEngine from separate // voice and video engine classes. -template class CompositeMediaEngine : public MediaEngineInterface { public: - template - CompositeMediaEngine(std::tuple first_args, - std::tuple second_args) - : engines_(std::piecewise_construct, - std::move(first_args), - std::move(second_args)) {} + CompositeMediaEngine(std::unique_ptr audio_engine, + std::unique_ptr video_engine); + ~CompositeMediaEngine() override; + bool Init() override; - virtual ~CompositeMediaEngine() {} - virtual bool Init() { - voice().Init(); - return true; - } - - virtual rtc::scoped_refptr GetAudioState() const { - return voice().GetAudioState(); - } - virtual VoiceMediaChannel* CreateChannel( + rtc::scoped_refptr GetAudioState() const override; + VoiceMediaChannel* CreateChannel( webrtc::Call* call, const MediaConfig& config, const AudioOptions& options, - const webrtc::CryptoOptions& crypto_options) { - return voice().CreateMediaChannel(call, config, options, crypto_options); - } - virtual VideoMediaChannel* CreateVideoChannel( + const webrtc::CryptoOptions& crypto_options) override; + + VideoMediaChannel* CreateVideoChannel( webrtc::Call* call, const MediaConfig& config, const VideoOptions& options, - const webrtc::CryptoOptions& crypto_options) { - return video().CreateMediaChannel(call, config, options, crypto_options); - } + const webrtc::CryptoOptions& crypto_options) override; - virtual const std::vector& audio_send_codecs() { - return voice().send_codecs(); - } - virtual const std::vector& audio_recv_codecs() { - return voice().recv_codecs(); - } - virtual RtpCapabilities GetAudioCapabilities() { - return voice().GetCapabilities(); - } - virtual std::vector video_codecs() { return video().codecs(); } - virtual RtpCapabilities GetVideoCapabilities() { - return video().GetCapabilities(); - } + const std::vector& audio_send_codecs() override; + const std::vector& audio_recv_codecs() override; + RtpCapabilities GetAudioCapabilities() override; - virtual bool StartAecDump(rtc::PlatformFile file, int64_t max_size_bytes) { - return voice().StartAecDump(file, max_size_bytes); - } + std::vector video_codecs() override; + RtpCapabilities GetVideoCapabilities() override; - virtual void StopAecDump() { voice().StopAecDump(); } + bool StartAecDump(rtc::PlatformFile file, int64_t max_size_bytes) override; + void StopAecDump() override; protected: - VOICE& voice() { return engines_.first; } - VIDEO& video() { return engines_.second; } - const VOICE& voice() const { return engines_.first; } - const VIDEO& video() const { return engines_.second; } + VoiceEngineInterface& voice(); + VideoEngineInterface& video(); + const VoiceEngineInterface& voice() const; + const VideoEngineInterface& video() const; private: - std::pair engines_; + std::unique_ptr voice_engine_; + std::unique_ptr video_engine_; }; enum DataChannelType { diff --git a/media/engine/nullwebrtcvideoengine_unittest.cc b/media/engine/nullwebrtcvideoengine_unittest.cc index 6c740b8c8c..e718d1bb69 100644 --- a/media/engine/nullwebrtcvideoengine_unittest.cc +++ b/media/engine/nullwebrtcvideoengine_unittest.cc @@ -9,6 +9,7 @@ */ #include "media/engine/nullwebrtcvideoengine.h" +#include "absl/memory/memory.h" #include "media/engine/webrtcvoiceengine.h" #include "modules/audio_device/include/mock_audio_device.h" #include "modules/audio_processing/include/audio_processing.h" @@ -18,8 +19,7 @@ namespace cricket { -class WebRtcMediaEngineNullVideo - : public CompositeMediaEngine { +class WebRtcMediaEngineNullVideo : public CompositeMediaEngine { public: WebRtcMediaEngineNullVideo( webrtc::AudioDeviceModule* adm, @@ -27,13 +27,13 @@ class WebRtcMediaEngineNullVideo audio_encoder_factory, const rtc::scoped_refptr& audio_decoder_factory) - : CompositeMediaEngine( - std::forward_as_tuple(adm, - audio_encoder_factory, - audio_decoder_factory, - nullptr, - webrtc::AudioProcessingBuilder().Create()), - std::forward_as_tuple()) {} + : CompositeMediaEngine(absl::make_unique( + adm, + audio_encoder_factory, + audio_decoder_factory, + nullptr, + webrtc::AudioProcessingBuilder().Create()), + absl::make_unique()) {} }; // Simple test to check if NullWebRtcVideoEngine implements the methods diff --git a/media/engine/webrtcmediaengine.cc b/media/engine/webrtcmediaengine.cc index 2dc4c0bc69..bc7d3c3e12 100644 --- a/media/engine/webrtcmediaengine.cc +++ b/media/engine/webrtcmediaengine.cc @@ -11,10 +11,9 @@ #include "media/engine/webrtcmediaengine.h" #include -#include -#include #include +#include "absl/memory/memory.h" #include "api/video/builtin_video_bitrate_allocator_factory.h" #include "api/video_codecs/video_decoder_factory.h" #include "api/video_codecs/video_encoder_factory.h" @@ -43,23 +42,20 @@ MediaEngineInterface* CreateWebRtcMediaEngine( video_bitrate_allocator_factory, rtc::scoped_refptr audio_mixer, rtc::scoped_refptr audio_processing) { + std::unique_ptr video_engine; #ifdef HAVE_WEBRTC_VIDEO - typedef WebRtcVideoEngine VideoEngine; - std::tuple, - std::unique_ptr, - std::unique_ptr> - video_args( - (std::unique_ptr(video_encoder_factory)), - (std::unique_ptr(video_decoder_factory)), - (std::move(video_bitrate_allocator_factory))); + video_engine = absl::make_unique( + std::unique_ptr(video_encoder_factory), + std::unique_ptr(video_decoder_factory), + std::move(video_bitrate_allocator_factory)); #else - typedef NullWebRtcVideoEngine VideoEngine; - std::tuple<> video_args; + video_engine = absl::make_unique(); #endif - return new CompositeMediaEngine( - std::forward_as_tuple(adm, audio_encoder_factory, audio_decoder_factory, - audio_mixer, audio_processing), - std::move(video_args)); + return new CompositeMediaEngine( + absl::make_unique(adm, audio_encoder_factory, + audio_decoder_factory, audio_mixer, + audio_processing), + std::move(video_engine)); } } // namespace @@ -156,23 +152,17 @@ std::unique_ptr WebRtcMediaEngineFactory::Create( rtc::scoped_refptr audio_mixer, rtc::scoped_refptr audio_processing) { #ifdef HAVE_WEBRTC_VIDEO - typedef WebRtcVideoEngine VideoEngine; - std::tuple, - std::unique_ptr, - std::unique_ptr> - video_args(std::move(video_encoder_factory), - std::move(video_decoder_factory), - std::move(video_bitrate_allocator_factory)); + auto video_engine = absl::make_unique( + std::move(video_encoder_factory), std::move(video_decoder_factory), + std::move(video_bitrate_allocator_factory)); #else - typedef NullWebRtcVideoEngine VideoEngine; - std::tuple<> video_args; + auto video_engine = absl::make_unique(); #endif - return std::unique_ptr( - new CompositeMediaEngine( - std::forward_as_tuple(adm, audio_encoder_factory, - audio_decoder_factory, audio_mixer, - audio_processing), - std::move(video_args))); + return std::unique_ptr(new CompositeMediaEngine( + absl::make_unique(adm, audio_encoder_factory, + audio_decoder_factory, audio_mixer, + audio_processing), + std::move(video_engine))); } namespace {