admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[Unified Plan] Fix old GetStats() not associating track id

Browse Source 
The method for looking up track ID by SSRC was never updated for
Unified Plan so it only looked at the first audio section and the
first video section.

This CL changes the method to look through all audio and video
media sections rather than just the first.

Bug: chromium:906988
Change-Id: Ie79e6162b2bd24b8ac9e983b5fa7360c96f030da
Reviewed-on: https://webrtc-review.googlesource.com/c/112223
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Seth Hampson <shampson@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25833}
This commit is contained in:
Steve Anton 2018-11-28 11:15:33 -08:00 committed by Commit Bot
parent c64078fdc0
commit a41959e550
9 changed files with 101 additions and 83 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

65
pc/peerconnection.cc
View File

@ -518,33 +518,6 @@ bool VerifyIceUfragPwdPresent(const SessionDescription* desc) {
return true;
}
bool GetTrackIdBySsrc(const SessionDescription* session_description,
uint32_t ssrc,
std::string* track_id) {
RTC_DCHECK(track_id != NULL);
const cricket::AudioContentDescription* audio_desc =
cricket::GetFirstAudioContentDescription(session_description);
if (audio_desc) {
const auto* found = cricket::GetStreamBySsrc(audio_desc->streams(), ssrc);
if (found) {
*track_id = found->id;
return true;
}
}
const cricket::VideoContentDescription* video_desc =
cricket::GetFirstVideoContentDescription(session_description);
if (video_desc) {
const auto* found = cricket::GetStreamBySsrc(video_desc->streams(), ssrc);
if (found) {
*track_id = found->id;
return true;
}
}
return false;
}
// Get the SCTP port out of a SessionDescription.
// Return -1 if not found.
int GetSctpPort(const SessionDescription* session_description) {
@ -5246,22 +5219,34 @@ cricket::IceConfig PeerConnection::ParseIceConfig(
return ice_config;
}
bool PeerConnection::GetLocalTrackIdBySsrc(uint32_t ssrc,
std::string* track_id) {
if (!local_description()) {
return false;
static absl::string_view GetTrackIdBySsrc(
const SessionDescriptionInterface* session_description,
uint32_t ssrc) {
if (!session_description) {
return {};
}
return webrtc::GetTrackIdBySsrc(local_description()->description(), ssrc,
track_id);
for (const cricket::ContentInfo& content :
session_description->description()->contents()) {
const cricket::MediaContentDescription& media =
*content.media_description();
if (media.type() == cricket::MEDIA_TYPE_AUDIO ||
media.type() == cricket::MEDIA_TYPE_VIDEO) {
const cricket::StreamParams* stream_params =
cricket::GetStreamBySsrc(media.streams(), ssrc);
if (stream_params) {
return stream_params->id;
}
}
}
return {};
}
bool PeerConnection::GetRemoteTrackIdBySsrc(uint32_t ssrc,
std::string* track_id) {
if (!remote_description()) {
return false;
}
return webrtc::GetTrackIdBySsrc(remote_description()->description(), ssrc,
track_id);
absl::string_view PeerConnection::GetLocalTrackIdBySsrc(uint32_t ssrc) {
return GetTrackIdBySsrc(local_description(), ssrc);
}
absl::string_view PeerConnection::GetRemoteTrackIdBySsrc(uint32_t ssrc) {
return GetTrackIdBySsrc(remote_description(), ssrc);
}
bool PeerConnection::SendData(const cricket::SendDataParams& params,

4
pc/peerconnection.h
View File

@ -228,8 +228,8 @@ class PeerConnection : public PeerConnectionInternal,
return transceivers_;
}
bool GetLocalTrackIdBySsrc(uint32_t ssrc, std::string* track_id) override;
bool GetRemoteTrackIdBySsrc(uint32_t ssrc, std::string* track_id) override;
absl::string_view GetLocalTrackIdBySsrc(uint32_t ssrc) override;
absl::string_view GetRemoteTrackIdBySsrc(uint32_t ssrc) override;
sigslot::signal1<DataChannel*>& SignalDataChannelCreated() override {
return SignalDataChannelCreated_;

29
pc/peerconnection_integrationtest.cc
View File

@ -78,6 +78,7 @@ using cricket::StreamParams;
using rtc::SocketAddress;
using ::testing::Combine;
using ::testing::ElementsAre;
using ::testing::UnorderedElementsAreArray;
using ::testing::Return;
using ::testing::SetArgPointee;
using ::testing::Values;
@ -2616,6 +2617,34 @@ TEST_P(PeerConnectionIntegrationTest, GetCaptureStartNtpTimeWithOldStatsApi) {
2 * kMaxWaitForFramesMs);
}
// Test that the track ID is associated with all local and remote SSRC stats
// using the old GetStats() and more than 1 audio and more than 1 video track.
// This is a regression test for crbug.com/906988
TEST_F(PeerConnectionIntegrationTestUnifiedPlan,
OldGetStatsAssociatesTrackIdForManyMediaSections) {
ASSERT_TRUE(CreatePeerConnectionWrappers());
ConnectFakeSignaling();
auto audio_sender_1 = caller()->AddAudioTrack();
auto video_sender_1 = caller()->AddVideoTrack();
auto audio_sender_2 = caller()->AddAudioTrack();
auto video_sender_2 = caller()->AddVideoTrack();
caller()->CreateAndSetAndSignalOffer();
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
MediaExpectations media_expectations;
media_expectations.CalleeExpectsSomeAudioAndVideo();
ASSERT_TRUE_WAIT(ExpectNewFrames(media_expectations), kDefaultTimeout);
std::vector<std::string> track_ids = {
audio_sender_1->track()->id(), video_sender_1->track()->id(),
audio_sender_2->track()->id(), video_sender_2->track()->id()};
auto caller_stats = caller()->OldGetStats();
EXPECT_THAT(caller_stats->TrackIds(), UnorderedElementsAreArray(track_ids));
auto callee_stats = callee()->OldGetStats();
EXPECT_THAT(callee_stats->TrackIds(), UnorderedElementsAreArray(track_ids));
}
// Test that we can get stats (using the new stats implemnetation) for
// unsignaled streams. Meaning when SSRCs/MSIDs aren't signaled explicitly in
// SDP.

4
pc/peerconnectioninternal.h
View File

@ -42,8 +42,8 @@ class PeerConnectionInternal : public PeerConnectionInterface {
GetTransceiversInternal() const = 0;
// Get the id used as a media stream track's "id" field from ssrc.
virtual bool GetLocalTrackIdBySsrc(uint32_t ssrc, std::string* track_id) = 0;
virtual bool GetRemoteTrackIdBySsrc(uint32_t ssrc, std::string* track_id) = 0;
virtual absl::string_view GetLocalTrackIdBySsrc(uint32_t ssrc) = 0;
virtual absl::string_view GetRemoteTrackIdBySsrc(uint32_t ssrc) = 0;
virtual sigslot::signal1<DataChannel*>& SignalDataChannelCreated() = 0;

43
pc/statscollector.cc
View File

@ -586,28 +586,31 @@ StatsReport* StatsCollector::PrepareReport(bool local,
StatsReport* report = reports_.Find(id);
// 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)) {
absl::string_view track_id = GetTrackIdBySsrc(ssrc, direction);
if (track_id.empty()) {
// The SSRC is not used by any existing track (or lookup failed since the
// SSRC wasn't signaled in SDP). Try copying the track ID from a previous
// report: if one exists.
if (report) {
const StatsReport::Value* v =
report->FindValue(StatsReport::kStatsValueNameTrackId);
if (v)
if (v) {
track_id = v->string_val();
}
}
}
if (!report)
if (!report) {
report = reports_.InsertNew(id);
}
// FYI - for remote reports, the timestamp will be overwritten later.
report->set_timestamp(stats_gathering_started_);
report->AddInt64(StatsReport::kStatsValueNameSsrc, ssrc);
if (!track_id.empty()) {
report->AddString(StatsReport::kStatsValueNameTrackId, track_id);
report->AddString(StatsReport::kStatsValueNameTrackId,
std::string(track_id));
}
// Add the mapping of SSRC to transport.
report->AddId(StatsReport::kStatsValueNameTransportId, transport_id);
@ -1095,26 +1098,24 @@ void StatsCollector::UpdateReportFromAudioTrack(AudioTrackInterface* track,
}
}
bool StatsCollector::GetTrackIdBySsrc(uint32_t ssrc,
std::string* track_id,
StatsReport::Direction direction) {
absl::string_view StatsCollector::GetTrackIdBySsrc(
uint32_t ssrc,
StatsReport::Direction direction) {
RTC_DCHECK(pc_->signaling_thread()->IsCurrent());
absl::string_view track_id;
if (direction == StatsReport::kSend) {
if (!pc_->GetLocalTrackIdBySsrc(ssrc, track_id)) {
RTC_LOG(LS_WARNING) << "The SSRC " << ssrc
<< " is not associated with a sending track";
return false;
}
track_id = pc_->GetLocalTrackIdBySsrc(ssrc);
} else {
RTC_DCHECK(direction == StatsReport::kReceive);
if (!pc_->GetRemoteTrackIdBySsrc(ssrc, track_id)) {
RTC_LOG(LS_WARNING) << "The SSRC " << ssrc
<< " is not associated with a receiving track";
return false;
}
RTC_DCHECK_EQ(direction, StatsReport::kReceive);
track_id = pc_->GetRemoteTrackIdBySsrc(ssrc);
}
return true;
if (track_id.empty()) {
RTC_LOG(LS_WARNING) << "The SSRC " << ssrc << " is not associated with a "
<< (direction == StatsReport::kSend ? "sending"
: "receiving")
<< " track";
}
return track_id;
}
void StatsCollector::UpdateTrackReports() {

5
pc/statscollector.h
View File

@ -132,9 +132,8 @@ 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_t ssrc,
std::string* track_id,
StatsReport::Direction direction);
absl::string_view GetTrackIdBySsrc(uint32_t ssrc,
StatsReport::Direction direction);
// Helper method to update the timestamp of track records.
void UpdateTrackReports();

8
pc/test/fakepeerconnectionbase.h
View File

@ -229,11 +229,9 @@ class FakePeerConnectionBase : public PeerConnectionInternal {
return {};
}
bool GetLocalTrackIdBySsrc(uint32_t ssrc, std::string* track_id) override {
return false;
}
bool GetRemoteTrackIdBySsrc(uint32_t ssrc, std::string* track_id) override {
return false;
absl::string_view GetLocalTrackIdBySsrc(uint32_t ssrc) override { return {}; }
absl::string_view GetRemoteTrackIdBySsrc(uint32_t ssrc) override {
return {};
}
sigslot::signal1<DataChannel*>& SignalDataChannelCreated() override {

16
pc/test/fakepeerconnectionforstats.h
View File

@ -249,24 +249,20 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase {
return transceivers_;
}
bool GetLocalTrackIdBySsrc(uint32_t ssrc, std::string* track_id) override {
absl::string_view GetLocalTrackIdBySsrc(uint32_t ssrc) override {
auto it = local_track_id_by_ssrc_.find(ssrc);
if (it != local_track_id_by_ssrc_.end()) {
*track_id = it->second;
return true;
} else {
return false;
return it->second;
}
return {};
}
bool GetRemoteTrackIdBySsrc(uint32_t ssrc, std::string* track_id) override {
absl::string_view GetRemoteTrackIdBySsrc(uint32_t ssrc) override {
auto it = remote_track_id_by_ssrc_.find(ssrc);
if (it != remote_track_id_by_ssrc_.end()) {
*track_id = it->second;
return true;
} else {
return false;
return it->second;
}
return {};
}
std::vector<rtc::scoped_refptr<DataChannel>> sctp_data_channels()

10
pc/test/mockpeerconnectionobservers.h
View File

@ -366,6 +366,9 @@ class MockStatsObserver : public webrtc::StatsObserver {
&stats_.bytes_sent);
GetInt64Value(r, StatsReport::kStatsValueNameCaptureStartNtpTimeMs,
&stats_.capture_start_ntp_time);
stats_.track_ids.emplace_back();
GetStringValue(r, StatsReport::kStatsValueNameTrackId,
&stats_.track_ids.back());
} else if (r->type() == StatsReport::kStatsReportTypeBwe) {
stats_.timestamp = r->timestamp();
GetIntValue(r, StatsReport::kStatsValueNameAvailableReceiveBandwidth,
@ -424,6 +427,11 @@ class MockStatsObserver : public webrtc::StatsObserver {
return stats_.srtp_cipher;
}
std::vector<std::string> TrackIds() const {
RTC_CHECK(called_);
return stats_.track_ids;
}
private:
bool GetIntValue(const StatsReport* report,
StatsReport::StatsValueName name,
@ -469,6 +477,7 @@ class MockStatsObserver : public webrtc::StatsObserver {
available_receive_bandwidth = 0;
dtls_cipher.clear();
srtp_cipher.clear();
track_ids.clear();
}
size_t number_of_reports;
@ -481,6 +490,7 @@ class MockStatsObserver : public webrtc::StatsObserver {
int available_receive_bandwidth;
std::string dtls_cipher;
std::string srtp_cipher;
std::vector<std::string> track_ids;
} stats_;
};