Fix gpu crash when switching from HDR to SDR video in Intel platform

When playing a SDR video after playing a HDR video, GPU process will
crash on Intel platform. The root cause is Chrome sets different color
space in VP without reset VP, which will cause something wrong happens
in VP.

This CL provides a fix that VP will reset when input/output color space
changes. What's more, color space will be set to VP only when it
changes, which reduces frequency of calling
VideoProcessorSetStream/*Output*/ColorSpace1()

Bug: 1121061
Change-Id: I096e5fb47593e0df034452719029a3750850bc77
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2371478
Commit-Queue: Richard Li <richard.li@intel.com>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805571}

gpu/config/gpu_driver_bug_list.json

@@ -3531,6 +3531,18 @@
       "features": [
         "disable_accelerated_vp8_decode"
       ]
+    },
+    {
+      "id": 350,
+      "cr_bugs": [1121061],
+      "description": "Intel GPUs need reset VP when colorspace changes",
+      "os": {
+        "type": "win"
+      },
+      "vendor_id": "0x8086",
+      "features": [
+        "reset_vp_when_colorspace_changes"
+      ]
     }
   ]
 }

gpu/config/gpu_workaround_list.txt

@@ -102,6 +102,7 @@ remove_invariant_and_centroid_for_essl3
 remove_pow_with_constant_exponent
 reset_base_mipmap_level_before_texstorage
 reset_teximage2d_base_level
+reset_vp_when_colorspace_changes
 restore_scissor_on_fbo_change
 rewrite_do_while_loops
 rewrite_float_unary_minus_operator

gpu/ipc/service/image_transport_surface_win.cc

@@ -32,6 +32,8 @@ CreateDirectCompositionSurfaceSettings(
       workarounds.disable_larger_than_screen_overlays;
   settings.disable_vp_scaling = workarounds.disable_vp_scaling;
   settings.use_angle_texture_offset = features::IsUsingSkiaRenderer();
+  settings.reset_vp_when_colorspace_changes =
+      workarounds.reset_vp_when_colorspace_changes;
   return settings;
 }
 }  // namespace

ui/gl/dc_layer_tree.cc

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <d3d11_1.h>
+
 #include "ui/gl/dc_layer_tree.h"
 
 #include "base/metrics/histogram_functions.h"
@@ -19,10 +21,12 @@ bool SizeContains(const gfx::Size& a, const gfx::Size& b) {
 
 DCLayerTree::DCLayerTree(bool disable_nv12_dynamic_textures,
                          bool disable_larger_than_screen_overlays,
-                         bool disable_vp_scaling)
+                         bool disable_vp_scaling,
+                         bool reset_vp_when_colorspace_changes)
     : disable_nv12_dynamic_textures_(disable_nv12_dynamic_textures),
       disable_larger_than_screen_overlays_(disable_larger_than_screen_overlays),
-      disable_vp_scaling_(disable_vp_scaling) {}
+      disable_vp_scaling_(disable_vp_scaling),
+      reset_vp_when_colorspace_changes_(reset_vp_when_colorspace_changes) {}
 
 DCLayerTree::~DCLayerTree() = default;
 
@@ -59,8 +63,11 @@ bool DCLayerTree::Initialize(
   return true;
 }
 
