Bug 1542685 - Avoid integer overflows by multiplying doubles. r=padenot
authorAndreas Pehrson <apehrson@mozilla.com>
Mon, 08 Apr 2019 14:27:13 +0000
changeset 468352 b8855a45d798aac9bdf82cd328eb17375c5510e5
parent 468351 b306079acdcb517d6460e73b1f6a153f6b07baa0
child 468353 d689bb3db16ea6b533e733efd9773cc65d5f1d47
push id35835
push useraciure@mozilla.com
push dateMon, 08 Apr 2019 19:00:29 +0000
treeherdermozilla-central@40456af7da1c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot
bugs1542685
milestone68.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 1542685 - Avoid integer overflows by multiplying doubles. r=padenot SaferMultDiv(time, audioScale, videoScale) could easily result in overflow because all three args are roughly equal, and SaferMultDiv would always do the multiplication first. The worst-case is then multiplying an int64_t to another int64_t that have very similar values. Since we represent time here in microseconds, this would overflow after only 50 minutes. Differential Revision: https://phabricator.services.mozilla.com/D26494
dom/media/DriftCompensation.h
--- a/dom/media/DriftCompensation.h
+++ b/dom/media/DriftCompensation.h
@@ -98,34 +98,35 @@ class DriftCompensator {
     if (samples / mAudioRate < 10) {
       // We don't apply compensation for the first 10 seconds because of the
       // higher inaccuracy during this time.
       LOG(LogLevel::Debug, "DriftCompensator %p %" PRId64 "ms so far; ignoring",
           this, samples * 1000 / mAudioRate);
       return aTime;
     }
 
-    int64_t videoScaleUs = (aNow - mAudioStartTime).ToMicroseconds();
-    int64_t audioScaleUs = FramesToUsecs(samples, mAudioRate).value();
-    int64_t videoDurationUs = (aTime - mAudioStartTime).ToMicroseconds();
-
-    if (videoScaleUs == 0) {
-      videoScaleUs = audioScaleUs;
+    if (aNow == mAudioStartTime) {
+      LOG(LogLevel::Warning,
+          "DriftCompensator %p video scale 0, assuming no drift", this);
+      return aTime;
     }
 
+    double videoScaleUs = (aNow - mAudioStartTime).ToMicroseconds();
+    double audioScaleUs = FramesToUsecs(samples, mAudioRate).value();
+    double videoDurationUs = (aTime - mAudioStartTime).ToMicroseconds();
+
     TimeStamp reclocked =
-        mAudioStartTime +
-        TimeDuration::FromMicroseconds(
-            SaferMultDiv(videoDurationUs, audioScaleUs, videoScaleUs).value());
+        mAudioStartTime + TimeDuration::FromMicroseconds(
+                              videoDurationUs * audioScaleUs / videoScaleUs);
 
     LOG(LogLevel::Debug,
         "DriftCompensator %p GetVideoTime, v-now: %.3fs, a-now: %.3fs; %.3fs "
         "-> %.3fs (d %.3fms)",
         this, (aNow - mAudioStartTime).ToSeconds(),
-        static_cast<double>(audioScaleUs) / 1000000.0,
+        TimeDuration::FromMicroseconds(audioScaleUs).ToSeconds(),
         (aTime - mAudioStartTime).ToSeconds(),
         (reclocked - mAudioStartTime).ToSeconds(),
         (reclocked - aTime).ToMilliseconds());
 
     return reclocked;
   }
 };