Bug 1091008 - Remove the aStartTime argument from MediaDecoderReader::GetBuffered. r=cpearce

We now have this stashed on the superclass.

dom/media/MediaDecoderReader.cpp

@@ -6,6 +6,7 @@
 
 #include "MediaDecoderReader.h"
 #include "AbstractMediaDecoder.h"
+#include "MediaResource.h"
 #include "VideoUtils.h"
 #include "ImageContainer.h"
 
@@ -129,10 +130,9 @@ MediaDecoderReader::SetStartTime(int64_t aStartTime)
 }
 
 nsresult
-MediaDecoderReader::GetBuffered(mozilla::dom::TimeRanges* aBuffered,
-                                int64_t aStartTime)
+MediaDecoderReader::GetBuffered(mozilla::dom::TimeRanges* aBuffered)
 {
-  MediaResource* stream = mDecoder->GetResource();
+  AutoPinned<MediaResource> stream(mDecoder->GetResource());
   int64_t durationUs = 0;
   {
     ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());

dom/media/MediaDecoderReader.h

@@ -135,9 +135,7 @@ public:
     mIgnoreAudioOutputFormat = true;
   }
 
-  // Populates aBuffered with the time ranges which are buffered. aStartTime
-  // must be the presentation time of the first frame in the media, e.g.
-  // the media time corresponding to playback time/position 0. This function
+  // Populates aBuffered with the time ranges which are buffered. This function
   // is called on the main, decode, and state machine threads.
   //
   // This base implementation in MediaDecoderReader estimates the time ranges
@@ -151,8 +149,7 @@ public:
   // The OggReader relies on this base implementation not performing I/O,
   // since in FirefoxOS we can't do I/O on the main thread, where this is
   // called.
-  virtual nsresult GetBuffered(dom::TimeRanges* aBuffered,
-                               int64_t aStartTime);
+  virtual nsresult GetBuffered(dom::TimeRanges* aBuffered);
 
   virtual int64_t ComputeStartTime(const VideoData* aVideo, const AudioData* aAudio);
 

dom/media/MediaDecoderStateMachine.cpp

@@ -1496,8 +1496,11 @@ void MediaDecoderStateMachine::NotifyDataArrived(const char* aBuffer,
   // faster than played, mEndTime won't reflect the end of playable data
   // since we haven't played the frame at the end of buffered data. So update
   // mEndTime here as new data is downloaded to prevent such a lag.
+  //
+  // Make sure to only do this if we have a start time, otherwise the reader
+  // doesn't know how to compute GetBuffered.
   nsRefPtr<dom::TimeRanges> buffered = new dom::TimeRanges();
-  if (mDecoder->IsInfinite() &&
+  if (mDecoder->IsInfinite() && (mStartTime != -1) &&
       NS_SUCCEEDED(mDecoder->GetBuffered(buffered)))
   {
     uint32_t length = 0;
@@ -3126,24 +3129,6 @@ void MediaDecoderStateMachine::StartBuffering()
 #endif
 }
 
-nsresult MediaDecoderStateMachine::GetBuffered(dom::TimeRanges* aBuffered)
-{
-  // It's possible for JS to query .buffered before we've determined the start
-  // time from metadata, in which case the reader isn't ready to be asked this
-  // question.
-  ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
-  if (mStartTime < 0) {
-    return NS_OK;
-  }
-
-  MediaResource* resource = mDecoder->GetResource();
-  NS_ENSURE_TRUE(resource, NS_ERROR_FAILURE);
-  resource->Pin();
-  nsresult res = mReader->GetBuffered(aBuffered, mStartTime);
-  resource->Unpin();
-  return res;
-}
-
 void MediaDecoderStateMachine::SetPlayStartTime(const TimeStamp& aTimeStamp)
 {
   AssertCurrentThreadInMonitor();

dom/media/MediaDecoderStateMachine.h

@@ -269,7 +269,17 @@ public:
     return mState == DECODER_STATE_SEEKING;
   }
 
-  nsresult GetBuffered(dom::TimeRanges* aBuffered);
+  nsresult GetBuffered(dom::TimeRanges* aBuffered) {
+    // It's possible for JS to query .buffered before we've determined the start
+    // time from metadata, in which case the reader isn't ready to be asked this
+    // question.
+    ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
+    if (mStartTime < 0) {
+      return NS_OK;
+    }
+
+    return mReader->GetBuffered(aBuffered);
+  }
 
   void SetPlaybackRate(double aPlaybackRate);
   void SetPreservesPitch(bool aPreservesPitch);

dom/media/MediaResource.h

@@ -718,6 +718,33 @@ protected:
   bool mIsTransportSeekable;
 };
 
+/**
+ * RAII class that handles pinning and unpinning for MediaResource and derived.
+ * This should be used when making calculations that involve potentially-cached
+ * MediaResource data, so that the state of the world can't change out from under
+ * us.
+ */
+template<class T>
+class MOZ_STACK_CLASS AutoPinned {
+ public:
+  explicit AutoPinned(T* aResource MOZ_GUARD_OBJECT_NOTIFIER_PARAM) : mResource(aResource) {
+    MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    MOZ_ASSERT(mResource);
+    mResource->Pin();
+  }
+
+  ~AutoPinned() {
+    mResource->Unpin();
+  }
+
+  operator T*() const { return mResource; }
+  T* operator->() const { return mResource; }
+
+private:
+  T* mResource;
+  MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
+};
+
 } // namespace mozilla
 
 #endif

