Bug 639391 - Ensure WebM GetBuffered() is threadsafe. r=kinetik
authorChris Pearce <chris@pearce.org.nz>
Thu, 24 Mar 2011 11:28:58 +1300
changeset 63629 4902d72f6072326ea3195de8dd0bf3c0c899c9c5
parent 63628 d3aeb102da2b5fe244d0eb367492560a6d9f4daf
child 63677 ab95ab9e389bfc5af0c0bafdb526fef40e8cd86f
push id19247
push usercpearce@mozilla.com
push dateWed, 23 Mar 2011 22:39:37 +0000
treeherdermozilla-central@4902d72f6072 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskinetik
bugs639391
milestone2.2a1pre
first release with
nightly linux32
4902d72f6072 / 4.2a1pre / 20110323161216 / files
nightly linux64
4902d72f6072 / 4.2a1pre / 20110323161236 / files
nightly mac
4902d72f6072 / 4.2a1pre / 20110323161229 / files
nightly win32
4902d72f6072 / 4.2a1pre / 20110323161246 / files
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
Bug 639391 - Ensure WebM GetBuffered() is threadsafe. r=kinetik
content/media/webm/nsWebMBufferedParser.cpp
content/media/webm/nsWebMBufferedParser.h
content/media/webm/nsWebMReader.cpp
--- a/content/media/webm/nsWebMBufferedParser.cpp
+++ b/content/media/webm/nsWebMBufferedParser.cpp
@@ -34,16 +34,19 @@
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "nsAlgorithm.h"
 #include "nsWebMBufferedParser.h"
 #include "nsTimeRanges.h"
