admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix inaccurate contentType in RTCInbound/OutboundRtpStreamStats

Browse Source 
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}
This commit is contained in:
Joachim Reiersen 2023-07-19 12:47:29 -07:00 committed by WebRTC LUCI CQ
parent 2206b63af0
commit e9e03a9160
3 changed files with 29 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
api/video/video_content_type.h
View File

@ -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,

4
pc/rtc_stats_collector.cc
View File

@ -667,7 +667,7 @@ CreateInboundRTPStreamStatsFromVideoReceiverInfo(
}
// 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 @@ CreateOutboundRTPStreamStatsFromVideoSenderInfo(
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 =

24
pc/rtc_stats_collector_unittest.cc
View File

@ -2464,6 +2464,18 @@ TEST_F(RTCStatsCollectorTest, CollectRTCInboundRtpStreamStats_Video) {
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 @@ TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRtpStreamStats_Video) {
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) {