Improve thread safety of AndroidVideoTrackSource::SetState.

1. Prevents deadlocks from AsyncInvoker destructor
2. Makes future state() calls are guaranteed to return the new state after
   SetState() completes.

I am not sure if it is allowed to call FireOnChanged from non-signaling
threads so I will leave the post for now.

Bug: webrtc:10813
Change-Id: I5712a45f71431765898037867382397d537570a0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/147727
Commit-Queue: Sami Kalliomäki <sakal@webrtc.org>
Reviewed-by: Magnus Jedvert <magjed@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28741}
This commit is contained in:
Sami Kalliomäki
2019-08-01 17:16:29 +02:00
committed by Commit Bot
parent b3f78deb78
commit 9160b627d7
2 changed files with 18 additions and 19 deletions

View File

@ -14,6 +14,7 @@
#include <utility>
#include "rtc_base/bind.h"
#include "rtc_base/logging.h"
namespace webrtc {
@ -60,25 +61,25 @@ absl::optional<bool> AndroidVideoTrackSource::needs_denoising() const {
void AndroidVideoTrackSource::SetState(JNIEnv* env,
jboolean j_is_live) {
InternalSetState(j_is_live ? kLive : kEnded);
}
void AndroidVideoTrackSource::InternalSetState(SourceState state) {
if (rtc::Thread::Current() != signaling_thread_) {
invoker_.AsyncInvoke<void>(
RTC_FROM_HERE, signaling_thread_,
rtc::Bind(&AndroidVideoTrackSource::InternalSetState, this, state));
return;
}
if (state_ != state) {
state_ = state;
FireOnChanged();
const SourceState state = j_is_live ? kLive : kEnded;
if (state_.exchange(state) != state) {
if (rtc::Thread::Current() == signaling_thread_) {
FireOnChanged();
} else {
// TODO(sakal): Is this even necessary, does FireOnChanged have to be
// called from signaling thread?
signaling_thread_->PostTask(
RTC_FROM_HERE,
rtc::Bind(
&AndroidVideoTrackSource::FireOnChanged,
static_cast<webrtc::Notifier<webrtc::VideoTrackSourceInterface>*>(
this)));
}
}
}
AndroidVideoTrackSource::SourceState AndroidVideoTrackSource::state() const {
return state_;
return state_.load();
}
bool AndroidVideoTrackSource::remote() const {

View File

@ -18,6 +18,7 @@
#include "media/base/adapted_video_track_source.h"
#include "rtc_base/async_invoker.h"
#include "rtc_base/checks.h"
#include "rtc_base/thread.h"
#include "rtc_base/timestamp_aligner.h"
#include "sdk/android/src/jni/video_frame.h"
@ -84,11 +85,8 @@ class AndroidVideoTrackSource : public rtc::AdaptedVideoTrackSource {
const JavaRef<jobject>& j_max_fps);
private:
void InternalSetState(SourceState state);
rtc::Thread* signaling_thread_;
rtc::AsyncInvoker invoker_;
SourceState state_;
std::atomic<SourceState> state_;
const bool is_screencast_;
rtc::TimestampAligner timestamp_aligner_;
const bool align_timestamps_;