From c58f8c096213a5e97554a1576cc4b51d1a98c4c4 Mon Sep 17 00:00:00 2001 From: saza Date: Wed, 19 Jul 2017 00:39:19 -0700 Subject: [PATCH] Adds a histogram metric tracking for how long audio RTP packets are sent through streams related to a call object. The Call object does not know directly when packets pass through it, only which AudioSendStreams are used. Each AudioSendStream has a pointer to the Transport object through which its packets are send. This CL: By registering an internal wrapper class, TimedTransport, the AudioSendStream can stay up-to-date on when packets have passed through its Transport. This lifetime (as an interval) is then queried by the Call when the AudioSendStream is destroyed. When Call is destroyed, all streams are guaranteed to have been destroyed and hence Call is up-to-date on packet activity. The class TimeInterval keeps the code in Call and AudioSendStream smaller, with fewer get methods in their APIs and less code for updating values. Also modifies the unit test for AudioSendStream: it previously enforced that the stream registers (with its channel proxy) the same transport that it was constructed with. BUG=webrtc:7882 Review-Url: https://codereview.webrtc.org/2979833002 Cr-Commit-Position: refs/heads/master@{#19087} --- webrtc/audio/BUILD.gn | 3 ++ webrtc/audio/audio_send_stream.cc | 38 ++++++++++++++- webrtc/audio/audio_send_stream.h | 7 +++ webrtc/audio/time_interval.cc | 56 ++++++++++++++++++++++ webrtc/audio/time_interval.h | 65 ++++++++++++++++++++++++++ webrtc/audio/time_interval_unittest.cc | 48 +++++++++++++++++++ webrtc/call/call.cc | 8 ++++ 7 files changed, 223 insertions(+), 2 deletions(-) create mode 100644 webrtc/audio/time_interval.cc create mode 100644 webrtc/audio/time_interval.h create mode 100644 webrtc/audio/time_interval_unittest.cc diff --git a/webrtc/audio/BUILD.gn b/webrtc/audio/BUILD.gn index 4fc7337691..495875de03 100644 --- a/webrtc/audio/BUILD.gn +++ b/webrtc/audio/BUILD.gn @@ -24,6 +24,8 @@ rtc_static_library("audio") { "audio_transport_proxy.h", "conversion.h", "scoped_voe_interface.h", + "time_interval.cc", + "time_interval.h", ] if (!build_with_chromium && is_clang) { @@ -73,6 +75,7 @@ if (rtc_include_tests) { "audio_receive_stream_unittest.cc", "audio_send_stream_unittest.cc", "audio_state_unittest.cc", + "time_interval_unittest.cc", ] deps = [ ":audio", diff --git a/webrtc/audio/audio_send_stream.cc b/webrtc/audio/audio_send_stream.cc index 0659cbfd2d..103d6306fd 100644 --- a/webrtc/audio/audio_send_stream.cc +++ b/webrtc/audio/audio_send_stream.cc @@ -51,6 +51,30 @@ void CallEncoder(const std::unique_ptr& channel_proxy, } } // namespace +// TODO(saza): Move this declaration further down when we can use +// std::make_unique. +class AudioSendStream::TimedTransport : public Transport { + public: + TimedTransport(Transport* transport, TimeInterval* time_interval) + : transport_(transport), lifetime_(time_interval) {} + bool SendRtp(const uint8_t* packet, + size_t length, + const PacketOptions& options) { + if (lifetime_) { + lifetime_->Extend(); + } + return transport_->SendRtp(packet, length, options); + } + bool SendRtcp(const uint8_t* packet, size_t length) { + return transport_->SendRtcp(packet, length); + } + ~TimedTransport() {} + + private: + Transport* transport_; + TimeInterval* lifetime_; +}; + AudioSendStream::AudioSendStream( const webrtc::AudioSendStream::Config& config, const rtc::scoped_refptr& audio_state, @@ -137,8 +161,14 @@ void AudioSendStream::ConfigureStream( if (old_config.send_transport) { channel_proxy->DeRegisterExternalTransport(); } - - channel_proxy->RegisterExternalTransport(new_config.send_transport); + if (new_config.send_transport) { + stream->timed_send_transport_adapter_.reset(new TimedTransport( + new_config.send_transport, &stream->active_lifetime_)); + } else { + stream->timed_send_transport_adapter_.reset(nullptr); + } + channel_proxy->RegisterExternalTransport( + stream->timed_send_transport_adapter_.get()); } // RFC 5285: Each distinct extension MUST have a unique ID. The value 0 is @@ -391,6 +421,10 @@ RtpState AudioSendStream::GetRtpState() const { return rtp_rtcp_module_->GetRtpState(); } +const TimeInterval& AudioSendStream::GetActiveLifetime() const { + return active_lifetime_; +} + VoiceEngine* AudioSendStream::voice_engine() const { internal::AudioState* audio_state = static_cast(audio_state_.get()); diff --git a/webrtc/audio/audio_send_stream.h b/webrtc/audio/audio_send_stream.h index 70cf1c874d..42a04aee09 100644 --- a/webrtc/audio/audio_send_stream.h +++ b/webrtc/audio/audio_send_stream.h @@ -14,6 +14,7 @@ #include #include +#include "webrtc/audio/time_interval.h" #include "webrtc/call/audio_send_stream.h" #include "webrtc/call/audio_state.h" #include "webrtc/call/bitrate_allocator.h" @@ -76,8 +77,11 @@ class AudioSendStream final : public webrtc::AudioSendStream, void SetTransportOverhead(int transport_overhead_per_packet); RtpState GetRtpState() const; + const TimeInterval& GetActiveLifetime() const; private: + class TimedTransport; + VoiceEngine* voice_engine() const; // These are all static to make it less likely that (the old) config_ is @@ -117,6 +121,9 @@ class AudioSendStream final : public webrtc::AudioSendStream, RtpRtcp* rtp_rtcp_module_; rtc::Optional const suspended_rtp_state_; + std::unique_ptr timed_send_transport_adapter_; + TimeInterval active_lifetime_; + RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(AudioSendStream); }; } // namespace internal diff --git a/webrtc/audio/time_interval.cc b/webrtc/audio/time_interval.cc new file mode 100644 index 0000000000..f156af06c8 --- /dev/null +++ b/webrtc/audio/time_interval.cc @@ -0,0 +1,56 @@ +/* + * Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "webrtc/audio/time_interval.h" +#include "webrtc/rtc_base/checks.h" +#include "webrtc/rtc_base/timeutils.h" + +namespace webrtc { + +TimeInterval::TimeInterval() = default; +TimeInterval::~TimeInterval() = default; + +void TimeInterval::Extend() { + Extend(rtc::TimeMillis()); +} + +void TimeInterval::Extend(int64_t time) { + if (!interval_) { + interval_.emplace(time, time); + } else { + if (time < interval_->first) { + interval_->first = time; + } + if (time > interval_->last) { + interval_->last = time; + } + } +} + +void TimeInterval::Extend(const TimeInterval& other_interval) { + if (!other_interval.Empty()) { + Extend(other_interval.interval_->first); + Extend(other_interval.interval_->last); + } +} + +bool TimeInterval::Empty() const { + return !interval_; +} + +int64_t TimeInterval::Length() const { + RTC_DCHECK(interval_); + return interval_->last - interval_->first; +} + +TimeInterval::Interval::Interval(int64_t first, int64_t last) + : first(first), last(last) {} + +} // namespace webrtc diff --git a/webrtc/audio/time_interval.h b/webrtc/audio/time_interval.h new file mode 100644 index 0000000000..069127d641 --- /dev/null +++ b/webrtc/audio/time_interval.h @@ -0,0 +1,65 @@ +/* + * Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef WEBRTC_AUDIO_TIME_INTERVAL_H_ +#define WEBRTC_AUDIO_TIME_INTERVAL_H_ + +#include + +#include "webrtc/rtc_base/optional.h" + +namespace webrtc { + +// This class logs the first and last time its Extend() function is called. +// +// This class is not thread-safe; Extend() calls should only be made by a +// single thread at a time, such as within a lock or destructor. +// +// Example usage: +// // let x < y < z < u < v +// rtc::TimeInterval interval; +// ... // interval.Extend(); // at time x +// ... +// interval.Extend(); // at time y +// ... +// interval.Extend(); // at time u +// ... +// interval.Extend(z); // at time v +// ... +// if (!interval.Empty()) { +// int64_t active_time = interval.Length(); // returns (u - x) +// } +class TimeInterval { + public: + TimeInterval(); + ~TimeInterval(); + // Extend the interval with the current time. + void Extend(); + // Extend the interval with a given time. + void Extend(int64_t time); + // Take the convex hull with another interval. + void Extend(const TimeInterval& other_interval); + // True iff Extend has never been called. + bool Empty() const; + // Returns the time between the first and the last tick, in milliseconds. + int64_t Length() const; + + private: + struct Interval { + Interval(int64_t first, int64_t last); + + int64_t first, last; + }; + rtc::Optional interval_; +}; + +} // namespace webrtc + +#endif // WEBRTC_AUDIO_TIME_INTERVAL_H_ diff --git a/webrtc/audio/time_interval_unittest.cc b/webrtc/audio/time_interval_unittest.cc new file mode 100644 index 0000000000..592f9fc3d4 --- /dev/null +++ b/webrtc/audio/time_interval_unittest.cc @@ -0,0 +1,48 @@ +/* + * Copyright 2017 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "webrtc/audio/time_interval.h" +#include "webrtc/rtc_base/fakeclock.h" +#include "webrtc/rtc_base/timedelta.h" +#include "webrtc/test/gtest.h" + +namespace webrtc { + +TEST(TimeIntervalTest, TimeInMs) { + rtc::ScopedFakeClock fake_clock; + TimeInterval interval; + interval.Extend(); + fake_clock.AdvanceTime(rtc::TimeDelta::FromMilliseconds(100)); + interval.Extend(); + EXPECT_EQ(interval.Length(), 100); +} + +TEST(TimeIntervalTest, Empty) { + TimeInterval interval; + EXPECT_TRUE(interval.Empty()); + interval.Extend(); + EXPECT_FALSE(interval.Empty()); + interval.Extend(200); + EXPECT_FALSE(interval.Empty()); +} + +TEST(TimeIntervalTest, MonotoneIncreasing) { + const size_t point_count = 7; + const int64_t interval_points[] = {3, 2, 5, 0, 4, 1, 6}; + const int64_t interval_differences[] = {0, 1, 3, 5, 5, 5, 6}; + TimeInterval interval; + EXPECT_TRUE(interval.Empty()); + for (size_t i = 0; i < point_count; ++i) { + interval.Extend(interval_points[i]); + EXPECT_EQ(interval_differences[i], interval.Length()); + } +} + +} // namespace webrtc diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc index 0bddde9fc6..1a78d8e261 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc @@ -20,6 +20,7 @@ #include "webrtc/audio/audio_send_stream.h" #include "webrtc/audio/audio_state.h" #include "webrtc/audio/scoped_voe_interface.h" +#include "webrtc/audio/time_interval.h" #include "webrtc/call/bitrate_allocator.h" #include "webrtc/call/call.h" #include "webrtc/call/flexfec_receive_stream_impl.h" @@ -329,6 +330,7 @@ class Call : public webrtc::Call, rtc::Optional last_received_rtp_audio_ms_; rtc::Optional first_received_rtp_video_ms_; rtc::Optional last_received_rtp_video_ms_; + TimeInterval sent_rtp_audio_timer_ms_; // TODO(holmer): Remove this lock once BitrateController no longer calls // OnNetworkChanged from multiple threads. @@ -510,6 +512,11 @@ void Call::UpdateHistograms() { void Call::UpdateSendHistograms(int64_t first_sent_packet_ms) { if (first_sent_packet_ms == -1) return; + if (!sent_rtp_audio_timer_ms_.Empty()) { + RTC_HISTOGRAM_COUNTS_100000( + "WebRTC.Call.TimeSendingAudioRtpPacketsInSeconds", + sent_rtp_audio_timer_ms_.Length() / 1000); + } int64_t elapsed_sec = (clock_->TimeInMilliseconds() - first_sent_packet_ms) / 1000; if (elapsed_sec < metrics::kMinRunTimeInSeconds) @@ -648,6 +655,7 @@ void Call::DestroyAudioSendStream(webrtc::AudioSendStream* send_stream) { } } UpdateAggregateNetworkState(); + sent_rtp_audio_timer_ms_.Extend(audio_send_stream->GetActiveLifetime()); delete audio_send_stream; }