zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Replace sequence checker with lock in IvfFrameGemerator.

Browse Source 
It was found that generator can be destroyed on another thread
comparing to the one, from which frame were generated. It can happen
because generator injected into PC though scoped_ref object and the
last pointer to that object can be destroyed on different thread
depending on machine load.

To fix this sequence checker is replaced with lock. It is required
to ensure that generator won't be destroyed while it is reading frame,
because otherwise it can catch SIGSEGV.

Bug: webrtc:10138
Change-Id: Ia3488bd8ae396c209b90977469593784bb82114b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/162182
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30086}
This commit is contained in:
Artem Titov
2019-12-13 14:32:19 +01:00
committed by Commit Bot
parent b57fe17e7c
commit 9d06bc2e6d
2 changed files with 21 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
test/testsupport/ivf_video_frame_generator.cc
View File

@ -45,10 +45,9 @@ IvfVideoFrameGenerator::IvfVideoFrameGenerator(const std::string& file_name)
RTC_CHECK_EQ(
video_decoder_->InitDecode(&codec_settings, /*number_of_cores=*/1),
WEBRTC_VIDEO_CODEC_OK);
sequence_checker_.Detach();
}
IvfVideoFrameGenerator::~IvfVideoFrameGenerator() {
RTC_DCHECK_RUN_ON(&sequence_checker_);
rtc::CritScope crit(&lock_);
if (!file_reader_) {
return;
}
@ -57,7 +56,7 @@ IvfVideoFrameGenerator::~IvfVideoFrameGenerator() {
// Reset decoder to prevent it from async access to |this|.
video_decoder_.reset();
{
rtc::CritScope crit(&lock_);
rtc::CritScope frame_crit(&frame_decode_lock_);
next_frame_ = absl::nullopt;
// Set event in case another thread is waiting on it.
next_frame_decoded_.Set();
@ -65,7 +64,7 @@ IvfVideoFrameGenerator::~IvfVideoFrameGenerator() {
}
FrameGeneratorInterface::VideoFrameData IvfVideoFrameGenerator::NextFrame() {
RTC_DCHECK_RUN_ON(&sequence_checker_);
rtc::CritScope crit(&lock_);
next_frame_decoded_.Reset();
RTC_CHECK(file_reader_);
if (!file_reader_->HasMoreFrames()) {
@ -81,7 +80,7 @@ FrameGeneratorInterface::VideoFrameData IvfVideoFrameGenerator::NextFrame() {
RTC_CHECK(decoded) << "Failed to decode next frame in "
<< kMaxNextFrameWaitTemeoutMs << "ms. Can't continue";
rtc::CritScope crit(&lock_);
rtc::CritScope frame_crit(&frame_decode_lock_);
rtc::scoped_refptr<VideoFrameBuffer> buffer =
next_frame_->video_frame_buffer();
if (width_ != static_cast<size_t>(buffer->width()) ||
@ -97,7 +96,7 @@ FrameGeneratorInterface::VideoFrameData IvfVideoFrameGenerator::NextFrame() {
}
void IvfVideoFrameGenerator::ChangeResolution(size_t width, size_t height) {
RTC_DCHECK_RUN_ON(&sequence_checker_);
rtc::CritScope crit(&lock_);
width_ = width;
height_ = height;
}
@ -121,7 +120,7 @@ void IvfVideoFrameGenerator::DecodedCallback::Decoded(
}
void IvfVideoFrameGenerator::OnFrameDecoded(const VideoFrame& decoded_frame) {
rtc::CritScope crit(&lock_);
rtc::CritScope crit(&frame_decode_lock_);
next_frame_ = decoded_frame;
next_frame_decoded_.Set();
}

19
test/testsupport/ivf_video_frame_generator.h
View File

@ -63,11 +63,22 @@ class IvfVideoFrameGenerator : public FrameGeneratorInterface {
size_t width_;
size_t height_;
rtc::Event next_frame_decoded_;
SequenceChecker sequence_checker_;
// This lock is used to ensure that all API method will be called
// sequentially. It is required because we need to ensure that generator
// won't be destroyed while it is reading the next frame on another thread,
// because it will cause SIGSEGV when decoder callback will be invoked.
//
// FrameGenerator is injected into PeerConnection via some scoped_ref object
// and it can happen that the last pointer will be destroyed on the different
// thread comparing to the one from which frames were read.
rtc::CriticalSection lock_;
absl::optional<VideoFrame> next_frame_ RTC_GUARDED_BY(lock_);
// This lock is used to sync between sending and receiving frame from decoder.
// We can't reuse |lock_| because then generator can be destroyed between
// frame was sent to decoder and decoder callback was invoked.
rtc::CriticalSection frame_decode_lock_;
rtc::Event next_frame_decoded_;
absl::optional<VideoFrame> next_frame_ RTC_GUARDED_BY(frame_decode_lock_);
};
} // namespace test