Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(505)

Unified Diff: media/gpu/ipc/service/gpu_video_encode_accelerator.cc

Issue 2849443003: Fix errors in destruction sequence of GpuVideoEncodeAccelerator::OnWillDestroyStub() (Closed)
Patch Set: Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « media/gpu/ipc/service/gpu_video_encode_accelerator.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: media/gpu/ipc/service/gpu_video_encode_accelerator.cc
diff --git a/media/gpu/ipc/service/gpu_video_encode_accelerator.cc b/media/gpu/ipc/service/gpu_video_encode_accelerator.cc
index 533e2cd95de4ed3640239454d8f7c89de8e65849..7d31fcf60a36af77599672fba163c89093789816 100644
--- a/media/gpu/ipc/service/gpu_video_encode_accelerator.cc
+++ b/media/gpu/ipc/service/gpu_video_encode_accelerator.cc
@@ -173,6 +173,9 @@ GpuVideoEncodeAccelerator::GpuVideoEncodeAccelerator(
encode_task_runner_(main_task_runner_),
weak_this_factory_for_encoder_worker_(this),
weak_this_factory_(this) {
+ weak_this_for_encoder_worker_ =
+ weak_this_factory_for_encoder_worker_.GetWeakPtr();
+ weak_this_ = weak_this_factory_.GetWeakPtr();
stub_->AddDestructionObserver(this);
make_context_current_ =
base::Bind(&MakeDecoderContextCurrent, stub_->AsWeakPtr());
@@ -182,13 +185,7 @@ GpuVideoEncodeAccelerator::~GpuVideoEncodeAccelerator() {
// This class can only be self-deleted from OnWillDestroyStub(), which means
// the VEA has already been destroyed in there.
DCHECK(!encoder_);
- if (encoder_worker_thread_.IsRunning()) {
- encoder_worker_task_runner_->PostTask(
- FROM_HERE,
- base::Bind(&GpuVideoEncodeAccelerator::DestroyOnEncoderWorker,
- weak_this_factory_for_encoder_worker_.GetWeakPtr()));
- encoder_worker_thread_.Stop();
- }
+ DCHECK(!encoder_worker_thread_.IsRunning());
}
bool GpuVideoEncodeAccelerator::Initialize(VideoPixelFormat input_format,
@@ -229,8 +226,8 @@ bool GpuVideoEncodeAccelerator::Initialize(VideoPixelFormat input_format,
input_visible_size_ = input_visible_size;
// Attempt to set up performing encoding tasks on IO thread, if supported
// by the VEA.
- if (encoder_->TryToSetupEncodeOnSeparateThread(
- weak_this_factory_.GetWeakPtr(), io_task_runner_)) {
+ if (encoder_->TryToSetupEncodeOnSeparateThread(weak_this_,
+ io_task_runner_)) {
filter_ = new MessageFilter(this, host_route_id_);
stub_->channel()->AddFilter(filter_.get());
encode_task_runner_ = io_task_runner_;
@@ -325,6 +322,16 @@ void GpuVideoEncodeAccelerator::OnWillDestroyStub() {
filter_removed_.Wait();
}
+ // We should stop |encoder_worker_thread_| before releasing |encoder_|, see
+ // crbug.com/715759.
+ if (encoder_worker_thread_.IsRunning()) {
+ encoder_worker_task_runner_->PostTask(
+ FROM_HERE,
+ base::Bind(&GpuVideoEncodeAccelerator::DestroyOnEncoderWorker,
+ weak_this_for_encoder_worker_));
+ encoder_worker_thread_.Stop();
+ }
+
stub_->channel()->RemoveRoute(host_route_id_);
stub_->RemoveDestructionObserver(this);
encoder_.reset();
@@ -404,7 +411,7 @@ void GpuVideoEncodeAccelerator::OnEncode(
encoder_worker_task_runner_->PostTask(
FROM_HERE,
base::Bind(&GpuVideoEncodeAccelerator::CreateEncodeFrameOnEncoderWorker,
- weak_this_factory_for_encoder_worker_.GetWeakPtr(), params));
+ weak_this_for_encoder_worker_, params));
}
void GpuVideoEncodeAccelerator::OnUseOutputBitstreamBuffer(
@@ -465,18 +472,18 @@ void GpuVideoEncodeAccelerator::CreateEncodeFrameOnEncoderWorker(
if (!map_offset.IsValid() || !map_size.IsValid()) {
DLOG(ERROR) << __func__ << " invalid map_offset or map_size";
encode_task_runner_->PostTask(
- FROM_HERE, base::Bind(&GpuVideoEncodeAccelerator::NotifyError,
- weak_this_factory_.GetWeakPtr(),
- VideoEncodeAccelerator::kPlatformFailureError));
+ FROM_HERE,
+ base::Bind(&GpuVideoEncodeAccelerator::NotifyError, weak_this_,
+ VideoEncodeAccelerator::kPlatformFailureError));
return;
}
if (!shm->MapAt(map_offset.ValueOrDie(), map_size.ValueOrDie())) {
DLOG(ERROR) << __func__ << " could not map frame_id=" << params.frame_id;
encode_task_runner_->PostTask(
- FROM_HERE, base::Bind(&GpuVideoEncodeAccelerator::NotifyError,
- weak_this_factory_.GetWeakPtr(),
- VideoEncodeAccelerator::kPlatformFailureError));
+ FROM_HERE,
+ base::Bind(&GpuVideoEncodeAccelerator::NotifyError, weak_this_,
+ VideoEncodeAccelerator::kPlatformFailureError));
return;
}
@@ -489,9 +496,9 @@ void GpuVideoEncodeAccelerator::CreateEncodeFrameOnEncoderWorker(
if (!frame) {
DLOG(ERROR) << __func__ << " could not create a frame";
encode_task_runner_->PostTask(
- FROM_HERE, base::Bind(&GpuVideoEncodeAccelerator::NotifyError,
- weak_this_factory_.GetWeakPtr(),
- VideoEncodeAccelerator::kPlatformFailureError));
+ FROM_HERE,
+ base::Bind(&GpuVideoEncodeAccelerator::NotifyError, weak_this_,
+ VideoEncodeAccelerator::kPlatformFailureError));
return;
}
@@ -500,9 +507,9 @@ void GpuVideoEncodeAccelerator::CreateEncodeFrameOnEncoderWorker(
frame->AddDestructionObserver(
base::Bind(&DropSharedMemory, base::Passed(&shm)));
encode_task_runner_->PostTask(
- FROM_HERE, base::Bind(&GpuVideoEncodeAccelerator::OnEncodeFrameCreated,
- weak_this_factory_.GetWeakPtr(), params.frame_id,
- params.force_keyframe, frame));
+ FROM_HERE,
+ base::Bind(&GpuVideoEncodeAccelerator::OnEncodeFrameCreated, weak_this_,
+ params.frame_id, params.force_keyframe, frame));
}
void GpuVideoEncodeAccelerator::DestroyOnEncoderWorker() {
@@ -517,6 +524,9 @@ void GpuVideoEncodeAccelerator::OnEncodeFrameCreated(
DVLOG(3) << __func__;
DCHECK(CheckIfCalledOnCorrectThread());
+ if (filter_removed_.IsSignaled())
+ return;
+
if (!frame) {
DLOG(ERROR) << __func__ << " could not create a frame";
NotifyError(VideoEncodeAccelerator::kPlatformFailureError);
« no previous file with comments | « media/gpu/ipc/service/gpu_video_encode_accelerator.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698