Prevent the frame decryptor being set if the channel is stopped.

This change deals with a race condition if the media channel has been stopped
and is in the process of changing while we get a call to set a FrameDecryptor
or FrameEncryptor.

Bug: webrtc:9926, webrtc:9932
Change-Id: Ie2da2fa1f31f5cb5eb0b481861a7008e635f562d
Reviewed-on: https://webrtc-review.googlesource.com/c/107986
Commit-Queue: Benjamin Wright <benwright@webrtc.org>
Reviewed-by: Qingsi Wang <qingsi@webrtc.org>
Reviewed-by: Seth Hampson <shampson@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25398}
This commit is contained in:
Benjamin Wright
2018-10-26 13:16:16 -07:00
committed by Commit Bot
parent 625771d1d2
commit c462a6ef9b
3 changed files with 86 additions and 16 deletions

View File

@ -50,8 +50,9 @@ void MaybeAttachFrameDecryptorToMediaChannel(
const absl::optional<uint32_t>& ssrc,
rtc::Thread* worker_thread,
rtc::scoped_refptr<webrtc::FrameDecryptorInterface> frame_decryptor,
cricket::MediaChannel* media_channel) {
if (media_channel && frame_decryptor && ssrc.has_value()) {
cricket::MediaChannel* media_channel,
bool stopped) {
if (media_channel && frame_decryptor && ssrc.has_value() && !stopped) {
worker_thread->Invoke<void>(RTC_FROM_HERE, [&] {
media_channel->SetFrameDecryptor(*ssrc, frame_decryptor);
});
@ -157,7 +158,7 @@ void AudioRtpReceiver::SetFrameDecryptor(
rtc::scoped_refptr<FrameDecryptorInterface> frame_decryptor) {
frame_decryptor_ = std::move(frame_decryptor);
// Special Case: Set the frame decryptor to any value on any existing channel.
if (media_channel_ && ssrc_.has_value()) {
if (media_channel_ && ssrc_.has_value() && !stopped_) {
worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] {
media_channel_->SetFrameDecryptor(*ssrc_, frame_decryptor_);
});
@ -255,8 +256,8 @@ void AudioRtpReceiver::Reconfigure() {
RTC_NOTREACHED();
}
// Reattach the frame decryptor if we were reconfigured.
MaybeAttachFrameDecryptorToMediaChannel(ssrc_, worker_thread_,
frame_decryptor_, media_channel_);
MaybeAttachFrameDecryptorToMediaChannel(
ssrc_, worker_thread_, frame_decryptor_, media_channel_, stopped_);
}
void AudioRtpReceiver::SetObserver(RtpReceiverObserverInterface* observer) {
@ -351,7 +352,7 @@ void VideoRtpReceiver::SetFrameDecryptor(
rtc::scoped_refptr<FrameDecryptorInterface> frame_decryptor) {
frame_decryptor_ = std::move(frame_decryptor);
// Special Case: Set the frame decryptor to any value on any existing channel.
if (media_channel_ && ssrc_.has_value()) {
if (media_channel_ && ssrc_.has_value() && !stopped_) {
worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] {
media_channel_->SetFrameDecryptor(*ssrc_, frame_decryptor_);
});
@ -393,8 +394,8 @@ void VideoRtpReceiver::SetupMediaChannel(uint32_t ssrc) {
ssrc_ = ssrc;
SetSink(source_->sink());
// Attach any existing frame decryptor to the media channel.
MaybeAttachFrameDecryptorToMediaChannel(ssrc_, worker_thread_,
frame_decryptor_, media_channel_);
MaybeAttachFrameDecryptorToMediaChannel(
ssrc_, worker_thread_, frame_decryptor_, media_channel_, stopped_);
}
void VideoRtpReceiver::set_stream_ids(std::vector<std::string> stream_ids) {

View File

@ -72,8 +72,9 @@ void MaybeAttachFrameEncryptorToMediaChannel(
const uint32_t ssrc,
rtc::Thread* worker_thread,
rtc::scoped_refptr<webrtc::FrameEncryptorInterface> frame_encryptor,
cricket::MediaChannel* media_channel) {
if (media_channel && frame_encryptor && ssrc) {
cricket::MediaChannel* media_channel,
bool stopped) {
if (media_channel && frame_encryptor && ssrc && !stopped) {
worker_thread->Invoke<void>(RTC_FROM_HERE, [&] {
media_channel->SetFrameEncryptor(ssrc, frame_encryptor);
});
@ -307,7 +308,7 @@ void AudioRtpSender::SetFrameEncryptor(
rtc::scoped_refptr<FrameEncryptorInterface> frame_encryptor) {
frame_encryptor_ = std::move(frame_encryptor);
// Special Case: Set the frame encryptor to any value on any existing channel.
if (media_channel_ && ssrc_) {
if (media_channel_ && ssrc_ && !stopped_) {
worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] {
media_channel_->SetFrameEncryptor(ssrc_, frame_encryptor_);
});
@ -361,8 +362,8 @@ void AudioRtpSender::SetSsrc(uint32_t ssrc) {
});
}
// Each time there is an ssrc update.
MaybeAttachFrameEncryptorToMediaChannel(ssrc_, worker_thread_,
frame_encryptor_, media_channel_);
MaybeAttachFrameEncryptorToMediaChannel(
ssrc_, worker_thread_, frame_encryptor_, media_channel_, stopped_);
}
void AudioRtpSender::Stop() {
@ -563,7 +564,7 @@ void VideoRtpSender::SetFrameEncryptor(
rtc::scoped_refptr<FrameEncryptorInterface> frame_encryptor) {
frame_encryptor_ = std::move(frame_encryptor);
// Special Case: Set the frame encryptor to any value on any existing channel.
if (media_channel_ && ssrc_) {
if (media_channel_ && ssrc_ && !stopped_) {
worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] {
media_channel_->SetFrameEncryptor(ssrc_, frame_encryptor_);
});
@ -610,8 +611,8 @@ void VideoRtpSender::SetSsrc(uint32_t ssrc) {
init_parameters_.encodings.clear();
});
}
MaybeAttachFrameEncryptorToMediaChannel(ssrc_, worker_thread_,
frame_encryptor_, media_channel_);
MaybeAttachFrameEncryptorToMediaChannel(
ssrc_, worker_thread_, frame_encryptor_, media_channel_, stopped_);
}
void VideoRtpSender::Stop() {

View File

@ -1424,6 +1424,18 @@ TEST_F(RtpSenderReceiverTest, AudioSenderCanSetFrameEncryptor) {
audio_rtp_sender_->GetFrameEncryptor().get());
}
// Validate that setting a FrameEncryptor after the send stream is stopped does
// nothing.
TEST_F(RtpSenderReceiverTest, AudioSenderCannotSetFrameEncryptorAfterStop) {
CreateAudioRtpSender();
rtc::scoped_refptr<FrameEncryptorInterface> fake_frame_encryptor(
new FakeFrameEncryptor());
EXPECT_EQ(nullptr, audio_rtp_sender_->GetFrameEncryptor());
audio_rtp_sender_->Stop();
audio_rtp_sender_->SetFrameEncryptor(fake_frame_encryptor);
// TODO(webrtc:9926) - Validate media channel not set once fakes updated.
}
// Validate that the default FrameEncryptor setting is nullptr.
TEST_F(RtpSenderReceiverTest, AudioReceiverCanSetFrameDecryptor) {
CreateAudioRtpReceiver();
@ -1435,4 +1447,60 @@ TEST_F(RtpSenderReceiverTest, AudioReceiverCanSetFrameDecryptor) {
audio_rtp_receiver_->GetFrameDecryptor().get());
}
// Validate that the default FrameEncryptor setting is nullptr.
TEST_F(RtpSenderReceiverTest, AudioReceiverCannotSetFrameDecryptorAfterStop) {
CreateAudioRtpReceiver();
rtc::scoped_refptr<FrameDecryptorInterface> fake_frame_decryptor(
new FakeFrameDecryptor());
EXPECT_EQ(nullptr, audio_rtp_receiver_->GetFrameDecryptor());
audio_rtp_receiver_->Stop();
audio_rtp_receiver_->SetFrameDecryptor(fake_frame_decryptor);
// TODO(webrtc:9926) - Validate media channel not set once fakes updated.
}
// Validate that the default FrameEncryptor setting is nullptr.
TEST_F(RtpSenderReceiverTest, VideoSenderCanSetFrameEncryptor) {
CreateVideoRtpSender();
rtc::scoped_refptr<FrameEncryptorInterface> fake_frame_encryptor(
new FakeFrameEncryptor());
EXPECT_EQ(nullptr, video_rtp_sender_->GetFrameEncryptor());
video_rtp_sender_->SetFrameEncryptor(fake_frame_encryptor);
EXPECT_EQ(fake_frame_encryptor.get(),
video_rtp_sender_->GetFrameEncryptor().get());
}
// Validate that setting a FrameEncryptor after the send stream is stopped does
// nothing.
TEST_F(RtpSenderReceiverTest, VideoSenderCannotSetFrameEncryptorAfterStop) {
CreateVideoRtpSender();
rtc::scoped_refptr<FrameEncryptorInterface> fake_frame_encryptor(
new FakeFrameEncryptor());
EXPECT_EQ(nullptr, video_rtp_sender_->GetFrameEncryptor());
video_rtp_sender_->Stop();
video_rtp_sender_->SetFrameEncryptor(fake_frame_encryptor);
// TODO(webrtc:9926) - Validate media channel not set once fakes updated.
}
// Validate that the default FrameEncryptor setting is nullptr.
TEST_F(RtpSenderReceiverTest, VideoReceiverCanSetFrameDecryptor) {
CreateVideoRtpReceiver();
rtc::scoped_refptr<FrameDecryptorInterface> fake_frame_decryptor(
new FakeFrameDecryptor());
EXPECT_EQ(nullptr, video_rtp_receiver_->GetFrameDecryptor());
video_rtp_receiver_->SetFrameDecryptor(fake_frame_decryptor);
EXPECT_EQ(fake_frame_decryptor.get(),
video_rtp_receiver_->GetFrameDecryptor().get());
}
// Validate that the default FrameEncryptor setting is nullptr.
TEST_F(RtpSenderReceiverTest, VideoReceiverCannotSetFrameDecryptorAfterStop) {
CreateVideoRtpReceiver();
rtc::scoped_refptr<FrameDecryptorInterface> fake_frame_decryptor(
new FakeFrameDecryptor());
EXPECT_EQ(nullptr, video_rtp_receiver_->GetFrameDecryptor());
video_rtp_receiver_->Stop();
video_rtp_receiver_->SetFrameDecryptor(fake_frame_decryptor);
// TODO(webrtc:9926) - Validate media channel not set once fakes updated.
}
} // namespace webrtc