Filter out some variables with initial -1 in the stats report.
If we call GetStats in PeerConnection before receiving the remote answer, we will get some variables in the StatsReports which are initially set to be -1.
Several conditions are added when extracting the info for the report in StatsCollector.
Those variables include:
gooRtt,
dataChannelId,
googEchoCancellationEchoDelayMedian,
googEchoCancellationEchoQualityMin,
googEchoCancellationEchoDelayStdDev,
googJitterReceived,
audioInputLevel,
googCaptureStartNtpTimeMs
packetsLost.
BUG=webrtc:3377
Review-Url: https://codereview.webrtc.org/1875873002
Cr-Commit-Position: refs/heads/master@{#12735}
diff --git a/webrtc/api/statscollector.cc b/webrtc/api/statscollector.cc
index 82c97cc..df1d0aa 100644
--- a/webrtc/api/statscollector.cc
+++ b/webrtc/api/statscollector.cc
@@ -88,7 +88,9 @@
StatsReport* report) {
report->AddString(StatsReport::kStatsValueNameCodecName, info.codec_name);
report->AddInt64(StatsReport::kStatsValueNameBytesSent, info.bytes_sent);
- report->AddInt64(StatsReport::kStatsValueNameRtt, info.rtt_ms);
+ if (info.rtt_ms >= 0) {
+ report->AddInt64(StatsReport::kStatsValueNameRtt, info.rtt_ms);
+ }
}
void ExtractCommonReceiveProperties(const cricket::MediaReceiverInfo& info,
@@ -105,17 +107,23 @@
int echo_delay_std_ms) {
report->AddBoolean(StatsReport::kStatsValueNameTypingNoiseState,
typing_noise_detected);
- report->AddFloat(StatsReport::kStatsValueNameEchoCancellationQualityMin,
- aec_quality_min);
+ if (aec_quality_min >= 0.0f) {
+ report->AddFloat(StatsReport::kStatsValueNameEchoCancellationQualityMin,
+ aec_quality_min);
+ }
const IntForAdd ints[] = {
- { StatsReport::kStatsValueNameEchoReturnLoss, echo_return_loss },
- { StatsReport::kStatsValueNameEchoReturnLossEnhancement,
- echo_return_loss_enhancement },
{ StatsReport::kStatsValueNameEchoDelayMedian, echo_delay_median_ms },
{ StatsReport::kStatsValueNameEchoDelayStdDev, echo_delay_std_ms },
};
- for (const auto& i : ints)
- report->AddInt(i.name, i.value);
+ for (const auto& i : ints) {
+ if (i.value >= 0) {
+ report->AddInt(i.name, i.value);
+ }
+ }
+ // These can take on valid negative values.
+ report->AddInt(StatsReport::kStatsValueNameEchoReturnLoss, echo_return_loss);
+ report->AddInt(StatsReport::kStatsValueNameEchoReturnLossEnhancement,
+ echo_return_loss_enhancement);
}
void ExtractStats(const cricket::VoiceReceiverInfo& info, StatsReport* report) {
@@ -131,7 +139,6 @@
};
const IntForAdd ints[] = {
- { StatsReport::kStatsValueNameAudioOutputLevel, info.audio_level },
{ StatsReport::kStatsValueNameCurrentDelayMs, info.delay_estimate_ms },
{ StatsReport::kStatsValueNameDecodingCNG, info.decoding_cng },
{ StatsReport::kStatsValueNameDecodingCTN, info.decoding_calls_to_neteq },
@@ -153,11 +160,17 @@
for (const auto& i : ints)
report->AddInt(i.name, i.value);
+ if (info.audio_level >= 0) {
+ report->AddInt(StatsReport::kStatsValueNameAudioOutputLevel,
+ info.audio_level);
+ }
report->AddInt64(StatsReport::kStatsValueNameBytesReceived,
info.bytes_rcvd);
- report->AddInt64(StatsReport::kStatsValueNameCaptureStartNtpTimeMs,
- info.capture_start_ntp_time_ms);
+ if (info.capture_start_ntp_time_ms >= 0) {
+ report->AddInt64(StatsReport::kStatsValueNameCaptureStartNtpTimeMs,
+ info.capture_start_ntp_time_ms);
+ }
report->AddString(StatsReport::kStatsValueNameMediaType, "audio");
}
@@ -177,8 +190,11 @@
{ StatsReport::kStatsValueNamePacketsSent, info.packets_sent },
};
- for (const auto& i : ints)
- report->AddInt(i.name, i.value);
+ for (const auto& i : ints) {
+ if (i.value >= 0) {
+ report->AddInt(i.name, i.value);
+ }
+ }
report->AddString(StatsReport::kStatsValueNameMediaType, "audio");
}
@@ -188,8 +204,10 @@
info.decoder_implementation_name);
report->AddInt64(StatsReport::kStatsValueNameBytesReceived,
info.bytes_rcvd);
- report->AddInt64(StatsReport::kStatsValueNameCaptureStartNtpTimeMs,
- info.capture_start_ntp_time_ms);
+ if (info.capture_start_ntp_time_ms >= 0) {
+ report->AddInt64(StatsReport::kStatsValueNameCaptureStartNtpTimeMs,
+ info.capture_start_ntp_time_ms);
+ }
const IntForAdd ints[] = {
{ StatsReport::kStatsValueNameCurrentDelayMs, info.current_delay_ms },
{ StatsReport::kStatsValueNameDecodeMs, info.decode_ms },
@@ -870,7 +888,10 @@
StatsReport* report = reports_.ReplaceOrAddNew(id);
report->set_timestamp(stats_gathering_started_);
report->AddString(StatsReport::kStatsValueNameLabel, dc->label());
- report->AddInt(StatsReport::kStatsValueNameDataChannelId, dc->id());
+ // Filter out the initial id (-1).
+ if (dc->id() >= 0) {
+ report->AddInt(StatsReport::kStatsValueNameDataChannelId, dc->id());
+ }
report->AddString(StatsReport::kStatsValueNameProtocol, dc->protocol());
report->AddString(StatsReport::kStatsValueNameState,
DataChannelInterface::DataStateString(dc->state()));
diff --git a/webrtc/api/statscollector_unittest.cc b/webrtc/api/statscollector_unittest.cc
index 00b50b6..9243aed 100644
--- a/webrtc/api/statscollector_unittest.cc
+++ b/webrtc/api/statscollector_unittest.cc
@@ -165,6 +165,48 @@
rtc::scoped_refptr<FakeAudioProcessor> processor_;
};
+// This fake audio processor is used to verify that the undesired initial values
+// (-1) will be filtered out.
+class FakeAudioProcessorWithInitValue : public webrtc::AudioProcessorInterface {
+ public:
+ FakeAudioProcessorWithInitValue() {}
+ ~FakeAudioProcessorWithInitValue() {}
+
+ private:
+ void GetStats(AudioProcessorInterface::AudioProcessorStats* stats) override {
+ stats->typing_noise_detected = false;
+ stats->echo_return_loss = -100;
+ stats->echo_return_loss_enhancement = -100;
+ stats->echo_delay_median_ms = -1;
+ stats->aec_quality_min = -1.0f;
+ stats->echo_delay_std_ms = -1;
+ }
+};
+
+class FakeAudioTrackWithInitValue
+ : public webrtc::MediaStreamTrack<webrtc::AudioTrackInterface> {
+ public:
+ explicit FakeAudioTrackWithInitValue(const std::string& id)
+ : webrtc::MediaStreamTrack<webrtc::AudioTrackInterface>(id),
+ processor_(
+ new rtc::RefCountedObject<FakeAudioProcessorWithInitValue>()) {}
+ std::string kind() const override { return "audio"; }
+ webrtc::AudioSourceInterface* GetSource() const override { return NULL; }
+ void AddSink(webrtc::AudioTrackSinkInterface* sink) override {}
+ void RemoveSink(webrtc::AudioTrackSinkInterface* sink) override {}
+ bool GetSignalLevel(int* level) override {
+ *level = 1;
+ return true;
+ }
+ rtc::scoped_refptr<webrtc::AudioProcessorInterface> GetAudioProcessor()
+ override {
+ return processor_;
+ }
+
+ private:
+ rtc::scoped_refptr<FakeAudioProcessorWithInitValue> processor_;
+};
+
bool GetValue(const StatsReport* report,
StatsReport::StatsValueName name,
std::string* value) {
@@ -444,7 +486,8 @@
}
void UpdateVoiceSenderInfoFromAudioTrack(
- FakeAudioTrack* audio_track, cricket::VoiceSenderInfo* voice_sender_info) {
+ AudioTrackInterface* audio_track,
+ cricket::VoiceSenderInfo* voice_sender_info) {
audio_track->GetSignalLevel(&voice_sender_info->audio_level);
webrtc::AudioProcessorInterface::AudioProcessorStats audio_processor_stats;
audio_track->GetAudioProcessor()->GetStats(&audio_processor_stats);
@@ -778,6 +821,29 @@
std::vector<rtc::scoped_refptr<DataChannel>> data_channels_;
};
+TEST_F(StatsCollectorTest, FilterOutNegativeDataChannelId) {
+ const std::string label = "hacks";
+ // The data channel id is from the Config which is -1 initially.
+ const int id = -1;
+ const std::string state = DataChannelInterface::DataStateString(
+ DataChannelInterface::DataState::kConnecting);
+
+ AddDataChannel(cricket::DCT_SCTP, label, id);
+ StatsCollectorForTest stats(&pc_);
+
+ stats.UpdateStats(PeerConnectionInterface::kStatsOutputLevelStandard);
+
+ StatsReports reports;
+ stats.GetStats(NULL, &reports);
+
+ const StatsReport* report =
+ FindNthReportByType(reports, StatsReport::kStatsReportTypeDataChannel, 1);
+
+ std::string value_in_report;
+ EXPECT_FALSE(GetValue(report, StatsReport::kStatsValueNameDataChannelId,
+ &value_in_report));
+}
+
// Verify that ExtractDataInfo populates reports.
TEST_F(StatsCollectorTest, ExtractDataInfo) {
const std::string label = "hacks";
@@ -1500,6 +1566,113 @@
std::move(remote_cert), std::vector<std::string>());
}
+// This test verifies that the audio/video related stats which are -1 initially
+// will be filtered out.
+TEST_F(StatsCollectorTest, FilterOutNegativeInitialValues) {
+ StatsCollectorForTest stats(&pc_);
+
+ EXPECT_CALL(session_, GetLocalCertificate(_, _))
+ .WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_))
+ .WillRepeatedly(Return(nullptr));
+
+ MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
+ // The transport_name known by the voice channel.
+ const std::string kVcName("vcname");
+ cricket::VoiceChannel voice_channel(worker_thread_, network_thread_,
+ media_engine_, media_channel, nullptr,
+ kVcName, false);
+
+ // Create a local stream with a local audio track and adds it to the stats.
+ if (stream_ == NULL)
+ stream_ = webrtc::MediaStream::Create("streamlabel");
+
+ rtc::scoped_refptr<FakeAudioTrackWithInitValue> local_track(
+ new rtc::RefCountedObject<FakeAudioTrackWithInitValue>(kLocalTrackId));
+ stream_->AddTrack(local_track);
+ EXPECT_CALL(session_, GetLocalTrackIdBySsrc(kSsrcOfTrack, _))
+ .WillOnce(DoAll(SetArgPointee<1>(kLocalTrackId), Return(true)));
+ stats.AddStream(stream_);
+ stats.AddLocalAudioTrack(local_track.get(), kSsrcOfTrack);
+
+ // Create a remote stream with a remote audio track and adds it to the stats.
+ rtc::scoped_refptr<webrtc::MediaStream> remote_stream(
+ webrtc::MediaStream::Create("remotestreamlabel"));
+ rtc::scoped_refptr<FakeAudioTrackWithInitValue> remote_track(
+ new rtc::RefCountedObject<FakeAudioTrackWithInitValue>(kRemoteTrackId));
+ EXPECT_CALL(session_, GetRemoteTrackIdBySsrc(kSsrcOfTrack, _))
+ .WillOnce(DoAll(SetArgPointee<1>(kRemoteTrackId), Return(true)));
+ remote_stream->AddTrack(remote_track);
+ stats.AddStream(remote_stream);
+
+ // Instruct the session to return stats containing the transport channel.
+ InitSessionStats(kVcName);
+ EXPECT_CALL(session_, GetTransportStats(_))
+ .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_), Return(true)));
+
+ cricket::VoiceSenderInfo voice_sender_info;
+ voice_sender_info.add_ssrc(kSsrcOfTrack);
+ // These values are set to -1 initially in audio_send_stream.
+ // The voice_sender_info will read the values from audio_send_stream.
+ voice_sender_info.rtt_ms = -1;
+ voice_sender_info.packets_lost = -1;
+ voice_sender_info.jitter_ms = -1;
+
+ // Some of the contents in |voice_sender_info| needs to be updated from the
+ // |audio_track_|.
+ UpdateVoiceSenderInfoFromAudioTrack(local_track.get(), &voice_sender_info);
+
+ cricket::VoiceReceiverInfo voice_receiver_info;
+ voice_receiver_info.add_ssrc(kSsrcOfTrack);
+ voice_receiver_info.capture_start_ntp_time_ms = -1;
+ voice_receiver_info.audio_level = -1;
+
+ // Constructs an ssrc stats update.
+ cricket::VoiceMediaInfo stats_read;
+ stats_read.senders.push_back(voice_sender_info);
+ stats_read.receivers.push_back(voice_receiver_info);
+
+ EXPECT_CALL(session_, voice_channel()).WillRepeatedly(Return(&voice_channel));
+ EXPECT_CALL(session_, video_channel()).WillRepeatedly(ReturnNull());
+ EXPECT_CALL(*media_channel, GetStats(_))
+ .WillRepeatedly(DoAll(SetArgPointee<0>(stats_read), Return(true)));
+
+ StatsReports reports;
+ stats.UpdateStats(PeerConnectionInterface::kStatsOutputLevelStandard);
+
+ // Get stats for the local track.
+ stats.GetStats(local_track.get(), &reports);
+ const StatsReport* report =
+ FindNthReportByType(reports, StatsReport::kStatsReportTypeSsrc, 1);
+ EXPECT_TRUE(report);
+ // The -1 will not be added to the stats report.
+ std::string value_in_report;
+ EXPECT_FALSE(
+ GetValue(report, StatsReport::kStatsValueNameRtt, &value_in_report));
+ EXPECT_FALSE(GetValue(report, StatsReport::kStatsValueNamePacketsLost,
+ &value_in_report));
+ EXPECT_FALSE(GetValue(report, StatsReport::kStatsValueNameJitterReceived,
+ &value_in_report));
+ EXPECT_FALSE(GetValue(report,
+ StatsReport::kStatsValueNameEchoCancellationQualityMin,
+ &value_in_report));
+ EXPECT_FALSE(GetValue(report, StatsReport::kStatsValueNameEchoDelayMedian,
+ &value_in_report));
+ EXPECT_FALSE(GetValue(report, StatsReport::kStatsValueNameEchoDelayStdDev,
+ &value_in_report));
+
+ // Get stats for the remote track.
+ reports.clear();
+ stats.GetStats(remote_track.get(), &reports);
+ report = FindNthReportByType(reports, StatsReport::kStatsReportTypeSsrc, 1);
+ EXPECT_TRUE(report);
+ EXPECT_FALSE(GetValue(report,
+ StatsReport::kStatsValueNameCaptureStartNtpTimeMs,
+ &value_in_report));
+ EXPECT_FALSE(GetValue(report, StatsReport::kStatsValueNameAudioInputLevel,
+ &value_in_report));
+}
+
// This test verifies that a local stats object can get statistics via
// AudioTrackInterface::GetStats() method.
TEST_F(StatsCollectorTest, GetStatsFromLocalAudioTrack) {
