Delete the WebRTC.Call.TimeSendingAudioRtpPacketsInSeconds metric

It never saw much use, and is blocking refactoring.

Histograms.xml-side cleanup:
https://chromium-review.googlesource.com/c/chromium/src/+/1344141

Bug: webrtc:7882
Change-Id: I112232a573fcd218dc7a51bfcdd7898847d14f18
Reviewed-on: https://webrtc-review.googlesource.com/c/111506
Commit-Queue: Niels Moller <nisse@webrtc.org>
Commit-Queue: Sam Zackrisson <saza@webrtc.org>
Reviewed-by: Fredrik Solenberg <solenberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25780}
This commit is contained in:
Sam Zackrisson 2018-11-20 17:15:13 +01:00 committed by Commit Bot
parent 8ce0d2b956
commit ff0581672e
9 changed files with 7 additions and 274 deletions

View File

@ -33,8 +33,6 @@ rtc_static_library("audio") {
"null_audio_poller.h",
"remix_resample.cc",
"remix_resample.h",
"time_interval.cc",
"time_interval.h",
"transport_feedback_packet_loss_tracker.cc",
"transport_feedback_packet_loss_tracker.h",
]
@ -126,7 +124,6 @@ if (rtc_include_tests) {
"remix_resample_unittest.cc",
"test/audio_stats_test.cc",
"test/media_transport_test.cc",
"time_interval_unittest.cc",
"transport_feedback_packet_loss_tracker_unittest.cc",
]
deps = [

View File

@ -90,29 +90,6 @@ void UpdateEventLogStreamConfig(RtcEventLog* event_log,
} // namespace
// Helper class to track the actively sending lifetime of this stream.
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<webrtc::AudioState>& audio_state,
@ -122,8 +99,7 @@ AudioSendStream::AudioSendStream(
BitrateAllocatorInterface* bitrate_allocator,
RtcEventLog* event_log,
RtcpRttStats* rtcp_rtt_stats,
const absl::optional<RtpState>& suspended_rtp_state,
TimeInterval* overall_call_lifetime)
const absl::optional<RtpState>& suspended_rtp_state)
: AudioSendStream(config,
audio_state,
worker_queue,
@ -132,7 +108,6 @@ AudioSendStream::AudioSendStream(
event_log,
rtcp_rtt_stats,
suspended_rtp_state,
overall_call_lifetime,
voe::CreateChannelSend(worker_queue,
module_process_thread,
config.media_transport,
@ -152,7 +127,6 @@ AudioSendStream::AudioSendStream(
RtcEventLog* event_log,
RtcpRttStats* rtcp_rtt_stats,
const absl::optional<RtpState>& suspended_rtp_state,
TimeInterval* overall_call_lifetime,
std::unique_ptr<voe::ChannelSendInterface> channel_send)
: worker_queue_(worker_queue),
config_(Config(/*send_transport=*/nullptr,
@ -166,8 +140,7 @@ AudioSendStream::AudioSendStream(
kPacketLossRateMinNumAckedPackets,
kRecoverablePacketLossRateMinNumAckedPairs),
rtp_rtcp_module_(nullptr),
suspended_rtp_state_(suspended_rtp_state),
overall_call_lifetime_(overall_call_lifetime) {
suspended_rtp_state_(suspended_rtp_state) {
RTC_LOG(LS_INFO) << "AudioSendStream: " << config.rtp.ssrc;
RTC_DCHECK(worker_queue_);
RTC_DCHECK(audio_state_);
@ -178,7 +151,6 @@ AudioSendStream::AudioSendStream(
// should be no rtp_transport, and below check should be strengthened to XOR
// (either rtp_transport or media_transport but not both).
RTC_DCHECK(rtp_transport || config.media_transport);
RTC_DCHECK(overall_call_lifetime_);
rtp_rtcp_module_ = channel_send_->GetRtpRtcp();
RTC_DCHECK(rtp_rtcp_module_);
@ -202,10 +174,6 @@ AudioSendStream::~AudioSendStream() {
channel_send_->RegisterTransport(nullptr);
channel_send_->ResetSenderCongestionControlObjects();
}
// Lifetime can only be updated after deregistering
// |timed_send_transport_adapter_| in the underlying channel object to avoid
// data races in |active_lifetime_|.
overall_call_lifetime_->Extend(active_lifetime_);
}
const webrtc::AudioSendStream::Config& AudioSendStream::GetConfig() const {
@ -257,17 +225,7 @@ void AudioSendStream::ConfigureStream(
}
if (first_time || new_config.send_transport != old_config.send_transport) {
if (old_config.send_transport) {
channel_send->RegisterTransport(nullptr);
}
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_send->RegisterTransport(
stream->timed_send_transport_adapter_.get());
channel_send->RegisterTransport(new_config.send_transport);
}
// Enable the frame encryptor if a new frame encryptor has been provided.

View File

@ -15,7 +15,6 @@
#include <vector>
#include "audio/channel_send.h"
#include "audio/time_interval.h"
#include "audio/transport_feedback_packet_loss_tracker.h"
#include "call/audio_send_stream.h"
#include "call/audio_state.h"
@ -46,8 +45,7 @@ class AudioSendStream final : public webrtc::AudioSendStream,
BitrateAllocatorInterface* bitrate_allocator,
RtcEventLog* event_log,
RtcpRttStats* rtcp_rtt_stats,
const absl::optional<RtpState>& suspended_rtp_state,
TimeInterval* overall_call_lifetime);
const absl::optional<RtpState>& suspended_rtp_state);
// For unit tests, which need to supply a mock ChannelSend.
AudioSendStream(const webrtc::AudioSendStream::Config& config,
const rtc::scoped_refptr<webrtc::AudioState>& audio_state,
@ -57,7 +55,6 @@ class AudioSendStream final : public webrtc::AudioSendStream,
RtcEventLog* event_log,
RtcpRttStats* rtcp_rtt_stats,
const absl::optional<RtpState>& suspended_rtp_state,
TimeInterval* overall_call_lifetime,
std::unique_ptr<voe::ChannelSendInterface> channel_send);
~AudioSendStream() override;
@ -144,10 +141,6 @@ class AudioSendStream final : public webrtc::AudioSendStream,
RtpRtcp* rtp_rtcp_module_;
absl::optional<RtpState> const suspended_rtp_state_;
std::unique_ptr<TimedTransport> timed_send_transport_adapter_;
TimeInterval active_lifetime_;
TimeInterval* overall_call_lifetime_ = nullptr;
// RFC 5285: Each distinct extension MUST have a unique ID. The value 0 is
// reserved for padding and MUST NOT be used as a local identifier.
// So it should be safe to use 0 here to indicate "not configured".

View File

@ -14,7 +14,6 @@
#include "absl/memory/memory.h"
#include "api/test/mock_frame_encryptor.h"
#include "api/units/time_delta.h"
#include "audio/audio_send_stream.h"
#include "audio/audio_state.h"
#include "audio/conversion.h"
@ -28,12 +27,10 @@
#include "modules/rtp_rtcp/mocks/mock_rtcp_bandwidth_observer.h"
#include "modules/rtp_rtcp/mocks/mock_rtcp_rtt_stats.h"
#include "modules/rtp_rtcp/mocks/mock_rtp_rtcp.h"
#include "rtc_base/fakeclock.h"
#include "rtc_base/task_queue.h"
#include "test/gtest.h"
#include "test/mock_audio_encoder.h"
#include "test/mock_audio_encoder_factory.h"
#include "test/mock_transport.h"
namespace webrtc {
namespace test {
@ -168,7 +165,6 @@ struct ConfigHelper {
new internal::AudioSendStream(
stream_config_, audio_state_, &worker_queue_, &rtp_transport_,
&bitrate_allocator_, &event_log_, &rtcp_rtt_stats_, absl::nullopt,
&active_lifetime_,
std::unique_ptr<voe::ChannelSendInterface>(channel_send_)));
}
@ -179,7 +175,6 @@ struct ConfigHelper {
}
MockChannelSend* channel_send() { return channel_send_; }
RtpTransportControllerSendInterface* transport() { return &rtp_transport_; }
TimeInterval* active_lifetime() { return &active_lifetime_; }
static void AddBweToConfig(AudioSendStream::Config* config) {
config->rtp.extensions.push_back(RtpExtension(
@ -216,11 +211,7 @@ struct ConfigHelper {
.Times(1);
}
EXPECT_CALL(*channel_send_, ResetSenderCongestionControlObjects()).Times(1);
{
::testing::InSequence unregister_on_destruction;
EXPECT_CALL(*channel_send_, RegisterTransport(_)).Times(1);
EXPECT_CALL(*channel_send_, RegisterTransport(nullptr)).Times(1);
}
EXPECT_CALL(*channel_send_, RegisterTransport(nullptr)).Times(2);
}
void SetupMockForSetupSendCodec(bool expect_set_encoder_call) {
@ -300,7 +291,6 @@ struct ConfigHelper {
testing::StrictMock<MockChannelSend>* channel_send_ = nullptr;
rtc::scoped_refptr<MockAudioProcessing> audio_processing_;
AudioProcessingStats audio_processing_stats_;
TimeInterval active_lifetime_;
testing::StrictMock<MockRtcpBandwidthObserver> bandwidth_observer_;
testing::NiceMock<MockRtcEventLog> event_log_;
testing::NiceMock<MockRtpTransportControllerSend> rtp_transport_;
@ -561,33 +551,5 @@ TEST(AudioSendStreamTest, ReconfigureWithFrameEncryptor) {
EXPECT_CALL(*helper.channel_send(), SetFrameEncryptor(Ne(nullptr))).Times(1);
send_stream->Reconfigure(new_config);
}
// Checks that AudioSendStream logs the times at which RTP packets are sent
// through its interface.
TEST(AudioSendStreamTest, UpdateLifetime) {
ConfigHelper helper(false, true);
MockTransport mock_transport;
helper.config().send_transport = &mock_transport;
Transport* registered_transport;
ON_CALL(*helper.channel_send(), RegisterTransport(_))
.WillByDefault(Invoke([&registered_transport](Transport* transport) {
registered_transport = transport;
}));
rtc::ScopedFakeClock fake_clock;
constexpr int64_t kTimeBetweenSendRtpCallsMs = 100;
{
auto send_stream = helper.CreateAudioSendStream();
EXPECT_CALL(mock_transport, SendRtp(_, _, _)).Times(2);
const PacketOptions options;
registered_transport->SendRtp(nullptr, 0, options);
fake_clock.AdvanceTime(TimeDelta::ms(kTimeBetweenSendRtpCallsMs));
registered_transport->SendRtp(nullptr, 0, options);
}
EXPECT_TRUE(!helper.active_lifetime()->Empty());
EXPECT_EQ(helper.active_lifetime()->Length(), kTimeBetweenSendRtpCallsMs);
}
} // namespace test
} // namespace webrtc

View File

@ -118,11 +118,10 @@ TEST(AudioWithMediaTransport, DeliversAudio) {
rtc::TaskQueue send_tq("audio send queue");
std::unique_ptr<ProcessThread> send_process_thread =
ProcessThread::Create("audio send thread");
TimeInterval life_time;
webrtc::internal::AudioSendStream send_stream(
send_config, audio_state, &send_tq, send_process_thread.get(),
/*transport=*/nullptr, &bitrate_allocator, null_event_log.get(),
/*rtcp_rtt_stats=*/nullptr, absl::optional<RtpState>(), &life_time);
/*rtcp_rtt_stats=*/nullptr, absl::optional<RtpState>());
audio_device->Init(); // Starts thread.
audio_device->RegisterAudioCallback(audio_state->audio_transport());

View File

@ -1,56 +0,0 @@
/*
* 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 "audio/time_interval.h"
#include "rtc_base/checks.h"
#include "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

View File

@ -1,65 +0,0 @@
/*
* 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 AUDIO_TIME_INTERVAL_H_
#define AUDIO_TIME_INTERVAL_H_
#include <stdint.h>
#include "absl/types/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;
};
absl::optional<Interval> interval_;
};
} // namespace webrtc
#endif // AUDIO_TIME_INTERVAL_H_

View File

@ -1,48 +0,0 @@
/*
* 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 "audio/time_interval.h"
#include "api/units/time_delta.h"
#include "rtc_base/fakeclock.h"
#include "test/gtest.h"
namespace webrtc {
TEST(TimeIntervalTest, TimeInMs) {
rtc::ScopedFakeClock fake_clock;
TimeInterval interval;
interval.Extend();
fake_clock.AdvanceTime(TimeDelta::ms(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

View File

@ -22,7 +22,6 @@
#include "audio/audio_receive_stream.h"
#include "audio/audio_send_stream.h"
#include "audio/audio_state.h"
#include "audio/time_interval.h"
#include "call/bitrate_allocator.h"
#include "call/call.h"
#include "call/flexfec_receive_stream_impl.h"
@ -347,7 +346,6 @@ class Call final : public webrtc::Call,
absl::optional<int64_t> last_received_rtp_audio_ms_;
absl::optional<int64_t> first_received_rtp_video_ms_;
absl::optional<int64_t> last_received_rtp_video_ms_;
TimeInterval sent_rtp_audio_timer_ms_;
rtc::CriticalSection last_bandwidth_bps_crit_;
uint32_t last_bandwidth_bps_ RTC_GUARDED_BY(&last_bandwidth_bps_crit_);
@ -541,11 +539,6 @@ 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)
@ -649,7 +642,7 @@ webrtc::AudioSendStream* Call::CreateAudioSendStream(
config, config_.audio_state, transport_send_ptr_->GetWorkerQueue(),
module_process_thread_.get(), transport_send_ptr_,
bitrate_allocator_.get(), event_log_, call_stats_.get(),
suspended_rtp_state, &sent_rtp_audio_timer_ms_);
suspended_rtp_state);
{
WriteLockScoped write_lock(*send_crit_);
RTC_DCHECK(audio_send_ssrcs_.find(config.rtp.ssrc) ==