Verify supposed splice frames; don't generate for end ts overlap.

We can't rely on the splice frame marking from SourceBufferStream
since the demuxed timestamps and durations may not be accurate.

The code now early returns if a splice doesn't actually exist and
also avoids generating splice frames for end timestamp overlap.

BUG=356073
TEST=new unittests. manual verification against crashes.
R=acolwell@chromium.org

Review URL: https://codereview.chromium.org/211553002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@259329 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 78 insertions, 3 deletions

diff --git a/media/base/audio_splicer.cc b/media/base/audio_splicer.cc
index aae2595..e5dc312 100644
--- a/media/base/audio_splicer.cc
+++ b/media/base/audio_splicer.cc
@@ -99,6 +99,10 @@ class AudioStreamSanitizer {
     return output_timestamp_helper_;
   }
 
+  // Transfer all buffers into |output|.  Returns false if AddInput() on the
+  // |output| sanitizer fails for any buffer removed from |this|.
+  bool DrainInto(AudioStreamSanitizer* output);
+
  private:
   void AddOutputBuffer(const scoped_refptr<AudioBuffer>& buffer);
 
@@ -245,6 +249,14 @@ base::TimeDelta AudioStreamSanitizer::GetDuration() const {
          output_timestamp_helper_.base_timestamp();
 }
 
+bool AudioStreamSanitizer::DrainInto(AudioStreamSanitizer* output) {
+  while (HasNextBuffer()) {
+    if (!output->AddInput(GetNextBuffer()))
+      return false;
+  }
+  return true;
+}
+
 AudioSplicer::AudioSplicer(int samples_per_second)
     : max_crossfade_duration_(
           base::TimeDelta::FromMilliseconds(kCrossfadeDurationInMilliseconds)),
@@ -330,6 +342,19 @@ bool AudioSplicer::AddInput(const scoped_refptr<AudioBuffer>& input) {
     return true;
   }
 
+  // If a splice frame was incorrectly marked due to poor demuxed timestamps, we
+  // may not actually have a splice frame.  In this case, just transfer all data
+  // to the output sanitizer.
+  if (pre_splice_sanitizer_->timestamp_helper().GetTimestamp() ==
+      post_splice_sanitizer_->timestamp_helper().base_timestamp()) {
+    CHECK(pre_splice_sanitizer_->DrainInto(output_sanitizer_.get()));
+    CHECK(post_splice_sanitizer_->DrainInto(output_sanitizer_.get()));
+    splice_timestamp_ = kNoTimestamp();
+    pre_splice_sanitizer_->Reset();
+    post_splice_sanitizer_->Reset();
+    return true;
+  }
+
   scoped_refptr<AudioBuffer> crossfade_buffer;
   scoped_ptr<AudioBus> pre_splice =
       ExtractCrossfadeFromPreSplice(&crossfade_buffer);
@@ -390,6 +415,9 @@ scoped_ptr<AudioBus> AudioSplicer::ExtractCrossfadeFromPreSplice(
       max_crossfade_frame_count,
       std::min(pre_splice_sanitizer_->GetFrameCount() - frames_before_splice,
                post_splice_sanitizer_->GetFrameCount()));
+  // There must always be frames to crossfade, otherwise the splice should not
+  // have been generated.
+  DCHECK_GT(frames_to_crossfade, 0);
 
   int frames_read = 0;
   scoped_ptr<AudioBus> output_bus;
@@ -501,8 +529,7 @@ void AudioSplicer::CrossfadePostSplice(
   }
 
   // Transfer all remaining buffers out and reset once empty.
-  while (post_splice_sanitizer_->HasNextBuffer())
-    CHECK(output_sanitizer_->AddInput(post_splice_sanitizer_->GetNextBuffer()));
+  CHECK(post_splice_sanitizer_->DrainInto(output_sanitizer_.get()));
   post_splice_sanitizer_->Reset();
 }
 
