From ef165eefc79cf28bb67779afe303cc2365885547 Mon Sep 17 00:00:00 2001 From: sprang Date: Tue, 22 Sep 2015 05:10:52 -0700 Subject: [PATCH] Wire up send-side bandwidth estimation. BUG=webrtc:4173 Review URL: https://codereview.webrtc.org/1338203003 Cr-Commit-Position: refs/heads/master@{#10012} --- webrtc/modules/modules.gyp | 1 + .../modules/remote_bitrate_estimator/BUILD.gn | 4 + .../transport_feedback_adapter.cc | 1 + .../rtp_rtcp/interface/rtp_rtcp_defines.h | 3 +- .../source/rtcp_format_remb_unittest.cc | 10 +- webrtc/modules/rtp_rtcp/source/rtcp_packet.h | 4 +- .../modules/rtp_rtcp/source/rtcp_receiver.cc | 51 +++++++++- .../modules/rtp_rtcp/source/rtcp_receiver.h | 9 ++ .../rtp_rtcp/source/rtcp_receiver_help.cc | 4 +- .../rtp_rtcp/source/rtcp_receiver_help.h | 5 + .../rtp_rtcp/source/rtcp_receiver_unittest.cc | 33 +++---- webrtc/modules/rtp_rtcp/source/rtcp_sender.h | 3 - .../modules/rtp_rtcp/source/rtcp_utility.cc | 80 +++++++++------- webrtc/modules/rtp_rtcp/source/rtcp_utility.h | 11 +++ .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 1 + webrtc/video/end_to_end_tests.cc | 63 +++++++++++++ webrtc/video/rampup_tests.cc | 94 +++++++++++++++++++ webrtc/video/rampup_tests.h | 6 ++ webrtc/video/screenshare_loopback.cc | 21 ++--- webrtc/video/video_loopback.cc | 22 ++--- webrtc/video/video_quality_test.cc | 11 +++ webrtc/video/video_quality_test.h | 1 + webrtc/video/video_receive_stream.cc | 2 +- webrtc/video/video_send_stream.cc | 2 +- webrtc/video_engine/vie_channel_group.cc | 80 +++++++++++++--- webrtc/video_engine/vie_channel_group.h | 19 +++- 26 files changed, 430 insertions(+), 111 deletions(-) diff --git a/webrtc/modules/modules.gyp b/webrtc/modules/modules.gyp index 4422c33ee7..41db70d85c 100644 --- a/webrtc/modules/modules.gyp +++ b/webrtc/modules/modules.gyp @@ -215,6 +215,7 @@ 'pacing/packet_router_unittest.cc', 'remote_bitrate_estimator/bwe_simulations.cc', 'remote_bitrate_estimator/include/mock/mock_remote_bitrate_observer.h', + 'remote_bitrate_estimator/include/mock/mock_remote_bitrate_estimator.h', 'remote_bitrate_estimator/inter_arrival_unittest.cc', 'remote_bitrate_estimator/overuse_detector_unittest.cc', 'remote_bitrate_estimator/rate_statistics_unittest.cc', diff --git a/webrtc/modules/remote_bitrate_estimator/BUILD.gn b/webrtc/modules/remote_bitrate_estimator/BUILD.gn index 5f0be091df..99c297dda6 100644 --- a/webrtc/modules/remote_bitrate_estimator/BUILD.gn +++ b/webrtc/modules/remote_bitrate_estimator/BUILD.gn @@ -36,7 +36,11 @@ source_set("rbe_components") { "overuse_estimator.h", "remote_bitrate_estimator_abs_send_time.cc", "remote_bitrate_estimator_single_stream.cc", + "remote_estimator_proxy.cc", + "remote_estimator_proxy.h", "send_time_history.cc", + "transport_feedback_adapter.cc", + "transport_feedback_adapter.h", ] configs += [ "../..:common_config" ] diff --git a/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc b/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc index 4c01098f0b..5f51bc55e9 100644 --- a/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc +++ b/webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.cc @@ -107,6 +107,7 @@ void TransportFeedbackAdapter::OnTransportFeedback( << ". Send time history too small?"; } } + RTC_DCHECK(bitrate_estimator_.get() != nullptr); bitrate_estimator_->IncomingPacketFeedbackVector(packet_feedback_vector); } diff --git a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp_defines.h b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp_defines.h index 9ff7406b4f..5e18116c34 100644 --- a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp_defines.h +++ b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp_defines.h @@ -111,7 +111,8 @@ enum RTCPPacketType : uint32_t { kRtcpRemb = 0x10000, kRtcpTransmissionTimeOffset = 0x20000, kRtcpXrReceiverReferenceTime = 0x40000, - kRtcpXrDlrrReportBlock = 0x80000 + kRtcpXrDlrrReportBlock = 0x80000, + kRtcpTransportFeedback = 0x100000, }; enum KeyFrameRequestMethod diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc index a03af2435f..eb6d35ed3e 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc @@ -60,7 +60,11 @@ class RtcpFormatRembTest : public ::testing::Test { RtcpFormatRembTest() : over_use_detector_options_(), system_clock_(Clock::GetRealTimeClock()), + dummy_rtp_rtcp_impl_(nullptr), receive_statistics_(ReceiveStatistics::Create(system_clock_)), + rtcp_sender_(nullptr), + rtcp_receiver_(nullptr), + test_transport_(nullptr), remote_bitrate_observer_(), remote_bitrate_estimator_(new RemoteBitrateEstimatorSingleStream( &remote_bitrate_observer_, @@ -87,9 +91,9 @@ void RtcpFormatRembTest::SetUp() { configuration.remote_bitrate_estimator = remote_bitrate_estimator_.get(); dummy_rtp_rtcp_impl_ = new ModuleRtpRtcpImpl(configuration); rtcp_sender_ = - new RTCPSender(false, system_clock_, receive_statistics_.get(), NULL); - rtcp_receiver_ = new RTCPReceiver(system_clock_, false, NULL, NULL, NULL, - dummy_rtp_rtcp_impl_); + new RTCPSender(false, system_clock_, receive_statistics_.get(), nullptr); + rtcp_receiver_ = new RTCPReceiver(system_clock_, false, nullptr, nullptr, + nullptr, nullptr, dummy_rtp_rtcp_impl_); test_transport_ = new TestTransport(rtcp_receiver_); EXPECT_EQ(0, rtcp_sender_->RegisterSendTransport(test_transport_)); diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet.h b/webrtc/modules/rtp_rtcp/source/rtcp_packet.h index f29540100b..3c34957c36 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_packet.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet.h @@ -91,6 +91,9 @@ class RtcpPacket { size_t max_length, PacketReadyCallback* callback) const; + // Size of this packet in bytes (including headers, excluding nested packets). + virtual size_t BlockLength() const = 0; + protected: RtcpPacket() {} @@ -109,7 +112,6 @@ class RtcpPacket { size_t* index, RtcpPacket::PacketReadyCallback* callback) const; - virtual size_t BlockLength() const = 0; size_t HeaderLength() const; static const size_t kHeaderLength = 4; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc index 4392f52e6a..168557d224 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -17,6 +17,7 @@ #include "webrtc/base/checks.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h" +#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include "webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h" #include "webrtc/system_wrappers/interface/critical_section_wrapper.h" #include "webrtc/system_wrappers/interface/logging.h" @@ -29,12 +30,15 @@ using namespace RTCPHelp; // The number of RTCP time intervals needed to trigger a timeout. const int kRrTimeoutIntervals = 3; +const int64_t kMaxWarningLogIntervalMs = 10000; + RTCPReceiver::RTCPReceiver( Clock* clock, bool receiver_only, RtcpPacketTypeCounterObserver* packet_type_counter_observer, RtcpBandwidthObserver* rtcp_bandwidth_observer, RtcpIntraFrameObserver* rtcp_intra_frame_observer, + TransportFeedbackObserver* transport_feedback_observer, ModuleRtpRtcpImpl* owner) : TMMBRHelp(), _clock(clock), @@ -46,6 +50,7 @@ RTCPReceiver::RTCPReceiver( CriticalSectionWrapper::CreateCriticalSection()), _cbRtcpBandwidthObserver(rtcp_bandwidth_observer), _cbRtcpIntraFrameObserver(rtcp_intra_frame_observer), + _cbTransportFeedbackObserver(transport_feedback_observer), _criticalSectionRTCPReceiver( CriticalSectionWrapper::CreateCriticalSection()), main_ssrc_(0), @@ -61,7 +66,9 @@ RTCPReceiver::RTCPReceiver( _lastReceivedRrMs(0), _lastIncreasedSequenceNumberMs(0), stats_callback_(NULL), - packet_type_counter_observer_(packet_type_counter_observer) { + packet_type_counter_observer_(packet_type_counter_observer), + num_skipped_packets_(0), + last_skipped_packets_warning_(clock->TimeInMilliseconds()) { memset(&_remoteSenderInfo, 0, sizeof(_remoteSenderInfo)); } @@ -349,6 +356,9 @@ RTCPReceiver::IncomingRTCPPacket(RTCPPacketInformation& rtcpPacketInformation, // generic application messages HandleAPPItem(*rtcpParser, rtcpPacketInformation); break; + case RTCPPacketTypes::kTransportFeedback: + HandleTransportFeedback(rtcpParser, &rtcpPacketInformation); + break; default: rtcpParser->Iterate(); break; @@ -361,6 +371,19 @@ RTCPReceiver::IncomingRTCPPacket(RTCPPacketInformation& rtcpPacketInformation, main_ssrc_, packet_type_counter_); } + num_skipped_packets_ += rtcpParser->NumSkippedBlocks(); + + int64_t now = _clock->TimeInMilliseconds(); + if (now - last_skipped_packets_warning_ >= kMaxWarningLogIntervalMs && + num_skipped_packets_ > 0) { + last_skipped_packets_warning_ = now; + LOG(LS_WARNING) + << num_skipped_packets_ + << " RTCP blocks were skipped due to being malformed or of " + "unrecognized/unsupported type, during the past " + << (kMaxWarningLogIntervalMs / 1000) << " second period."; + } + return 0; } @@ -1252,6 +1275,17 @@ void RTCPReceiver::HandleAPPItem(RTCPUtility::RTCPParserV2& rtcpParser, rtcpParser.Iterate(); } +void RTCPReceiver::HandleTransportFeedback( + RTCPUtility::RTCPParserV2* rtcp_parser, + RTCPHelp::RTCPPacketInformation* rtcp_packet_information) { + rtcp::RtcpPacket* packet = rtcp_parser->ReleaseRtcpPacket(); + RTC_DCHECK(packet != nullptr); + rtcp_packet_information->rtcpPacketTypeFlags |= kRtcpTransportFeedback; + rtcp_packet_information->transport_feedback_.reset( + static_cast(packet)); + + rtcp_parser->Iterate(); +} int32_t RTCPReceiver::UpdateTMMBR() { int32_t numBoundingSet = 0; uint32_t bitrate = 0; @@ -1321,11 +1355,11 @@ void RTCPReceiver::TriggerCallbacksFromRTCPPacket( local_ssrc = main_ssrc_; } if (!receiver_only_ && - rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpSrReq) { + (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpSrReq)) { _rtpRtcp.OnRequestSendReport(); } if (!receiver_only_ && - rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpNack) { + (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpNack)) { if (rtcpPacketInformation.nackSequenceNumbers.size() > 0) { LOG(LS_VERBOSE) << "Incoming NACK length: " << rtcpPacketInformation.nackSequenceNumbers.size(); @@ -1376,6 +1410,17 @@ void RTCPReceiver::TriggerCallbacksFromRTCPPacket( now); } } + if (_cbTransportFeedbackObserver && + (rtcpPacketInformation.rtcpPacketTypeFlags & kRtcpTransportFeedback)) { + uint32_t media_source_ssrc = + rtcpPacketInformation.transport_feedback_->GetMediaSourceSsrc(); + if (media_source_ssrc == main_ssrc_ || + registered_ssrcs_.find(media_source_ssrc) != + registered_ssrcs_.end()) { + _cbTransportFeedbackObserver->OnTransportFeedback( + *rtcpPacketInformation.transport_feedback_.get()); + } + } } if (!receiver_only_) { diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h index 9392e51d26..68cf231be1 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h @@ -34,6 +34,7 @@ public: RtcpPacketTypeCounterObserver* packet_type_counter_observer, RtcpBandwidthObserver* rtcp_bandwidth_observer, RtcpIntraFrameObserver* rtcp_intra_frame_observer, + TransportFeedbackObserver* transport_feedback_observer, ModuleRtpRtcpImpl* owner); virtual ~RTCPReceiver(); @@ -216,6 +217,10 @@ protected: void HandleAPPItem(RTCPUtility::RTCPParserV2& rtcpParser, RTCPHelp::RTCPPacketInformation& rtcpPacketInformation); + void HandleTransportFeedback( + RTCPUtility::RTCPParserV2* rtcp_parser, + RTCPHelp::RTCPPacketInformation* rtcp_packet_information); + private: typedef std::map ReceivedInfoMap; @@ -241,6 +246,7 @@ protected: CriticalSectionWrapper* _criticalSectionFeedbacks; RtcpBandwidthObserver* const _cbRtcpBandwidthObserver; RtcpIntraFrameObserver* const _cbRtcpIntraFrameObserver; + TransportFeedbackObserver* const _cbTransportFeedbackObserver; CriticalSectionWrapper* _criticalSectionRTCPReceiver; uint32_t main_ssrc_; @@ -282,6 +288,9 @@ protected: RtcpPacketTypeCounter packet_type_counter_; RTCPUtility::NackStats nack_stats_; + + size_t num_skipped_packets_; + int64_t last_skipped_packets_warning_; }; } // namespace webrtc #endif // WEBRTC_MODULES_RTP_RTCP_SOURCE_RTCP_RECEIVER_H_ diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc index b86e5cc2d8..718990d10b 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.cc @@ -13,6 +13,7 @@ #include // assert #include // memset +#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include "webrtc/modules/rtp_rtcp/source/rtp_utility.h" namespace webrtc { @@ -36,8 +37,7 @@ RTCPPacketInformation::RTCPPacketInformation() rtp_timestamp(0), xr_originator_ssrc(0), xr_dlrr_item(false), - VoIPMetric(NULL) { -} + VoIPMetric(nullptr) {} RTCPPacketInformation::~RTCPPacketInformation() { diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.h b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.h index 790fe819ef..37b7b88370 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_help.h @@ -20,6 +20,9 @@ #include "webrtc/typedefs.h" namespace webrtc { +namespace rtcp { +class TransportFeedback; +} namespace RTCPHelp { @@ -84,6 +87,8 @@ public: bool xr_dlrr_item; RTCPVoIPMetric* VoIPMetric; + rtc::scoped_ptr transport_feedback_; + private: RTC_DISALLOW_COPY_AND_ASSIGN(RTCPPacketInformation); }; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index 6ef2a148fa..e09c29d72c 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -33,9 +33,7 @@ namespace { // Anonymous namespace; hide utility functions and classes. class TestTransport : public Transport, public NullRtpData { public: - explicit TestTransport() - : rtcp_receiver_(NULL) { - } + explicit TestTransport() : rtcp_receiver_(nullptr) {} void SetRTCPReceiver(RTCPReceiver* rtcp_receiver) { rtcp_receiver_ = rtcp_receiver; } @@ -78,8 +76,8 @@ class RtcpReceiverTest : public ::testing::Test { configuration.outgoing_transport = test_transport_; configuration.remote_bitrate_estimator = remote_bitrate_estimator_.get(); rtp_rtcp_impl_ = new ModuleRtpRtcpImpl(configuration); - rtcp_receiver_ = new RTCPReceiver(&system_clock_, false, NULL, NULL, NULL, - rtp_rtcp_impl_); + rtcp_receiver_ = new RTCPReceiver(&system_clock_, false, nullptr, nullptr, + nullptr, nullptr, rtp_rtcp_impl_); test_transport_->SetRTCPReceiver(rtcp_receiver_); } ~RtcpReceiverTest() { @@ -362,7 +360,8 @@ TEST_F(RtcpReceiverTest, GetRtt) { rtcp_receiver_->SetSsrcs(kSourceSsrc, ssrcs); // No report block received. - EXPECT_EQ(-1, rtcp_receiver_->RTT(kSenderSsrc, NULL, NULL, NULL, NULL)); + EXPECT_EQ( + -1, rtcp_receiver_->RTT(kSenderSsrc, nullptr, nullptr, nullptr, nullptr)); rtcp::ReportBlock rb; rb.To(kSourceSsrc); @@ -374,10 +373,12 @@ TEST_F(RtcpReceiverTest, GetRtt) { EXPECT_EQ(kSenderSsrc, rtcp_packet_info_.remoteSSRC); EXPECT_EQ(kRtcpRr, rtcp_packet_info_.rtcpPacketTypeFlags); EXPECT_EQ(1u, rtcp_packet_info_.report_blocks.size()); - EXPECT_EQ(0, rtcp_receiver_->RTT(kSenderSsrc, NULL, NULL, NULL, NULL)); + EXPECT_EQ( + 0, rtcp_receiver_->RTT(kSenderSsrc, nullptr, nullptr, nullptr, nullptr)); // Report block not received. - EXPECT_EQ(-1, rtcp_receiver_->RTT(kSenderSsrc + 1, NULL, NULL, NULL, NULL)); + EXPECT_EQ(-1, rtcp_receiver_->RTT(kSenderSsrc + 1, nullptr, nullptr, nullptr, + nullptr)); } TEST_F(RtcpReceiverTest, InjectIjWithNoItem) { @@ -589,7 +590,7 @@ TEST_F(RtcpReceiverTest, InjectXrVoipPacket) { xr.WithVoipMetric(&voip_metric); rtc::scoped_ptr packet(xr.Build()); EXPECT_EQ(0, InjectRtcpPacket(packet->Buffer(), packet->Length())); - ASSERT_TRUE(rtcp_packet_info_.VoIPMetric != NULL); + ASSERT_TRUE(rtcp_packet_info_.VoIPMetric != nullptr); EXPECT_EQ(kLossRate, rtcp_packet_info_.VoIPMetric->lossRate); EXPECT_EQ(kRtcpXrVoipMetric, rtcp_packet_info_.rtcpPacketTypeFlags); } @@ -843,7 +844,7 @@ TEST_F(RtcpReceiverTest, ReceiveReportTimeout) { TEST_F(RtcpReceiverTest, TmmbrReceivedWithNoIncomingPacket) { // This call is expected to fail because no data has arrived. - EXPECT_EQ(-1, rtcp_receiver_->TMMBRReceived(0, 0, NULL)); + EXPECT_EQ(-1, rtcp_receiver_->TMMBRReceived(0, 0, nullptr)); } TEST_F(RtcpReceiverTest, TmmbrPacketAccepted) { @@ -864,7 +865,7 @@ TEST_F(RtcpReceiverTest, TmmbrPacketAccepted) { rtc::scoped_ptr packet(sr.Build()); EXPECT_EQ(0, InjectRtcpPacket(packet->Buffer(), packet->Length())); - EXPECT_EQ(1, rtcp_receiver_->TMMBRReceived(0, 0, NULL)); + EXPECT_EQ(1, rtcp_receiver_->TMMBRReceived(0, 0, nullptr)); TMMBRSet candidate_set; candidate_set.VerifyAndAllocateSet(1); EXPECT_EQ(1, rtcp_receiver_->TMMBRReceived(1, 0, &candidate_set)); @@ -890,7 +891,7 @@ TEST_F(RtcpReceiverTest, TmmbrPacketNotForUsIgnored) { ssrcs.insert(kMediaFlowSsrc); rtcp_receiver_->SetSsrcs(kMediaFlowSsrc, ssrcs); EXPECT_EQ(0, InjectRtcpPacket(packet->Buffer(), packet->Length())); - EXPECT_EQ(0, rtcp_receiver_->TMMBRReceived(0, 0, NULL)); + EXPECT_EQ(0, rtcp_receiver_->TMMBRReceived(0, 0, nullptr)); } TEST_F(RtcpReceiverTest, TmmbrPacketZeroRateIgnored) { @@ -911,7 +912,7 @@ TEST_F(RtcpReceiverTest, TmmbrPacketZeroRateIgnored) { rtc::scoped_ptr packet(sr.Build()); EXPECT_EQ(0, InjectRtcpPacket(packet->Buffer(), packet->Length())); - EXPECT_EQ(0, rtcp_receiver_->TMMBRReceived(0, 0, NULL)); + EXPECT_EQ(0, rtcp_receiver_->TMMBRReceived(0, 0, nullptr)); } TEST_F(RtcpReceiverTest, TmmbrThreeConstraintsTimeOut) { @@ -938,7 +939,7 @@ TEST_F(RtcpReceiverTest, TmmbrThreeConstraintsTimeOut) { system_clock_.AdvanceTimeMilliseconds(5000); } // It is now starttime + 15. - EXPECT_EQ(3, rtcp_receiver_->TMMBRReceived(0, 0, NULL)); + EXPECT_EQ(3, rtcp_receiver_->TMMBRReceived(0, 0, nullptr)); TMMBRSet candidate_set; candidate_set.VerifyAndAllocateSet(3); EXPECT_EQ(3, rtcp_receiver_->TMMBRReceived(3, 0, &candidate_set)); @@ -947,7 +948,7 @@ TEST_F(RtcpReceiverTest, TmmbrThreeConstraintsTimeOut) { // seconds, timing out the first packet. system_clock_.AdvanceTimeMilliseconds(12000); // Odd behaviour: Just counting them does not trigger the timeout. - EXPECT_EQ(3, rtcp_receiver_->TMMBRReceived(0, 0, NULL)); + EXPECT_EQ(3, rtcp_receiver_->TMMBRReceived(0, 0, nullptr)); EXPECT_EQ(2, rtcp_receiver_->TMMBRReceived(3, 0, &candidate_set)); EXPECT_EQ(kSenderSsrc + 1, candidate_set.Ssrc(0)); } @@ -1008,7 +1009,7 @@ TEST_F(RtcpReceiverTest, Callbacks) { EXPECT_TRUE(callback.Matches(kSourceSsrc, kSequenceNumber, kFractionLoss, kCumulativeLoss, kJitter)); - rtcp_receiver_->RegisterRtcpStatisticsCallback(NULL); + rtcp_receiver_->RegisterRtcpStatisticsCallback(nullptr); // Add arbitrary numbers, callback should not be called (retain old values). rtcp::ReportBlock rb2; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.h b/webrtc/modules/rtp_rtcp/source/rtcp_sender.h index e0195f1758..fa6468e519 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.h @@ -33,9 +33,6 @@ namespace webrtc { class ModuleRtpRtcpImpl; class RTCPReceiver; -namespace rtcp { -class TransportFeedback; -} class NACKStringBuilder { public: NACKStringBuilder(); diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_utility.cc b/webrtc/modules/rtp_rtcp/source/rtcp_utility.cc index caffb6342c..6c1deb467e 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_utility.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_utility.cc @@ -8,7 +8,9 @@ * be found in the AUTHORS file in the root of the source tree. */ +#include "webrtc/base/checks.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h" +#include "webrtc/modules/rtp_rtcp/source/rtcp_packet/transport_feedback.h" #include #include // ceil @@ -55,6 +57,7 @@ RTCPUtility::RTCPParserV2::RTCPParserV2(const uint8_t* rtcpData, _ptrRTCPBlockEnd(NULL), _state(ParseState::State_TopLevel), _numberOfBlocks(0), + num_skipped_blocks_(0), _packetType(RTCPPacketTypes::kInvalid) { Validate(); } @@ -80,6 +83,9 @@ RTCPUtility::RTCPParserV2::Packet() const return _packet; } +rtcp::RtcpPacket* RTCPUtility::RTCPParserV2::ReleaseRtcpPacket() { + return rtcp_packet_.release(); +} RTCPUtility::RTCPPacketTypes RTCPUtility::RTCPParserV2::Begin() { @@ -147,7 +153,7 @@ RTCPUtility::RTCPParserV2::Iterate() IterateAppItem(); break; default: - assert(false); // Invalid state! + RTC_NOTREACHED() << "Invalid state!"; break; } } @@ -170,7 +176,7 @@ RTCPUtility::RTCPParserV2::IterateTopLevel() _ptrRTCPBlockEnd = _ptrRTCPData + header.BlockSize(); if (_ptrRTCPBlockEnd > _ptrRTCPDataEnd) { - // Bad block! + ++num_skipped_blocks_; return; } @@ -219,16 +225,15 @@ RTCPUtility::RTCPParserV2::IterateTopLevel() ParseIJ(); return; } - case PT_RTPFB: // Fall through! + case PT_RTPFB: + FALLTHROUGH(); case PT_PSFB: { - const bool ok = ParseFBCommon(header); - if (!ok) - { - // Nothing supported found, continue to next block! - break; - } - return; + if (!ParseFBCommon(header)) { + // Nothing supported found, continue to next block! + break; + } + return; } case PT_APP: { @@ -252,6 +257,7 @@ RTCPUtility::RTCPParserV2::IterateTopLevel() } default: // Not supported! Skip! + ++num_skipped_blocks_; EndCurrentBlock(); break; } @@ -1160,28 +1166,26 @@ bool RTCPUtility::RTCPParserV2::ParseXrUnsupportedBlockType( } bool RTCPUtility::RTCPParserV2::ParseFBCommon(const RtcpCommonHeader& header) { - assert((header.packet_type == PT_RTPFB) || - (header.packet_type == PT_PSFB)); // Parser logic check + RTC_CHECK((header.packet_type == PT_RTPFB) || + (header.packet_type == PT_PSFB)); // Parser logic check const ptrdiff_t length = _ptrRTCPBlockEnd - _ptrRTCPData; - if (length < 12) // 4 * 3, RFC4585 section 6.1 - { - EndCurrentBlock(); + // 4 * 3, RFC4585 section 6.1 + if (length < 12) { + LOG(LS_WARNING) + << "Invalid RTCP packet: Too little data (" << length + << " bytes) left in buffer to parse a 12 byte RTPFB/PSFB message."; return false; } _ptrRTCPData += 4; // Skip RTCP header - uint32_t senderSSRC = *_ptrRTCPData++ << 24; - senderSSRC += *_ptrRTCPData++ << 16; - senderSSRC += *_ptrRTCPData++ << 8; - senderSSRC += *_ptrRTCPData++; + uint32_t senderSSRC = ByteReader::ReadBigEndian(_ptrRTCPData); + _ptrRTCPData += 4; - uint32_t mediaSSRC = *_ptrRTCPData++ << 24; - mediaSSRC += *_ptrRTCPData++ << 16; - mediaSSRC += *_ptrRTCPData++ << 8; - mediaSSRC += *_ptrRTCPData++; + uint32_t mediaSSRC = ByteReader::ReadBigEndian(_ptrRTCPData); + _ptrRTCPData += 4; if (header.packet_type == PT_RTPFB) { // Transport layer feedback @@ -1198,12 +1202,6 @@ bool RTCPUtility::RTCPParserV2::ParseFBCommon(const RtcpCommonHeader& header) { return true; } - case 2: - { - // used to be ACK is this code point, which is removed - // conficts with http://tools.ietf.org/html/draft-levin-avt-rtcp-burst-00 - break; - } case 3: { // TMMBR @@ -1236,10 +1234,23 @@ bool RTCPUtility::RTCPParserV2::ParseFBCommon(const RtcpCommonHeader& header) { // Note: No state transition, SR REQ is empty! return true; } + case 15: { + _packetType = RTCPPacketTypes::kTransportFeedback; + rtcp_packet_ = + rtcp::TransportFeedback::ParseFrom(_ptrRTCPData - 12, length); + // Since we parse the whole packet here, keep the TopLevel state and + // just end the current block. + if (rtcp_packet_.get()) { + EndCurrentBlock(); + return true; + } + break; + } default: break; } - EndCurrentBlock(); + // Unsupported RTPFB message. Skip and move to next block. + ++num_skipped_blocks_; return false; } else if (header.packet_type == PT_PSFB) { // Payload specific feedback @@ -1287,14 +1298,11 @@ bool RTCPUtility::RTCPParserV2::ParseFBCommon(const RtcpCommonHeader& header) { break; } - EndCurrentBlock(); return false; } else { - assert(false); - - EndCurrentBlock(); + RTC_NOTREACHED(); return false; } } @@ -1654,6 +1662,10 @@ RTCPUtility::RTCPParserV2::ParseAPPItem() return true; } +size_t RTCPUtility::RTCPParserV2::NumSkippedBlocks() const { + return num_skipped_blocks_; +} + RTCPUtility::RTCPPacketIterator::RTCPPacketIterator(uint8_t* rtcpData, size_t rtcpDataLength) : _ptrBegin(rtcpData), diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_utility.h b/webrtc/modules/rtp_rtcp/source/rtcp_utility.h index 73658a01f8..f05d512919 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_utility.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_utility.h @@ -13,11 +13,15 @@ #include // size_t, ptrdiff_t +#include "webrtc/base/scoped_ptr.h" #include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp_defines.h" #include "webrtc/modules/rtp_rtcp/source/rtp_rtcp_config.h" #include "webrtc/typedefs.h" namespace webrtc { +namespace rtcp { +class RtcpPacket; +} namespace RTCPUtility { class NackStats { @@ -294,6 +298,9 @@ enum class RTCPPacketTypes { kApp, kAppItem, + + // draft-holmer-rmcat-transport-wide-cc-extensions + kTransportFeedback, }; struct RTCPRawPacket { @@ -359,10 +366,12 @@ class RTCPParserV2 { RTCPPacketTypes PacketType() const; const RTCPPacket& Packet() const; + rtcp::RtcpPacket* ReleaseRtcpPacket(); const RTCPRawPacket& RawPacket() const; ptrdiff_t LengthLeft() const; bool IsValid() const; + size_t NumSkippedBlocks() const; RTCPPacketTypes Begin(); RTCPPacketTypes Iterate(); @@ -454,9 +463,11 @@ class RTCPParserV2 { ParseState _state; uint8_t _numberOfBlocks; + size_t num_skipped_blocks_; RTCPPacketTypes _packetType; RTCPPacket _packet; + rtc::scoped_ptr rtcp_packet_; }; class RTCPPacketIterator { diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index d941201cb1..ae4caf7bde 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -78,6 +78,7 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) configuration.rtcp_packet_type_counter_observer, configuration.bandwidth_callback, configuration.intra_frame_callback, + configuration.transport_feedback_callback, this), clock_(configuration.clock), audio_(configuration.audio), diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index 00e7279adb..7224478e14 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -15,6 +15,7 @@ #include "testing/gtest/include/gtest/gtest.h" #include "webrtc/base/checks.h" +#include "webrtc/base/event.h" #include "webrtc/base/scoped_ptr.h" #include "webrtc/call.h" #include "webrtc/frame_callback.h" @@ -1445,6 +1446,68 @@ TEST_F(EndToEndTest, AssignsTransportSequenceNumbers) { tester.RunTest(); } +TEST_F(EndToEndTest, ReceivesTransportFeedback) { + static const int kExtensionId = 5; + + class TransportFeedbackObserver : public test::DirectTransport { + public: + TransportFeedbackObserver(rtc::Event* done_event) : done_(done_event) {} + virtual ~TransportFeedbackObserver() {} + + bool SendRtcp(const uint8_t* data, size_t length) override { + RTCPUtility::RTCPParserV2 parser(data, length, true); + EXPECT_TRUE(parser.IsValid()); + + RTCPUtility::RTCPPacketTypes packet_type = parser.Begin(); + while (packet_type != RTCPUtility::RTCPPacketTypes::kInvalid) { + if (packet_type == RTCPUtility::RTCPPacketTypes::kTransportFeedback) { + done_->Set(); + break; + } + packet_type = parser.Iterate(); + } + + return test::DirectTransport::SendRtcp(data, length); + } + + rtc::Event* done_; + }; + + class TransportFeedbackTester : public MultiStreamTest { + public: + TransportFeedbackTester() : done_(false, false) {} + virtual ~TransportFeedbackTester() {} + + protected: + void Wait() override { + EXPECT_TRUE(done_.Wait(CallTest::kDefaultTimeoutMs)); + } + + void UpdateSendConfig( + size_t stream_index, + VideoSendStream::Config* send_config, + VideoEncoderConfig* encoder_config, + test::FrameGeneratorCapturer** frame_generator) override { + send_config->rtp.extensions.push_back( + RtpExtension(RtpExtension::kTransportSequenceNumber, kExtensionId)); + } + + void UpdateReceiveConfig( + size_t stream_index, + VideoReceiveStream::Config* receive_config) override { + receive_config->rtp.extensions.push_back( + RtpExtension(RtpExtension::kTransportSequenceNumber, kExtensionId)); + } + + virtual test::DirectTransport* CreateReceiveTransport() { + return new TransportFeedbackObserver(&done_); + } + + private: + rtc::Event done_; + } tester; + tester.RunTest(); +} TEST_F(EndToEndTest, ObserversEncodedFrames) { class EncodedFrameTestObserver : public EncodedFrameObserver { public: diff --git a/webrtc/video/rampup_tests.cc b/webrtc/video/rampup_tests.cc index d308f2ddb7..fd60fc4d64 100644 --- a/webrtc/video/rampup_tests.cc +++ b/webrtc/video/rampup_tests.cc @@ -11,14 +11,18 @@ #include "testing/gtest/include/gtest/gtest.h" #include "webrtc/base/checks.h" #include "webrtc/base/common.h" +#include "webrtc/base/event.h" +#include "webrtc/modules/pacing/include/packet_router.h" #include "webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h" #include "webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h" +#include "webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.h" #include "webrtc/modules/rtp_rtcp/interface/receive_statistics.h" #include "webrtc/modules/rtp_rtcp/interface/rtp_header_parser.h" #include "webrtc/modules/rtp_rtcp/interface/rtp_payload_registry.h" #include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h" #include "webrtc/system_wrappers/interface/critical_section_wrapper.h" +#include "webrtc/system_wrappers/interface/thread_wrapper.h" #include "webrtc/test/testsupport/perf_test.h" #include "webrtc/video/rampup_tests.h" @@ -70,14 +74,22 @@ StreamObserver::StreamObserver(const SsrcMap& rtx_media_ssrcs, rtp_rtcp_.reset(RtpRtcp::CreateRtpRtcp(config)); rtp_rtcp_->SetREMBStatus(true); rtp_rtcp_->SetRTCPStatus(kRtcpNonCompound); + packet_router_.reset(new PacketRouter()); + packet_router_->AddRtpModule(rtp_rtcp_.get()); rtp_parser_->RegisterRtpHeaderExtension(kRtpExtensionAbsoluteSendTime, kAbsSendTimeExtensionId); rtp_parser_->RegisterRtpHeaderExtension(kRtpExtensionTransmissionTimeOffset, kTransmissionTimeOffsetExtensionId); + rtp_parser_->RegisterRtpHeaderExtension(kRtpExtensionTransportSequenceNumber, + kTransportSequenceNumberExtensionId); payload_registry_->SetRtxPayloadType(RampUpTest::kSendRtxPayloadType, RampUpTest::kFakeSendPayloadType); } +StreamObserver::~StreamObserver() { + packet_router_->RemoveRtpModule(rtp_rtcp_.get()); +} + void StreamObserver::set_expected_bitrate_bps( unsigned int expected_bitrate_bps) { rtc::CritScope lock(&crit_); @@ -163,6 +175,10 @@ void StreamObserver::SetRemoteBitrateEstimator(RemoteBitrateEstimator* rbe) { remote_bitrate_estimator_.reset(rbe); } +PacketRouter* StreamObserver::GetPacketRouter() { + return packet_router_.get(); +} + void StreamObserver::ReportResult(const std::string& measurement, size_t value, const std::string& units) { @@ -371,6 +387,48 @@ EventTypeWrapper LowRateStreamObserver::Wait() { return test_done_->Wait(test::CallTest::kLongTimeoutMs); } +class SendBitrateAdapter { + public: + static const int64_t kPollIntervalMs = 250; + + SendBitrateAdapter(const Call& call, + const std::vector& ssrcs, + RemoteBitrateObserver* bitrate_observer) + : event_(false, false), + call_(call), + ssrcs_(ssrcs), + bitrate_observer_(bitrate_observer) { + RTC_DCHECK(bitrate_observer != nullptr); + poller_thread_ = ThreadWrapper::CreateThread(&SendBitrateAdapterThread, + this, "SendBitratePoller"); + RTC_DCHECK(poller_thread_->Start()); + } + + virtual ~SendBitrateAdapter() { + event_.Set(); + poller_thread_->Stop(); + } + + private: + static bool SendBitrateAdapterThread(void* obj) { + return static_cast(obj)->PollStats(); + } + + bool PollStats() { + Call::Stats stats = call_.GetStats(); + + bitrate_observer_->OnReceiveBitrateChanged(ssrcs_, + stats.send_bandwidth_bps); + return !event_.Wait(kPollIntervalMs); + } + + rtc::Event event_; + rtc::scoped_ptr poller_thread_; + const Call& call_; + const std::vector ssrcs_; + RemoteBitrateObserver* const bitrate_observer_; +}; + void RampUpTest::RunRampUpTest(size_t num_streams, unsigned int start_bitrate_bps, const std::string& extension_type, @@ -391,6 +449,8 @@ void RampUpTest::RunRampUpTest(size_t num_streams, CreateSendConfig(num_streams, &stream_observer); send_config_.rtp.extensions.clear(); + rtc::scoped_ptr send_bitrate_adapter_; + if (extension_type == RtpExtension::kAbsSendTime) { stream_observer.SetRemoteBitrateEstimator( new RemoteBitrateEstimatorAbsSendTime( @@ -398,6 +458,11 @@ void RampUpTest::RunRampUpTest(size_t num_streams, kRemoteBitrateEstimatorMinBitrateBps)); send_config_.rtp.extensions.push_back(RtpExtension( extension_type.c_str(), kAbsSendTimeExtensionId)); + } else if (extension_type == RtpExtension::kTransportSequenceNumber) { + stream_observer.SetRemoteBitrateEstimator(new RemoteEstimatorProxy( + Clock::GetRealTimeClock(), stream_observer.GetPacketRouter())); + send_config_.rtp.extensions.push_back(RtpExtension( + extension_type.c_str(), kTransportSequenceNumberExtensionId)); } else { stream_observer.SetRemoteBitrateEstimator( new RemoteBitrateEstimatorSingleStream( @@ -449,10 +514,18 @@ void RampUpTest::RunRampUpTest(size_t num_streams, CreateStreams(); CreateFrameGeneratorCapturer(); + if (extension_type == RtpExtension::kTransportSequenceNumber) { + send_bitrate_adapter_.reset( + new SendBitrateAdapter(*sender_call_.get(), ssrcs, &stream_observer)); + } Start(); EXPECT_EQ(kEventSignaled, stream_observer.Wait()); + // Destroy the SendBitrateAdapter (if any) to stop the poller thread in it, + // otherwise we might get a data race with the destruction of the call. + send_bitrate_adapter_.reset(); + Stop(); DestroyStreams(); } @@ -563,4 +636,25 @@ TEST_F(RampUpTest, AbsSendTimeSingleStreamWithHighStartBitrate) { RunRampUpTest(1, 0.9 * kSingleStreamTargetBps, RtpExtension::kAbsSendTime, false, false); } + +TEST_F(RampUpTest, TransportSequenceNumberSingleStream) { + RunRampUpTest(1, 0, RtpExtension::kTransportSequenceNumber, false, false); +} + +TEST_F(RampUpTest, TransportSequenceNumberSimulcast) { + RunRampUpTest(3, 0, RtpExtension::kTransportSequenceNumber, false, false); +} + +TEST_F(RampUpTest, TransportSequenceNumberSimulcastWithRtx) { + RunRampUpTest(3, 0, RtpExtension::kTransportSequenceNumber, true, false); +} + +TEST_F(RampUpTest, TransportSequenceNumberSimulcastByRedWithRtx) { + RunRampUpTest(3, 0, RtpExtension::kTransportSequenceNumber, true, true); +} + +TEST_F(RampUpTest, TransportSequenceNumberSingleStreamWithHighStartBitrate) { + RunRampUpTest(1, 0.9 * kSingleStreamTargetBps, + RtpExtension::kTransportSequenceNumber, false, false); +} } // namespace webrtc diff --git a/webrtc/video/rampup_tests.h b/webrtc/video/rampup_tests.h index 56c5e75a6e..8c18a52eb9 100644 --- a/webrtc/video/rampup_tests.h +++ b/webrtc/video/rampup_tests.h @@ -26,9 +26,11 @@ namespace webrtc { static const int kTransmissionTimeOffsetExtensionId = 6; static const int kAbsSendTimeExtensionId = 7; +static const int kTransportSequenceNumberExtensionId = 8; static const unsigned int kSingleStreamTargetBps = 1000000; class Clock; +class PacketRouter; class ReceiveStatistics; class RtpHeaderParser; class RTPPayloadRegistry; @@ -41,6 +43,7 @@ class StreamObserver : public newapi::Transport, public RemoteBitrateObserver { StreamObserver(const SsrcMap& rtx_media_ssrcs, newapi::Transport* feedback_transport, Clock* clock); + virtual ~StreamObserver(); void set_expected_bitrate_bps(unsigned int expected_bitrate_bps); @@ -57,6 +60,8 @@ class StreamObserver : public newapi::Transport, public RemoteBitrateObserver { void SetRemoteBitrateEstimator(RemoteBitrateEstimator* rbe); + PacketRouter* GetPacketRouter(); + private: void ReportResult(const std::string& measurement, size_t value, @@ -67,6 +72,7 @@ class StreamObserver : public newapi::Transport, public RemoteBitrateObserver { const rtc::scoped_ptr test_done_; const rtc::scoped_ptr rtp_parser_; rtc::scoped_ptr rtp_rtcp_; + rtc::scoped_ptr packet_router_; internal::TransportAdapter feedback_transport_; const rtc::scoped_ptr receive_stats_; const rtc::scoped_ptr payload_registry_; diff --git a/webrtc/video/screenshare_loopback.cc b/webrtc/video/screenshare_loopback.cc index 34f68947f6..9897783eb9 100644 --- a/webrtc/video/screenshare_loopback.cc +++ b/webrtc/video/screenshare_loopback.cc @@ -139,6 +139,8 @@ int DurationSecs() { return static_cast(FLAGS_duration); } +DEFINE_bool(send_side_bwe, true, "Use send-side bandwidth estimation"); + DEFINE_string( force_fieldtrials, "", @@ -162,19 +164,12 @@ void Loopback() { call_bitrate_config.max_bitrate_bps = flags::MaxBitrateKbps() * 1000; VideoQualityTest::Params params{ - { - flags::Width(), - flags::Height(), - flags::Fps(), - flags::MinBitrateKbps() * 1000, - flags::TargetBitrateKbps() * 1000, - flags::MaxBitrateKbps() * 1000, - flags::Codec(), - flags::NumTemporalLayers(), - flags::MinTransmitBitrateKbps() * 1000, - call_bitrate_config, - flags::TLDiscardThreshold() - }, + {flags::Width(), flags::Height(), flags::Fps(), + flags::MinBitrateKbps() * 1000, flags::TargetBitrateKbps() * 1000, + flags::MaxBitrateKbps() * 1000, flags::Codec(), + flags::NumTemporalLayers(), flags::MinTransmitBitrateKbps() * 1000, + call_bitrate_config, flags::TLDiscardThreshold(), + flags::FLAGS_send_side_bwe}, {}, // Video specific. {true, flags::SlideChangeInterval(), flags::ScrollDuration()}, {"screenshare", 0.0, 0.0, flags::DurationSecs(), flags::OutputFilename()}, diff --git a/webrtc/video/video_loopback.cc b/webrtc/video/video_loopback.cc index c5e9254c08..0dceea82e6 100644 --- a/webrtc/video/video_loopback.cc +++ b/webrtc/video/video_loopback.cc @@ -140,6 +140,8 @@ int DurationSecs() { return static_cast(FLAGS_duration); } +DEFINE_bool(send_side_bwe, true, "Use send-side bandwidth estimation"); + } // namespace flags void Loopback() { @@ -158,19 +160,13 @@ void Loopback() { std::string clip = flags::Clip(); std::string graph_title = clip.empty() ? "" : "video " + clip; VideoQualityTest::Params params{ - { - flags::Width(), - flags::Height(), - flags::Fps(), - flags::MinBitrateKbps() * 1000, - flags::TargetBitrateKbps() * 1000, - flags::MaxBitrateKbps() * 1000, - flags::Codec(), - flags::NumTemporalLayers(), - 0, // No min transmit bitrate. - call_bitrate_config, - flags::TLDiscardThreshold() - }, + {flags::Width(), flags::Height(), flags::Fps(), + flags::MinBitrateKbps() * 1000, flags::TargetBitrateKbps() * 1000, + flags::MaxBitrateKbps() * 1000, flags::Codec(), + flags::NumTemporalLayers(), + 0, // No min transmit bitrate. + call_bitrate_config, flags::TLDiscardThreshold(), + flags::FLAGS_send_side_bwe}, {clip}, {}, // Screenshare specific. {graph_title, 0.0, 0.0, flags::DurationSecs(), flags::OutputFilename()}, diff --git a/webrtc/video/video_quality_test.cc b/webrtc/video/video_quality_test.cc index d440858155..cbd1e6065d 100644 --- a/webrtc/video/video_quality_test.cc +++ b/webrtc/video/video_quality_test.cc @@ -32,6 +32,8 @@ namespace webrtc { +static const int kTransportSeqExtensionId = + VideoQualityTest::kAbsSendTimeExtensionId + 1; static const int kSendStatsPollingIntervalMs = 1000; static const int kPayloadTypeVP8 = 123; static const int kPayloadTypeVP9 = 124; @@ -598,6 +600,15 @@ void VideoQualityTest::SetupFullStack(const Params& params, send_config_.rtp.rtx.ssrcs.push_back(kSendRtxSsrcs[0]); send_config_.rtp.rtx.payload_type = kSendRtxPayloadType; + send_config_.rtp.extensions.clear(); + if (params.common.send_side_bwe) { + send_config_.rtp.extensions.push_back(RtpExtension( + RtpExtension::kTransportSequenceNumber, kTransportSeqExtensionId)); + } else { + send_config_.rtp.extensions.push_back( + RtpExtension(RtpExtension::kAbsSendTime, kAbsSendTimeExtensionId)); + } + // Automatically fill out streams[0] with params. VideoStream* stream = &encoder_config_.streams[0]; stream->width = params.common.width; diff --git a/webrtc/video/video_quality_test.h b/webrtc/video/video_quality_test.h index d606de7cac..d403d655e9 100644 --- a/webrtc/video/video_quality_test.h +++ b/webrtc/video/video_quality_test.h @@ -38,6 +38,7 @@ class VideoQualityTest : public test::CallTest { int min_transmit_bps; Call::Config::BitrateConfig call_bitrate_config; size_t tl_discard_threshold; + bool send_side_bwe; } common; struct { // Video-specific settings. std::string clip_name; diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index 2c8d8f85f3..1be63e07d1 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -140,7 +140,7 @@ VideoReceiveStream::VideoReceiveStream(int num_cpu_cores, channel_group_(channel_group), channel_id_(channel_id) { RTC_CHECK(channel_group_->CreateReceiveChannel( - channel_id_, &transport_adapter_, num_cpu_cores)); + channel_id_, &transport_adapter_, num_cpu_cores, config)); vie_channel_ = channel_group_->GetChannel(channel_id_); diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index 462d252116..c9cdfa860c 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -119,7 +119,7 @@ VideoSendStream::VideoSendStream( stats_proxy_(Clock::GetRealTimeClock(), config) { RTC_DCHECK(!config_.rtp.ssrcs.empty()); RTC_CHECK(channel_group->CreateSendChannel(channel_id_, &transport_adapter_, - num_cpu_cores, config_.rtp.ssrcs)); + num_cpu_cores, config_)); vie_channel_ = channel_group_->GetChannel(channel_id_); vie_encoder_ = channel_group_->GetEncoder(channel_id_); diff --git a/webrtc/video_engine/vie_channel_group.cc b/webrtc/video_engine/vie_channel_group.cc index 42f6c852f0..ce04795682 100644 --- a/webrtc/video_engine/vie_channel_group.cc +++ b/webrtc/video_engine/vie_channel_group.cc @@ -18,6 +18,7 @@ #include "webrtc/modules/remote_bitrate_estimator/include/send_time_history.h" #include "webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h" #include "webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h" +#include "webrtc/modules/remote_bitrate_estimator/remote_estimator_proxy.h" #include "webrtc/modules/remote_bitrate_estimator/transport_feedback_adapter.h" #include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h" #include "webrtc/modules/utility/interface/process_thread.h" @@ -145,7 +146,6 @@ ChannelGroup::ChannelGroup(ProcessThread* process_thread) : remb_(new VieRemb()), bitrate_allocator_(new BitrateAllocator()), call_stats_(new CallStats()), - encoder_state_feedback_(new EncoderStateFeedback()), packet_router_(new PacketRouter()), pacer_(new PacedSender(Clock::GetRealTimeClock(), packet_router_.get(), @@ -153,6 +153,12 @@ ChannelGroup::ChannelGroup(ProcessThread* process_thread) PacedSender::kDefaultPaceMultiplier * BitrateController::kDefaultStartBitrateKbps, 0)), + remote_bitrate_estimator_( + new WrappingBitrateEstimator(remb_.get(), Clock::GetRealTimeClock())), + remote_estimator_proxy_( + new RemoteEstimatorProxy(Clock::GetRealTimeClock(), + packet_router_.get())), + encoder_state_feedback_(new EncoderStateFeedback()), process_thread_(process_thread), pacer_thread_(ProcessThread::Create("PacerThread")), // Constructed last as this object calls the provided callback on @@ -160,14 +166,12 @@ ChannelGroup::ChannelGroup(ProcessThread* process_thread) bitrate_controller_( BitrateController::CreateBitrateController(Clock::GetRealTimeClock(), this)) { - remote_bitrate_estimator_.reset(new WrappingBitrateEstimator( - remb_.get(), Clock::GetRealTimeClock())); - call_stats_->RegisterStatsObserver(remote_bitrate_estimator_.get()); pacer_thread_->RegisterModule(pacer_.get()); pacer_thread_->Start(); + process_thread->RegisterModule(remote_estimator_proxy_.get()); process_thread->RegisterModule(remote_bitrate_estimator_.get()); process_thread->RegisterModule(call_stats_.get()); process_thread->RegisterModule(bitrate_controller_.get()); @@ -179,7 +183,10 @@ ChannelGroup::~ChannelGroup() { process_thread_->DeRegisterModule(bitrate_controller_.get()); process_thread_->DeRegisterModule(call_stats_.get()); process_thread_->DeRegisterModule(remote_bitrate_estimator_.get()); + process_thread_->DeRegisterModule(remote_estimator_proxy_.get()); call_stats_->DeregisterStatsObserver(remote_bitrate_estimator_.get()); + if (transport_feedback_adapter_.get()) + call_stats_->DeregisterStatsObserver(transport_feedback_adapter_.get()); RTC_DCHECK(channel_map_.empty()); RTC_DCHECK(!remb_->InUse()); RTC_DCHECK(vie_encoder_map_.empty()); @@ -188,7 +195,30 @@ ChannelGroup::~ChannelGroup() { bool ChannelGroup::CreateSendChannel(int channel_id, Transport* transport, int number_of_cores, - const std::vector& ssrcs) { + const VideoSendStream::Config& config) { + TransportFeedbackObserver* transport_feedback_observer = nullptr; + bool transport_seq_enabled = false; + for (const RtpExtension& extension : config.rtp.extensions) { + if (extension.name == RtpExtension::kTransportSequenceNumber) { + transport_seq_enabled = true; + break; + } + } + if (transport_seq_enabled) { + if (transport_feedback_adapter_.get() == nullptr) { + transport_feedback_adapter_.reset(new TransportFeedbackAdapter( + bitrate_controller_->CreateRtcpBandwidthObserver(), + Clock::GetRealTimeClock(), process_thread_)); + transport_feedback_adapter_->SetBitrateEstimator( + new RemoteBitrateEstimatorAbsSendTime( + transport_feedback_adapter_.get(), Clock::GetRealTimeClock(), + RemoteBitrateEstimator::kDefaultMinBitrateBps)); + call_stats_->RegisterStatsObserver(transport_feedback_adapter_.get()); + } + transport_feedback_observer = transport_feedback_adapter_.get(); + } + + const std::vector& ssrcs = config.rtp.ssrcs; RTC_DCHECK(!ssrcs.empty()); rtc::scoped_ptr vie_encoder( new ViEEncoder(channel_id, number_of_cores, *process_thread_, @@ -198,7 +228,9 @@ bool ChannelGroup::CreateSendChannel(int channel_id, } ViEEncoder* encoder = vie_encoder.get(); if (!CreateChannel(channel_id, transport, number_of_cores, - vie_encoder.release(), ssrcs.size(), true)) { + vie_encoder.release(), ssrcs.size(), true, + remote_bitrate_estimator_.get(), + transport_feedback_observer)) { return false; } ViEChannel* channel = channel_map_[channel_id]; @@ -212,11 +244,27 @@ bool ChannelGroup::CreateSendChannel(int channel_id, return true; } -bool ChannelGroup::CreateReceiveChannel(int channel_id, - Transport* transport, - int number_of_cores) { - return CreateChannel(channel_id, transport, number_of_cores, - nullptr, 1, false); +bool ChannelGroup::CreateReceiveChannel( + int channel_id, + Transport* transport, + int number_of_cores, + const VideoReceiveStream::Config& config) { + bool send_side_bwe = false; + for (const RtpExtension& extension : config.rtp.extensions) { + if (extension.name == RtpExtension::kTransportSequenceNumber) { + send_side_bwe = true; + break; + } + } + + RemoteBitrateEstimator* bitrate_estimator; + if (send_side_bwe) { + bitrate_estimator = remote_estimator_proxy_.get(); + } else { + bitrate_estimator = remote_bitrate_estimator_.get(); + } + return CreateChannel(channel_id, transport, number_of_cores, nullptr, 1, + false, bitrate_estimator, nullptr); } bool ChannelGroup::CreateChannel(int channel_id, @@ -224,13 +272,15 @@ bool ChannelGroup::CreateChannel(int channel_id, int number_of_cores, ViEEncoder* vie_encoder, size_t max_rtp_streams, - bool sender) { + bool sender, + RemoteBitrateEstimator* bitrate_estimator, + TransportFeedbackObserver* feedback_observer) { rtc::scoped_ptr channel(new ViEChannel( number_of_cores, transport, process_thread_, encoder_state_feedback_->GetRtcpIntraFrameObserver(), - bitrate_controller_->CreateRtcpBandwidthObserver(), nullptr, - remote_bitrate_estimator_.get(), call_stats_->rtcp_rtt_stats(), - pacer_.get(), packet_router_.get(), max_rtp_streams, sender)); + bitrate_controller_->CreateRtcpBandwidthObserver(), feedback_observer, + bitrate_estimator, call_stats_->rtcp_rtt_stats(), pacer_.get(), + packet_router_.get(), max_rtp_streams, sender)); if (channel->Init() != 0) { return false; } diff --git a/webrtc/video_engine/vie_channel_group.h b/webrtc/video_engine/vie_channel_group.h index ec1708318d..1c4c732d07 100644 --- a/webrtc/video_engine/vie_channel_group.h +++ b/webrtc/video_engine/vie_channel_group.h @@ -19,6 +19,8 @@ #include "webrtc/base/criticalsection.h" #include "webrtc/base/scoped_ptr.h" #include "webrtc/modules/bitrate_controller/include/bitrate_controller.h" +#include "webrtc/video_receive_stream.h" +#include "webrtc/video_send_stream.h" namespace webrtc { @@ -30,6 +32,8 @@ class PacedSender; class PacketRouter; class ProcessThread; class RemoteBitrateEstimator; +class RemoteEstimatorProxy; +class TransportFeedbackAdapter; class ViEChannel; class ViEEncoder; class VieRemb; @@ -46,10 +50,11 @@ class ChannelGroup : public BitrateObserver { bool CreateSendChannel(int channel_id, Transport* transport, int number_of_cores, - const std::vector& ssrcs); + const VideoSendStream::Config& config); bool CreateReceiveChannel(int channel_id, Transport* transport, - int number_of_cores); + int number_of_cores, + const VideoReceiveStream::Config& config); void DeleteChannel(int channel_id); ViEChannel* GetChannel(int channel_id) const; ViEEncoder* GetEncoder(int channel_id) const; @@ -77,16 +82,19 @@ class ChannelGroup : public BitrateObserver { int number_of_cores, ViEEncoder* vie_encoder, size_t max_rtp_streams, - bool sender); + bool sender, + RemoteBitrateEstimator* bitrate_estimator, + TransportFeedbackObserver* feedback_observer); ViEChannel* PopChannel(int channel_id); rtc::scoped_ptr remb_; rtc::scoped_ptr bitrate_allocator_; rtc::scoped_ptr call_stats_; - rtc::scoped_ptr remote_bitrate_estimator_; - rtc::scoped_ptr encoder_state_feedback_; rtc::scoped_ptr packet_router_; rtc::scoped_ptr pacer_; + rtc::scoped_ptr remote_bitrate_estimator_; + rtc::scoped_ptr remote_estimator_proxy_; + rtc::scoped_ptr encoder_state_feedback_; ChannelMap channel_map_; // Maps Channel id -> ViEEncoder. mutable rtc::CriticalSection encoder_map_crit_; @@ -97,6 +105,7 @@ class ChannelGroup : public BitrateObserver { rtc::scoped_ptr pacer_thread_; rtc::scoped_ptr bitrate_controller_; + rtc::scoped_ptr transport_feedback_adapter_; }; } // namespace webrtc