Bug 1176071 - Handle WMF MFTDecoder returning success but no output, with telemetry. r=mattwoodrow,r=vladan

Sometimes the Windows Media Foundation MFT video decoder reports that it will
provide memory for output video frames, and the decoder returns success, but it
doesn't output a valid video frame. So change our code to not assume that
output is always valid (to fix a null pointer dereference). We can't reproduce
this locally, so we don't know how to get a best behaviour here, so add
telemetry to figure out whether the decoder will right itself, or whether we
should just give up completely.

--HG--
extra : amend_source : 7b5a51647593744f92e69db30764dcdadb3d7396
extra : histedit_source : dd72f96023f33cb7be4bfc5e5db6f67f18e61aca%2C305b8cf6ffdef0f2ab52a2ae03bd9457b123d669

dom/media/platforms/wmf/MFTDecoder.cpp

@@ -236,7 +236,9 @@ MFTDecoder::Output(RefPtr<IMFSample>* aOutput)
   // Treat other errors as unexpected, and warn.
   NS_ENSURE_TRUE(SUCCEEDED(hr), hr);
 
-  MOZ_ASSERT(output.pSample);
+  if (!output.pSample) {
+    return S_OK;
+  }
 
   if (mDiscontinuity) {
     output.pSample->SetUINT32(MFSampleExtension_Discontinuity, TRUE);

dom/media/platforms/wmf/WMFAudioMFTManager.cpp

@@ -10,7 +10,7 @@
 #include "WMFUtils.h"
 #include "nsTArray.h"
 #include "TimeUnits.h"
-
+#include "mozilla/Telemetry.h"
 #include "mozilla/Logging.h"
 
 extern mozilla::LogModule* GetPDMLog();
@@ -226,6 +226,16 @@ WMFAudioMFTManager::Output(int64_t aStreamOffset,
 
   NS_ENSURE_TRUE(SUCCEEDED(hr), hr);
 
+  if (!sample) {
+    LOG("Audio MFTDecoder returned success but null output.");
+    nsCOMPtr<nsIRunnable> task = NS_NewRunnableFunction([]() -> void {
+      LOG("Reporting telemetry AUDIO_MFT_OUTPUT_NULL_SAMPLES");
+      Telemetry::Accumulate(Telemetry::ID::AUDIO_MFT_OUTPUT_NULL_SAMPLES, 1);
+    });
+    AbstractThread::MainThread()->Dispatch(task.forget());
+    return E_FAIL;
+  }
+
   RefPtr<IMFMediaBuffer> buffer;
   hr = sample->ConvertToContiguousBuffer(getter_AddRefs(buffer));
   NS_ENSURE_TRUE(SUCCEEDED(hr), hr);

dom/media/platforms/wmf/WMFVideoMFTManager.cpp

@@ -21,6 +21,7 @@
 #include "IMFYCbCrImage.h"
 #include "mozilla/WindowsVersion.h"
 #include "mozilla/Preferences.h"
+#include "mozilla/Telemetry.h"
 #include "nsPrintfCString.h"
 
 extern mozilla::LogModule* GetPDMLog();
@@ -78,6 +79,9 @@ WMFVideoMFTManager::WMFVideoMFTManager(
   , mImageContainer(aImageContainer)
   , mDXVAEnabled(aDXVAEnabled)
   , mLayersBackend(aLayersBackend)
+  , mNullOutputCount(0)
+  , mGotValidOutputAfterNullOutput(false)
+  , mGotExcessiveNullOutput(false)
   // mVideoStride, mVideoWidth, mVideoHeight, mUseHwAccel are initialized in
   // Init().
 {
@@ -103,6 +107,20 @@ WMFVideoMFTManager::~WMFVideoMFTManager()
   if (mDXVA2Manager) {
     DeleteOnMainThread(mDXVA2Manager);
   }
+
+  // Record whether the video decoder successfully decoded, or output null
+  // samples but did/didn't recover.
+  uint32_t telemetry = (mNullOutputCount == 0) ? 0 :
+                       (mGotValidOutputAfterNullOutput && mGotExcessiveNullOutput) ? 1 :
+                       mGotExcessiveNullOutput ? 2 :
+                       mGotValidOutputAfterNullOutput ? 3 :
+                       4;
+
+  nsCOMPtr<nsIRunnable> task = NS_NewRunnableFunction([=]() -> void {
+    LOG(nsPrintfCString("Reporting telemetry VIDEO_MFT_OUTPUT_NULL_SAMPLES=%d", telemetry).get());
+    Telemetry::Accumulate(Telemetry::ID::VIDEO_MFT_OUTPUT_NULL_SAMPLES, telemetry);
+  });
+  AbstractThread::MainThread()->Dispatch(task.forget());
 }
 
 const GUID&
@@ -575,6 +593,23 @@ WMFVideoMFTManager::Output(int64_t aStreamOffset,
       continue;
     }
     if (SUCCEEDED(hr)) {
+      if (!sample) {
+        LOG("Video MFTDecoder returned success but no output!");
+        // On some machines/input the MFT returns success but doesn't output
+        // a video frame. If we detect this, try again, but only up to a
+        // point; after 250 failures, give up. Note we count all failures
+        // over the life of the decoder, as we may end up exiting with a
+        // NEED_MORE_INPUT and coming back to hit the same error. So just
+        // counting with a local variable (like typeChangeCount does) may
+        // not work in this situation.
+        ++mNullOutputCount;
+        if (mNullOutputCount > 250) {
+          LOG("Excessive Video MFTDecoder returning success but no output; giving up");
+          mGotExcessiveNullOutput = true;
+          return E_FAIL;
+        }
+        continue;
+      }
       break;
     }
     // Else unexpected error, assert, and bail.
@@ -595,6 +630,10 @@ WMFVideoMFTManager::Output(int64_t aStreamOffset,
 
   aOutData = frame;
 
+  if (mNullOutputCount) {
+    mGotValidOutputAfterNullOutput = true;
+  }
+
   return S_OK;
 }
 

dom/media/platforms/wmf/WMFVideoMFTManager.h

@@ -88,6 +88,10 @@ private:
 
   const GUID& GetMFTGUID();
   const GUID& GetMediaSubtypeGUID();
+
+  uint32_t mNullOutputCount;
+  bool mGotValidOutputAfterNullOutput;
+  bool mGotExcessiveNullOutput;
 };
 
 } // namespace mozilla

toolkit/components/telemetry/Histograms.json

@@ -6113,6 +6113,21 @@
     "description": "Whether Ogg audio/video encountered are chained or not.",
     "bug_numbers": [1230295]
   },
+  "VIDEO_MFT_OUTPUT_NULL_SAMPLES": {
+    "alert_emails": ["cpearce@mozilla.com"],
+    "expires_in_version": "53",
+    "kind": "enumerated",
+    "n_values": 10,
+    "description": "Does the WMF video decoder return success but null output? 0 = playback successful, 1 = excessive null output but able to decode some frames, 2 = excessive null output and gave up, 3 = null output but recovered, 4 = non-excessive null output without being able to decode frames.",
+    "bug_numbers": [1176071]
+  },
+  "AUDIO_MFT_OUTPUT_NULL_SAMPLES": {
+    "alert_emails": ["cpearce@mozilla.com"],
+    "expires_in_version": "53",
+    "kind": "count",
+    "description": "How many times the audio MFT decoder returns success but output nothing.",
+    "bug_numbers": [1176071]
+  },
   "VIDEO_CAN_CREATE_AAC_DECODER": {
     "alert_emails": ["cpearce@mozilla.com"],
     "expires_in_version": "50",