MSE: Signal SBS of new CFG more granularly when buffering ByPts

This change includes multiple fixes for various cases where
SourceBufferStream requires notification of a new coded frame group when
buffering by PTS (currently available via kMseBufferByPts feature) even
if there is no MSE coded frame processing algorithm DTS discontinuity
detected. This additional granularity allows for SourceBufferStream to
understand that the next buffers appended to it may overlap recently
appended buffers, and also allows for handling cases where the next
keyframe (continuous in DTS) jumps significantly forward in PTS but
still needs to remain continuous with the current append sequence.

On every keyframe, when buffering by PTS, FrameProcessor now considers
doing the extra signalling if DTS is continuous and buffering ByPts,
and:
a) keyframe PTS jumps significantly into the future relative to the
   highest PTS emitted already in the current coded frame group (in
   which case the highest PTS emitted already in the current CFG is used
   as the signalled PTS value), or
b) keyframe PTS is before the highest PTS emitted already in the current
   coded frame group (in which case the keyframe PTS is used as the
   signalled PTS value and tracking of the highest PTS emitted is reset
   to enable correct detection and signalling of both (a) and (b) cases
   for future keyframes in the continuous DTS append sequence.)

This change also includes a fix to SourceBufferStream when buffering
ByPts to appropriately split a range that is being overlap-appended.

A benign DCHECK is also removed from SBS::UpdateLastAppendStateForRemove
which could fail for a SAP Type 2 GOP at the beginning of a range (the
last appended buffer in that range might indeed be a non-keyframe with a
PTS prior to the range start time, which could be as late as the PTS of
the keyframe of that non-keyframe's GOP.)

New and updated unit tests are included. I locally confirmed this fixes
bugs 773115 and 788344, and appears to fix nest.com and twitch.com
renderer crashes with dcheck_always_on=true.

Pre-existing bug 791095 is demonstrated, but not yet fixed, by new
unit tests:
  BufferingByPts_ContinuousDts_SapType2_and_PtsJumpForward
  BufferingByPts_ContinuousDts_NewSap2GopEndOverlapsLastGop_1

Pre-existing bug 763620 is demonstrated, but not yet fixed, by new
unit tests:
  BufferingByPts_ContinuousDts_NewGopEndOverlapsLastGop_2
  BufferingByPts_ContinuousDts_NewSap2GopEndOverlapsLastGop_2
  BufferingByPts_ContinuousDts_GopKeyframePtsOrder_2_1_3

BUG=773115,788344,791095,763620

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: I6f95cf85e1d1fa5b5f74ed1d99a3853ec6ccf686
Reviewed-on: https://chromium-review.googlesource.com/777778Reviewed-by: Sergey Volk <servolk@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521961}

media/base/test_helpers.h

@@ -312,6 +312,18 @@ MATCHER_P(SkippingSpliceAlreadySpliced, time_microseconds, "") {
                "us are in a previously buffered splice.");
 }
 
