Bug 843214. Make SourceMediaStream::AddTrack/AppendToTrack/HaveEnoughBuffered/DispatchWhenNotEnoughBuffered/EndTrack smoothly handle cases where track does not exist. r=jesup
☠☠ backed out by 786e3cd28515 ☠ ☠
authorRobert O'Callahan <robert@ocallahan.org>
Mon, 25 Feb 2013 22:25:07 +1300
changeset 123144 dba7a059ed222ed1515a878a3e3b906a55850aeb
parent 123143 05c35dc73323764e05fc366cc8d2fd5274bdd598
child 123145 786e3cd28515715ac78bbd46667f58b62bb1e51e
push id24372
push useremorley@mozilla.com
push dateWed, 27 Feb 2013 13:22:59 +0000
treeherdermozilla-central@0a91da5f5eab [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjesup
bugs843214
milestone22.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 843214. Make SourceMediaStream::AddTrack/AppendToTrack/HaveEnoughBuffered/DispatchWhenNotEnoughBuffered/EndTrack smoothly handle cases where track does not exist. r=jesup
content/media/MediaSegment.h
content/media/MediaStreamGraph.cpp
content/media/MediaStreamGraph.h
content/media/webrtc/MediaEngineWebRTCVideo.cpp
--- a/content/media/MediaSegment.h
+++ b/content/media/MediaSegment.h
@@ -95,16 +95,20 @@ public:
   /**
    * Insert aDuration of null data at the start of the segment.
    */
   virtual void InsertNullDataAtStart(TrackTicks aDuration) = 0;
   /**
    * Insert aDuration of null data at the end of the segment.
    */
   virtual void AppendNullData(TrackTicks aDuration) = 0;
+  /**
+   * Remove all contents, setting duration to 0.
+   */
+  virtual void Clear() = 0;
 
 protected:
   MediaSegment(Type aType) : mDuration(0), mType(aType)
   {
     MOZ_COUNT_CTOR(MediaSegment);
   }
 
   TrackTicks mDuration; // total of mDurations of all chunks
@@ -181,16 +185,21 @@ public:
     }
     if (!mChunks.IsEmpty() && mChunks[mChunks.Length() - 1].IsNull()) {
       mChunks[mChunks.Length() - 1].mDuration += aDuration;
     } else {
       mChunks.AppendElement()->SetNull(aDuration);
     }
     mDuration += aDuration;
   }
