From b2be392c708c975ff5a81d8cd4dba588752a8dad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Fri, 2 Sep 2022 09:37:08 +0200 Subject: [PATCH] Avoid duplicate RTCCodecStats entries. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The code incorrectly assumed that codecs exist on a per-mid/transceiver basis, but codec payload types are unique on a per-transport basis and in practise most applications use BUNDLE (single transport for the entire PC). This CL makes the codecs per-transport instead of per-transceiver. We still need to iterate transceivers because codecs are exposed on a per-transceiver basis and as shown in https://jsfiddle.net/henbos/7kqxgnr8/ it is possible for FMTP lines to be different on different m= sections despite BUNDLE. Manual testing shows that this CL brings down the number of "codec" stats in Google Meet 50p from 872 objects to 43 objects. Bug: webrtc:14414 Change-Id: Ic854b31bd595799554b99fff22cbd48264ebd141 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/273707 Reviewed-by: Taylor Brandstetter Reviewed-by: Philipp Hancke Commit-Queue: Henrik Boström Cr-Commit-Position: refs/heads/main@{#37989} --- pc/rtc_stats_collector.cc | 174 +++++++++++++++++------ pc/rtc_stats_collector_unittest.cc | 148 ++++++++++++++++--- pc/test/fake_peer_connection_for_stats.h | 35 +++-- 3 files changed, 288 insertions(+), 69 deletions(-) diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc index be6c23e843..1f944a0ebb 100644 --- a/pc/rtc_stats_collector.cc +++ b/pc/rtc_stats_collector.cc @@ -70,13 +70,18 @@ std::string RTCCertificateIDFromFingerprint(const std::string& fingerprint) { return "RTCCertificate_" + fingerprint; } -std::string RTCCodecStatsIDFromMidDirectionAndPayload(const std::string& mid, - bool inbound, - uint32_t payload_type) { +std::string RTCCodecStatsIDFromTransportAndCodecParameters( + const std::string& transport_id, + bool inbound, + const RtpCodecParameters& codec_params) { char buf[1024]; rtc::SimpleStringBuilder sb(buf); - sb << "RTCCodec_" << mid << (inbound ? "_Inbound_" : "_Outbound_") - << payload_type; + sb << "RTCCodec_" << transport_id << (inbound ? "_Recv_" : "_Send_") + << codec_params.payload_type; + rtc::StringBuilder fmtp; + if (WriteFmtpParameters(codec_params.parameters, &fmtp)) { + sb << "_" << fmtp.Release(); + } return sb.str(); } @@ -354,7 +359,6 @@ double DoubleAudioLevelFromIntAudioLevel(int audio_level) { std::unique_ptr CodecStatsFromRtpCodecParameters( uint64_t timestamp_us, - const std::string& mid, const std::string& transport_id, bool inbound, const RtpCodecParameters& codec_params) { @@ -362,9 +366,10 @@ std::unique_ptr CodecStatsFromRtpCodecParameters( 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(new RTCCodecStats( - RTCCodecStatsIDFromMidDirectionAndPayload(mid, inbound, payload_type), - timestamp_us)); + std::unique_ptr codec_stats( + new RTCCodecStats(RTCCodecStatsIDFromTransportAndCodecParameters( + transport_id, inbound, codec_params), + timestamp_us)); codec_stats->payload_type = payload_type; codec_stats->mime_type = codec_params.mime_type(); if (codec_params.clock_rate) { @@ -421,7 +426,9 @@ void SetInboundRTPStreamStatsFromMediaReceiverInfo( } std::unique_ptr CreateInboundAudioStreamStats( + const cricket::VoiceMediaInfo& voice_media_info, const cricket::VoiceReceiverInfo& voice_receiver_info, + const std::string& transport_id, const std::string& mid, int64_t timestamp_us) { auto inbound_audio = std::make_unique( @@ -430,12 +437,18 @@ std::unique_ptr CreateInboundAudioStreamStats( timestamp_us); SetInboundRTPStreamStatsFromMediaReceiverInfo(voice_receiver_info, inbound_audio.get()); + inbound_audio->transport_id = transport_id; inbound_audio->mid = mid; inbound_audio->media_type = "audio"; inbound_audio->kind = "audio"; - if (voice_receiver_info.codec_payload_type) { - inbound_audio->codec_id = RTCCodecStatsIDFromMidDirectionAndPayload( - mid, /*inbound=*/true, *voice_receiver_info.codec_payload_type); + if (voice_receiver_info.codec_payload_type.has_value()) { + auto codec_param_it = voice_media_info.receive_codecs.find( + 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( + transport_id, /*inbound=*/true, codec_param_it->second); + } } inbound_audio->jitter = static_cast(voice_receiver_info.jitter_ms) / rtc::kNumMillisecsPerSec; @@ -479,7 +492,7 @@ std::unique_ptr CreateRemoteOutboundAudioStreamStats( const cricket::VoiceReceiverInfo& voice_receiver_info, const std::string& mid, - const std::string& inbound_audio_id, + const RTCInboundRTPStreamStats& inbound_audio_stats, const std::string& transport_id) { if (!voice_receiver_info.last_sender_report_timestamp_ms.has_value()) { // Cannot create `RTCRemoteOutboundRtpStreamStats` when the RTCP SR arrival @@ -501,15 +514,14 @@ CreateRemoteOutboundAudioStreamStats( stats->ssrc = voice_receiver_info.ssrc(); stats->kind = "audio"; stats->transport_id = transport_id; - stats->codec_id = RTCCodecStatsIDFromMidDirectionAndPayload( - mid, - /*inbound=*/true, // Remote-outbound same as local-inbound. - *voice_receiver_info.codec_payload_type); + if (inbound_audio_stats.codec_id.is_defined()) { + stats->codec_id = *inbound_audio_stats.codec_id; + } // - RTCSentRtpStreamStats. stats->packets_sent = voice_receiver_info.sender_reports_packets_sent; stats->bytes_sent = voice_receiver_info.sender_reports_bytes_sent; // - RTCRemoteOutboundRtpStreamStats. - stats->local_id = inbound_audio_id; + stats->local_id = inbound_audio_stats.id(); RTC_DCHECK( voice_receiver_info.last_sender_report_remote_timestamp_ms.has_value()); stats->remote_timestamp = static_cast( @@ -528,17 +540,25 @@ CreateRemoteOutboundAudioStreamStats( } void SetInboundRTPStreamStatsFromVideoReceiverInfo( + const std::string& transport_id, const std::string& mid, + const cricket::VideoMediaInfo& video_media_info, const cricket::VideoReceiverInfo& video_receiver_info, RTCInboundRTPStreamStats* inbound_video) { SetInboundRTPStreamStatsFromMediaReceiverInfo(video_receiver_info, inbound_video); + inbound_video->transport_id = transport_id; inbound_video->mid = mid; inbound_video->media_type = "video"; inbound_video->kind = "video"; - if (video_receiver_info.codec_payload_type) { - inbound_video->codec_id = RTCCodecStatsIDFromMidDirectionAndPayload( - mid, /*inbound=*/true, *video_receiver_info.codec_payload_type); + if (video_receiver_info.codec_payload_type.has_value()) { + auto codec_param_it = video_media_info.receive_codecs.find( + 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( + transport_id, /*inbound=*/true, codec_param_it->second); + } } inbound_video->jitter = static_cast(video_receiver_info.jitter_ms) / rtc::kNumMillisecsPerSec; @@ -622,37 +642,53 @@ void SetOutboundRTPStreamStatsFromMediaSenderInfo( } void SetOutboundRTPStreamStatsFromVoiceSenderInfo( + const std::string& transport_id, const std::string& mid, + const cricket::VoiceMediaInfo& voice_media_info, const cricket::VoiceSenderInfo& voice_sender_info, RTCOutboundRTPStreamStats* outbound_audio) { SetOutboundRTPStreamStatsFromMediaSenderInfo(voice_sender_info, outbound_audio); + outbound_audio->transport_id = transport_id; outbound_audio->mid = mid; outbound_audio->media_type = "audio"; outbound_audio->kind = "audio"; if (voice_sender_info.target_bitrate > 0) { outbound_audio->target_bitrate = voice_sender_info.target_bitrate; } - if (voice_sender_info.codec_payload_type) { - outbound_audio->codec_id = RTCCodecStatsIDFromMidDirectionAndPayload( - mid, /*inbound=*/false, *voice_sender_info.codec_payload_type); + if (voice_sender_info.codec_payload_type.has_value()) { + auto codec_param_it = voice_media_info.send_codecs.find( + 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( + transport_id, /*inbound=*/false, codec_param_it->second); + } } // `fir_count`, `pli_count` and `sli_count` are only valid for video and are // purposefully left undefined for audio. } void SetOutboundRTPStreamStatsFromVideoSenderInfo( + const std::string& transport_id, const std::string& mid, + const cricket::VideoMediaInfo& video_media_info, const cricket::VideoSenderInfo& video_sender_info, RTCOutboundRTPStreamStats* outbound_video) { SetOutboundRTPStreamStatsFromMediaSenderInfo(video_sender_info, outbound_video); + outbound_video->transport_id = transport_id; outbound_video->mid = mid; outbound_video->media_type = "video"; outbound_video->kind = "video"; - if (video_sender_info.codec_payload_type) { - outbound_video->codec_id = RTCCodecStatsIDFromMidDirectionAndPayload( - mid, /*inbound=*/false, *video_sender_info.codec_payload_type); + if (video_sender_info.codec_payload_type.has_value()) { + auto codec_param_it = video_media_info.send_codecs.find( + 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( + transport_id, /*inbound=*/false, codec_param_it->second); + } } outbound_video->fir_count = static_cast(video_sender_info.firs_rcvd); @@ -1543,6 +1579,19 @@ void RTCStatsCollector::ProduceCodecStats_n( 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; @@ -1550,19 +1599,39 @@ void RTCStatsCollector::ProduceCodecStats_n( 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, *stats.mid, transport_id, true, pair.second)); + timestamp_us, transport_id, true, 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, *stats.mid, transport_id, false, pair.second)); + timestamp_us, transport_id, false, codec)); } } // Video @@ -1570,14 +1639,28 @@ void RTCStatsCollector::ProduceCodecStats_n( // 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, *stats.mid, transport_id, true, pair.second)); + timestamp_us, transport_id, true, 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, *stats.mid, transport_id, false, pair.second)); + timestamp_us, transport_id, false, codec)); } } } @@ -1924,8 +2007,9 @@ void RTCStatsCollector::ProduceAudioRTPStreamStats_n( if (!voice_receiver_info.connected()) continue; // Inbound. - auto inbound_audio = - CreateInboundAudioStreamStats(voice_receiver_info, mid, timestamp_us); + auto inbound_audio = CreateInboundAudioStreamStats( + stats.track_media_info_map.voice_media_info().value(), + voice_receiver_info, transport_id, mid, timestamp_us); // 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); @@ -1937,10 +2021,9 @@ void RTCStatsCollector::ProduceAudioRTPStreamStats_n( .value()); inbound_audio->track_identifier = audio_track->id(); } - inbound_audio->transport_id = transport_id; // Remote-outbound. auto remote_outbound_audio = CreateRemoteOutboundAudioStreamStats( - voice_receiver_info, mid, inbound_audio->id(), transport_id); + voice_receiver_info, mid, *inbound_audio.get(), transport_id); // Add stats. if (remote_outbound_audio) { // When the remote outbound stats are available, the remote ID for the @@ -1960,8 +2043,10 @@ void RTCStatsCollector::ProduceAudioRTPStreamStats_n( RTCOutboundRTPStreamStatsIDFromSSRC(cricket::MEDIA_TYPE_AUDIO, voice_sender_info.ssrc()), timestamp_us); - SetOutboundRTPStreamStatsFromVoiceSenderInfo(mid, voice_sender_info, - outbound_audio.get()); + SetOutboundRTPStreamStatsFromVoiceSenderInfo( + transport_id, mid, + stats.track_media_info_map.voice_media_info().value(), + voice_sender_info, outbound_audio.get()); rtc::scoped_refptr audio_track = stats.track_media_info_map.GetAudioTrack(voice_sender_info); if (audio_track) { @@ -1975,7 +2060,6 @@ void RTCStatsCollector::ProduceAudioRTPStreamStats_n( RTCMediaSourceStatsIDFromKindAndAttachment(cricket::MEDIA_TYPE_AUDIO, attachment_id); } - outbound_audio->transport_id = transport_id; audio_outbound_rtps.insert( std::make_pair(outbound_audio->id(), outbound_audio.get())); report->AddStats(std::move(outbound_audio)); @@ -2018,8 +2102,10 @@ void RTCStatsCollector::ProduceVideoRTPStreamStats_n( RTCInboundRTPStreamStatsIDFromSSRC(cricket::MEDIA_TYPE_VIDEO, video_receiver_info.ssrc()), timestamp_us); - SetInboundRTPStreamStatsFromVideoReceiverInfo(mid, video_receiver_info, - inbound_video.get()); + SetInboundRTPStreamStatsFromVideoReceiverInfo( + transport_id, mid, + stats.track_media_info_map.video_media_info().value(), + video_receiver_info, inbound_video.get()); rtc::scoped_refptr video_track = stats.track_media_info_map.GetVideoTrack(video_receiver_info); if (video_track) { @@ -2030,7 +2116,6 @@ void RTCStatsCollector::ProduceVideoRTPStreamStats_n( .value()); inbound_video->track_identifier = video_track->id(); } - inbound_video->transport_id = transport_id; report->AddStats(std::move(inbound_video)); } // Outbound @@ -2043,8 +2128,10 @@ void RTCStatsCollector::ProduceVideoRTPStreamStats_n( RTCOutboundRTPStreamStatsIDFromSSRC(cricket::MEDIA_TYPE_VIDEO, video_sender_info.ssrc()), timestamp_us); - SetOutboundRTPStreamStatsFromVideoSenderInfo(mid, video_sender_info, - outbound_video.get()); + SetOutboundRTPStreamStatsFromVideoSenderInfo( + transport_id, mid, + stats.track_media_info_map.video_media_info().value(), + video_sender_info, outbound_video.get()); rtc::scoped_refptr video_track = stats.track_media_info_map.GetVideoTrack(video_sender_info); if (video_track) { @@ -2058,7 +2145,6 @@ void RTCStatsCollector::ProduceVideoRTPStreamStats_n( RTCMediaSourceStatsIDFromKindAndAttachment(cricket::MEDIA_TYPE_VIDEO, attachment_id); } - outbound_video->transport_id = transport_id; video_outbound_rtps.insert( std::make_pair(outbound_video->id(), outbound_video.get())); report->AddStats(std::move(outbound_video)); diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc index d3c4584555..871dd4d448 100644 --- a/pc/rtc_stats_collector_unittest.cc +++ b/pc/rtc_stats_collector_unittest.cc @@ -705,7 +705,7 @@ class RTCStatsCollectorTest : public ::testing::Test { ExampleStatsGraph graph; // codec (send) - graph.send_codec_id = "RTCCodec_VideoMid_Outbound_1"; + graph.send_codec_id = "RTCCodec_RTCTransport_TransportName_1_Send_1"; cricket::VideoMediaInfo video_media_info; RtpCodecParameters send_codec; send_codec.payload_type = 1; @@ -713,7 +713,7 @@ class RTCStatsCollectorTest : public ::testing::Test { video_media_info.send_codecs.insert( std::make_pair(send_codec.payload_type, send_codec)); // codec (recv) - graph.recv_codec_id = "RTCCodec_VideoMid_Inbound_2"; + graph.recv_codec_id = "RTCCodec_RTCTransport_TransportName_1_Recv_2"; RtpCodecParameters recv_codec; recv_codec.payload_type = 2; recv_codec.clock_rate = 0; @@ -806,7 +806,7 @@ class RTCStatsCollectorTest : public ::testing::Test { ExampleStatsGraph graph; // codec (send) - graph.send_codec_id = "RTCCodec_VoiceMid_Outbound_1"; + graph.send_codec_id = "RTCCodec_RTCTransport_TransportName_1_Send_1"; cricket::VoiceMediaInfo media_info; RtpCodecParameters send_codec; send_codec.payload_type = 1; @@ -814,7 +814,7 @@ class RTCStatsCollectorTest : public ::testing::Test { media_info.send_codecs.insert( std::make_pair(send_codec.payload_type, send_codec)); // codec (recv) - graph.recv_codec_id = "RTCCodec_VoiceMid_Inbound_2"; + graph.recv_codec_id = "RTCCodec_RTCTransport_TransportName_1_Recv_2"; RtpCodecParameters recv_codec; recv_codec.payload_type = 2; recv_codec.clock_rate = 0; @@ -1062,8 +1062,9 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCodecStats) { rtc::scoped_refptr report = stats_->GetStatsReport(); - RTCCodecStats expected_inbound_audio_codec("RTCCodec_AudioMid_Inbound_1", - report->timestamp_us()); + RTCCodecStats expected_inbound_audio_codec( + "RTCCodec_RTCTransport_TransportName_1_Recv_1_minptime=10;useinbandfec=1", + report->timestamp_us()); expected_inbound_audio_codec.payload_type = 1; expected_inbound_audio_codec.mime_type = "audio/opus"; expected_inbound_audio_codec.clock_rate = 1337; @@ -1071,16 +1072,18 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCodecStats) { expected_inbound_audio_codec.sdp_fmtp_line = "minptime=10;useinbandfec=1"; expected_inbound_audio_codec.transport_id = "RTCTransport_TransportName_1"; - RTCCodecStats expected_outbound_audio_codec("RTCCodec_AudioMid_Outbound_2", - report->timestamp_us()); + RTCCodecStats expected_outbound_audio_codec( + "RTCCodec_RTCTransport_TransportName_1_Send_2", report->timestamp_us()); expected_outbound_audio_codec.payload_type = 2; expected_outbound_audio_codec.mime_type = "audio/isac"; expected_outbound_audio_codec.clock_rate = 1338; expected_outbound_audio_codec.channels = 2; expected_outbound_audio_codec.transport_id = "RTCTransport_TransportName_1"; - RTCCodecStats expected_inbound_video_codec("RTCCodec_VideoMid_Inbound_3", - report->timestamp_us()); + RTCCodecStats expected_inbound_video_codec( + "RTCCodec_RTCTransport_TransportName_1_Recv_3_level-asymmetry-allowed=1;" + "packetization-mode=1;profile-level-id=42001f", + report->timestamp_us()); expected_inbound_video_codec.payload_type = 3; expected_inbound_video_codec.mime_type = "video/H264"; expected_inbound_video_codec.clock_rate = 1339; @@ -1088,8 +1091,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCodecStats) { "level-asymmetry-allowed=1;packetization-mode=1;profile-level-id=42001f"; expected_inbound_video_codec.transport_id = "RTCTransport_TransportName_1"; - RTCCodecStats expected_outbound_video_codec("RTCCodec_VideoMid_Outbound_4", - report->timestamp_us()); + RTCCodecStats expected_outbound_video_codec( + "RTCCodec_RTCTransport_TransportName_1_Send_4", report->timestamp_us()); expected_outbound_video_codec.payload_type = 4; expected_outbound_video_codec.mime_type = "video/VP8"; expected_outbound_video_codec.clock_rate = 1340; @@ -1116,6 +1119,117 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCodecStats) { ->cast_to()); } +TEST_F(RTCStatsCollectorTest, CodecStatsAreCollectedPerTransport) { + // PT=10 + RtpCodecParameters outbound_codec_pt10; + outbound_codec_pt10.payload_type = 10; + outbound_codec_pt10.kind = cricket::MEDIA_TYPE_VIDEO; + outbound_codec_pt10.name = "VP8"; + outbound_codec_pt10.clock_rate = 9000; + + // PT=11 + RtpCodecParameters outbound_codec_pt11; + outbound_codec_pt11.payload_type = 11; + outbound_codec_pt11.kind = cricket::MEDIA_TYPE_VIDEO; + outbound_codec_pt11.name = "VP8"; + outbound_codec_pt11.clock_rate = 9000; + + cricket::VideoMediaInfo info_pt10; + info_pt10.send_codecs.insert( + std::make_pair(outbound_codec_pt10.payload_type, outbound_codec_pt10)); + cricket::VideoMediaInfo info_pt11; + info_pt11.send_codecs.insert( + std::make_pair(outbound_codec_pt11.payload_type, outbound_codec_pt11)); + 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)); + + // First two mids contain subsets, the third one contains all PTs. + pc_->AddVideoChannel("Mid1", "FirstTransport", info_pt10); + pc_->AddVideoChannel("Mid2", "FirstTransport", info_pt11); + pc_->AddVideoChannel("Mid3", "FirstTransport", info_pt10_pt11); + + // There should be no duplicate codecs because all codec references are on the + // same transport. + rtc::scoped_refptr report = stats_->GetStatsReport(); + auto codec_stats = report->GetStatsOfType(); + EXPECT_EQ(codec_stats.size(), 2u); + + // If a second transport is added with the same PT information, this does + // count as different codec objects. + pc_->AddVideoChannel("Mid4", "SecondTransport", info_pt10_pt11); + stats_->stats_collector()->ClearCachedStatsReport(); + report = stats_->GetStatsReport(); + codec_stats = report->GetStatsOfType(); + EXPECT_EQ(codec_stats.size(), 4u); +} + +TEST_F(RTCStatsCollectorTest, SamePayloadTypeButDifferentFmtpLines) { + // PT=111, useinbandfec=0 + RtpCodecParameters inbound_codec_pt111_nofec; + inbound_codec_pt111_nofec.payload_type = 111; + inbound_codec_pt111_nofec.kind = cricket::MEDIA_TYPE_AUDIO; + inbound_codec_pt111_nofec.name = "opus"; + inbound_codec_pt111_nofec.clock_rate = 48000; + inbound_codec_pt111_nofec.parameters.insert( + std::make_pair("useinbandfec", "0")); + + // PT=111, useinbandfec=1 + RtpCodecParameters inbound_codec_pt111_fec; + inbound_codec_pt111_fec.payload_type = 111; + inbound_codec_pt111_fec.kind = cricket::MEDIA_TYPE_AUDIO; + inbound_codec_pt111_fec.name = "opus"; + inbound_codec_pt111_fec.clock_rate = 48000; + inbound_codec_pt111_fec.parameters.insert( + std::make_pair("useinbandfec", "1")); + + cricket::VideoMediaInfo info_nofec; + info_nofec.send_codecs.insert(std::make_pair( + inbound_codec_pt111_nofec.payload_type, inbound_codec_pt111_nofec)); + cricket::VideoMediaInfo info_fec; + info_fec.send_codecs.insert(std::make_pair( + inbound_codec_pt111_fec.payload_type, inbound_codec_pt111_fec)); + + // First two mids contain subsets, the third one contains all PTs. + pc_->AddVideoChannel("Mid1", "BundledTransport", info_nofec); + pc_->AddVideoChannel("Mid2", "BundledTransport", info_fec); + + // Despite having the same PT we should see two codec stats because their FMTP + // lines are different. + rtc::scoped_refptr report = stats_->GetStatsReport(); + auto codec_stats = report->GetStatsOfType(); + EXPECT_EQ(codec_stats.size(), 2u); + + // Adding more m= sections that does have the same FMTP lines does not result + // in duplicates. + pc_->AddVideoChannel("Mid3", "BundledTransport", info_nofec); + pc_->AddVideoChannel("Mid4", "BundledTransport", info_fec); + stats_->stats_collector()->ClearCachedStatsReport(); + report = stats_->GetStatsReport(); + codec_stats = report->GetStatsOfType(); + EXPECT_EQ(codec_stats.size(), 2u); + + // Same FMTP line but a different PT does count as a new codec. + // PT=112, useinbandfec=1 + RtpCodecParameters inbound_codec_pt112_fec; + inbound_codec_pt112_fec.payload_type = 112; + inbound_codec_pt112_fec.kind = cricket::MEDIA_TYPE_AUDIO; + inbound_codec_pt112_fec.name = "opus"; + inbound_codec_pt112_fec.clock_rate = 48000; + 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( + inbound_codec_pt112_fec.payload_type, inbound_codec_pt112_fec)); + pc_->AddVideoChannel("Mid5", "BundledTransport", info_fec_pt112); + stats_->stats_collector()->ClearCachedStatsReport(); + report = stats_->GetStatsReport(); + codec_stats = report->GetStatsOfType(); + EXPECT_EQ(codec_stats.size(), 3u); +} + TEST_F(RTCStatsCollectorTest, CollectRTCCertificateStatsMultiple) { const char kAudioTransport[] = "audio"; const char kVideoTransport[] = "video"; @@ -2102,7 +2216,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCInboundRTPStreamStats_Audio) { expected_audio.mid = "AudioMid"; expected_audio.track_id = stats_of_track_type[0]->id(); expected_audio.transport_id = "RTCTransport_TransportName_1"; - expected_audio.codec_id = "RTCCodec_AudioMid_Inbound_42"; + expected_audio.codec_id = "RTCCodec_RTCTransport_TransportName_1_Recv_42"; expected_audio.packets_received = 2; expected_audio.nack_count = 5; expected_audio.fec_packets_discarded = 5566; @@ -2222,7 +2336,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCInboundRTPStreamStats_Video) { expected_video.mid = "VideoMid"; expected_video.track_id = IdForType(report.get()); expected_video.transport_id = "RTCTransport_TransportName_1"; - expected_video.codec_id = "RTCCodec_VideoMid_Inbound_42"; + expected_video.codec_id = "RTCCodec_RTCTransport_TransportName_1_Recv_42"; expected_video.fir_count = 5; expected_video.pli_count = 6; expected_video.nack_count = 7; @@ -2322,7 +2436,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Audio) { expected_audio.kind = "audio"; expected_audio.track_id = IdForType(report.get()); expected_audio.transport_id = "RTCTransport_TransportName_1"; - expected_audio.codec_id = "RTCCodec_AudioMid_Outbound_42"; + expected_audio.codec_id = "RTCCodec_RTCTransport_TransportName_1_Send_42"; expected_audio.packets_sent = 2; expected_audio.retransmitted_packets_sent = 20; expected_audio.bytes_sent = 3; @@ -2412,7 +2526,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Video) { expected_video.kind = "video"; expected_video.track_id = stats_of_track_type[0]->id(); expected_video.transport_id = "RTCTransport_TransportName_1"; - expected_video.codec_id = "RTCCodec_VideoMid_Outbound_42"; + expected_video.codec_id = "RTCCodec_RTCTransport_TransportName_1_Send_42"; expected_video.fir_count = 2; expected_video.pli_count = 3; expected_video.nack_count = 4; @@ -2756,7 +2870,7 @@ TEST_F(RTCStatsCollectorTest, CollectNoStreamRTCOutboundRTPStreamStats_Audio) { expected_audio.kind = "audio"; expected_audio.track_id = IdForType(report.get()); expected_audio.transport_id = "RTCTransport_TransportName_1"; - expected_audio.codec_id = "RTCCodec_AudioMid_Outbound_42"; + expected_audio.codec_id = "RTCCodec_RTCTransport_TransportName_1_Send_42"; expected_audio.packets_sent = 2; expected_audio.retransmitted_packets_sent = 20; expected_audio.bytes_sent = 3; diff --git a/pc/test/fake_peer_connection_for_stats.h b/pc/test/fake_peer_connection_for_stats.h index 9daf3739eb..d4ccfa30bd 100644 --- a/pc/test/fake_peer_connection_for_stats.h +++ b/pc/test/fake_peer_connection_for_stats.h @@ -224,10 +224,17 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { worker_thread_, network_thread_, signaling_thread_, std::move(voice_media_channel), mid, kDefaultSrtpRequired, webrtc::CryptoOptions(), context_->ssrc_generator(), transport_name); - GetOrCreateFirstTransceiverOfType(cricket::MEDIA_TYPE_AUDIO) - ->internal() - ->SetChannel(std::move(voice_channel), - [](const std::string&) { return nullptr; }); + auto transceiver = + GetOrCreateFirstTransceiverOfType(cricket::MEDIA_TYPE_AUDIO) + ->internal(); + if (transceiver->channel()) { + // This transceiver already has a channel, create a new one. + transceiver = + CreateTransceiverOfType(cricket::MEDIA_TYPE_AUDIO)->internal(); + } + RTC_DCHECK(!transceiver->channel()); + transceiver->SetChannel(std::move(voice_channel), + [](const std::string&) { return nullptr; }); voice_media_channel_ptr->SetStats(initial_stats); return voice_media_channel_ptr; } @@ -243,10 +250,17 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { worker_thread_, network_thread_, signaling_thread_, std::move(video_media_channel), mid, kDefaultSrtpRequired, webrtc::CryptoOptions(), context_->ssrc_generator(), transport_name); - GetOrCreateFirstTransceiverOfType(cricket::MEDIA_TYPE_VIDEO) - ->internal() - ->SetChannel(std::move(video_channel), - [](const std::string&) { return nullptr; }); + auto transceiver = + GetOrCreateFirstTransceiverOfType(cricket::MEDIA_TYPE_VIDEO) + ->internal(); + if (transceiver->channel()) { + // This transceiver already has a channel, create a new one. + transceiver = + CreateTransceiverOfType(cricket::MEDIA_TYPE_VIDEO)->internal(); + } + RTC_DCHECK(!transceiver->channel()); + transceiver->SetChannel(std::move(video_channel), + [](const std::string&) { return nullptr; }); video_media_channel_ptr->SetStats(initial_stats); return video_media_channel_ptr; } @@ -413,6 +427,11 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { return transceiver; } } + return CreateTransceiverOfType(media_type); + } + + rtc::scoped_refptr> + CreateTransceiverOfType(cricket::MediaType media_type) { auto transceiver = RtpTransceiverProxyWithInternal::Create( signaling_thread_, rtc::make_ref_counted(media_type, context_.get()));