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

Reland "Only include payload in bytes sent/received."

Browse Source 
This is a reland of 74a1b4b1321b426392d4c32e4a02361226ad5358

Original change's description:
> Only include payload in bytes sent/received.
>
> According to https://www.w3.org/TR/webrtc-stats/#sentrtpstats-dict* and
> https://tools.ietf.org/html/rfc3550#section-6.4.1, the bytes sent
> statistic should not include headers or padding.
>
> Similarly, according to
> https://www.w3.org/TR/webrtc-stats/#inboundrtpstats-dict*, bytes
> received are calculated the same way as bytes sent (eg. not including
> padding or headers).
>
> This change stops adding padding and headers to these statistics.
>
> Bug: webrtc:8516,webrtc:10525
> Change-Id: I891ad5a11a493cc3212afe93e13f62795bf4031f
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146180
> Reviewed-by: Stefan Holmer <stefan@webrtc.org>
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Steve Anton <steveanton@webrtc.org>
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
> Reviewed-by: Oskar Sundbom <ossu@webrtc.org>
> Commit-Queue: Bjorn Mellem <mellem@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#28647}

Bug: webrtc:8516, webrtc:10525
Change-Id: Iaa1613e5becdfaa0af0f6b9f00e5b871937a719c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/147520
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Fredrik Solenberg <solenberg@webrtc.org>
Commit-Queue: Bjorn Mellem <mellem@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28731}
This commit is contained in:
Bjorn A Mellem 2019-07-30 08:34:03 -07:00 committed by Commit Bot
parent fd643a4782
commit da4f09315f
5 changed files with 88 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
audio/channel_receive.cc
View File

