Added IsEndOfStream and IsDiscontiguous flags to buffers.

Added IsEndOfStream and IsDiscontiguous flags to buffers.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=10406

[ralphl, 2009-02-25 01:22:56]

Buffers now have two flags: IsEndOfStream and IsDiscontiguous which the video
renderer base class now pays attention to (allowed me to remove the last TODO
from the base class).  I have added a new test that makes sure that the
IsEndOfStream flag is properly interpreted if we have fewer than 4 frames to
pre-roll.

The StreamSample class is now a base class to avoid lots of duplicate boiler
plate code.

[scherkus (not reviewing), 2009-02-25 01:44:56]

You're going to hate me, but after reviewing 3 other sets of media interfaces
they all use the term "discontinuous" as opposed to "discontiguous" -- so for
the sake of speaking a common language think we could change?

Sorry for not double-checking this earlier, but I wanted to send this out before
too much time passed.

[scherkus (not reviewing), 2009-02-25 19:09:15]

http://codereview.chromium.org/27120/diff/1/2
File media/base/buffers.h (right):

http://codereview.chromium.org/27120/diff/1/2#newcode1
Line 1: // Copyright (c) 2006-2009 The Chromium Authors. All rights reserved.
should probably be 2008-2009

http://codereview.chromium.org/27120/diff/1/2#newcode81
Line 81: StreamSample()
would it make sense to force passing in the timestamp + duration here?

http://codereview.chromium.org/27120/diff/1/4
File media/base/data_buffer.cc (right):

http://codereview.chromium.org/27120/diff/1/4#newcode1
Line 1: // Copyright (c) 2006-2009 The Chromium Authors. All rights reserved.
should probably be 2008-2009

http://codereview.chromium.org/27120/diff/1/5
File media/base/data_buffer.h (right):

http://codereview.chromium.org/27120/diff/1/5#newcode1
Line 1: // Copyright (c) 2006-2009 The Chromium Authors. All rights reserved.
should probably be 2008-2009

http://codereview.chromium.org/27120/diff/1/3
File media/base/data_buffer_unittest.cc (right):

http://codereview.chromium.org/27120/diff/1/3#newcode1
Line 1: // Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
should probably be 2008-2009

http://codereview.chromium.org/27120/diff/1/6
File media/base/mock_media_filters.h (right):

http://codereview.chromium.org/27120/diff/1/6#newcode326
Line 326: surface_locked_(false) {
indent by two

http://codereview.chromium.org/27120/diff/1/6#newcode367
Line 367: *surface = surface_;
considering we haven't declared the copy constructor, I'm a little scared of
assigning structs like this

When I implemented this function I had to use a memcpy, so you may want to
double check

[ralphl, 2009-02-25 19:53:54]

Fixed all formatting, etc.  Also added locking video frames to the test renderer
which uncovered a bug in the mock (which I fixed too!)

http://codereview.chromium.org/27120/diff/1/2
File media/base/buffers.h (right):

http://codereview.chromium.org/27120/diff/1/2#newcode1
Line 1: // Copyright (c) 2006-2009 The Chromium Authors. All rights reserved.
On 2009/02/25 19:09:15, scherkus wrote:
> should probably be 2008-2009

Done.

http://codereview.chromium.org/27120/diff/1/4
File media/base/data_buffer.cc (right):

http://codereview.chromium.org/27120/diff/1/4#newcode1
Line 1: // Copyright (c) 2006-2009 The Chromium Authors. All rights reserved.
On 2009/02/25 19:09:15, scherkus wrote:
> should probably be 2008-2009

Done.

http://codereview.chromium.org/27120/diff/1/5
File media/base/data_buffer.h (right):

http://codereview.chromium.org/27120/diff/1/5#newcode1
Line 1: // Copyright (c) 2006-2009 The Chromium Authors. All rights reserved.
On 2009/02/25 19:09:15, scherkus wrote:
> should probably be 2008-2009

Done.

http://codereview.chromium.org/27120/diff/1/3
File media/base/data_buffer_unittest.cc (right):

http://codereview.chromium.org/27120/diff/1/3#newcode1
Line 1: // Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
On 2009/02/25 19:09:15, scherkus wrote:
> should probably be 2008-2009

Done.

http://codereview.chromium.org/27120/diff/1/6
File media/base/mock_media_filters.h (right):

http://codereview.chromium.org/27120/diff/1/6#newcode326
Line 326: surface_locked_(false) {
On 2009/02/25 19:09:15, scherkus wrote:
> indent by two

Done.

http://codereview.chromium.org/27120/diff/1/6#newcode367
Line 367: *surface = surface_;
On 2009/02/25 19:09:15, scherkus wrote:
> considering we haven't declared the copy constructor, I'm a little scared of
> assigning structs like this
> 
> When I implemented this function I had to use a memcpy, so you may want to
> double check

Fixed a bug in this too.  Should have returned true if it worked.  Updated test
video renderer to lock surfaces.

By the way, the code generated for *surface = surface_ was just fine, but I
changed it anyway.

[scherkus (not reviewing), 2009-02-25 21:07:31]

LGTM

some nits but nothing serious

http://codereview.chromium.org/27120/diff/1015/28
File media/base/buffers.h (right):

http://codereview.chromium.org/27120/diff/1015/28#newcode53
Line 53: // Indicates that this sample is discontiuous from the previous one,
for
spelling on discontinuous

http://codereview.chromium.org/27120/diff/1015/28#newcode74
Line 74: // Sets the value returned by IsDiscontinuous.
erikkay suggested adding parentheses to functions inside comments -- sort of
like how |variable| is used, I guess this would IsDiscontinuous()

I kind of like the idea!

http://codereview.chromium.org/27120/diff/1015/32
File media/base/mock_media_filters.h (right):

http://codereview.chromium.org/27120/diff/1015/32#newcode348
Line 348: EXPECT_TRUE(false);
Use ADD_FAILURE instead

http://codereview.chromium.org/27120/diff/1015/32#newcode352
Line 352: EXPECT_TRUE(false);
Use ADD_FAILURE instead

[ralphl, 2009-02-25 23:25:55]

Fixed all comments.   Uploaded for one more "try" to make sure nothing's broken
on linux.

http://codereview.chromium.org/27120/diff/1015/28
File media/base/buffers.h (right):

http://codereview.chromium.org/27120/diff/1015/28#newcode53
Line 53: // Indicates that this sample is discontiuous from the previous one,
for
On 2009/02/25 21:07:31, scherkus wrote:
> spelling on discontinuous

Done.

http://codereview.chromium.org/27120/diff/1015/28#newcode74
Line 74: // Sets the value returned by IsDiscontinuous.
On 2009/02/25 21:07:31, scherkus wrote:
> erikkay suggested adding parentheses to functions inside comments -- sort of
> like how |variable| is used, I guess this would IsDiscontinuous()
> 
> I kind of like the idea!

Done.

http://codereview.chromium.org/27120/diff/1015/32
File media/base/mock_media_filters.h (right):

http://codereview.chromium.org/27120/diff/1015/32#newcode348
Line 348: EXPECT_TRUE(false);
On 2009/02/25 21:07:31, scherkus wrote:
> Use ADD_FAILURE instead

Done.

http://codereview.chromium.org/27120/diff/1015/32#newcode352
Line 352: EXPECT_TRUE(false);
On 2009/02/25 21:07:31, scherkus wrote:
> Use ADD_FAILURE instead

Done.