diff --git a/webrtc/video/rampup_tests.cc b/webrtc/video/rampup_tests.cc index 1772444bed..f44a30b587 100644 --- a/webrtc/video/rampup_tests.cc +++ b/webrtc/video/rampup_tests.cc @@ -217,7 +217,9 @@ class LowRateStreamObserver : public test::DirectTransport, sent_bytes_(0), total_overuse_bytes_(0), number_of_streams_(number_of_streams), - rtx_used_(rtx_used) { + rtx_used_(rtx_used), + send_stream_(NULL), + suspended_in_stats_(true) { RtpRtcp::Configuration config; config.receive_statistics = receive_stats_.get(); feedback_transport_.Enable(); @@ -238,6 +240,10 @@ class LowRateStreamObserver : public test::DirectTransport, test::DirectTransport::SetReceiver(this); } + virtual void SetSendStream(const VideoSendStream* send_stream) { + send_stream_ = send_stream; + } + virtual void OnReceiveBitrateChanged(const std::vector& ssrcs, unsigned int bitrate) { CriticalSectionScoped lock(critical_section_.get()); @@ -278,6 +284,7 @@ class LowRateStreamObserver : public test::DirectTransport, if (remote_bitrate_estimator_->TimeUntilNextProcess() <= 0) { remote_bitrate_estimator_->Process(); } + suspended_in_stats_ = send_stream_->GetStats().suspended; return true; } @@ -303,8 +310,11 @@ class LowRateStreamObserver : public test::DirectTransport, // This method defines the state machine for the ramp up-down-up test. void EvolveTestState(unsigned int bitrate_bps) { int64_t now = clock_->TimeInMilliseconds(); + assert(send_stream_ != NULL); + CriticalSectionScoped lock(critical_section_.get()); switch (test_state_) { case kFirstRampup: { + EXPECT_FALSE(suspended_in_stats_); if (bitrate_bps > kExpectedHighBitrateBps) { // The first ramp-up has reached the target bitrate. Change the // channel limit, and move to the next test state. @@ -325,7 +335,7 @@ class LowRateStreamObserver : public test::DirectTransport, break; } case kLowRate: { - if (bitrate_bps < kExpectedLowBitrateBps) { + if (bitrate_bps < kExpectedLowBitrateBps && suspended_in_stats_) { // The ramp-down was successful. Change the channel limit back to a // high value, and move to the next test state. forward_transport_config_.link_capacity_kbps = @@ -345,7 +355,7 @@ class LowRateStreamObserver : public test::DirectTransport, break; } case kSecondRampup: { - if (bitrate_bps > kExpectedHighBitrateBps) { + if (bitrate_bps > kExpectedHighBitrateBps && !suspended_in_stats_) { webrtc::test::PrintResult("ramp_up_down_up", GetModifierString(), "second_rampup", @@ -391,6 +401,8 @@ class LowRateStreamObserver : public test::DirectTransport, size_t total_overuse_bytes_; const size_t number_of_streams_; const bool rtx_used_; + const VideoSendStream* send_stream_; + bool suspended_in_stats_ GUARDED_BY(critical_section_); }; } @@ -496,6 +508,7 @@ class RampUpTest : public ::testing::Test { send_config.suspend_below_min_bitrate = true; VideoSendStream* send_stream = call->CreateVideoSendStream(send_config); + stream_observer.SetSendStream(send_stream); scoped_ptr frame_generator_capturer( test::FrameGeneratorCapturer::Create(send_stream->Input(), diff --git a/webrtc/video/send_statistics_proxy.cc b/webrtc/video/send_statistics_proxy.cc index 096f0a92ad..1cd4e26afc 100644 --- a/webrtc/video/send_statistics_proxy.cc +++ b/webrtc/video/send_statistics_proxy.cc @@ -32,6 +32,11 @@ void SendStatisticsProxy::OutgoingRate(const int video_channel, stats_.encode_frame_rate = framerate; } +void SendStatisticsProxy::SuspendChange(int video_channel, bool is_suspended) { + CriticalSectionScoped cs(lock_.get()); + stats_.suspended = is_suspended; +} + void SendStatisticsProxy::CapturedFrameRate(const int capture_id, const unsigned char frame_rate) { CriticalSectionScoped cs(lock_.get()); diff --git a/webrtc/video/send_statistics_proxy.h b/webrtc/video/send_statistics_proxy.h index 5ad4c4550d..2f45ff7f9b 100644 --- a/webrtc/video/send_statistics_proxy.h +++ b/webrtc/video/send_statistics_proxy.h @@ -18,6 +18,7 @@ #include "webrtc/video_engine/include/vie_capture.h" #include "webrtc/video_send_stream.h" #include "webrtc/system_wrappers/interface/scoped_ptr.h" +#include "webrtc/system_wrappers/interface/thread_annotations.h" namespace webrtc { @@ -67,7 +68,7 @@ class SendStatisticsProxy : public RtcpStatisticsCallback, const unsigned int framerate, const unsigned int bitrate) OVERRIDE; - virtual void SuspendChange(int video_channel, bool is_suspended) OVERRIDE {} + virtual void SuspendChange(int video_channel, bool is_suspended) OVERRIDE; // From ViECaptureObserver. virtual void BrightnessAlarm(const int capture_id, @@ -80,12 +81,12 @@ class SendStatisticsProxy : public RtcpStatisticsCallback, const CaptureAlarm alarm) OVERRIDE {} private: - StreamStats* GetStatsEntry(uint32_t ssrc); + StreamStats* GetStatsEntry(uint32_t ssrc) EXCLUSIVE_LOCKS_REQUIRED(lock_); const VideoSendStream::Config config_; scoped_ptr lock_; - VideoSendStream::Stats stats_; - StatsProvider* stats_provider_; + VideoSendStream::Stats stats_ GUARDED_BY(lock_); + StatsProvider* const stats_provider_; }; } // namespace webrtc diff --git a/webrtc/video/send_statistics_proxy_unittest.cc b/webrtc/video/send_statistics_proxy_unittest.cc index ed74a4f775..8f35ee4d03 100644 --- a/webrtc/video/send_statistics_proxy_unittest.cc +++ b/webrtc/video/send_statistics_proxy_unittest.cc @@ -54,6 +54,7 @@ class SendStatisticsProxyTest : public ::testing::Test, 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.c_name, other.c_name); EXPECT_EQ(one.substreams.size(), other.substreams.size()); @@ -131,6 +132,20 @@ TEST_F(SendStatisticsProxyTest, FrameRates) { EXPECT_EQ(encode_fps, stats.encode_frame_rate); } +TEST_F(SendStatisticsProxyTest, Suspended) { + // Verify that the value is false by default. + EXPECT_FALSE(statistics_proxy_->GetStats().suspended); + + // Verify that we can set it to true. + ViEEncoderObserver* encoder_observer = statistics_proxy_.get(); + encoder_observer->SuspendChange(0, true); + EXPECT_TRUE(statistics_proxy_->GetStats().suspended); + + // Verify that we can set it back to false again. + encoder_observer->SuspendChange(0, false); + EXPECT_FALSE(statistics_proxy_->GetStats().suspended); +} + TEST_F(SendStatisticsProxyTest, FrameCounts) { FrameCountObserver* observer = statistics_proxy_.get(); for (std::vector::const_iterator it = config_.rtp.ssrcs.begin(); diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index 41243e6404..6cc0f6862d 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -876,7 +876,7 @@ TEST_F(VideoSendStreamTest, SuspendBelowMinBitrate) { class RembObserver : public test::RtpRtcpObserver, public I420FrameCallback { public: - RembObserver() + RembObserver(VideoSendStream** send_stream_ptr) : RtpRtcpObserver(30 * 1000), // Timeout after 30 seconds. transport_adapter_(&transport_), clock_(Clock::GetRealTimeClock()), @@ -886,7 +886,8 @@ TEST_F(VideoSendStreamTest, SuspendBelowMinBitrate) { suspended_frame_count_(0), low_remb_bps_(0), high_remb_bps_(0), - crit_sect_(CriticalSectionWrapper::CreateCriticalSection()) { + crit_sect_(CriticalSectionWrapper::CreateCriticalSection()), + send_stream_ptr_(send_stream_ptr) { transport_adapter_.Enable(); } @@ -917,15 +918,14 @@ TEST_F(VideoSendStreamTest, SuspendBelowMinBitrate) { if (header.paddingLength == 0) { // Received non-padding packet during suspension period. Reset the // counter. - // TODO(hlundin): We should probably make this test more advanced in - // the future, so that it verifies that the bitrate can go below the - // min_bitrate. This requires that the fake encoder sees the - // min_bitrate, and never goes below it. See WebRTC Issue 2655. suspended_frame_count_ = 0; } } else if (test_state_ == kWaitingForPacket) { if (header.paddingLength == 0) { // Non-padding packet observed. Test is complete. + assert(*send_stream_ptr_); + VideoSendStream::Stats stats = (*send_stream_ptr_)->GetStats(); + EXPECT_FALSE(stats.suspended); observation_complete_->Set(); } } @@ -938,6 +938,9 @@ TEST_F(VideoSendStreamTest, SuspendBelowMinBitrate) { CriticalSectionScoped lock(crit_sect_.get()); if (test_state_ == kDuringSuspend && ++suspended_frame_count_ > kSuspendTimeFrames) { + assert(*send_stream_ptr_); + VideoSendStream::Stats stats = (*send_stream_ptr_)->GetStats(); + EXPECT_TRUE(stats.suspended); SendRtcpFeedback(high_remb_bps_); test_state_ = kWaitingForPacket; } @@ -983,7 +986,10 @@ TEST_F(VideoSendStreamTest, SuspendBelowMinBitrate) { int low_remb_bps_; int high_remb_bps_; scoped_ptr crit_sect_; - } observer; + VideoSendStream** send_stream_ptr_; + } observer(&send_stream_); + // Note that |send_stream_| is created in RunSendTest(), called below. This + // is why a pointer to |send_stream_| must be provided here. Call::Config call_config(observer.SendTransport()); scoped_ptr call(Call::Create(call_config)); diff --git a/webrtc/video_send_stream.h b/webrtc/video_send_stream.h index 5a4c89d8b5..8279c052a5 100644 --- a/webrtc/video_send_stream.h +++ b/webrtc/video_send_stream.h @@ -43,12 +43,14 @@ class VideoSendStream { : input_frame_rate(0), encode_frame_rate(0), avg_delay_ms(0), - max_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::string c_name; std::map substreams; };