MSE: Replace crossfade splicing overlap trimming.

media/filters/source_buffer_stream_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/filters/source_buffer_stream.h"
 
 #include <stddef.h>
 #include <stdint.h>
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "media/base/data_buffer.h"
+#include "media/base/decoder_buffer.h"
 #include "media/base/media_log.h"
 #include "media/base/media_util.h"
 #include "media/base/mock_media_log.h"
 #include "media/base/test_helpers.h"
 #include "media/base/text_track_config.h"
 #include "media/base/timestamp_constants.h"
 #include "media/filters/webvtt_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using ::testing::HasSubstr;
@@ skipping 21 matching lines @@
 MATCHER_P(ContainsTrackBufferExhaustionSkipLog, skip_milliseconds, "") {
   return CONTAINS_STRING(arg,
                          "Media append that overlapped current playback "
                          "position caused time gap in playing VIDEO stream "
                          "because the next keyframe is " +
                              base::IntToString(skip_milliseconds) +
                              "ms beyond last overlapped frame. Media may "
                              "appear temporarily frozen.");
 }
 
-MATCHER_P2(ContainsGeneratedSpliceLog,
-           duration_microseconds,
-           time_microseconds,
+MATCHER_P3(TrimmedSpliceOverlap,
+           splice_time_us,
+           overlapped_start_us,
+           trim_duration_us,
            "") {
-  return CONTAINS_STRING(arg, "Generated splice of overlap duration " +
-                                  base::IntToString(duration_microseconds) +
-                                  "us into new buffer at " +
-                                  base::IntToString(time_microseconds) + "us.");
+  return CONTAINS_STRING(
+      arg, "Audio buffer splice at PTS=" + base::IntToString(splice_time_us) +
+               "us. Trimmed tail of overlapped buffer (PTS=" +
+               base::IntToString(overlapped_start_us) + "us) by " +
+               base::IntToString(trim_duration_us));
 }
 
 class SourceBufferStreamTest : public testing::Test {
  protected:
   SourceBufferStreamTest() : media_log_(new StrictMock<MockMediaLog>()) {
     video_config_ = TestVideoConfig::Normal();
     SetStreamInfo(kDefaultFramesPerSecond, kDefaultKeyframesPerSecond);
-    stream_.reset(new SourceBufferStream(video_config_, media_log_, true));
+    stream_.reset(new SourceBufferStream(video_config_, media_log_));
   }
 
   void SetMemoryLimit(size_t buffers_of_data) {
     stream_->set_memory_limit(buffers_of_data * kDataSize);
   }
 
   void SetStreamInfo(int frames_per_second, int keyframes_per_second) {
     frames_per_second_ = frames_per_second;
     keyframes_per_second_ = keyframes_per_second;
     frame_duration_ = ConvertToFrameDuration(frames_per_second);
   }
 
   void SetTextStream() {
     video_config_ = TestVideoConfig::Invalid();
     TextTrackConfig config(kTextSubtitles, "", "", "");
-    stream_.reset(new SourceBufferStream(config, media_log_, true));
+    stream_.reset(new SourceBufferStream(config, media_log_));
     SetStreamInfo(2, 2);
   }
 
   void SetAudioStream() {
     video_config_ = TestVideoConfig::Invalid();
     audio_config_.Initialize(kCodecVorbis, kSampleFormatPlanarF32,
                              CHANNEL_LAYOUT_STEREO, 1000, EmptyExtraData(),
                              Unencrypted(), base::TimeDelta(), 0);
-    stream_.reset(new SourceBufferStream(audio_config_, media_log_, true));
+    stream_.reset(new SourceBufferStream(audio_config_, media_log_));
 
     // Equivalent to 2ms per frame.
     SetStreamInfo(500, 500);
   }
 
   void NewCodedFrameGroupAppend(int starting_position, int number_of_buffers) {
     AppendBuffers(starting_position, number_of_buffers, true,
                   base::TimeDelta(), true, &kDataA, kDataSize);
   }
 
@@ skipping 187 matching lines @@
     }
 
     EXPECT_EQ(ending_position + 1, current_position);
   }
 
   void CheckExpectedBuffers(const std::string& expected) {
     std::vector<std::string> timestamps = base::SplitString(
         expected, " ", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
     std::stringstream ss;
     const SourceBufferStream::Type type = stream_->GetType();
-    base::TimeDelta active_splice_timestamp = kNoTimestamp;
     for (size_t i = 0; i < timestamps.size(); i++) {
       scoped_refptr<StreamParserBuffer> buffer;
       SourceBufferStream::Status status = stream_->GetNextBuffer(&buffer);
 
       if (i > 0)
         ss << " ";
 
       if (status == SourceBufferStream::kConfigChange) {
         switch (type) {
           case SourceBufferStream::kVideo:
@@ skipping 53 matching lines @@
         ASSERT_EQ(buffer->GetDecodeTimestamp(),
                   preroll_buffer->GetDecodeTimestamp());
         ASSERT_EQ(kInfiniteDuration, preroll_buffer->discard_padding().first);
         ASSERT_EQ(base::TimeDelta(), preroll_buffer->discard_padding().second);
         ASSERT_TRUE(buffer->is_key_frame());
 
         ss << "P";
       } else if (buffer->is_key_frame()) {
         ss << "K";
       }
-
-      // Until the last splice frame is seen, indicated by a matching timestamp,
-      // all buffers must have the same splice_timestamp().
-      if (buffer->timestamp() == active_splice_timestamp) {
-        ASSERT_EQ(buffer->splice_timestamp(), kNoTimestamp);
-      } else {
-        ASSERT_TRUE(active_splice_timestamp == kNoTimestamp ||
-                    active_splice_timestamp == buffer->splice_timestamp());
-      }
-
-      active_splice_timestamp = buffer->splice_timestamp();
     }
     EXPECT_EQ(expected, ss.str());
   }
 
   void CheckNoNextBuffer() {
     scoped_refptr<StreamParserBuffer> buffer;
     EXPECT_EQ(SourceBufferStream::kNeedBuffer, stream_->GetNextBuffer(&buffer));
   }
 
   void CheckEOSReached() {
@@ skipping 116 matching lines @@
   // Indicates that the buffer should be marked as containing an *estimated*
   // duration. E.g., "0D20E 20 25E 30"
   //
   // P:
   // Indicates the buffer with will also have a preroll buffer
   // associated with it. The preroll buffer will just be dummy data.
   // E.g. "0P 5 10"
   //
   // K:
   // Indicates the buffer is a keyframe. E.g., "0K 1|2K 2|4D2K 6 8".
-  //
-  // S(a# ... y# z#)
-  // Indicates a splice frame buffer should be created with timestamp z#.  The
-  // preceding timestamps a# ... y# will be treated as the fade out preroll for
-  // the splice frame.  If a timestamp within the preroll ends with C the config

[wolenetz, 2016/09/26 23:52:37]

I think we still need some configchange testing (see my SBS.cc comment)

[chcunningham, 2016/10/27 23:36:06]

See new test, SourceBufferStreamTest.Audio_ConfigChangeWithPreroll

[wolenetz, 2016/10/28 23:08:19]

Acknowledged.

-  // id to use for that and subsequent preroll appends is incremented by one.
-  // The config id for non-splice frame appends will not be affected.
   BufferQueue StringToBufferQueue(const std::string& buffers_to_append) {
     std::vector<std::string> timestamps = base::SplitString(
         buffers_to_append, " ", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
 
     CHECK_GT(timestamps.size(), 0u);
 
-    bool splice_frame = false;
-    size_t splice_config_id = stream_->append_config_index_;
-    BufferQueue pre_splice_buffers;
     BufferQueue buffers;
     for (size_t i = 0; i < timestamps.size(); i++) {
       bool is_keyframe = false;
       bool has_preroll = false;
-      bool last_splice_frame = false;
       bool is_duration_estimated = false;
 
-      // Handle splice frame starts.
-      if (base::StartsWith(timestamps[i], "S(", base::CompareCase::SENSITIVE)) {
-        CHECK(!splice_frame);
-        splice_frame = true;
-        // Remove the "S(" off of the token.
-        timestamps[i] = timestamps[i].substr(2, timestamps[i].length());
-      }
-      if (splice_frame &&
-          base::EndsWith(timestamps[i], ")", base::CompareCase::SENSITIVE)) {
-        splice_frame = false;
-        last_splice_frame = true;
-        // Remove the ")" off of the token.
-        timestamps[i] = timestamps[i].substr(0, timestamps[i].length() - 1);
-      }
-      // Handle config changes within the splice frame.
-      if (splice_frame &&
-          base::EndsWith(timestamps[i], "C", base::CompareCase::SENSITIVE)) {
-        splice_config_id++;
-        CHECK(splice_config_id < stream_->audio_configs_.size() ||
-              splice_config_id < stream_->video_configs_.size());
-        // Remove the "C" off of the token.
-        timestamps[i] = timestamps[i].substr(0, timestamps[i].length() - 1);
-      }
       if (base::EndsWith(timestamps[i], "K", base::CompareCase::SENSITIVE)) {
         is_keyframe = true;
         // Remove the "K" off of the token.
         timestamps[i] = timestamps[i].substr(0, timestamps[i].length() - 1);
       }
       // Handle preroll buffers.
       if (base::EndsWith(timestamps[i], "P", base::CompareCase::SENSITIVE)) {
         is_keyframe = true;
         has_preroll = true;
         // Remove the "P" off of the token.
         timestamps[i] = timestamps[i].substr(0, timestamps[i].length() - 1);
       }
 
       if (base::EndsWith(timestamps[i], "E", base::CompareCase::SENSITIVE)) {
         is_duration_estimated = true;
         // Remove the "E" off of the token.
         timestamps[i] = timestamps[i].substr(0, timestamps[i].length() - 1);
       }
 
-      int duration_in_ms = 0;
+      int duration_in_ms = -1;
       size_t duration_pos = timestamps[i].find('D');
       if (duration_pos != std::string::npos) {
         CHECK(base::StringToInt(timestamps[i].substr(duration_pos + 1),
                                 &duration_in_ms));
         timestamps[i] = timestamps[i].substr(0, duration_pos);
       }
 
       std::vector<std::string> buffer_timestamps = base::SplitString(
           timestamps[i], "|", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
 
@@ skipping 12 matching lines @@
           StreamParserBuffer::CopyFrom(&kDataA, kDataSize, is_keyframe,
                                        DemuxerStream::AUDIO, 0);
       buffer->set_timestamp(base::TimeDelta::FromMilliseconds(pts_in_ms));
       buffer->set_is_duration_estimated(is_duration_estimated);
 
       if (dts_in_ms != pts_in_ms) {
         buffer->SetDecodeTimestamp(
             DecodeTimestamp::FromMilliseconds(dts_in_ms));
       }
 
-      if (duration_in_ms)
+      if (duration_in_ms >= 0)
         buffer->set_duration(base::TimeDelta::FromMilliseconds(duration_in_ms));
 
       // Simulate preroll buffers by just generating another buffer and sticking
       // it as the preroll.
       if (has_preroll) {
         scoped_refptr<StreamParserBuffer> preroll_buffer =
             StreamParserBuffer::CopyFrom(
                 &kDataA, kDataSize, is_keyframe, DemuxerStream::AUDIO, 0);
         preroll_buffer->set_duration(frame_duration_);
         buffer->SetPrerollBuffer(preroll_buffer);
       }
 
-      if (splice_frame) {
-        // Make sure that splice frames aren't used with content where decode
-        // and presentation timestamps can differ. (i.e., B-frames)
-        CHECK_EQ(buffer->GetDecodeTimestamp().InMicroseconds(),
-                 buffer->timestamp().InMicroseconds());
-        if (!pre_splice_buffers.empty()) {
-          // Enforce strictly monotonically increasing timestamps.
-          CHECK_GT(
-              buffer->timestamp().InMicroseconds(),
-              pre_splice_buffers.back()->timestamp().InMicroseconds());
-          CHECK_GT(
-              buffer->GetDecodeTimestamp().InMicroseconds(),
-              pre_splice_buffers.back()->GetDecodeTimestamp().InMicroseconds());
-        }
-        buffer->SetConfigId(splice_config_id);
-        UpdateLastBufferDuration(buffer->GetDecodeTimestamp(),
-                                 &pre_splice_buffers);
-        pre_splice_buffers.push_back(buffer);
-        continue;
-      }
-
-      if (last_splice_frame) {
-        // Require at least one additional buffer for a splice.
-        CHECK(!pre_splice_buffers.empty());
-        buffer->SetConfigId(splice_config_id);
-        buffer->ConvertToSpliceBuffer(pre_splice_buffers);
-        pre_splice_buffers.clear();
-      }
-
       UpdateLastBufferDuration(buffer->GetDecodeTimestamp(), &buffers);
       buffers.push_back(buffer);
     }
 
     // If the last buffer doesn't have a duration, assume it is the
     // same as the second to last buffer.
-    if (buffers.size() >= 2 &&
-        buffers.back()->duration() <= base::TimeDelta()) {
+    if (buffers.size() >= 2 && buffers.back()->duration() == kNoTimestamp) {
       buffers.back()->set_duration(
           buffers[buffers.size() - 2]->duration());
     }
 
     return buffers;
   }
 
   void AppendBuffers(const std::string& buffers_to_append,
                      bool start_new_coded_frame_group,
                      base::TimeDelta coded_frame_group_start_timestamp,
@@ skipping 2921 matching lines @@
 TEST_F(SourceBufferStreamTest, SameTimestamp_Video_SingleAppend) {
   Seek(0);
   NewCodedFrameGroupAppend("0K 30 30 60 90 120K 150");
   CheckExpectedBuffers("0K 30 30 60 90 120K 150");
 }
 
 // Verify that non-keyframes with the same timestamp can occur
 // in different appends.
 TEST_F(SourceBufferStreamTest, SameTimestamp_Video_TwoAppends) {
   Seek(0);
-  NewCodedFrameGroupAppend("0K 30");
+  NewCodedFrameGroupAppend("0K 30D0");
   AppendBuffers("30 60 90 120K 150");
   CheckExpectedBuffers("0K 30 30 60 90 120K 150");
 }
 
 // Verify that a non-keyframe followed by a keyframe with the same timestamp
 // is allowed, but also results in a MediaLog.
 TEST_F(SourceBufferStreamTest, SameTimestamp_Video_SingleAppend_Warning) {
   EXPECT_MEDIA_LOG(ContainsSameTimestampAt30MillisecondsLog());
 
   Seek(0);
   NewCodedFrameGroupAppend("0K 30 30K 60");
   CheckExpectedBuffers("0K 30 30K 60");
 }
 
 TEST_F(SourceBufferStreamTest, SameTimestamp_Video_TwoAppends_Warning) {
   EXPECT_MEDIA_LOG(ContainsSameTimestampAt30MillisecondsLog());
 
   Seek(0);
-  NewCodedFrameGroupAppend("0K 30");
+  NewCodedFrameGroupAppend("0K 30D0");
   AppendBuffers("30K 60");
   CheckExpectedBuffers("0K 30 30K 60");
 }
 
 // Verify that a keyframe followed by a non-keyframe with the same timestamp
 // is allowed.
 TEST_F(SourceBufferStreamTest, SameTimestamp_VideoKeyFrame_TwoAppends) {
   Seek(0);
-  NewCodedFrameGroupAppend("0K 30K");
+  NewCodedFrameGroupAppend("0K 30D0K");
   AppendBuffers("30 60");
   CheckExpectedBuffers("0K 30K 30 60");
 }
 
 TEST_F(SourceBufferStreamTest, SameTimestamp_VideoKeyFrame_SingleAppend) {
   Seek(0);
   NewCodedFrameGroupAppend("0K 30K 30 60");
   CheckExpectedBuffers("0K 30K 30 60");
 }
 
 TEST_F(SourceBufferStreamTest, SameTimestamp_Video_Overlap_1) {
   Seek(0);
   NewCodedFrameGroupAppend("0K 30 60 60 90 120K 150");
 
   NewCodedFrameGroupAppend("60K 91 121K 151");
   CheckExpectedBuffers("0K 30 60K 91 121K 151");
 }
 
 TEST_F(SourceBufferStreamTest, SameTimestamp_Video_Overlap_2) {
   Seek(0);
   NewCodedFrameGroupAppend("0K 30 60 60 90 120K 150");
   NewCodedFrameGroupAppend("0K 30 61");
   CheckExpectedBuffers("0K 30 61 120K 150");
 }
 
 TEST_F(SourceBufferStreamTest, SameTimestamp_Video_Overlap_3) {
   Seek(0);
   NewCodedFrameGroupAppend("0K 20 40 60 80 100K 101 102 103K");
-  NewCodedFrameGroupAppend("0K 20 40 60 80 90");
+  NewCodedFrameGroupAppend("0K 20 40 60 80 90D0");
   CheckExpectedBuffers("0K 20 40 60 80 90 100K 101 102 103K");
   AppendBuffers("90 110K 150");
   Seek(0);
   CheckExpectedBuffers("0K 20 40 60 80 90 90 110K 150");
   CheckNoNextBuffer();
   CheckExpectedRangesByTimestamp("{ [0,190) }");
 }
 
 // Test all the valid same timestamp cases for audio.
 TEST_F(SourceBufferStreamTest, SameTimestamp_Audio) {
   AudioDecoderConfig config(kCodecMP3, kSampleFormatF32, CHANNEL_LAYOUT_STEREO,
                             44100, EmptyExtraData(), Unencrypted());
-  stream_.reset(new SourceBufferStream(config, media_log_, true));
+  stream_.reset(new SourceBufferStream(config, media_log_));
   Seek(0);
   NewCodedFrameGroupAppend("0K 0K 30K 30 60 60");
   CheckExpectedBuffers("0K 0K 30K 30 60 60");
 }
 
 TEST_F(SourceBufferStreamTest, SameTimestamp_Audio_SingleAppend_Warning) {
   EXPECT_MEDIA_LOG(ContainsSameTimestampAt30MillisecondsLog());
 
   AudioDecoderConfig config(kCodecMP3, kSampleFormatF32, CHANNEL_LAYOUT_STEREO,
                             44100, EmptyExtraData(), Unencrypted());
-  stream_.reset(new SourceBufferStream(config, media_log_, true));
+  stream_.reset(new SourceBufferStream(config, media_log_));
   Seek(0);
 
   // Note, in reality, a non-keyframe audio frame is rare or perhaps not
   // possible.
   NewCodedFrameGroupAppend("0K 30 30K 60");
   CheckExpectedBuffers("0K 30 30K 60");
 }
 
 // If seeking past any existing range and the seek is pending
 // because no data has been provided for that position,
@@ skipping 371 matching lines @@
   SetTextStream();
   NewCodedFrameGroupAppend("1500K 2000K 2500K 3000K 3500K");
   CheckExpectedRangesByTimestamp("{ [1500,4000) }");
   NewCodedFrameGroupAppend("0K 501K 1001K 1501K 2001K");
   CheckExpectedRangesByTimestamp("{ [0,4000) }");
 
   Seek(0);
   CheckExpectedBuffers("0K 501K 1001K 1501K 2001K 3000K 3500K");
 }
 
-TEST_F(SourceBufferStreamTest, SpliceFrame_Basic) {
-  Seek(0);
-  NewCodedFrameGroupAppend("0K S(3K 6 9D3 10D5) 15 20 S(25K 30D5 35D5) 40");
-  CheckExpectedBuffers("0K 3K 6 9 C 10 15 20 25K 30 C 35 40");
-  CheckNoNextBuffer();
-}
-
-TEST_F(SourceBufferStreamTest, SpliceFrame_SeekClearsSplice) {
-  Seek(0);
-  NewCodedFrameGroupAppend("0K S(3K 6 9D3 10D5) 15K 20");
-  CheckExpectedBuffers("0K 3K 6");
-
-  SeekToTimestampMs(15);
-  CheckExpectedBuffers("15K 20");
-  CheckNoNextBuffer();
-}
-
-TEST_F(SourceBufferStreamTest, SpliceFrame_SeekClearsSpliceFromTrackBuffer) {
-  Seek(0);
-  NewCodedFrameGroupAppend("0K 2K S(3K 6 9D3 10D5) 15K 20");
-  CheckExpectedBuffers("0K 2K");
-
-  // Overlap the existing coded frame group.
-  NewCodedFrameGroupAppend("5K 15K 20");
-  CheckExpectedBuffers("3K 6");
-
-  SeekToTimestampMs(15);
-  CheckExpectedBuffers("15K 20");
-  CheckNoNextBuffer();
-}
-
-TEST_F(SourceBufferStreamTest, SpliceFrame_ConfigChangeWithinSplice) {
-  VideoDecoderConfig new_config = TestVideoConfig::Large();
-  ASSERT_FALSE(new_config.Matches(video_config_));
-
-  // Add a new video config, then reset the config index back to the original.
-  stream_->UpdateVideoConfig(new_config);
-  stream_->UpdateVideoConfig(video_config_);
-
-  Seek(0);
-  CheckVideoConfig(video_config_);
-  NewCodedFrameGroupAppend("0K S(3K 6C 9D3 10D5) 15");
-
-  CheckExpectedBuffers("0K 3K C");
-  CheckVideoConfig(new_config);
-  CheckExpectedBuffers("6 9 C");
-  CheckExpectedBuffers("10 C");
-  CheckVideoConfig(video_config_);
-  CheckExpectedBuffers("15");
-  CheckNoNextBuffer();
-}
-
-TEST_F(SourceBufferStreamTest, SpliceFrame_BasicFromTrackBuffer) {
-  Seek(0);
-  NewCodedFrameGroupAppend("0K 5K S(8K 9D1 10D10) 20");
-  CheckExpectedBuffers("0K 5K");
-
-  // Overlap the existing coded frame group.
-  NewCodedFrameGroupAppend("5K 20");
-  CheckExpectedBuffers("8K 9 C 10 20");
-  CheckNoNextBuffer();
-}
-
-TEST_F(SourceBufferStreamTest,
-       SpliceFrame_ConfigChangeWithinSpliceFromTrackBuffer) {
-  VideoDecoderConfig new_config = TestVideoConfig::Large();
-  ASSERT_FALSE(new_config.Matches(video_config_));
-
-  // Add a new video config, then reset the config index back to the original.
-  stream_->UpdateVideoConfig(new_config);
-  stream_->UpdateVideoConfig(video_config_);
-
-  Seek(0);
-  CheckVideoConfig(video_config_);
-  NewCodedFrameGroupAppend("0K 5K S(7K 8C 9D1 10D10) 20");
-  CheckExpectedBuffers("0K 5K");
-
-  // Overlap the existing coded frame group.
-  NewCodedFrameGroupAppend("5K 20");
-  CheckExpectedBuffers("7K C");
-  CheckVideoConfig(new_config);
-  CheckExpectedBuffers("8 9 C");
-  CheckExpectedBuffers("10 C");
-  CheckVideoConfig(video_config_);
-  CheckExpectedBuffers("20");
-  CheckNoNextBuffer();
-}
-
-TEST_F(SourceBufferStreamTest, Audio_SpliceFrame_Basic) {
-  EXPECT_MEDIA_LOG(ContainsGeneratedSpliceLog(3000, 11000));
-
+TEST_F(SourceBufferStreamTest, Audio_SpliceTrimmingForOverlap) {
   SetAudioStream();
   Seek(0);
   NewCodedFrameGroupAppend("0K 2K 4K 6K 8K 10K 12K");
-  NewCodedFrameGroupAppend("11K 13K 15K 17K");
-  CheckExpectedBuffers("0K 2K 4K 6K 8K 10K 12K C 11K 13K 15K 17K");
+  CheckExpectedRangesByTimestamp("{ [0,14) }");
+  // Note that duration  of frame at time 10 is verified to be 2 ms.
+  CheckExpectedBuffers("0K 2K 4K 6K 8K 10D2K 12K");
   CheckNoNextBuffer();
-}
 
-TEST_F(SourceBufferStreamTest, Audio_SpliceFrame_NoExactSplices) {
-  EXPECT_MEDIA_LOG(
-      HasSubstr("Skipping splice frame generation: first new buffer at 10000us "
-                "begins at or before existing buffer at 10000us."));
-
-  SetAudioStream();
-  Seek(0);
-  NewCodedFrameGroupAppend("0K 2K 4K 6K 8K 10K 12K");
-  NewCodedFrameGroupAppend("10K 14K");
-  CheckExpectedBuffers("0K 2K 4K 6K 8K 10K 14K");
-  CheckNoNextBuffer();
-}
-
-// Do not allow splices on top of splices.
-TEST_F(SourceBufferStreamTest, Audio_SpliceFrame_NoDoubleSplice) {
-  InSequence s;
-  EXPECT_MEDIA_LOG(ContainsGeneratedSpliceLog(3000, 11000));
-  EXPECT_MEDIA_LOG(
-      HasSubstr("Skipping splice frame generation: overlapped buffers at "
-                "10000us are in a previously buffered splice."));
-
-  SetAudioStream();
-  Seek(0);
-  NewCodedFrameGroupAppend("0K 2K 4K 6K 8K 10K 12K");
+  // Append new group with front slightly overlapping existing buffer at 10ms.
+  EXPECT_MEDIA_LOG(TrimmedSpliceOverlap(11000, 10000, 1000));
   NewCodedFrameGroupAppend("11K 13K 15K 17K");
 
-  // Verify the splice was created.
-  CheckExpectedBuffers("0K 2K 4K 6K 8K 10K 12K C 11K 13K 15K 17K");
-  CheckNoNextBuffer();
+  // Cross-fade splicing is no longer implemented. Instead we should expect
+  // wholly overlapped buffers to be removed (12K). If a buffer is partially
+  // overlapped (e.g. last millisecond of 10K), the existing buffer should be
+  // trimmed to perfectly abut the newly appended buffers.
   Seek(0);
 
-  // Create a splice before the first splice which would include it.
-  NewCodedFrameGroupAppend("9D2K");
-
-  // A splice on top of a splice should result in a discard of the original
-  // splice and no new splice frame being generated.
-  CheckExpectedBuffers("0K 2K 4K 6K 8K 9K 13K 15K 17K");
+  CheckExpectedRangesByTimestamp("{ [0,19) }");
+  CheckExpectedBuffers("0K 2K 4K 6K 8K 10D1K 11D2K 13K 15K 17K");
   CheckNoNextBuffer();
 }
 
 // Test that a splice is not created if an end timestamp and start timestamp
-// overlap.
+// perfectly overlap.
 TEST_F(SourceBufferStreamTest, Audio_SpliceFrame_NoSplice) {
   SetAudioStream();
   Seek(0);
+
+  // Add 10 frames across 2 *non-overlapping* appends.
   NewCodedFrameGroupAppend("0K 2K 4K 6K 8K 10K");
   NewCodedFrameGroupAppend("12K 14K 16K 18K");
-  CheckExpectedBuffers("0K 2K 4K 6K 8K 10K 12K 14K 16K 18K");
+
+  // Manually inspect the buffers at the no-splice boundary to verify duration
+  // and lack of discard padding (set when splicing).
+  scoped_refptr<StreamParserBuffer> buffer;
+  const DecoderBuffer::DiscardPadding kEmptyDiscardPadding;
+  for (int i = 0; i < 10; i++) {
+    // Verify buffer timestamps and durations are preserved and no buffers have
+    // discard padding (indicating no splice trimming).
+    EXPECT_EQ(SourceBufferStream::kSuccess, stream_->GetNextBuffer(&buffer));
+    EXPECT_EQ(base::TimeDelta::FromMilliseconds(i * 2), buffer->timestamp());
+    EXPECT_EQ(base::TimeDelta::FromMilliseconds(2), buffer->duration());
+    EXPECT_EQ(kEmptyDiscardPadding, buffer->discard_padding());
+  }
+
   CheckNoNextBuffer();
 }
 
-TEST_F(SourceBufferStreamTest, Audio_SpliceFrame_CorrectGroupStartTime) {
-  EXPECT_MEDIA_LOG(ContainsGeneratedSpliceLog(5000, 1000));
-
-  SetAudioStream();
-  Seek(0);
-  NewCodedFrameGroupAppend("0K 2K 4K");
-  CheckExpectedRangesByTimestamp("{ [0,6) }");
-  NewCodedFrameGroupAppend("6K 8K 10K");
-  CheckExpectedRangesByTimestamp("{ [0,12) }");
-  NewCodedFrameGroupAppend("1K 4D2K");
-  CheckExpectedRangesByTimestamp("{ [0,12) }");
-  CheckExpectedBuffers("0K 2K 4K C 1K 4K 6K 8K 10K");
-  CheckNoNextBuffer();
-}
-
-TEST_F(SourceBufferStreamTest, Audio_SpliceFrame_ConfigChange) {
-  EXPECT_MEDIA_LOG(ContainsGeneratedSpliceLog(3000, 5000));
-
-  SetAudioStream();
-
-  AudioDecoderConfig new_config(kCodecVorbis, kSampleFormatPlanarF32,
-                                CHANNEL_LAYOUT_MONO, 1000, EmptyExtraData(),
-                                Unencrypted());
-  ASSERT_NE(new_config.channel_layout(), audio_config_.channel_layout());
-
-  Seek(0);
-  CheckAudioConfig(audio_config_);
-  NewCodedFrameGroupAppend("0K 2K 4K 6K");
-  stream_->UpdateAudioConfig(new_config);
-  NewCodedFrameGroupAppend("5K 8K 12K");
-  CheckExpectedBuffers("0K 2K 4K 6K C 5K 8K 12K");
-  CheckAudioConfig(new_config);
-  CheckNoNextBuffer();
-}
-
-// Ensure splices are not created if there are not enough frames to crossfade.
-TEST_F(SourceBufferStreamTest, Audio_SpliceFrame_NoTinySplices) {
-  EXPECT_MEDIA_LOG(HasSubstr(
-      "Skipping splice frame generation: not enough samples for splicing new "
-      "buffer at 1000us. Have 1000us, but need 2000us."));
+TEST_F(SourceBufferStreamTest, Audio_SpliceTrimming_ExistingTrimming) {
+  const base::TimeDelta kDuration = base::TimeDelta::FromMilliseconds(4);
+  const base::TimeDelta kNoDiscard = base::TimeDelta();
+  const bool is_keyframe = true;
 
   SetAudioStream();
   Seek(0);
 
-  // Overlap the range [0, 2) with [1, 3).  Since each frame has a duration of
-  // 2ms this results in an overlap of 1ms between the ranges.  A splice frame
-  // should not be generated since it requires at least 2 frames, or 2ms in this
-  // case, of data to crossfade.
-  NewCodedFrameGroupAppend("0D2K");
-  CheckExpectedRangesByTimestamp("{ [0,2) }");
-  NewCodedFrameGroupAppend("1D2K");
-  CheckExpectedRangesByTimestamp("{ [0,3) }");
-  CheckExpectedBuffers("0K 1K");
+  // Make a 2 BufferQueues with a mix of buffers containing of start/end

[wolenetz, 2016/09/26 23:52:37]

nit: Make a 2? containing of?

[chcunningham, 2016/10/27 23:36:06]

Done.

+  // discard. Buffer PTS and duration have been adjusted to reflect discard.
+  // Buffers A will be appended first, then B. The start of B will overlap A
+  // to generate a splice.
+  BufferQueue A_buffers;
+  BufferQueue B_buffers;
+
+  // Buffer A1: PTS = 0, front discard = 2ms, duration = 2ms.
+  scoped_refptr<StreamParserBuffer> bufferA1 = StreamParserBuffer::CopyFrom(
+      &kDataA, kDataSize, is_keyframe, DemuxerStream::AUDIO, 0);
+  bufferA1->set_timestamp(base::TimeDelta::FromMilliseconds(0));
+  bufferA1->set_duration(kDuration / 2);
+  const DecoderBuffer::DiscardPadding discardA1 =
+      std::make_pair(kDuration / 2, kNoDiscard);
+  bufferA1->set_discard_padding(discardA1);
+  A_buffers.push_back(bufferA1);
+
+  // Buffer A2: PTS = 2, end discard = 2ms, duration = 2ms.
+  scoped_refptr<StreamParserBuffer> bufferA2 = StreamParserBuffer::CopyFrom(
+      &kDataA, kDataSize, is_keyframe, DemuxerStream::AUDIO, 0);
+  bufferA2->set_timestamp(base::TimeDelta::FromMilliseconds(2));
+  bufferA2->set_duration(kDuration / 2);
+  const DecoderBuffer::DiscardPadding discardA2 =
+      std::make_pair(kNoDiscard, kDuration / 2);
+  bufferA2->set_discard_padding(discardA2);
+  A_buffers.push_back(bufferA2);
+
+  // Buffer B1: PTS = 3, front discard = 2ms, duration = 2ms.
+  scoped_refptr<StreamParserBuffer> bufferB1 = StreamParserBuffer::CopyFrom(
+      &kDataA, kDataSize, is_keyframe, DemuxerStream::AUDIO, 0);
+  bufferB1->set_timestamp(base::TimeDelta::FromMilliseconds(3));
+  bufferB1->set_duration(kDuration / 2);
+  const DecoderBuffer::DiscardPadding discardB1 =
+      std::make_pair(kDuration / 2, kNoDiscard);
+  bufferB1->set_discard_padding(discardB1);
+  B_buffers.push_back(bufferB1);
+
+  // Buffer B2: PTS = 5, no discard padding, duration = 4ms.
+  scoped_refptr<StreamParserBuffer> bufferB2 = StreamParserBuffer::CopyFrom(
+      &kDataA, kDataSize, is_keyframe, DemuxerStream::AUDIO, 0);
+  bufferB2->set_timestamp(base::TimeDelta::FromMilliseconds(5));
+  bufferB2->set_duration(kDuration);
+  B_buffers.push_back(bufferB2);
+
+  // Append buffers, trigger splice trimming.
+  stream_->OnStartOfCodedFrameGroup(bufferA1->GetDecodeTimestamp());
+  stream_->Append(A_buffers);
+  EXPECT_MEDIA_LOG(TrimmedSpliceOverlap(3000, 2000, 1000));
+  stream_->Append(B_buffers);
+
+  // Verify buffers.
+  scoped_refptr<StreamParserBuffer> read_buffer;
+
+  // Buffer A1 was not spliced, should be unchanged.
+  EXPECT_EQ(SourceBufferStream::kSuccess, stream_->GetNextBuffer(&read_buffer));
+  EXPECT_EQ(base::TimeDelta::FromMilliseconds(0), read_buffer->timestamp());
+  EXPECT_EQ(kDuration / 2, read_buffer->duration());
+  EXPECT_EQ(discardA1, read_buffer->discard_padding());
+
+  // Buffer A2 was overlapped by buffer B1 1ms. Splice trimming should trim A2's
+  // duration and increase its discard padding by 1ms.
+  const base::TimeDelta overlap = base::TimeDelta::FromMilliseconds(1);
+  EXPECT_EQ(SourceBufferStream::kSuccess, stream_->GetNextBuffer(&read_buffer));
+  EXPECT_EQ(base::TimeDelta::FromMilliseconds(2), read_buffer->timestamp());
+  EXPECT_EQ((kDuration / 2) - overlap, read_buffer->duration());
+  const DecoderBuffer::DiscardPadding overlap_discard =
+      std::make_pair(discardA2.first, discardA2.second + overlap);
+  EXPECT_EQ(overlap_discard, read_buffer->discard_padding());
+
+  // Buffer B1 is overlapping A2, but B1 should be unchanged - splice trimming
+  // only modifies the earlier buffer (A1).
+  EXPECT_EQ(SourceBufferStream::kSuccess, stream_->GetNextBuffer(&read_buffer));
+  EXPECT_EQ(base::TimeDelta::FromMilliseconds(3), read_buffer->timestamp());
+  EXPECT_EQ(kDuration / 2, read_buffer->duration());
+  EXPECT_EQ(discardA1, read_buffer->discard_padding());

[wolenetz, 2016/09/26 23:52:38]

nit: s/discardA1/discardB1/

[chcunningham, 2016/10/27 23:36:06]

Done.

+
+  // Buffer B2 is not spliced, should be unchanged.
+  EXPECT_EQ(SourceBufferStream::kSuccess, stream_->GetNextBuffer(&read_buffer));
+  EXPECT_EQ(base::TimeDelta::FromMilliseconds(5), read_buffer->timestamp());
+  EXPECT_EQ(kDuration, read_buffer->duration());
+  EXPECT_EQ(std::make_pair(kNoDiscard, kNoDiscard),
+            read_buffer->discard_padding());
+
   CheckNoNextBuffer();
 }
 
 TEST_F(SourceBufferStreamTest, Audio_SpliceFrame_NoMillisecondSplices) {
   EXPECT_MEDIA_LOG(
-      HasSubstr("Skipping splice frame generation: not enough samples for "
-                "splicing new buffer at 1250us. Have 750us, but need 1000us."));
+      HasSubstr("Skipping audio splice trimming at PTS=1250us. Found only 250us"
+                " of overlap, need at least 1000us."));
 
   video_config_ = TestVideoConfig::Invalid();
   audio_config_.Initialize(kCodecVorbis, kSampleFormatPlanarF32,
                            CHANNEL_LAYOUT_STEREO, 4000, EmptyExtraData(),
                            Unencrypted(), base::TimeDelta(), 0);
-  stream_.reset(new SourceBufferStream(audio_config_, media_log_, true));
+  stream_.reset(new SourceBufferStream(audio_config_, media_log_));
   // Equivalent to 0.5ms per frame.
   SetStreamInfo(2000, 2000);
   Seek(0);
 
   // Append four buffers with a 0.5ms duration each.
   NewCodedFrameGroupAppend(0, 4);
   CheckExpectedRangesByTimestamp("{ [0,2) }");
 
   // Overlap the range [0, 2) with [1.25, 2); this results in an overlap of
   // 0.75ms between the ranges.

[wolenetz, 2016/09/26 23:52:38]

nit: s/ranges./ranges, but only 0.25ms end-overlap on the first overlapped
frame./

[chcunningham, 2016/10/27 23:36:06]

Done, but a little different wording.

   NewCodedFrameGroupAppend_OffsetFirstBuffer(
       2, 2, base::TimeDelta::FromMillisecondsD(0.25));
   CheckExpectedRangesByTimestamp("{ [0,2) }");
 
   // A splice frame should not be generated (indicated by the lack of a config

[wolenetz, 2016/09/26 23:52:38]

nit: this "lack of config change" would occur even in a generated-discard-abut
splice. reword?

[chcunningham, 2016/10/27 23:36:06]

Done.

   // change in the expected buffer string) since it requires at least 1ms of
   // data to crossfade.
   CheckExpectedBuffers("0K 0K 1K 1K");
   CheckNoNextBuffer();
 }
 
-TEST_F(SourceBufferStreamTest, Audio_SpliceFrame_Preroll) {
-  EXPECT_MEDIA_LOG(ContainsGeneratedSpliceLog(3000, 11000));
-
-  SetAudioStream();
-  Seek(0);
-  NewCodedFrameGroupAppend("0K 2K 4K 6K 8K 10K 12K");
-  NewCodedFrameGroupAppend("11P 13K 15K 17K");
-  CheckExpectedBuffers("0K 2K 4K 6K 8K 10K 12K C 11P 13K 15K 17K");

[wolenetz, 2016/09/26 23:52:37]

We should still test that config change across different configs at the splice
(discard-abutment) point including some preroll on the overlapping frames
results in the right sequence of : original frames up to the overlapped
truncated frame + config change + overlapping frames.

[chcunningham, 2016/10/27 23:36:06]

Done.

-  CheckNoNextBuffer();
-}
-
 TEST_F(SourceBufferStreamTest, Audio_PrerollFrame) {
   Seek(0);
   NewCodedFrameGroupAppend("0K 3P 6K");
   CheckExpectedBuffers("0K 3P 6K");
   CheckNoNextBuffer();
 }
 
 TEST_F(SourceBufferStreamTest, BFrames) {
   Seek(0);
   NewCodedFrameGroupAppend("0K 120|30 30|60 60|90 90|120");
   CheckExpectedRangesByTimestamp("{ [0,150) }");
 
   CheckExpectedBuffers("0K 120|30 30|60 60|90 90|120");
   CheckNoNextBuffer();
 }
 
-TEST_F(SourceBufferStreamTest, RemoveShouldAlwaysExcludeEnd) {
-  NewCodedFrameGroupAppend("10D2K 12D2 14D2");
-  CheckExpectedRangesByTimestamp("{ [10,16) }");
-
-  // Start new coded frame group, appending KF to abut the start of previous
-  // group.
-  NewCodedFrameGroupAppend("0D10K");
-  Seek(0);
-  CheckExpectedRangesByTimestamp("{ [0,16) }");
-  CheckExpectedBuffers("0K 10K 12 14");
-  CheckNoNextBuffer();

[wolenetz, 2016/09/26 23:52:37]

Why remove this?

[chcunningham, 2016/10/27 23:36:07]

See explanation in earlier patch set:
https://codereview.chromium.org/2343543002/diff/40001/media/filters/source_bu...

[wolenetz, 2016/10/28 23:08:19]

I see. I commented now on that
https://codereview.chromium.org/2343543002/diff/40001/media/filters/source_bu....
Let's use that thread to discuss further.

-
-  // Append another buffer with the same timestamp as the last KF. This triggers
-  // special logic that allows two buffers to have the same timestamp. When
-  // preparing for this new append, there is no reason to remove the later GOP
-  // starting at timestamp 10. This verifies the fix for http://crbug.com/469325
-  // where the decision *not* to remove the start of the overlapped range was
-  // erroneously triggering buffers with a timestamp matching the end
-  // of the append (and any later dependent frames) to be removed.
-  AppendBuffers("0D10");
-  Seek(0);
-  CheckExpectedRangesByTimestamp("{ [0,16) }");
-  CheckExpectedBuffers("0K 0 10K 12 14");
-  CheckNoNextBuffer();
-}
-
 TEST_F(SourceBufferStreamTest, RefinedDurationEstimates_BackOverlap) {
   // Append a few buffers, the last one having estimated duration.
   NewCodedFrameGroupAppend("0K 5 10 20D10E");
   CheckExpectedRangesByTimestamp("{ [0,30) }");
   Seek(0);
   CheckExpectedBuffers("0K 5 10 20D10E");
   CheckNoNextBuffer();
 
   // Append a buffer to the end that overlaps the *back* of the existing range.
   // This should trigger the estimated duration to be recomputed as a timestamp
@@ skipping 428 matching lines @@
   EXPECT_EQ(base::TimeDelta(), stream_->GetHighestPresentationTimestamp());
 }
 
 // TODO(vrk): Add unit tests where keyframes are unaligned between streams.
 // (crbug.com/133557)
 
 // TODO(vrk): Add unit tests with end of stream being called at interesting
 // times.
 
 }  // namespace media