Bug 1576990 - Stop the WMF decoder estimating durations on 0 duration samples. r=dminor a=lizzard
authorBryce Seager van Dyk <bvandyk@mozilla.com>
Fri, 20 Sep 2019 18:39:43 +0000
changeset 555350 b7d9fad2146028e288f40557431805e2cd8874fd
parent 555349 01474942164b3aebdbada6350fd4d5c134526180
child 555351 42692bbe0a2525e65aba2e52597f44fad5713243
push id2165
push userffxbld-merge
push dateMon, 14 Oct 2019 16:30:58 +0000
treeherdermozilla-release@0eae18af659f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdminor, lizzard
bugs1576990
milestone70.0
Bug 1576990 - Stop the WMF decoder estimating durations on 0 duration samples. r=dminor a=lizzard Set a trivial duration (1us) on 0 duration samples before feeding them to the WMF MFT decoder to prevent it from estimating the duration for such samples. Differential Revision: https://phabricator.services.mozilla.com/D46516
dom/media/platforms/wmf/MFTDecoder.cpp
--- a/dom/media/platforms/wmf/MFTDecoder.cpp
+++ b/dom/media/platforms/wmf/MFTDecoder.cpp
@@ -175,16 +175,34 @@ MFTDecoder::CreateInputSample(const uint
   NS_ENSURE_TRUE(SUCCEEDED(hr), hr);
 
   hr = sample->AddBuffer(buffer);
   NS_ENSURE_TRUE(SUCCEEDED(hr), hr);
 
   hr = sample->SetSampleTime(UsecsToHNs(aTimestamp));
   NS_ENSURE_TRUE(SUCCEEDED(hr), hr);
 
+  if (aDuration == 0) {
+    // If the sample duration is 0, the decoder will try and estimate the
+    // duration. In practice this can lead to some wildly incorrect durations,
+    // as in bug 1560440. The Microsoft docs seem conflicting here with
+    // `IMFSample::SetSampleDuration` stating 'The duration can also be zero.
+    // This might be valid for some types of data.' However,
+    // `IMFSample::GetSampleDuration method` states 'If the retrieved duration
+    // is zero, or if the method returns MF_E_NO_SAMPLE_DURATION, the duration
+    // is unknown. In that case, it might be possible to calculate the duration
+    // from the media type--for example, by using the video frame rate or the
+    // audio sampling rate.' The latter of those seems to be how the decoder
+    // handles 0 duration, hence why it estimates.
+    //
+    // Since our demuxing pipeline can create 0 duration samples, and since the
+    // decoder will override them to something positive anyway, setting them to
+    // have a trivial duration seems like the lesser of evils.
+    aDuration = 1;
+  }
   hr = sample->SetSampleDuration(UsecsToHNs(aDuration));
   NS_ENSURE_TRUE(SUCCEEDED(hr), hr);
 
   *aOutSample = sample.forget();
 
   return S_OK;
 }