Bug 1594067 - Fix RemoveTrailing to handle correctly keep frames equals to zero. r=padenot

Fix `MediaSegmentBase::RemoveTrailing` to be able to accept the first argument, keep frames, equal to zero. The patch avoids calling the `AudioChunk::SliceTo()` method with zero slice duration which hits an assert. The crash was being triggered when in the AudioSegment was including one or more chunks, with the first chunk containing silence (null). Then the `AudioSegment::FlushAfter` had to be called with a duration smaller or equal to the duration of the first chunk. A unit test has been created, verifying the duration of the final segment.

Differential Revision: https://phabricator.services.mozilla.com/D52092

--HG--
extra : moz-landing-system : lando

dom/media/MediaSegment.h

@@ -451,19 +451,17 @@ class MediaSegmentBase : public MediaSegment {
     NS_ASSERTION(aKeep >= 0, "Can't keep negative duration");
     TrackTime t = aKeep;
     uint32_t i;
-    for (i = aStartIndex; i < mChunks.Length(); ++i) {
+    for (i = aStartIndex; i < mChunks.Length() && t; ++i) {
       Chunk* c = &mChunks[i];
       if (c->GetDuration() > t) {
         c->SliceTo(0, t);
         break;
       }
       t -= c->GetDuration();
-      if (t == 0) {
-        break;
-      }
     }
-    if (i + 1 < mChunks.Length()) {
-      mChunks.RemoveElementsAt(i + 1, mChunks.Length() - (i + 1));
+    // At this point `i` is already advanced due to last check in the loop.
+    if (i < mChunks.Length()) {
+      mChunks.RemoveElementsAt(i, mChunks.Length() - i);
     }
     MOZ_ASSERT(mChunks.Capacity() >= DEFAULT_SEGMENT_CAPACITY,
                "Capacity must be retained after removing chunks");

dom/media/gtest/TestAudioSegment.cpp

@@ -263,7 +263,7 @@ void fillChunkWithStereo(AudioChunk* c, int duration) {
   c->mBufferFormat = AUDIO_FORMAT_FLOAT32;
 }
 
-TEST(AudioSegment, FlushAfterZero)
+TEST(AudioSegment, FlushAfter_ZeroDuration)
 {
   AudioChunk c;
   fillChunkWithStereo<float>(&c, 10);
@@ -274,4 +274,30 @@ TEST(AudioSegment, FlushAfterZero)
   EXPECT_EQ(s.GetDuration(), 0);
 }
 
+TEST(AudioSegment, FlushAfter_SmallerDuration)
+{
+  // It was crashing when the first chunk was silence (null) and FlushAfter
+  // was called for a duration, smaller or equal to the duration of the
+  // first chunk.
+  TrackTime duration = 10;
+  TrackTime smaller_duration = 8;
+  AudioChunk c1;
+  c1.SetNull(duration);
+  AudioChunk c2;
+  fillChunkWithStereo<float>(&c2, duration);
+
+  AudioSegment s;
+  s.AppendAndConsumeChunk(&c1);
+  s.AppendAndConsumeChunk(&c2);
+  s.FlushAfter(smaller_duration);
+  EXPECT_EQ(s.GetDuration(), smaller_duration) << "Check new duration";
+
+  TrackTime chunkByChunkDuration = 0;
+  for (AudioSegment::ChunkIterator iter(s); !iter.IsEnded(); iter.Next()) {
+    chunkByChunkDuration += iter->GetDuration();
+  }
+  EXPECT_EQ(s.GetDuration(), chunkByChunkDuration)
+      << "Confirm duration chunk by chunk";
+}
+
 }  // namespace audio_segment