admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Reland "Add ability to set RTCP sender ssrc at construction time""

Browse Source 
This reverts commit 6f420e424885dab1d9f885365ea9abea5cc4a901.

Reason for revert: Speculative revert (some perf test are failing)

Original change's description:
> Reland "Add ability to set RTCP sender ssrc at construction time"
>
> This is a reland of 94c58fd815f0c7c6429aa53a79621ea9ef39c770
>
> Patch set 1 is the original CL.
> Patch set 2 introduced a trivial fix. In RtcpSender::SetSSRC(), check
> if either current SSRC is 0 or if the SSRC is identical to the current
> one. If so, don't schedule an early report.
> This prevents a regression in which audio jitter became too low(?)
>
> Original change's description:
> > Add ability to set RTCP sender ssrc at construction time
> >
> > Bug: webrtc:10774
> > Change-Id: Iaf5857e24359e9795434bcd0cdbe1658a2f9f5d3
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144632
> > Reviewed-by: Åsa Persson <asapersson@webrtc.org>
> > Commit-Queue: Erik Språng <sprang@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#28506}
>
> Bug: webrtc:10774
> Change-Id: I103dfa48719aa41d6ab633cdac8b3a5c46b54843
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/144565
> Commit-Queue: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Åsa Persson <asapersson@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#28520}

TBR=asapersson@webrtc.org,sprang@webrtc.org,ilnik@webrtc.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: webrtc:10774
Change-Id: I39238d942b2bbe0a9c8ca752387a35ed9dd70650
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/145327
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28555}
This commit is contained in:
Mirko Bonadei 2019-07-12 08:38:36 +00:00 committed by Commit Bot
parent 01d04fac75
commit 8b3e4e2d11
6 changed files with 64 additions and 112 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
modules/rtp_rtcp/source/rtcp_sender.cc
View File

