Add initial implementation of SourceBufferStream

media/filters/source_buffer_stream_unittest.cc

Index: media/filters/source_buffer_stream_unittest.cc
diff --git a/media/filters/source_buffer_stream_unittest.cc b/media/filters/source_buffer_stream_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..2c0130abdc0b5d8e526040891fadf0164dceefe2
--- /dev/null
+++ b/media/filters/source_buffer_stream_unittest.cc
@@ -0,0 +1,379 @@
+// 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 "base/logging.h"
+#include "media/base/data_buffer.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace media {
+
+static const int kDefaultFramesPerSecond = 30;
+static const int kDefaultKeyframesPerSecond = 15;

[acolwell GONE FROM CHROMIUM, 2012/04/26 19:51:38]

nit: I'd make this 1 to make it more realistic

[vrk (LEFT CHROMIUM), 2012/04/27 23:19:30]

Set to 6; modified some other tests as necessary.

+
+class SourceBufferStreamTest : public testing::Test {
+ protected:
+  SourceBufferStreamTest() {
+    SetStreamInfo(kDefaultFramesPerSecond, kDefaultKeyframesPerSecond);
+  }
+
+  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 AppendBuffers(int starting_position, int number_of_buffers) {
+    int keyframe_interval = frames_per_second_ / keyframes_per_second_;
+
+    SourceBufferStream::BufferQueue queue;
+    for (int i = 0; i < number_of_buffers; i++) {
+      int position = starting_position + i;
+      bool is_keyframe = position % keyframe_interval == 0;
+      Buffer* buffer = new DataBuffer(0);
+      buffer->SetDuration(frame_duration_);
+      buffer->SetTimestamp(frame_duration_ * position);
+      buffer->SetKeyframe(is_keyframe);
+      queue.push_back(buffer);
+    }
+    stream_.Append(queue);
+  }
+
+  void Seek(int position) {
+    stream_.Seek(position * frame_duration_);
+  }
+
+  SourceBufferStream::Timespan CreateTimespan(
+      int start_position, int end_position) {
+    return std::make_pair<base::TimeDelta, base::TimeDelta>(
+        start_position * frame_duration_, (end_position + 1) * frame_duration_);
+  }
+
+  void CheckExpectedTimespans(
+      SourceBufferStream::TimespanList expected_times) {
+    SourceBufferStream::TimespanList actual_times = stream_.GetBufferedTime();
+    EXPECT_EQ(expected_times.size(), actual_times.size());
+
+    SourceBufferStream::TimespanList::iterator actual_itr, expected_itr;
+    actual_itr = actual_times.begin();
+    expected_itr = expected_times.begin();
+
+    while (actual_itr != actual_times.end()) {

[acolwell GONE FROM CHROMIUM, 2012/04/26 19:51:38]

nit: use for

[vrk (LEFT CHROMIUM), 2012/04/27 23:19:30]

Done.

+      EXPECT_NE(expected_itr, expected_times.end());
+      if (expected_itr == expected_times.end())

[acolwell GONE FROM CHROMIUM, 2012/04/26 19:51:38]

Should you just put expected_itr != expected_times.end() in the loop condition
instead of putting this code here?

[vrk (LEFT CHROMIUM), 2012/04/27 23:19:30]

Yeah you're right, I already essentially do the check for this on line 57. Done.

+        return;
+
+      EXPECT_EQ(actual_itr->first / frame_duration_,
+                expected_itr->first / frame_duration_);
+      EXPECT_EQ(actual_itr->second / frame_duration_,
+                expected_itr->second / frame_duration_);
+      actual_itr++;
+      expected_itr++;
+    }
+  }
+
+  void CheckExpectedBuffers(
+      int starting_position, int ending_position) {
+    base::TimeDelta first_timestamp = starting_position * frame_duration_;
+
+    int buffers_received = 0;
+    int number_of_buffers = ending_position - starting_position + 1;
+    while (buffers_received < number_of_buffers) {

[acolwell GONE FROM CHROMIUM, 2012/04/26 19:51:38]

nit:use for

[vrk (LEFT CHROMIUM), 2012/04/27 23:19:30]

Done.

+      scoped_refptr<Buffer> buffer;
+      if (!stream_.GetNextBuffer(&buffer))
+        break;
+
+      // Skip earlier buffers.

[acolwell GONE FROM CHROMIUM, 2012/04/26 19:51:38]

Do we really want to skip these buffers? Don't we want to verify the seeking to
the nearest keyframe logic too?

[vrk (LEFT CHROMIUM), 2012/04/27 23:19:30]

Changed and cleaned up some logic as well.

Also this helped me find an off by one bug! (In Merge(), I should not subtract
1.)

+      if (buffer->GetTimestamp() < first_timestamp)
+        continue;
+
+      base::TimeDelta expected_timestamp =
+          (starting_position + buffers_received) * frame_duration_;
+      EXPECT_EQ(buffer->GetTimestamp() / frame_duration_,
+                expected_timestamp / frame_duration_);
+      buffers_received++;
+    }
+    EXPECT_EQ(buffers_received, number_of_buffers);
+  }
+
+  SourceBufferStream stream_;
+
+ private:
+  base::TimeDelta ConvertToFrameDuration(int frames_per_second) {
+    return base::TimeDelta::FromMicroseconds(
+        base::Time::kMicrosecondsPerSecond / frames_per_second);
+  }
+
+  int frames_per_second_;
+  int keyframes_per_second_;
+  base::TimeDelta frame_duration_;
+  DISALLOW_COPY_AND_ASSIGN(SourceBufferStreamTest);
+};
+
+TEST_F(SourceBufferStreamTest, Append_SingleRange) {
+  // Append 12 buffers at positions 0 through 11.
+  AppendBuffers(0, 12);
+
+  // Check expected range.
+  SourceBufferStream::TimespanList expected;
+  expected.push_back(CreateTimespan(0, 11));
+  CheckExpectedTimespans(expected);
+  // Check buffers in range.
+  Seek(0);
+  CheckExpectedBuffers(0, 11);
+}
+
+TEST_F(SourceBufferStreamTest, Append_SingleRange_OneBufferAtATime) {
+  // Append 12 buffers starting at position 0, one buffer at a time.
+  for (int i = 0; i < 12; i++)
+    AppendBuffers(i, 1);
+
+  // Check expected range.
+  SourceBufferStream::TimespanList expected;
+  expected.push_back(CreateTimespan(0, 11));
+  CheckExpectedTimespans(expected);
+  // Check buffers in range.
+  Seek(0);
+  CheckExpectedBuffers(0, 11);
+}
+
+TEST_F(SourceBufferStreamTest, Append_DisjointRanges) {
+  // Append 12 buffers at positions 0 through 11.
+  AppendBuffers(0, 12);
+
+  // Append 12 buffers at positions 14 through 25.
+  AppendBuffers(14, 12);
+
+  // Check expected ranges.
+  SourceBufferStream::TimespanList expected;
+  expected.push_back(CreateTimespan(0, 11));
+  expected.push_back(CreateTimespan(14, 25));
+  CheckExpectedTimespans(expected);
+  // Check buffers in ranges.
+  Seek(0);
+  CheckExpectedBuffers(0, 11);
+  Seek(14);
+  CheckExpectedBuffers(14, 25);
+}
+
+TEST_F(SourceBufferStreamTest, Append_AdjacentRanges) {
+  // Append 12 buffers at positions 0 through 11.
+  AppendBuffers(0, 12);
+
+  // Append 12 buffers at positions 14 through 25.
+  AppendBuffers(14, 12);
+
+  // Append 2 buffers at positions 12 through 13 to bridge the gap.
+  AppendBuffers(12, 2);
+
+  // Check expected range.
+  SourceBufferStream::TimespanList expected;
+  expected.push_back(CreateTimespan(0, 25));
+  CheckExpectedTimespans(expected);
+  // Check buffers in range.
+  Seek(0);
+  CheckExpectedBuffers(0, 25);
+}
+
+TEST_F(SourceBufferStreamTest, Overlap_Complete) {
+  SetStreamInfo(30, 30);

[acolwell GONE FROM CHROMIUM, 2012/04/26 19:51:38]

Why are you basically setting "all buffers are keyframes" here?

[vrk (LEFT CHROMIUM), 2012/04/27 23:19:30]

Talked offline. Added comment + extra test.

+
+  // Append 6 buffers at positions 6 through 11.
+  AppendBuffers(6, 6);
+
+  // Append 8 buffers at positions 5 through 12.
+  AppendBuffers(5, 8);
+
+  // Check expected range.
+  SourceBufferStream::TimespanList expected;
+  expected.push_back(CreateTimespan(5, 12));
+  CheckExpectedTimespans(expected);
+  // Check buffers in range.
+  Seek(5);
+  CheckExpectedBuffers(5, 12);
+}
+
+TEST_F(SourceBufferStreamTest, Overlap_Start) {
+  // Append 6 buffers at positions 6 through 11.
+  AppendBuffers(6, 6);
+
+  // Append 6 buffers at positions 8 through 13.
+  AppendBuffers(8, 6);
+
+  // Check expected range.
+  SourceBufferStream::TimespanList expected;
+  expected.push_back(CreateTimespan(6, 13));
+  CheckExpectedTimespans(expected);
+  // Check buffers in range.
+  Seek(6);
+  CheckExpectedBuffers(6, 13);
+}
+
+TEST_F(SourceBufferStreamTest, Overlap_End) {
+  // Append 6 buffers at positions 8 through 13.
+  AppendBuffers(8, 6);
+
+  // Append 6 buffers at positions 6 through 11.
+  AppendBuffers(6, 6);
+
+  // Check expected range.
+  SourceBufferStream::TimespanList expected;
+  expected.push_back(CreateTimespan(6, 11));
+  CheckExpectedTimespans(expected);
+  // Check buffers in range.
+  Seek(6);
+  CheckExpectedBuffers(6, 11);
+}
+
+TEST_F(SourceBufferStreamTest, Overlap_Several) {
+  // Append 2 buffers at positions 4 through 5.
+  AppendBuffers(4, 2);
+
+  // Append 2 buffers at positions 8 through 9.
+  AppendBuffers(8, 2);
+
+  // Append 2 buffers at positions 12 through 13.
+  AppendBuffers(12, 2);
+
+  // Check expected ranges.
+  SourceBufferStream::TimespanList expected;
+  expected.push_back(CreateTimespan(4, 5));
+  expected.push_back(CreateTimespan(8, 9));
+  expected.push_back(CreateTimespan(12, 13));
+  CheckExpectedTimespans(expected);
+
+  // Append buffers at positions 0 through 15.
+  AppendBuffers(0, 16);
+
+  // Check expected range.
+  expected.clear();
+  expected.push_back(CreateTimespan(0, 15));
+  CheckExpectedTimespans(expected);
+  // Check buffers in range.
+  Seek(0);
+  CheckExpectedBuffers(0, 15);
+}
+
+TEST_F(SourceBufferStreamTest, Overlap_SeveralThenMerge) {
+  // Append 2 buffers at positions 4 through 5.
+  AppendBuffers(4, 2);
+
+  // Append 2 buffers at positions 8 through 9.
+  AppendBuffers(8, 2);
+
+  // Append 2 buffers at positions 12 through 13.
+  AppendBuffers(12, 2);
+
+  // Append 2 buffers at positions 16 through 17.
+  AppendBuffers(16, 2);
+
+  // Append buffers at positions 0 through 15.
+  AppendBuffers(0, 16);
+
+  // Check expected ranges.
+  SourceBufferStream::TimespanList expected;
+  expected.push_back(CreateTimespan(0, 17));
+  CheckExpectedTimespans(expected);
+  // Check buffers in range.
+  Seek(0);
+  CheckExpectedBuffers(0, 17);
+}
+
+TEST_F(SourceBufferStreamTest, Seek_Keyframe) {
+  // Append 6 buffers at positions 0 through 5.
+  AppendBuffers(0, 6);
+
+  // Seek to beginning.
+  Seek(0);
+  CheckExpectedBuffers(0, 5);
+}
+
+TEST_F(SourceBufferStreamTest, Seek_NonKeyframe) {
+  // Append 6 buffers at positions 0 through 5.
+  AppendBuffers(0, 6);
+
+  // Seek to buffer at position 3.
+  Seek(3);
+  CheckExpectedBuffers(3, 5);
+}
+
+TEST_F(SourceBufferStreamTest, Seek_NotBuffered) {
+  // Seek to beginning.
+  Seek(0);
+
+  // Try to get buffer; nothing's appended.
+  scoped_refptr<Buffer> buffer;
+  EXPECT_FALSE(stream_.GetNextBuffer(&buffer));
+
+  // Append 2 buffers at positions 0.
+  AppendBuffers(0, 2);
+  Seek(0);
+  CheckExpectedBuffers(0, 1);
+
+  // Try to get buffer out of range.
+  Seek(2);
+  EXPECT_FALSE(stream_.GetNextBuffer(&buffer));
+}
+
+TEST_F(SourceBufferStreamTest, GetNextBuffer_AfterOverlap) {
+  // Append 6 buffers at positions 0 through 5.
+  AppendBuffers(0, 6);
+
+  // Seek to buffer at position 3.
+  Seek(3);
+
+  // Append 6 buffers at positions 2 through 7.
+  AppendBuffers(2, 6);

[acolwell GONE FROM CHROMIUM, 2012/04/26 19:51:38]

I'm not sure about this case. If the frame at position 3 isn't a keyframe then I
don't think this is the right behavior. This is the exact case the track buffer
was meant to handle. It isn't clear to me that this test properly captures the
subtlety of what happens when you overwrite the current position.

[vrk (LEFT CHROMIUM), 2012/04/27 23:19:30]

Talked offline; modified test a bit and wrote a comment.

+
+  // Make sure we can still get the buffer at 3.
+  CheckExpectedBuffers(3, 3);
+}
+
+TEST_F(SourceBufferStreamTest, GetNextBuffer_AfterMerges) {
+  // Append 4 buffers at positions 6 through 9.
+  AppendBuffers(6, 4);
+
+  // Seek to buffer at position 6.
+  Seek(6);
+
+  // Append 4 buffers at positions 2 through 5.
+  AppendBuffers(2, 4);
+
+  // Make sure ranges are merged.
+  SourceBufferStream::TimespanList expected;
+  expected.push_back(CreateTimespan(2, 9));
+  CheckExpectedTimespans(expected);
+
+  // Make sure the next buffer is correct.
+  CheckExpectedBuffers(7, 7);
+
+  // Append 2 buffers at positions 10 through 11.
+  AppendBuffers(10, 2);
+  expected.clear();
+  expected.push_back(CreateTimespan(2, 11));
+  CheckExpectedTimespans(expected);
+
+  // Make sure the next buffer is correct.
+  CheckExpectedBuffers(8, 8);
+}
+
+TEST_F(SourceBufferStreamTest, GetNextBuffer_ExhaustThenAppend) {
+  // Append 4 buffers at positions 0 through 3.
+  AppendBuffers(0, 4);
+
+  // Seek to buffer at position 0 and get all buffers.
+  Seek(0);
+  CheckExpectedBuffers(0, 3);
+
+  // Next buffer is at position 4, so should not be able to fulfill request.
+  scoped_refptr<Buffer> buffer;
+  EXPECT_FALSE(stream_.GetNextBuffer(&buffer));
+
+  // Append 2 buffers at positions 4 through 5.
+  AppendBuffers(4, 2);
+  CheckExpectedBuffers(4, 5);
+}
+

[acolwell GONE FROM CHROMIUM, 2012/04/26 19:51:38]

I think you should add a test case that seeks to a position that doesn't exactly
match a timestamp on a buffer and make sure that it starts at the proper
keyframe.

[vrk (LEFT CHROMIUM), 2012/04/27 23:19:30]

Added a test, thought it looks a little funny. Let me know if you want me to
tweak it!

+}  // namespace media