From 2e06926c95f32b98cd911203cf6418aa48bf56ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Tue, 9 Apr 2019 13:59:31 +0200 Subject: [PATCH] Implement RTC[In/Out]boundRtpStreamStats.contentType. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spec: https://henbos.github.io/webrtc-provisional-stats/#dom-rtcinboundrtpstreamstats-contenttype This already exists as a goog-stat. This CL only plumbs the value to the new stats collector. Note: There is currently no distinction between the extension being missing and it being present but the value being "unspecified". Until https://crbug.com/webrtc/10529 is fixed, this metric is only exposed if SCREENSHARE was present. Bug: webrtc:10452 Change-Id: Ic8723f4d0efb43ab72a560e954676facd3b90659 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/131946 Reviewed-by: Harald Alvestrand Commit-Queue: Henrik Boström Cr-Commit-Position: refs/heads/master@{#27520} --- api/stats/rtcstats_objects.h | 10 ++++++++++ pc/rtc_stats_collector.cc | 9 +++++++++ pc/rtc_stats_collector_unittest.cc | 8 ++++++++ pc/rtc_stats_integrationtest.cc | 8 ++++++++ stats/rtcstats_objects.cc | 22 ++++++++++++++++------ 5 files changed, 51 insertions(+), 6 deletions(-) diff --git a/api/stats/rtcstats_objects.h b/api/stats/rtcstats_objects.h index 158ea92f88..8a1fe56b43 100644 --- a/api/stats/rtcstats_objects.h +++ b/api/stats/rtcstats_objects.h @@ -74,6 +74,12 @@ struct RTCNetworkType { static const char* const kUnknown; }; +// https://webrtc.org/experiments/rtp-hdrext/video-content-type/ +struct RTCContentType { + static const char* const kUnspecified; + static const char* const kScreenshare; +}; + // https://w3c.github.io/webrtc-stats/#certificatestats-dict* class RTC_EXPORT RTCCertificateStats final : public RTCStats { public: @@ -418,6 +424,8 @@ class RTC_EXPORT RTCInboundRTPStreamStats final : public RTCRTPStreamStats { // TODO(hbos): Collect and populate this value. https://bugs.webrtc.org/7065 RTCStatsMember gap_discard_rate; RTCStatsMember frames_decoded; + // https://henbos.github.io/webrtc-provisional-stats/#dom-rtcinboundrtpstreamstats-contenttype + RTCStatsMember content_type; }; // https://w3c.github.io/webrtc-stats/#outboundrtpstats-dict* @@ -438,6 +446,8 @@ class RTC_EXPORT RTCOutboundRTPStreamStats final : public RTCRTPStreamStats { RTCStatsMember target_bitrate; RTCStatsMember frames_encoded; RTCStatsMember total_encode_time; + // https://henbos.github.io/webrtc-provisional-stats/#dom-rtcoutboundrtpstreamstats-contenttype + RTCStatsMember content_type; }; // https://w3c.github.io/webrtc-stats/#transportstats-dict* diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc index df2f09ca0f..74273562a7 100644 --- a/pc/rtc_stats_collector.cc +++ b/pc/rtc_stats_collector.cc @@ -19,6 +19,7 @@ #include "api/candidate.h" #include "api/media_stream_interface.h" #include "api/peer_connection_interface.h" +#include "api/video/video_content_type.h" #include "media/base/media_channel.h" #include "p2p/base/p2p_constants.h" #include "p2p/base/port.h" @@ -266,6 +267,10 @@ void SetInboundRTPStreamStatsFromVideoReceiverInfo( inbound_video->frames_decoded = video_receiver_info.frames_decoded; if (video_receiver_info.qp_sum) inbound_video->qp_sum = *video_receiver_info.qp_sum; + // TODO(https://crbug.com/webrtc/10529): When info's |content_info| is + // optional, support the "unspecified" value. + if (video_receiver_info.content_type == VideoContentType::SCREENSHARE) + inbound_video->content_type = RTCContentType::kScreenshare; } // Provides the media independent counters (both audio and video). @@ -322,6 +327,10 @@ void SetOutboundRTPStreamStatsFromVideoSenderInfo( outbound_video->total_encode_time = static_cast(video_sender_info.total_encode_time_ms) / rtc::kNumMillisecsPerSec; + // 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) + outbound_video->content_type = RTCContentType::kScreenshare; } void ProduceCertificateStatsFromSSLCertificateStats( diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc index ec2a9eba95..5b0d1644b4 100644 --- a/pc/rtc_stats_collector_unittest.cc +++ b/pc/rtc_stats_collector_unittest.cc @@ -1684,6 +1684,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCInboundRTPStreamStats_Video) { video_media_info.receivers[0].nacks_sent = 7; video_media_info.receivers[0].frames_decoded = 8; video_media_info.receivers[0].qp_sum = absl::nullopt; + video_media_info.receivers[0].content_type = VideoContentType::UNSPECIFIED; RtpCodecParameters codec_parameters; codec_parameters.payload_type = 42; @@ -1718,6 +1719,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCInboundRTPStreamStats_Video) { expected_video.fraction_lost = 4.5; expected_video.frames_decoded = 8; // |expected_video.qp_sum| should be undefined. + // |expected_video.content_type| should be undefined. ASSERT_TRUE(report->Get(expected_video.id())); EXPECT_EQ( @@ -1727,6 +1729,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCInboundRTPStreamStats_Video) { // Set previously undefined values and "GetStats" again. video_media_info.receivers[0].qp_sum = 9; expected_video.qp_sum = 9; + video_media_info.receivers[0].content_type = VideoContentType::SCREENSHARE; + expected_video.content_type = "screenshare"; video_media_channel->SetStats(video_media_info); report = stats_->GetFreshStatsReport(); @@ -1806,6 +1810,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Video) { video_media_info.senders[0].frames_encoded = 8; video_media_info.senders[0].total_encode_time_ms = 9000; video_media_info.senders[0].qp_sum = absl::nullopt; + video_media_info.senders[0].content_type = VideoContentType::UNSPECIFIED; RtpCodecParameters codec_parameters; codec_parameters.payload_type = 42; @@ -1843,6 +1848,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Video) { expected_video.bytes_sent = 6; expected_video.frames_encoded = 8; expected_video.total_encode_time = 9.0; + // |expected_video.content_type| should be undefined. // |expected_video.qp_sum| should be undefined. ASSERT_TRUE(report->Get(expected_video.id())); @@ -1853,6 +1859,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Video) { // Set previously undefined values and "GetStats" again. video_media_info.senders[0].qp_sum = 9; expected_video.qp_sum = 9; + video_media_info.senders[0].content_type = VideoContentType::SCREENSHARE; + expected_video.content_type = "screenshare"; video_media_channel->SetStats(video_media_info); report = stats_->GetFreshStatsReport(); diff --git a/pc/rtc_stats_integrationtest.cc b/pc/rtc_stats_integrationtest.cc index 13eb21362d..174e78e76a 100644 --- a/pc/rtc_stats_integrationtest.cc +++ b/pc/rtc_stats_integrationtest.cc @@ -740,8 +740,12 @@ class RTCStatsReportVerifier { if (inbound_stream.media_type.is_defined() && *inbound_stream.media_type == "video") { verifier.TestMemberIsDefined(inbound_stream.frames_decoded); + // The integration test is not set up to test screen share; don't require + // this to be present. + verifier.MarkMemberTested(inbound_stream.content_type, true); } else { verifier.TestMemberIsUndefined(inbound_stream.frames_decoded); + verifier.TestMemberIsUndefined(inbound_stream.content_type); } return verifier.ExpectAllMembersSuccessfullyTested(); } @@ -764,9 +768,13 @@ class RTCStatsReportVerifier { verifier.TestMemberIsDefined(outbound_stream.frames_encoded); verifier.TestMemberIsNonNegative( outbound_stream.total_encode_time); + // The integration test is not set up to test screen share; don't require + // this to be present. + verifier.MarkMemberTested(outbound_stream.content_type, true); } else { verifier.TestMemberIsUndefined(outbound_stream.frames_encoded); verifier.TestMemberIsUndefined(outbound_stream.total_encode_time); + verifier.TestMemberIsUndefined(outbound_stream.content_type); } return verifier.ExpectAllMembersSuccessfullyTested(); } diff --git a/stats/rtcstats_objects.cc b/stats/rtcstats_objects.cc index de317aa4ee..7c023d1822 100644 --- a/stats/rtcstats_objects.cc +++ b/stats/rtcstats_objects.cc @@ -53,6 +53,10 @@ const char* const RTCNetworkType::kWimax = "wimax"; const char* const RTCNetworkType::kVpn = "vpn"; const char* const RTCNetworkType::kUnknown = "unknown"; +// https://webrtc.org/experiments/rtp-hdrext/video-content-type/ +const char* const RTCContentType::kUnspecified = "unspecified"; +const char* const RTCContentType::kScreenshare = "screenshare"; + // clang-format off WEBRTC_RTCSTATS_IMPL(RTCCertificateStats, RTCStats, "certificate", &fingerprint, @@ -587,7 +591,8 @@ WEBRTC_RTCSTATS_IMPL( &burst_discard_rate, &gap_loss_rate, &gap_discard_rate, - &frames_decoded) + &frames_decoded, + &content_type) // clang-format on RTCInboundRTPStreamStats::RTCInboundRTPStreamStats(const std::string& id, @@ -613,7 +618,8 @@ RTCInboundRTPStreamStats::RTCInboundRTPStreamStats(std::string&& id, burst_discard_rate("burstDiscardRate"), gap_loss_rate("gapLossRate"), gap_discard_rate("gapDiscardRate"), - frames_decoded("framesDecoded") {} + frames_decoded("framesDecoded"), + content_type("contentType") {} RTCInboundRTPStreamStats::RTCInboundRTPStreamStats( const RTCInboundRTPStreamStats& other) @@ -634,7 +640,8 @@ RTCInboundRTPStreamStats::RTCInboundRTPStreamStats( burst_discard_rate(other.burst_discard_rate), gap_loss_rate(other.gap_loss_rate), gap_discard_rate(other.gap_discard_rate), - frames_decoded(other.frames_decoded) {} + frames_decoded(other.frames_decoded), + content_type(other.content_type) {} RTCInboundRTPStreamStats::~RTCInboundRTPStreamStats() {} @@ -645,7 +652,8 @@ WEBRTC_RTCSTATS_IMPL( &bytes_sent, &target_bitrate, &frames_encoded, - &total_encode_time) + &total_encode_time, + &content_type) // clang-format on RTCOutboundRTPStreamStats::RTCOutboundRTPStreamStats(const std::string& id, @@ -659,7 +667,8 @@ RTCOutboundRTPStreamStats::RTCOutboundRTPStreamStats(std::string&& id, bytes_sent("bytesSent"), target_bitrate("targetBitrate"), frames_encoded("framesEncoded"), - total_encode_time("totalEncodeTime") {} + total_encode_time("totalEncodeTime"), + content_type("contentType") {} RTCOutboundRTPStreamStats::RTCOutboundRTPStreamStats( const RTCOutboundRTPStreamStats& other) @@ -668,7 +677,8 @@ RTCOutboundRTPStreamStats::RTCOutboundRTPStreamStats( bytes_sent(other.bytes_sent), target_bitrate(other.target_bitrate), frames_encoded(other.frames_encoded), - total_encode_time(other.total_encode_time) {} + total_encode_time(other.total_encode_time), + content_type(other.content_type) {} RTCOutboundRTPStreamStats::~RTCOutboundRTPStreamStats() {}