Don't wait endlessly for flushing the batched queries sent to the GPU driver during H/W decode

content/common/gpu/media/dxva_video_decode_accelerator.cc

Index: content/common/gpu/media/dxva_video_decode_accelerator.cc
===================================================================
--- content/common/gpu/media/dxva_video_decode_accelerator.cc	(revision 167469)
+++ content/common/gpu/media/dxva_video_decode_accelerator.cc	(working copy)
@@ -73,6 +73,10 @@
                                log << ", HRESULT: 0x" << std::hex << result, \
                                error_code, ret);
 
+// Maximum number of iterations we allow before aborting the attempt to flush
+// the batched queries to the driver.
+enum { kMaxIterationsForD3DFlush = 10 };
+
 static IMFSample* CreateEmptySample() {
   base::win::ScopedComPtr<IMFSample> sample;
   HRESULT hr = MFCreateSample(sample.Receive());
@@ -396,11 +400,42 @@
     // the texture. Flush it once here though.
     hr = query_->Issue(D3DISSUE_END);
     RETURN_ON_HR_FAILURE(hr, "Failed to issue END", false);
-    do {
-      hr = query_->GetData(NULL, 0, D3DGETDATA_FLUSH);
-      if (hr == S_FALSE)
-        Sleep(1);  // Poor-man's Yield().
-    } while (hr == S_FALSE);
+
+    // The DXVA decoder has its own device which it uses for decoding. ANGLE
+    // has its own device which we don't have access to.
+    // The above code attempts to copy the decoded picture into a surface
+    // which is owned by ANGLE. As there are multiple devices involved in
+    // this, the StretchRect call above is not synchronous.
+    // We attempt to flush the batched operations to ensure that the picture is
+    // copied to the surface owned by ANGLE.
+    // We need to do this in a loop and call flush multiple times.
+    // However we have seen the GetData call for flushing the operations fail
+    // to return success on some intel/ATI GPU drivers thus resulting in an
+    // infinite loop.
+    // Workaround is to have a limit of 10 on the number of iterations for
+    // flushing and if we reach the upper limit, we force a readback from
+    // the ANGLE surface to a temporary surface created in system memory which
+    // ensures that the pending operations on the ANGLE surface complete.

[Ami GONE FROM CHROMIUM, 2012/11/14 01:26:07]

s/complete/completes/

[ananta, 2012/11/14 03:05:42]

Rephrased

+    int iterations = 0;
+    while ((query_->GetData(NULL, 0, D3DGETDATA_FLUSH) == S_FALSE) &&
+           ++iterations < kMaxIterationsForD3DFlush) {
+      Sleep(1);  // Poor-man's Yield().
+    }
+
+    if (iterations > kMaxIterationsForD3DFlush) {

[Ami GONE FROM CHROMIUM, 2012/11/14 01:26:07]

This can never be true.

[ananta, 2012/11/14 03:05:42]

This code has been removed in favor of the BeginFrame/EndFrame calls
encapsulating StretchRect. This is safe because as per docs StretchRect will
only fail in a BeginFrame/EndFrame sequence for depth/stencil surfaces.

+      base::win::ScopedComPtr<IDirect3DSurface9> temp_surface;
+      hr = device_->CreateOffscreenPlainSurface(surface_desc.Width,

[Ami GONE FROM CHROMIUM, 2012/11/14 01:26:07]

IIUC systems that have this problem exhibit it for every frame.

So this patch will cause each frame to allocate a new texture, copy to it
synchronously, and then discard it?
That sounds like it might be less efficient than simply doing the decode in SW
in the first place.  If that's the case, then maybe instead of this CL we should
simply blacklist the offending driver/card?

[ananta, 2012/11/14 03:05:42]

Removed.

+                                                surface_desc.Height,
+                                                D3DFMT_X8R8G8B8,
+                                                D3DPOOL_SYSTEMMEM,
+                                                temp_surface.Receive(),
+                                                NULL);
+      RETURN_ON_HR_FAILURE(hr, "Failed to create system offscreen surface",
+                           false);
+      hr = device_->GetRenderTargetData(d3d_surface, temp_surface);

[apatrick_chromium, 2012/11/14 01:27:49]

This call could be asynchronous as well (don't know if it is in practice). I
think you might have to also lock the temporary surface with LockRect. It could
be a read-only lock.

[ananta, 2012/11/14 03:05:42]

Code has been removed.

+      RETURN_ON_HR_FAILURE(hr, "Failed to render decoded data to system memory",
+                           false);
+    }
     eglBindTexImage(
         static_cast<EGLDisplay*>(eglGetDisplay(EGL_DEFAULT_DISPLAY)),
         decoding_surface_,