dom/media/fmp4/MP4Reader.cpp

@@ -809,13 +809,11 @@ MP4Reader::UpdateIndex()
     return;
   }
 
-  MediaResource* resource = mDecoder->GetResource();
-  resource->Pin();
+  AutoPinned<MediaResource> resource(mDecoder->GetResource());
   nsTArray<MediaByteRange> ranges;
   if (NS_SUCCEEDED(resource->GetCachedRanges(ranges))) {
     mDemuxer->UpdateIndex(ranges);
   }
-  resource->Unpin();
 }
 
 int64_t
@@ -830,25 +828,24 @@ MP4Reader::GetEvictionOffset(double aTime)
 }
 
 nsresult
-MP4Reader::GetBuffered(dom::TimeRanges* aBuffered, int64_t aStartTime)
+MP4Reader::GetBuffered(dom::TimeRanges* aBuffered)
 {
   MonitorAutoLock mon(mIndexMonitor);
   if (!mIndexReady) {
     return NS_OK;
   }
+  MOZ_ASSERT(mStartTime != -1, "Need to finish metadata decode first");
 
-  MediaResource* resource = mDecoder->GetResource();
+  AutoPinned<MediaResource> resource(mDecoder->GetResource());
   nsTArray<MediaByteRange> ranges;
-  resource->Pin();
   nsresult rv = resource->GetCachedRanges(ranges);
-  resource->Unpin();
 
   if (NS_SUCCEEDED(rv)) {
     nsTArray<Interval<Microseconds>> timeRanges;
     mDemuxer->ConvertByteRangesToTime(ranges, &timeRanges);
     for (size_t i = 0; i < timeRanges.Length(); i++) {
-      aBuffered->Add((timeRanges[i].start - aStartTime) / 1000000.0,
-                     (timeRanges[i].end - aStartTime) / 1000000.0);
+      aBuffered->Add((timeRanges[i].start - mStartTime) / 1000000.0,
+                     (timeRanges[i].end - mStartTime) / 1000000.0);
     }
   }
 

dom/media/fmp4/MP4Reader.h

@@ -59,8 +59,7 @@ public:
 
   virtual int64_t GetEvictionOffset(double aTime) MOZ_OVERRIDE;
 
-  virtual nsresult GetBuffered(dom::TimeRanges* aBuffered,
-                               int64_t aStartTime) MOZ_OVERRIDE;
+  virtual nsresult GetBuffered(dom::TimeRanges* aBuffered) MOZ_OVERRIDE;
 
   // For Media Resource Management
   virtual bool IsWaitingMediaResources() MOZ_OVERRIDE;

dom/media/gstreamer/GStreamerReader.cpp

@@ -816,8 +816,7 @@ void GStreamerReader::Seek(int64_t aTarget,
   GetCallback()->OnSeekCompleted(NS_OK);
 }
 
-nsresult GStreamerReader::GetBuffered(dom::TimeRanges* aBuffered,
-                                      int64_t aStartTime)
+nsresult GStreamerReader::GetBuffered(dom::TimeRanges* aBuffered)
 {
   if (!mInfo.HasValidMedia()) {
     return NS_OK;
@@ -826,7 +825,7 @@ nsresult GStreamerReader::GetBuffered(dom::TimeRanges* aBuffered,
 #if GST_VERSION_MAJOR == 0
   GstFormat format = GST_FORMAT_TIME;
 #endif
-  MediaResource* resource = mDecoder->GetResource();
+  AutoPinned<MediaResource> resource(mDecoder->GetResource());
   nsTArray<MediaByteRange> ranges;
   resource->GetCachedRanges(ranges);
 

dom/media/gstreamer/GStreamerReader.h

@@ -52,7 +52,7 @@ public:
                     int64_t aStartTime,
                     int64_t aEndTime,
                     int64_t aCurrentTime);
-  virtual nsresult GetBuffered(dom::TimeRanges* aBuffered, int64_t aStartTime);
+  virtual nsresult GetBuffered(dom::TimeRanges* aBuffered);
 
   virtual void NotifyDataArrived(const char *aBuffer,
                                  uint32_t aLength,

dom/media/gtest/TestMP4Reader.cpp

@@ -36,6 +36,12 @@ public:
     decoder->SetResource(resource);
 
     reader->Init(nullptr);
+    {
+      // This needs to be done before invoking GetBuffered. This is normally
+      // done by MediaDecoderStateMachine.
+      ReentrantMonitorAutoEnter mon(decoder->GetReentrantMonitor());
+      reader->SetStartTime(0);
+    }
   }
 
   void Init() {
@@ -72,7 +78,7 @@ TEST(MP4Reader, BufferedRange)
   b->resource->MockAddBufferedRange(248400, 327455);
 
   nsRefPtr<TimeRanges> ranges = new TimeRanges();
-  EXPECT_EQ(NS_OK, b->reader->GetBuffered(ranges, 0));
+  EXPECT_EQ(NS_OK, b->reader->GetBuffered(ranges));
   EXPECT_EQ(1U, ranges->Length());
   double start = 0;
   EXPECT_EQ(NS_OK, ranges->Start(0, &start));
@@ -93,7 +99,7 @@ TEST(MP4Reader, BufferedRangeMissingLastByte)
   b->resource->MockAddBufferedRange(324913, 327455);
 
   nsRefPtr<TimeRanges> ranges = new TimeRanges();
-  EXPECT_EQ(NS_OK, b->reader->GetBuffered(ranges, 0));
+  EXPECT_EQ(NS_OK, b->reader->GetBuffered(ranges));
   EXPECT_EQ(1U, ranges->Length());
   double start = 0;
   EXPECT_EQ(NS_OK, ranges->Start(0, &start));
@@ -114,7 +120,7 @@ TEST(MP4Reader, BufferedRangeSyncFrame)
   b->resource->MockAddBufferedRange(146336, 327455);
 
   nsRefPtr<TimeRanges> ranges = new TimeRanges();
-  EXPECT_EQ(NS_OK, b->reader->GetBuffered(ranges, 0));
+  EXPECT_EQ(NS_OK, b->reader->GetBuffered(ranges));
   EXPECT_EQ(1U, ranges->Length());
   double start = 0;
   EXPECT_EQ(NS_OK, ranges->Start(0, &start));
@@ -172,7 +178,7 @@ TEST(MP4Reader, CompositionOrder)
   b->resource->MockAddBufferedRange(13220, 13901);
 
   nsRefPtr<TimeRanges> ranges = new TimeRanges();
-  EXPECT_EQ(NS_OK, b->reader->GetBuffered(ranges, 0));
+  EXPECT_EQ(NS_OK, b->reader->GetBuffered(ranges));
   EXPECT_EQ(2U, ranges->Length());
 
   double start = 0;
@@ -222,7 +228,7 @@ TEST(MP4Reader, Normalised)
   b->resource->MockAddBufferedRange(48, 13901);
 
   nsRefPtr<TimeRanges> ranges = new TimeRanges();
-  EXPECT_EQ(NS_OK, b->reader->GetBuffered(ranges, 0));
+  EXPECT_EQ(NS_OK, b->reader->GetBuffered(ranges));
   EXPECT_EQ(1U, ranges->Length());
 
   double start = 0;

dom/media/mediasource/SourceBufferDecoder.cpp

@@ -225,8 +225,7 @@ SourceBufferDecoder::NotifyDataArrived(const char* aBuffer, uint32_t aLength, in
 nsresult
 SourceBufferDecoder::GetBuffered(dom::TimeRanges* aBuffered)
 {
-  // XXX: Need mStartTime (from StateMachine) instead of passing 0.
-  return mReader->GetBuffered(aBuffered, 0);
+  return mReader->GetBuffered(aBuffered);
 }
 
 int64_t

dom/media/ogg/OggReader.cpp

@@ -1890,15 +1890,16 @@ nsresult OggReader::SeekBisection(int64_t aTarget,
   return NS_OK;
 }
 
-nsresult OggReader::GetBuffered(dom::TimeRanges* aBuffered, int64_t aStartTime)
+nsresult OggReader::GetBuffered(dom::TimeRanges* aBuffered)
 {
+  MOZ_ASSERT(mStartTime != -1, "Need to finish metadata decode first");
   {
     mozilla::ReentrantMonitorAutoEnter mon(mMonitor);
     if (mIsChained)
       return NS_ERROR_FAILURE;
   }
 #ifdef OGG_ESTIMATE_BUFFERED
-  return MediaDecoderReader::GetBuffered(aBuffered, aStartTime);
+  return MediaDecoderReader::GetBuffered(aBuffered);
 #else
   // HasAudio and HasVideo are not used here as they take a lock and cause
   // a deadlock. Accessing mInfo doesn't require a lock - it doesn't change
@@ -1908,7 +1909,7 @@ nsresult OggReader::GetBuffered(dom::TimeRanges* aBuffered, int64_t aStartTime)
     return NS_OK;
   }
 
-  MediaResource* resource = mDecoder->GetResource();
+  AutoPinned<MediaResource> resource(mDecoder->GetResource());
   nsTArray<MediaByteRange> ranges;
   nsresult res = resource->GetCachedRanges(ranges);
   NS_ENSURE_SUCCESS(res, res);
@@ -1928,7 +1929,7 @@ nsresult OggReader::GetBuffered(dom::TimeRanges* aBuffered, int64_t aStartTime)
     // we special-case (startOffset == 0) so that the first
     // buffered range always appears to be buffered from the media start
     // time, rather than from the end-time of the first page.
-    int64_t startTime = (startOffset == 0) ? aStartTime : -1;
+    int64_t startTime = (startOffset == 0) ? mStartTime : -1;
 
     // Find the start time of the range. Read pages until we find one with a
     // granulepos which we can convert into a timestamp to use as the time of
@@ -1996,8 +1997,8 @@ nsresult OggReader::GetBuffered(dom::TimeRanges* aBuffered, int64_t aStartTime)
       // find an end time.
       int64_t endTime = RangeEndTime(startOffset, endOffset, true);
       if (endTime != -1) {
-        aBuffered->Add((startTime - aStartTime) / static_cast<double>(USECS_PER_S),
-                       (endTime - aStartTime) / static_cast<double>(USECS_PER_S));
+        aBuffered->Add((startTime - mStartTime) / static_cast<double>(USECS_PER_S),
+                       (endTime - mStartTime) / static_cast<double>(USECS_PER_S));
       }
     }
   }

dom/media/ogg/OggReader.h

@@ -79,7 +79,7 @@ public:
   virtual nsresult ReadMetadata(MediaInfo* aInfo,
                                 MetadataTags** aTags);
   virtual void Seek(int64_t aTime, int64_t aStartTime, int64_t aEndTime, int64_t aCurrentTime);
-  virtual nsresult GetBuffered(dom::TimeRanges* aBuffered, int64_t aStartTime);
+  virtual nsresult GetBuffered(dom::TimeRanges* aBuffered);
 
   virtual bool IsMediaSeekable() MOZ_OVERRIDE;
 

dom/media/omx/RtspMediaCodecReader.h

@@ -48,8 +48,7 @@ public:
   // we returned are not useful for the MediaDecodeStateMachine. Unlike the
   // ChannelMediaResource, it has a "cache" that can store the whole streaming
   // data so the |GetBuffered| function can retrieve useful time ranges.
-  virtual nsresult GetBuffered(dom::TimeRanges* aBuffered,
-                               int64_t aStartTime) MOZ_OVERRIDE {
+  virtual nsresult GetBuffered(dom::TimeRanges* aBuffered) MOZ_OVERRIDE {
     return NS_OK;
   }
 

dom/media/omx/RtspOmxReader.h

@@ -58,8 +58,7 @@ public:
   // we returned are not useful for the MediaDecodeStateMachine. Unlike the
   // ChannelMediaResource, it has a "cache" that can store the whole streaming
   // data so the |GetBuffered| function can retrieve useful time ranges.
-  virtual nsresult GetBuffered(mozilla::dom::TimeRanges* aBuffered,
-                               int64_t aStartTime) MOZ_FINAL MOZ_OVERRIDE {
+  virtual nsresult GetBuffered(mozilla::dom::TimeRanges* aBuffered) MOZ_FINAL MOZ_OVERRIDE {
     return NS_OK;
   }
 

dom/media/raw/RawReader.cpp

@@ -290,7 +290,7 @@ nsresult RawReader::SeekInternal(int64_t aTime)
   return NS_OK;
 }
 
-nsresult RawReader::GetBuffered(dom::TimeRanges* aBuffered, int64_t aStartTime)
+nsresult RawReader::GetBuffered(dom::TimeRanges* aBuffered)
 {
   return NS_OK;
 }

dom/media/raw/RawReader.h

@@ -40,7 +40,7 @@ public:
   virtual nsresult ReadMetadata(MediaInfo* aInfo,
                                 MetadataTags** aTags);
   virtual void Seek(int64_t aTime, int64_t aStartTime, int64_t aEndTime, int64_t aCurrentTime);
-  virtual nsresult GetBuffered(dom::TimeRanges* aBuffered, int64_t aStartTime);
+  virtual nsresult GetBuffered(dom::TimeRanges* aBuffered);
 
   virtual bool IsMediaSeekable() MOZ_OVERRIDE;
 

dom/media/wave/WaveReader.cpp

@@ -280,14 +280,15 @@ static double RoundToUsecs(double aSeconds) {
   return floor(aSeconds * USECS_PER_S) / USECS_PER_S;
 }
 
-nsresult WaveReader::GetBuffered(dom::TimeRanges* aBuffered, int64_t aStartTime)
+nsresult WaveReader::GetBuffered(dom::TimeRanges* aBuffered)
 {
   if (!mInfo.HasAudio()) {
     return NS_OK;
   }
-  int64_t startOffset = mDecoder->GetResource()->GetNextCachedData(mWavePCMOffset);
+  AutoPinned<MediaResource> resource(mDecoder->GetResource());
+  int64_t startOffset = resource->GetNextCachedData(mWavePCMOffset);
   while (startOffset >= 0) {
-    int64_t endOffset = mDecoder->GetResource()->GetCachedDataEnd(startOffset);
+    int64_t endOffset = resource->GetCachedDataEnd(startOffset);
     // Bytes [startOffset..endOffset] are cached.
     NS_ASSERTION(startOffset >= mWavePCMOffset, "Integer underflow in GetBuffered");
     NS_ASSERTION(endOffset >= mWavePCMOffset, "Integer underflow in GetBuffered");
@@ -297,7 +298,7 @@ nsresult WaveReader::GetBuffered(dom::TimeRanges* aBuffered, int64_t aStartTime)
     // the media element.
     aBuffered->Add(RoundToUsecs(BytesToTime(startOffset - mWavePCMOffset)),
                    RoundToUsecs(BytesToTime(endOffset - mWavePCMOffset)));
-    startOffset = mDecoder->GetResource()->GetNextCachedData(endOffset);
+    startOffset = resource->GetNextCachedData(endOffset);
   }
   return NS_OK;
 }

dom/media/wave/WaveReader.h

@@ -44,7 +44,7 @@ public:
   virtual nsresult ReadMetadata(MediaInfo* aInfo,
                                 MetadataTags** aTags);
   virtual void Seek(int64_t aTime, int64_t aStartTime, int64_t aEndTime, int64_t aCurrentTime);
-  virtual nsresult GetBuffered(dom::TimeRanges* aBuffered, int64_t aStartTime);
+  virtual nsresult GetBuffered(dom::TimeRanges* aBuffered);
 
   // To seek in a buffered range, we just have to seek the stream.
   virtual bool IsSeekableInBufferedRanges() {

dom/media/webm/WebMReader.cpp

@@ -1092,13 +1092,14 @@ nsresult WebMReader::SeekInternal(int64_t aTarget, int64_t aStartTime)
   return NS_OK;
 }
 
-nsresult WebMReader::GetBuffered(dom::TimeRanges* aBuffered, int64_t aStartTime)
+nsresult WebMReader::GetBuffered(dom::TimeRanges* aBuffered)
 {
+  MOZ_ASSERT(mStartTime != -1, "Need to finish metadata decode first");
   if (aBuffered->Length() != 0) {
     return NS_ERROR_FAILURE;
   }
 
-  MediaResource* resource = mDecoder->GetResource();
+  AutoPinned<MediaResource> resource(mDecoder->GetResource());
 
   // Special case completely cached files.  This also handles local files.
   if (mContext && resource->IsDataCachedToEndOfResource(0)) {
@@ -1121,7 +1122,7 @@ nsresult WebMReader::GetBuffered(dom::TimeRanges* aBuffered, int64_t aStartTime)
                                                         ranges[index].mEnd,
                                                         &start, &end);
     if (rv) {
-      int64_t startOffset = aStartTime * NS_PER_USEC;
+      int64_t startOffset = mStartTime * NS_PER_USEC;
       NS_ASSERTION(startOffset >= 0 && uint64_t(startOffset) <= start,
                    "startOffset negative or larger than start time");
       if (!(startOffset >= 0 && uint64_t(startOffset) <= start)) {

dom/media/webm/WebMReader.h

@@ -136,7 +136,7 @@ public:
                                 MetadataTags** aTags);
   virtual void Seek(int64_t aTime, int64_t aStartTime, int64_t aEndTime,
                     int64_t aCurrentTime);
-  virtual nsresult GetBuffered(dom::TimeRanges* aBuffered, int64_t aStartTime);
+  virtual nsresult GetBuffered(dom::TimeRanges* aBuffered);
   virtual void NotifyDataArrived(const char* aBuffer, uint32_t aLength,
                                  int64_t aOffset);
   virtual int64_t GetEvictionOffset(double aTime);