RTCPSender: migrate to own configuration struct.

The class depends on RtcRtcpInterface::Configuration which adds an
unneeded dependency, and inhibits well-manored changes to the
constructor interface.

Fix this so that RTCPSender uses it's own configuration struct which
can be extended in future CLs.

Also add a legacy constructor while downstream dependencies are
updated.

Bug: webrtc:11581
Change-Id: I8d166ab8253b27c08fcbe6aa7c7adde92688b7dc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/222322
Commit-Queue: Markus Handell <handellm@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34343}
This commit is contained in:
Markus Handell
2021-06-18 13:44:13 +02:00
committed by WebRTC LUCI CQ
parent f906ec40d4
commit 2e3edc1da9
6 changed files with 100 additions and 34 deletions

View File

@ -17,6 +17,7 @@
#include <utility>
#include "api/rtc_event_log/rtc_event_log.h"
#include "api/units/time_delta.h"
#include "api/units/timestamp.h"
#include "logging/rtc_event_log/events/rtc_event_rtcp_packet_outgoing.h"
#include "modules/rtp_rtcp/source/rtcp_packet/app.h"
@ -35,6 +36,7 @@
#include "modules/rtp_rtcp/source/rtcp_packet/tmmbr.h"
#include "modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_impl2.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_interface.h"
#include "modules/rtp_rtcp/source/time_util.h"
#include "modules/rtp_rtcp/source/tmmbr_help.h"
#include "rtc_base/checks.h"
@ -50,7 +52,6 @@ const uint32_t kRtcpAnyExtendedReports = kRtcpXrReceiverReferenceTime |
kRtcpXrTargetBitrate;
constexpr int32_t kDefaultVideoReportInterval = 1000;
constexpr int32_t kDefaultAudioReportInterval = 5000;
} // namespace
// Helper to put several RTCP packets into lower layer datagram RTCP packet.
@ -116,7 +117,26 @@ class RTCPSender::RtcpContext {
const Timestamp now_;
};
RTCPSender::RTCPSender(const RtpRtcpInterface::Configuration& config)
RTCPSender::Configuration RTCPSender::Configuration::FromRtpRtcpConfiguration(
const RtpRtcpInterface::Configuration& configuration) {
RTCPSender::Configuration result;
result.audio = configuration.audio;
result.local_media_ssrc = configuration.local_media_ssrc;
result.clock = configuration.clock;
result.outgoing_transport = configuration.outgoing_transport;
result.non_sender_rtt_measurement = configuration.non_sender_rtt_measurement;
result.event_log = configuration.event_log;
if (configuration.rtcp_report_interval_ms) {
result.rtcp_report_interval =
TimeDelta::Millis(configuration.rtcp_report_interval_ms);
}
result.receive_statistics = configuration.receive_statistics;
result.rtcp_packet_type_counter_observer =
configuration.rtcp_packet_type_counter_observer;
return result;
}
RTCPSender::RTCPSender(const Configuration& config)
: audio_(config.audio),
ssrc_(config.local_media_ssrc),
clock_(config.clock),
@ -124,10 +144,11 @@ RTCPSender::RTCPSender(const RtpRtcpInterface::Configuration& config)
method_(RtcpMode::kOff),
event_log_(config.event_log),
transport_(config.outgoing_transport),
report_interval_ms_(config.rtcp_report_interval_ms > 0
? config.rtcp_report_interval_ms
: (config.audio ? kDefaultAudioReportInterval
: kDefaultVideoReportInterval)),
report_interval_ms_(config.rtcp_report_interval
.value_or(TimeDelta::Millis(
config.audio ? kDefaultAudioReportInterval
: kDefaultVideoReportInterval))
.ms()),
sending_(false),
next_time_to_send_rtcp_(0),
timestamp_offset_(0),
@ -165,6 +186,9 @@ RTCPSender::RTCPSender(const RtpRtcpInterface::Configuration& config)
builders_[kRtcpAnyExtendedReports] = &RTCPSender::BuildExtendedReports;
}
RTCPSender::RTCPSender(const RtpRtcpInterface::Configuration& config)
: RTCPSender(Configuration::FromRtpRtcpConfiguration(config)) {}
RTCPSender::~RTCPSender() {}
RtcpMode RTCPSender::Status() const {

View File

@ -19,6 +19,7 @@
#include "absl/types/optional.h"
#include "api/call/transport.h"
#include "api/units/time_delta.h"
#include "api/video/video_bitrate_allocation.h"
#include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
#include "modules/rtp_rtcp/include/receive_statistics.h"
@ -42,6 +43,32 @@ class RtcEventLog;
class RTCPSender final {
public:
struct Configuration {
// TODO(bugs.webrtc.org/11581): Remove this temporary conversion utility
// once rtc_rtcp_impl.cc/h are gone.
static Configuration FromRtpRtcpConfiguration(
const RtpRtcpInterface::Configuration& config);
// True for a audio version of the RTP/RTCP module object false will create
// a video version.
bool audio = false;
// SSRCs for media and retransmission, respectively.
// FlexFec SSRC is fetched from |flexfec_sender|.
uint32_t local_media_ssrc = 0;
// The clock to use to read time. If nullptr then system clock will be used.
Clock* clock = nullptr;
// Transport object that will be called when packets are ready to be sent
// out on the network.
Transport* outgoing_transport = nullptr;
// Estimate RTT as non-sender as described in
// https://tools.ietf.org/html/rfc3611#section-4.4 and #section-4.5
bool non_sender_rtt_measurement = false;
RtcEventLog* event_log = nullptr;
absl::optional<TimeDelta> rtcp_report_interval;
ReceiveStatisticsProvider* receive_statistics = nullptr;
RtcpPacketTypeCounterObserver* rtcp_packet_type_counter_observer = nullptr;
};
struct FeedbackState {
FeedbackState();
FeedbackState(const FeedbackState&);
@ -63,6 +90,9 @@ class RTCPSender final {
RTCPReceiver* receiver;
};
explicit RTCPSender(const Configuration& config);
// TODO(bugs.webrtc.org/11581): delete this temporary compatibility helper
// once downstream dependencies migrates.
explicit RTCPSender(const RtpRtcpInterface::Configuration& config);
RTCPSender() = delete;

View File

@ -14,6 +14,7 @@
#include <utility>
#include "absl/base/macros.h"
#include "api/units/time_delta.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "modules/rtp_rtcp/source/rtcp_packet/bye.h"
#include "modules/rtp_rtcp/source/rtcp_packet/common_header.h"
@ -71,7 +72,7 @@ static const uint32_t kStartRtpTimestamp = 0x34567;
static const uint32_t kRtpTimestamp = 0x45678;
std::unique_ptr<RTCPSender> CreateRtcpSender(
const RtpRtcpInterface::Configuration& config,
const RTCPSender::Configuration& config,
bool init_timestamps = true) {
auto rtcp_sender = std::make_unique<RTCPSender>(config);
rtcp_sender->SetRemoteSSRC(kRemoteSsrc);
@ -83,31 +84,39 @@ std::unique_ptr<RTCPSender> CreateRtcpSender(
}
return rtcp_sender;
}
} // namespace
class RtcpSenderTest : public ::testing::Test {
protected:
RtcpSenderTest()
: clock_(1335900000),
receive_statistics_(ReceiveStatistics::Create(&clock_)),
retransmission_rate_limiter_(&clock_, 1000) {
RtpRtcpInterface::Configuration configuration = GetDefaultConfig();
rtp_rtcp_impl_.reset(new ModuleRtpRtcpImpl2(configuration));
receive_statistics_(ReceiveStatistics::Create(&clock_)) {
rtp_rtcp_impl_.reset(new ModuleRtpRtcpImpl2(GetDefaultRtpRtcpConfig()));
}
RtpRtcpInterface::Configuration GetDefaultConfig() {
RtpRtcpInterface::Configuration configuration;
RTCPSender::Configuration GetDefaultConfig() {
RTCPSender::Configuration configuration;
configuration.audio = false;
configuration.clock = &clock_;
configuration.outgoing_transport = &test_transport_;
configuration.retransmission_rate_limiter = &retransmission_rate_limiter_;
configuration.rtcp_report_interval_ms = 1000;
configuration.rtcp_report_interval = TimeDelta::Millis(1000);
configuration.receive_statistics = receive_statistics_.get();
configuration.local_media_ssrc = kSenderSsrc;
return configuration;
}
RtpRtcpInterface::Configuration GetDefaultRtpRtcpConfig() {
RTCPSender::Configuration config = GetDefaultConfig();
RtpRtcpInterface::Configuration result;
result.audio = config.audio;
result.clock = config.clock;
result.outgoing_transport = config.outgoing_transport;
result.rtcp_report_interval_ms = config.rtcp_report_interval->ms();
result.receive_statistics = config.receive_statistics;
result.local_media_ssrc = config.local_media_ssrc;
return result;
}
void InsertIncomingPacket(uint32_t remote_ssrc, uint16_t seq_num) {
RtpPacketReceived packet;
packet.SetSsrc(remote_ssrc);
@ -127,7 +136,6 @@ class RtcpSenderTest : public ::testing::Test {
TestTransport test_transport_;
std::unique_ptr<ReceiveStatistics> receive_statistics_;
std::unique_ptr<ModuleRtpRtcpImpl2> rtp_rtcp_impl_;
RateLimiter retransmission_rate_limiter_;
};
TEST_F(RtcpSenderTest, SetRtcpStatus) {
@ -206,11 +214,11 @@ TEST_F(RtcpSenderTest, SendConsecutiveSrWithExactSlope) {
}
TEST_F(RtcpSenderTest, DoNotSendSrBeforeRtp) {
RtpRtcpInterface::Configuration config;
RTCPSender::Configuration config;
config.clock = &clock_;
config.receive_statistics = receive_statistics_.get();
config.outgoing_transport = &test_transport_;
config.rtcp_report_interval_ms = 1000;
config.rtcp_report_interval = TimeDelta::Millis(1000);
config.local_media_ssrc = kSenderSsrc;
auto rtcp_sender = CreateRtcpSender(config, /*init_timestamps=*/false);
rtcp_sender->SetRTCPStatus(RtcpMode::kReducedSize);
@ -227,11 +235,11 @@ TEST_F(RtcpSenderTest, DoNotSendSrBeforeRtp) {
}
TEST_F(RtcpSenderTest, DoNotSendCompundBeforeRtp) {
RtpRtcpInterface::Configuration config;
RTCPSender::Configuration config;
config.clock = &clock_;
config.receive_statistics = receive_statistics_.get();
config.outgoing_transport = &test_transport_;
config.rtcp_report_interval_ms = 1000;
config.rtcp_report_interval = TimeDelta::Millis(1000);
config.local_media_ssrc = kSenderSsrc;
auto rtcp_sender = CreateRtcpSender(config, /*init_timestamps=*/false);
rtcp_sender->SetRTCPStatus(RtcpMode::kCompound);
@ -510,7 +518,7 @@ TEST_F(RtcpSenderTest, SendXrWithMultipleDlrrSubBlocks) {
}
TEST_F(RtcpSenderTest, SendXrWithRrtr) {
RtpRtcpInterface::Configuration config = GetDefaultConfig();
RTCPSender::Configuration config = GetDefaultConfig();
config.non_sender_rtt_measurement = true;
auto rtcp_sender = CreateRtcpSender(config);
rtcp_sender->SetRTCPStatus(RtcpMode::kCompound);
@ -525,7 +533,7 @@ TEST_F(RtcpSenderTest, SendXrWithRrtr) {
}
TEST_F(RtcpSenderTest, TestNoXrRrtrSentIfSending) {
RtpRtcpInterface::Configuration config = GetDefaultConfig();
RTCPSender::Configuration config = GetDefaultConfig();
config.non_sender_rtt_measurement = true;
auto rtcp_sender = CreateRtcpSender(config);
rtcp_sender->SetRTCPStatus(RtcpMode::kCompound);
@ -535,7 +543,7 @@ TEST_F(RtcpSenderTest, TestNoXrRrtrSentIfSending) {
}
TEST_F(RtcpSenderTest, TestNoXrRrtrSentIfNotEnabled) {
RtpRtcpInterface::Configuration config = GetDefaultConfig();
RTCPSender::Configuration config = GetDefaultConfig();
config.non_sender_rtt_measurement = false;
auto rtcp_sender = CreateRtcpSender(config);
rtcp_sender->SetRTCPStatus(RtcpMode::kCompound);
@ -546,12 +554,12 @@ TEST_F(RtcpSenderTest, TestNoXrRrtrSentIfNotEnabled) {
TEST_F(RtcpSenderTest, TestRegisterRtcpPacketTypeObserver) {
RtcpPacketTypeCounterObserverImpl observer;
RtpRtcpInterface::Configuration config;
RTCPSender::Configuration config;
config.clock = &clock_;
config.receive_statistics = receive_statistics_.get();
config.outgoing_transport = &test_transport_;
config.rtcp_packet_type_counter_observer = &observer;
config.rtcp_report_interval_ms = 1000;
config.rtcp_report_interval = TimeDelta::Millis(1000);
auto rtcp_sender = CreateRtcpSender(config);
rtcp_sender->SetRTCPStatus(RtcpMode::kReducedSize);
EXPECT_EQ(0, rtcp_sender->SendRTCP(feedback_state(), kRtcpPli));
@ -643,11 +651,11 @@ TEST_F(RtcpSenderTest, ByeMustBeLast) {
}));
// Re-configure rtcp_sender with mock_transport_
RtpRtcpInterface::Configuration config;
RTCPSender::Configuration config;
config.clock = &clock_;
config.receive_statistics = receive_statistics_.get();
config.outgoing_transport = &mock_transport;
config.rtcp_report_interval_ms = 1000;
config.rtcp_report_interval = TimeDelta::Millis(1000);
config.local_media_ssrc = kSenderSsrc;
auto rtcp_sender = CreateRtcpSender(config);

View File

@ -21,7 +21,9 @@
#include "api/transport/field_trial_based_config.h"
#include "modules/rtp_rtcp/source/rtcp_packet/dlrr.h"
#include "modules/rtp_rtcp/source/rtcp_sender.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_config.h"
#include "modules/rtp_rtcp/source/rtp_rtcp_interface.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "system_wrappers/include/ntp_time.h"
@ -58,7 +60,8 @@ std::unique_ptr<RtpRtcp> RtpRtcp::DEPRECATED_Create(
}
ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration)
: rtcp_sender_(configuration),
: rtcp_sender_(
RTCPSender::Configuration::FromRtpRtcpConfiguration(configuration)),
rtcp_receiver_(configuration, this),
clock_(configuration.clock),
last_bitrate_process_time_(clock_->TimeInMilliseconds()),

View File

@ -55,7 +55,8 @@ void ModuleRtpRtcpImpl2::RtpSenderContext::AssignSequenceNumber(
ModuleRtpRtcpImpl2::ModuleRtpRtcpImpl2(const Configuration& configuration)
: worker_queue_(TaskQueueBase::Current()),
rtcp_sender_(configuration),
rtcp_sender_(
RTCPSender::Configuration::FromRtpRtcpConfiguration(configuration)),
rtcp_receiver_(configuration, this),
clock_(configuration.clock),
last_rtt_process_time_(clock_->TimeInMilliseconds()),

View File

@ -948,10 +948,10 @@ void VideoSendStreamTest::TestNackRetransmission(
non_padding_sequence_numbers_.end() - kNackedPacketsAtOnceCount,
non_padding_sequence_numbers_.end());
RtpRtcpInterface::Configuration config;
RTCPSender::Configuration config;
config.clock = Clock::GetRealTimeClock();
config.outgoing_transport = transport_adapter_.get();
config.rtcp_report_interval_ms = kRtcpIntervalMs;
config.rtcp_report_interval = TimeDelta::Millis(kRtcpIntervalMs);
config.local_media_ssrc = kReceiverLocalVideoSsrc;
RTCPSender rtcp_sender(config);
@ -1164,11 +1164,11 @@ void VideoSendStreamTest::TestPacketFragmentationSize(VideoFormat format,
kVideoSendSsrcs[0], rtp_packet.SequenceNumber(),
packets_lost_, // Cumulative lost.
loss_ratio); // Loss percent.
RtpRtcpInterface::Configuration config;
RTCPSender::Configuration config;
config.clock = Clock::GetRealTimeClock();
config.receive_statistics = &lossy_receive_stats;
config.outgoing_transport = transport_adapter_.get();
config.rtcp_report_interval_ms = kRtcpIntervalMs;
config.rtcp_report_interval = TimeDelta::Millis(kRtcpIntervalMs);
config.local_media_ssrc = kVideoSendSsrcs[0];
RTCPSender rtcp_sender(config);