Only expose RTCCodecStats that are referenced by an RTP stream.
According to a pprof, creating RTCCodecStats is one of the places where
we spend the most CPU time in the event of creating hundreds of them:
https://screenshot.googleplex.com/B6QNDvvoX8dK5vk
The lifetime was recently updated so that we no longer have to risk
creating hundreds of them, here is the relevant section:
https://w3c.github.io/webrtc-stats/#codec-dict*
This allows code simplifications and the deletion of
ProduceCodecStats_n since we can now do a lazy instantiation of codec
stats at the point of being referenced.
Bug: webrtc:14444
Change-Id: I342c5bfebe6a4be0359da3ea106692c7a217779e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275763
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38209}
diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc
index 407f33b..3883a64 100644
--- a/pc/rtc_stats_collector.cc
+++ b/pc/rtc_stats_collector.cc
@@ -80,6 +80,9 @@
char buf[1024];
rtc::SimpleStringBuilder sb(buf);
sb << 'C' << direction << transport_id << '_' << codec_params.payload_type;
+ // TODO(https://crbug.com/webrtc/14420): If we stop supporting different FMTP
+ // lines for the same PT and transport, which should be illegal SDP, then we
+ // wouldn't need `fmtp` to be part of the ID here.
rtc::StringBuilder fmtp;
if (WriteFmtpParameters(codec_params.parameters, &fmtp)) {
sb << '_' << fmtp.Release();
@@ -354,19 +357,27 @@
return audio_level / 32767.0;
}
-std::unique_ptr<RTCCodecStats> CodecStatsFromRtpCodecParameters(
+// Gets the `codecId` identified by `transport_id` and `codec_params`. If no
+// such `RTCCodecStats` exist yet, create it and add it to `report`.
+std::string GetCodecIdAndMaybeCreateCodecStats(
uint64_t timestamp_us,
const char direction,
const std::string& transport_id,
- const RtpCodecParameters& codec_params) {
+ const RtpCodecParameters& codec_params,
+ RTCStatsReport* report) {
RTC_DCHECK_GE(codec_params.payload_type, 0);
RTC_DCHECK_LE(codec_params.payload_type, 127);
RTC_DCHECK(codec_params.clock_rate);
uint32_t payload_type = static_cast<uint32_t>(codec_params.payload_type);
- std::unique_ptr<RTCCodecStats> codec_stats(std::make_unique<RTCCodecStats>(
- RTCCodecStatsIDFromTransportAndCodecParameters(direction, transport_id,
- codec_params),
- timestamp_us));
+ std::string codec_id = RTCCodecStatsIDFromTransportAndCodecParameters(
+ direction, transport_id, codec_params);
+ if (report->Get(codec_id) != nullptr) {
+ // The `RTCCodecStats` already exists.
+ return codec_id;
+ }
+ // Create the `RTCCodecStats` that we want to reference.
+ std::unique_ptr<RTCCodecStats> codec_stats(
+ std::make_unique<RTCCodecStats>(codec_id, timestamp_us));
codec_stats->payload_type = payload_type;
codec_stats->mime_type = codec_params.mime_type();
if (codec_params.clock_rate) {
@@ -381,7 +392,8 @@
codec_stats->sdp_fmtp_line = fmtp.Release();
}
codec_stats->transport_id = transport_id;
- return codec_stats;
+ report->AddStats(std::move(codec_stats));
+ return codec_id;
}
void SetMediaStreamTrackStatsFromMediaStreamTrackInterface(
@@ -427,7 +439,8 @@
const cricket::VoiceReceiverInfo& voice_receiver_info,
const std::string& transport_id,
const std::string& mid,
- int64_t timestamp_us) {
+ int64_t timestamp_us,
+ RTCStatsReport* report) {
auto inbound_audio = std::make_unique<RTCInboundRTPStreamStats>(
/*id=*/RTCInboundRTPStreamStatsIDFromSSRC(
transport_id, cricket::MEDIA_TYPE_AUDIO, voice_receiver_info.ssrc()),
@@ -443,8 +456,9 @@
voice_receiver_info.codec_payload_type.value());
RTC_DCHECK(codec_param_it != voice_media_info.receive_codecs.end());
if (codec_param_it != voice_media_info.receive_codecs.end()) {
- inbound_audio->codec_id = RTCCodecStatsIDFromTransportAndCodecParameters(
- kDirectionInbound, transport_id, codec_param_it->second);
+ inbound_audio->codec_id = GetCodecIdAndMaybeCreateCodecStats(
+ inbound_audio->timestamp_us(), kDirectionInbound, transport_id,
+ codec_param_it->second, report);
}
}
inbound_audio->jitter = static_cast<double>(voice_receiver_info.jitter_ms) /
@@ -541,7 +555,8 @@
const std::string& mid,
const cricket::VideoMediaInfo& video_media_info,
const cricket::VideoReceiverInfo& video_receiver_info,
- RTCInboundRTPStreamStats* inbound_video) {
+ RTCInboundRTPStreamStats* inbound_video,
+ RTCStatsReport* report) {
SetInboundRTPStreamStatsFromMediaReceiverInfo(video_receiver_info,
inbound_video);
inbound_video->transport_id = transport_id;
@@ -553,8 +568,9 @@
video_receiver_info.codec_payload_type.value());
RTC_DCHECK(codec_param_it != video_media_info.receive_codecs.end());
if (codec_param_it != video_media_info.receive_codecs.end()) {
- inbound_video->codec_id = RTCCodecStatsIDFromTransportAndCodecParameters(
- kDirectionInbound, transport_id, codec_param_it->second);
+ inbound_video->codec_id = GetCodecIdAndMaybeCreateCodecStats(
+ inbound_video->timestamp_us(), kDirectionInbound, transport_id,
+ codec_param_it->second, report);
}
}
inbound_video->jitter = static_cast<double>(video_receiver_info.jitter_ms) /
@@ -643,7 +659,8 @@
const std::string& mid,
const cricket::VoiceMediaInfo& voice_media_info,
const cricket::VoiceSenderInfo& voice_sender_info,
- RTCOutboundRTPStreamStats* outbound_audio) {
+ RTCOutboundRTPStreamStats* outbound_audio,
+ RTCStatsReport* report) {
SetOutboundRTPStreamStatsFromMediaSenderInfo(voice_sender_info,
outbound_audio);
outbound_audio->transport_id = transport_id;
@@ -659,8 +676,9 @@
voice_sender_info.codec_payload_type.value());
RTC_DCHECK(codec_param_it != voice_media_info.send_codecs.end());
if (codec_param_it != voice_media_info.send_codecs.end()) {
- outbound_audio->codec_id = RTCCodecStatsIDFromTransportAndCodecParameters(
- kDirectionOutbound, transport_id, codec_param_it->second);
+ outbound_audio->codec_id = GetCodecIdAndMaybeCreateCodecStats(
+ outbound_audio->timestamp_us(), kDirectionOutbound, transport_id,
+ codec_param_it->second, report);
}
}
// `fir_count`, `pli_count` and `sli_count` are only valid for video and are
@@ -672,7 +690,8 @@
const std::string& mid,
const cricket::VideoMediaInfo& video_media_info,
const cricket::VideoSenderInfo& video_sender_info,
- RTCOutboundRTPStreamStats* outbound_video) {
+ RTCOutboundRTPStreamStats* outbound_video,
+ RTCStatsReport* report) {
SetOutboundRTPStreamStatsFromMediaSenderInfo(video_sender_info,
outbound_video);
outbound_video->transport_id = transport_id;
@@ -684,8 +703,9 @@
video_sender_info.codec_payload_type.value());
RTC_DCHECK(codec_param_it != video_media_info.send_codecs.end());
if (codec_param_it != video_media_info.send_codecs.end()) {
- outbound_video->codec_id = RTCCodecStatsIDFromTransportAndCodecParameters(
- kDirectionOutbound, transport_id, codec_param_it->second);
+ outbound_video->codec_id = GetCodecIdAndMaybeCreateCodecStats(
+ outbound_video->timestamp_us(), kDirectionOutbound, transport_id,
+ codec_param_it->second, report);
}
}
outbound_video->fir_count =
@@ -1484,7 +1504,6 @@
rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
ProduceCertificateStats_n(timestamp_us, transport_cert_stats, partial_report);
- ProduceCodecStats_n(timestamp_us, transceiver_stats_infos_, partial_report);
ProduceIceCandidateAndPairStats_n(timestamp_us, transport_stats_by_name,
call_stats_, partial_report);
ProduceTransportStats_n(timestamp_us, transport_stats_by_name,
@@ -1583,100 +1602,6 @@
}
}
-void RTCStatsCollector::ProduceCodecStats_n(
- int64_t timestamp_us,
- const std::vector<RtpTransceiverStatsInfo>& transceiver_stats_infos,
- RTCStatsReport* report) const {
- RTC_DCHECK_RUN_ON(network_thread_);
- rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
-
- // For each transport, payload types does uniquely identify codecs, but the
- // FMTP line could in theory be different on different m= sections. As such,
- // the (PT,FMTP) pair on a per-transport basis uniquely identifies an
- // RTCCodecStats. These maps are used to avoid duplicates.
- // TODO(https://crbug.com/webrtc/14420): If we stop supporting different FMTP
- // lines, this can be simplified to only looking at the set of PTs.
- typedef std::pair<int, cricket::CodecParameterMap>
- PayloadTypeAndCodecParametersMap;
- std::map<std::string, std::set<PayloadTypeAndCodecParametersMap>>
- send_codecs_by_transport;
- std::map<std::string, std::set<PayloadTypeAndCodecParametersMap>>
- receive_codecs_by_transport;
-
- for (const auto& stats : transceiver_stats_infos) {
- if (!stats.mid) {
- continue;
- }
- std::string transport_id = RTCTransportStatsIDFromTransportChannel(
- *stats.transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP);
-
- // Codecs (PT,FMTP) seen so far for this transport.
- std::set<PayloadTypeAndCodecParametersMap>& send_codecs =
- send_codecs_by_transport[transport_id];
- std::set<PayloadTypeAndCodecParametersMap>& receive_codecs =
- receive_codecs_by_transport[transport_id];
-
- // Audio
- if (stats.track_media_info_map.voice_media_info().has_value()) {
- // Inbound
- for (const auto& pair :
- stats.track_media_info_map.voice_media_info()->receive_codecs) {
- auto& codec = pair.second;
- if (receive_codecs
- .insert(PayloadTypeAndCodecParametersMap(codec.payload_type,
- codec.parameters))
- .second != true) {
- continue; // (PT,FMTP) already seen.
- }
- report->AddStats(CodecStatsFromRtpCodecParameters(
- timestamp_us, kDirectionInbound, transport_id, codec));
- }
- // Outbound
- for (const auto& pair :
- stats.track_media_info_map.voice_media_info()->send_codecs) {
- auto& codec = pair.second;
- if (send_codecs
- .insert(PayloadTypeAndCodecParametersMap(codec.payload_type,
- codec.parameters))
- .second != true) {
- continue; // (PT,FMTP) already seen.
- }
- report->AddStats(CodecStatsFromRtpCodecParameters(
- timestamp_us, kDirectionOutbound, transport_id, codec));
- }
- }
- // Video
- if (stats.track_media_info_map.video_media_info().has_value()) {
- // Inbound
- for (const auto& pair :
- stats.track_media_info_map.video_media_info()->receive_codecs) {
- auto& codec = pair.second;
- if (receive_codecs
- .insert(PayloadTypeAndCodecParametersMap(codec.payload_type,
- codec.parameters))
- .second != true) {
- continue; // (PT,FMTP) already seen.
- }
- report->AddStats(CodecStatsFromRtpCodecParameters(
- timestamp_us, kDirectionInbound, transport_id, codec));
- }
- // Outbound
- for (const auto& pair :
- stats.track_media_info_map.video_media_info()->send_codecs) {
- auto& codec = pair.second;
- if (send_codecs
- .insert(PayloadTypeAndCodecParametersMap(codec.payload_type,
- codec.parameters))
- .second != true) {
- continue; // (PT,FMTP) already seen.
- }
- report->AddStats(CodecStatsFromRtpCodecParameters(
- timestamp_us, kDirectionOutbound, transport_id, codec));
- }
- }
- }
-}
-
void RTCStatsCollector::ProduceDataChannelStats_s(
int64_t timestamp_us,
RTCStatsReport* report) const {
@@ -2020,7 +1945,7 @@
// Inbound.
auto inbound_audio = CreateInboundAudioStreamStats(
stats.track_media_info_map.voice_media_info().value(),
- voice_receiver_info, transport_id, mid, timestamp_us);
+ voice_receiver_info, transport_id, mid, timestamp_us, report);
// TODO(hta): This lookup should look for the sender, not the track.
rtc::scoped_refptr<AudioTrackInterface> audio_track =
stats.track_media_info_map.GetAudioTrack(voice_receiver_info);
@@ -2068,7 +1993,7 @@
SetOutboundRTPStreamStatsFromVoiceSenderInfo(
transport_id, mid,
stats.track_media_info_map.voice_media_info().value(),
- voice_sender_info, outbound_audio.get());
+ voice_sender_info, outbound_audio.get(), report);
rtc::scoped_refptr<AudioTrackInterface> audio_track =
stats.track_media_info_map.GetAudioTrack(voice_sender_info);
if (audio_track) {
@@ -2133,7 +2058,7 @@
SetInboundRTPStreamStatsFromVideoReceiverInfo(
transport_id, mid,
stats.track_media_info_map.video_media_info().value(),
- video_receiver_info, inbound_video.get());
+ video_receiver_info, inbound_video.get(), report);
rtc::scoped_refptr<VideoTrackInterface> video_track =
stats.track_media_info_map.GetVideoTrack(video_receiver_info);
if (video_track) {
@@ -2162,7 +2087,7 @@
SetOutboundRTPStreamStatsFromVideoSenderInfo(
transport_id, mid,
stats.track_media_info_map.video_media_info().value(),
- video_sender_info, outbound_video.get());
+ video_sender_info, outbound_video.get(), report);
rtc::scoped_refptr<VideoTrackInterface> video_track =
stats.track_media_info_map.GetVideoTrack(video_sender_info);
if (video_track) {
diff --git a/pc/rtc_stats_collector.h b/pc/rtc_stats_collector.h
index e5c6aed..0c297c4 100644
--- a/pc/rtc_stats_collector.h
+++ b/pc/rtc_stats_collector.h
@@ -180,11 +180,6 @@
int64_t timestamp_us,
const std::map<std::string, CertificateStatsPair>& transport_cert_stats,
RTCStatsReport* report) const;
- // Produces `RTCCodecStats`.
- void ProduceCodecStats_n(
- int64_t timestamp_us,
- const std::vector<RtpTransceiverStatsInfo>& transceiver_stats_infos,
- RTCStatsReport* report) const;
// Produces `RTCDataChannelStats`.
void ProduceDataChannelStats_s(int64_t timestamp_us,
RTCStatsReport* report) const;
@@ -208,9 +203,11 @@
// Produces `RTCPeerConnectionStats`.
void ProducePeerConnectionStats_s(int64_t timestamp_us,
RTCStatsReport* report) const;
- // Produces `RTCInboundRTPStreamStats` and `RTCOutboundRTPStreamStats`.
- // This has to be invoked after codecs and transport stats have been created
- // because some metrics are calculated through lookup of other metrics.
+ // Produces `RTCInboundRTPStreamStats`, `RTCOutboundRTPStreamStats`,
+ // `RTCRemoteInboundRtpStreamStats`, `RTCRemoteOutboundRtpStreamStats` and any
+ // referenced `RTCCodecStats`. This has to be invoked after transport stats
+ // have been created because some metrics are calculated through lookup of
+ // other metrics.
void ProduceRTPStreamStats_n(
int64_t timestamp_us,
const std::vector<RtpTransceiverStatsInfo>& transceiver_stats_infos,
diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc
index 88bb6ae..371f120 100644
--- a/pc/rtc_stats_collector_unittest.cc
+++ b/pc/rtc_stats_collector_unittest.cc
@@ -1095,7 +1095,7 @@
// should look. We only care about not crashing.
}
-TEST_F(RTCStatsCollectorTest, CollectRTCCodecStats) {
+TEST_F(RTCStatsCollectorTest, CollectRTCCodecStatsOnlyIfReferenced) {
// Audio
cricket::VoiceMediaInfo voice_media_info;
@@ -1118,8 +1118,6 @@
voice_media_info.send_codecs.insert(
std::make_pair(outbound_audio_codec.payload_type, outbound_audio_codec));
- pc_->AddVoiceChannel("AudioMid", "TransportName", voice_media_info);
-
// Video
cricket::VideoMediaInfo video_media_info;
@@ -1142,7 +1140,31 @@
video_media_info.send_codecs.insert(
std::make_pair(outbound_video_codec.payload_type, outbound_video_codec));
- pc_->AddVideoChannel("VideoMid", "TransportName", video_media_info);
+ // Ensure the above codecs are referenced.
+ cricket::VoiceReceiverInfo inbound_audio_info;
+ inbound_audio_info.add_ssrc(10);
+ inbound_audio_info.codec_payload_type = 1;
+ voice_media_info.receivers.push_back(inbound_audio_info);
+
+ cricket::VoiceSenderInfo outbound_audio_info;
+ outbound_audio_info.add_ssrc(20);
+ outbound_audio_info.codec_payload_type = 2;
+ voice_media_info.senders.push_back(outbound_audio_info);
+
+ cricket::VideoReceiverInfo inbound_video_info;
+ inbound_video_info.add_ssrc(30);
+ inbound_video_info.codec_payload_type = 3;
+ video_media_info.receivers.push_back(inbound_video_info);
+
+ cricket::VideoSenderInfo outbound_video_info;
+ outbound_video_info.add_ssrc(40);
+ outbound_video_info.codec_payload_type = 4;
+ video_media_info.senders.push_back(outbound_video_info);
+
+ FakeVoiceMediaChannelForStats* audio_channel =
+ pc_->AddVoiceChannel("AudioMid", "TransportName", voice_media_info);
+ FakeVideoMediaChannelForStats* video_channel =
+ pc_->AddVideoChannel("VideoMid", "TransportName", video_media_info);
rtc::scoped_refptr<const RTCStatsReport> report = stats_->GetStatsReport();
@@ -1200,6 +1222,22 @@
EXPECT_EQ(expected_outbound_video_codec,
report->Get(expected_outbound_video_codec.id())
->cast_to<RTCCodecStats>());
+
+ // Now remove all the RTP streams such that there are no live codecId
+ // references to the codecs, this should result in none of the RTCCodecStats
+ // being exposed, despite `send_codecs` and `receive_codecs` still being set.
+ voice_media_info.senders.clear();
+ voice_media_info.receivers.clear();
+ audio_channel->SetStats(voice_media_info);
+ video_media_info.senders.clear();
+ video_media_info.receivers.clear();
+ video_channel->SetStats(video_media_info);
+ stats_->stats_collector()->ClearCachedStatsReport();
+ report = stats_->GetStatsReport();
+ EXPECT_FALSE(report->Get(expected_inbound_audio_codec.id()));
+ EXPECT_FALSE(report->Get(expected_outbound_audio_codec.id()));
+ EXPECT_FALSE(report->Get(expected_inbound_video_codec.id()));
+ EXPECT_FALSE(report->Get(expected_outbound_video_codec.id()));
}
TEST_F(RTCStatsCollectorTest, CodecStatsAreCollectedPerTransport) {
@@ -1217,17 +1255,35 @@
outbound_codec_pt11.name = "VP8";
outbound_codec_pt11.clock_rate = 9000;
+ // Insert codecs into `send_codecs` and ensure the PTs are referenced by RTP
+ // streams.
cricket::VideoMediaInfo info_pt10;
info_pt10.send_codecs.insert(
std::make_pair(outbound_codec_pt10.payload_type, outbound_codec_pt10));
+ info_pt10.senders.emplace_back();
+ info_pt10.senders[0].add_ssrc(42);
+ info_pt10.senders[0].codec_payload_type = outbound_codec_pt10.payload_type;
+
cricket::VideoMediaInfo info_pt11;
info_pt11.send_codecs.insert(
std::make_pair(outbound_codec_pt11.payload_type, outbound_codec_pt11));
+ info_pt11.senders.emplace_back();
+ info_pt11.senders[0].add_ssrc(43);
+ info_pt11.senders[0].codec_payload_type = outbound_codec_pt11.payload_type;
+
cricket::VideoMediaInfo info_pt10_pt11;
info_pt10_pt11.send_codecs.insert(
std::make_pair(outbound_codec_pt10.payload_type, outbound_codec_pt10));
info_pt10_pt11.send_codecs.insert(
std::make_pair(outbound_codec_pt11.payload_type, outbound_codec_pt11));
+ info_pt10_pt11.senders.emplace_back();
+ info_pt10_pt11.senders[0].add_ssrc(44);
+ info_pt10_pt11.senders[0].codec_payload_type =
+ outbound_codec_pt10.payload_type;
+ info_pt10_pt11.senders.emplace_back();
+ info_pt10_pt11.senders[1].add_ssrc(45);
+ info_pt10_pt11.senders[1].codec_payload_type =
+ outbound_codec_pt11.payload_type;
// First two mids contain subsets, the third one contains all PTs.
pc_->AddVideoChannel("Mid1", "FirstTransport", info_pt10);
@@ -1269,11 +1325,19 @@
std::make_pair("useinbandfec", "1"));
cricket::VideoMediaInfo info_nofec;
- info_nofec.send_codecs.insert(std::make_pair(
+ info_nofec.receive_codecs.insert(std::make_pair(
inbound_codec_pt111_nofec.payload_type, inbound_codec_pt111_nofec));
+ info_nofec.receivers.emplace_back();
+ info_nofec.receivers[0].add_ssrc(123);
+ info_nofec.receivers[0].codec_payload_type =
+ inbound_codec_pt111_nofec.payload_type;
cricket::VideoMediaInfo info_fec;
- info_fec.send_codecs.insert(std::make_pair(
+ info_fec.receive_codecs.insert(std::make_pair(
inbound_codec_pt111_fec.payload_type, inbound_codec_pt111_fec));
+ info_fec.receivers.emplace_back();
+ info_fec.receivers[0].add_ssrc(321);
+ info_fec.receivers[0].codec_payload_type =
+ inbound_codec_pt111_fec.payload_type;
// First two mids contain subsets, the third one contains all PTs.
pc_->AddVideoChannel("Mid1", "BundledTransport", info_nofec);
@@ -1285,6 +1349,10 @@
auto codec_stats = report->GetStatsOfType<RTCCodecStats>();
EXPECT_EQ(codec_stats.size(), 2u);
+ // Ensure SSRC uniqueness before the next AddVideoChannel() call. SSRCs need
+ // to be unique on different m= sections when using BUNDLE.
+ info_nofec.receivers[0].local_stats[0].ssrc = 12;
+ info_fec.receivers[0].local_stats[0].ssrc = 21;
// Adding more m= sections that does have the same FMTP lines does not result
// in duplicates.
pc_->AddVideoChannel("Mid3", "BundledTransport", info_nofec);
@@ -1304,8 +1372,12 @@
inbound_codec_pt112_fec.parameters.insert(
std::make_pair("useinbandfec", "1"));
cricket::VideoMediaInfo info_fec_pt112;
- info_fec_pt112.send_codecs.insert(std::make_pair(
+ info_fec_pt112.receive_codecs.insert(std::make_pair(
inbound_codec_pt112_fec.payload_type, inbound_codec_pt112_fec));
+ info_fec_pt112.receivers.emplace_back();
+ info_fec_pt112.receivers[0].add_ssrc(112);
+ info_fec_pt112.receivers[0].codec_payload_type =
+ inbound_codec_pt112_fec.payload_type;
pc_->AddVideoChannel("Mid5", "BundledTransport", info_fec_pt112);
stats_->stats_collector()->ClearCachedStatsReport();
report = stats_->GetStatsReport();
