Bug Fix: Multiplex Codec Crash

This CL adds a lock to stashed_images_ in MultiplexEncoderAdapter.
Without lock, it is possible that different threads acts on
stashed_images_ simultaneously and leads to crash.

Bug: webrtc:8965
Change-Id: I887861092d185c3bd6047eb529d8c1cf57fa4648
Reviewed-on: https://webrtc-review.googlesource.com/59260
Reviewed-by: Emircan Uysaler <emircan@webrtc.org>
Commit-Queue: Qiang Chen <qiangchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#22261}
This commit is contained in:
Qiang Chen
2018-03-01 13:41:44 -08:00
committed by Commit Bot
parent 243cd32a9e
commit 6d30631f01
2 changed files with 21 additions and 11 deletions

View File

@ -20,6 +20,7 @@
#include "api/video_codecs/video_encoder_factory.h"
#include "modules/video_coding/codecs/multiplex/include/multiplex_encoded_image_packer.h"
#include "modules/video_coding/include/video_codec_interface.h"
#include "rtc_base/criticalsection.h"
namespace webrtc {
@ -66,13 +67,16 @@ class MultiplexEncoderAdapter : public VideoEncoder {
std::vector<std::unique_ptr<AdapterEncodedImageCallback>> adapter_callbacks_;
EncodedImageCallback* encoded_complete_callback_;
std::map<uint32_t /* timestamp */, MultiplexImage> stashed_images_;
std::map<uint32_t /* timestamp */, MultiplexImage> stashed_images_
RTC_GUARDED_BY(crit_);
uint16_t picture_index_ = 0;
std::vector<uint8_t> multiplex_dummy_planes_;
int key_frame_interval_;
EncodedImage combined_image_;
rtc::CriticalSection crit_;
};
} // namespace webrtc

View File

@ -122,10 +122,14 @@ int MultiplexEncoderAdapter::Encode(
}
const bool has_alpha = input_image.video_frame_buffer()->type() ==
VideoFrameBuffer::Type::kI420A;
stashed_images_.emplace(
std::piecewise_construct, std::forward_as_tuple(input_image.timestamp()),
std::forward_as_tuple(picture_index_,
has_alpha ? kAlphaCodecStreams : 1));
{
rtc::CritScope cs(&crit_);
stashed_images_.emplace(
std::piecewise_construct,
std::forward_as_tuple(input_image.timestamp()),
std::forward_as_tuple(picture_index_,
has_alpha ? kAlphaCodecStreams : 1));
}
++picture_index_;
@ -192,6 +196,7 @@ int MultiplexEncoderAdapter::Release() {
}
encoders_.clear();
adapter_callbacks_.clear();
rtc::CritScope cs(&crit_);
for (auto& stashed_image : stashed_images_) {
for (auto& image_component : stashed_image.second.image_components) {
delete[] image_component.encoded_image._buffer;
@ -214,12 +219,6 @@ EncodedImageCallback::Result MultiplexEncoderAdapter::OnEncodedImage(
const EncodedImage& encodedImage,
const CodecSpecificInfo* codecSpecificInfo,
const RTPFragmentationHeader* fragmentation) {
const auto& stashed_image_itr = stashed_images_.find(encodedImage._timeStamp);
const auto& stashed_image_next_itr = std::next(stashed_image_itr, 1);
RTC_DCHECK(stashed_image_itr != stashed_images_.end());
MultiplexImage& stashed_image = stashed_image_itr->second;
const uint8_t frame_count = stashed_image.component_count;
// Save the image
MultiplexImageComponent image_component;
image_component.component_index = stream_idx;
@ -230,6 +229,13 @@ EncodedImageCallback::Result MultiplexEncoderAdapter::OnEncodedImage(
std::memcpy(image_component.encoded_image._buffer, encodedImage._buffer,
encodedImage._length);
rtc::CritScope cs(&crit_);
const auto& stashed_image_itr = stashed_images_.find(encodedImage._timeStamp);
const auto& stashed_image_next_itr = std::next(stashed_image_itr, 1);
RTC_DCHECK(stashed_image_itr != stashed_images_.end());
MultiplexImage& stashed_image = stashed_image_itr->second;
const uint8_t frame_count = stashed_image.component_count;
stashed_image.image_components.push_back(image_component);
if (stashed_image.image_components.size() == frame_count) {