MSE: Try to coalesce buffered ranges when fudge room changes

Before this change, an unsorted sequence of buffered ranges could
result from complex scenarios (in both ByDts and ByPts API usage).
A simple example of such unsorted ranges is [a,b) [c,d), where b > c.

This change prevents one known cause of unsorted ranges: when buffering
a set of frames emitted from FrameProcessor and those frames change the
fudge room, use the new fudge room to try to coalesce any existing
buffered ranges before buffering the new frames. This allows the
buffering code to correctly identify the (at most one) existing buffered
range to which a new buffer or coded frame group start time belongs.
Code already exists to ensure that such fudge room changes are only
increases.

BUG=793247
TEST=SourceBufferStreamTest.RangeCoalescenceOnFudgeRoomIncrease_*, and no
more local repro of fuzzer case in crbug 793247

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: If6f78d56080ecb9bbdaef9df554c07f999d9a0ea
Reviewed-on: https://chromium-review.googlesource.com/820656Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523641}

media/filters/source_buffer_stream.cc

@@ -329,7 +329,12 @@ bool SourceBufferStream<RangeClass>::Append(const BufferQueue& buffers) {
     return false;
   }
 
-  UpdateMaxInterbufferDtsDistance(buffers);
+  if (UpdateMaxInterbufferDtsDistance(buffers)) {
+    // Coalesce |ranges_| using the new fudge room. This helps keep |ranges_|
+    // sorted in complex scenarios.  See https://crbug.com/793247.
+    MergeAllAdjacentRanges();
+  }
+
   SetConfigIds(buffers);
 
   // Save a snapshot of stream state before range modifications are made.
@@ -760,9 +765,10 @@ bool SourceBufferStream<RangeClass>::OnlySelectedRangeIsSeeked() const {
 }
 
 template <typename RangeClass>
