Commit d79a374a authored by tzik's avatar tzik Committed by Commit Bot

Avoid touching VEAEncoder's ref count before it's fully constructed

This contains a similar fix to the reverted previous attempt at
http://crrev.com/556813c93fa177cd

VEAEncoder is ref counted, and its first reference used to be retained
in its constructor through base::Bind(). The reference is passed to the
other sequence, and released when the callback instance is destroyed.

If the PostTask there failed or the posted task ran soon, the
VEAEncoder instance can be destroyed before another reference is
retained on the original sequence.
That is, `new VEAEncoder` may return a stale pointer.

This CL adds a static cosntructor to VEAEncoder, and keeps the first
reference before calling PostTask.

Bug: 866456
Change-Id: Icbb430bebbfa5c800f3c0248b5afd10f2c2de271
Reviewed-on: https://chromium-review.googlesource.com/1158316Reviewed-by: default avatarEmircan Uysaler <emircan@chromium.org>
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580050}
parent a44e5c52
...@@ -34,6 +34,23 @@ const uint32_t kMaxKeyframeInterval = 100; ...@@ -34,6 +34,23 @@ const uint32_t kMaxKeyframeInterval = 100;
} // anonymous namespace } // anonymous namespace
scoped_refptr<VEAEncoder> VEAEncoder::Create(
const VideoTrackRecorder::OnEncodedVideoCB& on_encoded_video_callback,
const VideoTrackRecorder::OnErrorCB& on_error_callback,
int32_t bits_per_second,
media::VideoCodecProfile codec,
const gfx::Size& size,
scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
auto encoder = base::WrapRefCounted(
new VEAEncoder(on_encoded_video_callback, on_error_callback,
bits_per_second, codec, size, std::move(task_runner)));
encoder->encoding_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&VEAEncoder::ConfigureEncoderOnEncodingTaskRunner, encoder,
size));
return encoder;
}
VEAEncoder::VEAEncoder( VEAEncoder::VEAEncoder(
const VideoTrackRecorder::OnEncodedVideoCB& on_encoded_video_callback, const VideoTrackRecorder::OnEncodedVideoCB& on_encoded_video_callback,
const VideoTrackRecorder::OnErrorCB& on_error_callback, const VideoTrackRecorder::OnErrorCB& on_error_callback,
...@@ -55,11 +72,6 @@ VEAEncoder::VEAEncoder( ...@@ -55,11 +72,6 @@ VEAEncoder::VEAEncoder(
DCHECK(gpu_factories_); DCHECK(gpu_factories_);
DCHECK_GE(size.width(), kVEAEncoderMinResolutionWidth); DCHECK_GE(size.width(), kVEAEncoderMinResolutionWidth);
DCHECK_GE(size.height(), kVEAEncoderMinResolutionHeight); DCHECK_GE(size.height(), kVEAEncoderMinResolutionHeight);
encoding_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&VEAEncoder::ConfigureEncoderOnEncodingTaskRunner, this,
size));
} }
VEAEncoder::~VEAEncoder() { VEAEncoder::~VEAEncoder() {
......
...@@ -29,7 +29,7 @@ namespace content { ...@@ -29,7 +29,7 @@ namespace content {
class VEAEncoder final : public VideoTrackRecorder::Encoder, class VEAEncoder final : public VideoTrackRecorder::Encoder,
public media::VideoEncodeAccelerator::Client { public media::VideoEncodeAccelerator::Client {
public: public:
VEAEncoder( static scoped_refptr<VEAEncoder> Create(
const VideoTrackRecorder::OnEncodedVideoCB& on_encoded_video_callback, const VideoTrackRecorder::OnEncodedVideoCB& on_encoded_video_callback,
const VideoTrackRecorder::OnErrorCB& on_error_callback, const VideoTrackRecorder::OnErrorCB& on_error_callback,
int32_t bits_per_second, int32_t bits_per_second,
...@@ -52,6 +52,14 @@ class VEAEncoder final : public VideoTrackRecorder::Encoder, ...@@ -52,6 +52,14 @@ class VEAEncoder final : public VideoTrackRecorder::Encoder,
using VideoParamsAndTimestamp = using VideoParamsAndTimestamp =
std::pair<media::WebmMuxer::VideoParameters, base::TimeTicks>; std::pair<media::WebmMuxer::VideoParameters, base::TimeTicks>;
VEAEncoder(
const VideoTrackRecorder::OnEncodedVideoCB& on_encoded_video_callback,
const VideoTrackRecorder::OnErrorCB& on_error_callback,
int32_t bits_per_second,
media::VideoCodecProfile codec,
const gfx::Size& size,
scoped_refptr<base::SingleThreadTaskRunner> task_runner);
void UseOutputBitstreamBufferId(int32_t bitstream_buffer_id); void UseOutputBitstreamBufferId(int32_t bitstream_buffer_id);
void FrameFinished(std::unique_ptr<base::SharedMemory> shm); void FrameFinished(std::unique_ptr<base::SharedMemory> shm);
......
...@@ -454,7 +454,7 @@ void VideoTrackRecorder::InitializeEncoder( ...@@ -454,7 +454,7 @@ void VideoTrackRecorder::InitializeEncoder(
input_size.height())) { input_size.height())) {
UMA_HISTOGRAM_BOOLEAN("Media.MediaRecorder.VEAUsed", true); UMA_HISTOGRAM_BOOLEAN("Media.MediaRecorder.VEAUsed", true);
const auto vea_profile = GetCodecEnumerator()->CodecIdToVEAProfile(codec); const auto vea_profile = GetCodecEnumerator()->CodecIdToVEAProfile(codec);
encoder_ = new VEAEncoder( encoder_ = VEAEncoder::Create(
on_encoded_video_callback, on_encoded_video_callback,
media::BindToCurrentLoop(base::Bind(&VideoTrackRecorder::OnError, media::BindToCurrentLoop(base::Bind(&VideoTrackRecorder::OnError,
weak_ptr_factory_.GetWeakPtr())), weak_ptr_factory_.GetWeakPtr())),
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment