Video Effects: Fix too-eager VideoEfectsProcessor resetting GPU state

GPU state is shared across Video Effects Processors and should ideally
be maintained by VideoEffectsServiceImpl. Processor should not have to
manage it, given that it does not have full access to the GPU state
(e.g. WebGpuDevice is also shared).

Plus a couple other fixes like explicitly using public inheritance and
turning GpuChannelHostProvider::Observer into a pure-virtual interface.

Change-Id: I17eae1451547aa3f5bc4cc94eec3406d899151e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6173152
Commit-Queue: Piotr Bialecki <bialpio@chromium.org>
Reviewed-by: Mark Foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1407658}

services/video_effects/gpu_channel_host_provider.h

@@ -30,11 +30,11 @@ class GpuChannelHostProvider : public base::RefCounted<GpuChannelHostProvider> {
   class Observer : public base::CheckedObserver {
    public:
     // The GPU context was lost.
-    virtual void OnContextLost(scoped_refptr<GpuChannelHostProvider>) {}
+    virtual void OnContextLost(scoped_refptr<GpuChannelHostProvider>) = 0;
 
     // Abandon ship! The GPU context has been lost multiple times and no further
     // attempts will be made to re-establish a connection to the GPU.
-    virtual void OnPermanentError(scoped_refptr<GpuChannelHostProvider>) {}
+    virtual void OnPermanentError(scoped_refptr<GpuChannelHostProvider>) = 0;
   };
 
   // Returns the context provider for WebGPU.
@@ -49,10 +49,6 @@ class GpuChannelHostProvider : public base::RefCounted<GpuChannelHostProvider> {
   virtual scoped_refptr<gpu::ClientSharedImageInterface>
   GetSharedImageInterface() = 0;
 
-  // Drop references to internal context objects.  Subsequent calls to the Get*
-  // methods will return references to new context objects.
-  virtual void Reset() = 0;
-
   virtual void AddObserver(Observer& observer) = 0;
   virtual void RemoveObserver(Observer& observer) = 0;
 

services/video_effects/test_gpu_channel_host_provider.cc

@@ -56,7 +56,6 @@ TestGpuChannelHostProvider::GetSharedImageInterface() {
   return nullptr;
 }
 
-void TestGpuChannelHostProvider::Reset() {}
 void TestGpuChannelHostProvider::AddObserver(Observer&) {}
 void TestGpuChannelHostProvider::RemoveObserver(Observer&) {}
 

services/video_effects/test_gpu_channel_host_provider.h

@@ -24,7 +24,7 @@ class TestGpuChannelHostProvider : public GpuChannelHostProvider {
       override;
   scoped_refptr<gpu::ClientSharedImageInterface> GetSharedImageInterface()
       override;
-  void Reset() override;
+
   void AddObserver(Observer& observer) override;
   void RemoveObserver(Observer& observer) override;
 

services/video_effects/video_effects_processor_impl.cc

@@ -131,7 +131,6 @@ void VideoEffectsProcessorImpl::OnMojoDisconnected() {
   processor_receiver_.reset();
   configuration_observer_.reset();
   manager_remote_.reset();
-  gpu_channel_host_provider_->Reset();
 
   MaybeCallOnUnrecoverableError();
 }

services/video_effects/video_effects_processor_webgpu.cc

@@ -39,11 +39,11 @@ namespace {
 scoped_refptr<gpu::ClientSharedImage> CreateSharedImageRGBA(
     gpu::SharedImageInterface* sii,
     const media::mojom::VideoFrameInfo& frame_info,
-    gpu::SharedImageUsageSet gpu_usage) {
+    gpu::SharedImageUsageSet gpu_usage,
+    std::string_view debug_label) {
   scoped_refptr<gpu::ClientSharedImage> destination = sii->CreateSharedImage(
       {viz::SinglePlaneFormat::kRGBA_8888, frame_info.coded_size,
-       frame_info.color_space, gpu_usage,
-       "VideoEffectsProcessorIntermediateSharedImage"},
+       frame_info.color_space, gpu_usage, debug_label},
       gpu::kNullSurfaceHandle);
   CHECK(destination);
   CHECK(!destination->mailbox().IsZero());
@@ -252,7 +252,8 @@ void VideoEffectsProcessorWebGpu::PostProcess(
   auto in_image =
       CreateSharedImageRGBA(shared_image_interface_.get(), *input_frame_info,
                             gpu::SHARED_IMAGE_USAGE_WEBGPU_READ |
-                                gpu::SHARED_IMAGE_USAGE_RASTER_WRITE);
+                                gpu::SHARED_IMAGE_USAGE_RASTER_WRITE,
+                            "VideoEffectsProcessorInImage");
   // t3=GenSyncToken()
   // Waiting on this sync token should ensure that the `in_image` shared image
   // is ready to be used.
@@ -270,7 +271,8 @@ void VideoEffectsProcessorWebGpu::PostProcess(
       CreateSharedImageRGBA(shared_image_interface_.get(), *input_frame_info,
                             gpu::SHARED_IMAGE_USAGE_WEBGPU_WRITE |
                                 gpu::SHARED_IMAGE_USAGE_RASTER_READ |
-                                gpu::SHARED_IMAGE_USAGE_WEBGPU_STORAGE_TEXTURE);
+                                gpu::SHARED_IMAGE_USAGE_WEBGPU_STORAGE_TEXTURE,
+                            "VideoEffectsProcessorOutImage");
   // t4=GenSyncToken()
   // Waiting on this sync token should ensure that the `out_image` shared image
   // is ready to be used.

services/video_effects/video_effects_service_impl.cc

@@ -88,6 +88,12 @@ void VideoEffectsServiceImpl::OnPermanentError(
   // and cleans up any related state.
 }
 
+void VideoEffectsServiceImpl::OnContextLost(
+    scoped_refptr<GpuChannelHostProvider>) {
+  // Nothing to do - the video effects processors also get notified about
+  // context losses - they will reinitialize their GPU state themselves.
+}
+
 void VideoEffectsServiceImpl::CreateWebGpuDeviceAndEffectsProcessors() {
   CHECK(!webgpu_device_);
   CHECK(gpu_channel_host_provider_);

services/video_effects/video_effects_service_impl.h

@@ -31,7 +31,7 @@ namespace video_effects {
 class VideoEffectsProcessorImpl;
 
 class VideoEffectsServiceImpl : public mojom::VideoEffectsService,
-                                GpuChannelHostProvider::Observer {
+                                public GpuChannelHostProvider::Observer {
  public:
   // Similarly to `VideoCaptureServiceImpl`, `VideoEfffectsServiceImpl` needs
   // to receive something that returns `gpu::GpuChannelHost` instances in order
@@ -57,6 +57,7 @@ class VideoEffectsServiceImpl : public mojom::VideoEffectsService,
  private:
   // GpuChannelHostProvider::Observer:
   void OnPermanentError(scoped_refptr<GpuChannelHostProvider>) override;
+  void OnContextLost(scoped_refptr<GpuChannelHostProvider>) override;
 
   // Creates `webgpu_device_` and initializes it asynchronously.  On completion,
   // invokes `FinishCreatingEffectsProcessors()`.

services/video_effects/viz_gpu_channel_host_provider.h

@@ -15,8 +15,8 @@
 namespace video_effects {
 
 class VizGpuChannelHostProvider : public GpuChannelHostProvider,
-                                  gpu::GpuChannelLostObserver,
-                                  viz::ContextLostObserver {
+                                  public gpu::GpuChannelLostObserver,
+                                  public viz::ContextLostObserver {
  public:
   explicit VizGpuChannelHostProvider(std::unique_ptr<viz::Gpu> viz_gpu);
 
@@ -27,7 +27,6 @@ class VizGpuChannelHostProvider : public GpuChannelHostProvider,
       override;
   scoped_refptr<gpu::ClientSharedImageInterface> GetSharedImageInterface()
       override;
-  void Reset() override;
   void AddObserver(Observer& observer) override;
   void RemoveObserver(Observer& observer) override;
 
@@ -36,6 +35,8 @@ class VizGpuChannelHostProvider : public GpuChannelHostProvider,
   scoped_refptr<gpu::GpuChannelHost> GetGpuChannelHost() override;
 
  private:
+  void Reset();
+
   // gpu::GpuChannelLostObserver:
   void OnGpuChannelLost() override;