@ -48,8 +48,6 @@ namespace {
const uint32_t kRtcpAnyExtendedReports = kRtcpXrReceiverReferenceTime |
kRtcpXrDlrrReportBlock |
kRtcpXrTargetBitrate;
constexpr int32_t kDefaultVideoReportInterval = 1000;
constexpr int32_t kDefaultAudioReportInterval = 5000;
} // namespace
RTCPSender::FeedbackState::FeedbackState()
@ -114,25 +112,29 @@ class RTCPSender::RtcpContext {
const int64_t now_us_;
};
RTCPSender::RTCPSender(const RtpRtcp::Configuration& config)
: audio_(config.audio),
clock_(config.clock),
RTCPSender::RTCPSender(
bool audio,
Clock* clock,
ReceiveStatisticsProvider* receive_statistics,
RtcpPacketTypeCounterObserver* packet_type_counter_observer,
RtcEventLog* event_log,
Transport* outgoing_transport,
int report_interval_ms)
: audio_(audio),
clock_(clock),
random_(clock_->TimeInMicroseconds()),
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)),
event_log_(event_log),
transport_(outgoing_transport),
report_interval_ms_(report_interval_ms),
sending_(false),
next_time_to_send_rtcp_(0),
timestamp_offset_(0),
last_rtp_timestamp_(0),
last_frame_capture_time_ms_(-1),
ssrc_(config.media_send_ssrc.value_or(0)),
ssrc_(0),
remote_ssrc_(0),
receive_statistics_(config.receive_statistics),
receive_statistics_(receive_statistics),
sequence_number_fir_(0),
@ -148,7 +150,7 @@ RTCPSender::RTCPSender(const RtpRtcp::Configuration& config)
app_length_(0),
xr_send_receiver_reference_time_enabled_(false),
packet_type_counter_observer_(config.rtcp_packet_type_counter_observer),
packet_type_counter_observer_(packet_type_counter_observer),
send_video_bitrate_allocation_(false),
last_payload_type_(-1) {
RTC_DCHECK(transport_ != nullptr);
@ -305,7 +307,7 @@ uint32_t RTCPSender::SSRC() const {
void RTCPSender::SetSSRC(uint32_t ssrc) {
rtc::CritScope lock(&critical_section_rtcp_sender_);
if (ssrc_ != 0 && ssrc != ssrc_) {
if (ssrc_ != 0) {
// not first SetSSRC, probably due to a collision
// schedule a new RTCP report
// make sure that we send a RTP packet

9
modules/rtp_rtcp/source/rtcp_sender.h
View File

@ -23,7 +23,6 @@
#include "modules/remote_bitrate_estimator/include/bwe_defines.h"
#include "modules/remote_bitrate_estimator/include/remote_bitrate_estimator.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/rtcp_nack_stats.h"
#include "modules/rtp_rtcp/source/rtcp_packet.h"
@ -63,7 +62,13 @@ class RTCPSender {
ModuleRtpRtcpImpl* module;
};
explicit RTCPSender(const RtpRtcp::Configuration& config);
RTCPSender(bool audio,
Clock* clock,
ReceiveStatisticsProvider* receive_statistics,
RtcpPacketTypeCounterObserver* packet_type_counter_observer,
RtcEventLog* event_log,
Transport* outgoing_transport,
int report_interval_ms);
virtual ~RTCPSender();
RtcpMode Status() const;

98
modules/rtp_rtcp/source/rtcp_sender_unittest.cc
View File

@ -75,25 +75,22 @@ class RtcpSenderTest : public ::testing::Test {
: clock_(1335900000),
receive_statistics_(ReceiveStatistics::Create(&clock_)),
retransmission_rate_limiter_(&clock_, 1000) {
RtpRtcp::Configuration configuration = GetDefaultConfig();
rtp_rtcp_impl_.reset(new ModuleRtpRtcpImpl(configuration));
rtcp_sender_.reset(new RTCPSender(configuration));
rtcp_sender_->SetRemoteSSRC(kRemoteSsrc);
rtcp_sender_->SetTimestampOffset(kStartRtpTimestamp);
rtcp_sender_->SetLastRtpTime(kRtpTimestamp, clock_.TimeInMilliseconds(),
/*payload_type=*/0);
}
RtpRtcp::Configuration GetDefaultConfig() {
RtpRtcp::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.receive_statistics = receive_statistics_.get();
configuration.media_send_ssrc = kSenderSsrc;
return configuration;
rtp_rtcp_impl_.reset(new ModuleRtpRtcpImpl(configuration));
rtcp_sender_.reset(new RTCPSender(false, &clock_, receive_statistics_.get(),
nullptr, nullptr, &test_transport_,
configuration.rtcp_report_interval_ms));
rtcp_sender_->SetSSRC(kSenderSsrc);
rtcp_sender_->SetRemoteSSRC(kRemoteSsrc);
rtcp_sender_->SetTimestampOffset(kStartRtpTimestamp);
rtcp_sender_->SetLastRtpTime(kRtpTimestamp, clock_.TimeInMilliseconds(),
/*payload_type=*/0);
}
void InsertIncomingPacket(uint32_t remote_ssrc, uint16_t seq_num) {
@ -190,13 +187,9 @@ TEST_F(RtcpSenderTest, SendConsecutiveSrWithExactSlope) {
}
TEST_F(RtcpSenderTest, DoNotSendSrBeforeRtp) {
RtpRtcp::Configuration config;
config.clock = &clock_;
config.receive_statistics = receive_statistics_.get();
config.outgoing_transport = &test_transport_;
config.rtcp_report_interval_ms = 1000;
config.media_send_ssrc = kSenderSsrc;
rtcp_sender_.reset(new RTCPSender(config));
rtcp_sender_.reset(new RTCPSender(false, &clock_, receive_statistics_.get(),
nullptr, nullptr, &test_transport_, 1000));
rtcp_sender_->SetSSRC(kSenderSsrc);
rtcp_sender_->SetRemoteSSRC(kRemoteSsrc);
rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize);
rtcp_sender_->SetSendingStatus(feedback_state(), true);
@ -212,13 +205,9 @@ TEST_F(RtcpSenderTest, DoNotSendSrBeforeRtp) {
}
TEST_F(RtcpSenderTest, DoNotSendCompundBeforeRtp) {
RtpRtcp::Configuration config;
config.clock = &clock_;
config.receive_statistics = receive_statistics_.get();
config.outgoing_transport = &test_transport_;
config.rtcp_report_interval_ms = 1000;
config.media_send_ssrc = kSenderSsrc;
rtcp_sender_.reset(new RTCPSender(config));
rtcp_sender_.reset(new RTCPSender(false, &clock_, receive_statistics_.get(),
nullptr, nullptr, &test_transport_, 1000));
rtcp_sender_->SetSSRC(kSenderSsrc);
rtcp_sender_->SetRemoteSSRC(kRemoteSsrc);
rtcp_sender_->SetRTCPStatus(RtcpMode::kCompound);
rtcp_sender_->SetSendingStatus(feedback_state(), true);
@ -562,14 +551,9 @@ TEST_F(RtcpSenderTest, TestNoXrRrtrSentIfNotEnabled) {
TEST_F(RtcpSenderTest, TestRegisterRtcpPacketTypeObserver) {
RtcpPacketTypeCounterObserverImpl observer;
RtpRtcp::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;
rtcp_sender_.reset(new RTCPSender(config));
rtcp_sender_.reset(new RTCPSender(false, &clock_, receive_statistics_.get(),
&observer, nullptr, &test_transport_,
1000));
rtcp_sender_->SetRemoteSSRC(kRemoteSsrc);
rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize);
EXPECT_EQ(0, rtcp_sender_->SendRTCP(feedback_state(), kRtcpPli));
@ -690,14 +674,9 @@ TEST_F(RtcpSenderTest, ByeMustBeLast) {
}));
// Re-configure rtcp_sender_ with mock_transport_
RtpRtcp::Configuration config;
config.clock = &clock_;
config.receive_statistics = receive_statistics_.get();
config.outgoing_transport = &mock_transport;
config.rtcp_report_interval_ms = 1000;
config.media_send_ssrc = kSenderSsrc;
rtcp_sender_.reset(new RTCPSender(config));
rtcp_sender_.reset(new RTCPSender(false, &clock_, receive_statistics_.get(),
nullptr, nullptr, &mock_transport, 1000));
rtcp_sender_->SetSSRC(kSenderSsrc);
rtcp_sender_->SetRemoteSSRC(kRemoteSsrc);
rtcp_sender_->SetTimestampOffset(kStartRtpTimestamp);
rtcp_sender_->SetLastRtpTime(kRtpTimestamp, clock_.TimeInMilliseconds(),
@ -816,37 +795,4 @@ TEST_F(RtcpSenderTest, SendTargetBitrateExplicitZeroOnStreamRemoval) {
EXPECT_EQ(bitrates[1].target_bitrate_kbps, 0u);
}
TEST_F(RtcpSenderTest, DoesntSchedulesInitialReportWhenSsrcSetOnConstruction) {
rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize);
rtcp_sender_->SetRemoteSSRC(kRemoteSsrc);
// New report should not have been scheduled yet.
clock_.AdvanceTimeMilliseconds(100);
EXPECT_FALSE(rtcp_sender_->TimeToSendRTCPReport(false));
}
TEST_F(RtcpSenderTest, DoesntSchedulesInitialReportOnFirstSetSsrc) {
// Set up without first SSRC not set at construction.
RtpRtcp::Configuration configuration = GetDefaultConfig();
configuration.media_send_ssrc = absl::nullopt;
rtcp_sender_.reset(new RTCPSender(configuration));
rtcp_sender_->SetRemoteSSRC(kRemoteSsrc);
rtcp_sender_->SetTimestampOffset(kStartRtpTimestamp);
rtcp_sender_->SetLastRtpTime(kRtpTimestamp, clock_.TimeInMilliseconds(),
/*payload_type=*/0);
rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize);
// Set SSRC for the first time. New report should not be scheduled.
rtcp_sender_->SetSSRC(kSenderSsrc);
clock_.AdvanceTimeMilliseconds(100);
EXPECT_FALSE(rtcp_sender_->TimeToSendRTCPReport(false));
}
TEST_F(RtcpSenderTest, SchedulesReportOnSsrcChange) {
rtcp_sender_->SetRTCPStatus(RtcpMode::kReducedSize);
rtcp_sender_->SetSSRC(kSenderSsrc + 1);
clock_.AdvanceTimeMilliseconds(100);
EXPECT_TRUE(rtcp_sender_->TimeToSendRTCPReport(false));
}
} // namespace webrtc

11
modules/rtp_rtcp/source/rtp_rtcp_impl.cc
View File

@ -61,7 +61,16 @@ RtpRtcp* RtpRtcp::CreateRtpRtcp(const RtpRtcp::Configuration& configuration) {
}
ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration)
: rtcp_sender_(configuration),
: rtcp_sender_(configuration.audio,
configuration.clock,
configuration.receive_statistics,
configuration.rtcp_packet_type_counter_observer,
configuration.event_log,
configuration.outgoing_transport,
configuration.rtcp_report_interval_ms > 0
? configuration.rtcp_report_interval_ms
: (configuration.audio ? kDefaultAudioReportInterval
: kDefaultVideoReportInterval)),
rtcp_receiver_(configuration.clock,
configuration.receiver_only,
configuration.rtcp_packet_type_counter_observer,

1
modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
View File

@ -162,7 +162,6 @@ class RtpRtcpModule : public RtcpPacketTypeCounterObserver {
config.rtcp_packet_type_counter_observer = this;
config.rtt_stats = &rtt_stats_;
config.rtcp_report_interval_ms = rtcp_report_interval_ms_;
config.media_send_ssrc = kSenderSsrc;
impl_.reset(new ModuleRtpRtcpImpl(config));
impl_->SetRTCPStatus(RtcpMode::kCompound);

25
video/video_send_stream_tests.cc
View File

@ -914,11 +914,9 @@ void VideoSendStreamTest::TestNackRetransmission(
non_padding_sequence_numbers_.end() - kNackedPacketsAtOnceCount,
non_padding_sequence_numbers_.end());
RtpRtcp::Configuration config;
config.clock = Clock::GetRealTimeClock();
config.outgoing_transport = transport_adapter_.get();
config.rtcp_report_interval_ms = kRtcpIntervalMs;
RTCPSender rtcp_sender(config);
RTCPSender rtcp_sender(false, Clock::GetRealTimeClock(), nullptr,
nullptr, nullptr, transport_adapter_.get(),
kRtcpIntervalMs);
rtcp_sender.SetRTCPStatus(RtcpMode::kReducedSize);
rtcp_sender.SetRemoteSSRC(kVideoSendSsrcs[0]);
@ -1129,12 +1127,9 @@ void VideoSendStreamTest::TestPacketFragmentationSize(VideoFormat format,
kVideoSendSsrcs[0], header.sequenceNumber,
packets_lost_, // Cumulative lost.
loss_ratio); // Loss percent.
RtpRtcp::Configuration config;
config.clock = Clock::GetRealTimeClock();
config.receive_statistics = &lossy_receive_stats;
config.outgoing_transport = transport_adapter_.get();
config.rtcp_report_interval_ms = kRtcpIntervalMs;
RTCPSender rtcp_sender(config);
RTCPSender rtcp_sender(false, Clock::GetRealTimeClock(),
&lossy_receive_stats, nullptr, nullptr,
transport_adapter_.get(), kRtcpIntervalMs);
rtcp_sender.SetRTCPStatus(RtcpMode::kReducedSize);
rtcp_sender.SetRemoteSSRC(kVideoSendSsrcs[0]);
@ -1380,12 +1375,8 @@ TEST_F(VideoSendStreamTest, SuspendBelowMinBitrate) {
RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_) {
FakeReceiveStatistics receive_stats(kVideoSendSsrcs[0],
last_sequence_number_, rtp_count_, 0);
RtpRtcp::Configuration config;
config.clock = clock_;
config.receive_statistics = &receive_stats;
config.outgoing_transport = transport_adapter_.get();
config.rtcp_report_interval_ms = kRtcpIntervalMs;
RTCPSender rtcp_sender(config);
RTCPSender rtcp_sender(false, clock_, &receive_stats, nullptr, nullptr,
transport_adapter_.get(), kRtcpIntervalMs);
rtcp_sender.SetRTCPStatus(RtcpMode::kReducedSize);
rtcp_sender.SetRemoteSSRC(kVideoSendSsrcs[0]);