Deprecate the static RtpRtcp::Create() method.

The method is being used externally to create instances
of the deprecated internal implementation.

Instead, I'm moving how we instantiate the internal implementation into
the implementation itself and move towards keeping the interface
separate from a single implementation.

Change-Id: I743aa86dc4c812b545699c546c253c104719260e
Bug: webrtc:11581
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176404
Commit-Queue: Tommi <tommi@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31420}
This commit is contained in:
Tomas Gunnarsson 2020-06-03 08:54:39 +02:00 committed by Commit Bot
parent 878808892d
commit fae05624ec
20 changed files with 71 additions and 32 deletions

View File

@ -33,11 +33,11 @@
#include "modules/pacing/packet_router.h"
#include "modules/rtp_rtcp/include/receive_statistics.h"
#include "modules/rtp_rtcp/include/remote_ntp_time_estimator.h"
#include "modules/rtp_rtcp/include/rtp_rtcp.h"
#include "modules/rtp_rtcp/source/absolute_capture_time_receiver.h"
#include "modules/rtp_rtcp/source/rtp_header_extensions.h"
#include "modules/rtp_rtcp/source/rtp_packet_received.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_config.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h"
#include "modules/utility/include/process_thread.h"
#include "rtc_base/checks.h"
#include "rtc_base/critical_section.h"
@ -507,7 +507,7 @@ ChannelReceive::ChannelReceive(
if (frame_transformer)
InitFrameTransformerDelegate(std::move(frame_transformer));
_rtpRtcpModule = RtpRtcp::Create(configuration);
_rtpRtcpModule = ModuleRtpRtcpImpl2::Create(configuration);
_rtpRtcpModule->SetSendingMediaStatus(false);
_rtpRtcpModule->SetRemoteSSRC(remote_ssrc_);

View File

@ -29,6 +29,7 @@
#include "modules/audio_coding/include/audio_coding_module.h"
#include "modules/audio_processing/rms_level.h"
#include "modules/pacing/packet_router.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h"
#include "modules/utility/include/process_thread.h"
#include "rtc_base/checks.h"
#include "rtc_base/event.h"
@ -530,7 +531,7 @@ ChannelSend::ChannelSend(
configuration.local_media_ssrc = ssrc;
_rtpRtcpModule = RtpRtcp::Create(configuration);
_rtpRtcpModule = ModuleRtpRtcpImpl2::Create(configuration);
_rtpRtcpModule->SetSendingMediaStatus(false);
rtp_sender_audio_ = std::make_unique<RTPSenderAudio>(

View File

@ -16,6 +16,7 @@
#include "api/audio_codecs/audio_format.h"
#include "api/task_queue/task_queue_factory.h"
#include "modules/rtp_rtcp/include/receive_statistics.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h"
#include "rtc_base/critical_section.h"
#include "rtc_base/location.h"
#include "rtc_base/logging.h"
@ -51,7 +52,7 @@ AudioChannel::AudioChannel(
rtp_config.outgoing_transport = transport;
rtp_config.local_media_ssrc = local_ssrc;
rtp_rtcp_ = RtpRtcp::Create(rtp_config);
rtp_rtcp_ = ModuleRtpRtcpImpl2::Create(rtp_config);
rtp_rtcp_->SetSendingMediaStatus(false);
rtp_rtcp_->SetRTCPStatus(RtcpMode::kCompound);

View File

@ -36,6 +36,7 @@ if (rtc_include_tests) {
"../../../api/task_queue:default_task_queue_factory",
"../../../modules/audio_mixer:audio_mixer_impl",
"../../../modules/audio_mixer:audio_mixer_test_utils",
"../../../modules/rtp_rtcp:rtp_rtcp",
"../../../modules/rtp_rtcp:rtp_rtcp_format",
"../../../modules/utility",
"../../../rtc_base:logging",
@ -56,6 +57,7 @@ if (rtc_include_tests) {
"../../../api/audio_codecs:builtin_audio_encoder_factory",
"../../../api/task_queue:default_task_queue_factory",
"../../../modules/audio_mixer:audio_mixer_test_utils",
"../../../modules/rtp_rtcp:rtp_rtcp",
"../../../rtc_base:logging",
"../../../rtc_base:rtc_event",
"../../../test:mock_transport",
@ -72,6 +74,7 @@ if (rtc_include_tests) {
"../../../api/audio_codecs:builtin_audio_encoder_factory",
"../../../api/task_queue:default_task_queue_factory",
"../../../modules/audio_mixer:audio_mixer_test_utils",
"../../../modules/rtp_rtcp:rtp_rtcp",
"../../../modules/rtp_rtcp:rtp_rtcp_format",
"../../../rtc_base:logging",
"../../../rtc_base:rtc_event",

View File

@ -14,6 +14,7 @@
#include "api/task_queue/default_task_queue_factory.h"
#include "modules/audio_mixer/sine_wave_generator.h"
#include "modules/rtp_rtcp/source/rtp_packet_received.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h"
#include "rtc_base/event.h"
#include "rtc_base/logging.h"
#include "test/gmock.h"
@ -36,7 +37,7 @@ std::unique_ptr<RtpRtcp> CreateRtpStack(Clock* clock,
rtp_config.rtcp_report_interval_ms = 5000;
rtp_config.outgoing_transport = transport;
rtp_config.local_media_ssrc = remote_ssrc;
auto rtp_rtcp = RtpRtcp::Create(rtp_config);
auto rtp_rtcp = ModuleRtpRtcpImpl2::Create(rtp_config);
rtp_rtcp->SetSendingMediaStatus(false);
rtp_rtcp->SetRTCPStatus(RtcpMode::kCompound);
return rtp_rtcp;

View File

@ -15,6 +15,7 @@
#include "api/task_queue/default_task_queue_factory.h"
#include "audio/voip/audio_egress.h"
#include "modules/audio_mixer/sine_wave_generator.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h"
#include "rtc_base/event.h"
#include "rtc_base/logging.h"
#include "test/gmock.h"
@ -45,7 +46,7 @@ class AudioIngressTest : public ::testing::Test {
rtp_config.rtcp_report_interval_ms = 5000;
rtp_config.outgoing_transport = &transport_;
rtp_config.local_media_ssrc = 0xdeadc0de;
rtp_rtcp_ = RtpRtcp::Create(rtp_config);
rtp_rtcp_ = ModuleRtpRtcpImpl2::Create(rtp_config);
rtp_rtcp_->SetSendingMediaStatus(false);
rtp_rtcp_->SetRTCPStatus(RtcpMode::kCompound);

View File

@ -22,8 +22,8 @@
#include "call/rtp_stream_receiver_controller_interface.h"
#include "modules/rtp_rtcp/include/flexfec_receiver.h"
#include "modules/rtp_rtcp/include/receive_statistics.h"
#include "modules/rtp_rtcp/include/rtp_rtcp.h"
#include "modules/rtp_rtcp/source/rtp_packet_received.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h"
#include "modules/utility/include/process_thread.h"
#include "rtc_base/checks.h"
#include "rtc_base/location.h"
@ -132,7 +132,7 @@ std::unique_ptr<RtpRtcp> CreateRtpRtcpModule(
configuration.outgoing_transport = config.rtcp_send_transport;
configuration.rtt_stats = rtt_stats;
configuration.local_media_ssrc = config.local_ssrc;
return RtpRtcp::Create(configuration);
return ModuleRtpRtcpImpl2::Create(configuration);
}
} // namespace

View File

@ -22,8 +22,8 @@
#include "api/video_codecs/video_codec.h"
#include "call/rtp_transport_controller_send_interface.h"
#include "modules/pacing/packet_router.h"
#include "modules/rtp_rtcp/include/rtp_rtcp.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h"
#include "modules/rtp_rtcp/source/rtp_sender.h"
#include "modules/utility/include/process_thread.h"
#include "modules/video_coding/include/video_codec_interface.h"
@ -253,7 +253,7 @@ std::vector<RtpStreamSender> CreateRtpStreamSenders(
configuration.need_rtp_packet_infos = rtp_config.lntf.enabled;
auto rtp_rtcp = RtpRtcp::Create(configuration);
auto rtp_rtcp = ModuleRtpRtcpImpl2::Create(configuration);
rtp_rtcp->SetSendingStatus(false);
rtp_rtcp->SetSendingMediaStatus(false);
rtp_rtcp->SetRTCPStatus(RtcpMode::kCompound);

View File

@ -50,6 +50,7 @@ namespace rtcp {
class TransportFeedback;
}
// TODO(tommi): See if we can remove Module.
class RtpRtcp : public Module, public RtcpFeedbackSenderInterface {
public:
struct Configuration {
@ -158,8 +159,15 @@ class RtpRtcp : public Module, public RtcpFeedbackSenderInterface {
RTC_DISALLOW_COPY_AND_ASSIGN(Configuration);
};
// Creates an RTP/RTCP module object using provided |configuration|.
static std::unique_ptr<RtpRtcp> Create(const Configuration& configuration);
// DEPRECATED. Do not use. Currently instantiates a deprecated version of the
// RtpRtcp module.
static std::unique_ptr<RtpRtcp> RTC_DEPRECATED
Create(const Configuration& configuration) {
return DEPRECATED_Create(configuration);
}
static std::unique_ptr<RtpRtcp> DEPRECATED_Create(
const Configuration& configuration);
// **************************************************************************
// Receiver functions

View File

@ -19,9 +19,9 @@
#include "call/rtp_stream_receiver_controller.h"
#include "call/rtx_receive_stream.h"
#include "modules/rtp_rtcp/include/receive_statistics.h"
#include "modules/rtp_rtcp/include/rtp_rtcp.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/source/rtp_packet_received.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h"
#include "modules/rtp_rtcp/source/rtp_sender_video.h"
#include "rtc_base/rate_limiter.h"
#include "test/gtest.h"
@ -134,7 +134,7 @@ class RtpRtcpRtxNackTest : public ::testing::Test {
configuration.retransmission_rate_limiter = &retransmission_rate_limiter_;
configuration.local_media_ssrc = kTestSsrc;
configuration.rtx_send_ssrc = kTestRtxSsrc;
rtp_rtcp_module_ = RtpRtcp::Create(configuration);
rtp_rtcp_module_ = ModuleRtpRtcpImpl2::Create(configuration);
FieldTrialBasedConfig field_trials;
RTPSenderVideo::Config video_config;
video_config.clock = &fake_clock;

View File

@ -48,6 +48,14 @@ ModuleRtpRtcpImpl::RtpSenderContext::RtpSenderContext(
&packet_history,
config.paced_sender ? config.paced_sender : &non_paced_sender) {}
std::unique_ptr<RtpRtcp> RtpRtcp::DEPRECATED_Create(
const Configuration& configuration) {
RTC_DCHECK(configuration.clock);
RTC_LOG(LS_ERROR)
<< "*********** USING WebRTC INTERNAL IMPLEMENTATION DETAILS ***********";
return std::make_unique<ModuleRtpRtcpImpl>(configuration);
}
ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration)
: rtcp_sender_(configuration),
rtcp_receiver_(configuration, this),

View File

@ -48,11 +48,6 @@ ModuleRtpRtcpImpl2::RtpSenderContext::RtpSenderContext(
&packet_history,
config.paced_sender ? config.paced_sender : &non_paced_sender) {}
std::unique_ptr<RtpRtcp> RtpRtcp::Create(const Configuration& configuration) {
RTC_DCHECK(configuration.clock);
return std::make_unique<ModuleRtpRtcpImpl2>(configuration);
}
ModuleRtpRtcpImpl2::ModuleRtpRtcpImpl2(const Configuration& configuration)
: rtcp_sender_(configuration),
rtcp_receiver_(configuration, this),
@ -86,6 +81,14 @@ ModuleRtpRtcpImpl2::~ModuleRtpRtcpImpl2() {
RTC_DCHECK_RUN_ON(&construction_thread_checker_);
}
// static
std::unique_ptr<RtpRtcp> ModuleRtpRtcpImpl2::Create(
const Configuration& configuration) {
RTC_DCHECK(configuration.clock);
RTC_DCHECK(TaskQueueBase::Current());
return std::make_unique<ModuleRtpRtcpImpl2>(configuration);
}
// Returns the number of milliseconds until the module want a worker thread
// to call Process.
int64_t ModuleRtpRtcpImpl2::TimeUntilNextProcess() {

View File

@ -24,13 +24,13 @@
#include "api/video/video_bitrate_allocation.h"
#include "modules/include/module_fec_types.h"
#include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
#include "modules/rtp_rtcp/include/rtp_rtcp.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" // RTCPPacketType
#include "modules/rtp_rtcp/source/rtcp_packet/tmmb_item.h"
#include "modules/rtp_rtcp/source/rtcp_receiver.h"
#include "modules/rtp_rtcp/source/rtcp_sender.h"
#include "modules/rtp_rtcp/source/rtp_packet_history.h"
#include "modules/rtp_rtcp/source/rtp_packet_to_send.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h"
#include "modules/rtp_rtcp/source/rtp_sender.h"
#include "modules/rtp_rtcp/source/rtp_sender_egress.h"
#include "rtc_base/critical_section.h"
@ -49,6 +49,14 @@ class ModuleRtpRtcpImpl2 final : public RtpRtcp,
explicit ModuleRtpRtcpImpl2(const RtpRtcp::Configuration& configuration);
~ModuleRtpRtcpImpl2() override;
// This method is provided to easy with migrating away from the
// RtpRtcp::Create factory method. Since this is an internal implementation
// detail though, creating an instance of ModuleRtpRtcpImpl2 directly should
// be fine.
static std::unique_ptr<RtpRtcp> Create(const Configuration& configuration);
// TODO(tommi): Make implementation private?
// Returns the number of milliseconds until the module want a worker thread to
// call Process.
int64_t TimeUntilNextProcess() override;

View File

@ -18,6 +18,7 @@
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/source/rtp_header_extensions.h"
#include "modules/rtp_rtcp/source/rtp_packet_received.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h"
#include "modules/rtp_rtcp/source/time_util.h"
#include "test/gmock.h"
#include "test/gtest.h"
@ -67,7 +68,7 @@ class RtpSenderAudioTest : public ::testing::Test {
public:
RtpSenderAudioTest()
: fake_clock_(kStartTime),
rtp_module_(RtpRtcp::Create([&] {
rtp_module_(ModuleRtpRtcpImpl2::Create([&] {
RtpRtcp::Configuration config;
config.audio = true;
config.clock = &fake_clock_;

View File

@ -24,7 +24,6 @@
#include "common_video/generic_frame_descriptor/generic_frame_info.h"
#include "modules/rtp_rtcp/include/rtp_cvo.h"
#include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
#include "modules/rtp_rtcp/include/rtp_rtcp.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/source/rtp_dependency_descriptor_extension.h"
#include "modules/rtp_rtcp/source/rtp_descriptor_authentication.h"
@ -33,6 +32,7 @@
#include "modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension.h"
#include "modules/rtp_rtcp/source/rtp_header_extensions.h"
#include "modules/rtp_rtcp/source/rtp_packet_received.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h"
#include "modules/rtp_rtcp/source/time_util.h"
#include "rtc_base/arraysize.h"
#include "rtc_base/rate_limiter.h"
@ -169,7 +169,7 @@ class RtpSenderVideoTest : public ::testing::TestWithParam<bool> {
: field_trials_(GetParam()),
fake_clock_(kStartTime),
retransmission_rate_limiter_(&fake_clock_, 1000),
rtp_module_(RtpRtcp::Create([&] {
rtp_module_(ModuleRtpRtcpImpl2::Create([&] {
RtpRtcp::Configuration config;
config.clock = &fake_clock_;
config.outgoing_transport = &transport_;
@ -920,7 +920,7 @@ class RtpSenderVideoWithFrameTransformerTest : public ::testing::Test {
RtpSenderVideoWithFrameTransformerTest()
: fake_clock_(kStartTime),
retransmission_rate_limiter_(&fake_clock_, 1000),
rtp_module_(RtpRtcp::Create([&] {
rtp_module_(ModuleRtpRtcpImpl2::Create([&] {
RtpRtcp::Configuration config;
config.clock = &fake_clock_;
config.outgoing_transport = &transport_;

View File

@ -16,7 +16,7 @@
#include "api/video/video_bitrate_allocation.h"
#include "call/fake_network_pipe.h"
#include "call/simulated_network.h"
#include "modules/rtp_rtcp/include/rtp_rtcp.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h"
#include "rtc_base/rate_limiter.h"
#include "rtc_base/task_queue_for_test.h"
#include "rtc_base/task_utils/to_queued_task.h"
@ -244,7 +244,7 @@ TEST_F(BandwidthEndToEndTest, RembWithSendSideBwe) {
config.outgoing_transport = receive_transport_;
config.retransmission_rate_limiter = &retransmission_rate_limiter_;
config.local_media_ssrc = (*receive_configs)[0].rtp.local_ssrc;
rtp_rtcp_ = RtpRtcp::Create(config);
rtp_rtcp_ = ModuleRtpRtcpImpl2::Create(config);
rtp_rtcp_->SetRemoteSSRC((*receive_configs)[0].rtp.remote_ssrc);
rtp_rtcp_->SetRTCPStatus(RtcpMode::kReducedSize);
}

View File

@ -25,7 +25,6 @@
#include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
#include "modules/rtp_rtcp/include/receive_statistics.h"
#include "modules/rtp_rtcp/include/rtp_cvo.h"
#include "modules/rtp_rtcp/include/rtp_rtcp.h"
#include "modules/rtp_rtcp/include/ulpfec_receiver.h"
#include "modules/rtp_rtcp/source/create_video_rtp_depacketizer.h"
#include "modules/rtp_rtcp/source/rtp_dependency_descriptor_extension.h"
@ -35,6 +34,7 @@
#include "modules/rtp_rtcp/source/rtp_header_extensions.h"
#include "modules/rtp_rtcp/source/rtp_packet_received.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_config.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h"
#include "modules/rtp_rtcp/source/video_rtp_depacketizer.h"
#include "modules/rtp_rtcp/source/video_rtp_depacketizer_raw.h"
#include "modules/utility/include/process_thread.h"
@ -97,7 +97,7 @@ std::unique_ptr<RtpRtcp> CreateRtpRtcpModule(
configuration.rtcp_cname_callback = rtcp_cname_callback;
configuration.local_media_ssrc = local_ssrc;
std::unique_ptr<RtpRtcp> rtp_rtcp = RtpRtcp::Create(configuration);
std::unique_ptr<RtpRtcp> rtp_rtcp = RtpRtcp::DEPRECATED_Create(configuration);
rtp_rtcp->SetRTCPStatus(RtcpMode::kCompound);
return rtp_rtcp;

View File

@ -25,7 +25,6 @@
#include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
#include "modules/rtp_rtcp/include/receive_statistics.h"
#include "modules/rtp_rtcp/include/rtp_cvo.h"
#include "modules/rtp_rtcp/include/rtp_rtcp.h"
#include "modules/rtp_rtcp/include/ulpfec_receiver.h"
#include "modules/rtp_rtcp/source/create_video_rtp_depacketizer.h"
#include "modules/rtp_rtcp/source/rtp_dependency_descriptor_extension.h"
@ -35,6 +34,7 @@
#include "modules/rtp_rtcp/source/rtp_header_extensions.h"
#include "modules/rtp_rtcp/source/rtp_packet_received.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_config.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h"
#include "modules/rtp_rtcp/source/video_rtp_depacketizer.h"
#include "modules/rtp_rtcp/source/video_rtp_depacketizer_raw.h"
#include "modules/utility/include/process_thread.h"
@ -97,7 +97,7 @@ std::unique_ptr<RtpRtcp> CreateRtpRtcpModule(
configuration.rtcp_cname_callback = rtcp_cname_callback;
configuration.local_media_ssrc = local_ssrc;
std::unique_ptr<RtpRtcp> rtp_rtcp = RtpRtcp::Create(configuration);
std::unique_ptr<RtpRtcp> rtp_rtcp = ModuleRtpRtcpImpl2::Create(configuration);
rtp_rtcp->SetRTCPStatus(RtcpMode::kCompound);
return rtp_rtcp;

View File

@ -37,6 +37,7 @@
#include "test/gmock.h"
#include "test/gtest.h"
#include "test/mock_frame_transformer.h"
#include "test/time_controller/simulated_task_queue.h"
using ::testing::_;
using ::testing::ElementsAre;
@ -237,6 +238,9 @@ class RtpVideoStreamReceiver2Test : public ::testing::Test {
return config;
}
TokenTaskQueue task_queue_;
TokenTaskQueue::CurrentTaskQueueSetter task_queue_setter_{&task_queue_};
const webrtc::test::ScopedFieldTrials override_field_trials_;
VideoReceiveStream::Config config_;
MockNackSender mock_nack_sender_;

View File

@ -25,10 +25,10 @@
#include "call/simulated_network.h"
#include "call/video_send_stream.h"
#include "modules/rtp_rtcp/include/rtp_header_extension_map.h"
#include "modules/rtp_rtcp/include/rtp_rtcp.h"
#include "modules/rtp_rtcp/source/rtcp_sender.h"
#include "modules/rtp_rtcp/source/rtp_header_extensions.h"
#include "modules/rtp_rtcp/source/rtp_packet.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h"
#include "modules/rtp_rtcp/source/video_rtp_depacketizer_vp9.h"
#include "modules/video_coding/codecs/vp8/include/vp8.h"
#include "modules/video_coding/codecs/vp9/include/vp9.h"
@ -1677,7 +1677,7 @@ TEST_F(VideoSendStreamTest, MinTransmitBitrateRespectsRemb) {
config.clock = Clock::GetRealTimeClock();
config.outgoing_transport = feedback_transport_.get();
config.retransmission_rate_limiter = &retranmission_rate_limiter_;
rtp_rtcp_ = RtpRtcp::Create(config);
rtp_rtcp_ = ModuleRtpRtcpImpl2::Create(config);
rtp_rtcp_->SetRTCPStatus(RtcpMode::kReducedSize);
}