diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc index 407f33b237..3883a64c7b 100644 --- a/pc/rtc_stats_collector.cc +++ b/pc/rtc_stats_collector.cc @@ -80,6 +80,9 @@ std::string RTCCodecStatsIDFromTransportAndCodecParameters( 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 @@ double DoubleAudioLevelFromIntAudioLevel(int audio_level) { return audio_level / 32767.0; } -std::unique_ptr 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(codec_params.payload_type); - std::unique_ptr codec_stats(std::make_unique( - 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 codec_stats( + std::make_unique(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 @@ std::unique_ptr CodecStatsFromRtpCodecParameters( 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 @@ std::unique_ptr CreateInboundAudioStreamStats( 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( /*id=*/RTCInboundRTPStreamStatsIDFromSSRC( transport_id, cricket::MEDIA_TYPE_AUDIO, voice_receiver_info.ssrc()), @@ -443,8 +456,9 @@ std::unique_ptr CreateInboundAudioStreamStats( 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(voice_receiver_info.jitter_ms) / @@ -541,7 +555,8 @@ void SetInboundRTPStreamStatsFromVideoReceiverInfo( 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 @@ void SetInboundRTPStreamStatsFromVideoReceiverInfo( 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(video_receiver_info.jitter_ms) / @@ -643,7 +659,8 @@ void SetOutboundRTPStreamStatsFromVoiceSenderInfo( 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 @@ void SetOutboundRTPStreamStatsFromVoiceSenderInfo( 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 @@ void SetOutboundRTPStreamStatsFromVideoSenderInfo( 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 @@ void SetOutboundRTPStreamStatsFromVideoSenderInfo( 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 @@ void RTCStatsCollector::ProducePartialResultsOnNetworkThreadImpl( 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::ProduceCertificateStats_n( } } -void RTCStatsCollector::ProduceCodecStats_n( - int64_t timestamp_us, - const std::vector& 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 - PayloadTypeAndCodecParametersMap; - std::map> - send_codecs_by_transport; - std::map> - 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& send_codecs = - send_codecs_by_transport[transport_id]; - std::set& 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 @@ void RTCStatsCollector::ProduceAudioRTPStreamStats_n( // 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 audio_track = stats.track_media_info_map.GetAudioTrack(voice_receiver_info); @@ -2068,7 +1993,7 @@ void RTCStatsCollector::ProduceAudioRTPStreamStats_n( 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 audio_track = stats.track_media_info_map.GetAudioTrack(voice_sender_info); if (audio_track) { @@ -2133,7 +2058,7 @@ void RTCStatsCollector::ProduceVideoRTPStreamStats_n( 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 video_track = stats.track_media_info_map.GetVideoTrack(video_receiver_info); if (video_track) { @@ -2162,7 +2087,7 @@ void RTCStatsCollector::ProduceVideoRTPStreamStats_n( 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 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 e5c6aedd88..0c297c45a7 100644 --- a/pc/rtc_stats_collector.h +++ b/pc/rtc_stats_collector.h @@ -180,11 +180,6 @@ class RTCStatsCollector : public rtc::RefCountInterface, int64_t timestamp_us, const std::map& transport_cert_stats, RTCStatsReport* report) const; - // Produces `RTCCodecStats`. - void ProduceCodecStats_n( - int64_t timestamp_us, - const std::vector& transceiver_stats_infos, - RTCStatsReport* report) const; // Produces `RTCDataChannelStats`. void ProduceDataChannelStats_s(int64_t timestamp_us, RTCStatsReport* report) const; @@ -208,9 +203,11 @@ class RTCStatsCollector : public rtc::RefCountInterface, // 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& transceiver_stats_infos, diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc index 88bb6aefa5..371f1201f1 100644 --- a/pc/rtc_stats_collector_unittest.cc +++ b/pc/rtc_stats_collector_unittest.cc @@ -1095,7 +1095,7 @@ TEST_F(RTCStatsCollectorTest, InvalidSsrcCollisionDoesNotCrash) { // 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 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCodecStats) { 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 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCodecStats) { 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 report = stats_->GetStatsReport(); @@ -1200,6 +1222,22 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCodecStats) { EXPECT_EQ(expected_outbound_video_codec, report->Get(expected_outbound_video_codec.id()) ->cast_to()); + + // 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 @@ TEST_F(RTCStatsCollectorTest, CodecStatsAreCollectedPerTransport) { 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 @@ TEST_F(RTCStatsCollectorTest, SamePayloadTypeButDifferentFmtpLines) { 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 @@ TEST_F(RTCStatsCollectorTest, SamePayloadTypeButDifferentFmtpLines) { auto codec_stats = report->GetStatsOfType(); 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 @@ TEST_F(RTCStatsCollectorTest, SamePayloadTypeButDifferentFmtpLines) { 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();