Move pacer to fully use webrtc::Clock instead of webrtc::TickTime.
This required rewriting the send-side delay stats api to be callback based, as otherwise the SuspendBelowMinBitrate test started flaking much more frequently since it had lock order inversion problems.
R=pbos@webrtc.org, tommi@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/21869005
git-svn-id: http://webrtc.googlecode.com/svn/trunk@6664 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/build/tsan_suppressions.cc b/webrtc/build/tsan_suppressions.cc
index 1e79ccb..8323306 100644
--- a/webrtc/build/tsan_suppressions.cc
+++ b/webrtc/build/tsan_suppressions.cc
@@ -71,7 +71,6 @@
"deadlock:webrtc::RTPSenderAudio::RegisterAudioPayload\n"
"deadlock:webrtc::test::UdpSocketManagerPosixImpl::RemoveSocket\n"
"deadlock:webrtc::vcm::VideoReceiver::RegisterPacketRequestCallback\n"
-"deadlock:webrtc::VideoSendStreamTest_SuspendBelowMinBitrate_Test::TestBody\n"
"deadlock:webrtc::ViECaptureImpl::ConnectCaptureDevice\n"
"deadlock:webrtc::ViEChannel::StartSend\n"
"deadlock:webrtc::ViECodecImpl::GetSendSideDelay\n"
diff --git a/webrtc/common_types.h b/webrtc/common_types.h
index 796ddca..05e1faf 100644
--- a/webrtc/common_types.h
+++ b/webrtc/common_types.h
@@ -269,6 +269,15 @@
const unsigned int ssrc) = 0;
};
+// Callback, used to notify an observer whenever the send-side delay is updated.
+class SendSideDelayObserver {
+ public:
+ virtual ~SendSideDelayObserver() {}
+ virtual void SendSideDelayUpdated(int avg_delay_ms,
+ int max_delay_ms,
+ uint32_t ssrc) = 0;
+};
+
// ==================================================================
// Voice specific types
// ==================================================================
diff --git a/webrtc/config.h b/webrtc/config.h
index 9c8a902..2e96ec1 100644
--- a/webrtc/config.h
+++ b/webrtc/config.h
@@ -34,10 +34,17 @@
};
struct StreamStats {
- StreamStats() : key_frames(0), delta_frames(0), bitrate_bps(0) {}
+ StreamStats()
+ : key_frames(0),
+ delta_frames(0),
+ bitrate_bps(0),
+ avg_delay_ms(0),
+ max_delay_ms(0) {}
uint32_t key_frames;
uint32_t delta_frames;
int32_t bitrate_bps;
+ int avg_delay_ms;
+ int max_delay_ms;
StreamDataCounters rtp_stats;
RtcpStatistics rtcp_stats;
};
diff --git a/webrtc/modules/pacing/include/paced_sender.h b/webrtc/modules/pacing/include/paced_sender.h
index 55497db..b9151a5 100644
--- a/webrtc/modules/pacing/include/paced_sender.h
+++ b/webrtc/modules/pacing/include/paced_sender.h
@@ -17,7 +17,6 @@
#include "webrtc/modules/interface/module.h"
#include "webrtc/system_wrappers/interface/scoped_ptr.h"
#include "webrtc/system_wrappers/interface/thread_annotations.h"
-#include "webrtc/system_wrappers/interface/tick_util.h"
#include "webrtc/typedefs.h"
namespace webrtc {
@@ -147,8 +146,8 @@
scoped_ptr<paced_sender::IntervalBudget> padding_budget_
GUARDED_BY(critsect_);
- TickTime time_last_update_ GUARDED_BY(critsect_);
- TickTime time_last_send_ GUARDED_BY(critsect_);
+ int64_t time_last_update_ GUARDED_BY(critsect_);
+ int64_t time_last_send_ GUARDED_BY(critsect_);
int64_t capture_time_ms_last_queued_ GUARDED_BY(critsect_);
int64_t capture_time_ms_last_sent_ GUARDED_BY(critsect_);
diff --git a/webrtc/modules/pacing/paced_sender.cc b/webrtc/modules/pacing/paced_sender.cc
index 323cafe..52e9cfb 100644
--- a/webrtc/modules/pacing/paced_sender.cc
+++ b/webrtc/modules/pacing/paced_sender.cc
@@ -142,7 +142,7 @@
max_queue_length_ms_(kDefaultMaxQueueLengthMs),
media_budget_(new paced_sender::IntervalBudget(max_bitrate_kbps)),
padding_budget_(new paced_sender::IntervalBudget(min_bitrate_kbps)),
- time_last_update_(TickTime::Now()),
+ time_last_update_(clock->TimeInMilliseconds()),
capture_time_ms_last_queued_(0),
capture_time_ms_last_sent_(0),
high_priority_packets_(new paced_sender::PacketList),
@@ -248,8 +248,7 @@
int32_t PacedSender::TimeUntilNextProcess() {
CriticalSectionScoped cs(critsect_.get());
- int64_t elapsed_time_ms =
- (TickTime::Now() - time_last_update_).Milliseconds();
+ int64_t elapsed_time_ms = clock_->TimeInMilliseconds() - time_last_update_;
if (elapsed_time_ms <= 0) {
return kMinPacketLimitMs;
}
@@ -260,9 +259,9 @@
}
int32_t PacedSender::Process() {
- TickTime now = TickTime::Now();
+ int64_t now = clock_->TimeInMilliseconds();
CriticalSectionScoped cs(critsect_.get());
- int elapsed_time_ms = (now - time_last_update_).Milliseconds();
+ int elapsed_time_ms = now - time_last_update_;
time_last_update_ = now;
if (!enabled_) {
return 0;
@@ -335,7 +334,7 @@
if (media_budget_->bytes_remaining() <= 0) {
// All bytes consumed for this interval.
// Check if we have not sent in a too long time.
- if ((TickTime::Now() - time_last_send_).Milliseconds() >
+ if (clock_->TimeInMilliseconds() - time_last_send_ >
kMaxQueueTimeWithoutSendingMs) {
if (!high_priority_packets_->empty()) {
*packet_list = high_priority_packets_.get();
@@ -389,7 +388,7 @@
// MUST have critsect_ when calling.
void PacedSender::UpdateMediaBytesSent(int num_bytes) {
- time_last_send_ = TickTime::Now();
+ time_last_send_ = clock_->TimeInMilliseconds();
media_budget_->UseBudget(num_bytes);
padding_budget_->UseBudget(num_bytes);
}
diff --git a/webrtc/modules/pacing/paced_sender_unittest.cc b/webrtc/modules/pacing/paced_sender_unittest.cc
index 5518855..14dcdbc 100644
--- a/webrtc/modules/pacing/paced_sender_unittest.cc
+++ b/webrtc/modules/pacing/paced_sender_unittest.cc
@@ -58,7 +58,6 @@
protected:
PacedSenderTest() : clock_(123456) {
srand(0);
- TickTime::UseFakeClock(123456);
// Need to initialize PacedSender after we initialize clock.
send_bucket_.reset(
new PacedSender(
@@ -99,10 +98,8 @@
EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0);
clock_.AdvanceTimeMilliseconds(4);
- TickTime::AdvanceFakeClock(4);
EXPECT_EQ(1, send_bucket_->TimeUntilNextProcess());
clock_.AdvanceTimeMilliseconds(1);
- TickTime::AdvanceFakeClock(1);
EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
EXPECT_CALL(callback_, TimeToSendPacket(
ssrc, sequence_number++, queued_packet_timestamp, false))
@@ -137,7 +134,6 @@
for (int k = 0; k < 10; ++k) {
EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
clock_.AdvanceTimeMilliseconds(5);
- TickTime::AdvanceFakeClock(5);
EXPECT_CALL(callback_,
TimeToSendPacket(ssrc, _, _, false))
.Times(3)
@@ -147,7 +143,6 @@
}
EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
clock_.AdvanceTimeMilliseconds(5);
- TickTime::AdvanceFakeClock(5);
EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
EXPECT_EQ(0, send_bucket_->Process());
SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++,
@@ -185,7 +180,6 @@
for (int k = 0; k < 10; ++k) {
EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
clock_.AdvanceTimeMilliseconds(5);
- TickTime::AdvanceFakeClock(5);
for (int i = 0; i < 3; ++i) {
EXPECT_CALL(callback_, TimeToSendPacket(ssrc, queued_sequence_number++,
@@ -199,7 +193,6 @@
}
EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
clock_.AdvanceTimeMilliseconds(5);
- TickTime::AdvanceFakeClock(5);
EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
EXPECT_EQ(0, send_bucket_->Process());
SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++,
@@ -233,7 +226,6 @@
false);
clock_.AdvanceTimeMilliseconds(1000);
- TickTime::AdvanceFakeClock(1000);
send_bucket_->Process();
}
@@ -253,7 +245,6 @@
EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0);
EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
clock_.AdvanceTimeMilliseconds(5);
- TickTime::AdvanceFakeClock(5);
EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
EXPECT_EQ(0, send_bucket_->Process());
@@ -262,7 +253,6 @@
WillOnce(Return(250));
EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
clock_.AdvanceTimeMilliseconds(5);
- TickTime::AdvanceFakeClock(5);
EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
EXPECT_EQ(0, send_bucket_->Process());
}
@@ -274,13 +264,11 @@
EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0);
EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
clock_.AdvanceTimeMilliseconds(5);
- TickTime::AdvanceFakeClock(5);
EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
EXPECT_EQ(0, send_bucket_->Process());
EXPECT_CALL(callback_, TimeToSendPadding(_)).Times(0);
EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
clock_.AdvanceTimeMilliseconds(5);
- TickTime::AdvanceFakeClock(5);
EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
EXPECT_EQ(0, send_bucket_->Process());
}
@@ -297,7 +285,6 @@
SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++,
capture_time_ms, 250, false);
clock_.AdvanceTimeMilliseconds(kTimeStep);
- TickTime::AdvanceFakeClock(kTimeStep);
EXPECT_CALL(callback_, TimeToSendPadding(250)).Times(1).
WillOnce(Return(250));
send_bucket_->Process();
@@ -323,7 +310,6 @@
media_payload, false));
media_bytes += media_payload;
clock_.AdvanceTimeMilliseconds(kTimeStep);
- TickTime::AdvanceFakeClock(kTimeStep);
send_bucket_->Process();
}
EXPECT_NEAR(kTargetBitrate, 8 * (media_bytes + callback.padding_sent()) /
@@ -365,7 +351,6 @@
EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
clock_.AdvanceTimeMilliseconds(5);
- TickTime::AdvanceFakeClock(5);
EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
EXPECT_EQ(0, send_bucket_->Process());
@@ -376,7 +361,6 @@
EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
clock_.AdvanceTimeMilliseconds(5);
- TickTime::AdvanceFakeClock(5);
EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
EXPECT_EQ(0, send_bucket_->Process());
}
@@ -408,7 +392,6 @@
ssrc, sequence_number++, capture_time_ms, 250, false));
clock_.AdvanceTimeMilliseconds(10000);
- TickTime::AdvanceFakeClock(10000);
int64_t second_capture_time_ms = clock_.TimeInMilliseconds();
// Expect everything to be queued.
@@ -425,7 +408,6 @@
for (int i = 0; i < 10; ++i) {
clock_.AdvanceTimeMilliseconds(5);
- TickTime::AdvanceFakeClock(5);
EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
EXPECT_EQ(0, send_bucket_->Process());
}
@@ -438,7 +420,6 @@
EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
clock_.AdvanceTimeMilliseconds(5);
- TickTime::AdvanceFakeClock(5);
EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
EXPECT_EQ(0, send_bucket_->Process());
@@ -448,7 +429,6 @@
.WillRepeatedly(Return(true));
EXPECT_EQ(5, send_bucket_->TimeUntilNextProcess());
clock_.AdvanceTimeMilliseconds(5);
- TickTime::AdvanceFakeClock(5);
EXPECT_EQ(0, send_bucket_->TimeUntilNextProcess());
EXPECT_EQ(0, send_bucket_->Process());
EXPECT_EQ(0, send_bucket_->QueueInMs());
@@ -467,7 +447,6 @@
250,
false));
clock_.AdvanceTimeMilliseconds(1);
- TickTime::AdvanceFakeClock(1);
EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority,
ssrc,
sequence_number + 1,
@@ -475,7 +454,6 @@
250,
false));
clock_.AdvanceTimeMilliseconds(9999);
- TickTime::AdvanceFakeClock(9999);
EXPECT_EQ(clock_.TimeInMilliseconds() - capture_time_ms,
send_bucket_->QueueInMs());
// Fails to send first packet so only one call.
@@ -484,7 +462,6 @@
.Times(1)
.WillOnce(Return(false));
clock_.AdvanceTimeMilliseconds(10000);
- TickTime::AdvanceFakeClock(10000);
send_bucket_->Process();
// Queue remains unchanged.
@@ -501,7 +478,6 @@
.Times(1)
.WillOnce(Return(false));
clock_.AdvanceTimeMilliseconds(10000);
- TickTime::AdvanceFakeClock(10000);
send_bucket_->Process();
// Queue is reduced by 1 packet.
@@ -514,7 +490,6 @@
.Times(1)
.WillOnce(Return(true));
clock_.AdvanceTimeMilliseconds(10000);
- TickTime::AdvanceFakeClock(10000);
send_bucket_->Process();
EXPECT_EQ(0, send_bucket_->QueueInMs());
}
@@ -535,7 +510,6 @@
}
clock_.AdvanceTimeMilliseconds(2001);
- TickTime::AdvanceFakeClock(2001);
SendAndExpectPacket(PacedSender::kNormalPriority,
ssrc,
sequence_number++,
@@ -546,7 +520,7 @@
send_bucket_->Process();
EXPECT_EQ(0, send_bucket_->QueueInMs());
clock_.AdvanceTimeMilliseconds(31);
- TickTime::AdvanceFakeClock(31);
+
send_bucket_->Process();
}
@@ -564,7 +538,6 @@
false);
clock_.AdvanceTimeMilliseconds(500);
- TickTime::AdvanceFakeClock(500);
EXPECT_EQ(500, send_bucket_->QueueInMs());
send_bucket_->Process();
EXPECT_EQ(0, send_bucket_->QueueInMs());
diff --git a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h
index b662849..7b0a4f8 100644
--- a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h
+++ b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h
@@ -47,8 +47,8 @@
* intra_frame_callback - Called when the receiver request a intra frame.
* bandwidth_callback - Called when we receive a changed estimate from
* the receiver of out stream.
- * audio_messages - Telehone events. May not be NULL; default callback
- * will do nothing.
+ * audio_messages - Telephone events. May not be NULL; default
+ * callback will do nothing.
* remote_bitrate_estimator - Estimates the bandwidth available for a set of
* streams from the same client.
* paced_sender - Spread any bursts of packets into smaller
@@ -69,6 +69,7 @@
PacedSender* paced_sender;
BitrateStatisticsObserver* send_bitrate_observer;
FrameCountObserver* send_frame_count_observer;
+ SendSideDelayObserver* send_side_delay_observer;
};
/*
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
index f706b42..0c771c5 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_rtcp_impl.cc
@@ -39,7 +39,8 @@
remote_bitrate_estimator(NULL),
paced_sender(NULL),
send_bitrate_observer(NULL),
- send_frame_count_observer(NULL) {
+ send_frame_count_observer(NULL),
+ send_side_delay_observer(NULL) {
}
RtpRtcp* RtpRtcp::CreateRtpRtcp(const RtpRtcp::Configuration& configuration) {
@@ -64,7 +65,8 @@
configuration.audio_messages,
configuration.paced_sender,
configuration.send_bitrate_observer,
- configuration.send_frame_count_observer),
+ configuration.send_frame_count_observer,
+ configuration.send_side_delay_observer),
rtcp_sender_(configuration.id,
configuration.audio,
configuration.clock,
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
index e638074..c24b15a 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc
@@ -47,7 +47,8 @@
RtpAudioFeedback* audio_feedback,
PacedSender* paced_sender,
BitrateStatisticsObserver* bitrate_callback,
- FrameCountObserver* frame_count_observer)
+ FrameCountObserver* frame_count_observer,
+ SendSideDelayObserver* send_side_delay_observer)
: clock_(clock),
bitrate_sent_(clock, this),
id_(id),
@@ -75,6 +76,7 @@
rtp_stats_callback_(NULL),
bitrate_callback_(bitrate_callback),
frame_count_observer_(frame_count_observer),
+ send_side_delay_observer_(send_side_delay_observer),
// RTP variables
start_timestamp_forced_(false),
start_timestamp_(0),
@@ -164,9 +166,7 @@
bool RTPSender::GetSendSideDelay(int* avg_send_delay_ms,
int* max_send_delay_ms) const {
- if (!SendingMedia())
- return false;
- CriticalSectionScoped cs(statistics_crit_.get());
+ CriticalSectionScoped lock(statistics_crit_.get());
SendDelayMap::const_iterator it = send_delays_.upper_bound(
clock_->TimeInMilliseconds() - kSendSideDelayWindowMs);
if (it == send_delays_.end())
@@ -997,10 +997,26 @@
}
void RTPSender::UpdateDelayStatistics(int64_t capture_time_ms, int64_t now_ms) {
- CriticalSectionScoped cs(statistics_crit_.get());
- send_delays_[now_ms] = now_ms - capture_time_ms;
- send_delays_.erase(send_delays_.begin(),
- send_delays_.lower_bound(now_ms - kSendSideDelayWindowMs));
+ uint32_t ssrc;
+ int avg_delay_ms = 0;
+ int max_delay_ms = 0;
+ {
+ CriticalSectionScoped lock(send_critsect_);
+ ssrc = ssrc_;
+ }
+ {
+ CriticalSectionScoped cs(statistics_crit_.get());
+ // TODO(holmer): Compute this iteratively instead.
+ send_delays_[now_ms] = now_ms - capture_time_ms;
+ send_delays_.erase(send_delays_.begin(),
+ send_delays_.lower_bound(now_ms -
+ kSendSideDelayWindowMs));
+ }
+ if (send_side_delay_observer_ &&
+ GetSendSideDelay(&avg_delay_ms, &max_delay_ms)) {
+ send_side_delay_observer_->SendSideDelayUpdated(avg_delay_ms,
+ max_delay_ms, ssrc);
+ }
}
void RTPSender::ProcessBitrate() {
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender.h b/webrtc/modules/rtp_rtcp/source/rtp_sender.h
index f65c8c2..4a9e10e 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h
@@ -71,7 +71,8 @@
Transport *transport, RtpAudioFeedback *audio_feedback,
PacedSender *paced_sender,
BitrateStatisticsObserver* bitrate_callback,
- FrameCountObserver* frame_count_observer);
+ FrameCountObserver* frame_count_observer,
+ SendSideDelayObserver* send_side_delay_observer);
virtual ~RTPSender();
void ProcessBitrate();
@@ -379,6 +380,7 @@
StreamDataCountersCallback* rtp_stats_callback_ GUARDED_BY(statistics_crit_);
BitrateStatisticsObserver* const bitrate_callback_;
FrameCountObserver* const frame_count_observer_;
+ SendSideDelayObserver* const send_side_delay_observer_;
// RTP variables
bool start_timestamp_forced_ GUARDED_BY(send_critsect_);
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
index 57f1460..40b1054 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -94,7 +94,7 @@
virtual void SetUp() {
rtp_sender_.reset(new RTPSender(0, false, &fake_clock_, &transport_, NULL,
- &mock_paced_sender_, NULL, NULL));
+ &mock_paced_sender_, NULL, NULL, NULL));
rtp_sender_->SetSequenceNumber(kSeqNum);
}
@@ -672,7 +672,7 @@
TEST_F(RtpSenderTest, SendRedundantPayloads) {
MockTransport transport;
rtp_sender_.reset(new RTPSender(0, false, &fake_clock_, &transport, NULL,
- &mock_paced_sender_, NULL, NULL));
+ &mock_paced_sender_, NULL, NULL, NULL));
rtp_sender_->SetSequenceNumber(kSeqNum);
// Make all packets go through the pacer.
EXPECT_CALL(mock_paced_sender_,
@@ -818,7 +818,7 @@
} callback;
rtp_sender_.reset(new RTPSender(0, false, &fake_clock_, &transport_, NULL,
- &mock_paced_sender_, NULL, &callback));
+ &mock_paced_sender_, NULL, &callback, NULL));
char payload_name[RTP_PAYLOAD_NAME_SIZE] = "GENERIC";
const uint8_t payload_type = 127;
@@ -867,7 +867,7 @@
BitrateStatistics bitrate_;
} callback;
rtp_sender_.reset(new RTPSender(0, false, &fake_clock_, &transport_, NULL,
- &mock_paced_sender_, &callback, NULL));
+ &mock_paced_sender_, &callback, NULL, NULL));
// Simulate kNumPackets sent with kPacketInterval ms intervals.
const uint32_t kNumPackets = 15;
@@ -923,7 +923,7 @@
virtual void SetUp() {
payload_ = kAudioPayload;
rtp_sender_.reset(new RTPSender(0, true, &fake_clock_, &transport_, NULL,
- &mock_paced_sender_, NULL, NULL));
+ &mock_paced_sender_, NULL, NULL, NULL));
rtp_sender_->SetSequenceNumber(kSeqNum);
}
};
diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc
index cdb0fc6..3cb9624 100644
--- a/webrtc/video/end_to_end_tests.cc
+++ b/webrtc/video/end_to_end_tests.cc
@@ -1345,9 +1345,6 @@
send_stats_filled_["NumStreams"] |=
stats.substreams.size() == expected_send_ssrcs_.size();
- send_stats_filled_["Delay"] |=
- stats.avg_delay_ms != 0 || stats.max_delay_ms != 0;
-
for (std::map<uint32_t, StreamStats>::const_iterator it =
stats.substreams.begin();
it != stats.substreams.end();
@@ -1380,6 +1377,9 @@
send_stats_filled_[CompoundKey("OutgoingRate", it->first)] |=
stats.encode_frame_rate != 0;
+
+ send_stats_filled_[CompoundKey("Delay", it->first)] |=
+ stream_stats.avg_delay_ms != 0 || stream_stats.max_delay_ms != 0;
}
return AllStatsFilled(send_stats_filled_);
diff --git a/webrtc/video/send_statistics_proxy.cc b/webrtc/video/send_statistics_proxy.cc
index cb81d5a..5b44fee 100644
--- a/webrtc/video/send_statistics_proxy.cc
+++ b/webrtc/video/send_statistics_proxy.cc
@@ -17,10 +17,8 @@
namespace webrtc {
SendStatisticsProxy::SendStatisticsProxy(
- const VideoSendStream::Config& config,
- SendStatisticsProxy::StatsProvider* stats_provider)
+ const VideoSendStream::Config& config)
: config_(config),
- stats_provider_(stats_provider),
crit_(CriticalSectionWrapper::CreateCriticalSection()) {
}
@@ -45,13 +43,8 @@
}
VideoSendStream::Stats SendStatisticsProxy::GetStats() const {
- VideoSendStream::Stats stats;
- {
- CriticalSectionScoped lock(crit_.get());
- stats = stats_;
- }
- stats_provider_->GetSendSideDelay(&stats);
- return stats;
+ CriticalSectionScoped lock(crit_.get());
+ return stats_;
}
StreamStats* SendStatisticsProxy::GetStatsEntry(uint32_t ssrc) {
@@ -119,4 +112,15 @@
}
}
+void SendStatisticsProxy::SendSideDelayUpdated(int avg_delay_ms,
+ int max_delay_ms,
+ uint32_t ssrc) {
+ CriticalSectionScoped lock(crit_.get());
+ StreamStats* stats = GetStatsEntry(ssrc);
+ if (stats == NULL)
+ return;
+ stats->avg_delay_ms = avg_delay_ms;
+ stats->max_delay_ms = max_delay_ms;
+}
+
} // namespace webrtc
diff --git a/webrtc/video/send_statistics_proxy.h b/webrtc/video/send_statistics_proxy.h
index a313c13..1b888b0 100644
--- a/webrtc/video/send_statistics_proxy.h
+++ b/webrtc/video/send_statistics_proxy.h
@@ -29,19 +29,10 @@
public BitrateStatisticsObserver,
public FrameCountObserver,
public ViEEncoderObserver,
- public ViECaptureObserver {
+ public ViECaptureObserver,
+ public SendSideDelayObserver {
public:
- class StatsProvider {
- protected:
- StatsProvider() {}
- virtual ~StatsProvider() {}
-
- public:
- virtual bool GetSendSideDelay(VideoSendStream::Stats* stats) = 0;
- };
-
- SendStatisticsProxy(const VideoSendStream::Config& config,
- StatsProvider* stats_provider);
+ explicit SendStatisticsProxy(const VideoSendStream::Config& config);
virtual ~SendStatisticsProxy();
VideoSendStream::Stats GetStats() const;
@@ -79,11 +70,14 @@
virtual void NoPictureAlarm(const int capture_id,
const CaptureAlarm alarm) OVERRIDE {}
+ virtual void SendSideDelayUpdated(int avg_delay_ms,
+ int max_delay_ms,
+ uint32_t ssrc) OVERRIDE;
+
private:
StreamStats* GetStatsEntry(uint32_t ssrc) EXCLUSIVE_LOCKS_REQUIRED(crit_);
const VideoSendStream::Config config_;
- StatsProvider* const stats_provider_;
scoped_ptr<CriticalSectionWrapper> crit_;
VideoSendStream::Stats stats_ GUARDED_BY(crit_);
};
diff --git a/webrtc/video/send_statistics_proxy_unittest.cc b/webrtc/video/send_statistics_proxy_unittest.cc
index d800761..c930a2b 100644
--- a/webrtc/video/send_statistics_proxy_unittest.cc
+++ b/webrtc/video/send_statistics_proxy_unittest.cc
@@ -19,8 +19,7 @@
namespace webrtc {
-class SendStatisticsProxyTest : public ::testing::Test,
- protected SendStatisticsProxy::StatsProvider {
+class SendStatisticsProxyTest : public ::testing::Test {
public:
SendStatisticsProxyTest() : avg_delay_ms_(0), max_delay_ms_(0) {}
virtual ~SendStatisticsProxyTest() {}
@@ -28,7 +27,7 @@
protected:
virtual void SetUp() {
statistics_proxy_.reset(
- new SendStatisticsProxy(GetTestConfig(), this));
+ new SendStatisticsProxy(GetTestConfig()));
config_ = GetTestConfig();
expected_ = VideoSendStream::Stats();
}
@@ -40,18 +39,9 @@
return config;
}
- virtual bool GetSendSideDelay(VideoSendStream::Stats* stats) OVERRIDE {
- stats->avg_delay_ms = avg_delay_ms_;
- stats->max_delay_ms = max_delay_ms_;
- return true;
- }
-
void ExpectEqual(VideoSendStream::Stats one, VideoSendStream::Stats other) {
- EXPECT_EQ(one.avg_delay_ms, other.avg_delay_ms);
EXPECT_EQ(one.input_frame_rate, other.input_frame_rate);
EXPECT_EQ(one.encode_frame_rate, other.encode_frame_rate);
- EXPECT_EQ(one.avg_delay_ms, other.avg_delay_ms);
- EXPECT_EQ(one.max_delay_ms, other.max_delay_ms);
EXPECT_EQ(one.suspended, other.suspended);
EXPECT_EQ(one.substreams.size(), other.substreams.size());
@@ -68,6 +58,8 @@
EXPECT_EQ(a.key_frames, b.key_frames);
EXPECT_EQ(a.delta_frames, b.delta_frames);
EXPECT_EQ(a.bitrate_bps, b.bitrate_bps);
+ EXPECT_EQ(a.avg_delay_ms, b.avg_delay_ms);
+ EXPECT_EQ(a.max_delay_ms, b.max_delay_ms);
EXPECT_EQ(a.rtp_stats.bytes, b.rtp_stats.bytes);
EXPECT_EQ(a.rtp_stats.header_bytes, b.rtp_stats.header_bytes);
@@ -190,6 +182,7 @@
++it) {
const uint32_t ssrc = *it;
BitrateStatistics bitrate;
+ // Use ssrc as bitrate_bps to get a unique value for each stream.
bitrate.bitrate_bps = ssrc;
observer->Notify(bitrate, ssrc);
expected_.substreams[ssrc].bitrate_bps = ssrc;
@@ -199,14 +192,23 @@
ExpectEqual(expected_, stats);
}
-TEST_F(SendStatisticsProxyTest, StreamStats) {
- avg_delay_ms_ = 1;
- max_delay_ms_ = 2;
+TEST_F(SendStatisticsProxyTest, SendSideDelay) {
+ SendSideDelayObserver* observer = statistics_proxy_.get();
+ for (std::vector<uint32_t>::const_iterator it = config_.rtp.ssrcs.begin();
+ it != config_.rtp.ssrcs.end();
+ ++it) {
+ const uint32_t ssrc = *it;
+ // Use ssrc as avg_delay_ms and max_delay_ms to get a unique value for each
+ // stream.
+ int avg_delay_ms = ssrc;
+ int max_delay_ms = ssrc + 1;
+ observer->SendSideDelayUpdated(avg_delay_ms, max_delay_ms, ssrc);
+ expected_.substreams[ssrc].avg_delay_ms = avg_delay_ms;
+ expected_.substreams[ssrc].max_delay_ms = max_delay_ms;
+ }
VideoSendStream::Stats stats = statistics_proxy_->GetStats();
-
- EXPECT_EQ(avg_delay_ms_, stats.avg_delay_ms);
- EXPECT_EQ(max_delay_ms_, stats.max_delay_ms);
+ ExpectEqual(expected_, stats);
}
TEST_F(SendStatisticsProxyTest, NoSubstreams) {
diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc
index 2a2adbf..45e5c69 100644
--- a/webrtc/video/video_send_stream.cc
+++ b/webrtc/video/video_send_stream.cc
@@ -125,7 +125,7 @@
suspended_ssrcs_(suspended_ssrcs),
external_codec_(NULL),
channel_(-1),
- stats_proxy_(new SendStatisticsProxy(config, this)) {
+ stats_proxy_(config) {
video_engine_base_ = ViEBase::GetInterface(video_engine);
video_engine_base_->CreateChannel(channel_, base_channel);
assert(channel_ != -1);
@@ -216,6 +216,8 @@
if (overuse_observer)
video_engine_base_->RegisterCpuOveruseObserver(channel_, overuse_observer);
+ video_engine_base_->RegisterSendSideDelayObserver(channel_, &stats_proxy_);
+
image_process_ = ViEImageProcess::GetInterface(video_engine);
image_process_->RegisterPreEncodeCallback(channel_,
config_.pre_encode_callback);
@@ -228,26 +230,26 @@
codec_->SuspendBelowMinBitrate(channel_);
rtp_rtcp_->RegisterSendChannelRtcpStatisticsCallback(channel_,
- stats_proxy_.get());
+ &stats_proxy_);
rtp_rtcp_->RegisterSendChannelRtpStatisticsCallback(channel_,
- stats_proxy_.get());
- rtp_rtcp_->RegisterSendBitrateObserver(channel_, stats_proxy_.get());
- rtp_rtcp_->RegisterSendFrameCountObserver(channel_, stats_proxy_.get());
+ &stats_proxy_);
+ rtp_rtcp_->RegisterSendBitrateObserver(channel_, &stats_proxy_);
+ rtp_rtcp_->RegisterSendFrameCountObserver(channel_, &stats_proxy_);
- codec_->RegisterEncoderObserver(channel_, *stats_proxy_);
- capture_->RegisterObserver(capture_id_, *stats_proxy_);
+ codec_->RegisterEncoderObserver(channel_, stats_proxy_);
+ capture_->RegisterObserver(capture_id_, stats_proxy_);
}
VideoSendStream::~VideoSendStream() {
capture_->DeregisterObserver(capture_id_);
codec_->DeregisterEncoderObserver(channel_);
- rtp_rtcp_->DeregisterSendFrameCountObserver(channel_, stats_proxy_.get());
- rtp_rtcp_->DeregisterSendBitrateObserver(channel_, stats_proxy_.get());
+ rtp_rtcp_->DeregisterSendFrameCountObserver(channel_, &stats_proxy_);
+ rtp_rtcp_->DeregisterSendBitrateObserver(channel_, &stats_proxy_);
rtp_rtcp_->DeregisterSendChannelRtpStatisticsCallback(channel_,
- stats_proxy_.get());
+ &stats_proxy_);
rtp_rtcp_->DeregisterSendChannelRtcpStatisticsCallback(channel_,
- stats_proxy_.get());
+ &stats_proxy_);
image_process_->DeRegisterPreEncodeCallback(channel_);
@@ -395,12 +397,7 @@
}
VideoSendStream::Stats VideoSendStream::GetStats() const {
- return stats_proxy_->GetStats();
-}
-
-bool VideoSendStream::GetSendSideDelay(VideoSendStream::Stats* stats) {
- return codec_->GetSendSideDelay(
- channel_, &stats->avg_delay_ms, &stats->max_delay_ms);
+ return stats_proxy_.GetStats();
}
void VideoSendStream::ConfigureSsrcs() {
diff --git a/webrtc/video/video_send_stream.h b/webrtc/video/video_send_stream.h
index fc4a865..b0cf0c6 100644
--- a/webrtc/video/video_send_stream.h
+++ b/webrtc/video/video_send_stream.h
@@ -38,8 +38,7 @@
namespace internal {
class VideoSendStream : public webrtc::VideoSendStream,
- public VideoSendStreamInput,
- public SendStatisticsProxy::StatsProvider {
+ public VideoSendStreamInput {
public:
VideoSendStream(newapi::Transport* transport,
CpuOveruseObserver* overuse_observer,
@@ -72,10 +71,6 @@
typedef std::map<uint32_t, RtpState> RtpStateMap;
RtpStateMap GetRtpStates() const;
- protected:
- // From SendStatisticsProxy::StreamStatsProvider.
- virtual bool GetSendSideDelay(VideoSendStream::Stats* stats) OVERRIDE;
-
private:
void ConfigureSsrcs();
TransportAdapter transport_adapter_;
@@ -96,7 +91,7 @@
int channel_;
int capture_id_;
- const scoped_ptr<SendStatisticsProxy> stats_proxy_;
+ SendStatisticsProxy stats_proxy_;
};
} // namespace internal
} // namespace webrtc
diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc
index ba95d4d..a11a54c 100644
--- a/webrtc/video/video_send_stream_tests.cc
+++ b/webrtc/video/video_send_stream_tests.cc
@@ -27,6 +27,7 @@
#include "webrtc/system_wrappers/interface/scoped_vector.h"
#include "webrtc/system_wrappers/interface/sleep.h"
#include "webrtc/system_wrappers/interface/thread_wrapper.h"
+#include "webrtc/system_wrappers/interface/logging.h"
#include "webrtc/test/call_test.h"
#include "webrtc/test/configurable_frame_size_encoder.h"
#include "webrtc/test/null_transport.h"
@@ -956,8 +957,8 @@
bool CheckStats() {
VideoSendStream::Stats stats = stream_->GetStats();
// Check that all applicable data sources have been used.
- if (stats.input_frame_rate > 0 && stats.encode_frame_rate > 0 &&
- stats.avg_delay_ms > 0 && !stats.substreams.empty()) {
+ if (stats.input_frame_rate > 0 && stats.encode_frame_rate > 0
+ && !stats.substreams.empty()) {
uint32_t ssrc = stats.substreams.begin()->first;
EXPECT_NE(
config_.rtp.ssrcs.end(),
@@ -967,7 +968,8 @@
// data is received from remote side. Tested in call tests instead.
const StreamStats& entry = stats.substreams[ssrc];
if (entry.key_frames > 0u && entry.bitrate_bps > 0 &&
- entry.rtp_stats.packets > 0u) {
+ entry.rtp_stats.packets > 0u && entry.avg_delay_ms > 0 &&
+ entry.max_delay_ms > 0) {
return true;
}
}
diff --git a/webrtc/video_engine/include/vie_base.h b/webrtc/video_engine/include/vie_base.h
index f4b99ae..2362570 100644
--- a/webrtc/video_engine/include/vie_base.h
+++ b/webrtc/video_engine/include/vie_base.h
@@ -204,6 +204,13 @@
// Gets cpu overuse measures.
virtual int GetCpuOveruseMetrics(int channel, CpuOveruseMetrics* metrics) = 0;
+ // Registers a callback which is called when send-side delay statistics has
+ // been updated.
+ // TODO(holmer): Remove the default implementation when fakevideoengine.h has
+ // been updated.
+ virtual void RegisterSendSideDelayObserver(
+ int channel, SendSideDelayObserver* observer) {}
+
// Specifies the VoiceEngine and VideoEngine channel pair to use for
// audio/video synchronization.
virtual int ConnectAudioChannel(const int video_channel,
diff --git a/webrtc/video_engine/vie_base_impl.cc b/webrtc/video_engine/vie_base_impl.cc
index 6ba2fd5..824d311 100644
--- a/webrtc/video_engine/vie_base_impl.cc
+++ b/webrtc/video_engine/vie_base_impl.cc
@@ -144,6 +144,14 @@
return -1;
}
+void ViEBaseImpl::RegisterSendSideDelayObserver(
+ int channel, SendSideDelayObserver* observer) {
+ ViEChannelManagerScoped cs(*(shared_data_.channel_manager()));
+ ViEChannel* vie_channel = cs.Channel(channel);
+ assert(vie_channel);
+ vie_channel->RegisterSendSideDelayObserver(observer);
+}
+
int ViEBaseImpl::CreateChannel(int& video_channel) { // NOLINT
return CreateChannel(video_channel, static_cast<const Config*>(NULL));
}
diff --git a/webrtc/video_engine/vie_base_impl.h b/webrtc/video_engine/vie_base_impl.h
index 2f847bc..20fd615 100644
--- a/webrtc/video_engine/vie_base_impl.h
+++ b/webrtc/video_engine/vie_base_impl.h
@@ -37,6 +37,8 @@
const CpuOveruseOptions& options);
virtual int GetCpuOveruseMetrics(int channel,
CpuOveruseMetrics* metrics);
+ virtual void RegisterSendSideDelayObserver(int channel,
+ SendSideDelayObserver* observer) OVERRIDE;
virtual int CreateChannel(int& video_channel); // NOLINT
virtual int CreateChannel(int& video_channel, // NOLINT
const Config* config);
diff --git a/webrtc/video_engine/vie_channel.cc b/webrtc/video_engine/vie_channel.cc
index 1f983e7..5b3b77f 100644
--- a/webrtc/video_engine/vie_channel.cc
+++ b/webrtc/video_engine/vie_channel.cc
@@ -118,6 +118,7 @@
configuration.receive_statistics = vie_receiver_.GetReceiveStatistics();
configuration.send_bitrate_observer = &send_bitrate_observer_;
configuration.send_frame_count_observer = &send_frame_count_observer_;
+ configuration.send_side_delay_observer = &send_side_delay_observer_;
rtp_rtcp_.reset(RtpRtcp::CreateRtpRtcp(configuration));
vie_receiver_.SetRtpRtcpModule(rtp_rtcp_.get());
@@ -1200,6 +1201,11 @@
return valid_estimate;
}
+void ViEChannel::RegisterSendSideDelayObserver(
+ SendSideDelayObserver* observer) {
+ send_side_delay_observer_.Set(observer);
+}
+
void ViEChannel::RegisterSendBitrateObserver(
BitrateStatisticsObserver* observer) {
send_bitrate_observer_.Set(observer);
@@ -1558,6 +1564,7 @@
configuration.bandwidth_callback = bandwidth_observer_.get();
configuration.rtt_stats = rtt_stats_;
configuration.paced_sender = paced_sender_;
+ configuration.send_side_delay_observer = &send_side_delay_observer_;
return RtpRtcp::CreateRtpRtcp(configuration);
}
diff --git a/webrtc/video_engine/vie_channel.h b/webrtc/video_engine/vie_channel.h
index ac417ec..54c1a21 100644
--- a/webrtc/video_engine/vie_channel.h
+++ b/webrtc/video_engine/vie_channel.h
@@ -212,7 +212,10 @@
uint32_t* video_bitrate_sent,
uint32_t* fec_bitrate_sent,
uint32_t* nackBitrateSent) const;
+ // TODO(holmer): Deprecated. We should use the SendSideDelayObserver instead
+ // to avoid deadlocks.
bool GetSendSideDelay(int* avg_send_delay, int* max_send_delay) const;
+ void RegisterSendSideDelayObserver(SendSideDelayObserver* observer);
void GetReceiveBandwidthEstimatorStats(
ReceiveBandwidthEstimatorStats* output) const;
@@ -429,6 +432,17 @@
}
} send_frame_count_observer_;
+ class RegisterableSendSideDelayObserver :
+ public RegisterableCallback<SendSideDelayObserver> {
+ virtual void SendSideDelayUpdated(int avg_delay_ms,
+ int max_delay_ms,
+ uint32_t ssrc) OVERRIDE {
+ CriticalSectionScoped cs(critsect_.get());
+ if (callback_)
+ callback_->SendSideDelayUpdated(avg_delay_ms, max_delay_ms, ssrc);
+ }
+ } send_side_delay_observer_;
+
int32_t channel_id_;
int32_t engine_id_;
uint32_t number_of_cores_;
diff --git a/webrtc/video_send_stream.h b/webrtc/video_send_stream.h
index bb4cff2..8c9d5b7 100644
--- a/webrtc/video_send_stream.h
+++ b/webrtc/video_send_stream.h
@@ -41,14 +41,9 @@
Stats()
: input_frame_rate(0),
encode_frame_rate(0),
- avg_delay_ms(0),
- max_delay_ms(0),
suspended(false) {}
-
int input_frame_rate;
int encode_frame_rate;
- int avg_delay_ms;
- int max_delay_ms;
bool suspended;
std::map<uint32_t, StreamStats> substreams;
};
