From 1d0fa5d352fe12092201fade249905c7e1ff974b Mon Sep 17 00:00:00 2001 From: "pbos@webrtc.org" Date: Thu, 19 Feb 2015 12:47:00 +0000 Subject: [PATCH] Add RtcpPacketTypeCounter stats to new API. R=mflodman@webrtc.org, stefan@webrtc.org BUG=1667,1788 Review URL: https://webrtc-codereview.appspot.com/37489004 Cr-Commit-Position: refs/heads/master@{#8429} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8429 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/media/webrtc/fakewebrtcvideoengine.h | 2 + talk/media/webrtc/webrtcvideoengine2.cc | 17 +++- .../webrtc/webrtcvideoengine2_unittest.cc | 47 ++++++++- .../webrtc/webrtcvideoengine2_unittest.h | 3 + webrtc/common_types.h | 8 ++ webrtc/config.h | 1 + webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h | 8 +- .../source/rtcp_format_remb_unittest.cc | 5 +- .../modules/rtp_rtcp/source/rtcp_receiver.cc | 31 +++--- .../modules/rtp_rtcp/source/rtcp_receiver.h | 8 +- .../rtp_rtcp/source/rtcp_receiver_unittest.cc | 2 +- webrtc/modules/rtp_rtcp/source/rtcp_sender.cc | 25 ++--- webrtc/modules/rtp_rtcp/source/rtcp_sender.h | 6 +- .../rtp_rtcp/source/rtcp_sender_unittest.cc | 4 +- .../modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 19 ++-- .../modules/rtp_rtcp/source/rtp_rtcp_impl.h | 4 - .../rtp_rtcp/source/rtp_rtcp_impl_unittest.cc | 31 ++++-- webrtc/video/end_to_end_tests.cc | 98 +++++++++++-------- webrtc/video/receive_statistics_proxy.cc | 18 +++- webrtc/video/receive_statistics_proxy.h | 5 + webrtc/video/send_statistics_proxy.cc | 11 +++ webrtc/video/send_statistics_proxy.h | 5 + webrtc/video/video_receive_stream.cc | 8 +- webrtc/video/video_send_stream.cc | 3 + webrtc/video/video_send_stream_tests.cc | 16 +-- webrtc/video_engine/include/vie_rtp_rtcp.h | 6 ++ webrtc/video_engine/vie_channel.cc | 28 +++--- webrtc/video_engine/vie_channel.h | 25 +++++ webrtc/video_engine/vie_rtp_rtcp_impl.cc | 14 +++ webrtc/video_engine/vie_rtp_rtcp_impl.h | 3 + 30 files changed, 333 insertions(+), 128 deletions(-) diff --git a/talk/media/webrtc/fakewebrtcvideoengine.h b/talk/media/webrtc/fakewebrtcvideoengine.h index ed612c302..b38a75703 100644 --- a/talk/media/webrtc/fakewebrtcvideoengine.h +++ b/talk/media/webrtc/fakewebrtcvideoengine.h @@ -1215,6 +1215,8 @@ class FakeWebRtcVideoEngine (int, webrtc::FrameCountObserver*)); WEBRTC_STUB(DeregisterSendFrameCountObserver, (int, webrtc::FrameCountObserver*)); + WEBRTC_STUB(RegisterRtcpPacketTypeCounterObserver, + (int, webrtc::RtcpPacketTypeCounterObserver*)); WEBRTC_STUB(StartRTPDump, (const int, const char*, webrtc::RTPDirections)); WEBRTC_STUB(StopRTPDump, (const int, webrtc::RTPDirections)); diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc index 967ab2193..765898685 100644 --- a/talk/media/webrtc/webrtcvideoengine2.cc +++ b/talk/media/webrtc/webrtcvideoengine2.cc @@ -1762,6 +1762,9 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::GetVideoSenderInfo() { info.send_frame_width = stream_stats.sent_width; if (stream_stats.sent_height > info.send_frame_height) info.send_frame_height = stream_stats.sent_height; + info.firs_rcvd += stream_stats.rtcp_packet_type_counts.fir_packets; + info.nacks_rcvd += stream_stats.rtcp_packet_type_counts.nack_packets; + info.plis_rcvd += stream_stats.rtcp_packet_type_counts.pli_packets; } if (!stats.substreams.empty()) { @@ -2027,10 +2030,16 @@ WebRtcVideoChannel2::WebRtcVideoReceiveStream::GetVideoReceiverInfo() { info.framerate_decoded = stats.decode_frame_rate; info.framerate_output = stats.render_frame_rate; - rtc::CritScope frame_cs(&renderer_lock_); - info.frame_width = last_width_; - info.frame_height = last_height_; - info.capture_start_ntp_time_ms = estimated_remote_start_ntp_time_ms_; + { + rtc::CritScope frame_cs(&renderer_lock_); + info.frame_width = last_width_; + info.frame_height = last_height_; + info.capture_start_ntp_time_ms = estimated_remote_start_ntp_time_ms_; + } + + info.firs_sent = stats.rtcp_packet_type_counts.fir_packets; + info.plis_sent = stats.rtcp_packet_type_counts.pli_packets; + info.nacks_sent = stats.rtcp_packet_type_counts.nack_packets; // TODO(pbos): Support or remove the following stats. info.packets_concealed = -1; diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.cc b/talk/media/webrtc/webrtcvideoengine2_unittest.cc index d9eb7d596..7dc769350 100644 --- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc @@ -196,7 +196,7 @@ void FakeVideoReceiveStream::InjectFrame(const webrtc::I420VideoFrame& frame, } webrtc::VideoReceiveStream::Stats FakeVideoReceiveStream::GetStats() const { - return webrtc::VideoReceiveStream::Stats(); + return stats_; } void FakeVideoReceiveStream::Start() { @@ -207,6 +207,11 @@ void FakeVideoReceiveStream::Stop() { receiving_ = false; } +void FakeVideoReceiveStream::SetStats( + const webrtc::VideoReceiveStream::Stats& stats) { + stats_ = stats; +} + FakeCall::FakeCall(const webrtc::Call::Config& config) : config_(config), network_state_(kNetworkUp), @@ -2053,6 +2058,46 @@ TEST_F(WebRtcVideoChannel2Test, GetStatsReportsUpperResolution) { EXPECT_EQ(90, info.senders[0].send_frame_height); } +TEST_F(WebRtcVideoChannel2Test, + GetStatsTranslatesSendRtcpPacketTypesCorrectly) { + FakeVideoSendStream* stream = AddSendStream(); + webrtc::VideoSendStream::Stats stats; + stats.substreams[17].rtcp_packet_type_counts.fir_packets = 2; + stats.substreams[17].rtcp_packet_type_counts.nack_packets = 3; + stats.substreams[17].rtcp_packet_type_counts.pli_packets = 4; + + stats.substreams[42].rtcp_packet_type_counts.fir_packets = 5; + stats.substreams[42].rtcp_packet_type_counts.nack_packets = 7; + stats.substreams[42].rtcp_packet_type_counts.pli_packets = 9; + + stream->SetStats(stats); + + cricket::VideoMediaInfo info; + ASSERT_TRUE(channel_->GetStats(cricket::StatsOptions(), &info)); + EXPECT_EQ(7, info.senders[0].firs_rcvd); + EXPECT_EQ(10, info.senders[0].nacks_rcvd); + EXPECT_EQ(13, info.senders[0].plis_rcvd); +} + +TEST_F(WebRtcVideoChannel2Test, + GetStatsTranslatesReceiveRtcpPacketTypesCorrectly) { + FakeVideoReceiveStream* stream = AddRecvStream(); + webrtc::VideoReceiveStream::Stats stats; + stats.rtcp_packet_type_counts.fir_packets = 2; + stats.rtcp_packet_type_counts.nack_packets = 3; + stats.rtcp_packet_type_counts.pli_packets = 4; + stream->SetStats(stats); + + cricket::VideoMediaInfo info; + ASSERT_TRUE(channel_->GetStats(cricket::StatsOptions(), &info)); + EXPECT_EQ(stats.rtcp_packet_type_counts.fir_packets, + info.receivers[0].firs_sent); + EXPECT_EQ(stats.rtcp_packet_type_counts.nack_packets, + info.receivers[0].nacks_sent); + EXPECT_EQ(stats.rtcp_packet_type_counts.pli_packets, + info.receivers[0].plis_sent); +} + TEST_F(WebRtcVideoChannel2Test, TranslatesCallStatsCorrectly) { AddSendStream(); AddSendStream(); diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.h b/talk/media/webrtc/webrtcvideoengine2_unittest.h index 72c52f9a2..cf2539b68 100644 --- a/talk/media/webrtc/webrtcvideoengine2_unittest.h +++ b/talk/media/webrtc/webrtcvideoengine2_unittest.h @@ -86,6 +86,8 @@ class FakeVideoReceiveStream : public webrtc::VideoReceiveStream { void InjectFrame(const webrtc::I420VideoFrame& frame, int time_to_render_ms); + void SetStats(const webrtc::VideoReceiveStream::Stats& stats); + private: virtual webrtc::VideoReceiveStream::Stats GetStats() const OVERRIDE; @@ -94,6 +96,7 @@ class FakeVideoReceiveStream : public webrtc::VideoReceiveStream { webrtc::VideoReceiveStream::Config config_; bool receiving_; + webrtc::VideoReceiveStream::Stats stats_; }; class FakeCall : public webrtc::Call { diff --git a/webrtc/common_types.h b/webrtc/common_types.h index ec7f1ed26..71fbedcfc 100644 --- a/webrtc/common_types.h +++ b/webrtc/common_types.h @@ -241,6 +241,14 @@ struct RtcpPacketTypeCounter { uint32_t unique_nack_requests; // Number of unique NACKed RTP packets. }; +class RtcpPacketTypeCounterObserver { + public: + virtual ~RtcpPacketTypeCounterObserver() {} + virtual void RtcpPacketTypesCounterUpdated( + uint32_t ssrc, + const RtcpPacketTypeCounter& packet_counter) = 0; +}; + // Rate statistics for a stream. struct BitrateStatistics { BitrateStatistics() : bitrate_bps(0), packet_rate(0), timestamp_ms(0) {} diff --git a/webrtc/config.h b/webrtc/config.h index cd5a23ffc..c69917060 100644 --- a/webrtc/config.h +++ b/webrtc/config.h @@ -38,6 +38,7 @@ struct SsrcStats { int avg_delay_ms; int max_delay_ms; StreamDataCounters rtp_stats; + RtcpPacketTypeCounter rtcp_packet_type_counts; RtcpStatistics rtcp_stats; }; diff --git a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h index f50933701..243b24c8f 100644 --- a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h @@ -61,6 +61,7 @@ class RtpRtcp : public Module { RtcpIntraFrameObserver* intra_frame_callback; RtcpBandwidthObserver* bandwidth_callback; RtcpRttStats* rtt_stats; + RtcpPacketTypeCounterObserver* rtcp_packet_type_counter_observer; RtpAudioFeedback* audio_messages; RemoteBitrateEstimator* remote_bitrate_estimator; PacedSender* paced_sender; @@ -455,13 +456,6 @@ class RtpRtcp : public Module { */ virtual int32_t RemoveRTCPReportBlock(uint32_t SSRC) = 0; - /* - * Get number of sent and received RTCP packet types. - */ - virtual void GetRtcpPacketTypeCounters( - RtcpPacketTypeCounter* packets_sent, - RtcpPacketTypeCounter* packets_received) const = 0; - /* * (APP) Application specific data * 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 51fda7c24..d89588000 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_format_remb_unittest.cc @@ -95,8 +95,9 @@ void RtcpFormatRembTest::SetUp() { configuration.remote_bitrate_estimator = remote_bitrate_estimator_.get(); dummy_rtp_rtcp_impl_ = new ModuleRtpRtcpImpl(configuration); rtcp_sender_ = - new RTCPSender(0, false, system_clock_, receive_statistics_.get()); - rtcp_receiver_ = new RTCPReceiver(0, system_clock_, dummy_rtp_rtcp_impl_); + new RTCPSender(0, false, system_clock_, receive_statistics_.get(), NULL); + rtcp_receiver_ = + new RTCPReceiver(0, system_clock_, NULL, 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_receiver.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc index 62d6676f0..b1b46dfcf 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.cc @@ -28,7 +28,11 @@ using namespace RTCPHelp; // The number of RTCP time intervals needed to trigger a timeout. const int kRrTimeoutIntervals = 3; -RTCPReceiver::RTCPReceiver(int32_t id, Clock* clock, ModuleRtpRtcpImpl* owner) +RTCPReceiver::RTCPReceiver( + int32_t id, + Clock* clock, + RtcpPacketTypeCounterObserver* packet_type_counter_observer, + ModuleRtpRtcpImpl* owner) : TMMBRHelp(), _clock(clock), _method(kRtcpOff), @@ -52,7 +56,8 @@ RTCPReceiver::RTCPReceiver(int32_t id, Clock* clock, ModuleRtpRtcpImpl* owner) _packetTimeOutMS(0), _lastReceivedRrMs(0), _lastIncreasedSequenceNumberMs(0), - stats_callback_(NULL) { + stats_callback_(NULL), + packet_type_counter_observer_(packet_type_counter_observer) { memset(&_remoteSenderInfo, 0, sizeof(_remoteSenderInfo)); } @@ -271,12 +276,6 @@ int32_t RTCPReceiver::StatisticsReceived( return 0; } -void RTCPReceiver::GetPacketTypeCounter( - RtcpPacketTypeCounter* packet_counter) const { - CriticalSectionScoped lock(_criticalSectionRTCPReceiver); - *packet_counter = packet_type_counter_; -} - int32_t RTCPReceiver::IncomingRTCPPacket(RTCPPacketInformation& rtcpPacketInformation, RTCPUtility::RTCPParserV2* rtcpParser) @@ -362,6 +361,12 @@ RTCPReceiver::IncomingRTCPPacket(RTCPPacketInformation& rtcpPacketInformation, } pktType = rtcpParser->PacketType(); } + + if (packet_type_counter_observer_ != NULL) { + packet_type_counter_observer_->RtcpPacketTypesCounterUpdated( + main_ssrc_, packet_type_counter_); + } + return 0; } @@ -770,10 +775,12 @@ void RTCPReceiver::HandleSDESChunk(RTCPUtility::RTCPParserV2& rtcpParser) { cnameInfo->name[RTCP_CNAME_SIZE - 1] = 0; strncpy(cnameInfo->name, rtcpPacket.CName.CName, RTCP_CNAME_SIZE - 1); - CriticalSectionScoped lock(_criticalSectionFeedbacks); - if (stats_callback_ != NULL) { - stats_callback_->CNameChanged(rtcpPacket.CName.CName, - rtcpPacket.CName.SenderSSRC); + { + CriticalSectionScoped lock(_criticalSectionFeedbacks); + if (stats_callback_ != NULL) { + stats_callback_->CNameChanged(rtcpPacket.CName.CName, + rtcpPacket.CName.SenderSSRC); + } } } diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h index 46d10f754..919e9669f 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver.h @@ -29,7 +29,10 @@ class ModuleRtpRtcpImpl; class RTCPReceiver : public TMMBRHelp { public: - RTCPReceiver(int32_t id, Clock* clock, ModuleRtpRtcpImpl* owner); + RTCPReceiver(int32_t id, + Clock* clock, + RtcpPacketTypeCounterObserver* packet_type_counter_observer, + ModuleRtpRtcpImpl* owner); virtual ~RTCPReceiver(); RTCPMethod Status() const; @@ -83,8 +86,6 @@ public: int32_t StatisticsReceived( std::vector* receiveBlocks) const; - void GetPacketTypeCounter(RtcpPacketTypeCounter* packet_counter) const; - // Returns true if we haven't received an RTCP RR for several RTCP // intervals, but only triggers true once. bool RtcpRrTimeout(int64_t rtcp_interval_ms); @@ -276,6 +277,7 @@ protected: RtcpStatisticsCallback* stats_callback_ GUARDED_BY(_criticalSectionFeedbacks); + RtcpPacketTypeCounterObserver* const packet_type_counter_observer_; RtcpPacketTypeCounter packet_type_counter_; RTCPUtility::NackStats nack_stats_; diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc index cceb54498..fc09542d4 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_receiver_unittest.cc @@ -86,7 +86,7 @@ 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(0, &system_clock_, rtp_rtcp_impl_); + rtcp_receiver_ = new RTCPReceiver(0, &system_clock_, NULL, rtp_rtcp_impl_); test_transport_->SetRTCPReceiver(rtcp_receiver_); } ~RtcpReceiverTest() { diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc index fbfe83299..c713852b2 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.cc @@ -76,10 +76,12 @@ RTCPSender::FeedbackState::FeedbackState() remote_sr(0), has_last_xr_rr(false) {} -RTCPSender::RTCPSender(int32_t id, - bool audio, - Clock* clock, - ReceiveStatistics* receive_statistics) +RTCPSender::RTCPSender( + int32_t id, + bool audio, + Clock* clock, + ReceiveStatistics* receive_statistics, + RtcpPacketTypeCounterObserver* packet_type_counter_observer) : _id(id), _audio(audio), _clock(clock), @@ -129,7 +131,8 @@ RTCPSender::RTCPSender(int32_t id, xrSendReceiverReferenceTimeEnabled_(false), _xrSendVoIPMetric(false), - _xrVoIPMetric() { + _xrVoIPMetric(), + packet_type_counter_observer_(packet_type_counter_observer) { memset(_CNAME, 0, sizeof(_CNAME)); memset(_lastSendReport, 0, sizeof(_lastSendReport)); memset(_lastRTCPTime, 0, sizeof(_lastRTCPTime)); @@ -466,12 +469,6 @@ bool RTCPSender::SendTimeOfXrRrReport(uint32_t mid_ntp, return true; } -void RTCPSender::GetPacketTypeCounter( - RtcpPacketTypeCounter* packet_counter) const { - CriticalSectionScoped lock(_criticalSectionRTCPSender); - *packet_counter = packet_type_counter_; -} - int32_t RTCPSender::AddExternalReportBlock( uint32_t SSRC, const RTCPReportBlock* reportBlock) { @@ -1889,6 +1886,12 @@ int RTCPSender::PrepareRTCP(const FeedbackState& feedback_state, return position; } } + + if (packet_type_counter_observer_ != NULL) { + packet_type_counter_observer_->RtcpPacketTypesCounterUpdated( + _remoteSSRC, packet_type_counter_); + } + return position; } diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender.h b/webrtc/modules/rtp_rtcp/source/rtcp_sender.h index fcc79b64b..13bdade33 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender.h @@ -70,7 +70,8 @@ public: RTCPSender(int32_t id, bool audio, Clock* clock, - ReceiveStatistics* receive_statistics); + ReceiveStatistics* receive_statistics, + RtcpPacketTypeCounterObserver* packet_type_counter_observer); virtual ~RTCPSender(); int32_t RegisterSendTransport(Transport* outgoingTransport); @@ -165,8 +166,6 @@ public: void SetTargetBitrate(unsigned int target_bitrate); - void GetPacketTypeCounter(RtcpPacketTypeCounter* packet_counter) const; - private: int32_t SendToNetwork(const uint8_t* dataBuffer, size_t length); @@ -341,6 +340,7 @@ private: bool _xrSendVoIPMetric GUARDED_BY(_criticalSectionRTCPSender); RTCPVoIPMetric _xrVoIPMetric GUARDED_BY(_criticalSectionRTCPSender); + RtcpPacketTypeCounterObserver* const packet_type_counter_observer_; RtcpPacketTypeCounter packet_type_counter_ GUARDED_BY(_criticalSectionRTCPSender); diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc index 699b09af3..0a0ee8915 100644 --- a/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtcp_sender_unittest.cc @@ -304,8 +304,8 @@ class RtcpSenderTest : public ::testing::Test { rtp_receiver_.reset(RtpReceiver::CreateVideoReceiver( 0, &clock_, test_transport_, NULL, rtp_payload_registry_.get())); rtcp_sender_ = - new RTCPSender(0, false, &clock_, receive_statistics_.get()); - rtcp_receiver_ = new RTCPReceiver(0, &clock_, rtp_rtcp_impl_); + new RTCPSender(0, false, &clock_, receive_statistics_.get(), NULL); + rtcp_receiver_ = new RTCPReceiver(0, &clock_, NULL, rtp_rtcp_impl_); test_transport_->SetRTCPReceiver(rtcp_receiver_); // Initialize EXPECT_EQ(0, rtcp_sender_->RegisterSendTransport(test_transport_)); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index fe5566bcf..a529d097f 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -36,6 +36,7 @@ RtpRtcp::Configuration::Configuration() intra_frame_callback(NULL), bandwidth_callback(NULL), rtt_stats(NULL), + rtcp_packet_type_counter_observer(NULL), audio_messages(NullObjectRtpAudioFeedback()), remote_bitrate_estimator(NULL), paced_sender(NULL), @@ -70,8 +71,12 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) rtcp_sender_(configuration.id, configuration.audio, configuration.clock, - configuration.receive_statistics), - rtcp_receiver_(configuration.id, configuration.clock, this), + configuration.receive_statistics, + configuration.rtcp_packet_type_counter_observer), + rtcp_receiver_(configuration.id, + configuration.clock, + configuration.rtcp_packet_type_counter_observer, + this), clock_(configuration.clock), id_(configuration.id), audio_(configuration.audio), @@ -722,13 +727,6 @@ int32_t ModuleRtpRtcpImpl::RemoveRTCPReportBlock( return rtcp_sender_.RemoveExternalReportBlock(ssrc); } -void ModuleRtpRtcpImpl::GetRtcpPacketTypeCounters( - RtcpPacketTypeCounter* packets_sent, - RtcpPacketTypeCounter* packets_received) const { - rtcp_sender_.GetPacketTypeCounter(packets_sent); - rtcp_receiver_.GetPacketTypeCounter(packets_received); -} - // (REMB) Receiver Estimated Max Bitrate. bool ModuleRtpRtcpImpl::REMB() const { return rtcp_sender_.REMB(); @@ -860,8 +858,7 @@ void ModuleRtpRtcpImpl::RegisterRtcpStatisticsCallback( rtcp_receiver_.RegisterRtcpStatisticsCallback(callback); } -RtcpStatisticsCallback* -ModuleRtpRtcpImpl::GetRtcpStatisticsCallback() { +RtcpStatisticsCallback* ModuleRtpRtcpImpl::GetRtcpStatisticsCallback() { return rtcp_receiver_.GetRtcpStatisticsCallback(); } diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h index 0c028bb57..5d5b7f4ca 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.h @@ -190,10 +190,6 @@ class ModuleRtpRtcpImpl : public RtpRtcp { virtual int32_t RemoveRTCPReportBlock(uint32_t ssrc) OVERRIDE; - virtual void GetRtcpPacketTypeCounters( - RtcpPacketTypeCounter* packets_sent, - RtcpPacketTypeCounter* packets_received) const OVERRIDE; - // (REMB) Receiver Estimated Max Bitrate. virtual bool REMB() const OVERRIDE; diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc index 0a3a9d7b6..68c53f69e 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc @@ -98,7 +98,7 @@ class SendTransport : public Transport, std::vector last_nack_list_; }; -class RtpRtcpModule { +class RtpRtcpModule : public RtcpPacketTypeCounterObserver { public: RtpRtcpModule(SimulatedClock* clock) : receive_statistics_(ReceiveStatistics::Create(clock)) { @@ -107,6 +107,7 @@ class RtpRtcpModule { config.clock = clock; config.outgoing_transport = &transport_; config.receive_statistics = receive_statistics_.get(); + config.rtcp_packet_type_counter_observer = this; config.rtt_stats = &rtt_stats_; impl_.reset(new ModuleRtpRtcpImpl(config)); @@ -121,14 +122,27 @@ class RtpRtcpModule { SendTransport transport_; RtcpRttStatsTestImpl rtt_stats_; scoped_ptr impl_; + uint32_t remote_ssrc_; + + void SetRemoteSsrc(uint32_t ssrc) { + remote_ssrc_ = ssrc; + impl_->SetRemoteSSRC(ssrc); + } + + void RtcpPacketTypesCounterUpdated( + uint32_t ssrc, + const RtcpPacketTypeCounter& packet_counter) override { + counter_map_[ssrc] = packet_counter; + } RtcpPacketTypeCounter RtcpSent() { - impl_->GetRtcpPacketTypeCounters(&packets_sent_, &packets_received_); - return packets_sent_; + // RTCP counters for remote SSRC. + return counter_map_[remote_ssrc_]; } + RtcpPacketTypeCounter RtcpReceived() { - impl_->GetRtcpPacketTypeCounters(&packets_sent_, &packets_received_); - return packets_received_; + // Received RTCP stats for (own) local SSRC. + return counter_map_[impl_->SSRC()]; } int RtpSent() { return transport_.rtp_packets_sent_; @@ -139,6 +153,9 @@ class RtpRtcpModule { std::vector LastNackListSent() { return transport_.last_nack_list_; } + + private: + std::map counter_map_; }; } // namespace @@ -152,7 +169,7 @@ class RtpRtcpImplTest : public ::testing::Test { EXPECT_EQ(0, sender_.impl_->SetSendingStatus(true)); sender_.impl_->SetSendingMediaStatus(true); sender_.impl_->SetSSRC(kSenderSsrc); - sender_.impl_->SetRemoteSSRC(kReceiverSsrc); + sender_.SetRemoteSsrc(kReceiverSsrc); sender_.impl_->SetSequenceNumber(kSequenceNumber); sender_.impl_->SetStorePacketsStatus(true, 100); @@ -167,7 +184,7 @@ class RtpRtcpImplTest : public ::testing::Test { EXPECT_EQ(0, receiver_.impl_->SetSendingStatus(false)); receiver_.impl_->SetSendingMediaStatus(false); receiver_.impl_->SetSSRC(kReceiverSsrc); - receiver_.impl_->SetRemoteSSRC(kSenderSsrc); + receiver_.SetRemoteSsrc(kSenderSsrc); // Transport settings. sender_.transport_.SetRtpRtcpModule(receiver_.impl_.get()); receiver_.transport_.SetRtpRtcpModule(sender_.impl_.get()); diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index c25cc6110..dcf4c0f24 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -1564,11 +1564,9 @@ TEST_F(EndToEndTest, GetStats) { static const int kStartBitrateBps = 3000000; class StatsObserver : public test::EndToEndTest, public I420FrameCallback { public: - StatsObserver() - : EndToEndTest(kLongTimeoutMs), - receive_stream_(NULL), + explicit StatsObserver(const FakeNetworkPipe::Config& config) + : EndToEndTest(kLongTimeoutMs, config), send_stream_(NULL), - expected_receive_ssrc_(), expected_send_ssrcs_(), check_stats_event_(EventWrapper::Create()) {} @@ -1602,40 +1600,49 @@ TEST_F(EndToEndTest, GetStats) { } bool CheckReceiveStats() { - assert(receive_stream_ != NULL); - VideoReceiveStream::Stats stats = receive_stream_->GetStats(); - EXPECT_EQ(expected_receive_ssrc_, stats.ssrc); + for (size_t i = 0; i < receive_streams_.size(); ++i) { + VideoReceiveStream::Stats stats = receive_streams_[i]->GetStats(); + EXPECT_EQ(expected_receive_ssrcs_[i], stats.ssrc); - // Make sure all fields have been populated. + // Make sure all fields have been populated. + // TODO(pbos): Use CompoundKey if/when we ever know that all stats are + // always filled for all receivers. + receive_stats_filled_["IncomingRate"] |= + stats.network_frame_rate != 0 || stats.total_bitrate_bps != 0; - receive_stats_filled_["IncomingRate"] |= - stats.network_frame_rate != 0 || stats.total_bitrate_bps != 0; + receive_stats_filled_["FrameCallback"] |= stats.decode_frame_rate != 0; - receive_stats_filled_["FrameCallback"] |= stats.decode_frame_rate != 0; + receive_stats_filled_["FrameRendered"] |= stats.render_frame_rate != 0; - receive_stats_filled_["FrameRendered"] |= stats.render_frame_rate != 0; + receive_stats_filled_["StatisticsUpdated"] |= + stats.rtcp_stats.cumulative_lost != 0 || + stats.rtcp_stats.extended_max_sequence_number != 0 || + stats.rtcp_stats.fraction_lost != 0 || stats.rtcp_stats.jitter != 0; - receive_stats_filled_["StatisticsUpdated"] |= - stats.rtcp_stats.cumulative_lost != 0 || - stats.rtcp_stats.extended_max_sequence_number != 0 || - stats.rtcp_stats.fraction_lost != 0 || stats.rtcp_stats.jitter != 0; + receive_stats_filled_["DataCountersUpdated"] |= + stats.rtp_stats.transmitted.payload_bytes != 0 || + stats.rtp_stats.fec.packets != 0 || + stats.rtp_stats.transmitted.header_bytes != 0 || + stats.rtp_stats.transmitted.packets != 0 || + stats.rtp_stats.transmitted.padding_bytes != 0 || + stats.rtp_stats.retransmitted.packets != 0; - receive_stats_filled_["DataCountersUpdated"] |= - stats.rtp_stats.transmitted.payload_bytes != 0 || - stats.rtp_stats.fec.packets != 0 || - stats.rtp_stats.transmitted.header_bytes != 0 || - stats.rtp_stats.transmitted.packets != 0 || - stats.rtp_stats.transmitted.padding_bytes != 0 || - stats.rtp_stats.retransmitted.packets != 0; + receive_stats_filled_["CodecStats"] |= + stats.avg_delay_ms != 0 || stats.discarded_packets != 0; - receive_stats_filled_["CodecStats"] |= - stats.avg_delay_ms != 0 || stats.discarded_packets != 0; + receive_stats_filled_["FrameCounts"] |= + stats.frame_counts.key_frames != 0 || + stats.frame_counts.delta_frames != 0; - receive_stats_filled_["FrameCounts"] |= - stats.frame_counts.key_frames != 0 || - stats.frame_counts.delta_frames != 0; + receive_stats_filled_["CName"] |= stats.c_name != ""; - receive_stats_filled_["CName"] |= stats.c_name != ""; + receive_stats_filled_["RtcpPacketTypeCount"] |= + stats.rtcp_packet_type_counts.fir_packets != 0 || + stats.rtcp_packet_type_counts.nack_packets != 0 || + stats.rtcp_packet_type_counts.pli_packets != 0 || + stats.rtcp_packet_type_counts.nack_requests != 0 || + stats.rtcp_packet_type_counts.unique_nack_requests != 0; + } return AllStatsFilled(receive_stats_filled_); } @@ -1654,7 +1661,7 @@ TEST_F(EndToEndTest, GetStats) { EXPECT_TRUE(expected_send_ssrcs_.find(it->first) != expected_send_ssrcs_.end()); - send_stats_filled_[CompoundKey("IncomingRate", it->first)] |= + send_stats_filled_[CompoundKey("CapturedFrameRate", it->first)] |= stats.input_frame_rate != 0; const SsrcStats& stream_stats = it->second; @@ -1683,6 +1690,15 @@ TEST_F(EndToEndTest, GetStats) { send_stats_filled_[CompoundKey("Delay", it->first)] |= stream_stats.avg_delay_ms != 0 || stream_stats.max_delay_ms != 0; + + // TODO(pbos): Use CompoundKey when the test makes sure that all SSRCs + // report dropped packets. + send_stats_filled_["RtcpPacketTypeCount"] |= + stream_stats.rtcp_packet_type_counts.fir_packets != 0 || + stream_stats.rtcp_packet_type_counts.nack_packets != 0 || + stream_stats.rtcp_packet_type_counts.pli_packets != 0 || + stream_stats.rtcp_packet_type_counts.nack_requests != 0 || + stream_stats.rtcp_packet_type_counts.unique_nack_requests != 0; } return AllStatsFilled(send_stats_filled_); @@ -1715,14 +1731,14 @@ TEST_F(EndToEndTest, GetStats) { std::vector* receive_configs, VideoEncoderConfig* encoder_config) override { send_config->pre_encode_callback = this; // Used to inject delay. - send_config->rtp.c_name = "SomeCName"; + expected_cname_ = send_config->rtp.c_name = "SomeCName"; - expected_receive_ssrc_ = (*receive_configs)[0].rtp.local_ssrc; const std::vector& ssrcs = send_config->rtp.ssrcs; - for (size_t i = 0; i < ssrcs.size(); ++i) + for (size_t i = 0; i < ssrcs.size(); ++i) { expected_send_ssrcs_.insert(ssrcs[i]); - - expected_cname_ = send_config->rtp.c_name; + expected_receive_ssrcs_.push_back( + (*receive_configs)[i].rtp.remote_ssrc); + } } virtual size_t GetNumStreams() const override { return kNumSsrcs; } @@ -1731,7 +1747,7 @@ TEST_F(EndToEndTest, GetStats) { VideoSendStream* send_stream, const std::vector& receive_streams) override { send_stream_ = send_stream; - receive_stream_ = receive_streams[0]; + receive_streams_ = receive_streams; } virtual void PerformTest() override { @@ -1776,19 +1792,23 @@ TEST_F(EndToEndTest, GetStats) { } } - VideoReceiveStream* receive_stream_; + std::vector receive_streams_; std::map receive_stats_filled_; VideoSendStream* send_stream_; std::map send_stats_filled_; - uint32_t expected_receive_ssrc_; + std::vector expected_receive_ssrcs_; std::set expected_send_ssrcs_; std::string expected_cname_; scoped_ptr check_stats_event_; - } test; + }; + FakeNetworkPipe::Config network_config; + network_config.loss_percent = 5; + + StatsObserver test(network_config); RunBaseTest(&test); } diff --git a/webrtc/video/receive_statistics_proxy.cc b/webrtc/video/receive_statistics_proxy.cc index 15238f6d0..444f7c160 100644 --- a/webrtc/video/receive_statistics_proxy.cc +++ b/webrtc/video/receive_statistics_proxy.cc @@ -50,16 +50,32 @@ void ReceiveStatisticsProxy::DecoderTiming(int decode_ms, stats_.avg_delay_ms = target_delay_ms; } +void ReceiveStatisticsProxy::RtcpPacketTypesCounterUpdated( + uint32_t ssrc, + const RtcpPacketTypeCounter& packet_counter) { + CriticalSectionScoped lock(crit_.get()); + if (stats_.ssrc != ssrc) + return; + stats_.rtcp_packet_type_counts = packet_counter; +} + void ReceiveStatisticsProxy::StatisticsUpdated( const webrtc::RtcpStatistics& statistics, uint32_t ssrc) { CriticalSectionScoped lock(crit_.get()); - + // TODO(pbos): Handle both local and remote ssrcs here and DCHECK that we + // receive stats from one of them. + if (stats_.ssrc != ssrc) + return; stats_.rtcp_stats = statistics; } void ReceiveStatisticsProxy::CNameChanged(const char* cname, uint32_t ssrc) { CriticalSectionScoped lock(crit_.get()); + // TODO(pbos): Handle both local and remote ssrcs here and DCHECK that we + // receive stats from one of them. + if (stats_.ssrc != ssrc) + return; stats_.c_name = cname; } diff --git a/webrtc/video/receive_statistics_proxy.h b/webrtc/video/receive_statistics_proxy.h index 2286d0815..6f5784443 100644 --- a/webrtc/video/receive_statistics_proxy.h +++ b/webrtc/video/receive_statistics_proxy.h @@ -33,6 +33,7 @@ class ViEDecoderObserver; class ReceiveStatisticsProxy : public ViEDecoderObserver, public VCMReceiveStatisticsCallback, public RtcpStatisticsCallback, + public RtcpPacketTypeCounterObserver, public StreamDataCountersCallback { public: ReceiveStatisticsProxy(uint32_t ssrc, Clock* clock); @@ -69,6 +70,10 @@ class ReceiveStatisticsProxy : public ViEDecoderObserver, uint32_t ssrc) OVERRIDE; virtual void CNameChanged(const char* cname, uint32_t ssrc) OVERRIDE; + // Overrides RtcpPacketTypeCounterObserver + virtual void RtcpPacketTypesCounterUpdated( + uint32_t ssrc, + const RtcpPacketTypeCounter& packet_counter) OVERRIDE; // Overrides StreamDataCountersCallback. virtual void DataCountersUpdated(const webrtc::StreamDataCounters& counters, uint32_t ssrc) OVERRIDE; diff --git a/webrtc/video/send_statistics_proxy.cc b/webrtc/video/send_statistics_proxy.cc index b593c4dd3..5e5d2e877 100644 --- a/webrtc/video/send_statistics_proxy.cc +++ b/webrtc/video/send_statistics_proxy.cc @@ -105,6 +105,17 @@ void SendStatisticsProxy::OnSendEncodedImage( update_times_[ssrc].resolution_update_ms = clock_->TimeInMilliseconds(); } +void SendStatisticsProxy::RtcpPacketTypesCounterUpdated( + uint32_t ssrc, + const RtcpPacketTypeCounter& packet_counter) { + CriticalSectionScoped lock(crit_.get()); + SsrcStats* stats = GetStatsEntry(ssrc); + if (stats == NULL) + return; + + stats->rtcp_packet_type_counts = packet_counter; +} + void SendStatisticsProxy::StatisticsUpdated(const RtcpStatistics& statistics, uint32_t ssrc) { CriticalSectionScoped lock(crit_.get()); diff --git a/webrtc/video/send_statistics_proxy.h b/webrtc/video/send_statistics_proxy.h index ac6e309d9..568ffc411 100644 --- a/webrtc/video/send_statistics_proxy.h +++ b/webrtc/video/send_statistics_proxy.h @@ -27,6 +27,7 @@ namespace webrtc { class CriticalSectionWrapper; class SendStatisticsProxy : public RtcpStatisticsCallback, + public RtcpPacketTypeCounterObserver, public StreamDataCountersCallback, public BitrateStatisticsObserver, public FrameCountObserver, @@ -49,6 +50,10 @@ class SendStatisticsProxy : public RtcpStatisticsCallback, virtual void StatisticsUpdated(const RtcpStatistics& statistics, uint32_t ssrc) OVERRIDE; virtual void CNameChanged(const char *cname, uint32_t ssrc) OVERRIDE; + // From RtcpPacketTypeCounterObserver + virtual void RtcpPacketTypesCounterUpdated( + uint32_t ssrc, + const RtcpPacketTypeCounter& packet_counter) OVERRIDE; // From StreamDataCountersCallback. virtual void DataCountersUpdated(const StreamDataCounters& counters, uint32_t ssrc) OVERRIDE; diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index 4699ca4f1..8665c1149 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -219,7 +219,7 @@ VideoReceiveStream::VideoReceiveStream(webrtc::VideoEngine* video_engine, } stats_proxy_.reset( - new ReceiveStatisticsProxy(config_.rtp.local_ssrc, clock_)); + new ReceiveStatisticsProxy(config_.rtp.remote_ssrc, clock_)); if (rtp_rtcp_->RegisterReceiveChannelRtcpStatisticsCallback( channel_, stats_proxy_.get()) != 0) { @@ -231,6 +231,11 @@ VideoReceiveStream::VideoReceiveStream(webrtc::VideoEngine* video_engine, abort(); } + if (rtp_rtcp_->RegisterRtcpPacketTypeCounterObserver( + channel_, stats_proxy_.get()) != 0) { + abort(); + } + if (codec_->RegisterDecoderObserver(channel_, *stats_proxy_) != 0) { abort(); } @@ -301,6 +306,7 @@ VideoReceiveStream::~VideoReceiveStream() { stats_proxy_.get()); rtp_rtcp_->DeregisterReceiveChannelRtcpStatisticsCallback(channel_, stats_proxy_.get()); + rtp_rtcp_->RegisterRtcpPacketTypeCounterObserver(channel_, NULL); codec_->Release(); network_->Release(); render_->Release(); diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index ab7f0c1b5..82af66e34 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -229,6 +229,7 @@ VideoSendStream::VideoSendStream( &stats_proxy_); rtp_rtcp_->RegisterSendChannelRtpStatisticsCallback(channel_, &stats_proxy_); + rtp_rtcp_->RegisterRtcpPacketTypeCounterObserver(channel_, &stats_proxy_); rtp_rtcp_->RegisterSendBitrateObserver(channel_, &stats_proxy_); rtp_rtcp_->RegisterSendFrameCountObserver(channel_, &stats_proxy_); @@ -242,6 +243,7 @@ VideoSendStream::~VideoSendStream() { rtp_rtcp_->DeregisterSendFrameCountObserver(channel_, &stats_proxy_); rtp_rtcp_->DeregisterSendBitrateObserver(channel_, &stats_proxy_); + rtp_rtcp_->RegisterRtcpPacketTypeCounterObserver(channel_, NULL); rtp_rtcp_->DeregisterSendChannelRtpStatisticsCallback(channel_, &stats_proxy_); rtp_rtcp_->DeregisterSendChannelRtcpStatisticsCallback(channel_, @@ -443,6 +445,7 @@ VideoSendStream::Stats VideoSendStream::GetStats() { } void VideoSendStream::ConfigureSsrcs() { + rtp_rtcp_->SetLocalSSRC(channel_, config_.rtp.ssrcs.front()); for (size_t i = 0; i < config_.rtp.ssrcs.size(); ++i) { uint32_t ssrc = config_.rtp.ssrcs[i]; rtp_rtcp_->SetLocalSSRC( diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index 79904ef18..8330b9df4 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -324,8 +324,8 @@ TEST_F(VideoSendStreamTest, SupportsFec) { // Receive statistics reporting having lost 50% of the packets. FakeReceiveStatistics lossy_receive_stats( kSendSsrcs[0], header.sequenceNumber, send_count_ / 2, 127); - RTCPSender rtcp_sender( - 0, false, Clock::GetRealTimeClock(), &lossy_receive_stats); + RTCPSender rtcp_sender(0, false, Clock::GetRealTimeClock(), + &lossy_receive_stats, nullptr); EXPECT_EQ(0, rtcp_sender.RegisterSendTransport(&transport_adapter_)); rtcp_sender.SetRTCPStatus(kRtcpNonCompound); @@ -400,7 +400,7 @@ void VideoSendStreamTest::TestNackRetransmission( nacked_sequence_number_ = nack_sequence_number; NullReceiveStatistics null_stats; RTCPSender rtcp_sender( - 0, false, Clock::GetRealTimeClock(), &null_stats); + 0, false, Clock::GetRealTimeClock(), &null_stats, nullptr); EXPECT_EQ(0, rtcp_sender.RegisterSendTransport(&transport_adapter_)); rtcp_sender.SetRTCPStatus(kRtcpNonCompound); @@ -582,8 +582,8 @@ void VideoSendStreamTest::TestPacketFragmentationSize(VideoFormat format, // Receive statistics reporting having lost 50% of the packets. FakeReceiveStatistics lossy_receive_stats( kSendSsrcs[0], header.sequenceNumber, packet_count_ / 2, 127); - RTCPSender rtcp_sender( - 0, false, Clock::GetRealTimeClock(), &lossy_receive_stats); + RTCPSender rtcp_sender(0, false, Clock::GetRealTimeClock(), + &lossy_receive_stats, nullptr); EXPECT_EQ(0, rtcp_sender.RegisterSendTransport(&transport_adapter_)); rtcp_sender.SetRTCPStatus(kRtcpNonCompound); @@ -812,7 +812,7 @@ TEST_F(VideoSendStreamTest, SuspendBelowMinBitrate) { EXCLUSIVE_LOCKS_REQUIRED(crit_) { FakeReceiveStatistics receive_stats( kSendSsrcs[0], last_sequence_number_, rtp_count_, 0); - RTCPSender rtcp_sender(0, false, clock_, &receive_stats); + RTCPSender rtcp_sender(0, false, clock_, &receive_stats, nullptr); EXPECT_EQ(0, rtcp_sender.RegisterSendTransport(&transport_adapter_)); rtcp_sender.SetRTCPStatus(kRtcpNonCompound); @@ -872,8 +872,8 @@ TEST_F(VideoSendStreamTest, NoPaddingWhenVideoIsMuted) { observation_complete_->Set(); // Receive statistics reporting having lost 50% of the packets. FakeReceiveStatistics receive_stats(kSendSsrcs[0], 1, 1, 0); - RTCPSender rtcp_sender( - 0, false, Clock::GetRealTimeClock(), &receive_stats); + RTCPSender rtcp_sender(0, false, Clock::GetRealTimeClock(), + &receive_stats, nullptr); EXPECT_EQ(0, rtcp_sender.RegisterSendTransport(&transport_adapter_)); rtcp_sender.SetRTCPStatus(kRtcpNonCompound); diff --git a/webrtc/video_engine/include/vie_rtp_rtcp.h b/webrtc/video_engine/include/vie_rtp_rtcp.h index 62c0b4f3c..3f519bc18 100644 --- a/webrtc/video_engine/include/vie_rtp_rtcp.h +++ b/webrtc/video_engine/include/vie_rtp_rtcp.h @@ -448,6 +448,12 @@ class WEBRTC_DLLEXPORT ViERTP_RTCP { virtual int DeregisterSendFrameCountObserver( int video_channel, FrameCountObserver* observer) = 0; + // Called when RTCP packet type counters might have been changed. User has to + // filter on SSRCs to determine whether it's status sent or received. + virtual int RegisterRtcpPacketTypeCounterObserver( + int video_channel, + RtcpPacketTypeCounterObserver* observer) = 0; + protected: virtual ~ViERTP_RTCP() {} }; diff --git a/webrtc/video_engine/vie_channel.cc b/webrtc/video_engine/vie_channel.cc index 83e7152c7..588e1a94b 100644 --- a/webrtc/video_engine/vie_channel.cc +++ b/webrtc/video_engine/vie_channel.cc @@ -1142,6 +1142,11 @@ void ViEChannel::RegisterReceiveChannelRtcpStatisticsCallback( rtp_rtcp_->RegisterRtcpStatisticsCallback(callback); } +void ViEChannel::RegisterRtcpPacketTypeCounterObserver( + RtcpPacketTypeCounterObserver* observer) { + rtcp_packet_type_counter_observer_.Set(observer); +} + int32_t ViEChannel::GetRtpStatistics(size_t* bytes_sent, uint32_t* packets_sent, size_t* bytes_received, @@ -1241,25 +1246,24 @@ void ViEChannel::RegisterReceiveChannelRtpStatisticsCallback( void ViEChannel::GetRtcpPacketTypeCounters( RtcpPacketTypeCounter* packets_sent, RtcpPacketTypeCounter* packets_received) const { - rtp_rtcp_->GetRtcpPacketTypeCounters(packets_sent, packets_received); + std::map counter_map = + rtcp_packet_type_counter_observer_.GetPacketTypeCounterMap(); + RtcpPacketTypeCounter sent_counter; + sent_counter.Add(counter_map[rtp_rtcp_->SSRC()]); + RtcpPacketTypeCounter received_counter; + received_counter.Add(counter_map[vie_receiver_.GetRemoteSsrc()]); CriticalSectionScoped cs(rtp_rtcp_cs_.get()); for (std::list::const_iterator it = simulcast_rtp_rtcp_.begin(); it != simulcast_rtp_rtcp_.end(); ++it) { - RtcpPacketTypeCounter sent; - RtcpPacketTypeCounter received; - (*it)->GetRtcpPacketTypeCounters(&sent, &received); - packets_sent->Add(sent); - packets_received->Add(received); + sent_counter.Add(counter_map[(*it)->SSRC()]); } for (std::list::const_iterator it = removed_rtp_rtcp_.begin(); it != removed_rtp_rtcp_.end(); ++it) { - RtcpPacketTypeCounter sent; - RtcpPacketTypeCounter received; - (*it)->GetRtcpPacketTypeCounters(&sent, &received); - packets_sent->Add(sent); - packets_received->Add(received); + sent_counter.Add(counter_map[(*it)->SSRC()]); } + *packets_sent = sent_counter; + *packets_received = received_counter; } void ViEChannel::GetBandwidthUsage(uint32_t* total_bitrate_sent, @@ -1694,6 +1698,8 @@ RtpRtcp::Configuration ViEChannel::CreateRtpRtcpConfiguration() { configuration.intra_frame_callback = intra_frame_observer_; configuration.bandwidth_callback = bandwidth_observer_.get(); configuration.rtt_stats = rtt_stats_; + configuration.rtcp_packet_type_counter_observer = + &rtcp_packet_type_counter_observer_; configuration.paced_sender = paced_sender_; configuration.send_bitrate_observer = &send_bitrate_observer_; configuration.send_frame_count_observer = &send_frame_count_observer_; diff --git a/webrtc/video_engine/vie_channel.h b/webrtc/video_engine/vie_channel.h index d79065dd5..093e179dd 100644 --- a/webrtc/video_engine/vie_channel.h +++ b/webrtc/video_engine/vie_channel.h @@ -358,6 +358,8 @@ class ViEChannel EncodedImageCallback* pre_decode_callback); void RegisterSendFrameCountObserver(FrameCountObserver* observer); + void RegisterRtcpPacketTypeCounterObserver( + RtcpPacketTypeCounterObserver* observer); void RegisterReceiveStatisticsProxy( ReceiveStatisticsProxy* receive_statistics_proxy); void ReceivedBWEPacket(int64_t arrival_time_ms, size_t payload_size, @@ -455,6 +457,29 @@ class ViEChannel } } send_side_delay_observer_; + class RegisterableRtcpPacketTypeCounterObserver + : public RegisterableCallback { + public: + virtual void RtcpPacketTypesCounterUpdated( + uint32_t ssrc, + const RtcpPacketTypeCounter& packet_counter) override { + CriticalSectionScoped cs(critsect_.get()); + if (callback_) + callback_->RtcpPacketTypesCounterUpdated(ssrc, packet_counter); + counter_map_[ssrc] = packet_counter; + } + + virtual std::map GetPacketTypeCounterMap() + const { + CriticalSectionScoped cs(critsect_.get()); + return counter_map_; + } + + private: + std::map counter_map_ + GUARDED_BY(critsect_); + } rtcp_packet_type_counter_observer_; + int32_t channel_id_; int32_t engine_id_; uint32_t number_of_cores_; diff --git a/webrtc/video_engine/vie_rtp_rtcp_impl.cc b/webrtc/video_engine/vie_rtp_rtcp_impl.cc index b14d8a22c..bf27d8ab4 100644 --- a/webrtc/video_engine/vie_rtp_rtcp_impl.cc +++ b/webrtc/video_engine/vie_rtp_rtcp_impl.cc @@ -1019,4 +1019,18 @@ int ViERTP_RTCPImpl::DeregisterSendFrameCountObserver( vie_channel->RegisterSendFrameCountObserver(NULL); return 0; } + +int ViERTP_RTCPImpl::RegisterRtcpPacketTypeCounterObserver( + int video_channel, + RtcpPacketTypeCounterObserver* observer) { + ViEChannelManagerScoped cs(*(shared_data_->channel_manager())); + ViEChannel* vie_channel = cs.Channel(video_channel); + if (!vie_channel) { + shared_data_->SetLastError(kViERtpRtcpInvalidChannelId); + return -1; + } + vie_channel->RegisterRtcpPacketTypeCounterObserver(observer); + return 0; +} + } // namespace webrtc diff --git a/webrtc/video_engine/vie_rtp_rtcp_impl.h b/webrtc/video_engine/vie_rtp_rtcp_impl.h index 6c20f1eb0..88d039f2b 100644 --- a/webrtc/video_engine/vie_rtp_rtcp_impl.h +++ b/webrtc/video_engine/vie_rtp_rtcp_impl.h @@ -157,6 +157,9 @@ class ViERTP_RTCPImpl int channel, FrameCountObserver* callback); virtual int DeregisterSendFrameCountObserver( int channel, FrameCountObserver* callback); + virtual int RegisterRtcpPacketTypeCounterObserver( + int video_channel, + RtcpPacketTypeCounterObserver* observer) override; protected: explicit ViERTP_RTCPImpl(ViESharedData* shared_data);