diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index bf501d534e..211c5a7073 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -5684,36 +5684,6 @@ cricket::IceConfig PeerConnection::ParseIceConfig( return ice_config; } -static absl::string_view GetTrackIdBySsrc( - const SessionDescriptionInterface* session_description, - uint32_t ssrc) { - if (!session_description) { - return {}; - } - 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 {}; -} - -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, const rtc::CopyOnWriteBuffer& payload, cricket::SendDataResult* result) { diff --git a/pc/peer_connection.h b/pc/peer_connection.h index 66e3f00b07..30155eaf07 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -237,9 +237,6 @@ class PeerConnection : public PeerConnectionInternal, return transceivers_; } - absl::string_view GetLocalTrackIdBySsrc(uint32_t ssrc) override; - absl::string_view GetRemoteTrackIdBySsrc(uint32_t ssrc) override; - sigslot::signal1& SignalDataChannelCreated() override { return SignalDataChannelCreated_; } diff --git a/pc/peer_connection_internal.h b/pc/peer_connection_internal.h index 2cfee76167..a51ba4b8f6 100644 --- a/pc/peer_connection_internal.h +++ b/pc/peer_connection_internal.h @@ -41,10 +41,6 @@ class PeerConnectionInternal : public PeerConnectionInterface { rtc::scoped_refptr>> GetTransceiversInternal() const = 0; - // Get the id used as a media stream track's "id" field from ssrc. - virtual absl::string_view GetLocalTrackIdBySsrc(uint32_t ssrc) = 0; - virtual absl::string_view GetRemoteTrackIdBySsrc(uint32_t ssrc) = 0; - virtual sigslot::signal1& SignalDataChannelCreated() = 0; // Only valid when using deprecated RTP data channels. diff --git a/pc/stats_collector.cc b/pc/stats_collector.cc index 6bfce676ca..e1930a1fc3 100644 --- a/pc/stats_collector.cc +++ b/pc/stats_collector.cc @@ -15,6 +15,7 @@ #include #include +#include "absl/memory/memory.h" #include "pc/channel.h" #include "pc/peer_connection.h" #include "rtc_base/checks.h" @@ -372,26 +373,56 @@ void ExtractRemoteStats(const cricket::MediaReceiverInfo& info, // TODO(hta): Extract some stats here. } +std::string GetTrackIdBySsrc( + uint32_t ssrc, + StatsReport::Direction direction, + const std::map& track_id_by_ssrc) { + auto it = track_id_by_ssrc.find(ssrc); + if (it != track_id_by_ssrc.end()) { + return it->second; + } + if (direction == StatsReport::kReceive) { + // If the track ID was not found, this might be an unsignaled receive + // SSRC, so try looking up by the special SSRC 0. + it = track_id_by_ssrc.find(0); + if (it != track_id_by_ssrc.end()) { + RTC_LOG(LS_INFO) << "Assuming SSRC=" << ssrc + << " is an unsignalled receive stream corresponding " + "to the RtpReceiver with track ID \"" + << it->second << "\"."; + return it->second; + } + } + RTC_LOG(LS_INFO) << "Missing track ID for " + << (direction == StatsReport::kSend ? "send" : "recv") + << " SSRC=" << ssrc << "."; + return ""; +} + // Template to extract stats from a data vector. // In order to use the template, the functions that are called from it, // ExtractStats and ExtractRemoteStats, must be defined and overloaded // for each type. template -void ExtractStatsFromList(const std::vector& data, - const StatsReport::Id& transport_id, - StatsCollector* collector, - StatsReport::Direction direction) { +void ExtractStatsFromList( + const std::vector& data, + const StatsReport::Id& transport_id, + StatsCollector* collector, + StatsReport::Direction direction, + const std::map& track_id_by_ssrc) { for (const auto& d : data) { uint32_t ssrc = d.ssrc(); + std::string track_id = GetTrackIdBySsrc(ssrc, direction, track_id_by_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->PrepareReport(true, ssrc, transport_id, direction); + collector->PrepareReport(true, ssrc, track_id, transport_id, direction); if (report) ExtractStats(d, report); if (!d.remote_stats.empty()) { - report = collector->PrepareReport(false, ssrc, transport_id, direction); + report = collector->PrepareReport(false, ssrc, track_id, transport_id, + direction); if (report) ExtractRemoteStats(d, report); } @@ -580,6 +611,7 @@ void StatsCollector::UpdateStats( StatsReport* StatsCollector::PrepareReport(bool local, uint32_t ssrc, + const std::string& track_id, const StatsReport::Id& transport_id, StatsReport::Direction direction) { RTC_DCHECK(pc_->signaling_thread()->IsCurrent()); @@ -588,22 +620,6 @@ StatsReport* StatsCollector::PrepareReport(bool local, : StatsReport::kStatsReportTypeRemoteSsrc, rtc::ToString(ssrc), direction)); StatsReport* report = reports_.Find(id); - - // Use the ID of the track that is currently mapped to the SSRC, if any. - 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) { - track_id = v->string_val(); - } - } - } - if (!report) { report = reports_.InsertNew(id); } @@ -613,8 +629,7 @@ StatsReport* StatsCollector::PrepareReport(bool local, report->AddInt64(StatsReport::kStatsValueNameSsrc, ssrc); if (!track_id.empty()) { - report->AddString(StatsReport::kStatsValueNameTrackId, - std::string(track_id)); + report->AddString(StatsReport::kStatsValueNameTrackId, track_id); } // Add the mapping of SSRC to transport. report->AddId(StatsReport::kStatsValueNameTransportId, transport_id); @@ -895,68 +910,140 @@ void StatsCollector::ExtractBweInfo() { namespace { -struct VoiceChannelStatsInfo { +class MediaChannelStatsGatherer { + public: + virtual ~MediaChannelStatsGatherer() = default; + + virtual bool GetStatsOnWorkerThread() = 0; + + virtual void ExtractStats(StatsCollector* collector) const = 0; + + virtual bool HasRemoteAudio() const = 0; + + std::string mid; std::string transport_name; - cricket::VoiceMediaChannel* voice_media_channel; + std::map sender_track_id_by_ssrc; + std::map receiver_track_id_by_ssrc; + + protected: + template + void ExtractSenderReceiverStats( + StatsCollector* collector, + const std::vector& receiver_data, + const std::vector& sender_data) const { + RTC_DCHECK(collector); + StatsReport::Id transport_id = StatsReport::NewComponentId( + transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP); + ExtractStatsFromList(receiver_data, transport_id, collector, + StatsReport::kReceive, receiver_track_id_by_ssrc); + ExtractStatsFromList(sender_data, transport_id, collector, + StatsReport::kSend, sender_track_id_by_ssrc); + } +}; + +class VoiceMediaChannelStatsGatherer final : public MediaChannelStatsGatherer { + public: + VoiceMediaChannelStatsGatherer( + cricket::VoiceMediaChannel* voice_media_channel) + : voice_media_channel_(voice_media_channel) { + RTC_DCHECK(voice_media_channel_); + } + + bool GetStatsOnWorkerThread() override { + return voice_media_channel_->GetStats(&voice_media_info); + } + + void ExtractStats(StatsCollector* collector) const override { + ExtractSenderReceiverStats(collector, voice_media_info.receivers, + voice_media_info.senders); + } + + bool HasRemoteAudio() const override { + return !voice_media_info.receivers.empty(); + } + + private: + cricket::VoiceMediaChannel* voice_media_channel_; cricket::VoiceMediaInfo voice_media_info; }; -struct VideoChannelStatsInfo { - std::string transport_name; - cricket::VideoMediaChannel* video_media_channel; +class VideoMediaChannelStatsGatherer final : public MediaChannelStatsGatherer { + public: + VideoMediaChannelStatsGatherer( + cricket::VideoMediaChannel* video_media_channel) + : video_media_channel_(video_media_channel) { + RTC_DCHECK(video_media_channel_); + } + + bool GetStatsOnWorkerThread() override { + return video_media_channel_->GetStats(&video_media_info); + } + + void ExtractStats(StatsCollector* collector) const override { + ExtractSenderReceiverStats(collector, video_media_info.receivers, + video_media_info.senders); + } + + bool HasRemoteAudio() const override { return false; } + + private: + cricket::VideoMediaChannel* video_media_channel_; cricket::VideoMediaInfo video_media_info; }; +std::unique_ptr CreateMediaChannelStatsGatherer( + cricket::MediaChannel* channel) { + RTC_DCHECK(channel); + if (channel->media_type() == cricket::MEDIA_TYPE_AUDIO) { + return absl::make_unique( + static_cast(channel)); + } else { + RTC_DCHECK_EQ(channel->media_type(), cricket::MEDIA_TYPE_VIDEO); + return absl::make_unique( + static_cast(channel)); + } +} + } // namespace void StatsCollector::ExtractMediaInfo() { RTC_DCHECK_RUN_ON(pc_->signaling_thread()); - std::vector voice_channel_infos; - std::vector video_channel_infos; + std::vector> gatherers; { rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls; for (const auto& transceiver : pc_->GetTransceiversInternal()) { - if (!transceiver->internal()->channel()) { + cricket::ChannelInterface* channel = transceiver->internal()->channel(); + if (!channel) { continue; } - cricket::MediaType media_type = transceiver->internal()->media_type(); - if (media_type == cricket::MEDIA_TYPE_AUDIO) { - auto* voice_channel = static_cast( - transceiver->internal()->channel()); - voice_channel_infos.emplace_back(); - VoiceChannelStatsInfo& info = voice_channel_infos.back(); - info.transport_name = voice_channel->transport_name(); - info.voice_media_channel = voice_channel->media_channel(); - } else { - RTC_DCHECK_EQ(media_type, cricket::MEDIA_TYPE_VIDEO); - auto* video_channel = static_cast( - transceiver->internal()->channel()); - video_channel_infos.emplace_back(); - VideoChannelStatsInfo& info = video_channel_infos.back(); - info.transport_name = video_channel->transport_name(); - info.video_media_channel = video_channel->media_channel(); + std::unique_ptr gatherer = + CreateMediaChannelStatsGatherer(channel->media_channel()); + gatherer->mid = channel->content_name(); + gatherer->transport_name = channel->transport_name(); + for (const auto& sender : transceiver->internal()->senders()) { + std::string track_id = (sender->track() ? sender->track()->id() : ""); + gatherer->sender_track_id_by_ssrc.insert( + std::make_pair(sender->ssrc(), track_id)); } + for (const auto& receiver : transceiver->internal()->receivers()) { + gatherer->receiver_track_id_by_ssrc.insert(std::make_pair( + receiver->internal()->ssrc(), receiver->track()->id())); + } + gatherers.push_back(std::move(gatherer)); } } pc_->worker_thread()->Invoke(RTC_FROM_HERE, [&] { rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls; - for (auto it = voice_channel_infos.begin(); it != voice_channel_infos.end(); + for (auto it = gatherers.begin(); it != gatherers.end(); /* incremented manually */) { - if (!it->voice_media_channel->GetStats(&it->voice_media_info)) { - RTC_LOG(LS_ERROR) << "Failed to get voice channel stats"; - it = voice_channel_infos.erase(it); - continue; - } - ++it; - } - for (auto it = video_channel_infos.begin(); it != video_channel_infos.end(); - /* incremented manually */) { - if (!it->video_media_channel->GetStats(&it->video_media_info)) { - RTC_LOG(LS_ERROR) << "Failed to get video channel stats"; - it = video_channel_infos.erase(it); + MediaChannelStatsGatherer* gatherer = it->get(); + if (!gatherer->GetStatsOnWorkerThread()) { + RTC_LOG(LS_ERROR) << "Failed to get media channel stats for mid=" + << gatherer->mid; + it = gatherers.erase(it); continue; } ++it; @@ -966,24 +1053,9 @@ void StatsCollector::ExtractMediaInfo() { rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls; bool has_remote_audio = false; - for (const auto& info : voice_channel_infos) { - StatsReport::Id transport_id = StatsReport::NewComponentId( - info.transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP); - ExtractStatsFromList(info.voice_media_info.receivers, transport_id, this, - StatsReport::kReceive); - ExtractStatsFromList(info.voice_media_info.senders, transport_id, this, - StatsReport::kSend); - if (!info.voice_media_info.receivers.empty()) { - has_remote_audio = true; - } - } - for (const auto& info : video_channel_infos) { - StatsReport::Id transport_id = StatsReport::NewComponentId( - info.transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP); - ExtractStatsFromList(info.video_media_info.receivers, transport_id, this, - StatsReport::kReceive); - ExtractStatsFromList(info.video_media_info.senders, transport_id, this, - StatsReport::kSend); + for (const auto& gatherer : gatherers) { + gatherer->ExtractStats(this); + has_remote_audio |= gatherer->HasRemoteAudio(); } UpdateStatsFromExistingLocalAudioTracks(has_remote_audio); @@ -1103,17 +1175,6 @@ void StatsCollector::UpdateReportFromAudioTrack(AudioTrackInterface* track, } } -absl::string_view StatsCollector::GetTrackIdBySsrc( - uint32_t ssrc, - StatsReport::Direction direction) { - RTC_DCHECK(pc_->signaling_thread()->IsCurrent()); - if (direction == StatsReport::kSend) { - return pc_->GetLocalTrackIdBySsrc(ssrc); - } - RTC_DCHECK_EQ(direction, StatsReport::kReceive); - return pc_->GetRemoteTrackIdBySsrc(ssrc); -} - void StatsCollector::UpdateTrackReports() { RTC_DCHECK(pc_->signaling_thread()->IsCurrent()); diff --git a/pc/stats_collector.h b/pc/stats_collector.h index d0be338d52..b3d5cb956f 100644 --- a/pc/stats_collector.h +++ b/pc/stats_collector.h @@ -79,6 +79,7 @@ class StatsCollector { // in the ExtractStatsFromList template. StatsReport* PrepareReport(bool local, uint32_t ssrc, + const std::string& track_id, const StatsReport::Id& transport_id, StatsReport::Direction direction); @@ -130,11 +131,6 @@ class StatsCollector { StatsReport* report, bool has_remote_tracks); - // Helper method to get the id for the track identified by ssrc. - // |direction| tells if the track is for sending or receiving. - absl::string_view GetTrackIdBySsrc(uint32_t ssrc, - StatsReport::Direction direction); - // Helper method to update the timestamp of track records. void UpdateTrackReports(); diff --git a/pc/stats_collector_unittest.cc b/pc/stats_collector_unittest.cc index eed30ef1d1..7ebac7f598 100644 --- a/pc/stats_collector_unittest.cc +++ b/pc/stats_collector_unittest.cc @@ -27,6 +27,8 @@ #include "pc/stats_collector.h" #include "pc/test/fake_peer_connection_for_stats.h" #include "pc/test/fake_video_track_source.h" +#include "pc/test/mock_rtp_receiver_internal.h" +#include "pc/test/mock_rtp_sender_internal.h" #include "pc/transport_stats.h" #include "pc/video_track.h" #include "rtc_base/fake_ssl_identity.h" @@ -51,6 +53,8 @@ using cricket::VideoSenderInfo; using cricket::VoiceMediaInfo; using cricket::VoiceReceiverInfo; using cricket::VoiceSenderInfo; +using ::testing::Return; +using ::testing::UnorderedElementsAre; namespace webrtc { @@ -208,6 +212,38 @@ const StatsReport* FindNthReportByType(const StatsReports& reports, return nullptr; } +// Returns the value of the stat identified by |name| in the |n|-th report of +// type |type| in |reports|. +// |n| starts from 1 for finding the first report. +// If either the |n|-th report is not found, or the stat is not present in that +// report, then nullopt is returned. +absl::optional GetValueInNthReportByType( + const StatsReports& reports, + StatsReport::StatsType type, + StatsReport::StatsValueName name, + int n) { + const StatsReport* report = FindNthReportByType(reports, type, n); + if (!report) { + return absl::nullopt; + } + std::string value; + if (!GetValue(report, name, &value)) { + return absl::nullopt; + } + return value; +} + +std::vector GetReportsByType(const StatsReports& reports, + StatsReport::StatsType type) { + std::vector filtered_reports; + for (const StatsReport* report : reports) { + if (report->type() == type) { + filtered_reports.push_back(report); + } + } + return filtered_reports; +} + const StatsReport* FindReportById(const StatsReports& reports, const StatsReport::Id& id) { for (const auto* r : reports) { @@ -484,8 +520,9 @@ void VerifyVoiceSenderInfoReport(const StatsReport* report, } // Helper methods to avoid duplication of code. -void InitVoiceSenderInfo(cricket::VoiceSenderInfo* voice_sender_info) { - voice_sender_info->add_ssrc(kSsrcOfTrack); +void InitVoiceSenderInfo(cricket::VoiceSenderInfo* voice_sender_info, + uint32_t ssrc = kSsrcOfTrack) { + voice_sender_info->add_ssrc(ssrc); voice_sender_info->codec_name = "fake_codec"; voice_sender_info->bytes_sent = 100; voice_sender_info->packets_sent = 101; @@ -692,6 +729,36 @@ class StatsCollectorTest : public testing::Test { } }; +static rtc::scoped_refptr CreateMockSender( + rtc::scoped_refptr track, + uint32_t ssrc) { + rtc::scoped_refptr sender( + new rtc::RefCountedObject()); + EXPECT_CALL(*sender, track()).WillRepeatedly(Return(track)); + EXPECT_CALL(*sender, ssrc()).WillRepeatedly(Return(ssrc)); + EXPECT_CALL(*sender, media_type()) + .WillRepeatedly( + Return(track->kind() == MediaStreamTrackInterface::kAudioKind + ? cricket::MEDIA_TYPE_AUDIO + : cricket::MEDIA_TYPE_VIDEO)); + return sender; +} + +static rtc::scoped_refptr CreateMockReceiver( + rtc::scoped_refptr track, + uint32_t ssrc) { + rtc::scoped_refptr receiver( + new rtc::RefCountedObject()); + EXPECT_CALL(*receiver, track()).WillRepeatedly(Return(track)); + EXPECT_CALL(*receiver, ssrc()).WillRepeatedly(Return(ssrc)); + EXPECT_CALL(*receiver, media_type()) + .WillRepeatedly( + Return(track->kind() == MediaStreamTrackInterface::kAudioKind + ? cricket::MEDIA_TYPE_AUDIO + : cricket::MEDIA_TYPE_VIDEO)); + return receiver; +} + class StatsCollectorTrackTest : public StatsCollectorTest, public ::testing::WithParamInterface { public: @@ -710,7 +777,7 @@ class StatsCollectorTrackTest : public StatsCollectorTest, } else { stats->AddTrack(track_); } - pc->AddLocalTrack(kSsrcOfTrack, kLocalTrackId); + pc->AddSender(CreateMockSender(track_, kSsrcOfTrack)); } // Adds a incoming video track with a given SSRC into the stats. @@ -725,15 +792,16 @@ class StatsCollectorTrackTest : public StatsCollectorTest, } else { stats->AddTrack(track_); } - pc->AddRemoteTrack(kSsrcOfTrack, kRemoteTrackId); + pc->AddReceiver(CreateMockReceiver(track_, kSsrcOfTrack)); } // Adds a outgoing audio track with a given SSRC into the stats, // and register it into the stats object. // If GetParam() returns true, the track is also inserted into the local // stream, which is created if necessary. - void AddOutgoingAudioTrack(FakePeerConnectionForStats* pc, - StatsCollectorForTest* stats) { + rtc::scoped_refptr AddOutgoingAudioTrack( + FakePeerConnectionForStats* pc, + StatsCollectorForTest* stats) { audio_track_ = new rtc::RefCountedObject(kLocalTrackId); if (GetParam()) { if (!stream_) @@ -743,7 +811,7 @@ class StatsCollectorTrackTest : public StatsCollectorTest, } else { stats->AddTrack(audio_track_); } - pc->AddLocalTrack(kSsrcOfTrack, kLocalTrackId); + return pc->AddSender(CreateMockSender(audio_track_, kSsrcOfTrack)); } // Adds a incoming audio track with a given SSRC into the stats. @@ -758,7 +826,7 @@ class StatsCollectorTrackTest : public StatsCollectorTest, } else { stats->AddTrack(audio_track_); } - pc->AddRemoteTrack(kSsrcOfTrack, kRemoteTrackId); + pc->AddReceiver(CreateMockReceiver(audio_track_, kSsrcOfTrack)); } rtc::scoped_refptr stream_; @@ -1406,7 +1474,7 @@ TEST_P(StatsCollectorTrackTest, FilterOutNegativeInitialValues) { rtc::scoped_refptr local_track( new rtc::RefCountedObject(kLocalTrackId)); stream_->AddTrack(local_track); - pc->AddLocalTrack(kSsrcOfTrack, kLocalTrackId); + pc->AddSender(CreateMockSender(local_track, kSsrcOfTrack)); if (GetParam()) { stats->AddStream(stream_); } @@ -1418,7 +1486,7 @@ TEST_P(StatsCollectorTrackTest, FilterOutNegativeInitialValues) { rtc::scoped_refptr remote_track( new rtc::RefCountedObject(kRemoteTrackId)); remote_stream->AddTrack(remote_track); - pc->AddRemoteTrack(kSsrcOfTrack, kRemoteTrackId); + pc->AddReceiver(CreateMockReceiver(remote_track, kSsrcOfTrack)); if (GetParam()) { stats->AddStream(remote_stream); } @@ -1587,7 +1655,7 @@ TEST_P(StatsCollectorTrackTest, LocalAndRemoteTracksWithSameSsrc) { MediaStream::Create("remotestreamid")); rtc::scoped_refptr remote_track( new rtc::RefCountedObject(kRemoteTrackId)); - pc->AddRemoteTrack(kSsrcOfTrack, kRemoteTrackId); + pc->AddReceiver(CreateMockReceiver(remote_track, kSsrcOfTrack)); remote_stream->AddTrack(remote_track); stats->AddStream(remote_stream); @@ -1652,7 +1720,7 @@ TEST_P(StatsCollectorTrackTest, TwoLocalTracksWithSameSsrc) { auto stats = CreateStatsCollector(pc); // Create a local stream with a local audio track and adds it to the stats. - AddOutgoingAudioTrack(pc, stats.get()); + auto sender = AddOutgoingAudioTrack(pc, stats.get()); stats->AddLocalAudioTrack(audio_track_, kSsrcOfTrack); VoiceSenderInfo voice_sender_info; @@ -1671,12 +1739,13 @@ TEST_P(StatsCollectorTrackTest, TwoLocalTracksWithSameSsrc) { // Remove the previous audio track from the stream. stream_->RemoveTrack(audio_track_.get()); stats->RemoveLocalAudioTrack(audio_track_.get(), kSsrcOfTrack); + pc->RemoveSender(sender); // Create a new audio track and adds it to the stream and stats. static const std::string kNewTrackId = "new_track_id"; rtc::scoped_refptr new_audio_track( new rtc::RefCountedObject(kNewTrackId)); - pc->AddLocalTrack(kSsrcOfTrack, kNewTrackId); + pc->AddSender(CreateMockSender(new_audio_track, kSsrcOfTrack)); stream_->AddTrack(new_audio_track); stats->AddLocalAudioTrack(new_audio_track, kSsrcOfTrack); @@ -1694,6 +1763,72 @@ TEST_P(StatsCollectorTrackTest, TwoLocalTracksWithSameSsrc) { VerifyAudioTrackStats(new_audio_track, stats.get(), new_voice_info, &reports); } +// Test that if there are two local senders with the same track then two SSRC +// reports will be created, one for each sender, with the same track ID and one +// track report will be created for the shared track. +TEST_P(StatsCollectorTrackTest, TwoLocalSendersWithSameTrack) { + constexpr uint32_t kFirstSsrc = 22; + constexpr uint32_t kSecondSsrc = 33; + + auto pc = CreatePeerConnection(); + auto stats = CreateStatsCollector(pc); + + rtc::scoped_refptr local_track( + new rtc::RefCountedObject(kLocalTrackId)); + pc->AddSender(CreateMockSender(local_track, kFirstSsrc)); + stats->AddLocalAudioTrack(local_track.get(), kFirstSsrc); + pc->AddSender(CreateMockSender(local_track, kSecondSsrc)); + stats->AddLocalAudioTrack(local_track.get(), kSecondSsrc); + + VoiceSenderInfo first_sender_info; + InitVoiceSenderInfo(&first_sender_info, kFirstSsrc); + UpdateVoiceSenderInfoFromAudioTrack(local_track.get(), &first_sender_info, + false); + + VoiceSenderInfo second_sender_info; + InitVoiceSenderInfo(&second_sender_info, kSecondSsrc); + UpdateVoiceSenderInfoFromAudioTrack(local_track.get(), &second_sender_info, + false); + + VoiceMediaInfo voice_info; + voice_info.senders.push_back(first_sender_info); + voice_info.senders.push_back(second_sender_info); + + auto* voice_media_channel = pc->AddVoiceChannel("voice", "transport"); + voice_media_channel->SetStats(voice_info); + + stats->UpdateStats(PeerConnectionInterface::kStatsOutputLevelStandard); + + StatsReports reports; + stats->GetStats(local_track.get(), &reports); + RTC_LOG(LS_INFO) << reports.size(); + + // Both SSRC reports have the same track ID. + EXPECT_EQ(kLocalTrackId, GetValueInNthReportByType( + reports, StatsReport::kStatsReportTypeSsrc, + StatsReport::kStatsValueNameTrackId, 1)); + EXPECT_EQ(kLocalTrackId, GetValueInNthReportByType( + reports, StatsReport::kStatsReportTypeSsrc, + StatsReport::kStatsValueNameTrackId, 2)); + + // The SSRC in each SSRC report is different and correspond to the sender + // SSRC. + std::vector> ssrcs = { + GetValueInNthReportByType(reports, StatsReport::kStatsReportTypeSsrc, + StatsReport::kStatsValueNameSsrc, 1), + GetValueInNthReportByType(reports, StatsReport::kStatsReportTypeSsrc, + StatsReport::kStatsValueNameSsrc, 2)}; + EXPECT_THAT(ssrcs, UnorderedElementsAre(rtc::ToString(kFirstSsrc), + rtc::ToString(kSecondSsrc))); + + // There is one track report with the same track ID as the SSRC reports. + EXPECT_EQ( + 1u, GetReportsByType(reports, StatsReport::kStatsReportTypeTrack).size()); + EXPECT_EQ(kLocalTrackId, GetValueInNthReportByType( + reports, StatsReport::kStatsReportTypeTrack, + StatsReport::kStatsValueNameTrackId, 1)); +} + // This test verifies that stats are correctly set in video send ssrc stats. TEST_P(StatsCollectorTrackTest, VerifyVideoSendSsrcStats) { auto pc = CreatePeerConnection(); diff --git a/pc/test/fake_peer_connection_base.h b/pc/test/fake_peer_connection_base.h index d2fa1fd7d8..f67b305e89 100644 --- a/pc/test/fake_peer_connection_base.h +++ b/pc/test/fake_peer_connection_base.h @@ -239,11 +239,6 @@ class FakePeerConnectionBase : public PeerConnectionInternal { return {}; } - absl::string_view GetLocalTrackIdBySsrc(uint32_t ssrc) override { return {}; } - absl::string_view GetRemoteTrackIdBySsrc(uint32_t ssrc) override { - return {}; - } - sigslot::signal1& SignalDataChannelCreated() override { return SignalDataChannelCreated_; } diff --git a/pc/test/fake_peer_connection_for_stats.h b/pc/test/fake_peer_connection_for_stats.h index b954fc58dc..826439196b 100644 --- a/pc/test/fake_peer_connection_for_stats.h +++ b/pc/test/fake_peer_connection_for_stats.h @@ -98,16 +98,25 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { return remote_streams_; } - void AddSender(rtc::scoped_refptr sender) { + rtc::scoped_refptr AddSender( + rtc::scoped_refptr sender) { // TODO(steveanton): Switch tests to use RtpTransceivers directly. auto sender_proxy = RtpSenderProxyWithInternal::Create( signaling_thread_, sender); GetOrCreateFirstTransceiverOfType(sender->media_type()) ->internal() ->AddSender(sender_proxy); + return sender_proxy; } - void AddReceiver(rtc::scoped_refptr receiver) { + void RemoveSender(rtc::scoped_refptr sender) { + GetOrCreateFirstTransceiverOfType(sender->media_type()) + ->internal() + ->RemoveSender(sender); + } + + rtc::scoped_refptr AddReceiver( + rtc::scoped_refptr receiver) { // TODO(steveanton): Switch tests to use RtpTransceivers directly. auto receiver_proxy = RtpReceiverProxyWithInternal::Create( @@ -115,6 +124,13 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { GetOrCreateFirstTransceiverOfType(receiver->media_type()) ->internal() ->AddReceiver(receiver_proxy); + return receiver_proxy; + } + + void RemoveReceiver(rtc::scoped_refptr receiver) { + GetOrCreateFirstTransceiverOfType(receiver->media_type()) + ->internal() + ->RemoveReceiver(receiver); } FakeVoiceMediaChannelForStats* AddVoiceChannel( @@ -153,14 +169,6 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { return video_media_channel_ptr; } - void AddLocalTrack(uint32_t ssrc, const std::string& track_id) { - local_track_id_by_ssrc_[ssrc] = track_id; - } - - void AddRemoteTrack(uint32_t ssrc, const std::string& track_id) { - remote_track_id_by_ssrc_[ssrc] = track_id; - } - void AddSctpDataChannel(const std::string& label) { AddSctpDataChannel(label, InternalDataChannelInit()); } @@ -250,22 +258,6 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { return transceivers_; } - 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()) { - return it->second; - } - return {}; - } - - 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()) { - return it->second; - } - return {}; - } - std::vector> sctp_data_channels() const override { return sctp_data_channels_; @@ -367,8 +359,6 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { std::unique_ptr voice_channel_; std::unique_ptr video_channel_; - std::map local_track_id_by_ssrc_; - std::map remote_track_id_by_ssrc_; std::vector> sctp_data_channels_;