Bug 1176071 - Handle WMF MFTDecoder returning success but no output, with telemetry. r?mattwoodrow,r?vladan draft
authorChris Pearce <cpearce@mozilla.com>
Tue, 19 Jan 2016 18:36:02 +1300
changeset 322561 97d861506572b645003f4d18bd5b60ac1e21eff5
parent 322386 a77b73c7723e1060993045fb31eb2f0a30473486
child 513131 aaf040cfaeec62376638ca794c740b2189917222
push id9634
push usercpearce@mozilla.com
push dateTue, 19 Jan 2016 05:51:17 +0000
reviewersmattwoodrow, vladan
bugs1176071
milestone46.0a1
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.
dom/media/platforms/wmf/MFTDecoder.cpp
dom/media/platforms/wmf/WMFAudioMFTManager.cpp
dom/media/platforms/wmf/WMFVideoMFTManager.cpp
dom/media/platforms/wmf/WMFVideoMFTManager.h
toolkit/components/telemetry/Histograms.json
--- a/dom/media/platforms/wmf/MFTDecoder.cpp
+++ b/dom/media/platforms/wmf/MFTDecoder.cpp
@@ -231,17 +231,19 @@ MFTDecoder::Output(RefPtr<IMFSample>* aO
   if (hr == MF_E_TRANSFORM_NEED_MORE_INPUT) {
     // Not enough input to produce output. This is an expected failure,
     // so don't warn on encountering it.
     return hr;
   }
   // 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);
     mDiscontinuity = false;
   }
 
   *aOutput = output.pSample; // AddRefs
   if (mMFTProvidesOutputSamples && !providedSample) {
--- a/dom/media/platforms/wmf/WMFAudioMFTManager.cpp
+++ b/dom/media/platforms/wmf/WMFAudioMFTManager.cpp
@@ -5,17 +5,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "WMFAudioMFTManager.h"
 #include "MediaInfo.h"
 #include "VideoUtils.h"
 #include "WMFUtils.h"
 #include "nsTArray.h"
 #include "TimeUnits.h"
-
+#include "mozilla/Telemetry.h"
 #include "mozilla/Logging.h"
 
 extern mozilla::LogModule* GetPDMLog();
 #define LOG(...) MOZ_LOG(GetPDMLog(), mozilla::LogLevel::Debug, (__VA_ARGS__))
 
 namespace mozilla {
 
 static void
@@ -221,16 +221,26 @@ WMFAudioMFTManager::Output(int64_t aStre
       ++typeChangeCount;
       continue;
     }
     break;
   }
 
   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, true);
+    });
+    AbstractThread::MainThread()->Dispatch(task.forget());
+    return E_FAIL;
+  }
+
   RefPtr<IMFMediaBuffer> buffer;
   hr = sample->ConvertToContiguousBuffer(getter_AddRefs(buffer));
   NS_ENSURE_TRUE(SUCCEEDED(hr), hr);
 
   BYTE* data = nullptr; // Note: *data will be owned by the IMFMediaBuffer, we don't need to free it.
   DWORD maxLength = 0, currentLength = 0;
   hr = buffer->Lock(&data, &maxLength, &currentLength);
   NS_ENSURE_TRUE(SUCCEEDED(hr), hr);
--- a/dom/media/platforms/wmf/WMFVideoMFTManager.cpp
+++ b/dom/media/platforms/wmf/WMFVideoMFTManager.cpp
@@ -16,16 +16,17 @@
 #include "mozilla/layers/LayersTypes.h"
 #include "MediaInfo.h"
 #include "mozilla/Logging.h"
 #include "gfx2DGlue.h"
 #include "gfxWindowsPlatform.h"
 #include "IMFYCbCrImage.h"
 #include "mozilla/WindowsVersion.h"
 #include "mozilla/Preferences.h"
+#include "mozilla/Telemetry.h"
 #include "nsPrintfCString.h"
 
 extern mozilla::LogModule* GetPDMLog();
 #define LOG(...) MOZ_LOG(GetPDMLog(), mozilla::LogLevel::Debug, (__VA_ARGS__))
 
 using mozilla::layers::Image;
 using mozilla::layers::IMFYCbCrImage;
 using mozilla::layers::LayerManager;
