From 4cb012858f7461015e405c0c2cfc4b9f10a086ce Mon Sep 17 00:00:00 2001 From: "xians@webrtc.org" Date: Thu, 12 Jun 2014 14:57:05 +0000 Subject: [PATCH] Fixed GetStats when local and remote track are using the same ssrc. R=hta@chromium.org, kjellander@webrtc.org, tommi@webrtc.org Review URL: https://webrtc-codereview.appspot.com/20589004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@6414 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/app/webrtc/mediastream_unittest.cc | 3 +- .../app/webrtc/mediastreamhandler_unittest.cc | 3 +- talk/app/webrtc/statscollector.cc | 108 +++-- talk/app/webrtc/statscollector.h | 22 +- talk/app/webrtc/statscollector_unittest.cc | 426 ++++++++++++------ talk/app/webrtc/webrtcsession.cc | 23 +- talk/app/webrtc/webrtcsession.h | 7 +- talk/libjingle_tests.gyp | 27 +- .../valgrind-webrtc/memcheck/suppressions.txt | 14 + 9 files changed, 435 insertions(+), 198 deletions(-) diff --git a/talk/app/webrtc/mediastream_unittest.cc b/talk/app/webrtc/mediastream_unittest.cc index bb2d50e6ae..113242faf5 100644 --- a/talk/app/webrtc/mediastream_unittest.cc +++ b/talk/app/webrtc/mediastream_unittest.cc @@ -33,7 +33,8 @@ #include "talk/base/refcount.h" #include "talk/base/scoped_ptr.h" #include "talk/base/gunit.h" -#include "testing/base/public/gmock.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" static const char kStreamLabel1[] = "local_stream_1"; static const char kVideoTrackId[] = "dummy_video_cam_1"; diff --git a/talk/app/webrtc/mediastreamhandler_unittest.cc b/talk/app/webrtc/mediastreamhandler_unittest.cc index 6eedb7e89e..9a53f35561 100644 --- a/talk/app/webrtc/mediastreamhandler_unittest.cc +++ b/talk/app/webrtc/mediastreamhandler_unittest.cc @@ -38,7 +38,8 @@ #include "talk/base/gunit.h" #include "talk/media/base/fakevideocapturer.h" #include "talk/media/base/mediachannel.h" -#include "testing/base/public/gmock.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" using ::testing::_; using ::testing::Exactly; diff --git a/talk/app/webrtc/statscollector.cc b/talk/app/webrtc/statscollector.cc index bc8b4f8a3c..149ad5c315 100644 --- a/talk/app/webrtc/statscollector.cc +++ b/talk/app/webrtc/statscollector.cc @@ -187,7 +187,6 @@ const char StatsReport::kStatsReportTypeCertificate[] = "googCertificate"; const char StatsReport::kStatsReportVideoBweId[] = "bweforvideo"; - // Implementations of functions in statstypes.h void StatsReport::AddValue(const std::string& name, const std::string& value) { Value temp; @@ -238,6 +237,20 @@ 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, const std::string& name, @@ -476,20 +489,22 @@ void ExtractRemoteStats(const cricket::MediaReceiverInfo& info, template void ExtractStatsFromList(const std::vector& data, const std::string& transport_id, - StatsCollector* collector) { + 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(); - // Each object can result in 2 objects, a local and a remote object. + // 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); - if (!report) { - continue; - } - ExtractStats(*it, report); + StatsReport* report = collector->PrepareLocalReport(ssrc, transport_id, + direction); + if (report) + ExtractStats(*it, report); + if (it->remote_stats.size() > 0) { - report = collector->PrepareRemoteReport(ssrc, transport_id); + report = collector->PrepareRemoteReport(ssrc, transport_id, + direction); if (!report) { continue; } @@ -606,18 +621,16 @@ StatsCollector::UpdateStats(PeerConnectionInterface::StatsOutputLevel level) { StatsReport* StatsCollector::PrepareLocalReport( uint32 ssrc, - const std::string& transport_id) { - std::string ssrc_id = talk_base::ToString(ssrc); + const std::string& transport_id, + TrackDirection direction) { + const std::string ssrc_id = talk_base::ToString(ssrc); StatsMap::iterator it = reports_.find(StatsId( - StatsReport::kStatsReportTypeSsrc, ssrc_id)); + StatsReport::kStatsReportTypeSsrc, ssrc_id, direction)); std::string track_id; if (it == reports_.end()) { - if (!session()->GetTrackIdBySsrc(ssrc, &track_id)) { - LOG(LS_WARNING) << "The SSRC " << ssrc - << " is not associated with a track"; + if (!GetTrackIdBySsrc(ssrc, &track_id, direction)) return NULL; - } } else { // Keeps the old track id since we want to report the stats for inactive // tracks. @@ -627,7 +640,7 @@ StatsReport* StatsCollector::PrepareLocalReport( } StatsReport* report = GetOrCreateReport(StatsReport::kStatsReportTypeSsrc, - ssrc_id); + ssrc_id, direction); // Clear out stats from previous GatherStats calls if any. if (report->timestamp != stats_gathering_started_) { @@ -645,18 +658,16 @@ StatsReport* StatsCollector::PrepareLocalReport( StatsReport* StatsCollector::PrepareRemoteReport( uint32 ssrc, - const std::string& transport_id) { - std::string ssrc_id = talk_base::ToString(ssrc); + const std::string& transport_id, + TrackDirection direction) { + const std::string ssrc_id = talk_base::ToString(ssrc); StatsMap::iterator it = reports_.find(StatsId( - StatsReport::kStatsReportTypeRemoteSsrc, ssrc_id)); + StatsReport::kStatsReportTypeRemoteSsrc, ssrc_id, direction)); std::string track_id; if (it == reports_.end()) { - if (!session()->GetTrackIdBySsrc(ssrc, &track_id)) { - LOG(LS_WARNING) << "The SSRC " << ssrc - << " is not associated with a track"; + if (!GetTrackIdBySsrc(ssrc, &track_id, direction)) return NULL; - } } else { // Keeps the old track id since we want to report the stats for inactive // tracks. @@ -666,7 +677,7 @@ StatsReport* StatsCollector::PrepareRemoteReport( } StatsReport* report = GetOrCreateReport( - StatsReport::kStatsReportTypeRemoteSsrc, ssrc_id); + StatsReport::kStatsReportTypeRemoteSsrc, ssrc_id, direction); // Clear out stats from previous GatherStats calls if any. // The timestamp will be added later. Zero it for debugging. @@ -865,8 +876,8 @@ void StatsCollector::ExtractVoiceInfo() { << session_->voice_channel()->content_name(); return; } - ExtractStatsFromList(voice_info.receivers, transport_id, this); - ExtractStatsFromList(voice_info.senders, transport_id, this); + ExtractStatsFromList(voice_info.receivers, transport_id, this, kReceiving); + ExtractStatsFromList(voice_info.senders, transport_id, this, kSending); UpdateStatsFromExistingLocalAudioTracks(); } @@ -892,8 +903,8 @@ void StatsCollector::ExtractVideoInfo( << session_->video_channel()->content_name(); return; } - ExtractStatsFromList(video_info.receivers, transport_id, this); - ExtractStatsFromList(video_info.senders, transport_id, this); + ExtractStatsFromList(video_info.receivers, transport_id, this, kReceiving); + ExtractStatsFromList(video_info.senders, transport_id, this, kSending); if (video_info.bw_estimations.size() != 1) { LOG(LS_ERROR) << "BWEs count: " << video_info.bw_estimations.size(); } else { @@ -926,8 +937,11 @@ bool StatsCollector::GetTransportIdFromProxy(const std::string& proxy, } StatsReport* StatsCollector::GetReport(const std::string& type, - const std::string& id) { - std::string statsid = StatsId(type, id); + const std::string& id, + TrackDirection direction) { + ASSERT(type == StatsReport::kStatsReportTypeSsrc || + type == StatsReport::kStatsReportTypeRemoteSsrc); + std::string statsid = StatsId(type, id, direction); StatsReport* report = NULL; std::map::iterator it = reports_.find(statsid); if (it != reports_.end()) @@ -937,10 +951,13 @@ StatsReport* StatsCollector::GetReport(const std::string& type, } StatsReport* StatsCollector::GetOrCreateReport(const std::string& type, - const std::string& id) { - StatsReport* report = GetReport(type, id); + const std::string& id, + TrackDirection direction) { + ASSERT(type == StatsReport::kStatsReportTypeSsrc || + type == StatsReport::kStatsReportTypeRemoteSsrc); + StatsReport* report = GetReport(type, id, direction); if (report == NULL) { - std::string statsid = StatsId(type, id); + std::string statsid = StatsId(type, id, direction); report = &reports_[statsid]; // Create new element. report->id = statsid; report->type = type; @@ -957,7 +974,8 @@ void StatsCollector::UpdateStatsFromExistingLocalAudioTracks() { uint32 ssrc = it->second; std::string ssrc_id = talk_base::ToString(ssrc); StatsReport* report = GetReport(StatsReport::kStatsReportTypeSsrc, - ssrc_id); + ssrc_id, + kSending); 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. @@ -1012,4 +1030,24 @@ void StatsCollector::UpdateReportFromAudioTrack(AudioTrackInterface* track, talk_base::ToString(stats.echo_delay_std_ms)); } +bool StatsCollector::GetTrackIdBySsrc(uint32 ssrc, std::string* track_id, + TrackDirection direction) { + if (direction == kSending) { + 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); + if (!session()->GetRemoteTrackIdBySsrc(ssrc, track_id)) { + LOG(LS_WARNING) << "The SSRC " << ssrc + << " is not associated with a receiving track"; + return false; + } + } + + return true; +} + } // namespace webrtc diff --git a/talk/app/webrtc/statscollector.h b/talk/app/webrtc/statscollector.h index fdb2961573..77a1ba086d 100644 --- a/talk/app/webrtc/statscollector.h +++ b/talk/app/webrtc/statscollector.h @@ -46,6 +46,11 @@ namespace webrtc { class StatsCollector { public: + enum TrackDirection { + kSending = 0, + kReceiving, + }; + StatsCollector(); // Register the session Stats should operate on. @@ -77,9 +82,11 @@ class StatsCollector { // Prepare an SSRC report for the given ssrc. Used internally // in the ExtractStatsFromList template. - StatsReport* PrepareLocalReport(uint32 ssrc, const std::string& transport); + 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); + StatsReport* PrepareRemoteReport(uint32 ssrc, const std::string& transport, + TrackDirection direction); // Extracts the ID of a Transport belonging to an SSRC. Used internally. bool GetTransportIdFromProxy(const std::string& proxy, std::string* transport_id); @@ -102,15 +109,22 @@ class StatsCollector { void BuildSsrcToTransportId(); WebRtcSession* session() { return session_; } webrtc::StatsReport* GetOrCreateReport(const std::string& type, - const std::string& id); + const std::string& id, + TrackDirection direction); webrtc::StatsReport* GetReport(const std::string& type, - const std::string& id); + const std::string& id, + TrackDirection direction); // Helper method to get stats from the local audio tracks. void UpdateStatsFromExistingLocalAudioTracks(); void UpdateReportFromAudioTrack(AudioTrackInterface* track, StatsReport* report); + // 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); + // A map from the report id to the report. std::map reports_; // Raw pointer to the session the statistics are gathered from. diff --git a/talk/app/webrtc/statscollector_unittest.cc b/talk/app/webrtc/statscollector_unittest.cc index 3003d1b2da..5cdc81e19c 100644 --- a/talk/app/webrtc/statscollector_unittest.cc +++ b/talk/app/webrtc/statscollector_unittest.cc @@ -1,5 +1,6 @@ /* * libjingle + * Copyright 2014, Google Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are met: @@ -39,7 +40,8 @@ #include "talk/media/devices/fakedevicemanager.h" #include "talk/p2p/base/fakesession.h" #include "talk/session/media/channelmanager.h" -#include "testing/base/public/gmock.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" using cricket::StatsOptions; using testing::_; @@ -66,8 +68,8 @@ const char kNotFound[] = "NOT FOUND"; const char kNoReports[] = "NO REPORTS"; // Constant names for track identification. -const char kTrackId[] = "somename"; -const char kAudioTrackId[] = "audio_track_id"; +const char kLocalTrackId[] = "local_track_id"; +const char kRemoteTrackId[] = "remote_track_id"; const uint32 kSsrcOfTrack = 1234; class MockWebRtcSession : public webrtc::WebRtcSession { @@ -78,7 +80,10 @@ class MockWebRtcSession : public webrtc::WebRtcSession { } MOCK_METHOD0(voice_channel, cricket::VoiceChannel*()); MOCK_METHOD0(video_channel, cricket::VideoChannel*()); - MOCK_METHOD2(GetTrackIdBySsrc, bool(uint32, std::string*)); + // Libjingle uses "local" for a outgoing track, and "remote" for a incoming + // track. + MOCK_METHOD2(GetLocalTrackIdBySsrc, bool(uint32, std::string*)); + MOCK_METHOD2(GetRemoteTrackIdBySsrc, bool(uint32, std::string*)); MOCK_METHOD1(GetStats, bool(cricket::SessionStats*)); MOCK_METHOD1(GetTransport, cricket::Transport*(const std::string&)); }; @@ -116,10 +121,10 @@ class FakeAudioProcessor : public webrtc::AudioProcessorInterface { } }; -class FakeLocalAudioTrack +class FakeAudioTrack : public webrtc::MediaStreamTrack { public: - explicit FakeLocalAudioTrack(const std::string& id) + explicit FakeAudioTrack(const std::string& id) : webrtc::MediaStreamTrack(id), processor_(new talk_base::RefCountedObject()) {} std::string kind() const OVERRIDE { @@ -263,55 +268,56 @@ void CheckCertChainReports(const StatsReports& reports, EXPECT_EQ(ders.size(), i); } -void VerifyVoiceReceiverInfoReport(const StatsReport* report, - const cricket::VoiceReceiverInfo& sinfo) { +void VerifyVoiceReceiverInfoReport( + const StatsReport* report, + const cricket::VoiceReceiverInfo& info) { std::string value_in_report; EXPECT_TRUE(GetValue( report, StatsReport::kStatsValueNameAudioOutputLevel, &value_in_report)); - EXPECT_EQ(talk_base::ToString(sinfo.audio_level), value_in_report); + EXPECT_EQ(talk_base::ToString(info.audio_level), value_in_report); EXPECT_TRUE(GetValue( report, StatsReport::kStatsValueNameBytesReceived, &value_in_report)); - EXPECT_EQ(talk_base::ToString(sinfo.bytes_rcvd), value_in_report); + EXPECT_EQ(talk_base::ToString(info.bytes_rcvd), value_in_report); EXPECT_TRUE(GetValue( report, StatsReport::kStatsValueNameJitterReceived, &value_in_report)); - EXPECT_EQ(talk_base::ToString(sinfo.jitter_ms), value_in_report); + EXPECT_EQ(talk_base::ToString(info.jitter_ms), value_in_report); EXPECT_TRUE(GetValue( report, StatsReport::kStatsValueNameJitterBufferMs, &value_in_report)); - EXPECT_EQ(talk_base::ToString(sinfo.jitter_buffer_ms), value_in_report); + EXPECT_EQ(talk_base::ToString(info.jitter_buffer_ms), value_in_report); EXPECT_TRUE(GetValue( report, StatsReport::kStatsValueNamePreferredJitterBufferMs, &value_in_report)); - EXPECT_EQ(talk_base::ToString(sinfo.jitter_buffer_preferred_ms), + EXPECT_EQ(talk_base::ToString(info.jitter_buffer_preferred_ms), value_in_report); EXPECT_TRUE(GetValue( report, StatsReport::kStatsValueNameCurrentDelayMs, &value_in_report)); - EXPECT_EQ(talk_base::ToString(sinfo.delay_estimate_ms), value_in_report); + EXPECT_EQ(talk_base::ToString(info.delay_estimate_ms), value_in_report); EXPECT_TRUE(GetValue( report, StatsReport::kStatsValueNameExpandRate, &value_in_report)); - EXPECT_EQ(talk_base::ToString(sinfo.expand_rate), value_in_report); + EXPECT_EQ(talk_base::ToString(info.expand_rate), value_in_report); EXPECT_TRUE(GetValue( report, StatsReport::kStatsValueNamePacketsReceived, &value_in_report)); - EXPECT_EQ(talk_base::ToString(sinfo.packets_rcvd), value_in_report); + EXPECT_EQ(talk_base::ToString(info.packets_rcvd), value_in_report); EXPECT_TRUE(GetValue( report, StatsReport::kStatsValueNameDecodingCTSG, &value_in_report)); - EXPECT_EQ(talk_base::ToString(sinfo.decoding_calls_to_silence_generator), + EXPECT_EQ(talk_base::ToString(info.decoding_calls_to_silence_generator), value_in_report); EXPECT_TRUE(GetValue( report, StatsReport::kStatsValueNameDecodingCTN, &value_in_report)); - EXPECT_EQ(talk_base::ToString(sinfo.decoding_calls_to_neteq), + EXPECT_EQ(talk_base::ToString(info.decoding_calls_to_neteq), value_in_report); EXPECT_TRUE(GetValue( report, StatsReport::kStatsValueNameDecodingNormal, &value_in_report)); - EXPECT_EQ(talk_base::ToString(sinfo.decoding_normal), value_in_report); + EXPECT_EQ(talk_base::ToString(info.decoding_normal), value_in_report); EXPECT_TRUE(GetValue( report, StatsReport::kStatsValueNameDecodingPLC, &value_in_report)); - EXPECT_EQ(talk_base::ToString(sinfo.decoding_plc), value_in_report); + EXPECT_EQ(talk_base::ToString(info.decoding_plc), value_in_report); EXPECT_TRUE(GetValue( report, StatsReport::kStatsValueNameDecodingCNG, &value_in_report)); - EXPECT_EQ(talk_base::ToString(sinfo.decoding_cng), value_in_report); + EXPECT_EQ(talk_base::ToString(info.decoding_cng), value_in_report); EXPECT_TRUE(GetValue( report, StatsReport::kStatsValueNameDecodingPLCCNG, &value_in_report)); - EXPECT_EQ(talk_base::ToString(sinfo.decoding_plc_cng), value_in_report); + EXPECT_EQ(talk_base::ToString(info.decoding_plc_cng), value_in_report); } @@ -369,6 +375,59 @@ void VerifyVoiceSenderInfoReport(const StatsReport* report, EXPECT_EQ(typing_detected, value_in_report); } +// Helper methods to avoid duplication of code. +void InitVoiceSenderInfo(cricket::VoiceSenderInfo* voice_sender_info) { + voice_sender_info->add_ssrc(kSsrcOfTrack); + voice_sender_info->codec_name = "fake_codec"; + voice_sender_info->bytes_sent = 100; + voice_sender_info->packets_sent = 101; + voice_sender_info->rtt_ms = 102; + voice_sender_info->fraction_lost = 103; + voice_sender_info->jitter_ms = 104; + voice_sender_info->packets_lost = 105; + voice_sender_info->ext_seqnum = 106; + voice_sender_info->audio_level = 107; + voice_sender_info->echo_return_loss = 108; + voice_sender_info->echo_return_loss_enhancement = 109; + voice_sender_info->echo_delay_median_ms = 110; + voice_sender_info->echo_delay_std_ms = 111; + voice_sender_info->aec_quality_min = 112.0f; + voice_sender_info->typing_noise_detected = false; +} + +void UpdateVoiceSenderInfoFromAudioTrack( + FakeAudioTrack* 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); + voice_sender_info->typing_noise_detected = + audio_processor_stats.typing_noise_detected; + voice_sender_info->echo_return_loss = audio_processor_stats.echo_return_loss; + voice_sender_info->echo_return_loss_enhancement = + audio_processor_stats.echo_return_loss_enhancement; + voice_sender_info->echo_delay_median_ms = + audio_processor_stats.echo_delay_median_ms; + voice_sender_info->aec_quality_min = audio_processor_stats.aec_quality_min; + voice_sender_info->echo_delay_std_ms = + audio_processor_stats.echo_delay_std_ms; +} + +void InitVoiceReceiverInfo(cricket::VoiceReceiverInfo* voice_receiver_info) { + voice_receiver_info->add_ssrc(kSsrcOfTrack); + voice_receiver_info->bytes_rcvd = 110; + voice_receiver_info->packets_rcvd = 111; + voice_receiver_info->packets_lost = 112; + voice_receiver_info->fraction_lost = 113; + voice_receiver_info->packets_lost = 114; + voice_receiver_info->ext_seqnum = 115; + voice_receiver_info->jitter_ms = 116; + voice_receiver_info->jitter_buffer_ms = 117; + voice_receiver_info->jitter_buffer_preferred_ms = 118; + voice_receiver_info->delay_estimate_ms = 119; + voice_receiver_info->audio_level = 120; + voice_receiver_info->expand_rate = 121; +} + class StatsCollectorTest : public testing::Test { protected: StatsCollectorTest() @@ -377,8 +436,7 @@ class StatsCollectorTest : public testing::Test { new cricket::ChannelManager(media_engine_, new cricket::FakeDeviceManager(), talk_base::Thread::Current())), - session_(channel_manager_.get()), - track_id_(kTrackId) { + session_(channel_manager_.get()) { // By default, we ignore session GetStats calls. EXPECT_CALL(session_, GetStats(_)).WillRepeatedly(Return(false)); } @@ -398,27 +456,56 @@ class StatsCollectorTest : public testing::Test { session_stats_.proxy_to_transport[vc_name] = kTransportName; } - // Adds a track with a given SSRC into the stats. - void AddVideoTrackStats() { + // Adds a outgoing video track with a given SSRC into the stats. + void AddOutgoingVideoTrackStats() { stream_ = webrtc::MediaStream::Create("streamlabel"); - track_= webrtc::VideoTrack::Create(kTrackId, NULL); + track_= webrtc::VideoTrack::Create(kLocalTrackId, NULL); stream_->AddTrack(track_); - EXPECT_CALL(session_, GetTrackIdBySsrc(kSsrcOfTrack, _)) - .WillRepeatedly(DoAll(SetArgPointee<1>(track_id_), - Return(true))); + EXPECT_CALL(session_, GetLocalTrackIdBySsrc(kSsrcOfTrack, _)) + .WillRepeatedly(DoAll(SetArgPointee<1>(kLocalTrackId), Return(true))); + EXPECT_CALL(session_, GetRemoteTrackIdBySsrc(kSsrcOfTrack, _)) + .WillRepeatedly(Return(false)); } - // Adds a local audio track with a given SSRC into the stats. - void AddLocalAudioTrackStats() { + // Adds a incoming video track with a given SSRC into the stats. + void AddIncomingVideoTrackStats() { + stream_ = webrtc::MediaStream::Create("streamlabel"); + track_= webrtc::VideoTrack::Create(kRemoteTrackId, NULL); + stream_->AddTrack(track_); + EXPECT_CALL(session_, GetLocalTrackIdBySsrc(kSsrcOfTrack, _)) + .WillRepeatedly(Return(false)); + EXPECT_CALL(session_, GetRemoteTrackIdBySsrc(kSsrcOfTrack, _)) + .WillRepeatedly(DoAll(SetArgPointee<1>(kRemoteTrackId), + Return(true))); + } + + // Adds a outgoing audio track with a given SSRC into the stats. + void AddOutgoingAudioTrackStats() { if (stream_ == NULL) stream_ = webrtc::MediaStream::Create("streamlabel"); - audio_track_ = - new talk_base::RefCountedObject(kAudioTrackId); + audio_track_ = new talk_base::RefCountedObject( + kLocalTrackId); stream_->AddTrack(audio_track_); - EXPECT_CALL(session_, GetTrackIdBySsrc(kSsrcOfTrack, _)) - .WillRepeatedly(DoAll(SetArgPointee<1>(kAudioTrackId), + EXPECT_CALL(session_, GetLocalTrackIdBySsrc(kSsrcOfTrack, _)) + .WillRepeatedly(DoAll(SetArgPointee<1>(kLocalTrackId), Return(true))); + EXPECT_CALL(session_, GetRemoteTrackIdBySsrc(kSsrcOfTrack, _)) + .WillRepeatedly(Return(false)); + } + + // Adds a incoming audio track with a given SSRC into the stats. + void AddIncomingAudioTrackStats() { + if (stream_ == NULL) + stream_ = webrtc::MediaStream::Create("streamlabel"); + + audio_track_ = new talk_base::RefCountedObject( + kRemoteTrackId); + stream_->AddTrack(audio_track_); + EXPECT_CALL(session_, GetLocalTrackIdBySsrc(kSsrcOfTrack, _)) + .WillRepeatedly(Return(false)); + EXPECT_CALL(session_, GetRemoteTrackIdBySsrc(kSsrcOfTrack, _)) + .WillRepeatedly(DoAll(SetArgPointee<1>(kRemoteTrackId), Return(true))); } void TestCertificateReports(const talk_base::FakeSSLCertificate& local_cert, @@ -507,14 +594,13 @@ class StatsCollectorTest : public testing::Test { cricket::SessionStats session_stats_; talk_base::scoped_refptr stream_; talk_base::scoped_refptr track_; - talk_base::scoped_refptr audio_track_; - std::string track_id_; + talk_base::scoped_refptr audio_track_; }; // This test verifies that 64-bit counters are passed successfully. TEST_F(StatsCollectorTest, BytesCounterHandles64Bits) { webrtc::StatsCollector stats; // Implementation under test. - MockVideoMediaChannel* media_channel = new MockVideoMediaChannel; + MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); cricket::VideoChannel video_channel(talk_base::Thread::Current(), media_engine_, media_channel, &session_, "", false, NULL); StatsReports reports; // returned values. @@ -525,7 +611,7 @@ TEST_F(StatsCollectorTest, BytesCounterHandles64Bits) { const std::string kBytesSentString("12345678901234"); stats.set_session(&session_); - AddVideoTrackStats(); + AddOutgoingVideoTrackStats(); stats.AddStream(stream_); // Construct a stats value to read. @@ -547,7 +633,7 @@ TEST_F(StatsCollectorTest, BytesCounterHandles64Bits) { // Test that BWE information is reported via stats. TEST_F(StatsCollectorTest, BandwidthEstimationInfoIsReported) { webrtc::StatsCollector stats; // Implementation under test. - MockVideoMediaChannel* media_channel = new MockVideoMediaChannel; + MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); cricket::VideoChannel video_channel(talk_base::Thread::Current(), media_engine_, media_channel, &session_, "", false, NULL); StatsReports reports; // returned values. @@ -559,7 +645,7 @@ TEST_F(StatsCollectorTest, BandwidthEstimationInfoIsReported) { const std::string kBytesSentString("12345678901234"); stats.set_session(&session_); - AddVideoTrackStats(); + AddOutgoingVideoTrackStats(); stats.AddStream(stream_); // Construct a stats value to read. @@ -624,10 +710,10 @@ TEST_F(StatsCollectorTest, OnlyOneSessionObjectExists) { // without calling StatsCollector::UpdateStats. TEST_F(StatsCollectorTest, TrackObjectExistsWithoutUpdateStats) { webrtc::StatsCollector stats; // Implementation under test. - MockVideoMediaChannel* media_channel = new MockVideoMediaChannel; + MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); cricket::VideoChannel video_channel(talk_base::Thread::Current(), media_engine_, media_channel, &session_, "", false, NULL); - AddVideoTrackStats(); + AddOutgoingVideoTrackStats(); stats.AddStream(stream_); stats.set_session(&session_); @@ -644,17 +730,17 @@ TEST_F(StatsCollectorTest, TrackObjectExistsWithoutUpdateStats) { ExtractStatsValue(StatsReport::kStatsReportTypeTrack, reports, StatsReport::kStatsValueNameTrackId); - EXPECT_EQ(kTrackId, trackValue); + EXPECT_EQ(kLocalTrackId, trackValue); } // This test verifies that the empty track report exists in the returned stats // when StatsCollector::UpdateStats is called with ssrc stats. TEST_F(StatsCollectorTest, TrackAndSsrcObjectExistAfterUpdateSsrcStats) { webrtc::StatsCollector stats; // Implementation under test. - MockVideoMediaChannel* media_channel = new MockVideoMediaChannel; + MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); cricket::VideoChannel video_channel(talk_base::Thread::Current(), media_engine_, media_channel, &session_, "", false, NULL); - AddVideoTrackStats(); + AddOutgoingVideoTrackStats(); stats.AddStream(stream_); stats.set_session(&session_); @@ -684,15 +770,16 @@ TEST_F(StatsCollectorTest, TrackAndSsrcObjectExistAfterUpdateSsrcStats) { EXPECT_LE((size_t)3, reports.size()); const StatsReport* track_report = FindNthReportByType( reports, StatsReport::kStatsReportTypeTrack, 1); - EXPECT_FALSE(track_report == NULL); + EXPECT_TRUE(track_report); + // Get report for the specific |track|. stats.GetStats(track_, &reports); // |reports| should contain at least one session report, one track report, // and one ssrc report. EXPECT_LE((size_t)3, reports.size()); track_report = FindNthReportByType( reports, StatsReport::kStatsReportTypeTrack, 1); - EXPECT_FALSE(track_report == NULL); + EXPECT_TRUE(track_report); std::string ssrc_id = ExtractSsrcStatsValue( reports, StatsReport::kStatsValueNameSsrc); @@ -700,19 +787,19 @@ TEST_F(StatsCollectorTest, TrackAndSsrcObjectExistAfterUpdateSsrcStats) { std::string track_id = ExtractSsrcStatsValue( reports, StatsReport::kStatsValueNameTrackId); - EXPECT_EQ(kTrackId, track_id); + EXPECT_EQ(kLocalTrackId, track_id); } // This test verifies that an SSRC object has the identifier of a Transport // stats object, and that this transport stats object exists in stats. TEST_F(StatsCollectorTest, TransportObjectLinkedFromSsrcObject) { webrtc::StatsCollector stats; // Implementation under test. - MockVideoMediaChannel* media_channel = new MockVideoMediaChannel; + MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); // The content_name known by the video channel. const std::string kVcName("vcname"); cricket::VideoChannel video_channel(talk_base::Thread::Current(), media_engine_, media_channel, &session_, kVcName, false, NULL); - AddVideoTrackStats(); + AddOutgoingVideoTrackStats(); stats.AddStream(stream_); stats.set_session(&session_); @@ -756,12 +843,12 @@ TEST_F(StatsCollectorTest, TransportObjectLinkedFromSsrcObject) { // an outgoing SSRC where remote stats are not returned. TEST_F(StatsCollectorTest, RemoteSsrcInfoIsAbsent) { webrtc::StatsCollector stats; // Implementation under test. - MockVideoMediaChannel* media_channel = new MockVideoMediaChannel; + MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); // The content_name known by the video channel. const std::string kVcName("vcname"); cricket::VideoChannel video_channel(talk_base::Thread::Current(), media_engine_, media_channel, &session_, kVcName, false, NULL); - AddVideoTrackStats(); + AddOutgoingVideoTrackStats(); stats.AddStream(stream_); stats.set_session(&session_); @@ -781,12 +868,12 @@ TEST_F(StatsCollectorTest, RemoteSsrcInfoIsAbsent) { // an outgoing SSRC where stats are returned. TEST_F(StatsCollectorTest, RemoteSsrcInfoIsPresent) { webrtc::StatsCollector stats; // Implementation under test. - MockVideoMediaChannel* media_channel = new MockVideoMediaChannel; + MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); // The content_name known by the video channel. const std::string kVcName("vcname"); cricket::VideoChannel video_channel(talk_base::Thread::Current(), media_engine_, media_channel, &session_, kVcName, false, NULL); - AddVideoTrackStats(); + AddOutgoingVideoTrackStats(); stats.AddStream(stream_); stats.set_session(&session_); @@ -818,12 +905,61 @@ TEST_F(StatsCollectorTest, RemoteSsrcInfoIsPresent) { stats.UpdateStats(PeerConnectionInterface::kStatsOutputLevelStandard); stats.GetStats(NULL, &reports); + const StatsReport* remote_report = FindNthReportByType(reports, StatsReport::kStatsReportTypeRemoteSsrc, 1); EXPECT_FALSE(remote_report == NULL); EXPECT_NE(0, remote_report->timestamp); } +// This test verifies that the empty track report exists in the returned stats +// when StatsCollector::UpdateStats is called with ssrc stats. +TEST_F(StatsCollectorTest, ReportsFromRemoteTrack) { + webrtc::StatsCollector stats; // Implementation under test. + MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); + cricket::VideoChannel video_channel(talk_base::Thread::Current(), + media_engine_, media_channel, &session_, "", false, NULL); + AddIncomingVideoTrackStats(); + stats.AddStream(stream_); + + stats.set_session(&session_); + + StatsReports reports; + + // Constructs an ssrc stats update. + cricket::VideoReceiverInfo video_receiver_info; + cricket::VideoMediaInfo stats_read; + const int64 kNumOfPacketsConcealed = 54321; + + // Construct a stats value to read. + video_receiver_info.add_ssrc(1234); + video_receiver_info.packets_concealed = kNumOfPacketsConcealed; + stats_read.receivers.push_back(video_receiver_info); + + EXPECT_CALL(session_, video_channel()).WillRepeatedly(Return(&video_channel)); + EXPECT_CALL(session_, voice_channel()).WillRepeatedly(ReturnNull()); + EXPECT_CALL(*media_channel, GetStats(_, _)) + .WillOnce(DoAll(SetArgPointee<1>(stats_read), + Return(true))); + + stats.UpdateStats(PeerConnectionInterface::kStatsOutputLevelStandard); + stats.GetStats(NULL, &reports); + // |reports| should contain at least one session report, one track report, + // and one ssrc report. + EXPECT_LE(static_cast(3), reports.size()); + const StatsReport* track_report = FindNthReportByType( + reports, StatsReport::kStatsReportTypeTrack, 1); + EXPECT_TRUE(track_report); + + std::string ssrc_id = ExtractSsrcStatsValue( + reports, StatsReport::kStatsValueNameSsrc); + EXPECT_EQ(talk_base::ToString(kSsrcOfTrack), ssrc_id); + + std::string track_id = ExtractSsrcStatsValue( + reports, StatsReport::kStatsValueNameTrackId); + EXPECT_EQ(kRemoteTrackId, track_id); +} + // This test verifies that all chained certificates are correctly // reported TEST_F(StatsCollectorTest, ChainedCertificateReportsCreated) { @@ -982,7 +1118,7 @@ TEST_F(StatsCollectorTest, UnsupportedDigestIgnored) { // verbose output level. TEST_F(StatsCollectorTest, StatsOutputLevelVerbose) { webrtc::StatsCollector stats; // Implementation under test. - MockVideoMediaChannel* media_channel = new MockVideoMediaChannel; + MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); cricket::VideoChannel video_channel(talk_base::Thread::Current(), media_engine_, media_channel, &session_, "", false, NULL); stats.set_session(&session_); @@ -1031,7 +1167,7 @@ TEST_F(StatsCollectorTest, GetStatsFromLocalAudioTrack) { const std::string kVcName("vcname"); cricket::VoiceChannel voice_channel(talk_base::Thread::Current(), media_engine_, media_channel, &session_, kVcName, false); - AddLocalAudioTrackStats(); + AddOutgoingAudioTrackStats(); stats.AddStream(stream_); stats.AddLocalAudioTrack(audio_track_.get(), kSsrcOfTrack); @@ -1044,25 +1180,7 @@ TEST_F(StatsCollectorTest, GetStatsFromLocalAudioTrack) { Return(true))); cricket::VoiceSenderInfo voice_sender_info; - // Contents won't be modified by the AudioTrackInterface::GetStats(). - voice_sender_info.add_ssrc(kSsrcOfTrack); - voice_sender_info.codec_name = "fake_codec"; - voice_sender_info.bytes_sent = 100; - voice_sender_info.packets_sent = 101; - voice_sender_info.rtt_ms = 102; - voice_sender_info.fraction_lost = 103; - voice_sender_info.jitter_ms = 104; - voice_sender_info.packets_lost = 105; - voice_sender_info.ext_seqnum = 106; - - // Contents will be modified by the AudioTrackInterface::GetStats(). - voice_sender_info.audio_level = 107; - voice_sender_info.echo_return_loss = 108;; - voice_sender_info.echo_return_loss_enhancement = 109; - voice_sender_info.echo_delay_median_ms = 110; - voice_sender_info.echo_delay_std_ms = 111; - voice_sender_info.aec_quality_min = 112.0f; - voice_sender_info.typing_noise_detected = false; + InitVoiceSenderInfo(&voice_sender_info); // Constructs an ssrc stats update. cricket::VoiceMediaInfo stats_read; @@ -1084,24 +1202,13 @@ TEST_F(StatsCollectorTest, GetStatsFromLocalAudioTrack) { EXPECT_FALSE(report == NULL); std::string track_id = ExtractSsrcStatsValue( reports, StatsReport::kStatsValueNameTrackId); - EXPECT_EQ(kAudioTrackId, track_id); + EXPECT_EQ(kLocalTrackId, track_id); std::string ssrc_id = ExtractSsrcStatsValue( reports, StatsReport::kStatsValueNameSsrc); EXPECT_EQ(talk_base::ToString(kSsrcOfTrack), ssrc_id); // Verifies the values in the track report. - audio_track_->GetSignalLevel(&voice_sender_info.audio_level); - webrtc::AudioProcessorInterface::AudioProcessorStats audio_processor_stats; - audio_track_->GetAudioProcessor()->GetStats(&audio_processor_stats); - voice_sender_info.typing_noise_detected = - audio_processor_stats.typing_noise_detected; - voice_sender_info.echo_return_loss = audio_processor_stats.echo_return_loss; - voice_sender_info.echo_return_loss_enhancement = - audio_processor_stats.echo_return_loss_enhancement; - voice_sender_info.echo_delay_median_ms = - audio_processor_stats.echo_delay_median_ms; - voice_sender_info.aec_quality_min = audio_processor_stats.aec_quality_min; - voice_sender_info.echo_delay_std_ms = audio_processor_stats.echo_delay_std_ms; + UpdateVoiceSenderInfoFromAudioTrack(audio_track_.get(), &voice_sender_info); VerifyVoiceSenderInfoReport(report, voice_sender_info); // Verify we get the same result by passing a track to GetStats(). @@ -1109,16 +1216,21 @@ TEST_F(StatsCollectorTest, GetStatsFromLocalAudioTrack) { stats.GetStats(audio_track_.get(), &track_reports); const StatsReport* track_report = FindNthReportByType( track_reports, StatsReport::kStatsReportTypeSsrc, 1); - EXPECT_FALSE(track_report == NULL); + EXPECT_TRUE(track_report); track_id = ExtractSsrcStatsValue(track_reports, StatsReport::kStatsValueNameTrackId); - EXPECT_EQ(kAudioTrackId, track_id); + EXPECT_EQ(kLocalTrackId, track_id); ssrc_id = ExtractSsrcStatsValue(track_reports, StatsReport::kStatsValueNameSsrc); EXPECT_EQ(talk_base::ToString(kSsrcOfTrack), ssrc_id); VerifyVoiceSenderInfoReport(track_report, voice_sender_info); -} + // Verify that there is no remote report for the local audio track because + // we did not set it up. + const StatsReport* remote_report = FindNthReportByType(reports, + StatsReport::kStatsReportTypeRemoteSsrc, 1); + EXPECT_TRUE(remote_report == NULL); +} // This test verifies that audio receive streams populate stats reports // correctly. @@ -1129,7 +1241,7 @@ TEST_F(StatsCollectorTest, GetStatsFromRemoteStream) { const std::string kVcName("vcname"); cricket::VoiceChannel voice_channel(talk_base::Thread::Current(), media_engine_, media_channel, &session_, kVcName, false); - stream_ = webrtc::MediaStream::Create("remoteStreamLabel"); + AddIncomingAudioTrackStats(); stats.AddStream(stream_); stats.set_session(&session_); @@ -1141,19 +1253,7 @@ TEST_F(StatsCollectorTest, GetStatsFromRemoteStream) { Return(true))); cricket::VoiceReceiverInfo voice_receiver_info; - voice_receiver_info.add_ssrc(kSsrcOfTrack); - voice_receiver_info.bytes_rcvd = 100; - voice_receiver_info.packets_rcvd = 101; - voice_receiver_info.packets_lost = 102; - voice_receiver_info.fraction_lost = 103; - voice_receiver_info.packets_lost = 104; - voice_receiver_info.ext_seqnum = 105; - voice_receiver_info.jitter_ms = 106; - voice_receiver_info.jitter_buffer_ms = 107; - voice_receiver_info.jitter_buffer_preferred_ms = 108; - voice_receiver_info.delay_estimate_ms = 109; - voice_receiver_info.audio_level = 110; - voice_receiver_info.expand_rate = 111; + InitVoiceReceiverInfo(&voice_receiver_info); // Constructs an ssrc stats update. cricket::VoiceMediaInfo stats_read; @@ -1164,21 +1264,23 @@ TEST_F(StatsCollectorTest, GetStatsFromRemoteStream) { EXPECT_CALL(*media_channel, GetStats(_)) .WillRepeatedly(DoAll(SetArgPointee<0>(stats_read), Return(true))); - EXPECT_CALL(session_, GetTrackIdBySsrc(kSsrcOfTrack, _)) - .WillRepeatedly(Return(true)); StatsReports reports; // returned values. stats.UpdateStats(PeerConnectionInterface::kStatsOutputLevelStandard); stats.GetStats(NULL, &reports); - // Verify the remote report. + // Verify the track id is |kRemoteTrackId|. + const std::string track_id = ExtractSsrcStatsValue( + reports, StatsReport::kStatsValueNameTrackId); + EXPECT_EQ(kRemoteTrackId, track_id); + + // Verify the report for this remote track. const StatsReport* report = FindNthReportByType( reports, StatsReport::kStatsReportTypeSsrc, 1); EXPECT_FALSE(report == NULL); VerifyVoiceReceiverInfoReport(report, voice_receiver_info); } - // This test verifies that a local stats object won't update its statistics // after a RemoveLocalAudioTrack() call. TEST_F(StatsCollectorTest, GetStatsAfterRemoveAudioStream) { @@ -1188,7 +1290,7 @@ TEST_F(StatsCollectorTest, GetStatsAfterRemoveAudioStream) { const std::string kVcName("vcname"); cricket::VoiceChannel voice_channel(talk_base::Thread::Current(), media_engine_, media_channel, &session_, kVcName, false); - AddLocalAudioTrackStats(); + AddOutgoingAudioTrackStats(); stats.AddStream(stream_); stats.AddLocalAudioTrack(audio_track_.get(), kSsrcOfTrack); @@ -1202,25 +1304,7 @@ TEST_F(StatsCollectorTest, GetStatsAfterRemoveAudioStream) { stats.RemoveLocalAudioTrack(audio_track_.get(), kSsrcOfTrack); cricket::VoiceSenderInfo voice_sender_info; - // Contents won't be modified by the AudioTrackInterface::GetStats(). - voice_sender_info.add_ssrc(kSsrcOfTrack); - voice_sender_info.codec_name = "fake_codec"; - voice_sender_info.bytes_sent = 100; - voice_sender_info.packets_sent = 101; - voice_sender_info.rtt_ms = 102; - voice_sender_info.fraction_lost = 103; - voice_sender_info.jitter_ms = 104; - voice_sender_info.packets_lost = 105; - voice_sender_info.ext_seqnum = 106; - - // Contents will be modified by the AudioTrackInterface::GetStats(). - voice_sender_info.audio_level = 107; - voice_sender_info.echo_return_loss = 108;; - voice_sender_info.echo_return_loss_enhancement = 109; - voice_sender_info.echo_delay_median_ms = 110; - voice_sender_info.echo_delay_std_ms = 111; - voice_sender_info.aec_quality_min = 112; - voice_sender_info.typing_noise_detected = false; + InitVoiceSenderInfo(&voice_sender_info); // Constructs an ssrc stats update. cricket::VoiceMediaInfo stats_read; @@ -1242,7 +1326,7 @@ TEST_F(StatsCollectorTest, GetStatsAfterRemoveAudioStream) { EXPECT_FALSE(report == NULL); std::string track_id = ExtractSsrcStatsValue( reports, StatsReport::kStatsValueNameTrackId); - EXPECT_EQ(kAudioTrackId, track_id); + EXPECT_EQ(kLocalTrackId, track_id); std::string ssrc_id = ExtractSsrcStatsValue( reports, StatsReport::kStatsValueNameSsrc); EXPECT_EQ(talk_base::ToString(kSsrcOfTrack), ssrc_id); @@ -1253,4 +1337,82 @@ TEST_F(StatsCollectorTest, GetStatsAfterRemoveAudioStream) { VerifyVoiceSenderInfoReport(report, voice_sender_info); } +// This test verifies that when ongoing and incoming audio tracks are using +// the same ssrc, they populate stats reports correctly. +TEST_F(StatsCollectorTest, LocalAndRemoteTracksWithSameSsrc) { + webrtc::StatsCollector stats; // Implementation under test. + MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel(); + // The content_name known by the voice channel. + const std::string kVcName("vcname"); + cricket::VoiceChannel voice_channel(talk_base::Thread::Current(), + media_engine_, media_channel, &session_, kVcName, false); + + // Create a local stream with a local audio track and adds it to the stats. + AddOutgoingAudioTrackStats(); + stats.AddStream(stream_); + stats.AddLocalAudioTrack(audio_track_.get(), kSsrcOfTrack); + + // Create a remote stream with a remote audio track and adds it to the stats. + talk_base::scoped_refptr remote_stream( + webrtc::MediaStream::Create("remotestreamlabel")); + talk_base::scoped_refptr remote_track( + new talk_base::RefCountedObject(kRemoteTrackId)); + EXPECT_CALL(session_, GetRemoteTrackIdBySsrc(kSsrcOfTrack, _)) + .WillRepeatedly(DoAll(SetArgPointee<1>(kRemoteTrackId), Return(true))); + remote_stream->AddTrack(remote_track); + stats.AddStream(remote_stream); + + stats.set_session(&session_); + + // Instruct the session to return stats containing the transport channel. + InitSessionStats(kVcName); + EXPECT_CALL(session_, GetStats(_)) + .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_), + Return(true))); + + cricket::VoiceSenderInfo voice_sender_info; + InitVoiceSenderInfo(&voice_sender_info); + + // Some of the contents in |voice_sender_info| needs to be updated from the + // |audio_track_|. + UpdateVoiceSenderInfoFromAudioTrack(audio_track_.get(), &voice_sender_info); + + cricket::VoiceReceiverInfo voice_receiver_info; + InitVoiceReceiverInfo(&voice_receiver_info); + + // 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; // returned values. + stats.UpdateStats(PeerConnectionInterface::kStatsOutputLevelStandard); + + // Get stats for the local track. + stats.GetStats(audio_track_.get(), &reports); + const StatsReport* track_report = FindNthReportByType( + reports, StatsReport::kStatsReportTypeSsrc, 1); + EXPECT_TRUE(track_report); + std::string track_id = ExtractSsrcStatsValue( + reports, StatsReport::kStatsValueNameTrackId); + EXPECT_EQ(kLocalTrackId, track_id); + VerifyVoiceSenderInfoReport(track_report, voice_sender_info); + + // Get stats for the remote track. + stats.GetStats(remote_track.get(), &reports); + track_report = FindNthReportByType(reports, + StatsReport::kStatsReportTypeSsrc, 1); + EXPECT_TRUE(track_report); + track_id = ExtractSsrcStatsValue(reports, + StatsReport::kStatsValueNameTrackId); + EXPECT_EQ(kRemoteTrackId, track_id); + VerifyVoiceReceiverInfoReport(track_report, voice_receiver_info); +} + } // namespace diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc index d3bae91c4c..887aa4fb90 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -932,31 +932,18 @@ bool WebRtcSession::UpdateIce(PeerConnectionInterface::IceTransportsType type) { return false; } -bool WebRtcSession::GetTrackIdBySsrc(uint32 ssrc, std::string* id) { - if (GetLocalTrackId(ssrc, id)) { - if (GetRemoteTrackId(ssrc, id)) { - LOG(LS_WARNING) << "SSRC " << ssrc - << " exists in both local and remote descriptions"; - return true; // We return the remote track id. - } - return true; - } else { - return GetRemoteTrackId(ssrc, id); - } -} - -bool WebRtcSession::GetLocalTrackId(uint32 ssrc, std::string* track_id) { +bool WebRtcSession::GetLocalTrackIdBySsrc(uint32 ssrc, std::string* track_id) { if (!BaseSession::local_description()) return false; return webrtc::GetTrackIdBySsrc( - BaseSession::local_description(), ssrc, track_id); + BaseSession::local_description(), ssrc, track_id); } -bool WebRtcSession::GetRemoteTrackId(uint32 ssrc, std::string* track_id) { +bool WebRtcSession::GetRemoteTrackIdBySsrc(uint32 ssrc, std::string* track_id) { if (!BaseSession::remote_description()) - return false; + return false; return webrtc::GetTrackIdBySsrc( - BaseSession::remote_description(), ssrc, track_id); + BaseSession::remote_description(), ssrc, track_id); } std::string WebRtcSession::BadStateErrMsg(State state) { diff --git a/talk/app/webrtc/webrtcsession.h b/talk/app/webrtc/webrtcsession.h index 791e95455e..3ffea8ffac 100644 --- a/talk/app/webrtc/webrtcsession.h +++ b/talk/app/webrtc/webrtcsession.h @@ -166,7 +166,9 @@ class WebRtcSession : public cricket::BaseSession, } // Get the id used as a media stream track's "id" field from ssrc. - virtual bool GetTrackIdBySsrc(uint32 ssrc, std::string* id); + virtual bool GetLocalTrackIdBySsrc(uint32 ssrc, std::string* track_id); + virtual bool GetRemoteTrackIdBySsrc(uint32 ssrc, std::string* track_id); + // AudioMediaProviderInterface implementation. virtual void SetAudioPlayout(uint32 ssrc, bool enable, @@ -289,9 +291,6 @@ class WebRtcSession : public cricket::BaseSession, const cricket::ReceiveDataParams& params, const talk_base::Buffer& payload); - bool GetLocalTrackId(uint32 ssrc, std::string* track_id); - bool GetRemoteTrackId(uint32 ssrc, std::string* track_id); - std::string BadStateErrMsg(State state); void SetIceConnectionState(PeerConnectionInterface::IceConnectionState state); diff --git a/talk/libjingle_tests.gyp b/talk/libjingle_tests.gyp index 9fefa91aa2..00e2230f59 100755 --- a/talk/libjingle_tests.gyp +++ b/talk/libjingle_tests.gyp @@ -386,20 +386,25 @@ 'target_name': 'libjingle_peerconnection_unittest', 'type': 'executable', 'dependencies': [ + '<(DEPTH)/testing/gmock.gyp:gmock', 'gunit', 'libjingle.gyp:libjingle', 'libjingle.gyp:libjingle_p2p', 'libjingle.gyp:libjingle_peerconnection', 'libjingle_unittest_main', ], - # TODO(ronghuawu): Reenable below unit tests that require gmock. + 'direct_dependent_settings': { + 'include_dirs': [ + '<(DEPTH)/testing/gmock/include', + ], + }, 'sources': [ 'app/webrtc/datachannel_unittest.cc', 'app/webrtc/dtmfsender_unittest.cc', 'app/webrtc/jsepsessiondescription_unittest.cc', 'app/webrtc/localaudiosource_unittest.cc', - # 'app/webrtc/mediastream_unittest.cc', - # 'app/webrtc/mediastreamhandler_unittest.cc', + 'app/webrtc/mediastream_unittest.cc', + 'app/webrtc/mediastreamhandler_unittest.cc', 'app/webrtc/mediastreamsignaling_unittest.cc', 'app/webrtc/peerconnection_unittest.cc', 'app/webrtc/peerconnectionendtoend_unittest.cc', @@ -408,6 +413,7 @@ # 'app/webrtc/peerconnectionproxy_unittest.cc', 'app/webrtc/remotevideocapturer_unittest.cc', 'app/webrtc/sctputils.cc', + 'app/webrtc/statscollector_unittest.cc', 'app/webrtc/test/fakeaudiocapturemodule.cc', 'app/webrtc/test/fakeaudiocapturemodule.h', 'app/webrtc/test/fakeaudiocapturemodule_unittest.cc', @@ -426,6 +432,21 @@ 'app/webrtc/webrtcsdp_unittest.cc', 'app/webrtc/webrtcsession_unittest.cc', ], + 'conditions': [ + ['OS=="android"', { + # We want gmock features that use tr1::tuple, but we currently + # don't support the variadic templates used by libstdc++'s + # implementation. gmock supports this scenario by providing its + # own implementation but we must opt in to it. + 'defines': [ + 'GTEST_USE_OWN_TR1_TUPLE=1', + # GTEST_USE_OWN_TR1_TUPLE only works if GTEST_HAS_TR1_TUPLE is set. + # gmock r625 made it so that GTEST_HAS_TR1_TUPLE is set to 0 + # automatically on android, so it has to be set explicitly here. + 'GTEST_HAS_TR1_TUPLE=1', + ], + }], + ], }, # target libjingle_peerconnection_unittest ], 'conditions': [ diff --git a/tools/valgrind-webrtc/memcheck/suppressions.txt b/tools/valgrind-webrtc/memcheck/suppressions.txt index 509535d6ff..2a3640e396 100644 --- a/tools/valgrind-webrtc/memcheck/suppressions.txt +++ b/tools/valgrind-webrtc/memcheck/suppressions.txt @@ -698,3 +698,17 @@ fun:_ZN21VideoMediaChannelTestIN7cricket17WebRtcVideoEngineENS0_23WebRtcVideoMediaChannelEE36TwoStreamsSendAndFailUnsignalledRecvERKNS0_10VideoCodecE fun:_ZN69WebRtcVideoMediaChannelTest_TwoStreamsSendAndFailUnsignalledRecv_Test8TestBodyEv } +{ + bug_3478 + Memcheck:Leak + fun:_Znw* + fun:_ZNK9talk_base18FakeSSLCertificate12GetReferenceEv + fun:_ZN9talk_base18FakeSSLCertificate7DupCertES0_ + fun:_ZSt9transformIN9__gnu_cxx17__normal_iteratorIPKN9talk_base18FakeSSLCertificateESt6vectorIS3_SaIS3_EEEENS1_IPPNS2_14SSLCertificateES6_ISB_SaISB_EEEEPFPS3_S3_EET0_T_SK_SJ_T1_ + fun:_ZNK9talk_base18FakeSSLCertificate8GetChainEPPNS_12SSLCertChainE + fun:_ZN6webrtc14StatsCollector21AddCertificateReportsEPKN9talk_base14SSLCertificateE + fun:_ZN6webrtc14StatsCollector18ExtractSessionInfoEv + fun:_ZN6webrtc14StatsCollector11UpdateStatsENS_23PeerConnectionInterface16StatsOutputLevelE + fun:_ZN12_GLOBAL__N_118StatsCollectorTest22TestCertificateReportsERKN9talk_base18FakeSSLCertificateERKSt6vectorISsSaISsEES4_S9_ + fun:_ZN12_GLOBAL__N_156StatsCollectorTest_ChainedCertificateReportsCreated_Test8TestBodyEv +}