generic-library/webrtc
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Avoid acquiring VCM::_receiveCritSect during decode callback.

Browse Source 
When VideoDecoder::Decode, Reset, or Release is called,
VideoCodingModuleImpl::_receiveCritSect may have been
acquired. Decode callback needs to acquire the same lock
in ViEChannel::FrameToRender. It is not a problem for
SW decode because decode callback is run on the same
WebRTC decoding thread and the lock is re-entrant. But
for HW decode, decode callback is run on a thread different
from WebRTC decoding thread. Decode callback gets the locks
in the opposite order. Deadlock can happen.

BUG=http://crbug.com/170345
TEST=Try apprtc.appspot.com/?debug=loopback on ARM Chromebook Daisy.
R=stefan@webrtc.org, wu@webrtc.org

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

git-svn-id: http://webrtc.googlecode.com/svn/trunk@4509 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
wuchengli@chromium.org 2013-08-09 04:08:38 +00:00
parent 64799da6c6
commit a717ee9962
2 changed files with 21 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
webrtc/video_engine/vie_channel.cc
View File

@ -1665,6 +1665,10 @@ CallStatsObserver* ViEChannel::GetStatsObserver() {
return stats_observer_.get(); return stats_observer_.get();
} }
// Do not acquire the lock of |vcm_| in this function. Decode callback won't
// necessarily be called from the decoding thread. The decoding thread may have
// held the lock when calling VideoDecoder::Decode, Reset, or Release. Acquiring
// the same lock in the path of decode callback can deadlock.
int32_t ViEChannel::FrameToRender( int32_t ViEChannel::FrameToRender(
I420VideoFrame& video_frame) { // NOLINT I420VideoFrame& video_frame) { // NOLINT
CriticalSectionScoped cs(callback_cs_.get()); CriticalSectionScoped cs(callback_cs_.get());
@ -1672,20 +1676,11 @@ int32_t ViEChannel::FrameToRender(
if (decoder_reset_) { if (decoder_reset_) {
// Trigger a callback to the user if the incoming codec has changed. // Trigger a callback to the user if the incoming codec has changed.
if (codec_observer_) { if (codec_observer_) {
VideoCodec decoder; // VCM::ReceiveCodec returns the codec set by RegisterReceiveCodec, which
memset(&decoder, 0, sizeof(decoder)); // might not be the size we're actually decoding.
if (vcm_.ReceiveCodec(&decoder) == VCM_OK) { receive_codec_.width = static_cast<uint16_t>(video_frame.width());
// VCM::ReceiveCodec returns the codec set by receive_codec_.height = static_cast<uint16_t>(video_frame.height());
// RegisterReceiveCodec, which might not be the size we're codec_observer_->IncomingCodecChanged(channel_id_, receive_codec_);
// actually decoding.
decoder.width = static_cast<uint16_t>(video_frame.width());
decoder.height = static_cast<uint16_t>(video_frame.height());
codec_observer_->IncomingCodecChanged(channel_id_, decoder);
} else {
assert(false);
WEBRTC_TRACE(kTraceInfo, kTraceVideo, ViEId(engine_id_, channel_id_),
"%s: Could not get receive codec", __FUNCTION__);
}
} }
decoder_reset_ = false; decoder_reset_ = false;
} }
@ -1949,9 +1944,17 @@ int32_t ViEChannel::OnInitializeDecoder(
payload_type, payload_name); payload_type, payload_name);
vcm_.ResetDecoder(); vcm_.ResetDecoder();
callback_cs_->Enter(); VideoCodec receive_codec;
decoder_reset_ = true; memset(&receive_codec, 0, sizeof(receive_codec));
callback_cs_->Leave(); if (vcm_.ReceiveCodec(&receive_codec) == VCM_OK) {
CriticalSectionScoped cs(callback_cs_.get());
decoder_reset_ = true;
receive_codec_ = receive_codec;
} else {
assert(false);
WEBRTC_TRACE(kTraceInfo, kTraceVideo, ViEId(engine_id_, channel_id_),
"%s: Could not get receive codec", __FUNCTION__);
}
return 0; return 0;
} }

1
webrtc/video_engine/vie_channel.h
View File

@ -383,6 +383,7 @@ class ViEChannel
Transport* external_transport_; Transport* external_transport_;
bool decoder_reset_; bool decoder_reset_;
VideoCodec receive_codec_;
bool wait_for_key_frame_; bool wait_for_key_frame_;
ThreadWrapper* decode_thread_; ThreadWrapper* decode_thread_;