Bug 1349421 - Update handling of values derived from chunk duration in OpusTrackEncoder. r=derf, a=gchang
authorBryce Van Dyk <bvandyk@mozilla.com>
Fri, 31 Mar 2017 07:57:37 +1300
changeset 393364 6fdfffd25064157b94f9a4e0f9a10c7634cdc564
parent 393363 d5a55164125fefa075ee1b6950c7b920686325f0
child 393365 119d04506d979ee0147d6b2abc0a933ac30bccbe
push id7198
push userjlorenzo@mozilla.com
push dateTue, 18 Apr 2017 12:07:49 +0000
treeherdermozilla-beta@d57aa49c3948 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersderf, gchang
bugs1349421
milestone54.0a2
Bug 1349421 - Update handling of values derived from chunk duration in OpusTrackEncoder. r=derf, a=gchang This changeset updates the handling of the variables based on chunk.GetDuration() within the OpusTrackEncoder. This changeset alters places where overflow could have taken place previously. It also adds asserts with the dual purpose of defense and documentation for future developers. MozReview-Commit-ID: 28vmAfE84ik
dom/media/encoder/OpusTrackEncoder.cpp
--- a/dom/media/encoder/OpusTrackEncoder.cpp
+++ b/dom/media/encoder/OpusTrackEncoder.cpp
@@ -330,41 +330,52 @@ OpusTrackEncoder::GetEncodedTrack(Encode
     pcm.SetLength(GetPacketDuration() * mChannels);
     AudioSegment::ChunkIterator iter(mSourceSegment);
     int frameCopied = 0;
 
     while (!iter.IsEnded() && frameCopied < framesToFetch) {
       AudioChunk chunk = *iter;
 
       // Chunk to the required frame size.
-      int frameToCopy = chunk.GetDuration();
-      if (frameCopied + frameToCopy > framesToFetch) {
+      StreamTime frameToCopy = chunk.GetDuration();
+      if (frameToCopy > framesToFetch - frameCopied) {
         frameToCopy = framesToFetch - frameCopied;
       }
+      // Possible greatest value of framesToFetch = 3844: see
+      // https://bugzilla.mozilla.org/show_bug.cgi?id=1349421#c8. frameToCopy
+      // should not be able to exceed this value.
+      MOZ_ASSERT(frameToCopy <= 3844, "frameToCopy exceeded expected range");
 
       if (!chunk.IsNull()) {
         // Append the interleaved data to the end of pcm buffer.
         AudioTrackEncoder::InterleaveTrackData(chunk, frameToCopy, mChannels,
                                                pcm.Elements() + frameCopied * mChannels);
       } else {
         CheckedInt<int> memsetLength = CheckedInt<int>(frameToCopy) *
                                        mChannels *
                                        sizeof(AudioDataValue);
         if (!memsetLength.isValid()) {
-          LOG(LogLevel::Error, ("Error calculating length of pcm buffer!"));
+          // This should never happen, but we use a defensive check because
+          // we really don't want a bad memset
+          MOZ_ASSERT_UNREACHABLE("memsetLength invalid!");
           return NS_ERROR_FAILURE;
         }
         memset(pcm.Elements() + frameCopied * mChannels, 0,
                memsetLength.value());
       }
 
       frameCopied += frameToCopy;
       iter.Next();
     }
 
+    // Possible greatest value of framesToFetch = 3844: see
+    // https://bugzilla.mozilla.org/show_bug.cgi?id=1349421#c8. frameCopied
+    // should not be able to exceed this value.
+    MOZ_ASSERT(frameCopied <= 3844, "frameCopied exceeded expected range");
+
     RefPtr<EncodedFrame> audiodata = new EncodedFrame();
     audiodata->SetFrameType(EncodedFrame::OPUS_AUDIO_FRAME);
     int framesInPCM = frameCopied;
     if (mResampler) {
       AutoTArray<AudioDataValue, 9600> resamplingDest;
       // We want to consume all the input data, so we slightly oversize the
       // resampled data buffer so we can fit the output data in. We cannot really
       // predict the output frame count at each call.