diff --git a/api/stats/rtcstats_objects.h b/api/stats/rtcstats_objects.h index ebc6affb8d..2030380918 100644 --- a/api/stats/rtcstats_objects.h +++ b/api/stats/rtcstats_objects.h @@ -553,8 +553,6 @@ class RTC_EXPORT RTCOutboundRTPStreamStats final : public RTCRTPStreamStats { // FIR and PLI counts are only defined for |media_type == "video"|. RTCStatsMember fir_count; RTCStatsMember pli_count; - // TODO(hbos): NACK count should be collected by |RTCStatsCollector| for both - // audio and video but is only defined in the "video" case. crbug.com/657856 RTCStatsMember nack_count; RTCStatsMember qp_sum; }; diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc index 5d7bc71659..62dd53d337 100644 --- a/audio/audio_send_stream.cc +++ b/audio/audio_send_stream.cc @@ -498,6 +498,8 @@ webrtc::AudioSendStream::Stats AudioSendStream::GetStats( stats.report_block_datas = std::move(call_stats.report_block_datas); + stats.nacks_rcvd = call_stats.nacks_rcvd; + return stats; } diff --git a/audio/channel_send.cc b/audio/channel_send.cc index 52dd528504..06e9238ce8 100644 --- a/audio/channel_send.cc +++ b/audio/channel_send.cc @@ -60,8 +60,9 @@ class TransportSequenceNumberProxy; class VoERtcpObserver; class ChannelSend : public ChannelSendInterface, - public AudioPacketizationCallback { // receive encoded - // packets from the ACM + public AudioPacketizationCallback, // receive encoded + // packets from the ACM + public RtcpPacketTypeCounterObserver { public: // TODO(nisse): Make OnUplinkPacketLossRate public, and delete friend // declaration. @@ -150,6 +151,11 @@ class ChannelSend : public ChannelSendInterface, rtc::scoped_refptr frame_transformer) override; + // RtcpPacketTypeCounterObserver. + void RtcpPacketTypesCounterUpdated( + uint32_t ssrc, + const RtcpPacketTypeCounter& packet_counter) override; + private: // From AudioPacketizationCallback in the ACM int32_t SendData(AudioFrameType frameType, @@ -187,6 +193,7 @@ class ChannelSend : public ChannelSendInterface, mutable Mutex volume_settings_mutex_; + const uint32_t ssrc_; bool sending_ RTC_GUARDED_BY(&worker_thread_checker_) = false; RtcEventLog* const event_log_; @@ -239,6 +246,10 @@ class ChannelSend : public ChannelSendInterface, rtc::TaskQueue encoder_queue_; const bool fixing_timestamp_stall_; + + mutable Mutex rtcp_counter_mutex_; + RtcpPacketTypeCounter rtcp_packet_type_counter_ + RTC_GUARDED_BY(rtcp_counter_mutex_); }; const int kTelephoneEventAttenuationdB = 10; @@ -452,7 +463,8 @@ ChannelSend::ChannelSend( uint32_t ssrc, rtc::scoped_refptr frame_transformer, TransportFeedbackObserver* feedback_observer) - : event_log_(rtc_event_log), + : ssrc_(ssrc), + event_log_(rtc_event_log), _timeStamp(0), // This is just an offset, RTP module will add it's own // random offset input_mute_(false), @@ -487,6 +499,7 @@ ChannelSend::ChannelSend( retransmission_rate_limiter_.get(); configuration.extmap_allow_mixed = extmap_allow_mixed; configuration.rtcp_report_interval_ms = rtcp_report_interval_ms; + configuration.rtcp_packet_type_counter_observer = this; configuration.local_media_ssrc = ssrc; @@ -777,9 +790,24 @@ CallSendStatistics ChannelSend::GetRTCPStatistics() const { stats.retransmitted_packets_sent = rtp_stats.retransmitted.packets; stats.report_block_datas = rtp_rtcp_->GetLatestReportBlockData(); + { + MutexLock lock(&rtcp_counter_mutex_); + stats.nacks_rcvd = rtcp_packet_type_counter_.nack_packets; + } + return stats; } +void ChannelSend::RtcpPacketTypesCounterUpdated( + uint32_t ssrc, + const RtcpPacketTypeCounter& packet_counter) { + if (ssrc != ssrc_) { + return; + } + MutexLock lock(&rtcp_counter_mutex_); + rtcp_packet_type_counter_ = packet_counter; +} + void ChannelSend::ProcessAndEncodeAudio( std::unique_ptr audio_frame) { RTC_DCHECK_RUNS_SERIALIZED(&audio_thread_race_checker_); diff --git a/audio/channel_send.h b/audio/channel_send.h index cbdb3ee70a..67391af956 100644 --- a/audio/channel_send.h +++ b/audio/channel_send.h @@ -45,6 +45,7 @@ struct CallSendStatistics { // ReportBlockData represents the latest Report Block that was received for // that pair. std::vector report_block_datas; + uint32_t nacks_rcvd; }; // See section 6.4.2 in http://www.ietf.org/rfc/rfc3550.txt for details. diff --git a/audio/test/nack_test.cc b/audio/test/nack_test.cc index 714a8775d6..13cfe74a28 100644 --- a/audio/test/nack_test.cc +++ b/audio/test/nack_test.cc @@ -20,7 +20,7 @@ using NackTest = CallTest; TEST_F(NackTest, ShouldNackInLossyNetwork) { class NackTest : public AudioEndToEndTest { public: - const int kTestDurationMs = 1000; + const int kTestDurationMs = 2000; const int64_t kRttMs = 30; const int64_t kLossPercent = 30; const int kNackHistoryMs = 1000; @@ -46,6 +46,9 @@ TEST_F(NackTest, ShouldNackInLossyNetwork) { AudioReceiveStream::Stats recv_stats = receive_stream()->GetStats(/*get_and_clear_legacy_stats=*/true); EXPECT_GT(recv_stats.nacks_sent, 0U); + AudioSendStream::Stats send_stats = send_stream()->GetStats(); + EXPECT_GT(send_stats.retransmitted_packets_sent, 0U); + EXPECT_GT(send_stats.nacks_rcvd, 0U); } } test; diff --git a/call/audio_send_stream.h b/call/audio_send_stream.h index d21dff4889..e084d4219d 100644 --- a/call/audio_send_stream.h +++ b/call/audio_send_stream.h @@ -70,6 +70,7 @@ class AudioSendStream : public AudioSender { // per-pair the ReportBlockData represents the latest Report Block that was // received for that pair. std::vector report_block_datas; + uint32_t nacks_rcvd = 0; }; struct Config { diff --git a/media/base/media_channel.h b/media/base/media_channel.h index 979a5c1477..7b9a6f138c 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h @@ -372,6 +372,8 @@ struct MediaSenderInfo { int packets_sent = 0; // https://w3c.github.io/webrtc-stats/#dom-rtcoutboundrtpstreamstats-retransmittedpacketssent uint64_t retransmitted_packets_sent = 0; + // https://w3c.github.io/webrtc-stats/#dom-rtcoutboundrtpstreamstats-nackcount + uint32_t nacks_rcvd = 0; int packets_lost = 0; float fraction_lost = 0.0f; int64_t rtt_ms = 0; @@ -535,7 +537,6 @@ struct VideoSenderInfo : public MediaSenderInfo { std::string encoder_implementation_name; int firs_rcvd = 0; int plis_rcvd = 0; - int nacks_rcvd = 0; int send_frame_width = 0; int send_frame_height = 0; int frames = 0; diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index e2d83127c4..1cad35a4bf 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -1894,7 +1894,7 @@ TEST_F(WebRtcVideoChannelBaseTest, GetStats) { EXPECT_EQ(DefaultCodec().id, *info.senders[0].codec_payload_type); EXPECT_EQ(0, info.senders[0].firs_rcvd); EXPECT_EQ(0, info.senders[0].plis_rcvd); - EXPECT_EQ(0, info.senders[0].nacks_rcvd); + EXPECT_EQ(0u, info.senders[0].nacks_rcvd); EXPECT_EQ(kVideoWidth, info.senders[0].send_frame_width); EXPECT_EQ(kVideoHeight, info.senders[0].send_frame_height); EXPECT_GT(info.senders[0].framerate_input, 0); @@ -5555,7 +5555,7 @@ TEST_F(WebRtcVideoChannelTest, GetAggregatedStatsReportWithoutSubStreams) { // Comes from substream only. EXPECT_EQ(sender.firs_rcvd, 0); EXPECT_EQ(sender.plis_rcvd, 0); - EXPECT_EQ(sender.nacks_rcvd, 0); + EXPECT_EQ(sender.nacks_rcvd, 0u); EXPECT_EQ(sender.send_frame_width, 0); EXPECT_EQ(sender.send_frame_height, 0); @@ -5679,9 +5679,8 @@ TEST_F(WebRtcVideoChannelTest, GetAggregatedStatsReportForSubStreams) { EXPECT_EQ( sender.plis_rcvd, static_cast(2 * substream.rtcp_packet_type_counts.pli_packets)); - EXPECT_EQ( - sender.nacks_rcvd, - static_cast(2 * substream.rtcp_packet_type_counts.nack_packets)); + EXPECT_EQ(sender.nacks_rcvd, + 2 * substream.rtcp_packet_type_counts.nack_packets); EXPECT_EQ(sender.send_frame_width, substream.width); EXPECT_EQ(sender.send_frame_height, substream.height); @@ -5800,8 +5799,7 @@ TEST_F(WebRtcVideoChannelTest, GetPerLayerStatsReportForSubStreams) { static_cast(substream.rtcp_packet_type_counts.fir_packets)); EXPECT_EQ(sender.plis_rcvd, static_cast(substream.rtcp_packet_type_counts.pli_packets)); - EXPECT_EQ(sender.nacks_rcvd, - static_cast(substream.rtcp_packet_type_counts.nack_packets)); + EXPECT_EQ(sender.nacks_rcvd, substream.rtcp_packet_type_counts.nack_packets); EXPECT_EQ(sender.send_frame_width, substream.width); EXPECT_EQ(sender.send_frame_height, substream.height); @@ -6122,15 +6120,15 @@ TEST_F(WebRtcVideoChannelTest, GetStatsTranslatesSendRtcpPacketTypesCorrectly) { cricket::VideoMediaInfo info; ASSERT_TRUE(channel_->GetStats(&info)); EXPECT_EQ(2, info.senders[0].firs_rcvd); - EXPECT_EQ(3, info.senders[0].nacks_rcvd); + EXPECT_EQ(3u, info.senders[0].nacks_rcvd); EXPECT_EQ(4, info.senders[0].plis_rcvd); EXPECT_EQ(5, info.senders[1].firs_rcvd); - EXPECT_EQ(7, info.senders[1].nacks_rcvd); + EXPECT_EQ(7u, info.senders[1].nacks_rcvd); EXPECT_EQ(9, info.senders[1].plis_rcvd); EXPECT_EQ(7, info.aggregated_senders[0].firs_rcvd); - EXPECT_EQ(10, info.aggregated_senders[0].nacks_rcvd); + EXPECT_EQ(10u, info.aggregated_senders[0].nacks_rcvd); EXPECT_EQ(13, info.aggregated_senders[0].plis_rcvd); } diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index f2783f6f92..aa80c8724a 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/media/engine/webrtc_voice_engine.cc @@ -2320,6 +2320,7 @@ bool WebRtcVoiceMediaChannel::GetStats(VoiceMediaInfo* info, sinfo.retransmitted_packets_sent = stats.retransmitted_packets_sent; sinfo.packets_lost = stats.packets_lost; sinfo.fraction_lost = stats.fraction_lost; + sinfo.nacks_rcvd = stats.nacks_rcvd; sinfo.codec_name = stats.codec_name; sinfo.codec_payload_type = stats.codec_payload_type; sinfo.jitter_ms = stats.jitter_ms; diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc index 070400c58f..6599d0ef49 100644 --- a/pc/rtc_stats_collector.cc +++ b/pc/rtc_stats_collector.cc @@ -516,6 +516,7 @@ void SetOutboundRTPStreamStatsFromMediaSenderInfo( static_cast(media_sender_info.header_and_padding_bytes_sent); outbound_stats->retransmitted_bytes_sent = media_sender_info.retransmitted_bytes_sent; + outbound_stats->nack_count = media_sender_info.nacks_rcvd; } void SetOutboundRTPStreamStatsFromVoiceSenderInfo( @@ -550,8 +551,6 @@ void SetOutboundRTPStreamStatsFromVideoSenderInfo( static_cast(video_sender_info.firs_rcvd); outbound_video->pli_count = static_cast(video_sender_info.plis_rcvd); - outbound_video->nack_count = - static_cast(video_sender_info.nacks_rcvd); if (video_sender_info.qp_sum) outbound_video->qp_sum = *video_sender_info.qp_sum; outbound_video->frames_encoded = video_sender_info.frames_encoded; diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc index ca4f48e913..2ac0737715 100644 --- a/pc/rtc_stats_collector_unittest.cc +++ b/pc/rtc_stats_collector_unittest.cc @@ -2165,6 +2165,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Audio) { voice_media_info.senders[0].payload_bytes_sent = 3; voice_media_info.senders[0].header_and_padding_bytes_sent = 12; voice_media_info.senders[0].retransmitted_bytes_sent = 30; + voice_media_info.senders[0].nacks_rcvd = 31; voice_media_info.senders[0].codec_payload_type = 42; RtpCodecParameters codec_parameters; @@ -2198,6 +2199,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Audio) { expected_audio.bytes_sent = 3; expected_audio.header_bytes_sent = 12; expected_audio.retransmitted_bytes_sent = 30; + expected_audio.nack_count = 31; ASSERT_TRUE(report->Get(expected_audio.id())); EXPECT_EQ( @@ -2562,6 +2564,7 @@ TEST_F(RTCStatsCollectorTest, CollectNoStreamRTCOutboundRTPStreamStats_Audio) { voice_media_info.senders[0].payload_bytes_sent = 3; voice_media_info.senders[0].header_and_padding_bytes_sent = 4; voice_media_info.senders[0].retransmitted_bytes_sent = 30; + voice_media_info.senders[0].nacks_rcvd = 31; voice_media_info.senders[0].codec_payload_type = 42; RtpCodecParameters codec_parameters; @@ -2595,6 +2598,7 @@ TEST_F(RTCStatsCollectorTest, CollectNoStreamRTCOutboundRTPStreamStats_Audio) { expected_audio.bytes_sent = 3; expected_audio.header_bytes_sent = 4; expected_audio.retransmitted_bytes_sent = 30; + expected_audio.nack_count = 31; ASSERT_TRUE(report->Get(expected_audio.id())); EXPECT_EQ( diff --git a/pc/rtc_stats_integrationtest.cc b/pc/rtc_stats_integrationtest.cc index c15ffbf94a..2dfe1b5cd5 100644 --- a/pc/rtc_stats_integrationtest.cc +++ b/pc/rtc_stats_integrationtest.cc @@ -934,7 +934,6 @@ class RTCStatsReportVerifier { RTCVideoSourceStats::kType); verifier.TestMemberIsNonNegative(outbound_stream.fir_count); verifier.TestMemberIsNonNegative(outbound_stream.pli_count); - verifier.TestMemberIsNonNegative(outbound_stream.nack_count); if (*outbound_stream.frames_encoded > 0) { verifier.TestMemberIsNonNegative(outbound_stream.qp_sum); } else { @@ -943,11 +942,11 @@ class RTCStatsReportVerifier { } else { verifier.TestMemberIsUndefined(outbound_stream.fir_count); verifier.TestMemberIsUndefined(outbound_stream.pli_count); - verifier.TestMemberIsUndefined(outbound_stream.nack_count); verifier.TestMemberIsIDReference(outbound_stream.media_source_id, RTCAudioSourceStats::kType); verifier.TestMemberIsUndefined(outbound_stream.qp_sum); } + verifier.TestMemberIsNonNegative(outbound_stream.nack_count); verifier.TestMemberIsOptionalIDReference( outbound_stream.remote_id, RTCRemoteInboundRtpStreamStats::kType); verifier.TestMemberIsNonNegative(outbound_stream.packets_sent);