Implement nack_count metric for outbound audio rtp streams.
Bug: webrtc:12510
Change-Id: Ia035885bced3c3d202bb9ffeb88c2556d4830e92
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/225021
Reviewed-by: Sam Zackrisson <saza@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Jakob Ivarsson <jakobi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34444}
diff --git a/api/stats/rtcstats_objects.h b/api/stats/rtcstats_objects.h
index ebc6aff..2030380 100644
--- a/api/stats/rtcstats_objects.h
+++ b/api/stats/rtcstats_objects.h
@@ -553,8 +553,6 @@
// FIR and PLI counts are only defined for |media_type == "video"|.
RTCStatsMember<uint32_t> fir_count;
RTCStatsMember<uint32_t> 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<uint32_t> nack_count;
RTCStatsMember<uint64_t> qp_sum;
};
diff --git a/audio/audio_send_stream.cc b/audio/audio_send_stream.cc
index 5d7bc71..62dd53d 100644
--- a/audio/audio_send_stream.cc
+++ b/audio/audio_send_stream.cc
@@ -498,6 +498,8 @@
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 52dd528..06e9238 100644
--- a/audio/channel_send.cc
+++ b/audio/channel_send.cc
@@ -60,8 +60,9 @@
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 @@
rtc::scoped_refptr<webrtc::FrameTransformerInterface> 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 @@
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 @@
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 @@
uint32_t ssrc,
rtc::scoped_refptr<FrameTransformerInterface> 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 @@
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 @@
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<AudioFrame> audio_frame) {
RTC_DCHECK_RUNS_SERIALIZED(&audio_thread_race_checker_);
diff --git a/audio/channel_send.h b/audio/channel_send.h
index cbdb3ee..67391af 100644
--- a/audio/channel_send.h
+++ b/audio/channel_send.h
@@ -45,6 +45,7 @@
// ReportBlockData represents the latest Report Block that was received for
// that pair.
std::vector<ReportBlockData> 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 714a877..13cfe74 100644
--- a/audio/test/nack_test.cc
+++ b/audio/test/nack_test.cc
@@ -20,7 +20,7 @@
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 @@
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 d21dff4..e084d42 100644
--- a/call/audio_send_stream.h
+++ b/call/audio_send_stream.h
@@ -70,6 +70,7 @@
// per-pair the ReportBlockData represents the latest Report Block that was
// received for that pair.
std::vector<ReportBlockData> 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 979a5c1..7b9a6f1 100644
--- a/media/base/media_channel.h
+++ b/media/base/media_channel.h
@@ -372,6 +372,8 @@
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 @@
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 e2d8312..1cad35a 100644
--- a/media/engine/webrtc_video_engine_unittest.cc
+++ b/media/engine/webrtc_video_engine_unittest.cc
@@ -1894,7 +1894,7 @@
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 @@
// 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 @@
EXPECT_EQ(
sender.plis_rcvd,
static_cast<int>(2 * substream.rtcp_packet_type_counts.pli_packets));
- EXPECT_EQ(
- sender.nacks_rcvd,
- static_cast<int>(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 @@
static_cast<int>(substream.rtcp_packet_type_counts.fir_packets));
EXPECT_EQ(sender.plis_rcvd,
static_cast<int>(substream.rtcp_packet_type_counts.pli_packets));
- EXPECT_EQ(sender.nacks_rcvd,
- static_cast<int>(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 @@
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 f2783f6..aa80c87 100644
--- a/media/engine/webrtc_voice_engine.cc
+++ b/media/engine/webrtc_voice_engine.cc
@@ -2320,6 +2320,7 @@
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 070400c..6599d0e 100644
--- a/pc/rtc_stats_collector.cc
+++ b/pc/rtc_stats_collector.cc
@@ -516,6 +516,7 @@
static_cast<uint64_t>(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 @@
static_cast<uint32_t>(video_sender_info.firs_rcvd);
outbound_video->pli_count =
static_cast<uint32_t>(video_sender_info.plis_rcvd);
- outbound_video->nack_count =
- static_cast<uint32_t>(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 ca4f48e..2ac0737 100644
--- a/pc/rtc_stats_collector_unittest.cc
+++ b/pc/rtc_stats_collector_unittest.cc
@@ -2165,6 +2165,7 @@
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 @@
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 @@
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 @@
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 c15ffbf..2dfe1b5 100644
--- a/pc/rtc_stats_integrationtest.cc
+++ b/pc/rtc_stats_integrationtest.cc
@@ -934,7 +934,6 @@
RTCVideoSourceStats::kType);
verifier.TestMemberIsNonNegative<uint32_t>(outbound_stream.fir_count);
verifier.TestMemberIsNonNegative<uint32_t>(outbound_stream.pli_count);
- verifier.TestMemberIsNonNegative<uint32_t>(outbound_stream.nack_count);
if (*outbound_stream.frames_encoded > 0) {
verifier.TestMemberIsNonNegative<uint64_t>(outbound_stream.qp_sum);
} else {
@@ -943,11 +942,11 @@
} 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<uint32_t>(outbound_stream.nack_count);
verifier.TestMemberIsOptionalIDReference(
outbound_stream.remote_id, RTCRemoteInboundRtpStreamStats::kType);
verifier.TestMemberIsNonNegative<uint32_t>(outbound_stream.packets_sent);