@ -44,6 +44,7 @@
#include "rtc_base/race_checker.h"
#include "rtc_base/thread_checker.h"
#include "rtc_base/time_utils.h"
#include "system_wrappers/include/field_trial.h"
#include "system_wrappers/include/metrics.h"
namespace webrtc {
@ -57,6 +58,11 @@ constexpr double kAudioSampleDurationSeconds = 0.01;
constexpr int kVoiceEngineMinMinPlayoutDelayMs = 0;
constexpr int kVoiceEngineMaxMinPlayoutDelayMs = 10000;
// Field trial which controls whether to report standard-compliant bytes
// sent/received per stream. If enabled, padding and headers are not included
// in bytes sent or received.
constexpr char kUseStandardBytesStats[] = "WebRTC-UseStandardBytesStats";
RTPHeader CreateRTPHeaderForMediaTransportFrame(
const MediaTransportEncodedAudioFrame& frame,
uint64_t channel_id) {
@ -265,6 +271,8 @@ class ChannelReceive : public ChannelReceiveInterface,
// E2EE Audio Frame Decryption
rtc::scoped_refptr<FrameDecryptorInterface> frame_decryptor_;
webrtc::CryptoOptions crypto_options_;
const bool use_standard_bytes_stats_;
};
int32_t ChannelReceive::OnReceivedPayloadData(const uint8_t* payloadData,
@ -466,7 +474,9 @@ ChannelReceive::ChannelReceive(
associated_send_channel_(nullptr),
media_transport_config_(media_transport_config),
frame_decryptor_(frame_decryptor),
crypto_options_(crypto_options) {
crypto_options_(crypto_options),
use_standard_bytes_stats_(
webrtc::field_trial::IsEnabled(kUseStandardBytesStats)) {
// TODO(nisse): Use _moduleProcessThreadPtr instead?
module_process_thread_checker_.Detach();
@ -767,11 +777,13 @@ CallReceiveStatistics ChannelReceive::GetRTCPStatistics() const {
if (statistician) {
StreamDataCounters data_counters;
statistician->GetReceiveStreamDataCounters(&data_counters);
// TODO(http://crbug.com/webrtc/10525): Bytes received should only include
// payload bytes, not header and padding bytes.
stats.bytesReceived = data_counters.transmitted.payload_bytes +
data_counters.transmitted.header_bytes +
data_counters.transmitted.padding_bytes;
if (use_standard_bytes_stats_) {
stats.bytesReceived = data_counters.transmitted.payload_bytes;
} else {
stats.bytesReceived = data_counters.transmitted.payload_bytes +
data_counters.transmitted.header_bytes +
data_counters.transmitted.padding_bytes;
}
stats.packetsReceived = data_counters.transmitted.packets;
stats.last_packet_received_timestamp_ms =
data_counters.last_packet_received_timestamp_ms;

26
audio/channel_send.cc
View File

@ -53,6 +53,11 @@ namespace {
constexpr int64_t kMaxRetransmissionWindowMs = 1000;
constexpr int64_t kMinRetransmissionWindowMs = 30;
// Field trial which controls whether to report standard-compliant bytes
// sent/received per stream. If enabled, padding and headers are not included
// in bytes sent or received.
constexpr char kUseStandardBytesStats[] = "WebRTC-UseStandardBytesStats";
MediaTransportEncodedAudioFrame::FrameType
MediaTransportFrameTypeForWebrtcFrameType(webrtc::AudioFrameType frame_type) {
switch (frame_type) {
@ -266,6 +271,7 @@ class ChannelSend : public ChannelSendInterface,
rtc::ThreadChecker construction_thread_;
const bool use_twcc_plr_for_ana_;
const bool use_standard_bytes_stats_;
bool encoder_queue_is_active_ RTC_GUARDED_BY(encoder_queue_) = false;
@ -654,6 +660,8 @@ ChannelSend::ChannelSend(Clock* clock,
new RateLimiter(clock, kMaxRetransmissionWindowMs)),
use_twcc_plr_for_ana_(
webrtc::field_trial::FindFullName("UseTwccPlrForAna") == "Enabled"),
use_standard_bytes_stats_(
webrtc::field_trial::IsEnabled(kUseStandardBytesStats)),
media_transport_config_(media_transport_config),
frame_encryptor_(frame_encryptor),
crypto_options_(crypto_options),
@ -1078,13 +1086,17 @@ CallSendStatistics ChannelSend::GetRTCPStatistics() const {
StreamDataCounters rtp_stats;
StreamDataCounters rtx_stats;
_rtpRtcpModule->GetSendStreamDataCounters(&rtp_stats, &rtx_stats);
// TODO(https://crbug.com/webrtc/10525): Bytes sent should only include
// payload bytes, not header and padding bytes.
stats.bytesSent =
rtp_stats.transmitted.payload_bytes +
rtp_stats.transmitted.padding_bytes + rtp_stats.transmitted.header_bytes +
rtx_stats.transmitted.payload_bytes +
rtx_stats.transmitted.padding_bytes + rtx_stats.transmitted.header_bytes;
if (use_standard_bytes_stats_) {
stats.bytesSent = rtp_stats.transmitted.payload_bytes +
rtx_stats.transmitted.payload_bytes;
} else {
stats.bytesSent = rtp_stats.transmitted.payload_bytes +
rtp_stats.transmitted.padding_bytes +
rtp_stats.transmitted.header_bytes +
rtx_stats.transmitted.payload_bytes +
rtx_stats.transmitted.padding_bytes +
rtx_stats.transmitted.header_bytes;
}
// TODO(https://crbug.com/webrtc/10555): RTX retransmissions should show up in
// separate outbound-rtp stream objects.
stats.retransmitted_bytes_sent = rtp_stats.retransmitted.payload_bytes;

35
media/engine/webrtc_video_engine.cc
View File

@ -45,6 +45,11 @@ namespace {
const int kMinLayerSize = 16;
// Field trial which controls whether to report standard-compliant bytes
// sent/received per stream. If enabled, padding and headers are not included
// in bytes sent or received.
constexpr char kUseStandardBytesStats[] = "WebRTC-UseStandardBytesStats";
// If this field trial is enabled, we will enable sending FlexFEC and disable
// sending ULPFEC whenever the former has been negotiated in the SDPs.
bool IsFlexfecFieldTrialEnabled() {
@ -1795,7 +1800,9 @@ WebRtcVideoChannel::WebRtcVideoSendStream::WebRtcVideoSendStream(
encoder_sink_(nullptr),
parameters_(std::move(config), options, max_bitrate_bps, codec_settings),
rtp_parameters_(CreateRtpParametersWithEncodings(sp)),
sending_(false) {
sending_(false),
use_standard_bytes_stats_(
webrtc::field_trial::IsEnabled(kUseStandardBytesStats)) {
// Maximum packet size may come in RtpConfig from external transport, for
// example from QuicTransportInterface implementation, so do not exceed
// given max_packet_size.
@ -2362,11 +2369,13 @@ VideoSenderInfo WebRtcVideoChannel::WebRtcVideoSendStream::GetVideoSenderInfo(
it != stats.substreams.end(); ++it) {
// TODO(pbos): Wire up additional stats, such as padding bytes.
webrtc::VideoSendStream::StreamStats stream_stats = it->second;
// TODO(http://crbug.com/webrtc/10525): Bytes sent should only include
// payload bytes, not header and padding bytes.
info.bytes_sent += stream_stats.rtp_stats.transmitted.payload_bytes +
stream_stats.rtp_stats.transmitted.header_bytes +
stream_stats.rtp_stats.transmitted.padding_bytes;
if (use_standard_bytes_stats_) {
info.bytes_sent += stream_stats.rtp_stats.transmitted.payload_bytes;
} else {
info.bytes_sent += stream_stats.rtp_stats.transmitted.payload_bytes +
stream_stats.rtp_stats.transmitted.header_bytes +
stream_stats.rtp_stats.transmitted.padding_bytes;
}
info.packets_sent += stream_stats.rtp_stats.transmitted.packets;
info.total_packet_send_delay_ms += stream_stats.total_packet_send_delay_ms;
// TODO(https://crbug.com/webrtc/10555): RTX retransmissions should show up
@ -2482,7 +2491,9 @@ WebRtcVideoChannel::WebRtcVideoReceiveStream::WebRtcVideoReceiveStream(
decoder_factory_(decoder_factory),
sink_(NULL),
first_frame_timestamp_(-1),
estimated_remote_start_ntp_time_ms_(0) {
estimated_remote_start_ntp_time_ms_(0),
use_standard_bytes_stats_(
webrtc::field_trial::IsEnabled(kUseStandardBytesStats)) {
config_.renderer = this;
ConfigureCodecs(recv_codecs);
ConfigureFlexfecCodec(flexfec_config.payload_type);
@ -2783,9 +2794,13 @@ WebRtcVideoChannel::WebRtcVideoReceiveStream::GetVideoReceiverInfo(
if (stats.current_payload_type != -1) {
info.codec_payload_type = stats.current_payload_type;
}
info.bytes_rcvd = stats.rtp_stats.transmitted.payload_bytes +
stats.rtp_stats.transmitted.header_bytes +
stats.rtp_stats.transmitted.padding_bytes;
if (use_standard_bytes_stats_) {
info.bytes_rcvd = stats.rtp_stats.transmitted.payload_bytes;
} else {
info.bytes_rcvd = stats.rtp_stats.transmitted.payload_bytes +
stats.rtp_stats.transmitted.header_bytes +
stats.rtp_stats.transmitted.padding_bytes;
}
info.packets_rcvd = stats.rtp_stats.transmitted.packets;
info.packets_lost = stats.rtcp_stats.packets_lost;

4
media/engine/webrtc_video_engine.h
View File

@ -378,6 +378,8 @@ class WebRtcVideoChannel : public VideoMediaChannel,
bool sending_ RTC_GUARDED_BY(&thread_checker_);
const bool use_standard_bytes_stats_;
// In order for the |invoker_| to protect other members from being
// destructed as they are used in asynchronous tasks it has to be destructed
// first.
@ -468,6 +470,8 @@ class WebRtcVideoChannel : public VideoMediaChannel,
// Start NTP time is estimated as current remote NTP time (estimated from
// RTCP) minus the elapsed time, as soon as remote NTP time is available.
int64_t estimated_remote_start_ntp_time_ms_ RTC_GUARDED_BY(sink_lock_);
const bool use_standard_bytes_stats_;
};
void Construct(webrtc::Call* call, WebRtcVideoEngine* engine);

29
media/engine/webrtc_video_engine_unittest.cc
View File

@ -87,6 +87,8 @@ static const uint32_t kFlexfecSsrc = 5;
static const uint32_t kIncomingUnsignalledSsrc = 0xC0FFEE;
static const uint32_t kDefaultRecvSsrc = 0;
constexpr uint32_t kRtpHeaderSize = 12;
static const char kUnsupportedExtensionName[] =
"urn:ietf:params:rtp-hdrext:unsupported";
@ -1593,6 +1595,10 @@ TEST_F(WebRtcVideoChannelBaseTest, InvalidRecvBufferSize) {
// Test that stats work properly for a 1-1 call.
TEST_F(WebRtcVideoChannelBaseTest, GetStats) {
webrtc::test::ScopedFieldTrials field_trials(
"WebRTC-UseStandardBytesStats/Enabled/");
SetUp();
const int kDurationSec = 3;
const int kFps = 10;
SendReceiveManyAndGetStats(DefaultCodec(), kDurationSec, kFps);
@ -1603,7 +1609,8 @@ TEST_F(WebRtcVideoChannelBaseTest, GetStats) {
ASSERT_EQ(1U, info.senders.size());
// TODO(whyuan): bytes_sent and bytes_rcvd are different. Are both payload?
// For webrtc, bytes_sent does not include the RTP header length.
EXPECT_GT(info.senders[0].bytes_sent, 0);
EXPECT_EQ(info.senders[0].bytes_sent,
NumRtpBytes() - kRtpHeaderSize * NumRtpPackets());
EXPECT_EQ(NumRtpPackets(), info.senders[0].packets_sent);
EXPECT_EQ(0.0, info.senders[0].fraction_lost);
ASSERT_TRUE(info.senders[0].codec_payload_type);
@ -1626,7 +1633,8 @@ TEST_F(WebRtcVideoChannelBaseTest, GetStats) {
EXPECT_EQ(info.senders[0].ssrcs()[0], info.receivers[0].ssrcs()[0]);
ASSERT_TRUE(info.receivers[0].codec_payload_type);
EXPECT_EQ(DefaultCodec().id, *info.receivers[0].codec_payload_type);
EXPECT_EQ(NumRtpBytes(), info.receivers[0].bytes_rcvd);
EXPECT_EQ(NumRtpBytes() - kRtpHeaderSize * NumRtpPackets(),
info.receivers[0].bytes_rcvd);
EXPECT_EQ(NumRtpPackets(), info.receivers[0].packets_rcvd);
EXPECT_EQ(0, info.receivers[0].packets_lost);
// TODO(asapersson): Not set for webrtc. Handle missing stats.
@ -1647,6 +1655,10 @@ TEST_F(WebRtcVideoChannelBaseTest, GetStats) {
// Test that stats work properly for a conf call with multiple recv streams.
TEST_F(WebRtcVideoChannelBaseTest, GetStatsMultipleRecvStreams) {
webrtc::test::ScopedFieldTrials field_trials(
"WebRTC-UseStandardBytesStats/Enabled/");
SetUp();
cricket::FakeVideoRenderer renderer1, renderer2;
EXPECT_TRUE(SetOneCodec(DefaultCodec()));
cricket::VideoSendParameters parameters;
@ -1677,7 +1689,8 @@ TEST_F(WebRtcVideoChannelBaseTest, GetStatsMultipleRecvStreams) {
ASSERT_EQ(1U, info.senders.size());
// TODO(whyuan): bytes_sent and bytes_rcvd are different. Are both payload?
// For webrtc, bytes_sent does not include the RTP header length.
EXPECT_GT(GetSenderStats(0).bytes_sent, 0);
EXPECT_EQ_WAIT(NumRtpBytes() - kRtpHeaderSize * NumRtpPackets(),
GetSenderStats(0).bytes_sent, kTimeout);
EXPECT_EQ_WAIT(NumRtpPackets(), GetSenderStats(0).packets_sent, kTimeout);
EXPECT_EQ(kVideoWidth, GetSenderStats(0).send_frame_width);
EXPECT_EQ(kVideoHeight, GetSenderStats(0).send_frame_height);
@ -1686,7 +1699,8 @@ TEST_F(WebRtcVideoChannelBaseTest, GetStatsMultipleRecvStreams) {
for (size_t i = 0; i < info.receivers.size(); ++i) {
EXPECT_EQ(1U, GetReceiverStats(i).ssrcs().size());
EXPECT_EQ(i + 1, GetReceiverStats(i).ssrcs()[0]);
EXPECT_EQ_WAIT(NumRtpBytes(), GetReceiverStats(i).bytes_rcvd, kTimeout);
EXPECT_EQ_WAIT(NumRtpBytes() - kRtpHeaderSize * NumRtpPackets(),
GetReceiverStats(i).bytes_rcvd, kTimeout);
EXPECT_EQ_WAIT(NumRtpPackets(), GetReceiverStats(i).packets_rcvd, kTimeout);
EXPECT_EQ_WAIT(kVideoWidth, GetReceiverStats(i).frame_width, kTimeout);
EXPECT_EQ_WAIT(kVideoHeight, GetReceiverStats(i).frame_height, kTimeout);
@ -5158,6 +5172,9 @@ TEST_F(WebRtcVideoChannelTest, GetStatsTranslatesDecodeStatsCorrectly) {
}
TEST_F(WebRtcVideoChannelTest, GetStatsTranslatesReceivePacketStatsCorrectly) {
webrtc::test::ScopedFieldTrials field_trials(
"WebRTC-UseStandardBytesStats/Enabled/");
FakeVideoReceiveStream* stream = AddRecvStream();
webrtc::VideoReceiveStream::Stats stats;
stats.rtp_stats.transmitted.payload_bytes = 2;
@ -5170,9 +5187,7 @@ TEST_F(WebRtcVideoChannelTest, GetStatsTranslatesReceivePacketStatsCorrectly) {
cricket::VideoMediaInfo info;
ASSERT_TRUE(channel_->GetStats(&info));
EXPECT_EQ(stats.rtp_stats.transmitted.payload_bytes +
stats.rtp_stats.transmitted.header_bytes +
stats.rtp_stats.transmitted.padding_bytes,
EXPECT_EQ(stats.rtp_stats.transmitted.payload_bytes,
rtc::checked_cast<size_t>(info.receivers[0].bytes_rcvd));
EXPECT_EQ(stats.rtp_stats.transmitted.packets,
rtc::checked_cast<unsigned int>(info.receivers[0].packets_rcvd));