Revert "Routing SuspendChange to VideoSendStream::Stats"

The test VideoSendStreamTest.SuspendBelowMinBitrate seems flaky.
Reverting and investigating.

BUG=3040

Review URL: https://webrtc-codereview.appspot.com/9799004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@5681 4adac7df-926f-26a2-2b94-8c16560cd09d

webrtc/video/rampup_tests.cc

@@ -217,9 +217,7 @@ 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();
@@ -240,10 +238,6 @@ 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<unsigned int>& ssrcs,
                                        unsigned int bitrate) {
     CriticalSectionScoped lock(critical_section_.get());
@@ -284,7 +278,6 @@ class LowRateStreamObserver : public test::DirectTransport,
     if (remote_bitrate_estimator_->TimeUntilNextProcess() <= 0) {
       remote_bitrate_estimator_->Process();
     }
-    suspended_in_stats_ = send_stream_->GetStats().suspended;
     return true;
   }
 
@@ -310,11 +303,8 @@ 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.
@@ -335,7 +325,7 @@ class LowRateStreamObserver : public test::DirectTransport,
         break;
       }
       case kLowRate: {
-        if (bitrate_bps < kExpectedLowBitrateBps && suspended_in_stats_) {
+        if (bitrate_bps < kExpectedLowBitrateBps) {
           // 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 =
@@ -355,7 +345,7 @@ class LowRateStreamObserver : public test::DirectTransport,
         break;
       }
       case kSecondRampup: {
-        if (bitrate_bps > kExpectedHighBitrateBps && !suspended_in_stats_) {
+        if (bitrate_bps > kExpectedHighBitrateBps) {
           webrtc::test::PrintResult("ramp_up_down_up",
                                     GetModifierString(),
                                     "second_rampup",
@@ -401,8 +391,6 @@ 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_);
 };
 }
 
@@ -508,7 +496,6 @@ 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<test::FrameGeneratorCapturer> frame_generator_capturer(
         test::FrameGeneratorCapturer::Create(send_stream->Input(),

webrtc/video/send_statistics_proxy.cc

@@ -32,11 +32,6 @@ 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());

webrtc/video/send_statistics_proxy.h

@@ -18,7 +18,6 @@
 #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 {
 
@@ -68,7 +67,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,
@@ -81,12 +80,12 @@ class SendStatisticsProxy : public RtcpStatisticsCallback,
                               const CaptureAlarm alarm) OVERRIDE {}
 
  private:
-  StreamStats* GetStatsEntry(uint32_t ssrc) EXCLUSIVE_LOCKS_REQUIRED(lock_);
+  StreamStats* GetStatsEntry(uint32_t ssrc);
 
   const VideoSendStream::Config config_;
   scoped_ptr<CriticalSectionWrapper> lock_;
-  VideoSendStream::Stats stats_ GUARDED_BY(lock_);
+  VideoSendStream::Stats stats_;
-  StatsProvider* const stats_provider_;
+  StatsProvider* stats_provider_;
 };
 
 }  // namespace webrtc

webrtc/video/send_statistics_proxy_unittest.cc

@@ -54,7 +54,6 @@ 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());
@@ -132,20 +131,6 @@ 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<uint32_t>::const_iterator it = config_.rtp.ssrcs.begin();

webrtc/video/video_send_stream_tests.cc

@@ -876,7 +876,7 @@ TEST_F(VideoSendStreamTest, SuspendBelowMinBitrate) {
 
   class RembObserver : public test::RtpRtcpObserver, public I420FrameCallback {
    public:
-    RembObserver(VideoSendStream** send_stream_ptr)
+    RembObserver()
         : RtpRtcpObserver(30 * 1000),  // Timeout after 30 seconds.
           transport_adapter_(&transport_),
           clock_(Clock::GetRealTimeClock()),
@@ -886,8 +886,7 @@ 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();
     }
 
@@ -915,9 +914,6 @@ TEST_F(VideoSendStreamTest, SuspendBelowMinBitrate) {
         SendRtcpFeedback(low_remb_bps_);
         test_state_ = kDuringSuspend;
       } else if (test_state_ == kDuringSuspend) {
-        assert(*send_stream_ptr_);
-        VideoSendStream::Stats stats = (*send_stream_ptr_)->GetStats();
-        EXPECT_TRUE(stats.suspended);
         if (header.paddingLength == 0) {
           // Received non-padding packet during suspension period. Reset the
           // counter.
@@ -930,9 +926,6 @@ TEST_F(VideoSendStreamTest, SuspendBelowMinBitrate) {
       } 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();
         }
       }
@@ -990,10 +983,7 @@ TEST_F(VideoSendStreamTest, SuspendBelowMinBitrate) {
     int low_remb_bps_;
     int high_remb_bps_;
     scoped_ptr<CriticalSectionWrapper> crit_sect_;
-    VideoSendStream** send_stream_ptr_;
+  } observer;
-  } 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(Call::Create(call_config));

webrtc/video_send_stream.h

@@ -43,14 +43,12 @@ 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<uint32_t, StreamStats> substreams;
   };