Bug 1256038: Remove special NotifyDataArrived handling in the DirectShow reader. r?cpearce draft
authorJean-Yves Avenard <jyavenard@mozilla.com>
Tue, 22 Mar 2016 22:59:50 +1100
changeset 343364 e14af5696e7d592efbc160535f7b25bb35bbd1d9
parent 343278 3587b25bae302c1eed72968dbd7cef883e715948
child 516749 758fc3ad9241397dfda0eb5f0c07017aa40f2cb1
push id13596
push userbmo:jyavenard@mozilla.com
push dateTue, 22 Mar 2016 12:00:33 +0000
reviewerscpearce
bugs1256038, 1223599
milestone48.0a1
Bug 1256038: Remove special NotifyDataArrived handling in the DirectShow reader. r?cpearce Following bug 1223599, NotifyDataArrived is called extremely often (every 32kB block downloaded). The combination of pinning the MediaResource with small cache reads while reading on another thread can causes incorrect read. While this issue needs to be addressed, the special handling of NotifyDataArrived with DirectShow ultimately serves no purpose. By the time we have read the metadata, we have already identified if the MP3 has a known duration. If not, the duration will be amended as data is being returned to the MDSM. Continually feeding new data to the MP3FrameParser only allow to account for the new data downloaded ahead of the last decoded sample and to get a slightly more accurate buffered range and duration. The new MP3Demuxer doesn't do it, and ultimately this makes the DirectShowReader behaves like the new MP3Demuxer and gives us consistency across the readers. MozReview-Commit-ID: HKNWWrofIqV
dom/media/directshow/DirectShowReader.cpp
dom/media/directshow/DirectShowReader.h
--- a/dom/media/directshow/DirectShowReader.cpp
+++ b/dom/media/directshow/DirectShowReader.cpp
@@ -28,18 +28,17 @@ GetDirectShowLog() {
 DirectShowReader::DirectShowReader(AbstractMediaDecoder* aDecoder)
   : MediaDecoderReader(aDecoder),
     mMP3FrameParser(aDecoder->GetResource()->GetLength()),
 #ifdef DIRECTSHOW_REGISTER_GRAPH
     mRotRegister(0),
 #endif
     mNumChannels(0),
     mAudioRate(0),
-    mBytesPerSample(0),
-    mDuration(0)
+    mBytesPerSample(0)
 {
   MOZ_ASSERT(NS_IsMainThread(), "Must be on main thread.");
   MOZ_COUNT_CTOR(DirectShowReader);
 }
 
 DirectShowReader::~DirectShowReader()
 {
   MOZ_ASSERT(NS_IsMainThread(), "Must be on main thread.");
@@ -357,48 +356,9 @@ DirectShowReader::SeekInternal(int64_t a
   NS_ENSURE_TRUE(SUCCEEDED(hr), NS_ERROR_FAILURE);
 
   hr = mControl->Run();
   NS_ENSURE_TRUE(SUCCEEDED(hr), NS_ERROR_FAILURE);
 
   return NS_OK;
 }
 
-void
-DirectShowReader::NotifyDataArrivedInternal()
-{
-  MOZ_ASSERT(OnTaskQueue());
-  if (!mMP3FrameParser.NeedsData()) {
-    return;
-  }
-
-  AutoPinned<MediaResource> resource(mDecoder->GetResource());
-  MediaByteRangeSet byteRanges;
-  nsresult rv = resource->GetCachedRanges(byteRanges);
-
-  if (NS_FAILED(rv)) {
-    return;
-  }
-
-  if (byteRanges == mLastCachedRanges) {
-    return;
-  }
-  MediaByteRangeSet intervals = byteRanges - mLastCachedRanges;
-  mLastCachedRanges = byteRanges;
-
-  for (const auto& interval : intervals) {
-    RefPtr<MediaByteBuffer> bytes =
-      resource->MediaReadAt(interval.mStart, interval.Length());
-    NS_ENSURE_TRUE_VOID(bytes);
-    mMP3FrameParser.Parse(bytes->Elements(), interval.Length(), interval.mStart);
-    if (!mMP3FrameParser.IsMP3()) {
-      return;
-    }
-  }
-  int64_t duration = mMP3FrameParser.GetDuration();
-  if (duration != mDuration) {
-    MOZ_ASSERT(mDecoder);
-    mDuration = duration;
-    mDecoder->DispatchUpdateEstimatedMediaDuration(mDuration);
-  }
-}
-
 } // namespace mozilla
--- a/dom/media/directshow/DirectShowReader.h
+++ b/dom/media/directshow/DirectShowReader.h
@@ -55,19 +55,16 @@ public:
                         int64_t aTimeThreshold) override;
 
   nsresult ReadMetadata(MediaInfo* aInfo,
                         MetadataTags** aTags) override;
 
   RefPtr<SeekPromise>
   Seek(SeekTarget aTarget, int64_t aEndTime) override;
 
-protected:
-  void NotifyDataArrivedInternal() override;
-
 private:
   // Notifies the filter graph that playback is complete. aStatus is
   // the code to send to the filter graph. Always returns false, so
   // that we can just "return Finish()" from DecodeAudioData().
   bool Finish(HRESULT aStatus);
 
   nsresult SeekInternal(int64_t aTime);
 
@@ -99,18 +96,13 @@ private:
   // Number of channels in the audio stream.
   uint32_t mNumChannels;
 
   // Samples per second in the audio stream.
   uint32_t mAudioRate;
 
   // Number of bytes per sample. Can be either 1 or 2.
   uint32_t mBytesPerSample;
-
-  // Duration of the stream, in microseconds.
-  int64_t mDuration;
-
-  MediaByteRangeSet mLastCachedRanges;
 };
 
 } // namespace mozilla
 
 #endif