+  virtual void Clear()
+  {
+    mDuration = 0;
+    mChunks.Clear();
+  }
 
   class ChunkIterator {
   public:
     ChunkIterator(MediaSegmentBase<C, Chunk>& aSegment)
       : mSegment(aSegment), mIndex(0) {}
     bool IsEnded() { return mIndex >= mSegment.mChunks.Length(); }
     void Next() { ++mIndex; }
     Chunk& operator*() { return mSegment.mChunks[mIndex]; }
--- a/content/media/MediaStreamGraph.cpp
+++ b/content/media/MediaStreamGraph.cpp
@@ -113,16 +113,19 @@ MediaStreamGraphImpl::ExtractPendingInpu
     if (aStream->mPullEnabled && !aStream->mFinished &&
         !aStream->mListeners.IsEmpty()) {
       // Compute how much stream time we'll need assuming we don't block
       // the stream at all between mBlockingDecisionsMadeUntilTime and
       // aDesiredUpToTime.
       StreamTime t =
         GraphTimeToStreamTime(aStream, mStateComputedTime) +
         (aDesiredUpToTime - mStateComputedTime);
+      LOG(PR_LOG_DEBUG, ("Calling NotifyPull aStream=%p t=%f current end=%f", aStream,
+	                     MediaTimeToSeconds(t),
+						 MediaTimeToSeconds(aStream->mBuffer.GetEnd())));
       if (t > aStream->mBuffer.GetEnd()) {
         *aEnsureNextIteration = true;
         for (uint32_t j = 0; j < aStream->mListeners.Length(); ++j) {
           MediaStreamListener* l = aStream->mListeners[j];
           {
             MutexAutoUnlock unlock(aStream->mMutex);
             l->NotifyPull(this, t);
           }
@@ -1632,54 +1635,56 @@ SourceMediaStream::AddTrack(TrackID aID,
   data->mCommands = TRACK_CREATE;
   data->mData = aSegment;
   data->mHaveEnough = false;
   if (!mDestroyed) {
     GraphImpl()->EnsureNextIteration();
   }
 }
 
-void
+bool
 SourceMediaStream::AppendToTrack(TrackID aID, MediaSegment* aSegment)
 {
   MutexAutoLock lock(mMutex);
   // ::EndAllTrackAndFinished() can end these before the sources notice
+  bool appended = false;
   if (!mFinished) {
     TrackData *track = FindDataForTrack(aID);
     if (track) {
       track->mData->AppendFrom(aSegment);
+	  appended = true;
     } else {
-      NS_ERROR("Append to non-existent track!");
-    }
+	  aSegment->Clear();
+	}
   }
   if (!mDestroyed) {
     GraphImpl()->EnsureNextIteration();
   }
+  return appended;
 }
 
 bool
 SourceMediaStream::HaveEnoughBuffered(TrackID aID)
 {
   MutexAutoLock lock(mMutex);
   TrackData *track = FindDataForTrack(aID);
   if (track) {
     return track->mHaveEnough;
   }
-  NS_ERROR("No track in HaveEnoughBuffered!");
-  return true;
+  return false;
 }
 
 void
 SourceMediaStream::DispatchWhenNotEnoughBuffered(TrackID aID,
     nsIThread* aSignalThread, nsIRunnable* aSignalRunnable)
 {
   MutexAutoLock lock(mMutex);
   TrackData* data = FindDataForTrack(aID);
   if (!data) {
-    NS_ERROR("No track in DispatchWhenNotEnoughBuffered");
+    aSignalThread->Dispatch(aSignalRunnable, 0);
     return;
   }
 
   if (data->mHaveEnough) {
     data->mDispatchWhenNotEnough.AppendElement()->Init(aSignalThread, aSignalRunnable);
   } else {
     aSignalThread->Dispatch(aSignalRunnable, 0);
   }
@@ -1689,18 +1694,16 @@ void
 SourceMediaStream::EndTrack(TrackID aID)
 {
   MutexAutoLock lock(mMutex);
   // ::EndAllTrackAndFinished() can end these before the sources call this
   if (!mFinished) {
     TrackData *track = FindDataForTrack(aID);
     if (track) {
       track->mCommands |= TRACK_END;
-    } else {
-      NS_ERROR("End of non-existant track");
     }
   }
   if (!mDestroyed) {
     GraphImpl()->EnsureNextIteration();
   }
 }
 
 void
--- a/content/media/MediaStreamGraph.h
+++ b/content/media/MediaStreamGraph.h
@@ -562,32 +562,37 @@ public:
    * AdvanceKnownTracksTime). Takes ownership of aSegment. aSegment should
    * contain data starting after aStart.
    */
   void AddTrack(TrackID aID, TrackRate aRate, TrackTicks aStart,
                 MediaSegment* aSegment);
   /**
    * Append media data to a track. Ownership of aSegment remains with the caller,
    * but aSegment is emptied.
+   * Returns false if the data was not appended because no such track exists
+   * or the stream was already finished.
    */
-  void AppendToTrack(TrackID aID, MediaSegment* aSegment);
+  bool AppendToTrack(TrackID aID, MediaSegment* aSegment);
   /**
    * Returns true if the buffer currently has enough data.
+   * Returns false if there isn't enough data or if no such track exists.
    */
   bool HaveEnoughBuffered(TrackID aID);
   /**
    * Ensures that aSignalRunnable will be dispatched to aSignalThread
    * when we don't have enough buffered data in the track (which could be
-   * immediately).
+   * immediately). Will dispatch the runnable immediately if the track
+   * does not exist.
    */
   void DispatchWhenNotEnoughBuffered(TrackID aID,
       nsIThread* aSignalThread, nsIRunnable* aSignalRunnable);
   /**
    * Indicate that a track has ended. Do not do any more API calls
    * affecting this track.
+   * Ignored if the track does not exist.
    */
   void EndTrack(TrackID aID);
   /**
    * Indicate that no tracks will be added starting before time aKnownTime.
    * aKnownTime must be >= its value at the last call to AdvanceKnownTracksTime.
    */
   void AdvanceKnownTracksTime(StreamTime aKnownTime);
   /**
@@ -648,17 +653,16 @@ public:
 protected:
   TrackData* FindDataForTrack(TrackID aID)
   {
     for (uint32_t i = 0; i < mUpdateTracks.Length(); ++i) {
       if (mUpdateTracks[i].mID == aID) {
         return &mUpdateTracks[i];
       }
     }
-    NS_ERROR("Bad track ID!");
     return nullptr;
   }
 
   // Media stream graph thread only
   MediaStreamListener::Consumption mLastConsumptionState;
 
   // This must be acquired *before* MediaStreamGraphImpl's lock, if they are
   // held together.
--- a/content/media/webrtc/MediaEngineWebRTCVideo.cpp
+++ b/content/media/webrtc/MediaEngineWebRTCVideo.cpp
@@ -119,18 +119,21 @@ MediaEngineWebRTCVideoSource::NotifyPull
   TrackTicks delta = target - aLastEndTime;
   LOGFRAME(("NotifyPull, desired = %ld, target = %ld, delta = %ld %s", (int64_t) aDesiredTime, 
             (int64_t) target, (int64_t) delta, image ? "" : "<null>"));
   // Don't append if we've already provided a frame that supposedly goes past the current aDesiredTime
   // Doing so means a negative delta and thus messes up handling of the graph
   if (delta > 0) {
     // NULL images are allowed
     segment.AppendFrame(image ? image.forget() : nullptr, delta, gfxIntSize(mWidth, mHeight));
-    aSource->AppendToTrack(aID, &(segment));
-    aLastEndTime = target;
+    // This can fail if either a) we haven't added the track yet, or b)
+    // we've removed or finished the track.
+    if (aSource->AppendToTrack(aID, &(segment))) {
+      aLastEndTime = target;
+    }
   }
 }
 
 void
 MediaEngineWebRTCVideoSource::ChooseCapability(uint32_t aWidth, uint32_t aHeight, uint32_t aMinFPS)
 {
   int num = mViECapture->NumberOfCapabilities(mUniqueId, KMaxUniqueIdLength);