Revert 5444 "Revert 5421 "Fix deadlock on register/unregister ob..."

We reverted the r5421 to allow us roll webrtc to chrome without any modifications
to libjingle. Since webrtc is rolled with r5444, we can add back the original CL
and changes to libjingle will be upstreamed in the next roll.

TBR=andresp@webrtc.org

> 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

TBR=mallinath@webrtc.org

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

git-svn-id: http://webrtc.googlecode.com/svn/trunk@5453 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
mallinath@webrtc.org 2014-01-29 00:56:02 +00:00
parent 84eb0e952e
commit 7433a088d2
8 changed files with 61 additions and 84 deletions

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

@ -20,6 +20,7 @@
#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"
@ -185,7 +186,7 @@ class ViECapturer
// Statistics observer.
scoped_ptr<CriticalSectionWrapper> observer_cs_;
ViECaptureObserver* observer_;
ViECaptureObserver* observer_ GUARDED_BY(observer_cs_.get());
CaptureCapability requested_capability_;