[Stats] Add support for SSRC collisions.
In non-BUNDLE use cases, it is possible for multiple RTP streams to have
the same SSRC (as long as the SSRC is unique within the same transport).
This CL adds support for "outbound-rtp" and "inbound-rtp" stream stats
to have the same SSRC on different transports by adding the transport to
the stats ID. This avoids multiple RTP stream stats having the same
stats ID and fixes the problem. It's a stupid use case, but it should
work.
There could still be a stats ID collision in the event of multiple
"remote-inbound-rtp" or "remote-outbound-rtp" reference the same SSRC
but on separate transports for the same reason, and would require the
same fix... but one bug at a time. Not addressed in this CL.
Bug: webrtc:14443
Change-Id: I1a2ffd79fc67c2765e6dbd1ccc6828d4e91c4589
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275769
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38201}
diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc
index 569312c..464e22c 100644
--- a/pc/rtc_stats_collector.cc
+++ b/pc/rtc_stats_collector.cc
@@ -114,19 +114,23 @@
return sb.str();
}
-std::string RTCInboundRTPStreamStatsIDFromSSRC(cricket::MediaType media_type,
+std::string RTCInboundRTPStreamStatsIDFromSSRC(const std::string& transport_id,
+ cricket::MediaType media_type,
uint32_t ssrc) {
char buf[1024];
rtc::SimpleStringBuilder sb(buf);
- sb << 'I' << (media_type == cricket::MEDIA_TYPE_AUDIO ? 'A' : 'V') << ssrc;
+ sb << 'I' << transport_id
+ << (media_type == cricket::MEDIA_TYPE_AUDIO ? 'A' : 'V') << ssrc;
return sb.str();
}
-std::string RTCOutboundRTPStreamStatsIDFromSSRC(cricket::MediaType media_type,
+std::string RTCOutboundRTPStreamStatsIDFromSSRC(const std::string& transport_id,
+ cricket::MediaType media_type,
uint32_t ssrc) {
char buf[1024];
rtc::SimpleStringBuilder sb(buf);
- sb << 'O' << (media_type == cricket::MEDIA_TYPE_AUDIO ? 'A' : 'V') << ssrc;
+ sb << 'O' << transport_id
+ << (media_type == cricket::MEDIA_TYPE_AUDIO ? 'A' : 'V') << ssrc;
return sb.str();
}
@@ -425,8 +429,8 @@
const std::string& mid,
int64_t timestamp_us) {
auto inbound_audio = std::make_unique<RTCInboundRTPStreamStats>(
- /*id=*/RTCInboundRTPStreamStatsIDFromSSRC(cricket::MEDIA_TYPE_AUDIO,
- voice_receiver_info.ssrc()),
+ /*id=*/RTCInboundRTPStreamStatsIDFromSSRC(
+ transport_id, cricket::MEDIA_TYPE_AUDIO, voice_receiver_info.ssrc()),
timestamp_us);
SetInboundRTPStreamStatsFromMediaReceiverInfo(voice_receiver_info,
inbound_audio.get());
@@ -740,6 +744,7 @@
std::unique_ptr<RTCRemoteInboundRtpStreamStats>
ProduceRemoteInboundRtpStreamStatsFromReportBlockData(
+ const std::string& transport_id,
const ReportBlockData& report_block_data,
cricket::MediaType media_type,
const std::map<std::string, RTCOutboundRTPStreamStats*>& outbound_rtps,
@@ -767,8 +772,8 @@
remote_inbound->round_trip_time_measurements =
report_block_data.num_rtts();
- std::string local_id =
- RTCOutboundRTPStreamStatsIDFromSSRC(media_type, report_block.source_ssrc);
+ std::string local_id = RTCOutboundRTPStreamStatsIDFromSSRC(
+ transport_id, media_type, report_block.source_ssrc);
// Look up local stat from `outbound_rtps` where the pointers are non-const.
auto local_id_it = outbound_rtps.find(local_id);
if (local_id_it != outbound_rtps.end()) {
@@ -777,9 +782,7 @@
outbound_rtp.remote_id = remote_inbound->id();
// The RTP/RTCP transport is obtained from the
// RTCOutboundRtpStreamStats's transport.
- const auto* transport_from_id = outbound_rtp.transport_id.is_defined()
- ? report.Get(*outbound_rtp.transport_id)
- : nullptr;
+ const auto* transport_from_id = report.Get(transport_id);
if (transport_from_id) {
const auto& transport = transport_from_id->cast_to<RTCTransportStats>();
// If RTP and RTCP are not multiplexed, there is a separate RTCP
@@ -2050,8 +2053,8 @@
if (!voice_sender_info.connected())
continue;
auto outbound_audio = std::make_unique<RTCOutboundRTPStreamStats>(
- RTCOutboundRTPStreamStatsIDFromSSRC(cricket::MEDIA_TYPE_AUDIO,
- voice_sender_info.ssrc()),
+ RTCOutboundRTPStreamStatsIDFromSSRC(
+ transport_id, cricket::MEDIA_TYPE_AUDIO, voice_sender_info.ssrc()),
timestamp_us);
SetOutboundRTPStreamStatsFromVoiceSenderInfo(
transport_id, mid,
@@ -2088,8 +2091,8 @@
stats.track_media_info_map.voice_media_info()->senders) {
for (const auto& report_block_data : voice_sender_info.report_block_datas) {
report->AddStats(ProduceRemoteInboundRtpStreamStatsFromReportBlockData(
- report_block_data, cricket::MEDIA_TYPE_AUDIO, audio_outbound_rtps,
- *report));
+ transport_id, report_block_data, cricket::MEDIA_TYPE_AUDIO,
+ audio_outbound_rtps, *report));
}
}
}
@@ -2114,7 +2117,8 @@
if (!video_receiver_info.connected())
continue;
auto inbound_video = std::make_unique<RTCInboundRTPStreamStats>(
- RTCInboundRTPStreamStatsIDFromSSRC(cricket::MEDIA_TYPE_VIDEO,
+ RTCInboundRTPStreamStatsIDFromSSRC(transport_id,
+ cricket::MEDIA_TYPE_VIDEO,
video_receiver_info.ssrc()),
timestamp_us);
SetInboundRTPStreamStatsFromVideoReceiverInfo(
@@ -2143,8 +2147,8 @@
if (!video_sender_info.connected())
continue;
auto outbound_video = std::make_unique<RTCOutboundRTPStreamStats>(
- RTCOutboundRTPStreamStatsIDFromSSRC(cricket::MEDIA_TYPE_VIDEO,
- video_sender_info.ssrc()),
+ RTCOutboundRTPStreamStatsIDFromSSRC(
+ transport_id, cricket::MEDIA_TYPE_VIDEO, video_sender_info.ssrc()),
timestamp_us);
SetOutboundRTPStreamStatsFromVideoSenderInfo(
transport_id, mid,
@@ -2181,8 +2185,8 @@
stats.track_media_info_map.video_media_info()->senders) {
for (const auto& report_block_data : video_sender_info.report_block_datas) {
report->AddStats(ProduceRemoteInboundRtpStreamStatsFromReportBlockData(
- report_block_data, cricket::MEDIA_TYPE_VIDEO, video_outbound_rtps,
- *report));
+ transport_id, report_block_data, cricket::MEDIA_TYPE_VIDEO,
+ video_outbound_rtps, *report));
}
}
}
diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc
index 8ffdf72..9d7ae24 100644
--- a/pc/rtc_stats_collector_unittest.cc
+++ b/pc/rtc_stats_collector_unittest.cc
@@ -720,7 +720,7 @@
video_media_info.receive_codecs.insert(
std::make_pair(recv_codec.payload_type, recv_codec));
// outbound-rtp
- graph.outbound_rtp_id = "OV3";
+ graph.outbound_rtp_id = "OTTransportName1V3";
video_media_info.senders.push_back(cricket::VideoSenderInfo());
video_media_info.senders[0].local_stats.push_back(
cricket::SsrcSenderInfo());
@@ -728,7 +728,7 @@
video_media_info.senders[0].codec_payload_type = send_codec.payload_type;
video_media_info.aggregated_senders.push_back(video_media_info.senders[0]);
// inbound-rtp
- graph.inbound_rtp_id = "IV4";
+ graph.inbound_rtp_id = "ITTransportName1V4";
video_media_info.receivers.push_back(cricket::VideoReceiverInfo());
video_media_info.receivers[0].local_stats.push_back(
cricket::SsrcReceiverInfo());
@@ -820,13 +820,13 @@
media_info.receive_codecs.insert(
std::make_pair(recv_codec.payload_type, recv_codec));
// outbound-rtp
- graph.outbound_rtp_id = "OA3";
+ graph.outbound_rtp_id = "OTTransportName1A3";
media_info.senders.push_back(cricket::VoiceSenderInfo());
media_info.senders[0].local_stats.push_back(cricket::SsrcSenderInfo());
media_info.senders[0].local_stats[0].ssrc = kLocalSsrc;
media_info.senders[0].codec_payload_type = send_codec.payload_type;
// inbound-rtp
- graph.inbound_rtp_id = "IA4";
+ graph.inbound_rtp_id = "ITTransportName1A4";
media_info.receivers.push_back(cricket::VoiceReceiverInfo());
media_info.receivers[0].local_stats.push_back(cricket::SsrcReceiverInfo());
media_info.receivers[0].local_stats[0].ssrc = kRemoteSsrc;
@@ -1043,10 +1043,8 @@
rtc::scoped_refptr<const RTCStatsReport> report = stats_->GetStatsReport();
auto inbound_rtps = report->GetStatsOfType<RTCInboundRTPStreamStats>();
auto outbound_rtps = report->GetStatsOfType<RTCOutboundRTPStreamStats>();
- // TODO(https://crbug.com/webrtc/14443): When valid SSRC collisions are
- // handled correctly, we should expect to see 4 of each type of object here.
- EXPECT_EQ(inbound_rtps.size(), 2u);
- EXPECT_EQ(outbound_rtps.size(), 2u);
+ EXPECT_EQ(inbound_rtps.size(), 4u);
+ EXPECT_EQ(outbound_rtps.size(), 4u);
}
// These SSRC collisions are illegal, so it is not clear if this setup can
@@ -2259,7 +2257,8 @@
auto stats_of_track_type = report->GetStatsOfType<RTCMediaStreamTrackStats>();
ASSERT_EQ(1U, stats_of_track_type.size());
- RTCInboundRTPStreamStats expected_audio("IA1", report->timestamp_us());
+ RTCInboundRTPStreamStats expected_audio("ITTransportName1A1",
+ report->timestamp_us());
expected_audio.ssrc = 1;
expected_audio.media_type = "audio";
expected_audio.kind = "audio";
@@ -2378,7 +2377,8 @@
rtc::scoped_refptr<const RTCStatsReport> report = stats_->GetStatsReport();
- RTCInboundRTPStreamStats expected_video("IV1", report->timestamp_us());
+ RTCInboundRTPStreamStats expected_video("ITTransportName1V1",
+ report->timestamp_us());
expected_video.ssrc = 1;
expected_video.media_type = "video";
expected_video.kind = "video";
@@ -2476,7 +2476,8 @@
rtc::scoped_refptr<const RTCStatsReport> report = stats_->GetStatsReport();
- RTCOutboundRTPStreamStats expected_audio("OA1", report->timestamp_us());
+ RTCOutboundRTPStreamStats expected_audio("OTTransportName1A1",
+ report->timestamp_us());
expected_audio.media_source_id = "SA50";
// `expected_audio.remote_id` should be undefined.
expected_audio.mid = "AudioMid";
@@ -2906,7 +2907,8 @@
rtc::scoped_refptr<const RTCStatsReport> report = stats_->GetStatsReport();
- RTCOutboundRTPStreamStats expected_audio("OA1", report->timestamp_us());
+ RTCOutboundRTPStreamStats expected_audio("OTTransportName1A1",
+ report->timestamp_us());
expected_audio.media_source_id = "SA50";
expected_audio.mid = "AudioMid";
expected_audio.ssrc = 1;
@@ -3234,7 +3236,8 @@
"TTransportName1"; // 1 for RTP (we have no RTCP
// transport)
expected_remote_inbound_rtp.packets_lost = 7;
- expected_remote_inbound_rtp.local_id = "O" + MediaTypeCharStr() + stream_id;
+ expected_remote_inbound_rtp.local_id =
+ "OTTransportName1" + MediaTypeCharStr() + stream_id;
expected_remote_inbound_rtp.round_trip_time = kRoundTripTimeSample2Seconds;
expected_remote_inbound_rtp.total_round_trip_time =
kRoundTripTimeSample1Seconds + kRoundTripTimeSample2Seconds;
