Revert 5421 "Fix deadlock on register/unregister observer while ..."

Failure to compile on Chromium Internal bots, because of API changes.

http://chromegw.corp.google.com/i/internal.chromium.webrtc.fyi/builders/Mac/builds/2805/steps/compile/logs/stdio

You need to follow the steps mentioned in 
https://docs.google.com/a/google.com/document/d/1aHrmXECnu3-Jovc2-zYI267EaQCYz-IclYyBp9iA9Fc/edit that of a API changer.

Since I will be rolling the libjingle this week, I can push your changes along with libjingle roll, if you prepare the CLs
as mentioned in the doc.

> Fix deadlock on register/unregister observer while there is a an going callback.
> 
> BUG=2835
> R=mallinath@webrtc.org
> 
> Review URL: https://webrtc-codereview.appspot.com/7119005

TBR=andresp@webrtc.org

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

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

talk/media/webrtc/fakewebrtcvideocapturemodule.h

@@ -59,16 +59,21 @@ class FakeWebRtcVideoCaptureModule : public webrtc::VideoCaptureModule {
     id_ = id;
     return 0;
   }
-  virtual void RegisterCaptureDataCallback(
+  virtual int32_t RegisterCaptureDataCallback(
       webrtc::VideoCaptureDataCallback& callback) {
     callback_ = &callback;
+    return 0;
   }
-  virtual void DeRegisterCaptureDataCallback() { callback_ = NULL; }
+  virtual int32_t DeRegisterCaptureDataCallback() {
-  virtual void RegisterCaptureCallback(webrtc::VideoCaptureFeedBack& callback) {
+    callback_ = NULL;
-    // Not implemented.
+    return 0;
   }
-  virtual void DeRegisterCaptureCallback() {
+  virtual int32_t RegisterCaptureCallback(
-    // Not implemented.
+      webrtc::VideoCaptureFeedBack& callback) {
+    return -1;  // not implemented
+  }
+  virtual int32_t DeRegisterCaptureCallback() {
+    return 0;
   }
   virtual int32_t StartCapture(
       const webrtc::VideoCaptureCapability& cap) {
@@ -93,8 +98,13 @@ class FakeWebRtcVideoCaptureModule : public webrtc::VideoCaptureModule {
     settings = cap_;
     return 0;
   }
-  virtual void SetCaptureDelay(int32_t delay) { delay_ = delay; }
+  virtual int32_t SetCaptureDelay(int32_t delay) {
-  virtual int32_t CaptureDelay() { return delay_; }
+    delay_ = delay;
+    return 0;
+  }
+  virtual int32_t CaptureDelay() {
+    return delay_;
+  }
   virtual int32_t SetCaptureRotation(
       webrtc::VideoCaptureRotation rotation) {
     return -1;  // not implemented
@@ -103,11 +113,11 @@ class FakeWebRtcVideoCaptureModule : public webrtc::VideoCaptureModule {
       const webrtc::VideoCodec& codec) {
     return NULL;  // not implemented
   }
-  virtual void EnableFrameRateCallback(const bool enable) {
+  virtual int32_t EnableFrameRateCallback(const bool enable) {
-    // not implemented
+    return -1;  // not implemented
   }
-  virtual void EnableNoPictureAlarm(const bool enable) {
+  virtual int32_t EnableNoPictureAlarm(const bool enable) {
-    // not implemented
+    return -1;  // not implemented
   }
   virtual int32_t AddRef() {
     return 0;

talk/media/webrtc/webrtcvideocapturer.cc

@@ -264,8 +264,8 @@ CaptureState WebRtcVideoCapturer::Start(const VideoFormat& capture_format) {
 
   std::string camera_id(GetId());
   uint32 start = talk_base::Time();
-  module_->RegisterCaptureDataCallback(*this);
+  if (module_->RegisterCaptureDataCallback(*this) != 0 ||
-  if (module_->StartCapture(cap) != 0) {
+      module_->StartCapture(cap) != 0) {
     LOG(LS_ERROR) << "Camera '" << camera_id << "' failed to start";
     return CS_FAILED;
   }

webrtc/modules/video_capture/include/video_capture.h

@@ -105,17 +105,18 @@ class VideoCaptureModule: public RefCountedModule {
   };
 
   //   Register capture data callback
-  virtual void RegisterCaptureDataCallback(
+  virtual int32_t RegisterCaptureDataCallback(
       VideoCaptureDataCallback& dataCallback) = 0;
 
   //  Remove capture data callback
-  virtual void DeRegisterCaptureDataCallback() = 0;
+  virtual int32_t DeRegisterCaptureDataCallback() = 0;
 
   // Register capture callback.
-  virtual void RegisterCaptureCallback(VideoCaptureFeedBack& callBack) = 0;
+  virtual int32_t RegisterCaptureCallback(
+      VideoCaptureFeedBack& callBack) = 0;
 
   //  Remove capture callback.
-  virtual void DeRegisterCaptureCallback() = 0;
+  virtual int32_t DeRegisterCaptureCallback() = 0;
 
   // Start capture device
   virtual int32_t StartCapture(
@@ -132,7 +133,7 @@ class VideoCaptureModule: public RefCountedModule {
   // Gets the current configuration.
   virtual int32_t CaptureSettings(VideoCaptureCapability& settings) = 0;
 
-  virtual void SetCaptureDelay(int32_t delayMS) = 0;
+  virtual int32_t SetCaptureDelay(int32_t delayMS) = 0;
 
   // Returns the current CaptureDelay. Only valid when the camera is running.
   virtual int32_t CaptureDelay() = 0;
@@ -148,8 +149,8 @@ class VideoCaptureModule: public RefCountedModule {
   virtual VideoCaptureEncodeInterface* GetEncodeInterface(
       const VideoCodec& codec) = 0;
 
-  virtual void EnableFrameRateCallback(const bool enable) = 0;
+  virtual int32_t EnableFrameRateCallback(const bool enable) = 0;
-  virtual void EnableNoPictureAlarm(const bool enable) = 0;
+  virtual int32_t EnableNoPictureAlarm(const bool enable) = 0;
 
 protected:
   virtual ~VideoCaptureModule() {};

webrtc/modules/video_capture/test/video_capture_unittest.cc

@@ -252,7 +252,7 @@ class VideoCaptureTest : public testing::Test {
 
     EXPECT_FALSE(module->CaptureStarted());
 
-    module->RegisterCaptureDataCallback(*callback);
+    EXPECT_EQ(0, module->RegisterCaptureDataCallback(*callback));
     return module;
   }
 
@@ -408,10 +408,11 @@ class VideoCaptureExternalTest : public testing::Test {
     memset(test_frame_.buffer(webrtc::kVPlane), 127,
            ((kTestWidth + 1) / 2) * ((kTestHeight + 1) / 2));
 
-    capture_module_->RegisterCaptureDataCallback(capture_callback_);
+    EXPECT_EQ(0, capture_module_->RegisterCaptureDataCallback(
-    capture_module_->RegisterCaptureCallback(capture_feedback_);
+        capture_callback_));
-    capture_module_->EnableFrameRateCallback(true);
+    EXPECT_EQ(0, capture_module_->RegisterCaptureCallback(capture_feedback_));
-    capture_module_->EnableNoPictureAlarm(true);
+    EXPECT_EQ(0, capture_module_->EnableFrameRateCallback(true));
+    EXPECT_EQ(0, capture_module_->EnableNoPictureAlarm(true));
   }
 
   void TearDown() {

webrtc/modules/video_capture/video_capture_impl.cc

@@ -187,33 +187,45 @@ VideoCaptureImpl::~VideoCaptureImpl()
         delete[] _deviceUniqueId;
 }
 
-void VideoCaptureImpl::RegisterCaptureDataCallback(
+int32_t VideoCaptureImpl::RegisterCaptureDataCallback(
-    VideoCaptureDataCallback& dataCallBack) {
+                                        VideoCaptureDataCallback& dataCallBack)
+{
     CriticalSectionScoped cs(&_apiCs);
     CriticalSectionScoped cs2(&_callBackCs);
     _dataCallBack = &dataCallBack;
+
+    return 0;
 }
 
-void VideoCaptureImpl::DeRegisterCaptureDataCallback() {
+int32_t VideoCaptureImpl::DeRegisterCaptureDataCallback()
+{
     CriticalSectionScoped cs(&_apiCs);
     CriticalSectionScoped cs2(&_callBackCs);
     _dataCallBack = NULL;
+    return 0;
 }
-void VideoCaptureImpl::RegisterCaptureCallback(VideoCaptureFeedBack& callBack) {
+int32_t VideoCaptureImpl::RegisterCaptureCallback(VideoCaptureFeedBack& callBack)
+{
 
     CriticalSectionScoped cs(&_apiCs);
     CriticalSectionScoped cs2(&_callBackCs);
     _captureCallBack = &callBack;
+    return 0;
 }
-void VideoCaptureImpl::DeRegisterCaptureCallback() {
+int32_t VideoCaptureImpl::DeRegisterCaptureCallback()
+{
 
     CriticalSectionScoped cs(&_apiCs);
     CriticalSectionScoped cs2(&_callBackCs);
     _captureCallBack = NULL;
+    return 0;
+
 }
-void VideoCaptureImpl::SetCaptureDelay(int32_t delayMS) {
+int32_t VideoCaptureImpl::SetCaptureDelay(int32_t delayMS)
+{
     CriticalSectionScoped cs(&_apiCs);
     _captureDelay = delayMS;
+    return 0;
 }
 int32_t VideoCaptureImpl::CaptureDelay()
 {
@@ -377,7 +389,8 @@ int32_t VideoCaptureImpl::SetCaptureRotation(VideoCaptureRotation rotation) {
   return 0;
 }
 
-void VideoCaptureImpl::EnableFrameRateCallback(const bool enable) {
+int32_t VideoCaptureImpl::EnableFrameRateCallback(const bool enable)
+{
     CriticalSectionScoped cs(&_apiCs);
     CriticalSectionScoped cs2(&_callBackCs);
     _frameRateCallBack = enable;
@@ -385,12 +398,15 @@ void VideoCaptureImpl::EnableFrameRateCallback(const bool enable) {
     {
         _lastFrameRateCallbackTime = TickTime::Now();
     }
+    return 0;
 }
 
-void VideoCaptureImpl::EnableNoPictureAlarm(const bool enable) {
+int32_t VideoCaptureImpl::EnableNoPictureAlarm(const bool enable)
+{
     CriticalSectionScoped cs(&_apiCs);
     CriticalSectionScoped cs2(&_callBackCs);
     _noPictureAlarmCallBack = enable;
+    return 0;
 }
 
 void VideoCaptureImpl::UpdateFrameCount()

webrtc/modules/video_capture/video_capture_impl.h

@@ -62,18 +62,17 @@ public:
     virtual int32_t ChangeUniqueId(const int32_t id);
 
     //Call backs
-    virtual void RegisterCaptureDataCallback(
+    virtual int32_t RegisterCaptureDataCallback(VideoCaptureDataCallback& dataCallback);
-        VideoCaptureDataCallback& dataCallback);
+    virtual int32_t DeRegisterCaptureDataCallback();
-    virtual void DeRegisterCaptureDataCallback();
+    virtual int32_t RegisterCaptureCallback(VideoCaptureFeedBack& callBack);
-    virtual void RegisterCaptureCallback(VideoCaptureFeedBack& callBack);
+    virtual int32_t DeRegisterCaptureCallback();
-    virtual void DeRegisterCaptureCallback();
 
-    virtual void SetCaptureDelay(int32_t delayMS);
+    virtual int32_t SetCaptureDelay(int32_t delayMS);
     virtual int32_t CaptureDelay();
     virtual int32_t SetCaptureRotation(VideoCaptureRotation rotation);
 
-    virtual void EnableFrameRateCallback(const bool enable);
+    virtual int32_t EnableFrameRateCallback(const bool enable);
-    virtual void EnableNoPictureAlarm(const bool enable);
+    virtual int32_t EnableNoPictureAlarm(const bool enable);
 
     virtual const char* CurrentDeviceName() const;
 

webrtc/video_engine/vie_capturer.cc

@@ -279,8 +279,7 @@ void ViECapturer::CpuOveruseMeasures(int* capture_jitter_ms,
 }
 
 int32_t ViECapturer::SetCaptureDelay(int32_t delay_ms) {
-  capture_module_->SetCaptureDelay(delay_ms);
+  return capture_module_->SetCaptureDelay(delay_ms);
-  return 0;
 }
 
 int32_t ViECapturer::SetRotateCapturedFrames(
@@ -639,31 +638,29 @@ bool ViECapturer::CaptureCapabilityFixed() {
 }
 
 int32_t ViECapturer::RegisterObserver(ViECaptureObserver* observer) {
-  {
   CriticalSectionScoped cs(observer_cs_.get());
   if (observer_) {
-      WEBRTC_TRACE(kTraceError,
+    WEBRTC_TRACE(kTraceError, kTraceVideo, ViEId(engine_id_, capture_id_),
-                   kTraceVideo,
+                 "%s Observer already registered", __FUNCTION__, capture_id_);
-                   ViEId(engine_id_, capture_id_),
-                   "%s Observer already registered",
-                   __FUNCTION__,
-                   capture_id_);
     return -1;
   }
-    observer_ = observer;
+  if (capture_module_->RegisterCaptureCallback(*this) != 0) {
+    return -1;
   }
-  capture_module_->RegisterCaptureCallback(*this);
   capture_module_->EnableFrameRateCallback(true);
   capture_module_->EnableNoPictureAlarm(true);
+  observer_ = observer;
   return 0;
 }
 
 int32_t ViECapturer::DeRegisterObserver() {
+  CriticalSectionScoped cs(observer_cs_.get());
+  if (!observer_) {
+    return 0;
+  }
   capture_module_->EnableFrameRateCallback(false);
   capture_module_->EnableNoPictureAlarm(false);
   capture_module_->DeRegisterCaptureCallback();
-
-  CriticalSectionScoped cs(observer_cs_.get());
   observer_ = NULL;
   return 0;
 }

webrtc/video_engine/vie_capturer.h

@@ -20,7 +20,6 @@
 #include "webrtc/modules/video_coding/main/interface/video_coding.h"
 #include "webrtc/modules/video_processing/main/interface/video_processing.h"
 #include "webrtc/system_wrappers/interface/scoped_ptr.h"
-#include "webrtc/system_wrappers/interface/thread_annotations.h"
 #include "webrtc/typedefs.h"
 #include "webrtc/video_engine/include/vie_capture.h"
 #include "webrtc/video_engine/vie_defines.h"
@@ -186,7 +185,7 @@ class ViECapturer
 
   // Statistics observer.
   scoped_ptr<CriticalSectionWrapper> observer_cs_;
-  ViECaptureObserver* observer_ GUARDED_BY(observer_cs_.get());
+  ViECaptureObserver* observer_;
 
   CaptureCapability requested_capability_;