Bug 847194 - Protect mCodecStates from concurrent accesses. r=cpearce
authorPaul Adenot <paul@paul.cx>
Tue, 09 Apr 2013 15:36:33 +0200
changeset 128311 1348d28d70fcbceeaaf366c36962ecab1d79ffa9
parent 128310 a2fbe8dc1ee1f7056372b144b561bd73df4f69b1
child 128312 f10884c6a91e01699b19aff130f58b7b2649252c
push idunknown
push userunknown
push dateunknown
reviewerscpearce
bugs847194
milestone23.0a1
Bug 847194 - Protect mCodecStates from concurrent accesses. r=cpearce
content/media/ogg/OggReader.cpp
content/media/ogg/OggReader.h
--- a/content/media/ogg/OggReader.cpp
+++ b/content/media/ogg/OggReader.cpp
@@ -102,17 +102,16 @@ OggReader::OggReader(AbstractMediaDecode
 
 OggReader::~OggReader()
 {
   ogg_sync_clear(&mOggState);
   MOZ_COUNT_DTOR(OggReader);
 }
 
 nsresult OggReader::Init(MediaDecoderReader* aCloneDonor) {
-  mCodecStates.Init();
   int ret = ogg_sync_init(&mOggState);
   NS_ENSURE_TRUE(ret == 0, NS_ERROR_FAILURE);
   return NS_OK;
 }
 
 nsresult OggReader::ResetDecode()
 {
   return ResetDecode(false);
@@ -194,24 +193,23 @@ nsresult OggReader::ReadMetadata(VideoIn
     int serial = ogg_page_serialno(&page);
     OggCodecState* codecState = 0;
 
     if (!ogg_page_bos(&page)) {
       // We've encountered a non Beginning Of Stream page. No more BOS pages
       // can follow in this Ogg segment, so there will be no other bitstreams
       // in the Ogg (unless it's invalid).
       readAllBOS = true;
-    } else if (!mCodecStates.Get(serial, nullptr)) {
+    } else if (!mCodecStore.Contains(serial)) {
       // We've not encountered a stream with this serial number before. Create
       // an OggCodecState to demux it, and map that to the OggCodecState
       // in mCodecStates.
       codecState = OggCodecState::Create(&page);
-      mCodecStates.Put(serial, codecState);
+      mCodecStore.Add(serial, codecState);
       bitstreams.AppendElement(codecState);
-      mKnownStreams.AppendElement(serial);
       if (codecState &&
           codecState->GetType() == OggCodecState::TYPE_VORBIS &&
           !mVorbisState)
       {
         // First Vorbis bitstream, we'll play this one. Subsequent Vorbis
         // bitstreams will be ignored.
         mVorbisState = static_cast<VorbisState*>(codecState);
       }
@@ -237,18 +235,18 @@ nsresult OggReader::ReadMetadata(VideoIn
       if (codecState &&
           codecState->GetType() == OggCodecState::TYPE_SKELETON &&
           !mSkeletonState)
       {
         mSkeletonState = static_cast<SkeletonState*>(codecState);
       }
     }
 
-    mCodecStates.Get(serial, &codecState);
-    NS_ENSURE_TRUE(codecState, NS_ERROR_FAILURE);
+    codecState = mCodecStore.Get(serial);
+    NS_ENSURE_TRUE(codecState != nullptr, NS_ERROR_FAILURE);
 
     if (NS_FAILED(codecState->PageIn(&page))) {
       return NS_ERROR_FAILURE;
     }
   }
 
   // We've read all BOS pages, so we know the streams contained in the media.
   // Now process all available header packets in the active Theora, Vorbis and
@@ -646,17 +644,16 @@ void OggReader::SetChained(bool aIsChain
   {
     ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
     mDecoder->SetMediaSeekable(false);
   }
 }
 
 bool OggReader::ReadOggChain()
 {
-
   bool chained = false;
   OpusState* newOpusState = nullptr;
   VorbisState* newVorbisState = nullptr;
   int channels = 0;
   long rate = 0;
   MetadataTags* tags = nullptr;
 
   if (HasVideo() || HasSkeleton() || !HasAudio()) {
@@ -665,17 +662,17 @@ bool OggReader::ReadOggChain()
 
   ogg_page page;
   int64_t pageOffset = ReadOggPage(&page);
   if ((pageOffset == -1) || (!ogg_page_bos(&page))) {
     return false;
   }
 
   int serial = ogg_page_serialno(&page);
-  if (mCodecStates.Get(serial, nullptr)) {
+  if (mCodecStore.Contains(serial)) {
     return false;
   }
 
   nsAutoPtr<OggCodecState> codecState;
   codecState = OggCodecState::Create(&page);
   if (!codecState) {
     return false;
   }
@@ -686,23 +683,22 @@ bool OggReader::ReadOggChain()
 #ifdef MOZ_OPUS
   else if (mOpusState && (codecState->GetType() == OggCodecState::TYPE_OPUS)) {
     newOpusState = static_cast<OpusState*>(codecState.get());
   }
 #endif
   else {
     return false;
   }
+  OggCodecState* state;
 
-  mCodecStates.Put(serial, codecState.forget());
-  mKnownStreams.AppendElement(serial);
-  OggCodecState* state = nullptr;
-  mCodecStates.Get(serial, &state);
+  mCodecStore.Add(serial, codecState.forget());
+  state = mCodecStore.Get(serial);
 
-  NS_ENSURE_TRUE(state, false);
+  NS_ENSURE_TRUE(state != nullptr, false);
 
   if (NS_FAILED(state->PageIn(&page))) {
     return false;
   }
 
   if ((newVorbisState && ReadHeaders(newVorbisState)) &&
       (mVorbisState->mInfo.rate == newVorbisState->mInfo.rate) &&
       (mVorbisState->mInfo.channels == newVorbisState->mInfo.channels)) {
@@ -915,17 +911,17 @@ ogg_packet* OggReader::NextOggPacket(Ogg
     // page from the channel.
     ogg_page page;
     if (ReadOggPage(&page) == -1) {
       return nullptr;
     }
 
     uint32_t serial = ogg_page_serialno(&page);
     OggCodecState* codecState = nullptr;
-    mCodecStates.Get(serial, &codecState);
+    codecState = mCodecStore.Get(serial);
     if (codecState && NS_FAILED(codecState->PageIn(&page))) {
       return nullptr;
     }
   }
 
   return packet;
 }
 
@@ -1084,17 +1080,17 @@ int64_t OggReader::RangeEndTime(int64_t 
       mustBackOff = true;
       continue;
     }
 
     int64_t granulepos = ogg_page_granulepos(&page);
     int serial = ogg_page_serialno(&page);
 
     OggCodecState* codecState = nullptr;
-    mCodecStates.Get(serial, &codecState);
+    codecState = mCodecStore.Get(serial);
 
     if (!codecState) {
       // This page is from a bitstream which we haven't encountered yet.
       // It's probably from a new "link" in a "chained" ogg. Don't
       // bother even trying to find a duration...
       SetChained(true);
       endTime = -1;
       break;
@@ -1245,18 +1241,17 @@ OggReader::IndexedSeekResult OggReader::
     return RollbackIndexedSeek(tell);
   }
   uint32_t serial = ogg_page_serialno(&page);
   if (serial != keyframe.mSerial) {
     // Serialno of page at offset isn't what the index told us to expect.
     // Assume the index is invalid.
     return RollbackIndexedSeek(tell);
   }
-  OggCodecState* codecState = nullptr;
-  mCodecStates.Get(serial, &codecState);
+  OggCodecState* codecState = mCodecStore.Get(serial);
   if (codecState &&
       codecState->mActive &&
       ogg_stream_pagein(&codecState->mState, &page) != 0)
   {
     // Couldn't insert page into the ogg resource, or somehow the resource
     // is no longer active.
     return RollbackIndexedSeek(tell);
   }
@@ -1635,18 +1630,17 @@ nsresult OggReader::SeekBisection(int64_
 
       // Read pages until we can determine the granule time of the audio and 
       // video bitstream.
       ogg_int64_t audioTime = -1;
       ogg_int64_t videoTime = -1;
       do {
         // Add the page to its codec state, determine its granule time.
         uint32_t serial = ogg_page_serialno(&page);
-        OggCodecState* codecState = nullptr;
-        mCodecStates.Get(serial, &codecState);
+        OggCodecState* codecState = mCodecStore.Get(serial);
         if (codecState && codecState->mActive) {
           int ret = ogg_stream_pagein(&codecState->mState, &page);
           NS_ENSURE_TRUE(ret == 0, NS_ERROR_FAILURE);
         }
 
         ogg_int64_t granulepos = ogg_page_granulepos(&page);
 
         if (HasAudio() && granulepos > 0 && audioTime == -1) {
@@ -1842,17 +1836,17 @@ nsresult OggReader::GetBuffered(TimeRang
       else if (mOpusState && serial == mOpusSerial) {
         startTime = OpusState::Time(mOpusPreSkip, granulepos);
         NS_ASSERTION(startTime > 0, "Must have positive start time");
       }
       else if (mTheoraState && serial == mTheoraSerial) {
         startTime = TheoraState::Time(&mTheoraInfo, granulepos);
         NS_ASSERTION(startTime > 0, "Must have positive start time");
       }
-      else if (IsKnownStream(serial)) {
+      else if (mCodecStore.Contains(serial)) {
         // Stream is not the theora or vorbis stream we're playing,
         // but is one that we have header data for.
         startOffset += page.header_len + page.body_len;
         continue;
       }
       else {
         // Page is for a stream we don't know about (possibly a chained
         // ogg), return OK to abort the finding any further ranges. This
@@ -1873,22 +1867,34 @@ nsresult OggReader::GetBuffered(TimeRang
       }
     }
   }
 
   return NS_OK;
 #endif
 }
 
-bool OggReader::IsKnownStream(uint32_t aSerial)
+OggCodecStore::OggCodecStore()
+: mMonitor("CodecStore")
+{
+  mCodecStates.Init();
+}
+
+void OggCodecStore::Add(uint32_t serial, OggCodecState* codecState)
 {
-  for (uint32_t i = 0; i < mKnownStreams.Length(); i++) {
-    uint32_t serial = mKnownStreams[i];
-    if (serial == aSerial) {
-      return true;
-    }
-  }
+  MonitorAutoLock mon(mMonitor);
+  mCodecStates.Put(serial, codecState);
+}
 
-  return false;
+bool OggCodecStore::Contains(uint32_t serial)
+{
+  MonitorAutoLock mon(mMonitor);
+  return mCodecStates.Get(serial, nullptr);
+}
+
+OggCodecState* OggCodecStore::Get(uint32_t serial)
+{
+  MonitorAutoLock mon(mMonitor);
+  return mCodecStates.Get(serial);
 }
 
 } // namespace mozilla
 
--- a/content/media/ogg/OggReader.h
+++ b/content/media/ogg/OggReader.h
@@ -11,25 +11,45 @@
 #ifdef MOZ_TREMOR
 #include <tremor/ivorbiscodec.h>
 #else
 #include <vorbis/codec.h>
 #endif
 #include "MediaDecoderReader.h"
 #include "OggCodecState.h"
 #include "VideoUtils.h"
+#include "mozilla/Monitor.h"
 
 namespace mozilla {
 namespace dom {
 class TimeRanges;
 }
 }
 
 namespace mozilla {
 
+// Thread safe container to store the codec information and the serial for each
+// streams.
+class OggCodecStore
+{
+  public:
+    OggCodecStore();
+    void Add(uint32_t serial, OggCodecState* codecState);
+    bool Contains(uint32_t serial);
+    OggCodecState* Get(uint32_t serial);
+    bool IsKnownStream(uint32_t aSerial);
+
+  private:
+    // Maps Ogg serialnos to OggStreams.
+    nsClassHashtable<nsUint32HashKey, OggCodecState> mCodecStates;
+
+    // Protects the |mCodecStates| and the |mKnownStreams| members.
+    Monitor mMonitor;
+};
+
 class OggReader : public MediaDecoderReader
 {
 public:
   OggReader(AbstractMediaDecoder* aDecoder);
   ~OggReader();
 
   virtual nsresult Init(MediaDecoderReader* aCloneDonor);
   virtual nsresult ResetDecode();
@@ -228,24 +248,17 @@ private:
   // pointer to an ogg_packet on success, or nullptr if the read failed.
   // The caller is responsible for deleting the packet and its |packet| field.
   ogg_packet* NextOggPacket(OggCodecState* aCodecState);
 
   // Fills aTracks with the serial numbers of each active stream, for use by
   // various SkeletonState functions.
   void BuildSerialList(nsTArray<uint32_t>& aTracks);
 
-  // Maps Ogg serialnos to nsOggStreams.
-  nsClassHashtable<nsUint32HashKey, OggCodecState> mCodecStates;
-
-  // Array of serial numbers of streams that were encountered during
-  // initial metadata load. Written on state machine thread during
-  // metadata loading and read on the main thread only after metadata
-  // is loaded.
-  nsAutoTArray<uint32_t,4> mKnownStreams;
+  OggCodecStore mCodecStore;
 
   // Decode state of the Theora bitstream we're decoding, if we have video.
   TheoraState* mTheoraState;
 
   // Decode state of the Vorbis bitstream we're decoding, if we have audio.
   VorbisState* mVorbisState;
 
   // Decode state of the Opus bitstream we're decoding, if we have one.