Fix inaccurate contentType in RTCInbound/OutboundRtpStreamStats
The existing equality check did not always work since content_type
is sometimes overloaded with extra internal information such as simulcast layer index. Fix by using the videocontenttypehelpers::IsScreenshare helper method.
Bug: webrtc:15381
Change-Id: I2fe84e7f036ea2c223e4fa6dd58af1c4c0bcfbdb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/312261
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40448}
diff --git a/api/video/video_content_type.h b/api/video/video_content_type.h
index 2d38a62..2251b4a 100644
--- a/api/video/video_content_type.h
+++ b/api/video/video_content_type.h
@@ -15,6 +15,9 @@
namespace webrtc {
+// VideoContentType can take on more values than the two below, therefore care
+// should be taken to avoid using the equality operator to check for screenshare
+// usage. See https://bugs.chromium.org/p/webrtc/issues/detail?id=15381.
enum class VideoContentType : uint8_t {
UNSPECIFIED = 0,
SCREENSHARE = 1,
diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc
index 334c76f..07e8771 100644
--- a/pc/rtc_stats_collector.cc
+++ b/pc/rtc_stats_collector.cc
@@ -667,7 +667,7 @@
}
// TODO(bugs.webrtc.org/10529): When info's `content_info` is optional
// support the "unspecified" value.
- if (video_receiver_info.content_type == VideoContentType::SCREENSHARE)
+ if (videocontenttypehelpers::IsScreenshare(video_receiver_info.content_type))
inbound_video->content_type = "screenshare";
if (video_receiver_info.decoder_implementation_name.has_value()) {
inbound_video->decoder_implementation =
@@ -809,7 +809,7 @@
video_sender_info.quality_limitation_resolution_changes;
// TODO(https://crbug.com/webrtc/10529): When info's `content_info` is
// optional, support the "unspecified" value.
- if (video_sender_info.content_type == VideoContentType::SCREENSHARE)
+ if (videocontenttypehelpers::IsScreenshare(video_sender_info.content_type))
outbound_video->content_type = "screenshare";
if (video_sender_info.encoder_implementation_name.has_value()) {
outbound_video->encoder_implementation =
diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc
index 8781b32..b0b99cc 100644
--- a/pc/rtc_stats_collector_unittest.cc
+++ b/pc/rtc_stats_collector_unittest.cc
@@ -2464,6 +2464,18 @@
expected_video);
EXPECT_TRUE(report->Get(*expected_video.transport_id));
EXPECT_TRUE(report->Get(*expected_video.codec_id));
+
+ // Make sure content type is still reported as "screenshare" even when the
+ // VideoContentType enum is overloaded with additional information.
+ videocontenttypehelpers::SetSimulcastId(
+ &video_media_info.receivers[0].content_type, 2);
+ video_media_channels.first->SetStats(video_media_info);
+ video_media_channels.second->SetStats(video_media_info);
+
+ report = stats_->GetFreshStatsReport();
+ EXPECT_EQ(
+ report->Get(expected_video.id())->cast_to<RTCInboundRtpStreamStats>(),
+ expected_video);
}
TEST_F(RTCStatsCollectorTest, CollectRTCAudioPlayoutStats) {
@@ -2714,6 +2726,18 @@
expected_video);
EXPECT_TRUE(report->Get(*expected_video.transport_id));
EXPECT_TRUE(report->Get(*expected_video.codec_id));
+
+ // Make sure content type is still reported as "screenshare" even when the
+ // VideoContentType enum is overloaded with additional information.
+ videocontenttypehelpers::SetSimulcastId(
+ &video_media_info.senders[0].content_type, 2);
+ video_media_channels.first->SetStats(video_media_info);
+ video_media_channels.second->SetStats(video_media_info);
+
+ report = stats_->GetFreshStatsReport();
+ EXPECT_EQ(
+ report->Get(expected_video.id())->cast_to<RTCOutboundRtpStreamStats>(),
+ expected_video);
}
TEST_F(RTCStatsCollectorTest, CollectRTCTransportStats) {
