Moves cc::Surfaces for Videos onto its own thread.
Previously we were running the cc::Surface for Videos project on the media thread.
This was found to cause some smoothness regressions. This CL fixes that by pushing the
work onto its own thread.
TBR=gab@chromium.org
Bug: 866508
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ide12cb6cebc9dea83a7686c09e6688537bf3389a
Reviewed-on: https://chromium-review.googlesource.com/1176879
Commit-Queue: CJ DiMeglio <lethalantidote@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587811}diff --git a/base/threading/thread_restrictions.h b/base/threading/thread_restrictions.h
index 791f4eb..64a5e8f 100644
--- a/base/threading/thread_restrictions.h
+++ b/base/threading/thread_restrictions.h
@@ -23,6 +23,9 @@
namespace audio {
class OutputDevice;
}
+namespace blink {
+class VideoFrameResourceProvider;
+}
namespace cc {
class CompletionEvent;
class SingleThreadTaskGraphRunner;
@@ -330,6 +333,7 @@
LaunchXdgUtilityScopedAllowBaseSyncPrimitives;
friend class webrtc::DesktopConfigurationMonitor;
friend class content::ServiceWorkerSubresourceLoader;
+ friend class blink::VideoFrameResourceProvider;
ScopedAllowBaseSyncPrimitives() EMPTY_BODY_IF_DCHECK_IS_OFF;
~ScopedAllowBaseSyncPrimitives() EMPTY_BODY_IF_DCHECK_IS_OFF;
diff --git a/content/renderer/media/media_factory.cc b/content/renderer/media/media_factory.cc
index f94c128..ce032999 100644
--- a/content/renderer/media/media_factory.cc
+++ b/content/renderer/media/media_factory.cc
@@ -24,6 +24,7 @@
#include "content/renderer/render_frame_impl.h"
#include "content/renderer/render_thread_impl.h"
#include "content/renderer/render_view_impl.h"
+#include "media/base/bind_to_current_loop.h"
#include "media/base/cdm_factory.h"
#include "media/base/decoder_factory.h"
#include "media/base/media_switches.h"
@@ -100,38 +101,26 @@
DISALLOW_COPY_AND_ASSIGN(FrameFetchContext);
};
-void ObtainAndSetContextProvider(
- base::OnceCallback<void(bool,
- scoped_refptr<ws::ContextProviderCommandBuffer>)>
- set_context_provider_callback,
- std::pair<media::GpuVideoAcceleratorFactories*, bool> gpu_info) {
- if (gpu_info.first) {
- scoped_refptr<ws::ContextProviderCommandBuffer> context_provider =
- gpu_info.first->GetMediaContextProvider();
- std::move(set_context_provider_callback)
- .Run(gpu_info.second, std::move(context_provider));
- } else {
- std::move(set_context_provider_callback).Run(false, nullptr);
- }
-}
-
// Obtains the media ContextProvider and calls the given callback on the same
// thread this is called on. Obtaining the media ContextProvider requires
-// getting GPuVideoAcceleratorFactories, which must be done on the main
-// thread.
-void PostMediaContextProviderToCallback(
+// establishing a GPUChannelHost, which must be done on the main thread.
+void PostContextProviderToCallback(
scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
- base::OnceCallback<void(bool,
- scoped_refptr<ws::ContextProviderCommandBuffer>)>
- set_context_provider_callback) {
- base::PostTaskAndReplyWithResult(
- main_task_runner.get(), FROM_HERE, base::BindOnce([]() {
- return std::pair<media::GpuVideoAcceleratorFactories*, bool>(
- content::RenderThreadImpl::current()->GetGpuFactories(),
- !content::RenderThreadImpl::current()->IsGpuCompositingDisabled());
- }),
- base::BindOnce(&ObtainAndSetContextProvider,
- std::move(set_context_provider_callback)));
+ scoped_refptr<viz::ContextProvider> unwanted_context_provider,
+ blink::WebSubmitterConfigurationCallback set_context_provider_callback) {
+ main_task_runner->PostTask(
+ FROM_HERE,
+ base::BindOnce(
+ [](scoped_refptr<viz::ContextProvider> unwanted_context_provider,
+ blink::WebSubmitterConfigurationCallback cb) {
+ auto* rti = content::RenderThreadImpl::current();
+ auto context_provider = rti->GetVideoFrameCompositorContextProvider(
+ unwanted_context_provider);
+ std::move(cb).Run(!rti->IsGpuCompositingDisabled(),
+ std::move(context_provider));
+ },
+ std::move(unwanted_context_provider),
+ media::BindToCurrentLoop(std::move(set_context_provider_callback))));
}
} // namespace
@@ -307,12 +296,11 @@
std::unique_ptr<blink::WebVideoFrameSubmitter> submitter;
bool use_surface_layer_for_video = VideoSurfaceLayerEnabled();
if (use_surface_layer_for_video) {
- // TODO(lethalantidote): Use a separate task_runner. https://crbug/753605.
video_frame_compositor_task_runner =
- render_thread->GetMediaThreadTaskRunner();
+ render_thread->CreateVideoFrameCompositorTaskRunner();
submitter = blink::WebVideoFrameSubmitter::Create(
base::BindRepeating(
- &PostMediaContextProviderToCallback,
+ &PostContextProviderToCallback,
RenderThreadImpl::current()->GetCompositorMainThreadTaskRunner()),
settings);
} else {
diff --git a/content/renderer/render_thread_impl.cc b/content/renderer/render_thread_impl.cc
index 86833909..e0208d4 100644
--- a/content/renderer/render_thread_impl.cc
+++ b/content/renderer/render_thread_impl.cc
@@ -31,6 +31,7 @@
#include "base/strings/string_split.h"
#include "base/strings/sys_string_conversions.h"
#include "base/strings/utf_string_conversions.h"
+#include "base/task/post_task.h"
#include "base/threading/simple_thread.h"
#include "base/threading/thread_local.h"
#include "base/threading/thread_restrictions.h"
@@ -1191,6 +1192,21 @@
#endif
}
+scoped_refptr<base::SingleThreadTaskRunner>
+RenderThreadImpl::CreateVideoFrameCompositorTaskRunner() {
+ if (!video_frame_compositor_task_runner_) {
+ // All of Chromium's GPU code must know which thread it's running on, and
+ // be the same thread on which the rendering context was initialized. This
+ // is why this must be a SingleThreadTaskRunner instead of a
+ // SequencedTaskRunner.
+ video_frame_compositor_task_runner_ =
+ base::CreateSingleThreadTaskRunnerWithTraits(
+ {base::MayBlock(), base::TaskPriority::USER_VISIBLE});
+ }
+
+ return video_frame_compositor_task_runner_;
+}
+
void RenderThreadImpl::InitializeWebKit(
service_manager::BinderRegistry* registry) {
DCHECK(!blink_platform_impl_);
@@ -1434,6 +1450,39 @@
return gpu_factories_.back().get();
}
+scoped_refptr<viz::ContextProvider>
+RenderThreadImpl::GetVideoFrameCompositorContextProvider(
+ scoped_refptr<viz::ContextProvider> unwanted_context_provider) {
+ if (video_frame_compositor_context_provider_ &&
+ video_frame_compositor_context_provider_ != unwanted_context_provider) {
+ return video_frame_compositor_context_provider_;
+ }
+
+ video_frame_compositor_context_provider_ = nullptr;
+
+ scoped_refptr<gpu::GpuChannelHost> gpu_channel_host =
+ EstablishGpuChannelSync();
+ if (!gpu_channel_host)
+ return nullptr;
+
+ // This context is only used to create textures and mailbox them, so
+ // use lower limits than the default.
+ gpu::SharedMemoryLimits limits = gpu::SharedMemoryLimits::ForMailboxContext();
+
+ bool support_locking = false;
+ bool support_gles2_interface = true;
+ bool support_raster_interface = false;
+ bool support_oop_rasterization = false;
+ bool support_grcontext = false;
+ video_frame_compositor_context_provider_ = CreateOffscreenContext(
+ gpu_channel_host, GetGpuMemoryBufferManager(), limits, support_locking,
+ support_gles2_interface, support_raster_interface,
+ support_oop_rasterization, support_grcontext,
+ ws::command_buffer_metrics::ContextType::RENDER_COMPOSITOR,
+ kGpuStreamIdMedia, kGpuStreamPriorityMedia);
+ return video_frame_compositor_context_provider_;
+}
+
scoped_refptr<ws::ContextProviderCommandBuffer>
RenderThreadImpl::SharedMainThreadContextProvider() {
DCHECK(IsMainThread());
diff --git a/content/renderer/render_thread_impl.h b/content/renderer/render_thread_impl.h
index b14c1b9..61d7956 100644
--- a/content/renderer/render_thread_impl.h
+++ b/content/renderer/render_thread_impl.h
@@ -392,6 +392,13 @@
// A TaskRunner instance that runs tasks on the raster worker pool.
base::TaskRunner* GetWorkerTaskRunner();
+ // Creates a ContextProvider if yet created, and returns it to be used for
+ // video frame compositing. The ContextProvider given as an argument is
+ // one that has been lost, and is a hint to the RenderThreadImpl to clear
+ // it's |video_frame_compositor_context_provider_| if it matches.
+ scoped_refptr<viz::ContextProvider> GetVideoFrameCompositorContextProvider(
+ scoped_refptr<viz::ContextProvider>);
+
// Returns a worker context provider that will be bound on the compositor
// thread.
scoped_refptr<viz::RasterContextProvider>
@@ -508,6 +515,9 @@
// Sets the current pipeline rendering color space.
void SetRenderingColorSpace(const gfx::ColorSpace& color_space);
+ scoped_refptr<base::SingleThreadTaskRunner>
+ CreateVideoFrameCompositorTaskRunner();
+
private:
void OnProcessFinalRelease() override;
// IPC::Listener
@@ -676,6 +686,10 @@
// regardless of whether |compositor_thread_| is overriden.
scoped_refptr<base::SingleThreadTaskRunner> compositor_task_runner_;
+ // Task to run the VideoFrameCompositor on.
+ scoped_refptr<base::SingleThreadTaskRunner>
+ video_frame_compositor_task_runner_;
+
// Pool of workers used for raster operations (e.g., tile rasterization).
scoped_refptr<CategorizedWorkerPool> categorized_worker_pool_;
@@ -687,6 +701,8 @@
base::ObserverList<RenderThreadObserver>::Unchecked observers_;
+ scoped_refptr<viz::ContextProvider> video_frame_compositor_context_provider_;
+
scoped_refptr<viz::RasterContextProvider> shared_worker_context_provider_;
std::unique_ptr<AudioRendererMixerManager> audio_renderer_mixer_manager_;
diff --git a/third_party/blink/public/platform/web_video_frame_submitter.h b/third_party/blink/public/platform/web_video_frame_submitter.h
index e4110cd..a6a61ed 100644
--- a/third_party/blink/public/platform/web_video_frame_submitter.h
+++ b/third_party/blink/public/platform/web_video_frame_submitter.h
@@ -14,17 +14,20 @@
class LayerTreeSettings;
}
-namespace ws {
-class ContextProviderCommandBuffer;
-} // namespace ui
+namespace viz {
+class ContextProvider;
+} // namespace viz
namespace blink {
+// Sets the proper context_provider and compositing mode onto the Submitter.
+using WebSubmitterConfigurationCallback =
+ base::OnceCallback<void(bool, scoped_refptr<viz::ContextProvider>)>;
// Callback to obtain the media ContextProvider and a bool indicating whether
// we are in software compositing mode.
-using WebContextProviderCallback = base::RepeatingCallback<void(
- base::OnceCallback<void(bool,
- scoped_refptr<ws::ContextProviderCommandBuffer>)>)>;
+using WebContextProviderCallback =
+ base::RepeatingCallback<void(scoped_refptr<viz::ContextProvider>,
+ WebSubmitterConfigurationCallback)>;
using WebFrameSinkDestroyedCallback = base::RepeatingCallback<void()>;
// Exposes the VideoFrameSubmitter, which submits CompositorFrames containing
diff --git a/third_party/blink/renderer/platform/graphics/DEPS b/third_party/blink/renderer/platform/graphics/DEPS
index f8c18a1..11fa8ee 100644
--- a/third_party/blink/renderer/platform/graphics/DEPS
+++ b/third_party/blink/renderer/platform/graphics/DEPS
@@ -7,6 +7,7 @@
# Dependencies.
"+base/threading/sequenced_task_runner_handle.h",
+ "+base/threading/thread_restrictions.h",
"+cc",
"+components/viz/client",
"+components/viz/common",
diff --git a/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.cc b/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.cc
index 6a0023a..b5d35d00 100644
--- a/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.cc
+++ b/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.cc
@@ -6,6 +6,7 @@
#include <memory>
#include "base/bind.h"
+#include "base/threading/thread_restrictions.h"
#include "base/trace_event/trace_event.h"
#include "components/viz/client/client_resource_provider.h"
#include "components/viz/common/gpu/context_provider.h"
@@ -92,7 +93,11 @@
break;
}
+ // When obtaining frame resources, we end up having to wait. See
+ // https://crbug/878070.
+ base::ScopedAllowBaseSyncPrimitives allow_base_sync_primitives;
resource_updater_->ObtainFrameResources(frame);
+
// TODO(lethalantidote) : update with true value;
gfx::Rect visible_layer_rect = gfx::Rect(rotated_size);
gfx::Rect clip_rect = gfx::Rect(frame->coded_size());
diff --git a/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.h b/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.h
index b689026..7eda79e5 100644
--- a/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.h
+++ b/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.h
@@ -56,7 +56,6 @@
private:
const cc::LayerTreeSettings settings_;
- WebContextProviderCallback context_provider_callback_;
viz::ContextProvider* context_provider_;
std::unique_ptr<viz::ClientResourceProvider> resource_provider_;
std::unique_ptr<media::VideoResourceUpdater> resource_updater_;
diff --git a/third_party/blink/renderer/platform/graphics/video_frame_submitter.cc b/third_party/blink/renderer/platform/graphics/video_frame_submitter.cc
index f45e670..e609f16 100644
--- a/third_party/blink/renderer/platform/graphics/video_frame_submitter.cc
+++ b/third_party/blink/renderer/platform/graphics/video_frame_submitter.cc
@@ -25,7 +25,8 @@
namespace {
// Delay to retry getting the context_provider.
-constexpr int kGetContextProviderRetryMS = 150;
+constexpr base::TimeDelta kGetContextProviderRetryTimeout =
+ base::TimeDelta::FromMilliseconds(150);
} // namespace
@@ -162,33 +163,45 @@
DCHECK(!provider_);
provider_ = provider;
context_provider_callback_.Run(
- base::BindOnce(&VideoFrameSubmitter::OnReceivedContextProvider,
- weak_ptr_factory_.GetWeakPtr()));
+ nullptr, base::BindOnce(&VideoFrameSubmitter::OnReceivedContextProvider,
+ weak_ptr_factory_.GetWeakPtr()));
}
}
void VideoFrameSubmitter::OnReceivedContextProvider(
bool use_gpu_compositing,
- scoped_refptr<ws::ContextProviderCommandBuffer> context_provider) {
- // We could get a null |context_provider| back if the context is still lost.
- if (!context_provider && use_gpu_compositing) {
- base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
- FROM_HERE,
- base::BindOnce(
- context_provider_callback_,
- base::BindOnce(&VideoFrameSubmitter::OnReceivedContextProvider,
- weak_ptr_factory_.GetWeakPtr())),
- base::TimeDelta::FromMilliseconds(kGetContextProviderRetryMS));
+ scoped_refptr<viz::ContextProvider> context_provider) {
+ if (!use_gpu_compositing) {
+ resource_provider_->Initialize(nullptr, this);
return;
}
- context_provider_ = std::move(context_provider);
- if (use_gpu_compositing) {
- context_provider_->AddObserver(this);
- resource_provider_->Initialize(context_provider_.get(), nullptr);
- } else {
- resource_provider_->Initialize(nullptr, this);
+ bool has_good_context = false;
+ while (!has_good_context) {
+ if (!context_provider) {
+ base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+ FROM_HERE,
+ base::BindOnce(
+ context_provider_callback_, context_provider_,
+ base::BindOnce(&VideoFrameSubmitter::OnReceivedContextProvider,
+ weak_ptr_factory_.GetWeakPtr())),
+ kGetContextProviderRetryTimeout);
+ return;
+ }
+
+ // Note that |context_provider| is now null after the move, such that if we
+ // end up having !|has_good_context|, we will retry to obtain the
+ // context_provider.
+ context_provider_ = std::move(context_provider);
+ auto result = context_provider_->BindToCurrentThread();
+
+ has_good_context =
+ result == gpu::ContextResult::kSuccess ||
+ context_provider_->ContextGL()->GetGraphicsResetStatusKHR() ==
+ GL_NO_ERROR;
}
+ context_provider_->AddObserver(this);
+ resource_provider_->Initialize(context_provider_.get(), nullptr);
if (frame_sink_id_.is_valid())
StartSubmitting();
@@ -339,7 +352,6 @@
if (context_provider_) {
context_provider_->RemoveObserver(this);
- context_provider_ = nullptr;
}
waiting_for_compositor_ack_ = false;
@@ -349,6 +361,7 @@
compositor_frame_sink_.reset();
context_provider_callback_.Run(
+ context_provider_,
base::BindOnce(&VideoFrameSubmitter::OnReceivedContextProvider,
weak_ptr_factory_.GetWeakPtr()));
diff --git a/third_party/blink/renderer/platform/graphics/video_frame_submitter.h b/third_party/blink/renderer/platform/graphics/video_frame_submitter.h
index 6ef7e42..d138494 100644
--- a/third_party/blink/renderer/platform/graphics/video_frame_submitter.h
+++ b/third_party/blink/renderer/platform/graphics/video_frame_submitter.h
@@ -47,9 +47,7 @@
return &binding_;
}
- void OnReceivedContextProvider(
- bool,
- scoped_refptr<ws::ContextProviderCommandBuffer>);
+ void OnReceivedContextProvider(bool, scoped_refptr<viz::ContextProvider>);
// cc::VideoFrameProvider::Client implementation.
void StopUsingProvider() override;
@@ -118,7 +116,7 @@
bool ShouldSubmit() const;
cc::VideoFrameProvider* provider_ = nullptr;
- scoped_refptr<ws::ContextProviderCommandBuffer> context_provider_;
+ scoped_refptr<viz::ContextProvider> context_provider_;
viz::mojom::blink::CompositorFrameSinkPtr compositor_frame_sink_;
mojo::Binding<viz::mojom::blink::CompositorFrameSinkClient> binding_;
WebContextProviderCallback context_provider_callback_;
diff --git a/third_party/blink/renderer/platform/graphics/video_frame_submitter_test.cc b/third_party/blink/renderer/platform/graphics/video_frame_submitter_test.cc
index f2d2eab..883e2ec 100644
--- a/third_party/blink/renderer/platform/graphics/video_frame_submitter_test.cc
+++ b/third_party/blink/renderer/platform/graphics/video_frame_submitter_test.cc
@@ -148,8 +148,9 @@
context_provider_.get(), nullptr);
submitter_ = std::make_unique<VideoFrameSubmitter>(
base::BindRepeating(
- [](base::OnceCallback<void(
- bool, scoped_refptr<ws::ContextProviderCommandBuffer>)>) {}),
+ [](scoped_refptr<viz::ContextProvider>,
+ base::OnceCallback<void(
+ bool, scoped_refptr<viz::ContextProvider>)>) {}),
base::WrapUnique<MockVideoFrameResourceProvider>(resource_provider_));
submitter_->Initialize(provider_.get());
