From a790d834c90bf7d29d67717ce9f0a2ad479974a6 Mon Sep 17 00:00:00 2001 From: sprang Date: Fri, 2 Dec 2016 07:29:44 -0800 Subject: [PATCH] Wire up rtcp xr target bitrate on receive side. BUG=webrtc:6301 Review-Url: https://codereview.webrtc.org/2540363003 Cr-Commit-Position: refs/heads/master@{#15388} --- webrtc/modules/rtp_rtcp/include/rtp_rtcp.h | 2 + .../modules/rtp_rtcp/source/rtcp_receiver.cc | 142 ++++++++++-------- .../modules/rtp_rtcp/source/rtcp_receiver.h | 18 ++- .../rtp_rtcp/source/rtcp_receiver_unittest.cc | 31 ++++ webrtc/modules/rtp_rtcp/source/rtcp_sender.cc | 1 + .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 1 + webrtc/test/fuzzers/rtcp_receiver_fuzzer.cc | 2 +- 7 files changed, 131 insertions(+), 66 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h index 94853d2f6b..28dfe239de 100644 --- a/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/include/rtp_rtcp.h @@ -34,6 +34,7 @@ class RemoteBitrateEstimator; class RtcEventLog; class RtpReceiver; class Transport; +class VideoBitrateAllocationObserver; RTPExtensionType StringToRtpExtensionType(const std::string& extension); @@ -68,6 +69,7 @@ class RtpRtcp : public Module { RtcpBandwidthObserver* bandwidth_callback = nullptr; TransportFeedbackObserver* transport_feedback_callback = nullptr; + VideoBitrateAllocationObserver* bitrate_allocation_observer = nullptr; RtcpRttStats* rtt_stats = nullptr; RtcpPacketTypeCounterObserver* rtcp_packet_type_counter_observer = nullptr; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc index c78637abf3..c94a9cc362 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -22,6 +22,7 @@ #include "webrtc/base/checks.h" #include "webrtc/base/logging.h" #include "webrtc/base/trace_event.h" +#include "webrtc/common_video/include/video_bitrate_allocator.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/bye.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/common_header.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/compound_packet.h" @@ -67,6 +68,7 @@ struct RTCPReceiver::PacketInformation { uint64_t rpsi_picture_id = 0; uint32_t receiver_estimated_max_bitrate_bps = 0; std::unique_ptr transport_feedback; + rtc::Optional target_bitrate_allocation; }; struct RTCPReceiver::ReceiveInformation { @@ -103,13 +105,15 @@ RTCPReceiver::RTCPReceiver( RtcpBandwidthObserver* rtcp_bandwidth_observer, RtcpIntraFrameObserver* rtcp_intra_frame_observer, TransportFeedbackObserver* transport_feedback_observer, + VideoBitrateAllocationObserver* bitrate_allocation_observer, ModuleRtpRtcp* owner) : _clock(clock), receiver_only_(receiver_only), _rtpRtcp(*owner), - _cbRtcpBandwidthObserver(rtcp_bandwidth_observer), - _cbRtcpIntraFrameObserver(rtcp_intra_frame_observer), - _cbTransportFeedbackObserver(transport_feedback_observer), + rtcp_bandwidth_observer_(rtcp_bandwidth_observer), + rtcp_intra_frame_observer_(rtcp_intra_frame_observer), + transport_feedback_observer_(transport_feedback_observer), + bitrate_allocation_observer_(bitrate_allocation_observer), main_ssrc_(0), _remoteSSRC(0), _remoteSenderInfo(), @@ -117,7 +121,7 @@ RTCPReceiver::RTCPReceiver( xr_rr_rtt_ms_(0), _lastReceivedRrMs(0), _lastIncreasedSequenceNumberMs(0), - stats_callback_(NULL), + stats_callback_(nullptr), packet_type_counter_observer_(packet_type_counter_observer), num_skipped_packets_(0), last_skipped_packets_warning_(clock->TimeInMilliseconds()) { @@ -627,7 +631,7 @@ void RTCPReceiver::HandleSDES(const CommonHeader& rtcp_block, for (const rtcp::Sdes::Chunk& chunk : sdes.chunks()) { received_cnames_[chunk.ssrc] = chunk.cname; { - rtc::CritScope lock(&_criticalSectionFeedbacks); + rtc::CritScope lock(&feedbacks_lock_); if (stats_callback_) stats_callback_->CNameChanged(chunk.cname.c_str(), chunk.ssrc); } @@ -693,6 +697,9 @@ void RTCPReceiver::HandleXr(const CommonHeader& rtcp_block, for (const rtcp::ReceiveTimeInfo& time_info : xr.dlrr().sub_blocks()) HandleXrDlrrReportBlock(time_info); + + if (xr.target_bitrate()) + HandleXrTargetBitrate(*xr.target_bitrate(), packet_information); } void RTCPReceiver::HandleXrReceiveReferenceTime( @@ -725,6 +732,17 @@ void RTCPReceiver::HandleXrDlrrReportBlock(const rtcp::ReceiveTimeInfo& rti) { xr_rr_rtt_ms_ = CompactNtpRttToMs(rtt_ntp); } +void RTCPReceiver::HandleXrTargetBitrate( + const rtcp::TargetBitrate& target_bitrate, + PacketInformation* packet_information) { + BitrateAllocation bitrate_allocation; + for (const auto& item : target_bitrate.GetTargetBitrates()) { + bitrate_allocation.SetBitrate(item.spatial_layer, item.temporal_layer, + item.target_bitrate_kbps * 1000); + } + packet_information->target_bitrate_allocation.emplace(bitrate_allocation); +} + void RTCPReceiver::HandlePLI(const CommonHeader& rtcp_block, PacketInformation* packet_information) { rtcp::Pli pli; @@ -898,11 +916,11 @@ void RTCPReceiver::UpdateTmmbr() { std::vector bounding = TMMBRHelp::FindBoundingSet(TmmbrReceived()); - if (!bounding.empty() && _cbRtcpBandwidthObserver) { + if (!bounding.empty() && rtcp_bandwidth_observer_) { // We have a new bandwidth estimate on this channel. uint64_t bitrate_bps = TMMBRHelp::CalcMinBitrateBps(bounding); if (bitrate_bps <= std::numeric_limits::max()) - _cbRtcpBandwidthObserver->OnReceivedEstimatedBitrate(bitrate_bps); + rtcp_bandwidth_observer_->OnReceivedEstimatedBitrate(bitrate_bps); } // Set bounding set: inform remote clients about the new bandwidth. @@ -911,12 +929,12 @@ void RTCPReceiver::UpdateTmmbr() { void RTCPReceiver::RegisterRtcpStatisticsCallback( RtcpStatisticsCallback* callback) { - rtc::CritScope cs(&_criticalSectionFeedbacks); + rtc::CritScope cs(&feedbacks_lock_); stats_callback_ = callback; } RtcpStatisticsCallback* RTCPReceiver::GetRtcpStatisticsCallback() { - rtc::CritScope cs(&_criticalSectionFeedbacks); + rtc::CritScope cs(&feedbacks_lock_); return stats_callback_; } @@ -947,68 +965,72 @@ void RTCPReceiver::TriggerCallbacksFromRTCPPacket( _rtpRtcp.OnReceivedNack(packet_information.nack_sequence_numbers); } } - { - // We need feedback that we have received a report block(s) so that we - // can generate a new packet in a conference relay scenario, one received - // report can generate several RTCP packets, based on number relayed/mixed - // a send report block should go out to all receivers. - if (_cbRtcpIntraFrameObserver) { - RTC_DCHECK(!receiver_only_); - if ((packet_information.packet_type_flags & kRtcpPli) || - (packet_information.packet_type_flags & kRtcpFir)) { - if (packet_information.packet_type_flags & kRtcpPli) { - LOG(LS_VERBOSE) << "Incoming PLI from SSRC " - << packet_information.remote_ssrc; - } else { - LOG(LS_VERBOSE) << "Incoming FIR from SSRC " - << packet_information.remote_ssrc; - } - _cbRtcpIntraFrameObserver->OnReceivedIntraFrameRequest(local_ssrc); - } - if (packet_information.packet_type_flags & kRtcpSli) { - _cbRtcpIntraFrameObserver->OnReceivedSLI( - local_ssrc, packet_information.sli_picture_id); - } - if (packet_information.packet_type_flags & kRtcpRpsi) { - _cbRtcpIntraFrameObserver->OnReceivedRPSI( - local_ssrc, packet_information.rpsi_picture_id); + + // We need feedback that we have received a report block(s) so that we + // can generate a new packet in a conference relay scenario, one received + // report can generate several RTCP packets, based on number relayed/mixed + // a send report block should go out to all receivers. + if (rtcp_intra_frame_observer_) { + RTC_DCHECK(!receiver_only_); + if ((packet_information.packet_type_flags & kRtcpPli) || + (packet_information.packet_type_flags & kRtcpFir)) { + if (packet_information.packet_type_flags & kRtcpPli) { + LOG(LS_VERBOSE) << "Incoming PLI from SSRC " + << packet_information.remote_ssrc; + } else { + LOG(LS_VERBOSE) << "Incoming FIR from SSRC " + << packet_information.remote_ssrc; } + rtcp_intra_frame_observer_->OnReceivedIntraFrameRequest(local_ssrc); } - if (_cbRtcpBandwidthObserver) { - RTC_DCHECK(!receiver_only_); - if (packet_information.packet_type_flags & kRtcpRemb) { - LOG(LS_VERBOSE) - << "Incoming REMB: " - << packet_information.receiver_estimated_max_bitrate_bps; - _cbRtcpBandwidthObserver->OnReceivedEstimatedBitrate( - packet_information.receiver_estimated_max_bitrate_bps); - } - if ((packet_information.packet_type_flags & kRtcpSr) || - (packet_information.packet_type_flags & kRtcpRr)) { - int64_t now = _clock->TimeInMilliseconds(); - _cbRtcpBandwidthObserver->OnReceivedRtcpReceiverReport( - packet_information.report_blocks, packet_information.rtt_ms, now); - } + if (packet_information.packet_type_flags & kRtcpSli) { + rtcp_intra_frame_observer_->OnReceivedSLI( + local_ssrc, packet_information.sli_picture_id); + } + if (packet_information.packet_type_flags & kRtcpRpsi) { + rtcp_intra_frame_observer_->OnReceivedRPSI( + local_ssrc, packet_information.rpsi_picture_id); + } + } + if (rtcp_bandwidth_observer_) { + RTC_DCHECK(!receiver_only_); + if (packet_information.packet_type_flags & kRtcpRemb) { + LOG(LS_VERBOSE) << "Incoming REMB: " + << packet_information.receiver_estimated_max_bitrate_bps; + rtcp_bandwidth_observer_->OnReceivedEstimatedBitrate( + packet_information.receiver_estimated_max_bitrate_bps); } if ((packet_information.packet_type_flags & kRtcpSr) || (packet_information.packet_type_flags & kRtcpRr)) { - _rtpRtcp.OnReceivedRtcpReportBlocks(packet_information.report_blocks); + int64_t now = _clock->TimeInMilliseconds(); + rtcp_bandwidth_observer_->OnReceivedRtcpReceiverReport( + packet_information.report_blocks, packet_information.rtt_ms, now); } + } + if ((packet_information.packet_type_flags & kRtcpSr) || + (packet_information.packet_type_flags & kRtcpRr)) { + _rtpRtcp.OnReceivedRtcpReportBlocks(packet_information.report_blocks); + } - if (_cbTransportFeedbackObserver && - (packet_information.packet_type_flags & kRtcpTransportFeedback)) { - uint32_t media_source_ssrc = - packet_information.transport_feedback->media_ssrc(); - if (media_source_ssrc == local_ssrc || - registered_ssrcs.find(media_source_ssrc) != registered_ssrcs.end()) { - _cbTransportFeedbackObserver->OnTransportFeedback( - *packet_information.transport_feedback); - } + if (transport_feedback_observer_ && + (packet_information.packet_type_flags & kRtcpTransportFeedback)) { + uint32_t media_source_ssrc = + packet_information.transport_feedback->media_ssrc(); + if (media_source_ssrc == local_ssrc || + registered_ssrcs.find(media_source_ssrc) != registered_ssrcs.end()) { + transport_feedback_observer_->OnTransportFeedback( + *packet_information.transport_feedback); } } + if (bitrate_allocation_observer_ && + packet_information.target_bitrate_allocation) { + bitrate_allocation_observer_->OnBitrateAllocationUpdated( + *packet_information.target_bitrate_allocation); + } + if (!receiver_only_) { - rtc::CritScope cs(&_criticalSectionFeedbacks); + rtc::CritScope cs(&feedbacks_lock_); if (stats_callback_) { for (const auto& report_block : packet_information.report_blocks) { RtcpStatistics stats; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h index 6bb771b066..8d1e2c2330 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h @@ -25,10 +25,12 @@ #include "webrtc/typedefs.h" namespace webrtc { +class VideoBitrateAllocationObserver; namespace rtcp { class CommonHeader; class ReportBlock; class Rrtr; +class TargetBitrate; class TmmbItem; } // namespace rtcp @@ -53,6 +55,7 @@ class RTCPReceiver { RtcpBandwidthObserver* rtcp_bandwidth_observer, RtcpIntraFrameObserver* rtcp_intra_frame_observer, TransportFeedbackObserver* transport_feedback_observer, + VideoBitrateAllocationObserver* bitrate_allocation_observer, ModuleRtpRtcp* owner); virtual ~RTCPReceiver(); @@ -163,6 +166,10 @@ class RTCPReceiver { void HandleXrDlrrReportBlock(const rtcp::ReceiveTimeInfo& rti) EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver); + void HandleXrTargetBitrate(const rtcp::TargetBitrate& target_bitrate, + PacketInformation* packet_information) + EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver); + void HandleNACK(const rtcp::CommonHeader& rtcp_block, PacketInformation* packet_information) EXCLUSIVE_LOCKS_REQUIRED(_criticalSectionRTCPReceiver); @@ -210,10 +217,11 @@ class RTCPReceiver { const bool receiver_only_; ModuleRtpRtcp& _rtpRtcp; - rtc::CriticalSection _criticalSectionFeedbacks; - RtcpBandwidthObserver* const _cbRtcpBandwidthObserver; - RtcpIntraFrameObserver* const _cbRtcpIntraFrameObserver; - TransportFeedbackObserver* const _cbTransportFeedbackObserver; + rtc::CriticalSection feedbacks_lock_; + RtcpBandwidthObserver* const rtcp_bandwidth_observer_; + RtcpIntraFrameObserver* const rtcp_intra_frame_observer_; + TransportFeedbackObserver* const transport_feedback_observer_; + VideoBitrateAllocationObserver* const bitrate_allocation_observer_; rtc::CriticalSection _criticalSectionRTCPReceiver; uint32_t main_ssrc_ GUARDED_BY(_criticalSectionRTCPReceiver); @@ -247,7 +255,7 @@ class RTCPReceiver { // delivered RTP packet to the remote side. int64_t _lastIncreasedSequenceNumberMs; - RtcpStatisticsCallback* stats_callback_ GUARDED_BY(_criticalSectionFeedbacks); + RtcpStatisticsCallback* stats_callback_ GUARDED_BY(feedbacks_lock_); RtcpPacketTypeCounterObserver* const packet_type_counter_observer_; RtcpPacketTypeCounter packet_type_counter_; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index c98400f3fb..1630089502 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -14,6 +14,7 @@ #include "webrtc/base/array_view.h" #include "webrtc/base/random.h" #include "webrtc/common_types.h" +#include "webrtc/common_video/include/video_bitrate_allocator.h" #include "webrtc/modules/rtp_rtcp/source/byte_io.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet.h" #include "webrtc/modules/rtp_rtcp/source/rtcp_packet/app.h" @@ -96,6 +97,13 @@ class MockModuleRtpRtcp : public RTCPReceiver::ModuleRtpRtcp { MOCK_METHOD1(OnReceivedRtcpReportBlocks, void(const ReportBlockList&)); }; +class MockVideoBitrateAllocationObserver + : public VideoBitrateAllocationObserver { + public: + MOCK_METHOD1(OnBitrateAllocationUpdated, + void(const BitrateAllocation& allocation)); +}; + // SSRC of remote peer, that sends rtcp packet to the rtcp receiver under test. constexpr uint32_t kSenderSsrc = 0x10203; // SSRCs of local peer, that rtcp packet addressed to. @@ -118,6 +126,7 @@ class RtcpReceiverTest : public ::testing::Test { &bandwidth_observer_, &intra_frame_observer_, &transport_feedback_observer_, + &bitrate_allocation_observer_, &rtp_rtcp_impl_) {} void SetUp() { std::set ssrcs = {kReceiverMainSsrc, kReceiverExtraSsrc}; @@ -142,6 +151,7 @@ class RtcpReceiverTest : public ::testing::Test { StrictMock bandwidth_observer_; StrictMock intra_frame_observer_; StrictMock transport_feedback_observer_; + StrictMock bitrate_allocation_observer_; StrictMock rtp_rtcp_impl_; RTCPReceiver rtcp_receiver_; @@ -1266,4 +1276,25 @@ TEST_F(RtcpReceiverTest, ForceSenderReport) { InjectRtcpPacket(rr); } +TEST_F(RtcpReceiverTest, ReceivesTargetBitrate) { + BitrateAllocation expected_allocation; + expected_allocation.SetBitrate(0, 0, 10000); + expected_allocation.SetBitrate(0, 1, 20000); + expected_allocation.SetBitrate(1, 0, 40000); + expected_allocation.SetBitrate(1, 1, 80000); + + rtcp::TargetBitrate bitrate; + bitrate.AddTargetBitrate(0, 0, expected_allocation.GetBitrate(0, 0) / 1000); + bitrate.AddTargetBitrate(0, 1, expected_allocation.GetBitrate(0, 1) / 1000); + bitrate.AddTargetBitrate(1, 0, expected_allocation.GetBitrate(1, 0) / 1000); + bitrate.AddTargetBitrate(1, 1, expected_allocation.GetBitrate(1, 1) / 1000); + + rtcp::ExtendedReports xr; + xr.SetTargetBitrate(bitrate); + + EXPECT_CALL(bitrate_allocation_observer_, + OnBitrateAllocationUpdated(expected_allocation)); + InjectRtcpPacket(xr); +} + } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc index c47e060454..6d42d38fb8 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc @@ -964,6 +964,7 @@ int32_t RTCPSender::SetApplicationSpecificData(uint8_t subType, return 0; } +// TODO(sprang): Remove support for VoIP metrics? (Not used in receiver.) int32_t RTCPSender::SetRTCPVoIPMetrics(const RTCPVoIPMetric* VoIPMetric) { rtc::CritScope lock(&critical_section_rtcp_sender_); xr_voip_metric_.emplace(*VoIPMetric); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index b7b22d900b..b867b9ddbe 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -96,6 +96,7 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) configuration.bandwidth_callback, configuration.intra_frame_callback, configuration.transport_feedback_callback, + configuration.bitrate_allocation_observer, this), clock_(configuration.clock), audio_(configuration.audio), diff --git a/webrtc/test/fuzzers/rtcp_receiver_fuzzer.cc b/webrtc/test/fuzzers/rtcp_receiver_fuzzer.cc index 8678d11dcf..b8ab9b9e54 100644 --- a/webrtc/test/fuzzers/rtcp_receiver_fuzzer.cc +++ b/webrtc/test/fuzzers/rtcp_receiver_fuzzer.cc @@ -29,7 +29,7 @@ void FuzzOneInput(const uint8_t* data, size_t size) { SimulatedClock clock(1234); RTCPReceiver receiver(&clock, false, nullptr, nullptr, nullptr, nullptr, - &rtp_rtcp_module); + nullptr, &rtp_rtcp_module); receiver.IncomingPacket(data, size); }