+#include "nsThreadUtils.h"
+
+using mozilla::MonitorAutoEnter;
 
 static const double NS_PER_S = 1e9;
 static const double MS_PER_S = 1e3;
 
 static PRUint32
 VIntLength(unsigned char aFirstByte, PRUint32* aMask)
 {
   PRUint32 count = 1;
@@ -58,17 +61,18 @@ VIntLength(unsigned char aFirstByte, PRU
   if (aMask) {
     *aMask = mask;
   }
   NS_ASSERTION(count >= 1 && count <= 8, "Insane VInt length.");
   return count;
 }
 
 void nsWebMBufferedParser::Append(const unsigned char* aBuffer, PRUint32 aLength,
-                                  nsTArray<nsWebMTimeDataOffset>& aMapping)
+                                  nsTArray<nsWebMTimeDataOffset>& aMapping,
+                                  Monitor& aMonitor)
 {
   static const unsigned char CLUSTER_ID[] = { 0x1f, 0x43, 0xb6, 0x75 };
   static const unsigned char TIMECODE_ID = 0xe7;
   static const unsigned char BLOCKGROUP_ID = 0xa0;
   static const unsigned char BLOCK_ID = 0xa1;
   static const unsigned char SIMPLEBLOCK_ID = 0xa3;
 
   const unsigned char* p = aBuffer;
@@ -162,20 +166,23 @@ void nsWebMBufferedParser::Append(const 
     case READ_BLOCK_TIMECODE:
       if (mBlockTimecodeLength) {
         mBlockTimecode <<= 8;
         mBlockTimecode |= *p++;
         mBlockTimecodeLength -= 1;
       } else {
         // It's possible we've parsed this data before, so avoid inserting
         // duplicate nsWebMTimeDataOffset entries.
-        PRUint32 idx;
-        if (!aMapping.GreatestIndexLtEq(mBlockOffset, idx)) {
-          nsWebMTimeDataOffset entry(mBlockOffset, mClusterTimecode + mBlockTimecode);
-          aMapping.InsertElementAt(idx, entry);
+        {
+          MonitorAutoEnter mon(aMonitor);
+          PRUint32 idx;
+          if (!aMapping.GreatestIndexLtEq(mBlockOffset, idx)) {
+            nsWebMTimeDataOffset entry(mBlockOffset, mClusterTimecode + mBlockTimecode);
+            aMapping.InsertElementAt(idx, entry);
+          }
         }
 
         // Skip rest of block header and the block's payload.
         mBlockSize -= mVIntLength;
         mBlockSize -= 2;
         mSkipBytes = PRUint32(mBlockSize);
         mState = SKIP_DATA;
         mNextState = ANY_BLOCK_SYNC;
@@ -203,16 +210,18 @@ void nsWebMBufferedParser::Append(const 
   mCurrentOffset += aLength;
 }
 
 void nsWebMBufferedState::CalculateBufferedForRange(nsTimeRanges* aBuffered,
                                                     PRInt64 aStartOffset, PRInt64 aEndOffset,
                                                     PRUint64 aTimecodeScale,
                                                     PRInt64 aStartTimeOffsetNS)
 {
+  MonitorAutoEnter mon(mMonitor);
+
   // Find the first nsWebMTimeDataOffset at or after aStartOffset.
   PRUint32 start;
   mTimeMapping.GreatestIndexLtEq(aStartOffset, start);
   if (start == mTimeMapping.Length()) {
     return;
   }
 
   // Find the first nsWebMTimeDataOffset at or before aEndOffset.
@@ -246,16 +255,17 @@ void nsWebMBufferedState::CalculateBuffe
 
   double startTime = (mTimeMapping[start].mTimecode * aTimecodeScale - aStartTimeOffsetNS) / NS_PER_S;
   double endTime = (mTimeMapping[end].mTimecode * aTimecodeScale - aStartTimeOffsetNS) / NS_PER_S;
   aBuffered->Add(startTime, endTime);
 }
 
 void nsWebMBufferedState::NotifyDataArrived(const char* aBuffer, PRUint32 aLength, PRUint32 aOffset)
 {
+  NS_ASSERTION(NS_IsMainThread(), "Should be on main thread.");
   PRUint32 idx;
   if (!mRangeParsers.GreatestIndexLtEq(aOffset, idx)) {
     // If the incoming data overlaps an already parsed range, adjust the
     // buffer so that we only reparse the new data.  It's also possible to
     // have an overlap where the end of the incoming data is within an
     // already parsed range, but we don't bother handling that other than by
     // avoiding storing duplicate timecodes when the parser runs.
     if (idx != mRangeParsers.Length() && mRangeParsers[idx].mStartOffset <= aOffset) {
@@ -269,17 +279,20 @@ void nsWebMBufferedState::NotifyDataArri
       NS_ASSERTION(adjust >= 0, "Overlap detection bug.");
       aBuffer += adjust;
       aLength -= PRUint32(adjust);
     } else {
       mRangeParsers.InsertElementAt(idx, nsWebMBufferedParser(aOffset));
     }
   }
 
-  mRangeParsers[idx].Append(reinterpret_cast<const unsigned char*>(aBuffer), aLength, mTimeMapping);
+  mRangeParsers[idx].Append(reinterpret_cast<const unsigned char*>(aBuffer),
+                            aLength,
+                            mTimeMapping,
+                            mMonitor);
 
   // Merge parsers with overlapping regions and clean up the remnants.
   PRUint32 i = 0;
   while (i + 1 < mRangeParsers.Length()) {
     if (mRangeParsers[i].mCurrentOffset >= mRangeParsers[i + 1].mStartOffset) {
       mRangeParsers[i + 1].mStartOffset = mRangeParsers[i].mStartOffset;
       mRangeParsers.RemoveElementAt(i);
     } else {
--- a/content/media/webm/nsWebMBufferedParser.h
+++ b/content/media/webm/nsWebMBufferedParser.h
@@ -35,18 +35,20 @@
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 #if !defined(nsWebMBufferedParser_h_)
 #define nsWebMBufferedParser_h_
 
 #include "nsISupportsImpl.h"
 #include "nsTArray.h"
+#include "mozilla/Monitor.h"
 
 class nsTimeRanges;
+using mozilla::Monitor;
 
 // Stores a stream byte offset and the scaled timecode of the block at
 // that offset.  The timecode must be scaled by the stream's timecode
 // scale before use.
 struct nsWebMTimeDataOffset
 {
   nsWebMTimeDataOffset(PRInt64 aOffset, PRUint64 aTimecode)
     : mOffset(aOffset), mTimecode(aTimecode)
@@ -72,19 +74,21 @@ struct nsWebMTimeDataOffset
 // within the stream.
 struct nsWebMBufferedParser
 {
   nsWebMBufferedParser(PRInt64 aOffset)
     : mStartOffset(aOffset), mCurrentOffset(aOffset), mState(CLUSTER_SYNC), mClusterIDPos(0)
   {}
 
   // Steps the parser through aLength bytes of data.  Always consumes
-  // aLength bytes.  Updates mCurrentOffset before returning.
+  // aLength bytes.  Updates mCurrentOffset before returning.  Acquires
+  // aMonitor before using aMapping.
   void Append(const unsigned char* aBuffer, PRUint32 aLength,
-              nsTArray<nsWebMTimeDataOffset>& aMapping);
+              nsTArray<nsWebMTimeDataOffset>& aMapping,
+              Monitor& aMonitor);
 
   bool operator==(PRInt64 aOffset) const {
     return mCurrentOffset == aOffset;
   }
 
   bool operator<(PRInt64 aOffset) const {
     return mCurrentOffset < aOffset;
   }
@@ -207,31 +211,34 @@ private:
   PRUint32 mSkipBytes;
 };
 
 class nsWebMBufferedState
 {
   NS_INLINE_DECL_REFCOUNTING(nsWebMBufferedState)
 
 public:
-  nsWebMBufferedState() {
+  nsWebMBufferedState() : mMonitor("nsWebMBufferedState") {
     MOZ_COUNT_CTOR(nsWebMBufferedState);
   }
 
   ~nsWebMBufferedState() {
     MOZ_COUNT_DTOR(nsWebMBufferedState);
   }
 
   void NotifyDataArrived(const char* aBuffer, PRUint32 aLength, PRUint32 aOffset);
   void CalculateBufferedForRange(nsTimeRanges* aBuffered,
                                  PRInt64 aStartOffset, PRInt64 aEndOffset,
                                  PRUint64 aTimecodeScale,
                                  PRInt64 aStartTimeOffsetNS);
 
 private:
+  // Synchronizes access to the mTimeMapping array.
+  Monitor mMonitor;
+
   // Sorted (by offset) map of data offsets to timecodes.  Populated
   // on the main thread as data is received and parsed by nsWebMBufferedParsers.
   nsTArray<nsWebMTimeDataOffset> mTimeMapping;
 
   // Sorted (by offset) live parser instances.  Main thread only.
   nsTArray<nsWebMBufferedParser> mRangeParsers;
 };
 
--- a/content/media/webm/nsWebMReader.cpp
+++ b/content/media/webm/nsWebMReader.cpp
@@ -791,32 +791,28 @@ nsresult nsWebMReader::GetBuffered(nsTim
 
   // Special case completely cached files.  This also handles local files.
   if (stream->IsDataCachedToEndOfStream(0)) {
     uint64_t duration = 0;
     if (nestegg_duration(mContext, &duration) == 0) {
       aBuffered->Add(0, duration / NS_PER_S);
     }
   } else {
-    PRInt64 startOffset = stream->GetNextCachedData(0);
+    nsMediaStream* stream = mDecoder->GetCurrentStream();
+    nsTArray<nsByteRange> ranges;
+    nsresult res = stream->GetCachedRanges(ranges);
+    NS_ENSURE_SUCCESS(res, res);
+
     PRInt64 startTimeOffsetNS = aStartTime * NS_PER_MS;
-    while (startOffset >= 0) {
-      PRInt64 endOffset = stream->GetCachedDataEnd(startOffset);
-      NS_ASSERTION(startOffset < endOffset, "Cached range invalid");
-
+    for (PRUint32 index = 0; index < ranges.Length(); index++) {
       mBufferedState->CalculateBufferedForRange(aBuffered,
-                                                startOffset,
-                                                endOffset,
+                                                ranges[index].mStart,
+                                                ranges[index].mEnd,
                                                 timecodeScale,
                                                 startTimeOffsetNS);
-
-      // Advance to the next cached data range.
-      startOffset = stream->GetNextCachedData(endOffset);
-      NS_ASSERTION(startOffset == -1 || startOffset > endOffset,
-                   "Next cached range invalid");
     }
   }
 
   return NS_OK;
 }
 
 void nsWebMReader::NotifyDataArrived(const char* aBuffer, PRUint32 aLength, PRUint32 aOffset)
 {