From 4fb7e2584326050a707aef544028fa9cb616ec89 Mon Sep 17 00:00:00 2001 From: "tommi@webrtc.org" Date: Wed, 21 Jan 2015 11:36:18 +0000 Subject: [PATCH] Update StatsReport and by extension StatsCollector to reduce data copying. Summary of changes: * We're now using an enum for types instead of strings which both eliminates unecessary string creations+copies and further restricts the type to a known set at compile time. * IDs are now a separate type instead of a string, copying of Values is not possible and values are const to allow grabbing references outside of the statscollector. * StatsReport member variables are no longer public. * Consolidated code in StatsCollector (e.g. merged PrepareLocalReport and PrepareRemoteReport). * Refactored methods that forced copies of string (e.g. ExtractValueFromReport). * More asserts for thread correctness. * Using std::list for the StatsSet instead of a set since order is not important and updates are more efficient in list<>. BUG=2822 R=hta@webrtc.org, perkj@webrtc.org Review URL: https://webrtc-codereview.appspot.com/40439004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@8110 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/app/webrtc/statscollector.cc | 331 +++++++----------- talk/app/webrtc/statscollector.h | 32 +- talk/app/webrtc/statscollector_unittest.cc | 116 +++--- talk/app/webrtc/statstypes.cc | 327 ++++++++++++----- talk/app/webrtc/statstypes.h | 239 ++++++------- .../webrtc/test/mockpeerconnectionobservers.h | 4 +- 6 files changed, 580 insertions(+), 469 deletions(-) diff --git a/talk/app/webrtc/statscollector.cc b/talk/app/webrtc/statscollector.cc index 935fef5c0..125e52cc2 100644 --- a/talk/app/webrtc/statscollector.cc +++ b/talk/app/webrtc/statscollector.cc @@ -35,6 +35,8 @@ #include "webrtc/base/scoped_ptr.h" #include "webrtc/base/timing.h" +using rtc::scoped_ptr; + namespace webrtc { namespace { @@ -73,55 +75,26 @@ bool GetTransportIdFromProxy(const cricket::ProxyTransportMap& map, return false; } - std::ostringstream ost; // Component 1 is always used for RTP. - ost << "Channel-" << found->second << "-1"; - *transport = ost.str(); + scoped_ptr id( + StatsReport::NewComponentId(found->second, 1)); + // TODO(tommi): Should |transport| simply be of type StatsReport::Id? + // When we support more value types than string (e.g. int, double, vector etc) + // we should also support a value type for Id. + *transport = id->ToString(); return true; } -std::string StatsId(const std::string& type, const std::string& id) { - return type + "_" + id; -} - -std::string StatsId(const std::string& type, const std::string& id, - StatsCollector::TrackDirection direction) { - ASSERT(direction == StatsCollector::kSending || - direction == StatsCollector::kReceiving); - - // Strings for the direction of the track. - const char kSendDirection[] = "send"; - const char kRecvDirection[] = "recv"; - - const std::string direction_id = (direction == StatsCollector::kSending) ? - kSendDirection : kRecvDirection; - return type + "_" + id + "_" + direction_id; -} - -bool ExtractValueFromReport( - const StatsReport& report, - StatsReport::StatsValueName name, - std::string* value) { - StatsReport::Values::const_iterator it = report.values().begin(); - for (; it != report.values().end(); ++it) { - if ((*it)->name == name) { - *value = (*it)->value; - return true; - } - } - return false; -} - -void AddTrackReport(StatsSet* reports, const std::string& track_id) { +void AddTrackReport(StatsCollection* reports, const std::string& track_id) { // Adds an empty track report. - StatsReport* report = reports->ReplaceOrAddNew( - StatsId(StatsReport::kStatsReportTypeTrack, track_id)); - report->type = StatsReport::kStatsReportTypeTrack; + rtc::scoped_ptr id( + StatsReport::NewTypedId(StatsReport::kStatsReportTypeTrack, track_id)); + StatsReport* report = reports->ReplaceOrAddNew(id.Pass()); report->AddValue(StatsReport::kStatsValueNameTrackId, track_id); } template -void CreateTrackReports(const TrackVector& tracks, StatsSet* reports) { +void CreateTrackReports(const TrackVector& tracks, StatsCollection* reports) { for (size_t j = 0; j < tracks.size(); ++j) { webrtc::MediaStreamTrackInterface* track = tracks[j]; AddTrackReport(reports, track->id()); @@ -284,7 +257,7 @@ void ExtractStats(const cricket::BandwidthEstimationInfo& info, double stats_gathering_started, PeerConnectionInterface::StatsOutputLevel level, StatsReport* report) { - report->type = StatsReport::kStatsReportTypeBwe; + ASSERT(report->type() == StatsReport::kStatsReportTypeBwe); // Clear out stats from previous GatherStats calls if any. if (report->timestamp() != stats_gathering_started) { @@ -341,25 +314,20 @@ template void ExtractStatsFromList(const std::vector& data, const std::string& transport_id, StatsCollector* collector, - StatsCollector::TrackDirection direction) { - typename std::vector::const_iterator it = data.begin(); - for (; it != data.end(); ++it) { - std::string id; - uint32 ssrc = it->ssrc(); + StatsReport::Direction direction) { + for (const auto& d : data) { + uint32 ssrc = d.ssrc(); // Each track can have stats for both local and remote objects. // TODO(hta): Handle the case of multiple SSRCs per object. - StatsReport* report = collector->PrepareLocalReport(ssrc, transport_id, - direction); + StatsReport* report = collector->PrepareReport(true, ssrc, transport_id, + direction); if (report) - ExtractStats(*it, report); + ExtractStats(d, report); - if (it->remote_stats.size() > 0) { - report = collector->PrepareRemoteReport(ssrc, transport_id, - direction); - if (!report) { - continue; - } - ExtractRemoteStats(*it, report); + if (!d.remote_stats.empty()) { + report = collector->PrepareReport(false, ssrc, transport_id, direction); + if (report) + ExtractRemoteStats(d, report); } } } @@ -436,10 +404,14 @@ void StatsCollector::AddLocalAudioTrack(AudioTrackInterface* audio_track, // Create the kStatsReportTypeTrack report for the new track if there is no // report yet. - StatsReport* found = reports_.Find( - StatsId(StatsReport::kStatsReportTypeTrack, audio_track->id())); - if (!found) - AddTrackReport(&reports_, audio_track->id()); + rtc::scoped_ptr id( + StatsReport::NewTypedId(StatsReport::kStatsReportTypeTrack, + audio_track->id())); + StatsReport* report = reports_.Find(*id.get()); + if (!report) { + report = reports_.InsertNew(id.Pass()); + report->AddValue(StatsReport::kStatsValueNameTrackId, audio_track->id()); + } } void StatsCollector::RemoveLocalAudioTrack(AudioTrackInterface* audio_track, @@ -463,20 +435,19 @@ void StatsCollector::GetStats(MediaStreamTrackInterface* track, ASSERT(reports->empty()); if (!track) { - StatsSet::const_iterator it; - for (it = reports_.begin(); it != reports_.end(); ++it) - reports->push_back(&(*it)); + reports->reserve(reports_.size()); + for (auto* r : reports_) + reports->push_back(r); return; } - StatsReport* report = - reports_.Find(StatsId(StatsReport::kStatsReportTypeSession, - session_->id())); + StatsReport* report = reports_.Find(StatsReport::NewTypedId( + StatsReport::kStatsReportTypeSession, session_->id())); if (report) reports->push_back(report); - report = reports_.Find( - StatsId(StatsReport::kStatsReportTypeTrack, track->id())); + report = reports_.Find(StatsReport::NewTypedId( + StatsReport::kStatsReportTypeTrack, track->id())); if (!report) return; @@ -484,18 +455,14 @@ void StatsCollector::GetStats(MediaStreamTrackInterface* track, reports->push_back(report); std::string track_id; - for (StatsSet::const_iterator it = reports_.begin(); it != reports_.end(); - ++it) { - if (it->type != StatsReport::kStatsReportTypeSsrc) + for (const auto* r : reports_) { + if (r->type() != StatsReport::kStatsReportTypeSsrc) continue; - if (ExtractValueFromReport(*it, - StatsReport::kStatsValueNameTrackId, - &track_id)) { - if (track_id == track->id()) { - reports->push_back(&(*it)); - } - } + const StatsReport::Value* v = + r->FindValue(StatsReport::kStatsValueNameTrackId); + if (v && v->value == track->id()) + reports->push_back(r); } } @@ -520,14 +487,18 @@ StatsCollector::UpdateStats(PeerConnectionInterface::StatsOutputLevel level) { } } -StatsReport* StatsCollector::PrepareLocalReport( +StatsReport* StatsCollector::PrepareReport( + bool local, uint32 ssrc, const std::string& transport_id, - TrackDirection direction) { + StatsReport::Direction direction) { ASSERT(session_->signaling_thread()->IsCurrent()); const std::string ssrc_id = rtc::ToString(ssrc); - StatsReport* report = reports_.Find( - StatsId(StatsReport::kStatsReportTypeSsrc, ssrc_id, direction)); + rtc::scoped_ptr id(StatsReport::NewIdWithDirection( + local ? StatsReport::kStatsReportTypeSsrc : + StatsReport::kStatsReportTypeRemoteSsrc, + ssrc_id, direction)); + StatsReport* report = reports_.Find(*id.get()); // Use the ID of the track that is currently mapped to the SSRC, if any. std::string track_id; @@ -540,20 +511,28 @@ StatsReport* StatsCollector::PrepareLocalReport( // The ssrc is not used by any existing track. Keeps the old track id // since we want to report the stats for inactive ssrc. - ExtractValueFromReport(*report, - StatsReport::kStatsValueNameTrackId, - &track_id); + const StatsReport::Value* v = + report->FindValue(StatsReport::kStatsValueNameTrackId); + if (v) + track_id = v->value; } - report = GetOrCreateReport( - StatsReport::kStatsReportTypeSsrc, ssrc_id, direction); + if (!report) { + report = reports_.InsertNew(id.Pass()); + } else { + // Clear out stats from previous GatherStats calls if any. + // This is required since the report will be returned for the new values. + // Having the old values in the report will lead to multiple values with + // the same name. + // TODO(tommi): This seems to be pretty wasteful if some of these values + // have not changed (we basically throw them away just to recreate them). + // Figure out a way to not have to do this while not breaking the existing + // functionality. + report->ResetValues(); + } - // Clear out stats from previous GatherStats calls if any. - // This is required since the report will be returned for the new values. - // Having the old values in the report will lead to multiple values with - // the same name. - // TODO(xians): Consider changing StatsReport to use map instead of vector. - report->ResetValues(); + ASSERT(report->values().empty()); + // FYI - for remote reports, the timestamp will be overwritten later. report->set_timestamp(stats_gathering_started_); report->AddValue(StatsReport::kStatsValueNameSsrc, ssrc_id); @@ -564,49 +543,10 @@ StatsReport* StatsCollector::PrepareLocalReport( return report; } -StatsReport* StatsCollector::PrepareRemoteReport( - uint32 ssrc, - const std::string& transport_id, - TrackDirection direction) { - ASSERT(session_->signaling_thread()->IsCurrent()); - const std::string ssrc_id = rtc::ToString(ssrc); - StatsReport* report = reports_.Find( - StatsId(StatsReport::kStatsReportTypeRemoteSsrc, ssrc_id, direction)); - - // Use the ID of the track that is currently mapped to the SSRC, if any. - std::string track_id; - if (!GetTrackIdBySsrc(ssrc, &track_id, direction)) { - if (!report) { - // The ssrc is not used by any track or existing report, return NULL - // in such case to indicate no report is prepared for the ssrc. - return NULL; - } - - // The ssrc is not used by any existing track. Keeps the old track id - // since we want to report the stats for inactive ssrc. - ExtractValueFromReport(*report, - StatsReport::kStatsValueNameTrackId, - &track_id); - } - - report = GetOrCreateReport( - StatsReport::kStatsReportTypeRemoteSsrc, ssrc_id, direction); - - // Clear out stats from previous GatherStats calls if any. - // The timestamp will be added later. Zero it for debugging. - report->ResetValues(); - report->set_timestamp(0); - - report->AddValue(StatsReport::kStatsValueNameSsrc, ssrc_id); - report->AddValue(StatsReport::kStatsValueNameTrackId, track_id); - // Add the mapping of SSRC to transport. - report->AddValue(StatsReport::kStatsValueNameTransportId, - transport_id); - return report; -} - std::string StatsCollector::AddOneCertificateReport( const rtc::SSLCertificate* cert, const std::string& issuer_id) { + ASSERT(session_->signaling_thread()->IsCurrent()); + // TODO(bemasc): Move this computation to a helper class that caches these // values to reduce CPU use in GetStats. This will require adding a fast // SSLCertificate::Equals() method to detect certificate changes. @@ -633,9 +573,10 @@ std::string StatsCollector::AddOneCertificateReport( rtc::Base64::EncodeFromArray( der_buffer.data(), der_buffer.length(), &der_base64); - StatsReport* report = reports_.ReplaceOrAddNew( - StatsId(StatsReport::kStatsReportTypeCertificate, fingerprint)); - report->type = StatsReport::kStatsReportTypeCertificate; + rtc::scoped_ptr id( + StatsReport::NewTypedId( + StatsReport::kStatsReportTypeCertificate, fingerprint)); + StatsReport* report = reports_.ReplaceOrAddNew(id.Pass()); report->set_timestamp(stats_gathering_started_); report->AddValue(StatsReport::kStatsValueNameFingerprint, fingerprint); report->AddValue(StatsReport::kStatsValueNameFingerprintAlgorithm, @@ -643,11 +584,13 @@ std::string StatsCollector::AddOneCertificateReport( report->AddValue(StatsReport::kStatsValueNameDer, der_base64); if (!issuer_id.empty()) report->AddValue(StatsReport::kStatsValueNameIssuerId, issuer_id); + // TODO(tommi): Can we avoid this? return report->id().ToString(); } std::string StatsCollector::AddCertificateReports( const rtc::SSLCertificate* cert) { + ASSERT(session_->signaling_thread()->IsCurrent()); // Produces a chain of StatsReports representing this certificate and the rest // of its chain, and adds those reports to |reports_|. The return value is // the id of the leaf report. The provided cert must be non-null, so at least @@ -673,20 +616,17 @@ std::string StatsCollector::AddCertificateReports( std::string StatsCollector::AddCandidateReport( const cricket::Candidate& candidate, - const std::string& report_type) { - std::ostringstream ost; - ost << "Cand-" << candidate.id(); - StatsReport* report = reports_.Find(ost.str()); + bool local) { + scoped_ptr id( + StatsReport::NewCandidateId(local, candidate.id())); + StatsReport* report = reports_.Find(*id.get()); if (!report) { - report = reports_.InsertNew(ost.str()); - DCHECK(StatsReport::kStatsReportTypeIceLocalCandidate == report_type || - StatsReport::kStatsReportTypeIceRemoteCandidate == report_type); - report->type = report_type; - if (report_type == StatsReport::kStatsReportTypeIceLocalCandidate) { + report = reports_.InsertNew(id.Pass()); + report->set_timestamp(stats_gathering_started_); + if (local) { report->AddValue(StatsReport::kStatsValueNameCandidateNetworkType, AdapterTypeToStatsType(candidate.network_type())); } - report->set_timestamp(stats_gathering_started_); report->AddValue(StatsReport::kStatsValueNameCandidateIPAddress, candidate.address().ipaddr().ToString()); report->AddValue(StatsReport::kStatsValueNameCandidatePortNumber, @@ -699,15 +639,17 @@ std::string StatsCollector::AddCandidateReport( candidate.protocol()); } - return ost.str(); + // TODO(tommi): Necessary? + return report->id().ToString(); } void StatsCollector::ExtractSessionInfo() { ASSERT(session_->signaling_thread()->IsCurrent()); // Extract information from the base session. - StatsReport* report = reports_.ReplaceOrAddNew( - StatsId(StatsReport::kStatsReportTypeSession, session_->id())); - report->type = StatsReport::kStatsReportTypeSession; + rtc::scoped_ptr id( + StatsReport::NewTypedId( + StatsReport::kStatsReportTypeSession, session_->id())); + StatsReport* report = reports_.ReplaceOrAddNew(id.Pass()); report->set_timestamp(stats_gathering_started_); report->ResetValues(); report->AddBoolean(StatsReport::kStatsValueNameInitiator, @@ -749,11 +691,10 @@ void StatsCollector::ExtractSessionInfo() { = transport_iter->second.channel_stats.begin(); channel_iter != transport_iter->second.channel_stats.end(); ++channel_iter) { - std::ostringstream ostc; - ostc << "Channel-" << transport_iter->second.content_name - << "-" << channel_iter->component; - StatsReport* channel_report = reports_.ReplaceOrAddNew(ostc.str()); - channel_report->type = StatsReport::kStatsReportTypeComponent; + rtc::scoped_ptr id( + StatsReport::NewComponentId(transport_iter->second.content_name, + channel_iter->component)); + StatsReport* channel_report = reports_.ReplaceOrAddNew(id.Pass()); channel_report->set_timestamp(stats_gathering_started_); channel_report->AddValue(StatsReport::kStatsValueNameComponent, channel_iter->component); @@ -770,13 +711,13 @@ void StatsCollector::ExtractSessionInfo() { for (size_t i = 0; i < channel_iter->connection_infos.size(); ++i) { - std::ostringstream ost; - ost << "Conn-" << transport_iter->first << "-" - << channel_iter->component << "-" << i; - StatsReport* report = reports_.ReplaceOrAddNew(ost.str()); - report->type = StatsReport::kStatsReportTypeCandidatePair; + rtc::scoped_ptr id( + StatsReport::NewCandidatePairId(transport_iter->first, + channel_iter->component, static_cast(i))); + StatsReport* report = reports_.ReplaceOrAddNew(id.Pass()); report->set_timestamp(stats_gathering_started_); // Link from connection to its containing channel. + // TODO(tommi): Any way to avoid ToString here? report->AddValue(StatsReport::kStatsValueNameChannelId, channel_report->id().ToString()); @@ -797,14 +738,10 @@ void StatsCollector::ExtractSessionInfo() { report->AddBoolean(StatsReport::kStatsValueNameActiveConnection, info.best_connection); report->AddValue(StatsReport::kStatsValueNameLocalCandidateId, - AddCandidateReport( - info.local_candidate, - StatsReport::kStatsReportTypeIceLocalCandidate)); + AddCandidateReport(info.local_candidate, true)); report->AddValue( StatsReport::kStatsValueNameRemoteCandidateId, - AddCandidateReport( - info.remote_candidate, - StatsReport::kStatsReportTypeIceRemoteCandidate)); + AddCandidateReport(info.remote_candidate, false)); report->AddValue(StatsReport::kStatsValueNameLocalAddress, info.local_candidate.address().ToString()); report->AddValue(StatsReport::kStatsValueNameRemoteAddress, @@ -841,8 +778,10 @@ void StatsCollector::ExtractVoiceInfo() { << session_->voice_channel()->content_name(); return; } - ExtractStatsFromList(voice_info.receivers, transport_id, this, kReceiving); - ExtractStatsFromList(voice_info.senders, transport_id, this, kSending); + ExtractStatsFromList(voice_info.receivers, transport_id, this, + StatsReport::kReceive); + ExtractStatsFromList(voice_info.senders, transport_id, this, + StatsReport::kSend); UpdateStatsFromExistingLocalAudioTracks(); } @@ -871,13 +810,16 @@ void StatsCollector::ExtractVideoInfo( << session_->video_channel()->content_name(); return; } - ExtractStatsFromList(video_info.receivers, transport_id, this, kReceiving); - ExtractStatsFromList(video_info.senders, transport_id, this, kSending); + ExtractStatsFromList(video_info.receivers, transport_id, this, + StatsReport::kReceive); + ExtractStatsFromList(video_info.senders, transport_id, this, + StatsReport::kSend); if (video_info.bw_estimations.size() != 1) { LOG(LS_ERROR) << "BWEs count: " << video_info.bw_estimations.size(); } else { - StatsReport* report = - reports_.FindOrAddNew(StatsReport::kStatsReportVideoBweId); + rtc::scoped_ptr report_id( + StatsReport::NewBandwidthEstimationId()); + StatsReport* report = reports_.FindOrAddNew(report_id.Pass()); ExtractStats( video_info.bw_estimations[0], stats_gathering_started_, level, report); } @@ -888,9 +830,10 @@ void StatsCollector::ExtractDataInfo() { for (const auto& dc : session_->mediastream_signaling()->sctp_data_channels()) { - StatsReport* report = reports_.ReplaceOrAddNew( - StatsId(StatsReport::kStatsReportTypeDataChannel, dc->label())); - report->type = StatsReport::kStatsReportTypeDataChannel; + rtc::scoped_ptr id( + StatsReport::NewTypedId(StatsReport::kStatsReportTypeDataChannel, + dc->label())); + StatsReport* report = reports_.ReplaceOrAddNew(id.Pass()); report->AddValue(StatsReport::kStatsValueNameLabel, dc->label()); report->AddValue(StatsReport::kStatsValueNameDataChannelId, dc->id()); report->AddValue(StatsReport::kStatsValueNameProtocol, dc->protocol()); @@ -899,26 +842,27 @@ void StatsCollector::ExtractDataInfo() { } } -StatsReport* StatsCollector::GetReport(const std::string& type, +StatsReport* StatsCollector::GetReport(const StatsReport::StatsType& type, const std::string& id, - TrackDirection direction) { + StatsReport::Direction direction) { ASSERT(session_->signaling_thread()->IsCurrent()); ASSERT(type == StatsReport::kStatsReportTypeSsrc || type == StatsReport::kStatsReportTypeRemoteSsrc); - return reports_.Find(StatsId(type, id, direction)); + return reports_.Find(StatsReport::NewIdWithDirection(type, id, direction)); } -StatsReport* StatsCollector::GetOrCreateReport(const std::string& type, - const std::string& id, - TrackDirection direction) { +StatsReport* StatsCollector::GetOrCreateReport( + const StatsReport::StatsType& type, + const std::string& id, + StatsReport::Direction direction) { ASSERT(session_->signaling_thread()->IsCurrent()); ASSERT(type == StatsReport::kStatsReportTypeSsrc || type == StatsReport::kStatsReportTypeRemoteSsrc); StatsReport* report = GetReport(type, id, direction); if (report == NULL) { - std::string statsid = StatsId(type, id, direction); - report = reports_.FindOrAddNew(statsid); - report->type = type; + rtc::scoped_ptr report_id( + StatsReport::NewIdWithDirection(type, id, direction)); + report = reports_.InsertNew(report_id.Pass()); } return report; @@ -934,7 +878,7 @@ void StatsCollector::UpdateStatsFromExistingLocalAudioTracks() { std::string ssrc_id = rtc::ToString(ssrc); StatsReport* report = GetReport(StatsReport::kStatsReportTypeSsrc, ssrc_id, - kSending); + StatsReport::kSend); if (report == NULL) { // This can happen if a local audio track is added to a stream on the // fly and the report has not been set up yet. Do nothing in this case. @@ -943,13 +887,10 @@ void StatsCollector::UpdateStatsFromExistingLocalAudioTracks() { } // The same ssrc can be used by both local and remote audio tracks. - std::string track_id; - if (!ExtractValueFromReport(*report, - StatsReport::kStatsValueNameTrackId, - &track_id) || - track_id != track->id()) { + const StatsReport::Value* v = + report->FindValue(StatsReport::kStatsValueNameTrackId); + if (!v || v->value != track->id()) continue; - } UpdateReportFromAudioTrack(track, report); } @@ -991,16 +932,16 @@ void StatsCollector::UpdateReportFromAudioTrack(AudioTrackInterface* track, } bool StatsCollector::GetTrackIdBySsrc(uint32 ssrc, std::string* track_id, - TrackDirection direction) { + StatsReport::Direction direction) { ASSERT(session_->signaling_thread()->IsCurrent()); - if (direction == kSending) { + if (direction == StatsReport::kSend) { if (!session_->GetLocalTrackIdBySsrc(ssrc, track_id)) { LOG(LS_WARNING) << "The SSRC " << ssrc << " is not associated with a sending track"; return false; } } else { - ASSERT(direction == kReceiving); + ASSERT(direction == StatsReport::kReceive); if (!session_->GetRemoteTrackIdBySsrc(ssrc, track_id)) { LOG(LS_WARNING) << "The SSRC " << ssrc << " is not associated with a receiving track"; diff --git a/talk/app/webrtc/statscollector.h b/talk/app/webrtc/statscollector.h index 1140d925e..499da44da 100644 --- a/talk/app/webrtc/statscollector.h +++ b/talk/app/webrtc/statscollector.h @@ -54,14 +54,9 @@ const char* AdapterTypeToStatsType(rtc::AdapterType type); class StatsCollector { public: - enum TrackDirection { - kSending = 0, - kReceiving, - }; - // The caller is responsible for ensuring that the session outlives the // StatsCollector instance. - StatsCollector(WebRtcSession* session); + explicit StatsCollector(WebRtcSession* session); virtual ~StatsCollector(); // Adds a MediaStream with tracks that can be used as a |selector| in a call @@ -89,13 +84,10 @@ class StatsCollector { void GetStats(MediaStreamTrackInterface* track, StatsReports* reports); - // Prepare an SSRC report for the given ssrc. Used internally + // Prepare a local or remote SSRC report for the given ssrc. Used internally // in the ExtractStatsFromList template. - StatsReport* PrepareLocalReport(uint32 ssrc, const std::string& transport, - TrackDirection direction); - // Prepare an SSRC report for the given remote ssrc. Used internally. - StatsReport* PrepareRemoteReport(uint32 ssrc, const std::string& transport, - TrackDirection direction); + StatsReport* PrepareReport(bool local, uint32 ssrc, + const std::string& transport_id, StatsReport::Direction direction); // Method used by the unittest to force a update of stats since UpdateStats() // that occur less than kMinGatherStatsPeriod number of ms apart will be @@ -114,7 +106,7 @@ class StatsCollector { // Helper method for creating IceCandidate report. |is_local| indicates // whether this candidate is local or remote. std::string AddCandidateReport(const cricket::Candidate& candidate, - const std::string& report_type); + bool local); // Adds a report for this certificate and every certificate in its chain, and // returns the leaf certificate's report's ID. @@ -125,12 +117,12 @@ class StatsCollector { void ExtractVoiceInfo(); void ExtractVideoInfo(PeerConnectionInterface::StatsOutputLevel level); void BuildSsrcToTransportId(); - webrtc::StatsReport* GetOrCreateReport(const std::string& type, + webrtc::StatsReport* GetOrCreateReport(const StatsReport::StatsType& type, const std::string& id, - TrackDirection direction); - webrtc::StatsReport* GetReport(const std::string& type, + StatsReport::Direction direction); + webrtc::StatsReport* GetReport(const StatsReport::StatsType& type, const std::string& id, - TrackDirection direction); + StatsReport::Direction direction); // Helper method to get stats from the local audio tracks. void UpdateStatsFromExistingLocalAudioTracks(); @@ -140,10 +132,10 @@ class StatsCollector { // Helper method to get the id for the track identified by ssrc. // |direction| tells if the track is for sending or receiving. bool GetTrackIdBySsrc(uint32 ssrc, std::string* track_id, - TrackDirection direction); + StatsReport::Direction direction); - // A map from the report id to the report. - StatsSet reports_; + // A collection for all of our stats reports. + StatsCollection reports_; // 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 b70fe7fea..b8c983b84 100644 --- a/talk/app/webrtc/statscollector_unittest.cc +++ b/talk/app/webrtc/statscollector_unittest.cc @@ -48,6 +48,7 @@ #include "webrtc/p2p/base/fakesession.h" using cricket::StatsOptions; +using rtc::scoped_ptr; using testing::_; using testing::DoAll; using testing::Field; @@ -69,7 +70,6 @@ namespace webrtc { // Error return values const char kNotFound[] = "NOT FOUND"; -const char kNoReports[] = "NO REPORTS"; // Constant names for track identification. const char kLocalTrackId[] = "local_track_id"; @@ -152,6 +152,7 @@ class FakeAudioTrack rtc::scoped_refptr processor_; }; +// TODO(tommi): Use FindValue(). bool GetValue(const StatsReport* report, StatsReport::StatsValueName name, std::string* value) { @@ -164,52 +165,69 @@ bool GetValue(const StatsReport* report, return false; } -std::string ExtractStatsValue(const std::string& type, +std::string ExtractStatsValue(const StatsReport::StatsType& type, const StatsReports& reports, StatsReport::StatsValueName name) { - if (reports.empty()) { - return kNoReports; - } - for (size_t i = 0; i < reports.size(); ++i) { - if (reports[i]->type != type) - continue; + for (const auto* r : reports) { std::string ret; - if (GetValue(reports[i], name, &ret)) { + if (r->type() == type && GetValue(r, name, &ret)) return ret; - } } return kNotFound; } +scoped_ptr TypedIdFromIdString(StatsReport::StatsType type, + const std::string& value) { + EXPECT_FALSE(value.empty()); + scoped_ptr id; + if (value.empty()) + return id.Pass(); + + // This has assumptions about how the ID is constructed. As is, this is + // OK since this is for testing purposes only, but if we ever need this + // in production, we should add a generic method that does this. + size_t index = value.find('_'); + EXPECT_NE(index, std::string::npos); + if (index == std::string::npos || index == (value.length() - 1)) + return id.Pass(); + + id = StatsReport::NewTypedId(type, value.substr(index + 1)); + EXPECT_EQ(id->ToString(), value); + return id.Pass(); +} + +scoped_ptr IdFromCertIdString(const std::string& cert_id) { + return TypedIdFromIdString(StatsReport::kStatsReportTypeCertificate, cert_id) + .Pass(); +} + // Finds the |n|-th report of type |type| in |reports|. // |n| starts from 1 for finding the first report. const StatsReport* FindNthReportByType( - const StatsReports& reports, const std::string& type, int n) { + const StatsReports& reports, const StatsReport::StatsType& type, int n) { for (size_t i = 0; i < reports.size(); ++i) { - if (reports[i]->type == type) { + if (reports[i]->type() == type) { n--; if (n == 0) return reports[i]; } } - return NULL; + return nullptr; } const StatsReport* FindReportById(const StatsReports& reports, - const std::string& id) { + const StatsReport::Id& id) { for (const auto* r : reports) { - if (r->id().ToString() == id) { + if (r->id().Equals(id)) return r; - } } return nullptr; } std::string ExtractSsrcStatsValue(StatsReports reports, StatsReport::StatsValueName name) { - return ExtractStatsValue( - StatsReport::kStatsReportTypeSsrc, reports, name); + return ExtractStatsValue(StatsReport::kStatsReportTypeSsrc, reports, name); } std::string ExtractBweStatsValue(StatsReports reports, @@ -234,18 +252,18 @@ std::vector DersToPems( void CheckCertChainReports(const StatsReports& reports, const std::vector& ders, - const std::string& start_id) { - std::string certificate_id = start_id; + const StatsReport::Id& start_id) { + scoped_ptr cert_id; + const StatsReport::Id* certificate_id = &start_id; size_t i = 0; while (true) { - const StatsReport* report = FindReportById(reports, certificate_id); + const StatsReport* report = FindReportById(reports, *certificate_id); ASSERT_TRUE(report != NULL); std::string der_base64; EXPECT_TRUE(GetValue( report, StatsReport::kStatsValueNameDer, &der_base64)); - std::string der = rtc::Base64::Decode(der_base64, - rtc::Base64::DO_STRICT); + std::string der = rtc::Base64::Decode(der_base64, rtc::Base64::DO_STRICT); EXPECT_EQ(ders[i], der); std::string fingerprint_algorithm; @@ -257,16 +275,20 @@ void CheckCertChainReports(const StatsReports& reports, std::string sha_1_str = rtc::DIGEST_SHA_1; EXPECT_EQ(sha_1_str, fingerprint_algorithm); - std::string dummy_fingerprint; // Value is not checked. - EXPECT_TRUE(GetValue( - report, - StatsReport::kStatsValueNameFingerprint, - &dummy_fingerprint)); + std::string fingerprint; + EXPECT_TRUE(GetValue(report, StatsReport::kStatsValueNameFingerprint, + &fingerprint)); + EXPECT_FALSE(fingerprint.empty()); ++i; - if (!GetValue( - report, StatsReport::kStatsValueNameIssuerId, &certificate_id)) + std::string issuer_id; + if (!GetValue(report, StatsReport::kStatsValueNameIssuerId, + &issuer_id)) { break; + } + + cert_id = IdFromCertIdString(issuer_id).Pass(); + certificate_id = cert_id.get(); } EXPECT_EQ(ders.size(), i); } @@ -510,8 +532,8 @@ class StatsCollectorTest : public testing::Test { std::string AddCandidateReport(StatsCollector* collector, const cricket::Candidate& candidate, - const std::string& report_type) { - return collector->AddCandidateReport(candidate, report_type); + bool local) { + return collector->AddCandidateReport(candidate, local); } void SetupAndVerifyAudioTrackStats( @@ -651,7 +673,8 @@ class StatsCollectorTest : public testing::Test { StatsReport::kStatsValueNameLocalCertificateId); if (local_ders.size() > 0) { EXPECT_NE(kNotFound, local_certificate_id); - CheckCertChainReports(reports, local_ders, local_certificate_id); + scoped_ptr id(IdFromCertIdString(local_certificate_id)); + CheckCertChainReports(reports, local_ders, *id.get()); } else { EXPECT_EQ(kNotFound, local_certificate_id); } @@ -663,7 +686,8 @@ class StatsCollectorTest : public testing::Test { StatsReport::kStatsValueNameRemoteCertificateId); if (remote_ders.size() > 0) { EXPECT_NE(kNotFound, remote_certificate_id); - CheckCertChainReports(reports, remote_ders, remote_certificate_id); + scoped_ptr id(IdFromCertIdString(remote_certificate_id)); + CheckCertChainReports(reports, remote_ders, *id.get()); } else { EXPECT_EQ(kNotFound, remote_certificate_id); } @@ -842,8 +866,7 @@ TEST_F(StatsCollectorTest, TrackObjectExistsWithoutUpdateStats) { StatsReports reports; stats.GetStats(NULL, &reports); EXPECT_EQ((size_t)1, reports.size()); - EXPECT_EQ(std::string(StatsReport::kStatsReportTypeTrack), - reports[0]->type); + EXPECT_EQ(StatsReport::kStatsReportTypeTrack, reports[0]->type()); std::string trackValue = ExtractStatsValue(StatsReport::kStatsReportTypeTrack, @@ -953,8 +976,19 @@ TEST_F(StatsCollectorTest, TransportObjectLinkedFromSsrcObject) { reports, StatsReport::kStatsValueNameTransportId); ASSERT_NE(kNotFound, transport_id); - const StatsReport* transport_report = FindReportById(reports, - transport_id); + // Transport id component ID will always be 1. + // This has assumptions about how the ID is constructed. As is, this is + // OK since this is for testing purposes only, but if we ever need this + // in production, we should add a generic method that does this. + size_t index = transport_id.find('-'); + ASSERT_NE(std::string::npos, index); + std::string content = transport_id.substr(index + 1); + index = content.rfind('-'); + ASSERT_NE(std::string::npos, index); + content = content.substr(0, index); + scoped_ptr id(StatsReport::NewComponentId(content, 1)); + ASSERT_EQ(transport_id, id->ToString()); + const StatsReport* transport_report = FindReportById(reports, *id.get()); ASSERT_FALSE(transport_report == NULL); } @@ -1101,8 +1135,7 @@ TEST_F(StatsCollectorTest, IceCandidateReport) { c.set_address(local_address); c.set_priority(priority); c.set_network_type(network_type); - std::string report_id = AddCandidateReport( - &stats, c, StatsReport::kStatsReportTypeIceLocalCandidate); + std::string report_id = AddCandidateReport(&stats, c, true); EXPECT_EQ("Cand-" + c.id(), report_id); c = cricket::Candidate(); @@ -1112,8 +1145,7 @@ TEST_F(StatsCollectorTest, IceCandidateReport) { c.set_address(remote_address); c.set_priority(priority); c.set_network_type(network_type); - report_id = AddCandidateReport( - &stats, c, StatsReport::kStatsReportTypeIceRemoteCandidate); + report_id = AddCandidateReport(&stats, c, false); EXPECT_EQ("Cand-" + c.id(), report_id); stats.GetStats(NULL, &reports); diff --git a/talk/app/webrtc/statstypes.cc b/talk/app/webrtc/statstypes.cc index 6ce2e4535..a9f30e592 100644 --- a/talk/app/webrtc/statstypes.cc +++ b/talk/app/webrtc/statstypes.cc @@ -30,89 +30,179 @@ using rtc::scoped_ptr; namespace webrtc { +namespace { -const char StatsReport::kStatsReportTypeSession[] = "googLibjingleSession"; -const char StatsReport::kStatsReportTypeBwe[] = "VideoBwe"; -const char StatsReport::kStatsReportTypeRemoteSsrc[] = "remoteSsrc"; -const char StatsReport::kStatsReportTypeSsrc[] = "ssrc"; -const char StatsReport::kStatsReportTypeTrack[] = "googTrack"; -const char StatsReport::kStatsReportTypeIceLocalCandidate[] = "localcandidate"; -const char StatsReport::kStatsReportTypeIceRemoteCandidate[] = - "remotecandidate"; -const char StatsReport::kStatsReportTypeTransport[] = "googTransport"; -const char StatsReport::kStatsReportTypeComponent[] = "googComponent"; -const char StatsReport::kStatsReportTypeCandidatePair[] = "googCandidatePair"; -const char StatsReport::kStatsReportTypeCertificate[] = "googCertificate"; -const char StatsReport::kStatsReportTypeDataChannel[] = "datachannel"; +// The id of StatsReport of type kStatsReportTypeBwe. +const char kStatsReportVideoBweId[] = "bweforvideo"; -const char StatsReport::kStatsReportVideoBweId[] = "bweforvideo"; - -StatsReport::StatsReport(const StatsReport& src) - : id_(src.id_), - type(src.type), - timestamp_(src.timestamp_), - values_(src.values_) { +// NOTE: These names need to be consistent with an external +// specification (W3C Stats Identifiers). +const char* InternalTypeToString(StatsReport::StatsType type) { + switch (type) { + case StatsReport::kStatsReportTypeSession: + return "googLibjingleSession"; + case StatsReport::kStatsReportTypeBwe: + return "VideoBwe"; + case StatsReport::kStatsReportTypeRemoteSsrc: + return "remoteSsrc"; + case StatsReport::kStatsReportTypeSsrc: + return "ssrc"; + case StatsReport::kStatsReportTypeTrack: + return "googTrack"; + case StatsReport::kStatsReportTypeIceLocalCandidate: + return "localcandidate"; + case StatsReport::kStatsReportTypeIceRemoteCandidate: + return "remotecandidate"; + case StatsReport::kStatsReportTypeTransport: + return "googTransport"; + case StatsReport::kStatsReportTypeComponent: + return "googComponent"; + case StatsReport::kStatsReportTypeCandidatePair: + return "googCandidatePair"; + case StatsReport::kStatsReportTypeCertificate: + return "googCertificate"; + case StatsReport::kStatsReportTypeDataChannel: + return "datachannel"; + } + ASSERT(false); + return nullptr; } -StatsReport::StatsReport(const std::string& id) - : id_(id), timestamp_(0) { -} +class BandwidthEstimationId : public StatsReport::Id { + public: + BandwidthEstimationId() : StatsReport::Id(StatsReport::kStatsReportTypeBwe) {} + std::string ToString() const override { return kStatsReportVideoBweId; } +}; -StatsReport::StatsReport(scoped_ptr id) - : id_(id->ToString()), timestamp_(0) { -} +class TypedId : public StatsReport::Id { + public: + static const char kSeparator = '_'; + TypedId(StatsReport::StatsType type, const std::string& id) + : StatsReport::Id(type), id_(id) {} -// static -scoped_ptr StatsReport::NewTypedId( - StatsReport::StatsType type, const std::string& id) { - std::string internal_id(type); - internal_id += '_'; - internal_id += id; - return scoped_ptr(new Id(internal_id)).Pass(); -} + bool Equals(const Id& other) const override { + return Id::Equals(other) && + static_cast(other).id_ == id_; + } -StatsReport& StatsReport::operator=(const StatsReport& src) { - ASSERT(id_ == src.id_); - type = src.type; - timestamp_ = src.timestamp_; - values_ = src.values_; - return *this; -} + std::string ToString() const override { + return std::string(InternalTypeToString(type_)) + kSeparator + id_; + } -// Operators provided for STL container/algorithm support. -bool StatsReport::operator<(const StatsReport& other) const { - return id_ < other.id_; -} + protected: + const std::string id_; +}; -bool StatsReport::operator==(const StatsReport& other) const { - return id_ == other.id_; -} +class IdWithDirection : public TypedId { + public: + IdWithDirection(StatsReport::StatsType type, const std::string& id, + StatsReport::Direction direction) + : TypedId(type, id), direction_(direction) {} -// Special support for being able to use std::find on a container -// without requiring a new StatsReport instance. -bool StatsReport::operator==(const std::string& other_id) const { - return id_ == other_id; -} + bool Equals(const Id& other) const override { + return TypedId::Equals(other) && + static_cast(other).direction_ == direction_; + } -// The copy ctor can't be declared as explicit due to problems with STL. -StatsReport::Value::Value(const Value& other) - : name(other.name), value(other.value) { -} + std::string ToString() const override { + std::string ret(TypedId::ToString()); + ret += '_'; + ret += direction_ == StatsReport::kSend ? "send" : "recv"; + return ret; + } -StatsReport::Value::Value(StatsValueName name) - : name(name) { + private: + const StatsReport::Direction direction_; +}; + +class CandidateId : public TypedId { + public: + CandidateId(bool local, const std::string& id) + : TypedId(local ? + StatsReport::kStatsReportTypeIceLocalCandidate : + StatsReport::kStatsReportTypeIceRemoteCandidate, + id) { + } + + std::string ToString() const override { + return "Cand-" + id_; + } +}; + +class ComponentId : public StatsReport::Id { + public: + ComponentId(const std::string& content_name, int component) + : ComponentId(StatsReport::kStatsReportTypeComponent, content_name, + component) {} + + bool Equals(const Id& other) const override { + return Id::Equals(other) && + static_cast(other).component_ == component_ && + static_cast(other).content_name_ == content_name_; + } + + std::string ToString() const override { + return ToString("Channel-"); + } + + protected: + ComponentId(StatsReport::StatsType type, const std::string& content_name, + int component) + : Id(type), + content_name_(content_name), + component_(component) {} + + std::string ToString(const char* prefix) const { + std::string ret(prefix); + ret += content_name_; + ret += '-'; + ret += rtc::ToString<>(component_); + return ret; + } + + private: + const std::string content_name_; + const int component_; +}; + +class CandidatePairId : public ComponentId { + public: + CandidatePairId(const std::string& content_name, int component, int index) + : ComponentId(StatsReport::kStatsReportTypeCandidatePair, content_name, + component), + index_(index) {} + + bool Equals(const Id& other) const override { + return ComponentId::Equals(other) && + static_cast(other).index_ == index_; + } + + std::string ToString() const override { + std::string ret(ComponentId::ToString("Conn-")); + ret += '-'; + ret += rtc::ToString<>(index_); + return ret; + } + + private: + const int index_; +}; + +} // namespace + +StatsReport::Id::Id(StatsType type) : type_(type) {} +StatsReport::Id::~Id() {} + +StatsReport::StatsType StatsReport::Id::type() const { return type_; } + +bool StatsReport::Id::Equals(const Id& other) const { + return other.type_ == type_; } StatsReport::Value::Value(StatsValueName name, const std::string& value) : name(name), value(value) { } -StatsReport::Value& StatsReport::Value::operator=(const Value& other) { - const_cast(name) = other.name; - value = other.value; - return *this; -} - const char* StatsReport::Value::display_name() const { switch (name) { case kStatsValueNameAudioOutputLevel: @@ -331,6 +421,50 @@ const char* StatsReport::Value::display_name() const { return nullptr; } +StatsReport::StatsReport(scoped_ptr id) : id_(id.Pass()), timestamp_(0.0) { + ASSERT(id_.get()); +} + +// static +scoped_ptr StatsReport::NewBandwidthEstimationId() { + return scoped_ptr(new BandwidthEstimationId()).Pass(); +} + +// static +scoped_ptr StatsReport::NewTypedId( + StatsType type, const std::string& id) { + return scoped_ptr(new TypedId(type, id)).Pass(); +} + +// static +scoped_ptr StatsReport::NewIdWithDirection( + StatsType type, const std::string& id, StatsReport::Direction direction) { + return scoped_ptr(new IdWithDirection(type, id, direction)).Pass(); +} + +// static +scoped_ptr StatsReport::NewCandidateId( + bool local, const std::string& id) { + return scoped_ptr(new CandidateId(local, id)).Pass(); +} + +// static +scoped_ptr StatsReport::NewComponentId( + const std::string& content_name, int component) { + return scoped_ptr(new ComponentId(content_name, component)).Pass(); +} + +// static +scoped_ptr StatsReport::NewCandidatePairId( + const std::string& content_name, int component, int index) { + return scoped_ptr(new CandidatePairId(content_name, component, index)) + .Pass(); +} + +const char* StatsReport::TypeToString() const { + return InternalTypeToString(id_->type()); +} + void StatsReport::AddValue(StatsReport::StatsValueName name, const std::string& value) { values_.push_back(ValuePtr(new Value(name, value))); @@ -340,6 +474,7 @@ void StatsReport::AddValue(StatsReport::StatsValueName name, int64 value) { AddValue(name, rtc::ToString(value)); } +// TODO(tommi): Change the way we store vector values. template void StatsReport::AddValue(StatsReport::StatsValueName name, const std::vector& value) { @@ -370,6 +505,7 @@ void StatsReport::AddValue( StatsReport::StatsValueName, const std::vector&); void StatsReport::AddBoolean(StatsReport::StatsValueName name, bool value) { + // TODO(tommi): Store bools as bool. AddValue(name, value ? "true" : "false"); } @@ -392,42 +528,67 @@ void StatsReport::ResetValues() { values_.clear(); } -StatsSet::StatsSet() { +const StatsReport::Value* StatsReport::FindValue(StatsValueName name) const { + Values::const_iterator it = std::find_if(values_.begin(), values_.end(), + [&name](const ValuePtr& v)->bool { return v->name == name; }); + return it == values_.end() ? nullptr : (*it).get(); } -StatsSet::~StatsSet() { +StatsCollection::StatsCollection() { } -StatsSet::const_iterator StatsSet::begin() const { +StatsCollection::~StatsCollection() { + for (auto* r : list_) + delete r; +} + +StatsCollection::const_iterator StatsCollection::begin() const { return list_.begin(); } -StatsSet::const_iterator StatsSet::end() const { +StatsCollection::const_iterator StatsCollection::end() const { return list_.end(); } -StatsReport* StatsSet::InsertNew(const std::string& id) { - ASSERT(Find(id) == NULL); - const StatsReport* ret = &(*list_.insert(StatsReportCopyable(id)).first); - return const_cast(ret); +size_t StatsCollection::size() const { + return list_.size(); } -StatsReport* StatsSet::FindOrAddNew(const std::string& id) { - StatsReport* ret = Find(id); - return ret ? ret : InsertNew(id); +StatsReport* StatsCollection::InsertNew(scoped_ptr id) { + ASSERT(Find(*id.get()) == NULL); + StatsReport* report = new StatsReport(id.Pass()); + list_.push_back(report); + return report; } -StatsReport* StatsSet::ReplaceOrAddNew(const std::string& id) { - list_.erase(id); - return InsertNew(id); +StatsReport* StatsCollection::FindOrAddNew(scoped_ptr id) { + StatsReport* ret = Find(*id.get()); + return ret ? ret : InsertNew(id.Pass()); +} + +StatsReport* StatsCollection::ReplaceOrAddNew(scoped_ptr id) { + ASSERT(id.get()); + Container::iterator it = std::find_if(list_.begin(), list_.end(), + [&id](const StatsReport* r)->bool { return r->id().Equals(*id.get()); }); + if (it != end()) { + delete *it; + StatsReport* report = new StatsReport(id.Pass()); + *it = report; + return report; + } + return InsertNew(id.Pass()); } // Looks for a report with the given |id|. If one is not found, NULL // will be returned. -StatsReport* StatsSet::Find(const std::string& id) { - const_iterator it = std::find(begin(), end(), id); - return it == end() ? NULL : - const_cast(static_cast(&(*it))); +StatsReport* StatsCollection::Find(const StatsReport::Id& id) { + Container::iterator it = std::find_if(list_.begin(), list_.end(), + [&id](const StatsReport* r)->bool { return r->id().Equals(id); }); + return it == list_.end() ? nullptr : *it; +} + +StatsReport* StatsCollection::Find(const scoped_ptr& id) { + return Find(*id.get()); } } // namespace webrtc diff --git a/talk/app/webrtc/statstypes.h b/talk/app/webrtc/statstypes.h index 2bc0f33c1..8f132dbdf 100644 --- a/talk/app/webrtc/statstypes.h +++ b/talk/app/webrtc/statstypes.h @@ -32,11 +32,13 @@ #define TALK_APP_WEBRTC_STATSTYPES_H_ #include -#include +#include #include #include #include "webrtc/base/basictypes.h" +#include "webrtc/base/linked_ptr.h" +#include "webrtc/base/scoped_ptr.h" #include "webrtc/base/common.h" #include "webrtc/base/linked_ptr.h" #include "webrtc/base/scoped_ptr.h" @@ -46,35 +48,71 @@ namespace webrtc { class StatsReport { public: - // TODO(tommi): Remove this ctor after removing reliance upon it in Chromium - // (mock_peer_connection_impl.cc). - StatsReport() : timestamp_(0) {} + // Indicates whether a track is for sending or receiving. + // Used in reports for audio/video tracks. + enum Direction { + kSend = 0, + kReceive, + }; - // TODO(tommi): Make protected and disallow copy completely once not needed. - StatsReport(const StatsReport& src); + enum StatsType { + // StatsReport types. + // A StatsReport of |type| = "googSession" contains overall information + // about the thing libjingle calls a session (which may contain one + // or more RTP sessions. + kStatsReportTypeSession, - // Constructor is protected to force use of StatsSet. - // TODO(tommi): Make this ctor protected. - explicit StatsReport(const std::string& id); + // A StatsReport of |type| = "googTransport" contains information + // about a libjingle "transport". + kStatsReportTypeTransport, - // TODO(tommi): Make this protected. - StatsReport& operator=(const StatsReport& src); + // A StatsReport of |type| = "googComponent" contains information + // about a libjingle "channel" (typically, RTP or RTCP for a transport). + // This is intended to be the same thing as an ICE "Component". + kStatsReportTypeComponent, - // Operators provided for STL container/algorithm support. - bool operator<(const StatsReport& other) const; - bool operator==(const StatsReport& other) const; - // Special support for being able to use std::find on a container - // without requiring a new StatsReport instance. - bool operator==(const std::string& other_id) const; + // A StatsReport of |type| = "googCandidatePair" contains information + // about a libjingle "connection" - a single source/destination port pair. + // This is intended to be the same thing as an ICE "candidate pair". + kStatsReportTypeCandidatePair, - // The unique identifier for this object. - // This is used as a key for this report in ordered containers, - // so it must never be changed. - // TODO(tommi): Make this member variable const. - std::string id_; // See below for contents. - std::string type; // See below for contents. + // A StatsReport of |type| = "VideoBWE" is statistics for video Bandwidth + // Estimation, which is global per-session. The |id| field is "bweforvideo" + // (will probably change in the future). + kStatsReportTypeBwe, + + // A StatsReport of |type| = "ssrc" is statistics for a specific rtp stream. + // The |id| field is the SSRC in decimal form of the rtp stream. + kStatsReportTypeSsrc, + + // A StatsReport of |type| = "remoteSsrc" is statistics for a specific + // rtp stream, generated by the remote end of the connection. + kStatsReportTypeRemoteSsrc, + + // A StatsReport of |type| = "googTrack" is statistics for a specific media + // track. The |id| field is the track id. + kStatsReportTypeTrack, + + // A StatsReport of |type| = "localcandidate" or "remotecandidate" is + // attributes on a specific ICE Candidate. It links to its connection pair + // by candidate id. The string value is taken from + // http://w3c.github.io/webrtc-stats/#rtcstatstype-enum*. + kStatsReportTypeIceLocalCandidate, + kStatsReportTypeIceRemoteCandidate, + + // A StatsReport of |type| = "googCertificate" contains an SSL certificate + // transmitted by one of the endpoints of this connection. The |id| is + // controlled by the fingerprint, and is used to identify the certificate in + // the Channel stats (as "googLocalCertificateId" or + // "googRemoteCertificateId") and in any child certificates (as + // "googIssuerId"). + kStatsReportTypeCertificate, + + // A StatsReport of |type| = "datachannel" with statistics for a + // particular DataChannel. + kStatsReportTypeDataChannel, + }; - // StatsValue names. enum StatsValueName { kStatsValueNameActiveConnection, kStatsValueNameAudioInputLevel, @@ -183,40 +221,54 @@ class StatsReport { class Id { public: - Id(const std::string& id) : id_(id) {} - Id(const Id& id) : id_(id.id_) {} - const std::string& ToString() const { return id_; } - private: - const std::string id_; + virtual ~Id(); + StatsType type() const; + virtual bool Equals(const Id& other) const; + virtual std::string ToString() const = 0; + + protected: + Id(StatsType type); // Only meant for derived classes. + const StatsType type_; }; struct Value { - // The copy ctor can't be declared as explicit due to problems with STL. - Value(const Value& other); - explicit Value(StatsValueName name); Value(StatsValueName name, const std::string& value); - // TODO(tommi): Remove this operator once we don't need it. - // The operator is provided for compatibility with STL containers. - // The public |name| member variable is otherwise meant to be read-only. - Value& operator=(const Value& other); - // Returns the string representation of |name|. const char* display_name() const; const StatsValueName name; + // TODO(tommi): Support more value types than string. + const std::string value; - std::string value; + private: + DISALLOW_COPY_AND_ASSIGN(Value); }; typedef rtc::linked_ptr ValuePtr; typedef std::vector Values; - typedef const char* StatsType; // Ownership of |id| is passed to |this|. explicit StatsReport(rtc::scoped_ptr id); + // Factory functions for various types of stats IDs. + static rtc::scoped_ptr NewBandwidthEstimationId(); static rtc::scoped_ptr NewTypedId(StatsType type, const std::string& id); + static rtc::scoped_ptr NewIdWithDirection( + StatsType type, const std::string& id, Direction direction); + static rtc::scoped_ptr NewCandidateId(bool local, const std::string& id); + static rtc::scoped_ptr NewComponentId( + const std::string& content_name, int component); + static rtc::scoped_ptr NewCandidatePairId( + const std::string& content_name, int component, int index); + + const Id& id() const { return *id_.get(); } + StatsType type() const { return id_->type(); } + double timestamp() const { return timestamp_; } + void set_timestamp(double t) { timestamp_ = t; } + const Values& values() const { return values_; } + + const char* TypeToString() const; void AddValue(StatsValueName name, const std::string& value); void AddValue(StatsValueName name, int64 value); @@ -228,122 +280,55 @@ class StatsReport { void ResetValues(); - const Id id() const { return Id(id_); } - double timestamp() const { return timestamp_; } - void set_timestamp(double t) { timestamp_ = t; } - const Values& values() const { return values_; } - - const char* TypeToString() const { return type.c_str(); } + const Value* FindValue(StatsValueName name) const; + private: + // The unique identifier for this object. + // This is used as a key for this report in ordered containers, + // so it must never be changed. + const rtc::scoped_ptr id_; double timestamp_; // Time since 1970-01-01T00:00:00Z in milliseconds. Values values_; - // TODO(tommi): These should all be enum values. - - // StatsReport types. - // A StatsReport of |type| = "googSession" contains overall information - // about the thing libjingle calls a session (which may contain one - // or more RTP sessions. - static const char kStatsReportTypeSession[]; - - // A StatsReport of |type| = "googTransport" contains information - // about a libjingle "transport". - static const char kStatsReportTypeTransport[]; - - // A StatsReport of |type| = "googComponent" contains information - // about a libjingle "channel" (typically, RTP or RTCP for a transport). - // This is intended to be the same thing as an ICE "Component". - static const char kStatsReportTypeComponent[]; - - // A StatsReport of |type| = "googCandidatePair" contains information - // about a libjingle "connection" - a single source/destination port pair. - // This is intended to be the same thing as an ICE "candidate pair". - static const char kStatsReportTypeCandidatePair[]; - - // StatsReport of |type| = "VideoBWE" is statistics for video Bandwidth - // Estimation, which is global per-session. The |id| field is "bweforvideo" - // (will probably change in the future). - static const char kStatsReportTypeBwe[]; - - // StatsReport of |type| = "ssrc" is statistics for a specific rtp stream. - // The |id| field is the SSRC in decimal form of the rtp stream. - static const char kStatsReportTypeSsrc[]; - - // StatsReport of |type| = "remoteSsrc" is statistics for a specific - // rtp stream, generated by the remote end of the connection. - static const char kStatsReportTypeRemoteSsrc[]; - - // StatsReport of |type| = "googTrack" is statistics for a specific media - // track. The |id| field is the track id. - static const char kStatsReportTypeTrack[]; - - // StatsReport of |type| = "localcandidate" or "remotecandidate" is attributes - // on a specific ICE Candidate. It links to its connection pair by candidate - // id. The string value is taken from - // http://w3c.github.io/webrtc-stats/#rtcstatstype-enum*. - static const char kStatsReportTypeIceLocalCandidate[]; - static const char kStatsReportTypeIceRemoteCandidate[]; - - // A StatsReport of |type| = "googCertificate" contains an SSL certificate - // transmitted by one of the endpoints of this connection. The |id| is - // controlled by the fingerprint, and is used to identify the certificate in - // the Channel stats (as "googLocalCertificateId" or - // "googRemoteCertificateId") and in any child certificates (as - // "googIssuerId"). - static const char kStatsReportTypeCertificate[]; - - // The id of StatsReport of type VideoBWE. - static const char kStatsReportVideoBweId[]; - - // A StatsReport of |type| = "datachannel" with statistics for a - // particular DataChannel. - static const char kStatsReportTypeDataChannel[]; -}; - -// This class is provided for the cases where we need to keep -// snapshots of reports around. This is an edge case. -// TODO(tommi): Move into the private section of StatsSet. -class StatsReportCopyable : public StatsReport { - public: - StatsReportCopyable(const std::string& id) : StatsReport(id) {} - explicit StatsReportCopyable(const StatsReport& src) - : StatsReport(src) {} - - using StatsReport::operator=; + DISALLOW_COPY_AND_ASSIGN(StatsReport); }; // Typedef for an array of const StatsReport pointers. // Ownership of the pointers held by this implementation is assumed to lie // elsewhere and lifetime guarantees are made by the implementation that uses -// this type. In the StatsCollector, object ownership lies with the StatsSet -// class. +// this type. In the StatsCollector, object ownership lies with the +// StatsCollection class. typedef std::vector StatsReports; // A map from the report id to the report. // This class wraps an STL container and provides a limited set of // functionality in order to keep things simple. // TODO(tommi): Use a thread checker here (currently not in libjingle). -class StatsSet { +class StatsCollection { public: - StatsSet(); - ~StatsSet(); + StatsCollection(); + ~StatsCollection(); - typedef std::set Container; + // TODO(tommi): shared_ptr (or linked_ptr)? + typedef std::list Container; typedef Container::iterator iterator; typedef Container::const_iterator const_iterator; const_iterator begin() const; const_iterator end() const; + size_t size() const; // Creates a new report object with |id| that does not already // exist in the list of reports. - StatsReport* InsertNew(const std::string& id); - StatsReport* FindOrAddNew(const std::string& id); - StatsReport* ReplaceOrAddNew(const std::string& id); + StatsReport* InsertNew(rtc::scoped_ptr id); + StatsReport* FindOrAddNew(rtc::scoped_ptr id); + StatsReport* ReplaceOrAddNew(rtc::scoped_ptr id); // Looks for a report with the given |id|. If one is not found, NULL // will be returned. - StatsReport* Find(const std::string& id); + StatsReport* Find(const StatsReport::Id& id); + // TODO(tommi): we should only need one of these. + StatsReport* Find(const rtc::scoped_ptr& id); private: Container list_; diff --git a/talk/app/webrtc/test/mockpeerconnectionobservers.h b/talk/app/webrtc/test/mockpeerconnectionobservers.h index 226e439ec..606d4858a 100644 --- a/talk/app/webrtc/test/mockpeerconnectionobservers.h +++ b/talk/app/webrtc/test/mockpeerconnectionobservers.h @@ -126,7 +126,7 @@ class MockStatsObserver : public webrtc::StatsObserver { memset(&stats_, sizeof(stats_), 0); stats_.number_of_reports = reports.size(); for (const auto* r : reports) { - if (r->type == StatsReport::kStatsReportTypeSsrc) { + if (r->type() == StatsReport::kStatsReportTypeSsrc) { GetIntValue(r, StatsReport::kStatsValueNameAudioOutputLevel, &stats_.audio_output_level); GetIntValue(r, StatsReport::kStatsValueNameAudioInputLevel, @@ -135,7 +135,7 @@ class MockStatsObserver : public webrtc::StatsObserver { &stats_.bytes_received); GetIntValue(r, StatsReport::kStatsValueNameBytesSent, &stats_.bytes_sent); - } else if (r->type == StatsReport::kStatsReportTypeBwe) { + } else if (r->type() == StatsReport::kStatsReportTypeBwe) { GetIntValue(r, StatsReport::kStatsValueNameAvailableReceiveBandwidth, &stats_.available_receive_bandwidth); }