Make MediaOptimization thread-safe.

HW encoder posts the encode callback to libjingle worker
thread. It accesses MediaOptimization and is not protected
by the critial section of VideoSender. Make MediaOptimization
thread-safe to fix it.

BUG=chromium:367691
TEST=Run apprtc loopback with SW or HW encoders.
     Run module_unittests.

R=stefan@webrtc.org

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

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

2 files changed, 137 insertions, 69 deletions

diff --git a/modules/video_coding/main/source/media_optimization.cc b/modules/video_coding/main/source/media_optimization.cc
index 4dc72253..0d9a4bdf 100644
--- a/modules/video_coding/main/source/media_optimization.cc
+++ b/modules/video_coding/main/source/media_optimization.cc
@@ -75,7 +75,8 @@ struct MediaOptimization::EncodedFrameSample {
 };
 
 MediaOptimization::MediaOptimization(Clock* clock)
-    : clock_(clock),
+    : crit_sect_(CriticalSectionWrapper::CreateCriticalSection()),
+      clock_(clock),
       max_bit_rate_(0),
       send_codec_type_(kVideoCodecUnknown),
       codec_width_(0),
@@ -113,7 +114,9 @@ MediaOptimization::~MediaOptimization(void) {
 }
 
 void MediaOptimization::Reset() {
-  SetEncodingData(kVideoCodecUnknown, 0, 0, 0, 0, 0, 0, max_payload_size_);
+  CriticalSectionScoped lock(crit_sect_.get());
+  SetEncodingDataInternal(
+      kVideoCodecUnknown, 0, 0, 0, 0, 0, 0, max_payload_size_);
   memset(incoming_frame_times_, -1, sizeof(incoming_frame_times_));
   incoming_frame_rate_ = 0.0;
   frame_dropper_->Reset();
@@ -145,6 +148,25 @@ void MediaOptimization::SetEncodingData(VideoCodecType send_codec_type,
                                         uint16_t height,
                                         int num_layers,
                                         int32_t mtu) {
+  CriticalSectionScoped lock(crit_sect_.get());
+  SetEncodingDataInternal(send_codec_type,
+                          max_bit_rate,
+                          frame_rate,
+                          target_bitrate,
+                          width,
+                          height,
+                          num_layers,
+                          mtu);
+}
+
+void MediaOptimization::SetEncodingDataInternal(VideoCodecType send_codec_type,
+                                                int32_t max_bit_rate,
+                                                uint32_t frame_rate,
+                                                uint32_t target_bitrate,
+                                                uint16_t width,
+                                                uint16_t height,
+                                                int num_layers,
+                                                int32_t mtu) {
   // Everything codec specific should be reset here since this means the codec
   // has changed. If native dimension values have changed, then either user
   // initiated change, or QM initiated change. Will be able to determine only
@@ -181,6 +203,7 @@ uint32_t MediaOptimization::SetTargetRates(
     uint32_t round_trip_time_ms,
     VCMProtectionCallback* protection_callback,
     VCMQMSettingsCallback* qmsettings_callback) {
+  CriticalSectionScoped lock(crit_sect_.get());
   // TODO(holmer): Consider putting this threshold only on the video bitrate,
   // and not on protection.
   if (max_bit_rate_ > 0 &&
@@ -194,7 +217,7 @@ uint32_t MediaOptimization::SetTargetRates(
   loss_prot_logic_->UpdateResidualPacketLoss(static_cast<float>(fraction_lost));
 
   // Get frame rate for encoder: this is the actual/sent frame rate.
-  float actual_frame_rate = SentFrameRate();
+  float actual_frame_rate = SentFrameRateInternal();
 
   // Sanity check.
   if (actual_frame_rate < 1.0) {
@@ -297,6 +320,7 @@ uint32_t MediaOptimization::SetTargetRates(
 
 void MediaOptimization::EnableProtectionMethod(bool enable,
                                                VCMProtectionMethodEnum method) {
+  CriticalSectionScoped lock(crit_sect_.get());
   bool updated = false;
   if (enable) {
     updated = loss_prot_logic_->SetMethod(method);
@@ -309,17 +333,28 @@ void MediaOptimization::EnableProtectionMethod(bool enable,
 }
 
 uint32_t MediaOptimization::InputFrameRate() {
+  CriticalSectionScoped lock(crit_sect_.get());
+  return InputFrameRateInternal();
+}
+
+uint32_t MediaOptimization::InputFrameRateInternal() {
   ProcessIncomingFrameRate(clock_->TimeInMilliseconds());
   return uint32_t(incoming_frame_rate_ + 0.5f);
 }
 
 uint32_t MediaOptimization::SentFrameRate() {
+  CriticalSectionScoped lock(crit_sect_.get());
+  return SentFrameRateInternal();
+}
+
+uint32_t MediaOptimization::SentFrameRateInternal() {
   PurgeOldFrameSamples(clock_->TimeInMilliseconds());
   UpdateSentFramerate();
   return avg_sent_framerate_;
 }
 
 uint32_t MediaOptimization::SentBitRate() {
+  CriticalSectionScoped lock(crit_sect_.get());
   const int64_t now_ms = clock_->TimeInMilliseconds();
   PurgeOldFrameSamples(now_ms);
   UpdateSentBitrate(now_ms);
@@ -327,6 +362,7 @@ uint32_t MediaOptimization::SentBitRate() {
 }
 
 VCMFrameCount MediaOptimization::SentFrameCount() {
+  CriticalSectionScoped lock(crit_sect_.get());
   VCMFrameCount count;
   count.numDeltaFrames = delta_frame_cnt_;
   count.numKeyFrames = key_frame_cnt_;
@@ -336,6 +372,7 @@ VCMFrameCount MediaOptimization::SentFrameCount() {
 int32_t MediaOptimization::UpdateWithEncodedData(int encoded_length,
                                                  uint32_t timestamp,
                                                  FrameType encoded_frame_type) {
+  CriticalSectionScoped lock(crit_sect_.get());
   const int64_t now_ms = clock_->TimeInMilliseconds();
   PurgeOldFrameSamples(now_ms);
   if (encoded_frame_samples_.size() > 0 &&
@@ -386,22 +423,55 @@ int32_t MediaOptimization::UpdateWithEncodedData(int encoded_length,
   return VCM_OK;
 }
 
-void MediaOptimization::EnableQM(bool enable) { enable_qm_ = enable; }
+void MediaOptimization::EnableQM(bool enable) {
+  CriticalSectionScoped lock(crit_sect_.get());
+  enable_qm_ = enable;
+}
 
 void MediaOptimization::EnableFrameDropper(bool enable) {
+  CriticalSectionScoped lock(crit_sect_.get());
   frame_dropper_->Enable(enable);
 }
 
+void MediaOptimization::SuspendBelowMinBitrate(int threshold_bps,
+                                               int window_bps) {
+  CriticalSectionScoped lock(crit_sect_.get());
+  assert(threshold_bps > 0 && window_bps >= 0);
+  suspension_threshold_bps_ = threshold_bps;
+  suspension_window_bps_ = window_bps;
+  suspension_enabled_ = true;
+  video_suspended_ = false;
+}
+
+bool MediaOptimization::IsVideoSuspended() const {
+  CriticalSectionScoped lock(crit_sect_.get());
+  return video_suspended_;
+}
+
 bool MediaOptimization::DropFrame() {
+  CriticalSectionScoped lock(crit_sect_.get());
   UpdateIncomingFrameRate();
   // Leak appropriate number of bytes.
-  frame_dropper_->Leak((uint32_t)(InputFrameRate() + 0.5f));
+  frame_dropper_->Leak((uint32_t)(InputFrameRateInternal() + 0.5f));
   if (video_suspended_) {
     return true;  // Drop all frames when muted.
   }
   return frame_dropper_->DropFrame();
 }
 
+void MediaOptimization::UpdateContentData(
+    const VideoContentMetrics* content_metrics) {
+  CriticalSectionScoped lock(crit_sect_.get());
+  // Updating content metrics.
+  if (content_metrics == NULL) {
+    // Disable QM if metrics are NULL.
+    enable_qm_ = false;
+    qm_resolution_->Reset();
+  } else {
+    content_->UpdateContentData(content_metrics);
+  }
+}
+
 void MediaOptimization::UpdateIncomingFrameRate() {
   int64_t now = clock_->TimeInMilliseconds();
   if (incoming_frame_times_[0] == 0) {
@@ -416,18 +486,6 @@ void MediaOptimization::UpdateIncomingFrameRate() {
   ProcessIncomingFrameRate(now);
 }
 
-void MediaOptimization::UpdateContentData(
-    const VideoContentMetrics* content_metrics) {
-  // Updating content metrics.
-  if (content_metrics == NULL) {
-    // Disable QM if metrics are NULL.
-    enable_qm_ = false;
-    qm_resolution_->Reset();
-  } else {
-    content_->UpdateContentData(content_metrics);
-  }
-}
-
 int32_t MediaOptimization::SelectQuality(
     VCMQMSettingsCallback* video_qmsettings_callback) {
   // Reset quantities for QM select.
@@ -458,17 +516,6 @@ int32_t MediaOptimization::SelectQuality(
   return VCM_OK;
 }
 
-void MediaOptimization::SuspendBelowMinBitrate(int threshold_bps,
-                                               int window_bps) {
-  assert(threshold_bps > 0 && window_bps >= 0);
-  suspension_threshold_bps_ = threshold_bps;
-  suspension_window_bps_ = window_bps;
-  suspension_enabled_ = true;
-  video_suspended_ = false;
-}
-
-bool MediaOptimization::IsVideoSuspended() const { return video_suspended_; }
-
 void MediaOptimization::PurgeOldFrameSamples(int64_t now_ms) {
   while (!encoded_frame_samples_.empty()) {
     if (now_ms - encoded_frame_samples_.front().time_complete_ms >
diff --git a/modules/video_coding/main/source/media_optimization.h b/modules/video_coding/main/source/media_optimization.h
index 35a49712..df3fbb64 100644
--- a/modules/video_coding/main/source/media_optimization.h
+++ b/modules/video_coding/main/source/media_optimization.h
@@ -17,6 +17,7 @@
 #include "webrtc/modules/video_coding/main/interface/video_coding.h"
 #include "webrtc/modules/video_coding/main/source/media_opt_util.h"
 #include "webrtc/modules/video_coding/main/source/qm_select.h"
+#include "webrtc/system_wrappers/interface/critical_section_wrapper.h"
 #include "webrtc/system_wrappers/interface/scoped_ptr.h"
 
 namespace webrtc {
@@ -28,7 +29,6 @@ class VCMContentMetricsProcessing;
 
 namespace media_optimization {
 
-// TODO(andresp): Make thread safe.
 class MediaOptimization {
  public:
   explicit MediaOptimization(Clock* clock);
@@ -100,59 +100,80 @@ class MediaOptimization {
   struct EncodedFrameSample;
   typedef std::list<EncodedFrameSample> FrameSampleList;
 
-  void UpdateIncomingFrameRate();
-  void PurgeOldFrameSamples(int64_t now_ms);
-  void UpdateSentBitrate(int64_t now_ms);
-  void UpdateSentFramerate();
+  void UpdateIncomingFrameRate() EXCLUSIVE_LOCKS_REQUIRED(crit_sect_);
+  void PurgeOldFrameSamples(int64_t now_ms)
+      EXCLUSIVE_LOCKS_REQUIRED(crit_sect_);
+  void UpdateSentBitrate(int64_t now_ms) EXCLUSIVE_LOCKS_REQUIRED(crit_sect_);
+  void UpdateSentFramerate() EXCLUSIVE_LOCKS_REQUIRED(crit_sect_);
 
   // Computes new Quality Mode.
-  int32_t SelectQuality(VCMQMSettingsCallback* qmsettings_callback);
+  int32_t SelectQuality(VCMQMSettingsCallback* qmsettings_callback)
+      EXCLUSIVE_LOCKS_REQUIRED(crit_sect_);
 
   // Verifies if QM settings differ from default, i.e. if an update is required.
   // Computes actual values, as will be sent to the encoder.
   bool QMUpdate(VCMResolutionScale* qm,
-                VCMQMSettingsCallback* qmsettings_callback);
+                VCMQMSettingsCallback* qmsettings_callback)
+      EXCLUSIVE_LOCKS_REQUIRED(crit_sect_);
 
   // Checks if we should make a QM change. Return true if yes, false otherwise.
-  bool CheckStatusForQMchange();
+  bool CheckStatusForQMchange() EXCLUSIVE_LOCKS_REQUIRED(crit_sect_);
 
-  void ProcessIncomingFrameRate(int64_t now);
+  void ProcessIncomingFrameRate(int64_t now)
+      EXCLUSIVE_LOCKS_REQUIRED(crit_sect_);
 
   // Checks conditions for suspending the video. The method compares
   // |target_bit_rate_| with the threshold values for suspension, and changes
   // the state of |video_suspended_| accordingly.
-  void CheckSuspendConditions();
-
-  Clock* clock_;
-  int32_t max_bit_rate_;
-  VideoCodecType send_codec_type_;
-  uint16_t codec_width_;
-  uint16_t codec_height_;
-  float user_frame_rate_;
-  scoped_ptr<FrameDropper> frame_dropper_;
-  scoped_ptr<VCMLossProtectionLogic> loss_prot_logic_;
-  uint8_t fraction_lost_;
-  uint32_t send_statistics_[4];
-  uint32_t send_statistics_zero_encode_;
-  int32_t max_payload_size_;
-  int target_bit_rate_;
-  float incoming_frame_rate_;
-  int64_t incoming_frame_times_[kFrameCountHistorySize];
-  bool enable_qm_;
-  std::list<EncodedFrameSample> encoded_frame_samples_;
-  uint32_t avg_sent_bit_rate_bps_;
-  uint32_t avg_sent_framerate_;
-  uint32_t key_frame_cnt_;
-  uint32_t delta_frame_cnt_;
-  scoped_ptr<VCMContentMetricsProcessing> content_;
-  scoped_ptr<VCMQmResolution> qm_resolution_;
-  int64_t last_qm_update_time_;
-  int64_t last_change_time_;  // Content/user triggered.
-  int num_layers_;
-  bool suspension_enabled_;
-  bool video_suspended_;
-  int suspension_threshold_bps_;
-  int suspension_window_bps_;
+  void CheckSuspendConditions() EXCLUSIVE_LOCKS_REQUIRED(crit_sect_);
+
+  void SetEncodingDataInternal(VideoCodecType send_codec_type,
+                               int32_t max_bit_rate,
+                               uint32_t frame_rate,
+                               uint32_t bit_rate,
+                               uint16_t width,
+                               uint16_t height,
+                               int num_temporal_layers,
+                               int32_t mtu)
+      EXCLUSIVE_LOCKS_REQUIRED(crit_sect_);
+
+  uint32_t InputFrameRateInternal() EXCLUSIVE_LOCKS_REQUIRED(crit_sect_);
+
+  uint32_t SentFrameRateInternal() EXCLUSIVE_LOCKS_REQUIRED(crit_sect_);
+
+  // Protect all members.
+  scoped_ptr<CriticalSectionWrapper> crit_sect_;
+
+  Clock* clock_ GUARDED_BY(crit_sect_);
+  int32_t max_bit_rate_ GUARDED_BY(crit_sect_);
+  VideoCodecType send_codec_type_ GUARDED_BY(crit_sect_);
+  uint16_t codec_width_ GUARDED_BY(crit_sect_);
+  uint16_t codec_height_ GUARDED_BY(crit_sect_);
+  float user_frame_rate_ GUARDED_BY(crit_sect_);
+  scoped_ptr<FrameDropper> frame_dropper_ GUARDED_BY(crit_sect_);
+  scoped_ptr<VCMLossProtectionLogic> loss_prot_logic_ GUARDED_BY(crit_sect_);
+  uint8_t fraction_lost_ GUARDED_BY(crit_sect_);
+  uint32_t send_statistics_[4] GUARDED_BY(crit_sect_);
+  uint32_t send_statistics_zero_encode_ GUARDED_BY(crit_sect_);
+  int32_t max_payload_size_ GUARDED_BY(crit_sect_);
+  int target_bit_rate_ GUARDED_BY(crit_sect_);
+  float incoming_frame_rate_ GUARDED_BY(crit_sect_);
+  int64_t incoming_frame_times_[kFrameCountHistorySize] GUARDED_BY(crit_sect_);
+  bool enable_qm_ GUARDED_BY(crit_sect_);
+  std::list<EncodedFrameSample> encoded_frame_samples_ GUARDED_BY(crit_sect_);
+  uint32_t avg_sent_bit_rate_bps_ GUARDED_BY(crit_sect_);
+  uint32_t avg_sent_framerate_ GUARDED_BY(crit_sect_);
+  uint32_t key_frame_cnt_ GUARDED_BY(crit_sect_);
+  uint32_t delta_frame_cnt_ GUARDED_BY(crit_sect_);
+  scoped_ptr<VCMContentMetricsProcessing> content_ GUARDED_BY(crit_sect_);
+  scoped_ptr<VCMQmResolution> qm_resolution_ GUARDED_BY(crit_sect_);
+  int64_t last_qm_update_time_ GUARDED_BY(crit_sect_);
+  int64_t last_change_time_ GUARDED_BY(crit_sect_);  // Content/user triggered.
+  int num_layers_ GUARDED_BY(crit_sect_);
+  bool suspension_enabled_ GUARDED_BY(crit_sect_);
+  bool video_suspended_ GUARDED_BY(crit_sect_);
+  int suspension_threshold_bps_ GUARDED_BY(crit_sect_);
+  int suspension_window_bps_ GUARDED_BY(crit_sect_);
 };
 }  // namespace media_optimization
 }  // namespace webrtc