diff --git a/webrtc/build/tsan_suppressions.cc b/webrtc/build/tsan_suppressions.cc index 1e79ccbf0..832330664 100644 --- a/webrtc/build/tsan_suppressions.cc +++ b/webrtc/build/tsan_suppressions.cc @@ -71,7 +71,6 @@ char kTSanDefaultSuppressions[] = "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 796ddca04..05e1faf55 100644 --- a/webrtc/common_types.h +++ b/webrtc/common_types.h @@ -269,6 +269,15 @@ class FrameCountObserver { 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 9c8a902a0..2e96ec1c0 100644 --- a/webrtc/config.h +++ b/webrtc/config.h @@ -34,10 +34,17 @@ struct RtpStatistics { }; 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 55497db39..b9151a5fc 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 @@ class PacedSender : public Module { scoped_ptr 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 323cafec2..52e9cfb42 100644 --- a/webrtc/modules/pacing/paced_sender.cc +++ b/webrtc/modules/pacing/paced_sender.cc @@ -142,7 +142,7 @@ PacedSender::PacedSender(Clock* clock, 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 @@ int PacedSender::QueueInMs() const { 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::TimeUntilNextProcess() { } 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 @@ bool PacedSender::ShouldSendNextPacket(paced_sender::PacketList** packet_list) { 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 @@ paced_sender::Packet PacedSender::GetNextPacketFromList( // 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 551885588..14dcdbc51 100644 --- a/webrtc/modules/pacing/paced_sender_unittest.cc +++ b/webrtc/modules/pacing/paced_sender_unittest.cc @@ -58,7 +58,6 @@ class PacedSenderTest : public ::testing::Test { 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 @@ TEST_F(PacedSenderTest, QueuePacket) { 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 @@ TEST_F(PacedSenderTest, PaceQueuedPackets) { 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 @@ TEST_F(PacedSenderTest, PaceQueuedPackets) { } 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 @@ TEST_F(PacedSenderTest, PaceQueuedPacketsWithDuplicates) { 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 @@ TEST_F(PacedSenderTest, PaceQueuedPacketsWithDuplicates) { } 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 @@ TEST_F(PacedSenderTest, CanQueuePacketsWithSameSequenceNumberOnDifferentSsrcs) { false); clock_.AdvanceTimeMilliseconds(1000); - TickTime::AdvanceFakeClock(1000); send_bucket_->Process(); } @@ -253,7 +245,6 @@ TEST_F(PacedSenderTest, Padding) { 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 @@ TEST_F(PacedSenderTest, Padding) { 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 @@ TEST_F(PacedSenderTest, NoPaddingWhenDisabled) { 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 @@ TEST_F(PacedSenderTest, VerifyPaddingUpToBitrate) { 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 @@ TEST_F(PacedSenderTest, VerifyAverageBitrateVaryingMediaPayload) { 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 @@ TEST_F(PacedSenderTest, Priority) { 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 @@ TEST_F(PacedSenderTest, Priority) { 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 @@ TEST_F(PacedSenderTest, Pause) { 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 @@ TEST_F(PacedSenderTest, Pause) { 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 @@ TEST_F(PacedSenderTest, Pause) { 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 @@ TEST_F(PacedSenderTest, Pause) { .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 @@ TEST_F(PacedSenderTest, ResendPacket) { 250, false)); clock_.AdvanceTimeMilliseconds(1); - TickTime::AdvanceFakeClock(1); EXPECT_FALSE(send_bucket_->SendPacket(PacedSender::kNormalPriority, ssrc, sequence_number + 1, @@ -475,7 +454,6 @@ TEST_F(PacedSenderTest, ResendPacket) { 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 @@ TEST_F(PacedSenderTest, ResendPacket) { .Times(1) .WillOnce(Return(false)); clock_.AdvanceTimeMilliseconds(10000); - TickTime::AdvanceFakeClock(10000); send_bucket_->Process(); // Queue remains unchanged. @@ -501,7 +478,6 @@ TEST_F(PacedSenderTest, ResendPacket) { .Times(1) .WillOnce(Return(false)); clock_.AdvanceTimeMilliseconds(10000); - TickTime::AdvanceFakeClock(10000); send_bucket_->Process(); // Queue is reduced by 1 packet. @@ -514,7 +490,6 @@ TEST_F(PacedSenderTest, ResendPacket) { .Times(1) .WillOnce(Return(true)); clock_.AdvanceTimeMilliseconds(10000); - TickTime::AdvanceFakeClock(10000); send_bucket_->Process(); EXPECT_EQ(0, send_bucket_->QueueInMs()); } @@ -535,7 +510,6 @@ TEST_F(PacedSenderTest, MaxQueueLength) { } clock_.AdvanceTimeMilliseconds(2001); - TickTime::AdvanceFakeClock(2001); SendAndExpectPacket(PacedSender::kNormalPriority, ssrc, sequence_number++, @@ -546,7 +520,7 @@ TEST_F(PacedSenderTest, MaxQueueLength) { send_bucket_->Process(); EXPECT_EQ(0, send_bucket_->QueueInMs()); clock_.AdvanceTimeMilliseconds(31); - TickTime::AdvanceFakeClock(31); + send_bucket_->Process(); } @@ -564,7 +538,6 @@ TEST_F(PacedSenderTest, QueueTimeGrowsOverTime) { 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 b662849ba..7b0a4f8a4 100644 --- a/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h +++ b/webrtc/modules/rtp_rtcp/interface/rtp_rtcp.h @@ -47,8 +47,8 @@ class RtpRtcp : public Module { * 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 @@ class RtpRtcp : public Module { 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 f706b42e8..0c771c510 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 @@ RtpRtcp::Configuration::Configuration() 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 @@ ModuleRtpRtcpImpl::ModuleRtpRtcpImpl(const Configuration& configuration) 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 e638074cd..c24b15a36 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.cc @@ -47,7 +47,8 @@ RTPSender::RTPSender(const int32_t id, 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 @@ RTPSender::RTPSender(const int32_t id, 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 @@ uint32_t RTPSender::NackOverheadRate() const { 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 @@ int32_t RTPSender::SendToNetwork( } 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 f65c8c27e..4a9e10edf 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender.h @@ -71,7 +71,8 @@ class RTPSender : public RTPSenderInterface, public Bitrate::Observer { 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 @@ class RTPSender : public RTPSenderInterface, public Bitrate::Observer { 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 57f1460f4..40b105485 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 @@ class RtpSenderTest : public ::testing::Test { 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, SendPadding) { 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 @@ TEST_F(RtpSenderTest, FrameCountCallbacks) { } 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 @@ TEST_F(RtpSenderTest, BitrateCallbacks) { 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 @@ class RtpSenderAudioTest : public RtpSenderTest { 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 cdb0fc6c0..3cb962415 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -1345,9 +1345,6 @@ TEST_F(EndToEndTest, GetStats) { 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::const_iterator it = stats.substreams.begin(); it != stats.substreams.end(); @@ -1380,6 +1377,9 @@ TEST_F(EndToEndTest, GetStats) { 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 cb81d5a1d..5b44fee35 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 @@ void SendStatisticsProxy::CapturedFrameRate(const int capture_id, } 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::FrameCountUpdated(FrameType frame_type, } } +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 a313c13ef..1b888b037 100644 --- a/webrtc/video/send_statistics_proxy.h +++ b/webrtc/video/send_statistics_proxy.h @@ -29,19 +29,10 @@ class SendStatisticsProxy : public RtcpStatisticsCallback, 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 @@ class SendStatisticsProxy : public RtcpStatisticsCallback, 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 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 d8007613c..c930a2bc2 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 @@ class SendStatisticsProxyTest : public ::testing::Test, protected: virtual void SetUp() { statistics_proxy_.reset( - new SendStatisticsProxy(GetTestConfig(), this)); + new SendStatisticsProxy(GetTestConfig())); config_ = GetTestConfig(); expected_ = VideoSendStream::Stats(); } @@ -40,18 +39,9 @@ class SendStatisticsProxyTest : public ::testing::Test, 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 @@ class SendStatisticsProxyTest : public ::testing::Test, 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 @@ TEST_F(SendStatisticsProxyTest, Bitrate) { ++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 @@ TEST_F(SendStatisticsProxyTest, Bitrate) { 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::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 2a2adbf8a..45e5c69da 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -125,7 +125,7 @@ VideoSendStream::VideoSendStream( 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 @@ VideoSendStream::VideoSendStream( 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 @@ VideoSendStream::VideoSendStream( 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 @@ bool VideoSendStream::DeliverRtcp(const uint8_t* packet, size_t length) { } 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 fc4a8650b..b0cf0c6a4 100644 --- a/webrtc/video/video_send_stream.h +++ b/webrtc/video/video_send_stream.h @@ -38,8 +38,7 @@ class ViERTP_RTCP; 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 @@ class VideoSendStream : public webrtc::VideoSendStream, typedef std::map 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 @@ class VideoSendStream : public webrtc::VideoSendStream, int channel_; int capture_id_; - const scoped_ptr 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 ba95d4d60..a11a54cd0 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 @@ TEST_F(VideoSendStreamTest, ProducesStats) { 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 @@ TEST_F(VideoSendStreamTest, ProducesStats) { // 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 f4b99ae79..236257060 100644 --- a/webrtc/video_engine/include/vie_base.h +++ b/webrtc/video_engine/include/vie_base.h @@ -204,6 +204,13 @@ class WEBRTC_DLLEXPORT ViEBase { // 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 6ba2fd5f3..824d311d0 100644 --- a/webrtc/video_engine/vie_base_impl.cc +++ b/webrtc/video_engine/vie_base_impl.cc @@ -144,6 +144,14 @@ int ViEBaseImpl::GetCpuOveruseMetrics(int video_channel, 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(NULL)); } diff --git a/webrtc/video_engine/vie_base_impl.h b/webrtc/video_engine/vie_base_impl.h index 2f847bc62..20fd61596 100644 --- a/webrtc/video_engine/vie_base_impl.h +++ b/webrtc/video_engine/vie_base_impl.h @@ -37,6 +37,8 @@ class ViEBaseImpl 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 1f983e73e..5b3b77fd1 100644 --- a/webrtc/video_engine/vie_channel.cc +++ b/webrtc/video_engine/vie_channel.cc @@ -118,6 +118,7 @@ ViEChannel::ViEChannel(int32_t channel_id, 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 @@ bool ViEChannel::GetSendSideDelay(int* avg_send_delay, 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 @@ RtpRtcp* ViEChannel::CreateRtpRtcpModule() { 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 ac417ec6b..54c1a212f 100644 --- a/webrtc/video_engine/vie_channel.h +++ b/webrtc/video_engine/vie_channel.h @@ -212,7 +212,10 @@ class ViEChannel 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 @@ class ViEChannel } } send_frame_count_observer_; + class RegisterableSendSideDelayObserver : + public RegisterableCallback { + 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 bb4cff20a..8c9d5b7fe 100644 --- a/webrtc/video_send_stream.h +++ b/webrtc/video_send_stream.h @@ -41,14 +41,9 @@ class VideoSendStream { 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 substreams; };