diff --git a/media/base/audio_splicer_unittest.cc b/media/base/audio_splicer_unittest.cc
index be72273..ba2d6e8 100644
--- a/media/base/audio_splicer_unittest.cc
+++ b/media/base/audio_splicer_unittest.cc
@@ -590,4 +590,41 @@ TEST_F(AudioSplicerTest, PartialOverlapCrossfadeShortPreSplice) {
   EXPECT_FALSE(splicer_.HasNextBuffer());
 }
 
+// Test behavior when a splice frame is incorrectly marked and does not actually
+// overlap.
+// +----------+
+// |1111111111|
+// +----------+
+//            +--------------+
+//            |22222222222222|
+//            +--------------+
+// Results in:
+// +----------+--------------+
+// |1111111111|22222222222222|
+// +----------+--------------+
+TEST_F(AudioSplicerTest, IncorrectlyMarkedSplice) {
+  const int kBufferSize =
+      input_timestamp_helper_.GetFramesToTarget(max_crossfade_duration()) * 2;
+
+  scoped_refptr<AudioBuffer> first_buffer =
+      GetNextInputBuffer(1.0f, kBufferSize);
+  splicer_.SetSpliceTimestamp(input_timestamp_helper_.GetTimestamp());
+  scoped_refptr<AudioBuffer> second_buffer =
+      GetNextInputBuffer(0.0f, kBufferSize);
+
+  // The splicer should be internally queuing input since |first_buffer| is part
+  // of the supposed splice.
+  EXPECT_TRUE(AddInput(first_buffer));
+  EXPECT_FALSE(splicer_.HasNextBuffer());
+
+  // |second_buffer| should complete the supposed splice, so ensure output is
+  // now available.
+  EXPECT_TRUE(AddInput(second_buffer));
+  ASSERT_TRUE(splicer_.HasNextBuffer());
+
+  VerifyNextBuffer(first_buffer);
+  VerifyNextBuffer(second_buffer);
+  EXPECT_FALSE(splicer_.HasNextBuffer());
+}
+
 }  // namespace media
diff --git a/media/filters/source_buffer_stream.cc b/media/filters/source_buffer_stream.cc
index 36024e3..a7d3ecf 100644
--- a/media/filters/source_buffer_stream.cc
+++ b/media/filters/source_buffer_stream.cc
@@ -2175,7 +2175,7 @@ bool SourceBufferRange::GetBuffersInRange(base::TimeDelta start,
     const scoped_refptr<StreamParserBuffer>& buffer = *it;
     // Buffers without duration are not supported, so bail if we encounter any.
     if (buffer->duration() == kNoTimestamp() ||
-        buffer->duration() < base::TimeDelta()) {
+        buffer->duration() <= base::TimeDelta()) {
       return false;
     }
     if (buffer->end_of_stream() || buffer->timestamp() >= end)
diff --git a/media/filters/source_buffer_stream_unittest.cc b/media/filters/source_buffer_stream_unittest.cc
index 0821f55..a41fb9c 100644
--- a/media/filters/source_buffer_stream_unittest.cc
+++ b/media/filters/source_buffer_stream_unittest.cc
@@ -3672,6 +3672,17 @@ TEST_F(SourceBufferStreamTest, Audio_SpliceFrame_DoubleSpliceLaterOverlap) {
   CheckNoNextBuffer();
 }
 
+// Test that a splice is not created if an end timestamp and start timestamp
+// overlap.
+TEST_F(SourceBufferStreamTest, Audio_SpliceFrame_NoSplice) {
+  SetAudioStream();
+  Seek(0);
+  NewSegmentAppend("0K 2K 4K 6K 8K 10K");
+  NewSegmentAppend("12K 14K 16K 18K");
+  CheckExpectedBuffers("0K 2K 4K 6K 8K 10K 12K 14K 16K 18K");
+  CheckNoNextBuffer();
+}
+
 // TODO(vrk): Add unit tests where keyframes are unaligned between streams.
 // (crbug.com/133557)