+MATCHER_P2(SkippingSpliceTooLittleOverlap,
+           pts_microseconds,
+           overlap_microseconds,
+           "") {
+  return CONTAINS_STRING(
+      arg, "Skipping audio splice trimming at PTS=" +
+               base::IntToString(pts_microseconds) + "us. Found only " +
+               base::IntToString(overlap_microseconds) +
+               "us of overlap, need at least 1000us. Multiple occurrences may "
+               "result in loss of A/V sync.");
+}
+
 MATCHER_P(WebMSimpleBlockDurationEstimated, estimated_duration_ms, "") {
   return CONTAINS_STRING(arg, "Estimating WebM block duration to be " +
                                   base::IntToString(estimated_duration_ms) +

media/filters/chunk_demuxer_unittest.cc

@@ -3465,7 +3465,29 @@ TEST_P(ChunkDemuxerTest, EmitBuffersDuringAbort) {
 
   scoped_refptr<DecoderBuffer> buffer = ReadTestDataFile("bear-1280x720.ts");
   EXPECT_CALL(*this, InitSegmentReceivedMock(_));
-  ASSERT_TRUE(AppendData(kSourceId, buffer->data(), buffer->data_size()));
+
+  // This mp2ts file contains buffers which can trigger media logs related to
+  // splicing. Related logic occurs more deterministically (and frequently) when
+  // buffering ByPts; we append in small chunks to force the same logic when
+  // buffering by either Pts or Dts here.
+  EXPECT_MEDIA_LOG(TrimmedSpliceOverlap(1655422, 1655419, 23217));
+  EXPECT_MEDIA_LOG(SkippingSpliceTooLittleOverlap(1957277, 4));
+  EXPECT_MEDIA_LOG(SkippingSpliceTooLittleOverlap(2514555, 6));
+  EXPECT_MEDIA_LOG(SkippingSpliceTooLittleOverlap(3071833, 6));
+  EXPECT_MEDIA_LOG(SkippingSpliceTooLittleOverlap(3652333, 6));
+
+  // Append the media in small chunks. 1 byte chunks would cause test timeout;
+  // 1k chunks appear to be small enough to let ByDts meet the logging
+  // expectations of the more deterministic ByPts logic, simplifying this test.
+  size_t appended_bytes = 0;
+  const size_t chunk_size = 1024;
+  while (appended_bytes < buffer->data_size()) {
+    size_t cur_chunk_size =
+        std::min(chunk_size, buffer->data_size() - appended_bytes);
+    ASSERT_TRUE(
+        AppendData(kSourceId, buffer->data() + appended_bytes, cur_chunk_size));
+    appended_bytes += cur_chunk_size;
+  }
 
   // Confirm we're in the middle of parsing a media segment.
   ASSERT_TRUE(demuxer_->IsParsingMediaSegment(kSourceId));
@@ -3513,7 +3535,29 @@ TEST_P(ChunkDemuxerTest, SeekCompleteDuringAbort) {
 
   scoped_refptr<DecoderBuffer> buffer = ReadTestDataFile("bear-1280x720.ts");
   EXPECT_CALL(*this, InitSegmentReceivedMock(_));
-  ASSERT_TRUE(AppendData(kSourceId, buffer->data(), buffer->data_size()));
+
+  // This mp2ts file contains buffers which can trigger media logs related to
+  // splicing. Related logic occurs more deterministically (and frequently) when
+  // buffering ByPts; we append in small chunks to force the same logic when
+  // buffering by either Pts or Dts here.
+  EXPECT_MEDIA_LOG(TrimmedSpliceOverlap(1655422, 1655419, 23217));
+  EXPECT_MEDIA_LOG(SkippingSpliceTooLittleOverlap(1957277, 4));
+  EXPECT_MEDIA_LOG(SkippingSpliceTooLittleOverlap(2514555, 6));
+  EXPECT_MEDIA_LOG(SkippingSpliceTooLittleOverlap(3071833, 6));
+  EXPECT_MEDIA_LOG(SkippingSpliceTooLittleOverlap(3652333, 6));
+
+  // Append the media in small chunks. 1 byte chunks would cause test timeout;
+  // 1k chunks appear to be small enough to let ByDts meet the logging
+  // expectations of the more deterministic ByPts logic, simplifying this test.
+  size_t appended_bytes = 0;
+  const size_t chunk_size = 1024;
+  while (appended_bytes < buffer->data_size()) {
+    size_t cur_chunk_size =
+        std::min(chunk_size, buffer->data_size() - appended_bytes);
+    ASSERT_TRUE(
+        AppendData(kSourceId, buffer->data() + appended_bytes, cur_chunk_size));
+    appended_bytes += cur_chunk_size;
+  }
 
   // Confirm we're in the middle of parsing a media segment.
   ASSERT_TRUE(demuxer_->IsParsingMediaSegment(kSourceId));

media/filters/frame_processor.cc

@@ -82,6 +82,9 @@ class MseTrackBuffer {
   // |needs_random_access_point_| to true.
   void Reset();
 
+  // Unsets |highest_presentation_timestamp_|.
+  void ResetHighestPresentationTimestamp();
+
   // If |highest_presentation_timestamp_| is unset or |timestamp| is greater
   // than |highest_presentation_timestamp_|, sets
   // |highest_presentation_timestamp_| to |timestamp|. Note that bidirectional
@@ -207,6 +210,10 @@ void MseTrackBuffer::Reset() {
   last_keyframe_presentation_timestamp_ = kNoTimestamp;
 }
 
+void MseTrackBuffer::ResetHighestPresentationTimestamp() {
+  highest_presentation_timestamp_ = kNoTimestamp;
+}
+
 void MseTrackBuffer::SetHighestPresentationTimestampIfIncreased(
     base::TimeDelta timestamp) {
   if (highest_presentation_timestamp_ == kNoTimestamp ||
@@ -901,16 +908,36 @@ bool FrameProcessor::ProcessFrame(
         (track_buffer->pending_group_start_pts() != kNoTimestamp &&
          track_buffer->pending_group_start_pts() > presentation_timestamp);
 
-    // When buffering by PTS intervals and a keyframe is discovered to have
-    // a decreasing PTS versus the previous keyframe for that track in the
-    // current coded frame group, signal a new coded frame group for that track
-    // buffer so that it can correctly process overlap-removals of the new GOP.
-    signal_new_cfg |=
-        range_api_ == ChunkDemuxerStream::RangeApi::kNewByPts &&
-        frame->is_key_frame() &&
-        track_buffer->last_keyframe_presentation_timestamp() != kNoTimestamp &&
-        track_buffer->last_keyframe_presentation_timestamp() >
-            presentation_timestamp;
+    if (range_api_ == ChunkDemuxerStream::RangeApi::kNewByPts &&
+        frame->is_key_frame()) {
+      // When buffering by PTS intervals and a keyframe is discovered to have a
+      // decreasing PTS versus the previous highest presentation timestamp for
+      // that track in the current coded frame group, signal a new coded frame
+      // group for that track buffer so that it can correctly process
+      // overlap-removals for the new GOP.
+      if (track_buffer->highest_presentation_timestamp() != kNoTimestamp &&
+          track_buffer->highest_presentation_timestamp() >
+              presentation_timestamp) {
+        signal_new_cfg = true;
+        // In case there is currently a decreasing keyframe PTS relative to the
+        // track buffer's highest PTS, that is later followed by a jump forward
+        // requiring overlap removal of media prior to the track buffer's
+        // highest PTS, reset that tracking now to ensure correctness of
+        // signalling the need for such overlap removal later.
+        track_buffer->ResetHighestPresentationTimestamp();
+      }
+
+      // When buffering by PTS intervals and an otherwise continuous coded frame
+      // group (by DTS, and with non-decreasing keyframe PTS) contains a
+      // keyframe with PTS in the future, signal a new coded frame group with
+      // start time set to the previous highest frame end time in the coded
+      // frame group for this track. This lets the stream coalesce a potential
+      // gap, and also pass internal buffer adjacency checks.
+      signal_new_cfg |=
+          track_buffer->highest_presentation_timestamp() != kNoTimestamp &&
+          track_buffer->highest_presentation_timestamp() <
+              presentation_timestamp;
+    }
 
     if (signal_new_cfg) {
       DCHECK(frame->is_key_frame());
@@ -924,10 +951,15 @@ bool FrameProcessor::ProcessFrame(
         NotifyStartOfCodedFrameGroup(decode_timestamp, presentation_timestamp);
         pending_notify_all_group_start_ = false;
       } else {
-        // Don't signal later times than previously signalled for this group.
         DecodeTimestamp updated_dts = std::min(
             track_buffer->last_processed_decode_timestamp(), decode_timestamp);
         base::TimeDelta updated_pts = track_buffer->pending_group_start_pts();
+        if (updated_pts == kNoTimestamp &&
+            track_buffer->highest_presentation_timestamp() != kNoTimestamp &&
+            track_buffer->highest_presentation_timestamp() <
+                presentation_timestamp) {
+          updated_pts = track_buffer->highest_presentation_timestamp();
+        }
         if (updated_pts == kNoTimestamp || updated_pts > presentation_timestamp)
           updated_pts = presentation_timestamp;
         track_buffer->NotifyStartOfCodedFrameGroup(updated_dts, updated_pts);

media/filters/source_buffer_stream.cc

@@ -545,9 +545,6 @@ void SourceBufferStream<RangeClass>::UpdateLastAppendStateForRemove(
 
   if (range_for_next_append_ != ranges_.end()) {
     if (last_appended_buffer_timestamp_ != kNoDecodeTimestamp()) {
-      DCHECK(RangeBelongsToRange(range_for_next_append_->get(),
-                                 last_appended_buffer_timestamp_));
-
       // Note start and end of last appended GOP.
       DecodeTimestamp gop_end = highest_timestamp_in_append_sequence_;
       DecodeTimestamp gop_start =
@@ -1317,9 +1314,40 @@ void SourceBufferStream<RangeClass>::PrepareRangesForNextAppend(
         std::min(coded_frame_group_start_time_, buffers_start_timestamp);
   }
 
-  // Return early if no further overlap removal is needed.
-  if (buffers_start_timestamp >= buffers_end_timestamp)
+  // Return early if no further overlap removal is needed. When buffering by PTS
+  // intervals, first check if |buffers_start_timestamp| is in the middle of the
+  // range; we could be overlap-appending the middle of a previous coded frame
+  // sequence's range with non-keyframes prior to
+  // |highest_timestamp_in_append_sequence_|, so we need to split that range
+  // appropriately here and then return early. If we don't return early here,
+  // overlap removal (including any necessary range splitting) will occur.
+  if (buffers_start_timestamp >= buffers_end_timestamp) {
+    if (!BufferingByPts())
+      return;
+
+    DCHECK(highest_timestamp_in_append_sequence_ != kNoDecodeTimestamp());
+    DCHECK(range_for_next_append_ != ranges_.end());
+    DCHECK(RangeBelongsToRange(range_for_next_append_->get(),
+                               buffers_start_timestamp));
+
+    // Split the range at |buffers_start_timestamp|, if necessary, then return
+    // early.
+    std::unique_ptr<RangeClass> new_range =
+        RangeSplitRange(range_for_next_append_->get(), buffers_start_timestamp);
+    if (!new_range)
+      return;
+
+    range_for_next_append_ =
+        ranges_.insert(++range_for_next_append_, std::move(new_range));
+
+    // Update the selected range if the next buffer position was transferred
+    // to the newly inserted range.
+    if ((*range_for_next_append_)->HasNextBufferPosition())
+      SetSelectedRange(range_for_next_append_->get());
+
+    --range_for_next_append_;
     return;
+  }
 
   // Exclude the start from removal to avoid deleting the highest appended
   // buffer in cases where the first buffer in |new_buffers| has same timestamp
@@ -1401,10 +1429,15 @@ template <typename RangeClass>
 bool SourceBufferStream<RangeClass>::
     IsNextGopAdjacentToEndOfCurrentAppendSequence(
         DecodeTimestamp next_gop_timestamp) const {
+  DecodeTimestamp upper_bound = highest_timestamp_in_append_sequence_ +
+                                ComputeFudgeRoom(GetMaxInterbufferDistance());
+  DVLOG(4) << __func__ << " " << GetStreamTypeName()
+           << " next_gop_timestamp=" << next_gop_timestamp.InMicroseconds()
+           << "us, highest_timestamp_in_append_sequence_="
+           << highest_timestamp_in_append_sequence_.InMicroseconds()
+           << "us, upper_bound=" << upper_bound.InMicroseconds() << "us";
   return highest_timestamp_in_append_sequence_ < next_gop_timestamp &&
-         next_gop_timestamp <=
-             highest_timestamp_in_append_sequence_ +
-                 ComputeFudgeRoom(GetMaxInterbufferDistance());
+         next_gop_timestamp <= upper_bound;
 }
 
 template <typename RangeClass>

media/filters/source_buffer_stream_unittest.cc

@@ -4643,9 +4643,7 @@ TEST_P(SourceBufferStreamTest, Audio_SpliceTrimming_ExistingTrimming) {
 }
 
 TEST_P(SourceBufferStreamTest, Audio_SpliceFrame_NoMillisecondSplices) {
-  EXPECT_MEDIA_LOG(
-      HasSubstr("Skipping audio splice trimming at PTS=1250us. Found only 250us"
-                " of overlap, need at least 1000us."));
+  EXPECT_MEDIA_LOG(SkippingSpliceTooLittleOverlap(1250, 250));
 
   video_config_ = TestVideoConfig::Invalid();
   audio_config_.Initialize(kCodecVorbis, kSampleFormatPlanarF32,