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

Unified Diff: content/renderer/media_recorder/video_track_recorder.cc

Issue 2750993002: Move destruction of VEAEncoder to encoding task runner (Closed)
Patch Set: Created 3 years, 9 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 | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/renderer/media_recorder/video_track_recorder.cc
diff --git a/content/renderer/media_recorder/video_track_recorder.cc b/content/renderer/media_recorder/video_track_recorder.cc
index 9ca982ed1ff3362d8065f43b2f0cb123e4217385..3b1b0a5578ee8617ee68c64058455517a096172e 100644
--- a/content/renderer/media_recorder/video_track_recorder.cc
+++ b/content/renderer/media_recorder/video_track_recorder.cc
@@ -452,6 +452,8 @@ class VEAEncoder final : public VideoTrackRecorder::Encoder,
void ConfigureEncoderOnEncodingTaskRunner(const gfx::Size& size);
+ void DestroyOnEncodingTaskRunner(base::WaitableEvent* async_waiter);
+
media::GpuVideoAcceleratorFactories* const gpu_factories_;
const media::VideoCodecProfile codec_;
@@ -615,9 +617,20 @@ VEAEncoder::VEAEncoder(
}
VEAEncoder::~VEAEncoder() {
+ base::WaitableEvent release_waiter(
+ base::WaitableEvent::ResetPolicy::MANUAL,
+ base::WaitableEvent::InitialState::NOT_SIGNALED);
+ // base::Unretained is safe because the class will be alive until
+ // |release_waiter| is signaled.
+ // TODO(emircan): Consider refactoring media::VideoEncodeAccelerator to avoid
+ // using naked pointers and using DeleteSoon() here, see
+ // http://crbug.com/701627.
+ // It is currently unsafe because |video_encoder_| might be in use on another
+ // function on |encoding_task_runner_|, see http://crbug.com/701030.
encoding_task_runner_->PostTask(
- FROM_HERE, base::Bind(&media::VideoEncodeAccelerator::Destroy,
- base::Unretained(video_encoder_.release())));
+ FROM_HERE, base::Bind(&VEAEncoder::DestroyOnEncodingTaskRunner,
+ base::Unretained(this), &release_waiter));
+ release_waiter.Wait();
}
void VEAEncoder::RequireBitstreamBuffers(unsigned int /*input_count*/,
@@ -772,10 +785,7 @@ void VEAEncoder::EncodeOnEncodingTaskRunner(
frames_in_encode_.push(std::make_pair(
media::WebmMuxer::VideoParameters(frame), capture_timestamp));
- encoding_task_runner_->PostTask(
- FROM_HERE,
- base::Bind(&media::VideoEncodeAccelerator::Encode,
- base::Unretained(video_encoder_.get()), video_frame, false));
+ video_encoder_->Encode(video_frame, false);
}
void VEAEncoder::ConfigureEncoderOnEncodingTaskRunner(const gfx::Size& size) {
@@ -793,6 +803,13 @@ void VEAEncoder::ConfigureEncoderOnEncodingTaskRunner(const gfx::Size& size) {
}
}
+void VEAEncoder::DestroyOnEncodingTaskRunner(
+ base::WaitableEvent* async_waiter) {
+ DCHECK(encoding_task_runner_->BelongsToCurrentThread());
+ video_encoder_.reset();
+ async_waiter->Signal();
+}
+
// static
void VpxEncoder::ShutdownEncoder(std::unique_ptr<base::Thread> encoding_thread,
ScopedVpxCodecCtxPtr encoder) {
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698