-void SourceBufferStream<RangeClass>::UpdateMaxInterbufferDtsDistance(
+bool SourceBufferStream<RangeClass>::UpdateMaxInterbufferDtsDistance(
     const BufferQueue& buffers) {
   DCHECK(!buffers.empty());
+  base::TimeDelta old_distance = max_interbuffer_distance_;
   DecodeTimestamp prev_timestamp = last_appended_buffer_decode_timestamp_;
   for (BufferQueue::const_iterator itr = buffers.begin();
        itr != buffers.end(); ++itr) {
@@ -783,6 +789,13 @@ void SourceBufferStream<RangeClass>::UpdateMaxInterbufferDtsDistance(
         std::max(max_interbuffer_distance_, interbuffer_distance);
     prev_timestamp = current_timestamp;
   }
+  bool changed_max = max_interbuffer_distance_ != old_distance;
+  DVLOG_IF(2, changed_max) << __func__ << " " << GetStreamTypeName()
+                           << " Changed max interbuffer DTS distance from "
+                           << old_distance.InMicroseconds() << "us to "
+                           << max_interbuffer_distance_.InMicroseconds()
+                           << "us";
+  return changed_max;
 }
 
 template <typename RangeClass>
@@ -1497,6 +1510,22 @@ void SourceBufferStream<RangeClass>::MergeWithAdjacentRangeIfNecessary(
   DeleteAndRemoveRange(&next_range_itr);
 }
 
+template <typename RangeClass>
+void SourceBufferStream<RangeClass>::MergeAllAdjacentRanges() {
+  DVLOG(1) << __func__ << " " << GetStreamTypeName()
+           << ": Before: ranges_=" << RangesToString<RangeClass>(ranges_);
+
+  typename RangeList::iterator range_itr = ranges_.begin();
+
+  while (range_itr != ranges_.end()) {
+    MergeWithAdjacentRangeIfNecessary(range_itr);
+    range_itr++;
+  }
+
+  DVLOG(1) << __func__ << " " << GetStreamTypeName()
+           << ": After: ranges_=" << RangesToString<RangeClass>(ranges_);
+}
+
 template <typename RangeClass>
 void SourceBufferStream<RangeClass>::Seek(base::TimeDelta timestamp) {
   DCHECK(timestamp >= base::TimeDelta());

media/filters/source_buffer_stream.h

@@ -225,10 +225,15 @@ class MEDIA_EXPORT SourceBufferStream {
   void PruneTrackBuffer(const DecodeTimestamp timestamp);
 
   // Checks to see if |range_with_new_buffers_itr| can be merged with the range
-  // next to it, and merges them if so.
+  // next to it, and merges them if so while preserving correctness of
+  // |range_for_next_append_| and |selected_range_|.
   void MergeWithAdjacentRangeIfNecessary(
       const typename RangeList::iterator& range_with_new_buffers_itr);
 
+  // Merges any adjacent ranges while preserving correctness of
+  // |range_for_next_append_| and |selected_range_|.
+  void MergeAllAdjacentRanges();
+
   // Returns true if |next_gop_timestamp| follows
   // |highest_timestamp_in_append_sequence_| within fudge room.
   bool IsNextGopAdjacentToEndOfCurrentAppendSequence(
@@ -285,8 +290,9 @@ class MEDIA_EXPORT SourceBufferStream {
 
   // Measures the distances between buffer decode timestamps and tracks the max.
   // This enables a reasonable approximation of adjacency fudge room, even for
-  // out-of-order PTS vs DTS sequences.
-  void UpdateMaxInterbufferDtsDistance(const BufferQueue& buffers);
+  // out-of-order PTS vs DTS sequences. Returns true if
+  // |max_interbuffer_distance_| was changed.
+  bool UpdateMaxInterbufferDtsDistance(const BufferQueue& buffers);
 
   // Sets the config ID for each buffer to |append_config_index_|.
   void SetConfigIds(const BufferQueue& buffers);

media/filters/source_buffer_stream_unittest.cc

@@ -5477,6 +5477,62 @@ TEST_P(SourceBufferStreamTest, RangeIsNextInPTS_OutOfOrder) {
   CheckIsNextInPTSSequenceWithFirstRange(1181, false);
 }
 
+TEST_P(SourceBufferStreamTest, RangeCoalescenceOnFudgeRoomIncrease_1) {
+  // Change the fudge room (by increasing frame duration) and verify coalescence
+  // behavior.
+  NewCodedFrameGroupAppend("0K 10K");
+  NewCodedFrameGroupAppend("100K 110K");
+  NewCodedFrameGroupAppend("500K 510K");
+  CheckExpectedRangesByTimestamp("{ [0,20) [100,120) [500,520) }");
+
+  // Increase the fudge room almost enough to merge the first two buffered
+  // ranges.
+  NewCodedFrameGroupAppend("1000D44K");
+  CheckExpectedRangesByTimestamp("{ [0,20) [100,120) [500,520) [1000,1044) }");
+
+  // Increase the fudge room again to merge the first two buffered ranges.
+  NewCodedFrameGroupAppend("2000D45K");
+  CheckExpectedRangesByTimestamp(
+      "{ [0,120) [500,520) [1000,1044) [2000,2045) }");
+
+  SeekToTimestampMs(0);
+  CheckExpectedBuffers("0K 10K 100K 110K");
+  CheckNoNextBuffer();
+  SeekToTimestampMs(500);
+  CheckExpectedBuffers("500K 510K");
+  CheckNoNextBuffer();
+  SeekToTimestampMs(1000);
+  CheckExpectedBuffers("1000K");
+  CheckNoNextBuffer();
+  SeekToTimestampMs(2000);
+  CheckExpectedBuffers("2000K");
+  CheckNoNextBuffer();
+}
+
+TEST_P(SourceBufferStreamTest, RangeCoalescenceOnFudgeRoomIncrease_2) {
+  // Change the fudge room (by increasing frame duration) and verify coalescence
+  // behavior.
+  NewCodedFrameGroupAppend("0K 10K");
+  NewCodedFrameGroupAppend("40K 50K 60K");
+  CheckExpectedRangesByTimestamp("{ [0,20) [40,70) }");
+
+  // Increase the fudge room to merge the first two buffered ranges.
+  NewCodedFrameGroupAppend("1000D20K");
+  CheckExpectedRangesByTimestamp("{ [0,70) [1000,1020) }");
+
+  // Try to trigger unsorted ranges, as might occur if the first two buffered
+  // ranges were not correctly coalesced.
+  NewCodedFrameGroupAppend("45D10K");
+
+  CheckExpectedRangesByTimestamp("{ [0,70) [1000,1020) }");
+  SeekToTimestampMs(0);
+  CheckExpectedBuffers("0K 10K 40K 45K 60K");
+  CheckNoNextBuffer();
+  SeekToTimestampMs(1000);
+  CheckExpectedBuffers("1000K");
+  CheckNoNextBuffer();
+}
+
 INSTANTIATE_TEST_CASE_P(LegacyByDts,
                         SourceBufferStreamTest,
                         Values(BufferingApi::kLegacyByDts));