b=1016177 reduce MEDIA_TIME_MAX to not overflow when multiplying by sample rates r=roc
authorKarl Tomlinson <karlt+@karlt.net>
Thu, 05 Jun 2014 22:08:05 +1200
changeset 206264 58f1f6dbe7ce9b178e9fe300a66da93522b9d5de
parent 206263 80d207a21b88667053b456eb0c8d43ab713b8837
child 206265 3f549afdb1b725cfbec523701706256d7985650d
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc
bugs1016177
milestone32.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
b=1016177 reduce MEDIA_TIME_MAX to not overflow when multiplying by sample rates r=roc MediaTimes are multiplied by sample rates when converting to ticks. This new maximum leaves 24 bits to count seconds, which corresponds to 194 days. TimeVarying::GetAt() can return INT64_MAX, which may be > GRAPH_TIME_MAX. The "Start time too early" assertion would now fail because conversion of STREAM_TIME_MAX to ticks no longer overflows.
content/media/MediaSegment.h
content/media/MediaStreamGraph.cpp
content/media/StreamBuffer.h
--- a/content/media/MediaSegment.h
+++ b/content/media/MediaSegment.h
@@ -11,22 +11,32 @@
 #include "mozilla/TimeStamp.h"
 #endif
 #include <algorithm>
 #include "Latency.h"
 
 namespace mozilla {
 
 /**
+ * Track rate in Hz. Maximum 1 << TRACK_RATE_MAX_BITS Hz. This
+ * maximum avoids overflow in conversions between track rates and conversions
+ * from seconds.
+ */
+typedef int32_t TrackRate;
+const int64_t TRACK_RATE_MAX_BITS = 20;
+const TrackRate TRACK_RATE_MAX = 1 << TRACK_RATE_MAX_BITS;
+
+/**
  * We represent media times in 64-bit fixed point. So 1 MediaTime is
- * 1/(2^MEDIA_TIME_FRAC_BITS) seconds.
+ * 1/(2^MEDIA_TIME_FRAC_BITS) seconds.  We want to make sure that multiplying
+ * MediaTime by a TrackRate doesn't overflow, so we set its max accordingly.
  */
 typedef int64_t MediaTime;
 const int64_t MEDIA_TIME_FRAC_BITS = 20;
-const int64_t MEDIA_TIME_MAX = INT64_MAX;
+const int64_t MEDIA_TIME_MAX = INT64_MAX >> TRACK_RATE_MAX_BITS;
 
 inline MediaTime MillisecondsToMediaTime(int32_t aMS)
 {
   return (MediaTime(aMS) << MEDIA_TIME_FRAC_BITS)/1000;
 }
 
 inline MediaTime SecondsToMediaTime(double aS)
 {
--- a/content/media/MediaStreamGraph.cpp
+++ b/content/media/MediaStreamGraph.cpp
@@ -2512,17 +2512,17 @@ MediaInputPort::InputInterval
 MediaInputPort::GetNextInputInterval(GraphTime aTime)
 {
   InputInterval result = { GRAPH_TIME_MAX, GRAPH_TIME_MAX, false };
   GraphTime t = aTime;
   GraphTime end;
   for (;;) {
     if (!mDest->mBlocked.GetAt(t, &end))
       break;
-    if (end == GRAPH_TIME_MAX)
+    if (end >= GRAPH_TIME_MAX)
       return result;
     t = end;
   }
   result.mStart = t;
   GraphTime sourceEnd;
   result.mInputIsBlocked = mSource->mBlocked.GetAt(t, &sourceEnd);
   result.mEnd = std::min(end, sourceEnd);
   return result;
--- a/content/media/StreamBuffer.h
+++ b/content/media/StreamBuffer.h
@@ -13,23 +13,16 @@ namespace mozilla {
 
 /**
  * Media time relative to the start of a StreamBuffer.
  */
 typedef MediaTime StreamTime;
 const StreamTime STREAM_TIME_MAX = MEDIA_TIME_MAX;
 
 /**
- * Track rate in Hz. Maximum 1 << MEDIA_TIME_FRAC_BITS Hz. This ensures
- * calculations below don't overflow.
- */
-typedef int32_t TrackRate;
-const TrackRate TRACK_RATE_MAX = 1 << MEDIA_TIME_FRAC_BITS;
-
-/**
  * Unique ID for track within a StreamBuffer. Tracks from different
  * StreamBuffers may have the same ID; this matters when appending StreamBuffers,
  * since tracks with the same ID are matched. Only IDs greater than 0 are allowed.
  */
 typedef int32_t TrackID;
 const TrackID TRACK_NONE = 0;
 
 inline TrackTicks TimeToTicksRoundUp(TrackRate aRate, StreamTime aTime)
@@ -219,18 +212,24 @@ public:
 
   /**
    * Takes ownership of aSegment. Don't do this while iterating, or while
    * holding a Track reference.
    * aSegment must have aStart worth of null data.
    */
   Track& AddTrack(TrackID aID, TrackRate aRate, TrackTicks aStart, MediaSegment* aSegment)
   {
-    NS_ASSERTION(TimeToTicksRoundDown(aRate, mTracksKnownTime) <= aStart,
-                 "Start time too early");
+    if (mTracksKnownTime == STREAM_TIME_MAX) {
+      // There exists code like
+      // http://mxr.mozilla.org/mozilla-central/source/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp?rev=96b197deb91e&mark=1292-1297#1292
+      NS_WARNING("Adding track to StreamBuffer that should have no more tracks");
+    } else {
+      NS_ASSERTION(TimeToTicksRoundDown(aRate, mTracksKnownTime) <= aStart,
+                   "Start time too early");
+    }
     NS_ASSERTION(!FindTrack(aID), "Track with this ID already exists");
 
     return **mTracks.InsertElementSorted(new Track(aID, aRate, aStart, aSegment),
                                          CompareTracksByID());
   }
   void AdvanceKnownTracksTime(StreamTime aKnownTime)
   {
     NS_ASSERTION(aKnownTime >= mTracksKnownTime, "Can't move tracks-known time earlier");