From e001b57d8413079581e6cfb12ac1a1a697149649 Mon Sep 17 00:00:00 2001 From: "fischman@webrtc.org" Date: Tue, 4 Jun 2013 03:29:37 +0000 Subject: [PATCH] Do not hold a lock when calling VCMReceiveCallback::FrameToRender. DecodedImageCallback is allowed to be called on a thread different from decoding thread. To avoid the deadlock in VCMDecodedFrameCallback::Decoded, VCMDecodedFrameCallback::_critSect should not be held while calling VCMReceiveCallback::FrameToRender. BUG=1832 R=stefan@webrtc.org Review URL: https://webrtc-codereview.appspot.com/1570004 Patch from Wu-Cheng Li . git-svn-id: http://webrtc.googlecode.com/svn/trunk@4162 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../main/source/generic_decoder.cc | 18 +++++++++++------- .../video_coding/main/source/generic_decoder.h | 9 ++++++--- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/webrtc/modules/video_coding/main/source/generic_decoder.cc b/webrtc/modules/video_coding/main/source/generic_decoder.cc index 1b74d229d..f90d7f933 100644 --- a/webrtc/modules/video_coding/main/source/generic_decoder.cc +++ b/webrtc/modules/video_coding/main/source/generic_decoder.cc @@ -45,9 +45,14 @@ int32_t VCMDecodedFrameCallback::Decoded(I420VideoFrame& decodedImage) { // TODO(holmer): We should improve this so that we can handle multiple // callbacks from one call to Decode(). - CriticalSectionScoped cs(_critSect); - VCMFrameInformation* frameInfo = static_cast( - _timestampMap.Pop(decodedImage.timestamp())); + VCMFrameInformation* frameInfo; + VCMReceiveCallback* callback; + { + CriticalSectionScoped cs(_critSect); + frameInfo = static_cast( + _timestampMap.Pop(decodedImage.timestamp())); + callback = _receiveCallback; + } if (frameInfo == NULL) { // The map should never be empty or full if this callback is called. @@ -59,11 +64,10 @@ int32_t VCMDecodedFrameCallback::Decoded(I420VideoFrame& decodedImage) frameInfo->decodeStartTimeMs, _clock->TimeInMilliseconds()); - if (_receiveCallback != NULL) + if (callback != NULL) { - _frame.SwapFrame(&decodedImage); - _frame.set_render_time_ms(frameInfo->renderTimeMs); - int32_t callbackReturn = _receiveCallback->FrameToRender(_frame); + decodedImage.set_render_time_ms(frameInfo->renderTimeMs); + int32_t callbackReturn = callback->FrameToRender(decodedImage); if (callbackReturn < 0) { WEBRTC_TRACE(webrtc::kTraceDebug, diff --git a/webrtc/modules/video_coding/main/source/generic_decoder.h b/webrtc/modules/video_coding/main/source/generic_decoder.h index 755adc792..21cb4c1dc 100644 --- a/webrtc/modules/video_coding/main/source/generic_decoder.h +++ b/webrtc/modules/video_coding/main/source/generic_decoder.h @@ -48,12 +48,12 @@ public: int32_t Pop(uint32_t timestamp); private: + // Protect |_receiveCallback| and |_timestampMap|. CriticalSectionWrapper* _critSect; Clock* _clock; - I420VideoFrame _frame; - VCMReceiveCallback* _receiveCallback; + VCMReceiveCallback* _receiveCallback; // Guarded by |_critSect|. VCMTiming& _timing; - VCMTimestampMap _timestampMap; + VCMTimestampMap _timestampMap; // Guarded by |_critSect|. uint64_t _lastReceivedPictureID; }; @@ -98,6 +98,9 @@ public: int32_t SetCodecConfigParameters(const uint8_t* /*buffer*/, int32_t /*size*/); + /** + * Set decode callback. Deregistering while decoding is illegal. + */ int32_t RegisterDecodeCompleteCallback(VCMDecodedFrameCallback* callback); bool External() const;