Skip to content

Commit

Permalink
media: Enable using SequencedTaskRunner for MojoVEAProvider
Browse files Browse the repository at this point in the history
MojoVideoEncodeAcceleratorProvider runs on a SingleThreadTaskRunner
made by ThreadPool. This CL enables using SequencedTaskRunner by flag.
It is expected to reduce the thread hops relying on the chrome
sequence scheduler. The flag is disabled by default now, so this must
not change the chrome behavior.

Bug: b:266774158
Test: Google Meet call on trogdor with
      --enable-features=UseSequencedTaskRunnerForMojoVEAProvider

Change-Id: I12dff2d658cf83ba5eb392bd024447c7c57f0635
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4190434
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1099130}
  • Loading branch information
Hirokazu Honda authored and Chromium LUCI CQ committed Jan 31, 2023
1 parent 23b654b commit 4abbb98
Show file tree
Hide file tree
Showing 14 changed files with 60 additions and 39 deletions.
1 change: 1 addition & 0 deletions components/viz/service/gl/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ include_rules = [
"+ipc",
"+media/base/android/media_codec_util.h",
"+media/base/media_log.h",
"+media/base/media_switches.h",
"+media/base/win/mf_feature_checks.h",
"+media/gpu",
"+media/mojo",
Expand Down
10 changes: 8 additions & 2 deletions components/viz/service/gl/gpu_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
#include "ipc/ipc_sync_channel.h"
#include "ipc/ipc_sync_message_filter.h"
#include "media/base/media_log.h"
#include "media/base/media_switches.h"
#include "media/gpu/buildflags.h"
#include "media/gpu/gpu_video_accelerator_util.h"
#include "media/gpu/gpu_video_encode_accelerator_factory.h"
Expand Down Expand Up @@ -812,7 +813,7 @@ void GpuServiceImpl::CreateVideoEncodeAcceleratorProvider(
// Offload VEA providers to a dedicated runner. Things like loading profiles
// and creating encoder might take quite some time, and they might block
// processing of other mojo calls if executed on the current runner.
scoped_refptr<base::SingleThreadTaskRunner> runner;
scoped_refptr<base::SequencedTaskRunner> runner;
#if BUILDFLAG(IS_FUCHSIA)
// TODO(crbug.com/1340041): Fuchsia does not support FIDL communication from
// ThreadPool's worker threads.
Expand All @@ -825,7 +826,12 @@ void GpuServiceImpl::CreateVideoEncodeAcceleratorProvider(
runner = vea_thread_->task_runner();
#else
// MayBlock() because MF VEA can take long time running GetSupportedProfiles()
runner = base::ThreadPool::CreateSingleThreadTaskRunner({base::MayBlock()});
if (base::FeatureList::IsEnabled(
media::kUseSequencedTaskRunnerForMojoVEAProvider)) {
runner = base::ThreadPool::CreateSequencedTaskRunner({base::MayBlock()});
} else {
runner = base::ThreadPool::CreateSingleThreadTaskRunner({base::MayBlock()});
}
#endif
media::MojoVideoEncodeAcceleratorProvider::Create(
std::move(vea_provider_receiver),
Expand Down
5 changes: 5 additions & 0 deletions media/base/media_switches.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1215,6 +1215,11 @@ BASE_FEATURE(kUseSequencedTaskRunnerForMediaService,
"UseSequencedTaskRunnerForMediaService",
base::FEATURE_DISABLED_BY_DEFAULT);

// Use SequencedTaskRunner for MojoVideoEncodeAcceleratorProvider.
BASE_FEATURE(kUseSequencedTaskRunnerForMojoVEAProvider,
"UseSequencedTaskRunnerForMojoVEAProvider",
base::FEATURE_DISABLED_BY_DEFAULT);

std::string GetEffectiveAutoplayPolicy(const base::CommandLine& command_line) {
// Return the autoplay policy set in the command line, if any.
if (command_line.HasSwitch(switches::kAutoplayPolicy))
Expand Down
1 change: 1 addition & 0 deletions media/base/media_switches.h
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ MEDIA_EXPORT BASE_DECLARE_FEATURE(kUseOutOfProcessVideoEncoding);

MEDIA_EXPORT BASE_DECLARE_FEATURE(kUseMojoVideoDecoderForPepper);
MEDIA_EXPORT BASE_DECLARE_FEATURE(kUseSequencedTaskRunnerForMediaService);
MEDIA_EXPORT BASE_DECLARE_FEATURE(kUseSequencedTaskRunnerForMojoVEAProvider);

#if BUILDFLAG(IS_FUCHSIA)
MEDIA_EXPORT BASE_DECLARE_FEATURE(kFuchsiaMediacodecVideoEncoder);
Expand Down
20 changes: 10 additions & 10 deletions media/gpu/android/android_video_encode_accelerator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#include "base/memory/shared_memory_mapping.h"
#include "base/memory/unsafe_shared_memory_region.h"
#include "base/metrics/histogram_macros.h"
#include "base/task/single_thread_task_runner.h"
#include "base/task/sequenced_task_runner.h"
#include "gpu/command_buffer/service/gles2_cmd_decoder.h"
#include "gpu/ipc/service/gpu_channel.h"
#include "media/base/android/media_codec_util.h"
Expand Down Expand Up @@ -97,7 +97,7 @@ static bool GetSupportedColorFormatForMime(const std::string& mime,
AndroidVideoEncodeAccelerator::AndroidVideoEncodeAccelerator() = default;

AndroidVideoEncodeAccelerator::~AndroidVideoEncodeAccelerator() {
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
}

VideoEncodeAccelerator::SupportedProfiles
Expand Down Expand Up @@ -145,7 +145,7 @@ bool AndroidVideoEncodeAccelerator::Initialize(
std::unique_ptr<MediaLog> media_log) {
DVLOG(3) << __func__ << " " << config.AsHumanReadableString();
DCHECK(!media_codec_);
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(client);
log_ = std::move(media_log);
client_ptr_factory_ = std::make_unique<base::WeakPtrFactory<Client>>(client);
Expand Down Expand Up @@ -213,7 +213,7 @@ bool AndroidVideoEncodeAccelerator::Initialize(
VideoFrame::AllocationSize(config.input_format,
config.input_visible_size) +
2048;
base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&VideoEncodeAccelerator::Client::RequireBitstreamBuffers,
client_ptr_factory_->GetWeakPtr(), frame_input_count,
Expand All @@ -239,7 +239,7 @@ void AndroidVideoEncodeAccelerator::MaybeStopIOTimer() {
void AndroidVideoEncodeAccelerator::Encode(scoped_refptr<VideoFrame> frame,
bool force_keyframe) {
DVLOG(3) << __PRETTY_FUNCTION__ << ": " << force_keyframe;
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
RETURN_ON_FAILURE(frame->format() == PIXEL_FORMAT_I420, "Unexpected format",
kInvalidArgumentError);
RETURN_ON_FAILURE(frame->visible_rect().size() == frame_size_,
Expand All @@ -252,7 +252,7 @@ void AndroidVideoEncodeAccelerator::Encode(scoped_refptr<VideoFrame> frame,
void AndroidVideoEncodeAccelerator::UseOutputBitstreamBuffer(
BitstreamBuffer buffer) {
DVLOG(3) << __PRETTY_FUNCTION__ << ": bitstream_buffer_id=" << buffer.id();
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
available_bitstream_buffers_.push_back(std::move(buffer));
DoIOTask();
}
Expand All @@ -266,7 +266,7 @@ void AndroidVideoEncodeAccelerator::RequestEncodingParametersChange(
"Unexpected bitrate mode", kInvalidArgumentError);
DVLOG(3) << __PRETTY_FUNCTION__ << ": bitrate: " << bitrate.ToString()
<< ", framerate: " << framerate;
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (bitrate.target_bps() != last_set_bitrate_) {
last_set_bitrate_ = bitrate.target_bps();
media_codec_->SetVideoBitrate(bitrate.target_bps(), framerate);
Expand All @@ -279,7 +279,7 @@ void AndroidVideoEncodeAccelerator::RequestEncodingParametersChange(

void AndroidVideoEncodeAccelerator::Destroy() {
DVLOG(3) << __PRETTY_FUNCTION__;
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
client_ptr_factory_.reset();
if (media_codec_) {
if (io_timer_.IsRunning())
Expand Down Expand Up @@ -462,7 +462,7 @@ void AndroidVideoEncodeAccelerator::DequeueOutput() {
metadata.encoded_size = *aligned_size_;
}

base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&VideoEncodeAccelerator::Client::BitstreamBufferReady,
client_ptr_factory_->GetWeakPtr(), bitstream_buffer.id(),
Expand Down Expand Up @@ -493,7 +493,7 @@ bool AndroidVideoEncodeAccelerator::SetInputBufferLayout() {
VideoEncoderInfo encoder_info;
encoder_info.requested_resolution_alignment = 16;
encoder_info.apply_alignment_to_all_simulcast_layers = true;
base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(&VideoEncodeAccelerator::Client::NotifyEncoderInfoChange,
client_ptr_factory_->GetWeakPtr(), encoder_info));
Expand Down
6 changes: 3 additions & 3 deletions media/gpu/android/android_video_encode_accelerator.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

#include "base/containers/queue.h"
#include "base/memory/weak_ptr.h"
#include "base/threading/thread_checker.h"
#include "base/sequence_checker.h"
#include "base/time/time.h"
#include "base/timer/timer.h"
#include "media/base/android/media_codec_bridge_impl.h"
Expand Down Expand Up @@ -80,8 +80,8 @@ class MEDIA_GPU_EXPORT AndroidVideoEncodeAccelerator
// frames are cropped to the nearest 16x16 alignment.
bool SetInputBufferLayout();

// Used to DCHECK that we are called on the correct thread.
base::ThreadChecker thread_checker_;
// Used to DCHECK that we are called on the correct sequence.
SEQUENCE_CHECKER(sequence_checker_);

// VideoDecodeAccelerator::Client callbacks go here. Invalidated once any
// error triggers.
Expand Down
4 changes: 2 additions & 2 deletions media/gpu/gpu_video_encode_accelerator_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include "base/feature_list.h"
#include "base/functional/bind.h"
#include "base/memory/ptr_util.h"
#include "base/task/single_thread_task_runner.h"
#include "base/task/sequenced_task_runner.h"
#include "build/build_config.h"
#include "gpu/config/gpu_driver_bug_workarounds.h"
#include "gpu/config/gpu_preferences.h"
Expand Down Expand Up @@ -69,7 +69,7 @@ std::unique_ptr<VideoEncodeAccelerator> CreateAndroidVEA() {
if (NdkVideoEncodeAccelerator::IsSupported()) {
return base::WrapUnique<VideoEncodeAccelerator>(
new NdkVideoEncodeAccelerator(
base::SingleThreadTaskRunner::GetCurrentDefault()));
base::SequencedTaskRunner::GetCurrentDefault()));
} else {
return base::WrapUnique<VideoEncodeAccelerator>(
new AndroidVideoEncodeAccelerator());
Expand Down
5 changes: 3 additions & 2 deletions media/gpu/v4l2/v4l2_video_encode_accelerator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "base/memory/shared_memory_mapping.h"
#include "base/memory/unsafe_shared_memory_region.h"
#include "base/numerics/safe_conversions.h"
#include "base/task/sequenced_task_runner.h"
#include "base/task/single_thread_task_runner.h"
#include "base/task/task_traits.h"
#include "base/task/thread_pool.h"
Expand Down Expand Up @@ -181,7 +182,7 @@ base::AtomicRefCount V4L2VideoEncodeAccelerator::num_instances_(0);
V4L2VideoEncodeAccelerator::V4L2VideoEncodeAccelerator(
scoped_refptr<V4L2Device> device)
: can_use_encoder_(num_instances_.Increment() < kMaxNumOfInstances),
child_task_runner_(base::SingleThreadTaskRunner::GetCurrentDefault()),
child_task_runner_(base::SequencedTaskRunner::GetCurrentDefault()),
native_input_mode_(false),
output_buffer_byte_size_(0),
output_format_fourcc_(0),
Expand Down Expand Up @@ -1542,7 +1543,7 @@ void V4L2VideoEncodeAccelerator::NotifyError(Error error) {
VLOGF(1) << "error=" << error;
DCHECK(child_task_runner_);

if (child_task_runner_->BelongsToCurrentThread()) {
if (child_task_runner_->RunsTasksInCurrentSequence()) {
if (client_) {
client_->NotifyError(error);
client_ptr_factory_.reset();
Expand Down
18 changes: 11 additions & 7 deletions media/gpu/v4l2/v4l2_video_encode_accelerator.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include "base/memory/scoped_refptr.h"
#include "base/memory/weak_ptr.h"
#include "base/sequence_checker.h"
#include "base/task/single_thread_task_runner.h"
#include "base/threading/thread.h"
#include "base/time/time.h"
#include "media/gpu/chromeos/image_processor.h"
Expand All @@ -28,6 +27,10 @@
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "ui/gfx/geometry/size.h"

namespace base {
class SequencedTaskRunner;
} // namespace base

namespace media {
class BitstreamBuffer;

Expand Down Expand Up @@ -173,7 +176,8 @@ class MEDIA_GPU_EXPORT V4L2VideoEncodeAccelerator
// Safe from any thread.
//

// Error notification (using PostTask() to child thread, if necessary).
// Error notification (using PostTask() to |child_task_runner_|, if
// necessary).
void NotifyError(Error error);

// Set the encoder_state_ to kError and notify the client (if necessary).
Expand Down Expand Up @@ -271,8 +275,8 @@ class MEDIA_GPU_EXPORT V4L2VideoEncodeAccelerator

std::string driver_name_;

// Our original calling task runner for the child thread and its checker.
const scoped_refptr<base::SingleThreadTaskRunner> child_task_runner_;
// Our original calling task runner for the child sequence and its checker.
const scoped_refptr<base::SequencedTaskRunner> child_task_runner_;
SEQUENCE_CHECKER(child_sequence_checker_);

// A coded_size() of VideoFrame on VEA::Encode(). This is updated on the first
Expand Down Expand Up @@ -350,10 +354,10 @@ class MEDIA_GPU_EXPORT V4L2VideoEncodeAccelerator
// Video frames for image processor output / VideoEncodeAccelerator input.
// Only accessed on child thread.
std::vector<scoped_refptr<VideoFrame>> image_processor_output_buffers_;
// Indexes of free image processor output buffers. Only accessed on child
// thread.
// Indexes of free image processor output buffers. Only accessed on
// |child_task_runner_|.
std::vector<size_t> free_image_processor_output_buffer_indices_;
// Video frames ready to be processed. Only accessed on child thread.
// Video frames ready to be processed. Only accessed on |child_task_runner_|.
base::queue<InputFrameInfo> image_processor_input_queue_;
// The number of frames that are being processed by |image_processor_|.
size_t num_frames_in_image_processor_ = 0;
Expand Down
5 changes: 3 additions & 2 deletions media/gpu/vaapi/vaapi_video_encode_accelerator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "base/memory/unsafe_shared_memory_region.h"
#include "base/numerics/safe_conversions.h"
#include "base/strings/stringprintf.h"
#include "base/task/sequenced_task_runner.h"
#include "base/task/single_thread_task_runner.h"
#include "base/task/task_traits.h"
#include "base/task/thread_pool.h"
Expand Down Expand Up @@ -113,7 +114,7 @@ VaapiVideoEncodeAccelerator::VaapiVideoEncodeAccelerator()
: can_use_encoder_(num_instances_.Increment() < kMaxNumOfInstances),
output_buffer_byte_size_(0),
state_(kUninitialized),
child_task_runner_(base::SingleThreadTaskRunner::GetCurrentDefault()),
child_task_runner_(base::SequencedTaskRunner::GetCurrentDefault()),
// TODO(akahuang): Change to use SequencedTaskRunner to see if the
// performance is affected.
encoder_task_runner_(base::ThreadPool::CreateSingleThreadTaskRunner(
Expand Down Expand Up @@ -1115,7 +1116,7 @@ void VaapiVideoEncodeAccelerator::SetState(State state) {
}

void VaapiVideoEncodeAccelerator::NotifyError(Error error) {
if (!child_task_runner_->BelongsToCurrentThread()) {
if (!child_task_runner_->RunsTasksInCurrentSequence()) {
child_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&VaapiVideoEncodeAccelerator::NotifyError,
child_weak_this_, error));
Expand Down
7 changes: 6 additions & 1 deletion media/gpu/vaapi/vaapi_video_encode_accelerator.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@
#include "media/gpu/vaapi/vaapi_wrapper.h"
#include "media/video/video_encode_accelerator.h"

namespace base {
class SequencedTaskRunner;
class SingleThreadTaskRunner;
} // namespace base

namespace media {

// A VideoEncodeAccelerator implementation that uses VA-API
Expand Down Expand Up @@ -289,7 +294,7 @@ class MEDIA_GPU_EXPORT VaapiVideoEncodeAccelerator
base::queue<std::unique_ptr<EncodeResult>> pending_encode_results_;

// Task runner for interacting with the client, and its checker.
const scoped_refptr<base::SingleThreadTaskRunner> child_task_runner_;
const scoped_refptr<base::SequencedTaskRunner> child_task_runner_;
SEQUENCE_CHECKER(child_sequence_checker_);

// Encoder sequence and its checker. All tasks are executed on it.
Expand Down
4 changes: 2 additions & 2 deletions media/mojo/services/mojo_video_encode_accelerator_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
#include <utility>

#include "base/memory/scoped_refptr.h"
#include "base/task/single_thread_task_runner.h"
#include "base/task/sequenced_task_runner.h"
#include "media/base/bind_to_current_loop.h"
#include "media/base/limits.h"
#include "media/gpu/gpu_video_encode_accelerator_factory.h"
Expand Down Expand Up @@ -41,7 +41,7 @@ void MojoVideoEncodeAcceleratorProvider::Create(
const gpu::GpuPreferences& gpu_preferences,
const gpu::GpuDriverBugWorkarounds& gpu_workarounds,
const gpu::GPUInfo::GPUDevice& gpu_device,
scoped_refptr<base::SingleThreadTaskRunner> runner) {
scoped_refptr<base::SequencedTaskRunner> runner) {
DCHECK(runner);
runner->PostTask(
FROM_HERE, base::BindOnce(BindVEAProvider, std::move(receiver),
Expand Down
4 changes: 2 additions & 2 deletions media/mojo/services/mojo_video_encode_accelerator_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
#include "mojo/public/cpp/bindings/pending_receiver.h"

namespace base {
class SingleThreadTaskRunner;
class SequencedTaskRunner;
}

namespace gpu {
Expand Down Expand Up @@ -46,7 +46,7 @@ class MEDIA_MOJO_EXPORT MojoVideoEncodeAcceleratorProvider
const gpu::GpuPreferences& gpu_preferences,
const gpu::GpuDriverBugWorkarounds& gpu_workarounds,
const gpu::GPUInfo::GPUDevice& gpu_device,
scoped_refptr<base::SingleThreadTaskRunner> runner);
scoped_refptr<base::SequencedTaskRunner> runner);

MojoVideoEncodeAcceleratorProvider(
CreateAndInitializeVideoEncodeAcceleratorCallback create_vea_callback,
Expand Down
9 changes: 3 additions & 6 deletions media/mojo/services/mojo_video_encode_accelerator_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
#include <utility>

#include "base/logging.h"
#include "base/task/single_thread_task_runner.h"
#include "base/task/sequenced_task_runner.h"
#include "base/trace_event/trace_event.h"
#include "media/base/bind_to_current_loop.h"
#include "media/base/bitstream_buffer.h"
Expand Down Expand Up @@ -69,11 +69,8 @@ void MojoVideoEncodeAcceleratorService::Initialize(
TRACE_EVENT1("media", "MojoVideoEncodeAcceleratorService::Initialize",
"config", config.AsHumanReadableString());

scoped_refptr<base::SingleThreadTaskRunner> task_runner =
base::SingleThreadTaskRunner::GetCurrentDefault();

media_log_ =
std::make_unique<MojoMediaLog>(std::move(media_log), task_runner);
media_log_ = std::make_unique<MojoMediaLog>(
std::move(media_log), base::SequencedTaskRunner::GetCurrentDefault());

if (gpu_workarounds_.disable_accelerated_vp8_encode &&
config.output_profile == VP8PROFILE_ANY) {
Expand Down

0 comments on commit 4abbb98

Please sign in to comment.