MSE: Allow range merge in specific estimated buffer duration scenarios

If a range B starts with the same timestamp as the end of the previous
range A, these would normally merge due to being adjacent within the fudge
room based on the end timestamp of range A (not the buffered end
timestamp of A). This is sufficient for coalescing adjacent ranges in
all cases except where fudge room has not been increased to account for
an increased estimated buffer's duration. In complex overlap/abutting
append scenarios, A and B previously would have remained outside of
fudge room of each other and not have been coalesced, if A ended with an
estimated duration buffer longer than the fudge room.

This change patches CanAppendBuffersToEnd to recognize this specific
scenario and allow A and B to coalesce.

BUG=806181
TEST=CF case no longer repros locally, new SBSTest.MergeAllowedIfRangeEndTimeWithEstimatedDurationMatchesNextRangeStart

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: Ic749c2e4e0bb6cc1b69983efd16ef6e346410c9e
Reviewed-on: https://chromium-review.googlesource.com/898604
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534871}

media/filters/source_buffer_range_by_dts.cc

@@ -108,17 +108,36 @@ void SourceBufferRangeByDts::AppendBuffersToEnd(
   }
 }
 
+bool SourceBufferRangeByDts::AllowableAppendAfterEstimatedDuration(
+    const BufferQueue& buffers,
+    DecodeTimestamp new_buffers_group_start_timestamp) const {
+  if (buffers_.empty() || !buffers_.back()->is_duration_estimated() ||
+      buffers.empty() || !buffers.front()->is_key_frame()) {
+    return false;
+  }
+
+  if (new_buffers_group_start_timestamp == kNoDecodeTimestamp()) {
+    return GetBufferedEndTimestamp() == buffers.front()->GetDecodeTimestamp();
+  }
+
+  return GetBufferedEndTimestamp() == new_buffers_group_start_timestamp;
+}
+
 bool SourceBufferRangeByDts::CanAppendBuffersToEnd(
     const BufferQueue& buffers,
     DecodeTimestamp new_buffers_group_start_timestamp) const {
   DCHECK(!buffers_.empty());
   if (new_buffers_group_start_timestamp == kNoDecodeTimestamp()) {
-    return IsNextInDecodeSequence(buffers.front()->GetDecodeTimestamp());
+    return IsNextInDecodeSequence(buffers.front()->GetDecodeTimestamp()) ||
+           AllowableAppendAfterEstimatedDuration(
+               buffers, new_buffers_group_start_timestamp);
   }
   DCHECK(new_buffers_group_start_timestamp >= GetEndTimestamp());
   DCHECK(buffers.front()->GetDecodeTimestamp() >=
          new_buffers_group_start_timestamp);
-  return IsNextInDecodeSequence(new_buffers_group_start_timestamp);
+  return IsNextInDecodeSequence(new_buffers_group_start_timestamp) ||
+         AllowableAppendAfterEstimatedDuration(
+             buffers, new_buffers_group_start_timestamp);
 }
 
 void SourceBufferRangeByDts::Seek(DecodeTimestamp timestamp) {

media/filters/source_buffer_range_by_dts.h

@@ -51,15 +51,26 @@ class MEDIA_EXPORT SourceBufferRangeByDts : public SourceBufferRange {
   // it encounters new keyframes.
   // If |new_buffers_group_start_timestamp| is kNoDecodeTimestamp(), then the
   // first buffer in |buffers| must come directly after the last buffer in this
-  // range (within the fudge room).
+  // range (within the fudge room), or be a keyframe that starts precisely where
+  // the last buffer in this range ends and that last buffer has estimated
+  // duration.
   // If |new_buffers_group_start_timestamp| is set otherwise, then that time
   // must come directly after the last buffer in this range (within the fudge
-  // room). The latter scenario is required when a muxed coded frame group has
-  // such a large jagged start across tracks that its first buffer is not within
-  // the fudge room, yet its group start was.
+  // room) or precisely where the last buffer in this range ends and that last
+  // buffer has estimated duration.
+  // The latter scenario is required when a muxed coded frame group has such a
+  // large jagged start across tracks that its first buffer is not within the
+  // fudge room, yet its group start was.
+  // The conditions around estimated duration are handled by
+  // AllowableAppendAfterEstimatedDuration, and are intended to solve the edge
+  // case in the SourceBufferStreamTest
+  // MergeAllowedIfRangeEndTimeWithEstimatedDurationMatchesNextRangeStart.
   // During append, |highest_frame_| is updated, if necessary.
   void AppendBuffersToEnd(const BufferQueue& buffers,
                           DecodeTimestamp new_buffers_group_start_timestamp);
+  bool AllowableAppendAfterEstimatedDuration(
+      const BufferQueue& buffers,
+      DecodeTimestamp new_buffers_group_start_timestamp) const;
   bool CanAppendBuffersToEnd(
       const BufferQueue& buffers,
       DecodeTimestamp new_buffers_group_start_timestamp) const;

media/filters/source_buffer_range_by_pts.cc

@@ -62,6 +62,21 @@ bool SourceBufferRangeByPts::CanAppendRangeToEnd(
                                NextRangeStartTimeForAppendRangeToEnd(range));
 }
 
+bool SourceBufferRangeByPts::AllowableAppendAfterEstimatedDuration(
+    const BufferQueue& buffers,
+    base::TimeDelta new_buffers_group_start_pts) const {
+  if (buffers_.empty() || !buffers_.back()->is_duration_estimated() ||
+      buffers.empty() || !buffers.front()->is_key_frame()) {
+    return false;
+  }
+
+  if (new_buffers_group_start_pts == kNoTimestamp) {
+    return GetBufferedEndTimestamp() == buffers.front()->timestamp();
+  }
+
+  return GetBufferedEndTimestamp() == new_buffers_group_start_pts;
+}
+
 bool SourceBufferRangeByPts::CanAppendBuffersToEnd(
     const BufferQueue& buffers,
     base::TimeDelta new_buffers_group_start_pts) const {
@@ -71,13 +86,17 @@ bool SourceBufferRangeByPts::CanAppendBuffersToEnd(
   DCHECK(!buffers_.empty());
   if (new_buffers_group_start_pts == kNoTimestamp) {
     return buffers.front()->is_key_frame()
-               ? IsNextInPresentationSequence(buffers.front()->timestamp())
+               ? (IsNextInPresentationSequence(buffers.front()->timestamp()) ||
+                  AllowableAppendAfterEstimatedDuration(
+                      buffers, new_buffers_group_start_pts))
                : IsNextInDecodeSequence(buffers.front()->GetDecodeTimestamp());
   }
   CHECK(buffers.front()->is_key_frame());
   DCHECK(new_buffers_group_start_pts >= GetEndTimestamp());
   DCHECK(buffers.front()->timestamp() >= new_buffers_group_start_pts);
-  return IsNextInPresentationSequence(new_buffers_group_start_pts);
+  return IsNextInPresentationSequence(new_buffers_group_start_pts) ||
+         AllowableAppendAfterEstimatedDuration(buffers,
+                                               new_buffers_group_start_pts);
 }
 
 void SourceBufferRangeByPts::AppendBuffersToEnd(

media/filters/source_buffer_range_by_pts.h

@@ -48,16 +48,26 @@ class MEDIA_EXPORT SourceBufferRangeByPts : public SourceBufferRange {
   // range (within the fudge room) - specifically, if the first buffer in
   // |buffers| is not a keyframe, then it must be next in DTS order w.r.t. last
   // buffer in |buffers|. Otherwise, it's a keyframe that must be next in PTS
-  // order w.r.t. |highest_frame_|.
+  // order w.r.t. |highest_frame_| or be immediately adjacent to the last buffer
+  // in this range if that buffer has estimated duration (only allowed in WebM
+  // streams).
   // If |new_buffers_group_start_pts| is set otherwise, then that time must come
-  // directly after |highest_frame_| (within the fudge room), and the first
-  // buffer in |buffers| must be a keyframe.
+  // directly after |highest_frame_| (within the fudge room), or directly after
+  // the last buffered frame if it has estimated duration (only allowed in WebM
+  // streams), and the first buffer in |buffers| must be a keyframe.
   // The latter scenario is required when a muxed coded frame group has such a
   // large jagged start across tracks that its first buffer is not within the
   // fudge room, yet its group start was.
+  // The conditions around estimated duration are handled by
+  // AllowableAppendAfterEstimatedDuration, and are intended to solve the edge
+  // case in the SourceBufferStreamTest
+  // MergeAllowedIfRangeEndTimeWithEstimatedDurationMatchesNextRangeStart.
   // During append, |highest_frame_| is updated, if necessary.
   void AppendBuffersToEnd(const BufferQueue& buffers,
                           base::TimeDelta new_buffers_group_start_timestamp);
+  bool AllowableAppendAfterEstimatedDuration(
+      const BufferQueue& buffers,
+      base::TimeDelta new_buffers_group_start_pts) const;
   bool CanAppendBuffersToEnd(const BufferQueue& buffers,
                              base::TimeDelta new_buffers_group_start_pts) const;
 

media/filters/source_buffer_stream.cc

@@ -145,7 +145,9 @@ std::string BufferQueueBuffersToLogString(
   result << "Buffers:\n";
   for (const auto& buf : buffers) {
     result << "\tdts=" << buf->GetDecodeTimestamp().InMicroseconds() << " "
-           << buf->AsHumanReadableString() << "\n";
+           << buf->AsHumanReadableString()
+           << ", is_duration_estimated=" << buf->is_duration_estimated()
+           << "\n";
   }
 
   return result.str();

media/filters/source_buffer_stream_unittest.cc

@@ -5835,6 +5835,51 @@ TEST_P(SourceBufferStreamTest, SapType2WithNonkeyframePtsInEarlierRange) {
   CheckNoNextBuffer();
 }
 
+TEST_P(SourceBufferStreamTest,
+       MergeAllowedIfRangeEndTimeWithEstimatedDurationMatchesNextRangeStart) {
+  // Tests the edge case where fudge room is not increased when an estimated
+  // duration is increased due to overlap appends, causing two ranges to not be
+  // within fudge room of each other (nor merged), yet abutting each other.
+  // Append a GOP that has fudge room as its interval (e.g. 2 frames of same
+  // duration >= minimum 1ms).
+  NewCodedFrameGroupAppend("0D10K 10D10");
+  CheckExpectedRangesByTimestamp("{ [0,20) }");
+
+  // Trigger a DTS discontinuity so later 21ms append also is discontinuous and
+  // retains 10ms*2 fudge room.
+  NewCodedFrameGroupAppend("100D10K");
+  CheckExpectedRangesByTimestamp("{ [0,20) [100,110) }");
+
+  // Append another keyframe that starts within fudge room distance of the
+  // non-keyframe in the GOP appended, above.
+  NewCodedFrameGroupAppend("21D10K");
+  CheckExpectedRangesByTimestamp("{ [0,31) [100,110) }");
+
+  // Overlap-append the original GOP with a single estimated-duration keyframe.
+  // Though its timestamp is not within fudge room of the next keyframe, that
+  // next keyframe at time 21ms was in the overlapped range and is retained in
+  // the result of the overlap append's range.
+  NewCodedFrameGroupAppend("0D10EK");
+  CheckExpectedRangesByTimestamp("{ [0,31) [100,110) }");
+
+  // That new keyframe at time 0 now has derived estimated duration 21ms.  That
+  // increased estimated duration did *not* increase the fudge room (which is
+  // still 2 * 10ms = 20ms.) So the next line, which splices in a new frame at
+  // time 21 causes the estimated keyframe at time 0 to not have a timestamp
+  // within fudge room of the new range that starts right at 21ms, the same time
+  // that ends the first buffered range, requiring CanAppendBuffersToEnd to
+  // handle this scenario specifically.
+  NewCodedFrameGroupAppend("21D10K");
+  CheckExpectedRangesByTimestamp("{ [0,31) [100,110) }");
+
+  SeekToTimestampMs(0);
+  CheckExpectedBuffers("0D21EK 21D10K");
+  CheckNoNextBuffer();
+  SeekToTimestampMs(100);
+  CheckExpectedBuffers("100D10K");
+  CheckNoNextBuffer();
+}
+
 INSTANTIATE_TEST_CASE_P(LegacyByDts,
                         SourceBufferStreamTest,
                         Values(BufferingApi::kLegacyByDts));