From be24c94c95056e4f0a22039f25f2fa8a27be6b66 Mon Sep 17 00:00:00 2001 From: jbauch Date: Mon, 22 Jun 2015 15:06:43 -0700 Subject: [PATCH] Set / verify stats report timestamps. This CL updates the track report timestamps which were fixed at "0" before and updates the timestamps in reports for local audio tracks. Also the timestamps are checked in various tests to make sure no "0" is returned. Original CL is at https://webrtc-codereview.appspot.com/51829004/ BUG=webrtc:4316 TBR=hta@webrtc.org, pthatcher@webrtc.org Review URL: https://codereview.webrtc.org/1204493002 Cr-Commit-Position: refs/heads/master@{#9485} --- talk/app/webrtc/peerconnection_unittest.cc | 7 ++++ talk/app/webrtc/statscollector.cc | 33 +++++++++++++++---- talk/app/webrtc/statscollector.h | 7 ++++ talk/app/webrtc/statscollector_unittest.cc | 8 +++++ .../webrtc/test/mockpeerconnectionobservers.h | 6 ++++ 5 files changed, 55 insertions(+), 6 deletions(-) diff --git a/talk/app/webrtc/peerconnection_unittest.cc b/talk/app/webrtc/peerconnection_unittest.cc index 99ee9be867..b9d80196e6 100644 --- a/talk/app/webrtc/peerconnection_unittest.cc +++ b/talk/app/webrtc/peerconnection_unittest.cc @@ -350,6 +350,7 @@ class PeerConnectionTestClientBase EXPECT_TRUE(peer_connection_->GetStats( observer, track, PeerConnectionInterface::kStatsOutputLevelStandard)); EXPECT_TRUE_WAIT(observer->called(), kMaxWaitMs); + EXPECT_NE(0, observer->timestamp()); return observer->AudioOutputLevel(); } @@ -359,6 +360,7 @@ class PeerConnectionTestClientBase EXPECT_TRUE(peer_connection_->GetStats( observer, NULL, PeerConnectionInterface::kStatsOutputLevelStandard)); EXPECT_TRUE_WAIT(observer->called(), kMaxWaitMs); + EXPECT_NE(0, observer->timestamp()); return observer->AudioInputLevel(); } @@ -368,6 +370,7 @@ class PeerConnectionTestClientBase EXPECT_TRUE(peer_connection_->GetStats( observer, track, PeerConnectionInterface::kStatsOutputLevelStandard)); EXPECT_TRUE_WAIT(observer->called(), kMaxWaitMs); + EXPECT_NE(0, observer->timestamp()); return observer->BytesReceived(); } @@ -377,6 +380,7 @@ class PeerConnectionTestClientBase EXPECT_TRUE(peer_connection_->GetStats( observer, track, PeerConnectionInterface::kStatsOutputLevelStandard)); EXPECT_TRUE_WAIT(observer->called(), kMaxWaitMs); + EXPECT_NE(0, observer->timestamp()); return observer->BytesSent(); } @@ -386,6 +390,7 @@ class PeerConnectionTestClientBase EXPECT_TRUE(peer_connection_->GetStats( observer, NULL, PeerConnectionInterface::kStatsOutputLevelStandard)); EXPECT_TRUE_WAIT(observer->called(), kMaxWaitMs); + EXPECT_NE(0, observer->timestamp()); int bw = observer->AvailableReceiveBandwidth(); return bw; } @@ -396,6 +401,7 @@ class PeerConnectionTestClientBase EXPECT_TRUE(peer_connection_->GetStats( observer, NULL, PeerConnectionInterface::kStatsOutputLevelStandard)); EXPECT_TRUE_WAIT(observer->called(), kMaxWaitMs); + EXPECT_NE(0, observer->timestamp()); return observer->DtlsCipher(); } @@ -405,6 +411,7 @@ class PeerConnectionTestClientBase EXPECT_TRUE(peer_connection_->GetStats( observer, NULL, PeerConnectionInterface::kStatsOutputLevelStandard)); EXPECT_TRUE_WAIT(observer->called(), kMaxWaitMs); + EXPECT_NE(0, observer->timestamp()); return observer->SrtpCipher(); } diff --git a/talk/app/webrtc/statscollector.cc b/talk/app/webrtc/statscollector.cc index 131d581e5d..ad64639eb9 100644 --- a/talk/app/webrtc/statscollector.cc +++ b/talk/app/webrtc/statscollector.cc @@ -80,18 +80,25 @@ StatsReport::Id GetTransportIdFromProxy(const cricket::ProxyTransportMap& map, found->second, cricket::ICE_CANDIDATE_COMPONENT_RTP); } -void AddTrackReport(StatsCollection* reports, const std::string& track_id) { +StatsReport* AddTrackReport(StatsCollection* reports, + const std::string& track_id) { // Adds an empty track report. StatsReport::Id id( StatsReport::NewTypedId(StatsReport::kStatsReportTypeTrack, track_id)); StatsReport* report = reports->ReplaceOrAddNew(id); report->AddString(StatsReport::kStatsValueNameTrackId, track_id); + return report; } template -void CreateTrackReports(const TrackVector& tracks, StatsCollection* reports) { - for (const auto& track : tracks) - AddTrackReport(reports, track->id()); +void CreateTrackReports(const TrackVector& tracks, StatsCollection* reports, + TrackIdMap& track_ids) { + for (const auto& track : tracks) { + const std::string& track_id = track->id(); + StatsReport* report = AddTrackReport(reports, track_id); + DCHECK(report != nullptr); + track_ids[track_id] = report; + } } void ExtractCommonSendProperties(const cricket::MediaSenderInfo& info, @@ -365,9 +372,9 @@ void StatsCollector::AddStream(MediaStreamInterface* stream) { DCHECK(stream != NULL); CreateTrackReports(stream->GetAudioTracks(), - &reports_); + &reports_, track_ids_); CreateTrackReports(stream->GetVideoTracks(), - &reports_); + &reports_, track_ids_); } void StatsCollector::AddLocalAudioTrack(AudioTrackInterface* audio_track, @@ -466,6 +473,7 @@ StatsCollector::UpdateStats(PeerConnectionInterface::StatsOutputLevel level) { ExtractVoiceInfo(); ExtractVideoInfo(level); ExtractDataInfo(); + UpdateTrackReports(); } } @@ -869,6 +877,7 @@ void StatsCollector::UpdateStatsFromExistingLocalAudioTracks() { if (!v || v->string_val() != track->id()) continue; + report->set_timestamp(stats_gathering_started_); UpdateReportFromAudioTrack(track, report); } } @@ -916,6 +925,18 @@ bool StatsCollector::GetTrackIdBySsrc(uint32 ssrc, std::string* track_id, return true; } +void StatsCollector::UpdateTrackReports() { + DCHECK(session_->signaling_thread()->IsCurrent()); + + rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls; + + for (const auto& entry : track_ids_) { + StatsReport* report = entry.second; + report->set_timestamp(stats_gathering_started_); + } + +} + void StatsCollector::ClearUpdateStatsCacheForTest() { stats_gathering_started_ = 0; } diff --git a/talk/app/webrtc/statscollector.h b/talk/app/webrtc/statscollector.h index 3c0aaf9b8e..99130a3f01 100644 --- a/talk/app/webrtc/statscollector.h +++ b/talk/app/webrtc/statscollector.h @@ -52,6 +52,9 @@ const char* IceCandidateTypeToStatsType(const std::string& candidate_type); // only used by stats collector. const char* AdapterTypeToStatsType(rtc::AdapterType type); +// A mapping between track ids and their StatsReport. +typedef std::map TrackIdMap; + class StatsCollector { public: // The caller is responsible for ensuring that the session outlives the @@ -139,8 +142,12 @@ class StatsCollector { bool GetTrackIdBySsrc(uint32 ssrc, std::string* track_id, StatsReport::Direction direction); + // Helper method to update the timestamp of track records. + void UpdateTrackReports(); + // A collection for all of our stats reports. StatsCollection reports_; + TrackIdMap track_ids_; // Raw pointer to the session the statistics are gathered from. WebRtcSession* const session_; double stats_gathering_started_; diff --git a/talk/app/webrtc/statscollector_unittest.cc b/talk/app/webrtc/statscollector_unittest.cc index ed035acba4..315366ceca 100644 --- a/talk/app/webrtc/statscollector_unittest.cc +++ b/talk/app/webrtc/statscollector_unittest.cc @@ -597,6 +597,7 @@ class StatsCollectorTest : public testing::Test { const StatsReport* report = FindNthReportByType( *reports, StatsReport::kStatsReportTypeSsrc, 1); EXPECT_FALSE(report == NULL); + EXPECT_EQ(stats->GetTimeNow(), report->timestamp()); std::string track_id = ExtractSsrcStatsValue( *reports, StatsReport::kStatsValueNameTrackId); EXPECT_EQ(audio_track->id(), track_id); @@ -619,6 +620,7 @@ class StatsCollectorTest : public testing::Test { const StatsReport* track_report = FindNthReportByType( track_reports, StatsReport::kStatsReportTypeSsrc, 1); EXPECT_TRUE(track_report); + EXPECT_EQ(stats->GetTimeNow(), track_report->timestamp()); track_id = ExtractSsrcStatsValue(track_reports, StatsReport::kStatsValueNameTrackId); EXPECT_EQ(audio_track->id(), track_id); @@ -929,6 +931,7 @@ TEST_F(StatsCollectorTest, TrackObjectExistsWithoutUpdateStats) { stats.GetStats(NULL, &reports); EXPECT_EQ((size_t)1, reports.size()); EXPECT_EQ(StatsReport::kStatsReportTypeTrack, reports[0]->type()); + EXPECT_EQ(0, reports[0]->timestamp()); std::string trackValue = ExtractStatsValue(StatsReport::kStatsReportTypeTrack, @@ -991,6 +994,7 @@ TEST_F(StatsCollectorTest, TrackAndSsrcObjectExistAfterUpdateSsrcStats) { track_report = FindNthReportByType( reports, StatsReport::kStatsReportTypeTrack, 1); EXPECT_TRUE(track_report); + EXPECT_EQ(stats.GetTimeNow(), track_report->timestamp()); std::string ssrc_id = ExtractSsrcStatsValue( reports, StatsReport::kStatsValueNameSsrc); @@ -1179,6 +1183,7 @@ TEST_F(StatsCollectorTest, ReportsFromRemoteTrack) { const StatsReport* track_report = FindNthReportByType( reports, StatsReport::kStatsReportTypeTrack, 1); EXPECT_TRUE(track_report); + EXPECT_EQ(stats.GetTimeNow(), track_report->timestamp()); std::string ssrc_id = ExtractSsrcStatsValue( reports, StatsReport::kStatsValueNameSsrc); @@ -1553,6 +1558,7 @@ TEST_F(StatsCollectorTest, GetStatsAfterRemoveAudioStream) { const StatsReport* report = FindNthReportByType( reports, StatsReport::kStatsReportTypeSsrc, 1); EXPECT_FALSE(report == NULL); + EXPECT_EQ(stats.GetTimeNow(), report->timestamp()); std::string track_id = ExtractSsrcStatsValue( reports, StatsReport::kStatsValueNameTrackId); EXPECT_EQ(kLocalTrackId, track_id); @@ -1630,6 +1636,7 @@ TEST_F(StatsCollectorTest, LocalAndRemoteTracksWithSameSsrc) { const StatsReport* track_report = FindNthReportByType( reports, StatsReport::kStatsReportTypeSsrc, 1); EXPECT_TRUE(track_report); + EXPECT_EQ(stats.GetTimeNow(), track_report->timestamp()); std::string track_id = ExtractSsrcStatsValue( reports, StatsReport::kStatsValueNameTrackId); EXPECT_EQ(kLocalTrackId, track_id); @@ -1641,6 +1648,7 @@ TEST_F(StatsCollectorTest, LocalAndRemoteTracksWithSameSsrc) { track_report = FindNthReportByType(reports, StatsReport::kStatsReportTypeSsrc, 1); EXPECT_TRUE(track_report); + EXPECT_EQ(stats.GetTimeNow(), track_report->timestamp()); track_id = ExtractSsrcStatsValue(reports, StatsReport::kStatsValueNameTrackId); EXPECT_EQ(kRemoteTrackId, track_id); diff --git a/talk/app/webrtc/test/mockpeerconnectionobservers.h b/talk/app/webrtc/test/mockpeerconnectionobservers.h index f31b16c744..580a0fbb85 100644 --- a/talk/app/webrtc/test/mockpeerconnectionobservers.h +++ b/talk/app/webrtc/test/mockpeerconnectionobservers.h @@ -127,6 +127,7 @@ class MockStatsObserver : public webrtc::StatsObserver { stats_.number_of_reports = reports.size(); for (const auto* r : reports) { if (r->type() == StatsReport::kStatsReportTypeSsrc) { + stats_.timestamp = r->timestamp(); GetIntValue(r, StatsReport::kStatsValueNameAudioOutputLevel, &stats_.audio_output_level); GetIntValue(r, StatsReport::kStatsValueNameAudioInputLevel, @@ -136,9 +137,11 @@ class MockStatsObserver : public webrtc::StatsObserver { GetIntValue(r, StatsReport::kStatsValueNameBytesSent, &stats_.bytes_sent); } else if (r->type() == StatsReport::kStatsReportTypeBwe) { + stats_.timestamp = r->timestamp(); GetIntValue(r, StatsReport::kStatsValueNameAvailableReceiveBandwidth, &stats_.available_receive_bandwidth); } else if (r->type() == StatsReport::kStatsReportTypeComponent) { + stats_.timestamp = r->timestamp(); GetStringValue(r, StatsReport::kStatsValueNameDtlsCipher, &stats_.dtls_cipher); GetStringValue(r, StatsReport::kStatsValueNameSrtpCipher, @@ -149,6 +152,7 @@ class MockStatsObserver : public webrtc::StatsObserver { bool called() const { return called_; } size_t number_of_reports() const { return stats_.number_of_reports; } + double timestamp() const { return stats_.timestamp; } int AudioOutputLevel() const { ASSERT(called_); @@ -210,6 +214,7 @@ class MockStatsObserver : public webrtc::StatsObserver { struct { void Clear() { number_of_reports = 0; + timestamp = 0; audio_output_level = 0; audio_input_level = 0; bytes_received = 0; @@ -220,6 +225,7 @@ class MockStatsObserver : public webrtc::StatsObserver { } size_t number_of_reports; + double timestamp; int audio_output_level; int audio_input_level; int bytes_received;