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

Fix shutdown race for ViEEncoder when there is a frame in the encoder.

Browse Source 
There is a potential race when deleting a channel and there is a frame
in the encoder. ViEEncoder::SendData can be called after
ViEEncoder::StopThreadsAndRemovePayloadRouter and payload_router is
then already removed.

Until we have the new API in place, use scoped_refptr in ViEChannel and
ViEEncoder and deregister channel/encoder before deleting.

BUG=769
R=stefan@webrtc.org

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

Cr-Commit-Position: refs/heads/master@{#8443}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8443 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
mflodman@webrtc.org 2015-02-20 12:45:40 +00:00
parent dc77d7447e
commit 7ac374abd7
6 changed files with 17 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
webrtc/video_engine/payload_router.h
View File

@ -17,6 +17,7 @@
#include "webrtc/base/constructormagic.h"
#include "webrtc/base/thread_annotations.h"
#include "webrtc/common_types.h"
#include "webrtc/system_wrappers/interface/atomic32.h"
#include "webrtc/system_wrappers/interface/scoped_ptr.h"
namespace webrtc {
@ -68,6 +69,9 @@ class PayloadRouter {
// and RTP headers.
size_t MaxPayloadLength() const;
void AddRef() { ++ref_count_; }
void Release() { if (--ref_count_ == 0) { delete this; } }
private:
// TODO(mflodman): When the new video API has launched, remove crit_ and
// assume rtp_modules_ will never change during a call.
@ -77,6 +81,8 @@ class PayloadRouter {
std::vector<RtpRtcp*> rtp_modules_ GUARDED_BY(crit_.get());
bool active_ GUARDED_BY(crit_.get());
Atomic32 ref_count_;
DISALLOW_COPY_AND_ASSIGN(PayloadRouter);
};

4
webrtc/video_engine/vie_channel.cc
View File

@ -1509,8 +1509,8 @@ RtpRtcp* ViEChannel::rtp_rtcp() {
return rtp_rtcp_.get();
}
PayloadRouter* ViEChannel::send_payload_router() {
return send_payload_router_.get();
scoped_refptr<PayloadRouter> ViEChannel::send_payload_router() {
return send_payload_router_;
}
CallStatsObserver* ViEChannel::GetStatsObserver() {

5
webrtc/video_engine/vie_channel.h
View File

@ -19,6 +19,7 @@
#include "webrtc/modules/video_coding/main/interface/video_coding_defines.h"
#include "webrtc/system_wrappers/interface/critical_section_wrapper.h"
#include "webrtc/system_wrappers/interface/scoped_ptr.h"
#include "webrtc/system_wrappers/interface/scoped_refptr.h"
#include "webrtc/system_wrappers/interface/tick_util.h"
#include "webrtc/typedefs.h"
#include "webrtc/video_engine/include/vie_network.h"
@ -303,7 +304,7 @@ class ViEChannel
// Gets the modules used by the channel.
RtpRtcp* rtp_rtcp();
PayloadRouter* send_payload_router();
scoped_refptr<PayloadRouter> send_payload_router();
CallStatsObserver* GetStatsObserver();
@ -495,7 +496,7 @@ class ViEChannel
scoped_ptr<RtpRtcp> rtp_rtcp_;
std::list<RtpRtcp*> simulcast_rtp_rtcp_;
std::list<RtpRtcp*> removed_rtp_rtcp_;
scoped_ptr<PayloadRouter> send_payload_router_;
scoped_refptr<PayloadRouter> send_payload_router_;
VideoCodingModule* const vcm_;
ViEReceiver vie_receiver_;

1
webrtc/video_engine/vie_channel_manager.cc
View File

@ -20,6 +20,7 @@
#include "webrtc/system_wrappers/interface/logging.h"
#include "webrtc/video_engine/call_stats.h"
#include "webrtc/video_engine/encoder_state_feedback.h"
#include "webrtc/video_engine/payload_router.h"
#include "webrtc/video_engine/vie_channel.h"
#include "webrtc/video_engine/vie_defines.h"
#include "webrtc/video_engine/vie_encoder.h"

3
webrtc/video_engine/vie_encoder.cc
View File

@ -229,7 +229,7 @@ bool ViEEncoder::Init() {
}
void ViEEncoder::StartThreadsAndSetSendPayloadRouter(
PayloadRouter* send_payload_router) {
scoped_refptr<PayloadRouter> send_payload_router) {
DCHECK(send_payload_router_ == NULL);
send_payload_router_ = send_payload_router;
@ -245,7 +245,6 @@ void ViEEncoder::StopThreadsAndRemovePayloadRouter() {
module_process_thread_.DeRegisterModule(&vcm_);
module_process_thread_.DeRegisterModule(&vpm_);
module_process_thread_.DeRegisterModule(default_rtp_rtcp_.get());
send_payload_router_ = nullptr;
}
ViEEncoder::~ViEEncoder() {

6
webrtc/video_engine/vie_encoder.h
View File

@ -24,6 +24,7 @@
#include "webrtc/modules/video_processing/main/interface/video_processing.h"
#include "webrtc/typedefs.h"
#include "webrtc/frame_callback.h"
#include "webrtc/system_wrappers/interface/scoped_refptr.h"
#include "webrtc/video_engine/vie_defines.h"
#include "webrtc/video_engine/vie_frame_provider_base.h"
@ -69,7 +70,8 @@ class ViEEncoder
// only once.
// Ideally this would be done in Init, but the dependencies between ViEEncoder
// and ViEChannel makes it really hard to do in a good way.
void StartThreadsAndSetSendPayloadRouter(PayloadRouter* send_payload_router);
void StartThreadsAndSetSendPayloadRouter(
scoped_refptr<PayloadRouter> send_payload_router);
// This function must be called before the corresponding ViEChannel is
// deleted.
@ -215,7 +217,7 @@ class ViEEncoder
VideoCodingModule& vcm_;
VideoProcessingModule& vpm_;
rtc::scoped_ptr<RtpRtcp> default_rtp_rtcp_;
PayloadRouter* send_payload_router_;
scoped_refptr<PayloadRouter> send_payload_router_;
rtc::scoped_ptr<CriticalSectionWrapper> callback_cs_;
rtc::scoped_ptr<CriticalSectionWrapper> data_cs_;
rtc::scoped_ptr<BitrateObserver> bitrate_observer_;