Fix shutdown race for ViEEncoder when there is a frame in the encoder.
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
diff --git a/webrtc/video_engine/payload_router.h b/webrtc/video_engine/payload_router.h
index 60f4747..ec9de99 100644
--- a/webrtc/video_engine/payload_router.h
+++ b/webrtc/video_engine/payload_router.h
@@ -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 @@
// 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 @@
std::vector<RtpRtcp*> rtp_modules_ GUARDED_BY(crit_.get());
bool active_ GUARDED_BY(crit_.get());
+ Atomic32 ref_count_;
+
DISALLOW_COPY_AND_ASSIGN(PayloadRouter);
};
diff --git a/webrtc/video_engine/vie_channel.cc b/webrtc/video_engine/vie_channel.cc
index 588e1a9..95cba40 100644
--- a/webrtc/video_engine/vie_channel.cc
+++ b/webrtc/video_engine/vie_channel.cc
@@ -1509,8 +1509,8 @@
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() {
diff --git a/webrtc/video_engine/vie_channel.h b/webrtc/video_engine/vie_channel.h
index 093e179..59ec250 100644
--- a/webrtc/video_engine/vie_channel.h
+++ b/webrtc/video_engine/vie_channel.h
@@ -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 @@
// 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 @@
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_;
diff --git a/webrtc/video_engine/vie_channel_manager.cc b/webrtc/video_engine/vie_channel_manager.cc
index c9f0f4e..d0fcaaf 100644
--- a/webrtc/video_engine/vie_channel_manager.cc
+++ b/webrtc/video_engine/vie_channel_manager.cc
@@ -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"
diff --git a/webrtc/video_engine/vie_encoder.cc b/webrtc/video_engine/vie_encoder.cc
index 9d4fdb2..cecb3a2 100644
--- a/webrtc/video_engine/vie_encoder.cc
+++ b/webrtc/video_engine/vie_encoder.cc
@@ -229,7 +229,7 @@
}
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 @@
module_process_thread_.DeRegisterModule(&vcm_);
module_process_thread_.DeRegisterModule(&vpm_);
module_process_thread_.DeRegisterModule(default_rtp_rtcp_.get());
- send_payload_router_ = nullptr;
}
ViEEncoder::~ViEEncoder() {
diff --git a/webrtc/video_engine/vie_encoder.h b/webrtc/video_engine/vie_encoder.h
index d9c6847..34120bc 100644
--- a/webrtc/video_engine/vie_encoder.h
+++ b/webrtc/video_engine/vie_encoder.h
@@ -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 @@
// 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 @@
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_;
