generic-library/webrtc
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Fixed the stats problem when new track is using the same ssrc as the previous track.

Browse Source 
Before this patch, when switching from voice mode to stereo mode, the stats won't be updated because StatsCollector binded the ssrc report with the old track, so the report can't be updated by the new track.
This patch fixes the porblem by changing the ssrc report track id to use the new track id.

TEST=libjingle_peerconnection_unittest --gtest_filter="*StatsCollectorTest*"
R=hta@chromium.org

Review URL: https://webrtc-codereview.appspot.com/17859004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@6632 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
xians@webrtc.org 2014-07-09 07:38:38 +00:00
parent b753762ce6
commit 01bda2068b
3 changed files with 188 additions and 117 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

72
talk/app/webrtc/statscollector.cc
View File

@ -270,17 +270,20 @@ bool ExtractValueFromReport(
return false;
}
void AddTrackReport(StatsMap* reports, const std::string& track_id) {
// Adds an empty track report.
StatsReport report;
report.type = StatsReport::kStatsReportTypeTrack;
report.id = StatsId(StatsReport::kStatsReportTypeTrack, track_id);
report.AddValue(StatsReport::kStatsValueNameTrackId, track_id);
(*reports)[report.id] = report;
}
template <class TrackVector>
void CreateTrackReports(const TrackVector& tracks, StatsMap* reports) {
for (size_t j = 0; j < tracks.size(); ++j) {
webrtc::MediaStreamTrackInterface* track = tracks[j];
// Adds an empty track report.
StatsReport report;
report.type = StatsReport::kStatsReportTypeTrack;
report.id = StatsId(StatsReport::kStatsReportTypeTrack, track->id());
report.AddValue(StatsReport::kStatsValueNameTrackId,
track->id());
(*reports)[report.id] = report;
AddTrackReport(reports, track->id());
}
}
@ -543,13 +546,19 @@ void StatsCollector::AddStream(MediaStreamInterface* stream) {
void StatsCollector::AddLocalAudioTrack(AudioTrackInterface* audio_track,
uint32 ssrc) {
ASSERT(audio_track != NULL);
#ifdef _DEBUG
for (LocalAudioTrackVector::iterator it = local_audio_tracks_.begin();
it != local_audio_tracks_.end(); ++it) {
ASSERT(it->first != audio_track || it->second != ssrc);
}
#endif
local_audio_tracks_.push_back(std::make_pair(audio_track, ssrc));
// Create the kStatsReportTypeTrack report for the new track if there is no
// report yet.
StatsMap::iterator it = reports_.find(
StatsId(StatsReport::kStatsReportTypeTrack, audio_track->id()));
if (it == reports_.end())
AddTrackReport(&reports_, audio_track->id());
}
void StatsCollector::RemoveLocalAudioTrack(AudioTrackInterface* audio_track,
@ -617,7 +626,8 @@ StatsCollector::UpdateStats(PeerConnectionInterface::StatsOutputLevel level) {
// Calls to UpdateStats() that occur less than kMinGatherStatsPeriod number of
// ms apart will be ignored.
const double kMinGatherStatsPeriod = 50;
if (stats_gathering_started_ + kMinGatherStatsPeriod > time_now) {
if (stats_gathering_started_ != 0 &&
stats_gathering_started_ + kMinGatherStatsPeriod > time_now) {
return;
}
stats_gathering_started_ = time_now;
@ -637,13 +647,17 @@ StatsReport* StatsCollector::PrepareLocalReport(
StatsMap::iterator it = reports_.find(StatsId(
StatsReport::kStatsReportTypeSsrc, ssrc_id, direction));
// Use the ID of the track that is currently mapped to the SSRC, if any.
std::string track_id;
if (it == reports_.end()) {
if (!GetTrackIdBySsrc(ssrc, &track_id, direction))
if (!GetTrackIdBySsrc(ssrc, &track_id, direction)) {
if (it == reports_.end()) {
// 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;
} else {
// Keeps the old track id since we want to report the stats for inactive
// tracks.
}
// 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(it->second,
StatsReport::kStatsValueNameTrackId,
&track_id);
@ -653,10 +667,12 @@ StatsReport* StatsCollector::PrepareLocalReport(
ssrc_id, direction);
// Clear out stats from previous GatherStats calls if any.
if (report->timestamp != stats_gathering_started_) {
report->values.clear();
report->timestamp = stats_gathering_started_;
}
// 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->values.clear();
report->timestamp = stats_gathering_started_;
report->AddValue(StatsReport::kStatsValueNameSsrc, ssrc_id);
report->AddValue(StatsReport::kStatsValueNameTrackId, track_id);
@ -674,13 +690,17 @@ StatsReport* StatsCollector::PrepareRemoteReport(
StatsMap::iterator it = 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 (it == reports_.end()) {
if (!GetTrackIdBySsrc(ssrc, &track_id, direction))
if (!GetTrackIdBySsrc(ssrc, &track_id, direction)) {
if (it == reports_.end()) {
// 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;
} else {
// Keeps the old track id since we want to report the stats for inactive
// tracks.
}
// 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(it->second,
StatsReport::kStatsValueNameTrackId,
&track_id);
@ -1060,4 +1080,8 @@ bool StatsCollector::GetTrackIdBySsrc(uint32 ssrc, std::string* track_id,
return true;
}
void StatsCollector::ClearUpdateStatsCache() {
stats_gathering_started_ = 0;
}
} // namespace webrtc

5
talk/app/webrtc/statscollector.h
View File

@ -89,6 +89,11 @@ class StatsCollector {
bool GetTransportIdFromProxy(const std::string& proxy,
std::string* transport_id);
// Method used by the unittest to force a update of stats since UpdateStats()
// that occur less than kMinGatherStatsPeriod number of ms apart will be
// ignored.
void ClearUpdateStatsCache();
private:
bool CopySelectedReports(const std::string& selector, StatsReports* reports);

228
talk/app/webrtc/statscollector_unittest.cc
View File

@ -465,8 +465,6 @@ class StatsCollectorTest : public testing::Test {
stream_->AddTrack(track_);
EXPECT_CALL(session_, GetLocalTrackIdBySsrc(kSsrcOfTrack, _))
.WillRepeatedly(DoAll(SetArgPointee<1>(kLocalTrackId), Return(true)));
EXPECT_CALL(session_, GetRemoteTrackIdBySsrc(kSsrcOfTrack, _))
.WillRepeatedly(Return(false));
}
// Adds a incoming video track with a given SSRC into the stats.
@ -474,11 +472,8 @@ class StatsCollectorTest : public testing::Test {
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)));
.WillRepeatedly(DoAll(SetArgPointee<1>(kRemoteTrackId), Return(true)));
}
// Adds a outgoing audio track with a given SSRC into the stats.
@ -490,10 +485,7 @@ class StatsCollectorTest : public testing::Test {
kLocalTrackId);
stream_->AddTrack(audio_track_);
EXPECT_CALL(session_, GetLocalTrackIdBySsrc(kSsrcOfTrack, _))
.WillRepeatedly(DoAll(SetArgPointee<1>(kLocalTrackId),
Return(true)));
EXPECT_CALL(session_, GetRemoteTrackIdBySsrc(kSsrcOfTrack, _))
.WillRepeatedly(Return(false));
.WillOnce(DoAll(SetArgPointee<1>(kLocalTrackId), Return(true)));
}
// Adds a incoming audio track with a given SSRC into the stats.
@ -504,10 +496,84 @@ class StatsCollectorTest : public testing::Test {
audio_track_ = new talk_base::RefCountedObject<FakeAudioTrack>(
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)));
.WillOnce(DoAll(SetArgPointee<1>(kRemoteTrackId), Return(true)));
}
void SetupAndVerifyAudioTrackStats(
FakeAudioTrack* audio_track,
webrtc::MediaStream* stream,
webrtc::StatsCollector* stats,
cricket::VoiceChannel* voice_channel,
const std::string& vc_name,
MockVoiceMediaChannel* media_channel,
cricket::VoiceSenderInfo* voice_sender_info,
cricket::VoiceReceiverInfo* voice_receiver_info,
cricket::VoiceMediaInfo* stats_read,
StatsReports* reports) {
// A track can't have both sender report and recv report at the same time
// for now, this might change in the future though.
ASSERT((voice_sender_info == NULL) ^ (voice_receiver_info == NULL));
stats->set_session(&session_);
// Instruct the session to return stats containing the transport channel.
InitSessionStats(vc_name);
EXPECT_CALL(session_, GetStats(_))
.WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_),
Return(true)));
// Constructs an ssrc stats update.
if (voice_sender_info)
stats_read->senders.push_back(*voice_sender_info);
if (voice_receiver_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(_))
.WillOnce(DoAll(SetArgPointee<0>(*stats_read), Return(true)));
stats->UpdateStats(PeerConnectionInterface::kStatsOutputLevelStandard);
stats->ClearUpdateStatsCache();
stats->GetStats(NULL, reports);
// Verify the existence of the track report.
const StatsReport* report = FindNthReportByType(
*reports, StatsReport::kStatsReportTypeSsrc, 1);
EXPECT_FALSE(report == NULL);
std::string track_id = ExtractSsrcStatsValue(
*reports, StatsReport::kStatsValueNameTrackId);
EXPECT_EQ(audio_track->id(), track_id);
std::string ssrc_id = ExtractSsrcStatsValue(
*reports, StatsReport::kStatsValueNameSsrc);
EXPECT_EQ(talk_base::ToString<uint32>(kSsrcOfTrack), ssrc_id);
// Verifies the values in the track report.
if (voice_sender_info) {
UpdateVoiceSenderInfoFromAudioTrack(audio_track, voice_sender_info);
VerifyVoiceSenderInfoReport(report, *voice_sender_info);
}
if (voice_receiver_info) {
VerifyVoiceReceiverInfoReport(report, *voice_receiver_info);
}
// Verify we get the same result by passing a track to GetStats().
StatsReports track_reports; // returned values.
stats->GetStats(audio_track, &track_reports);
const StatsReport* track_report = FindNthReportByType(
track_reports, StatsReport::kStatsReportTypeSsrc, 1);
EXPECT_TRUE(track_report);
track_id = ExtractSsrcStatsValue(track_reports,
StatsReport::kStatsValueNameTrackId);
EXPECT_EQ(audio_track->id(), track_id);
ssrc_id = ExtractSsrcStatsValue(track_reports,
StatsReport::kStatsValueNameSsrc);
EXPECT_EQ(talk_base::ToString<uint32>(kSsrcOfTrack), ssrc_id);
if (voice_sender_info)
VerifyVoiceSenderInfoReport(track_report, *voice_sender_info);
if (voice_receiver_info)
VerifyVoiceReceiverInfoReport(track_report, *voice_receiver_info);
}
void TestCertificateReports(const talk_base::FakeSSLCertificate& local_cert,
@ -1171,61 +1237,16 @@ TEST_F(StatsCollectorTest, GetStatsFromLocalAudioTrack) {
media_engine_, media_channel, &session_, kVcName, false);
AddOutgoingAudioTrackStats();
stats.AddStream(stream_);
stats.AddLocalAudioTrack(audio_track_.get(), kSsrcOfTrack);
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)));
stats.AddLocalAudioTrack(audio_track_, kSsrcOfTrack);
cricket::VoiceSenderInfo voice_sender_info;
InitVoiceSenderInfo(&voice_sender_info);
// Constructs an ssrc stats update.
cricket::VoiceMediaInfo stats_read;
stats_read.senders.push_back(voice_sender_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);
stats.GetStats(NULL, &reports);
// Verfy the existence of the track report.
const StatsReport* report = FindNthReportByType(
reports, StatsReport::kStatsReportTypeSsrc, 1);
EXPECT_FALSE(report == NULL);
std::string track_id = ExtractSsrcStatsValue(
reports, StatsReport::kStatsValueNameTrackId);
EXPECT_EQ(kLocalTrackId, track_id);
std::string ssrc_id = ExtractSsrcStatsValue(
reports, StatsReport::kStatsValueNameSsrc);
EXPECT_EQ(talk_base::ToString<uint32>(kSsrcOfTrack), ssrc_id);
// Verifies the values in the track report.
UpdateVoiceSenderInfoFromAudioTrack(audio_track_.get(), &voice_sender_info);
VerifyVoiceSenderInfoReport(report, voice_sender_info);
// Verify we get the same result by passing a track to GetStats().
StatsReports track_reports; // returned values.
stats.GetStats(audio_track_.get(), &track_reports);
const StatsReport* track_report = FindNthReportByType(
track_reports, StatsReport::kStatsReportTypeSsrc, 1);
EXPECT_TRUE(track_report);
track_id = ExtractSsrcStatsValue(track_reports,
StatsReport::kStatsValueNameTrackId);
EXPECT_EQ(kLocalTrackId, track_id);
ssrc_id = ExtractSsrcStatsValue(track_reports,
StatsReport::kStatsValueNameSsrc);
EXPECT_EQ(talk_base::ToString<uint32>(kSsrcOfTrack), ssrc_id);
VerifyVoiceSenderInfoReport(track_report, voice_sender_info);
SetupAndVerifyAudioTrackStats(
audio_track_.get(), stream_.get(), &stats, &voice_channel, kVcName,
media_channel, &voice_sender_info, NULL, &stats_read, &reports);
// Verify that there is no remote report for the local audio track because
// we did not set it up.
@ -1246,42 +1267,15 @@ TEST_F(StatsCollectorTest, GetStatsFromRemoteStream) {
AddIncomingAudioTrackStats();
stats.AddStream(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::VoiceReceiverInfo voice_receiver_info;
InitVoiceReceiverInfo(&voice_receiver_info);
voice_receiver_info.codec_name = "fake_codec";
// Constructs an ssrc stats update.
cricket::VoiceMediaInfo stats_read;
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);
stats.GetStats(NULL, &reports);
// 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);
SetupAndVerifyAudioTrackStats(
audio_track_.get(), stream_.get(), &stats, &voice_channel, kVcName,
media_channel, NULL, &voice_receiver_info, &stats_read, &reports);
}
// This test verifies that a local stats object won't update its statistics
@ -1361,7 +1355,7 @@ TEST_F(StatsCollectorTest, LocalAndRemoteTracksWithSameSsrc) {
talk_base::scoped_refptr<FakeAudioTrack> remote_track(
new talk_base::RefCountedObject<FakeAudioTrack>(kRemoteTrackId));
EXPECT_CALL(session_, GetRemoteTrackIdBySsrc(kSsrcOfTrack, _))
.WillRepeatedly(DoAll(SetArgPointee<1>(kRemoteTrackId), Return(true)));
.WillOnce(DoAll(SetArgPointee<1>(kRemoteTrackId), Return(true)));
remote_stream->AddTrack(remote_track);
stats.AddStream(remote_stream);
@ -1418,4 +1412,52 @@ TEST_F(StatsCollectorTest, LocalAndRemoteTracksWithSameSsrc) {
VerifyVoiceReceiverInfoReport(track_report, voice_receiver_info);
}
// This test verifies that when two outgoing audio tracks are using the same
// ssrc at different times, they populate stats reports correctly.
// TODO(xians): Figure out if it is possible to encapsulate the setup and
// avoid duplication of code in test cases.
TEST_F(StatsCollectorTest, TwoLocalTracksWithSameSsrc) {
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_, kSsrcOfTrack);
cricket::VoiceSenderInfo voice_sender_info;
voice_sender_info.add_ssrc(kSsrcOfTrack);
cricket::VoiceMediaInfo stats_read;
StatsReports reports; // returned values.
SetupAndVerifyAudioTrackStats(
audio_track_.get(), stream_.get(), &stats, &voice_channel, kVcName,
media_channel, &voice_sender_info, NULL, &stats_read, &reports);
// Remove the previous audio track from the stream.
stream_->RemoveTrack(audio_track_.get());
stats.RemoveLocalAudioTrack(audio_track_.get(), kSsrcOfTrack);
// Create a new audio track and adds it to the stream and stats.
static const std::string kNewTrackId = "new_track_id";
talk_base::scoped_refptr<FakeAudioTrack> new_audio_track(
new talk_base::RefCountedObject<FakeAudioTrack>(kNewTrackId));
EXPECT_CALL(session_, GetLocalTrackIdBySsrc(kSsrcOfTrack, _))
.WillOnce(DoAll(SetArgPointee<1>(kNewTrackId), Return(true)));
stream_->AddTrack(new_audio_track);
stats.AddLocalAudioTrack(new_audio_track, kSsrcOfTrack);
stats.ClearUpdateStatsCache();
cricket::VoiceSenderInfo new_voice_sender_info;
InitVoiceSenderInfo(&new_voice_sender_info);
cricket::VoiceMediaInfo new_stats_read;
SetupAndVerifyAudioTrackStats(
new_audio_track.get(), stream_.get(), &stats, &voice_channel, kVcName,
media_channel, &new_voice_sender_info, NULL, &new_stats_read, &reports);
}
} // namespace