-bool DCLayerTree::InitializeVideoProcessor(const gfx::Size& input_size,
-                                           const gfx::Size& output_size) {
+bool DCLayerTree::InitializeVideoProcessor(
+    const gfx::Size& input_size,
+    const gfx::Size& output_size,
+    const gfx::ColorSpace& input_color_space,
+    const gfx::ColorSpace& output_color_space) {
   if (!video_device_) {
     // This can fail if the D3D device is "Microsoft Basic Display Adapter".
     if (FAILED(d3d11_device_.As(&video_device_))) {
@@ -78,9 +85,13 @@ bool DCLayerTree::InitializeVideoProcessor(const gfx::Size& input_size,
     DCHECK(video_context_);
   }
 
+  bool colorspace_changed = !(input_color_space == video_input_color_space_ &&
+                              output_color_space == video_output_color_space_);
   if (video_processor_ && SizeContains(video_input_size_, input_size) &&
-      SizeContains(video_output_size_, output_size))
+      SizeContains(video_output_size_, output_size) &&
+      !(colorspace_changed && reset_vp_when_colorspace_changes_)) {
     return true;
+  }
   TRACE_EVENT2("gpu", "DCLayerTree::InitializeVideoProcessor", "input_size",
                input_size.ToString(), "output_size", output_size.ToString());
   video_input_size_ = input_size;
@@ -124,6 +135,10 @@ bool DCLayerTree::InitializeVideoProcessor(const gfx::Size& input_size,
     DirectCompositionSurfaceWin::DisableOverlays();
     return false;
   }
+  // Color spaces should be updated later through
+  // SetColorspaceForVideoProcessor() for the new VP.
+  video_input_color_space_ = gfx::ColorSpace();
+  video_output_color_space_ = gfx::ColorSpace();
 
   // Auto stream processing (the default) can hurt power consumption.
   video_context_->VideoProcessorSetStreamAutoProcessingMode(
@@ -131,6 +146,56 @@ bool DCLayerTree::InitializeVideoProcessor(const gfx::Size& input_size,
   return true;
 }
 
+void DCLayerTree::SetColorspaceForVideoProcessor(
+    const gfx::ColorSpace& input_color_space,
+    const gfx::ColorSpace& output_color_space,
+    Microsoft::WRL::ComPtr<IDXGISwapChain1> swapchain,
+    bool is_yuv_swapchain) {
+  // Reset colorspace of video processor in two circumstances:
+  // 1. Input/output colorspaces change.
+  // 2. Swapchain changes.
+  // Others just return.
+  if ((input_color_space == video_input_color_space_) &&
+      (output_color_space == video_output_color_space_) &&
+      swapchain == last_swapchain_setting_colorspace_)
+    return;
+
+  Microsoft::WRL::ComPtr<IDXGISwapChain3> swap_chain3;
+  Microsoft::WRL::ComPtr<ID3D11VideoContext1> context1;
+  if (SUCCEEDED(swapchain.As(&swap_chain3)) &&
+      SUCCEEDED(video_context_.As(&context1))) {
+    DCHECK(swap_chain3);
+    DCHECK(context1);
+    // Set input color space.
+    context1->VideoProcessorSetStreamColorSpace1(
+        video_processor_.Get(), 0,
+        gfx::ColorSpaceWin::GetDXGIColorSpace(input_color_space));
+    // Set output color space.
+    DXGI_COLOR_SPACE_TYPE output_dxgi_color_space =
+        gfx::ColorSpaceWin::GetDXGIColorSpace(output_color_space,
+                                              is_yuv_swapchain /* force_yuv */);
+
+    if (SUCCEEDED(swap_chain3->SetColorSpace1(output_dxgi_color_space))) {
+      context1->VideoProcessorSetOutputColorSpace1(video_processor_.Get(),
+                                                   output_dxgi_color_space);
+    }
+    last_swapchain_setting_colorspace_ = swapchain;
+  } else {
+    // This can't handle as many different types of color spaces, so use it
+    // only if ID3D11VideoContext1 isn't available.
+    D3D11_VIDEO_PROCESSOR_COLOR_SPACE src_d3d11_color_space =
+        gfx::ColorSpaceWin::GetD3D11ColorSpace(input_color_space);
+    video_context_->VideoProcessorSetStreamColorSpace(video_processor_.Get(), 0,
+                                                      &src_d3d11_color_space);
+    D3D11_VIDEO_PROCESSOR_COLOR_SPACE output_d3d11_color_space =
+        gfx::ColorSpaceWin::GetD3D11ColorSpace(output_color_space);
+    video_context_->VideoProcessorSetOutputColorSpace(
+        video_processor_.Get(), &output_d3d11_color_space);
+  }
+  video_input_color_space_ = input_color_space;
+  video_output_color_space_ = output_color_space;
+}
+
 Microsoft::WRL::ComPtr<IDXGISwapChain1>
 DCLayerTree::GetLayerSwapChainForTesting(size_t index) const {
   if (index < video_swap_chains_.size())

ui/gl/dc_layer_tree.h

@@ -12,6 +12,7 @@
 
 #include <memory>
 
+#include "ui/gfx/color_space_win.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/gl/dc_renderer_layer_params.h"
 
@@ -28,7 +29,8 @@ class DCLayerTree {
  public:
   DCLayerTree(bool disable_nv12_dynamic_textures,
               bool disable_larger_than_screen_overlays,
-              bool disable_vp_scaling);
+              bool disable_vp_scaling,
+              bool reset_vp_when_colorspace_changes);
   ~DCLayerTree();
 
   // Returns true on success.
@@ -49,7 +51,15 @@ class DCLayerTree {
   // layers so the same one can be reused if it's large enough.  Returns true on
   // success.
   bool InitializeVideoProcessor(const gfx::Size& input_size,
-                                const gfx::Size& output_size);
+                                const gfx::Size& output_size,
+                                const gfx::ColorSpace& input_color_space,
+                                const gfx::ColorSpace& output_color_space);
+
+  void SetColorspaceForVideoProcessor(
+      const gfx::ColorSpace& input_color_space,
+      const gfx::ColorSpace& output_color_space,
+      Microsoft::WRL::ComPtr<IDXGISwapChain1> swapchain,
+      bool is_yuv_swapchain);
 
   void SetNeedsRebuildVisualTree() { needs_rebuild_visual_tree_ = true; }
 
@@ -89,6 +99,7 @@ class DCLayerTree {
   const bool disable_nv12_dynamic_textures_;
   const bool disable_larger_than_screen_overlays_;
   const bool disable_vp_scaling_;
+  const bool reset_vp_when_colorspace_changes_;
 
   Microsoft::WRL::ComPtr<ID3D11Device> d3d11_device_;
   Microsoft::WRL::ComPtr<IDCompositionDevice2> dcomp_device_;
@@ -106,6 +117,13 @@ class DCLayerTree {
   gfx::Size video_input_size_;
   gfx::Size video_output_size_;
 
+  // Current video processor input and output colorspace.
+  gfx::ColorSpace video_input_color_space_;
+  gfx::ColorSpace video_output_color_space_;
+
+  // Cache the last swapchain that has been set output colorspace.
+  Microsoft::WRL::ComPtr<IDXGISwapChain1> last_swapchain_setting_colorspace_;
+
   // Set to true if a direct composition visual tree needs rebuild.
   bool needs_rebuild_visual_tree_ = false;
 

ui/gl/direct_composition_surface_win.cc

@@ -335,7 +335,8 @@ DirectCompositionSurfaceWin::DirectCompositionSurfaceWin(
       layer_tree_(std::make_unique<DCLayerTree>(
           settings.disable_nv12_dynamic_textures,
           settings.disable_larger_than_screen_overlays,
-          settings.disable_vp_scaling)) {
+          settings.disable_vp_scaling,
+          settings.reset_vp_when_colorspace_changes)) {
   ui::GpuSwitchingManager::GetInstance()->AddObserver(this);
 }
 

ui/gl/direct_composition_surface_win.h

@@ -35,6 +35,7 @@ class GL_EXPORT DirectCompositionSurfaceWin : public GLSurfaceEGL,
     bool disable_vp_scaling = false;
     size_t max_pending_frames = 2;
     bool use_angle_texture_offset = false;
+    bool reset_vp_when_colorspace_changes = false;
   };
 
   DirectCompositionSurfaceWin(

ui/gl/swap_chain_presenter.cc

@@ -973,19 +973,10 @@ bool SwapChainPresenter::VideoProcessorBlt(
   TRACE_EVENT2("gpu", "SwapChainPresenter::VideoProcessorBlt", "content_rect",
                content_rect.ToString(), "swap_chain_size",
                swap_chain_size_.ToString());
-  if (!layer_tree_->InitializeVideoProcessor(content_rect.size(),
-                                             swap_chain_size_)) {
-    return false;
-  }
-  Microsoft::WRL::ComPtr<ID3D11VideoContext> video_context =
-      layer_tree_->video_context();
-  Microsoft::WRL::ComPtr<ID3D11VideoProcessor> video_processor =
-      layer_tree_->video_processor();
 
   gfx::ColorSpace output_color_space = IsYUVSwapChainFormat(swap_chain_format_)
                                            ? src_color_space
                                            : gfx::ColorSpace::CreateSRGB();
-
   if (base::FeatureList::IsEnabled(kFallbackBT709VideoToBT601) &&
       (output_color_space == gfx::ColorSpace::CreateREC709())) {
     output_color_space = gfx::ColorSpace::CreateREC601();
@@ -993,38 +984,14 @@ bool SwapChainPresenter::VideoProcessorBlt(
   if (content_is_hdr)
     output_color_space = gfx::ColorSpace::CreateHDR10();
 
-  Microsoft::WRL::ComPtr<IDXGISwapChain3> swap_chain3;
-  Microsoft::WRL::ComPtr<ID3D11VideoContext1> context1;
-  if (SUCCEEDED(swap_chain_.As(&swap_chain3)) &&
-      SUCCEEDED(video_context.As(&context1))) {
-    DCHECK(swap_chain3);
-    DCHECK(context1);
-    // Set input color space.
-    context1->VideoProcessorSetStreamColorSpace1(
-        video_processor.Get(), 0,
-        gfx::ColorSpaceWin::GetDXGIColorSpace(src_color_space));
-    // Set output color space.
-    DXGI_COLOR_SPACE_TYPE output_dxgi_color_space =
-        gfx::ColorSpaceWin::GetDXGIColorSpace(
-            output_color_space,
-            IsYUVSwapChainFormat(swap_chain_format_) /* force_yuv */);
-
-    if (SUCCEEDED(swap_chain3->SetColorSpace1(output_dxgi_color_space))) {
-      context1->VideoProcessorSetOutputColorSpace1(video_processor.Get(),
-                                                   output_dxgi_color_space);
-    }
-  } else {
-    // This can't handle as many different types of color spaces, so use it
-    // only if ID3D11VideoContext1 isn't available.
-    D3D11_VIDEO_PROCESSOR_COLOR_SPACE src_d3d11_color_space =
-        gfx::ColorSpaceWin::GetD3D11ColorSpace(src_color_space);
-    video_context->VideoProcessorSetStreamColorSpace(video_processor.Get(), 0,
-                                                     &src_d3d11_color_space);
-    D3D11_VIDEO_PROCESSOR_COLOR_SPACE output_d3d11_color_space =
-        gfx::ColorSpaceWin::GetD3D11ColorSpace(output_color_space);
-    video_context->VideoProcessorSetOutputColorSpace(video_processor.Get(),
-                                                     &output_d3d11_color_space);
+  if (!layer_tree_->InitializeVideoProcessor(content_rect.size(),
+                                             swap_chain_size_, src_color_space,
+                                             output_color_space)) {
+    return false;
   }
+  layer_tree_->SetColorspaceForVideoProcessor(
+      src_color_space, output_color_space, swap_chain_,
+      IsYUVSwapChainFormat(swap_chain_format_));
 
   {
     base::Optional<ScopedReleaseKeyedMutex> release_keyed_mutex;
@@ -1062,6 +1029,10 @@ bool SwapChainPresenter::VideoProcessorBlt(
       return false;
     }
 
+    Microsoft::WRL::ComPtr<ID3D11VideoContext> video_context =
+        layer_tree_->video_context();
+    Microsoft::WRL::ComPtr<ID3D11VideoProcessor> video_processor =
+        layer_tree_->video_processor();
     D3D11_VIDEO_PROCESSOR_STREAM stream = {};
     stream.Enable = true;
     stream.OutputIndex = 0;