@@ -73,16 +74,19 @@ WMFVideoMFTManager::WMFVideoMFTManager(
                             mozilla::layers::LayersBackend aLayersBackend,
                             mozilla::layers::ImageContainer* aImageContainer,
                             bool aDXVAEnabled)
   : mVideoInfo(aConfig)
   , mVideoStride(0)
   , mImageContainer(aImageContainer)
   , mDXVAEnabled(aDXVAEnabled)
   , mLayersBackend(aLayersBackend)
+  , mNullOutputCount(0)
+  , mGotValidOutputAfterNullOutput(false)
+  , mGotExcessiveNullOutput(false)
   // mVideoStride, mVideoWidth, mVideoHeight, mUseHwAccel are initialized in
   // Init().
 {
   MOZ_COUNT_CTOR(WMFVideoMFTManager);
 
   // Need additional checks/params to check vp8/vp9
   if (aConfig.mMimeType.EqualsLiteral("video/mp4") ||
       aConfig.mMimeType.EqualsLiteral("video/avc")) {
@@ -98,16 +102,30 @@ WMFVideoMFTManager::WMFVideoMFTManager(
 
 WMFVideoMFTManager::~WMFVideoMFTManager()
 {
   MOZ_COUNT_DTOR(WMFVideoMFTManager);
   // Ensure DXVA/D3D9 related objects are released on the main thread.
   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&
 WMFVideoMFTManager::GetMFTGUID()
 {
   MOZ_ASSERT(mStreamType != Unknown);
   switch (mStreamType) {
     case H264: return CLSID_CMSH264DecoderMFT;
@@ -570,16 +588,33 @@ WMFVideoMFTManager::Output(int64_t aStre
       // changes on consecutive calls, so be permissive.
       // 100 is arbitrarily > 2.
       NS_ENSURE_TRUE(typeChangeCount < 100, MF_E_TRANSFORM_STREAM_CHANGE);
       // Loop back and try decoding again...
       ++typeChangeCount;
       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 500 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 > 500) {
+          LOG("Excessive Video MFTDecoder returning success but no output; giving up");
+          mGotExcessiveNullOutput = true;
+          return E_FAIL;
+        }
+        continue;
+      }
       break;
     }
     // Else unexpected error, assert, and bail.
     NS_WARNING("WMFVideoMFTManager::Output() unexpected error");
     return hr;
   }
 
   RefPtr<VideoData> frame;
@@ -590,16 +625,20 @@ WMFVideoMFTManager::Output(int64_t aStre
   }
   // Frame should be non null only when we succeeded.
   MOZ_ASSERT((frame != nullptr) == SUCCEEDED(hr));
   NS_ENSURE_TRUE(SUCCEEDED(hr), hr);
   NS_ENSURE_TRUE(frame, E_FAIL);
 
   aOutData = frame;
 
+  if (mNullOutputCount) {
+    mGotValidOutputAfterNullOutput = true;
+  }
+
   return S_OK;
 }
 
 void
 WMFVideoMFTManager::Shutdown()
 {
   mDecoder = nullptr;
   DeleteOnMainThread(mDXVA2Manager);
--- a/dom/media/platforms/wmf/WMFVideoMFTManager.h
+++ b/dom/media/platforms/wmf/WMFVideoMFTManager.h
@@ -83,13 +83,17 @@ private:
     VP8,
     VP9
   };
 
   StreamType mStreamType;
 
   const GUID& GetMFTGUID();
   const GUID& GetMediaSubtypeGUID();
+
+  uint32_t mNullOutputCount;
+  bool mGotValidOutputAfterNullOutput;
+  bool mGotExcessiveNullOutput;
 };
 
 } // namespace mozilla
 
 #endif // WMFVideoMFTManager_h_
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -6072,16 +6072,31 @@
   },
   "MEDIA_OGG_LOADED_IS_CHAINED": {
     "alert_emails": ["cpearce@mozilla.com"],
     "expires_in_version": "53",
     "kind": "boolean",
     "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": "flag",
+    "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",
     "kind": "boolean",
     "description": "Whether at startup we report we can playback MP4 (AAC) audio. This is single value is recorded at every startup.",
     "releaseChannelCollection": "opt-out"
   },
   "VIDEO_CAN_CREATE_H264_DECODER": {