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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.

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

CL description needs update.

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

Done.

 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/common/gpu/media/dxva_video_decode_accelerator.h"
 
 #if !defined(OS_WIN)
 #error This file should only be built on Windows.
 #endif   // !defined(OS_WIN)
 
 #include <ks.h>
@@ skipping 54 matching lines @@
       StopOnError(error_code);                                      \
       return ret;                                                   \
     }                                                               \
   } while (0)
 
 #define RETURN_AND_NOTIFY_ON_HR_FAILURE(result, log, error_code, ret)  \
   RETURN_AND_NOTIFY_ON_FAILURE(SUCCEEDED(result),                      \
                                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());
   RETURN_ON_HR_FAILURE(hr, "MFCreateSample failed", NULL);
   return sample.Detach();
 }
 
 // Creates a Media Foundation sample with one buffer of length |buffer_length|
 // on a |align|-byte boundary. Alignment must be a perfect power of 2 or 0.
 static IMFSample* CreateEmptySampleWithBuffer(int buffer_length, int align) {
@@ skipping 303 matching lines @@
                               NULL,
                               d3d_surface,
                               NULL,
                               D3DTEXF_NONE);
     RETURN_ON_HR_FAILURE(hr, "Colorspace conversion via StretchRect failed",
                          false);
     // Ideally, this should be done immediately before the draw call that uses
     // 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_,
         EGL_BACK_BUFFER);
   } else {
     scoped_array<char> bits;
     RETURN_ON_FAILURE(GetBitmapFromSurface(DXVAVideoDecodeAccelerator::device_,
                                            dest_surface, &bits),
                       "Failed to get bitmap from surface for rendering",
                       false);
@@ skipping 724 matching lines @@
       return;
   }
 
   MessageLoop::current()->PostTask(FROM_HERE, base::Bind(
       &DXVAVideoDecodeAccelerator::NotifyFlushDone, base::AsWeakPtr(this)));
 
   state_ = kNormal;
 }
 
 }  // namespace content