Changed several references from "char" to "uint8" which is the appropriate de...

Changed several references from "char" to "uint8" which is the appropriate definition for byte data buffers.

This change is small, but modifies the data source interface Read() method, and the VideoSurface data structure.  Mocks and
tests all have very small changes.

There are some changes in the mock_media_filters.h file that go beyond the scope of just changing char to uint8.  These
changes are required for future tests, and simply fill in parts of mocks that previously were NOTIMPLEMENTED().  The MockVideoFrame
was modified to allow other tests to create a video frame directly without using the MockFilterConfiguration data structure.  Also,
it is now possible to construct a mock pipeline that has no audio.   The currently checked-in pipeline will NOT work if you don't
have audio.  In a 2nd changelist, I will submit a new Pipeline_Impl and unit test that DOES support video without audio.  

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

[ralphl, 2009-03-03 23:05:34]

Changed data source and video buffers from using "char *" to 
"uint8 *".  Right now the try servers are not working correctly (they can't
apply the patch), but this SHOULD work fine once they get cleaned up.  I will
try before I commit.

Note that there are a few changes to the mock filters that are going to be used
in a future change.

[Erik does not do reviews, 2009-03-04 01:08:59]

LGTM

(yay! rietveld finally let me finish the review)

http://codereview.chromium.org/40059/diff/1/9
File chrome/renderer/media/data_source_impl.cc (right):

http://codereview.chromium.org/40059/diff/1/9#newcode151
Line 151: if (stream_->Read(reinterpret_cast<char*>(data), size,
&read_callback_) !=
bummer that FileStream is using char*.  Maybe file a cleanup bug against that?

[scherkus (not reviewing), 2009-03-04 01:10:09]

I having a really hard time reviewing code right now (site is completely messed
up) so I'll send out whatever I can and take another look when the site issues
are resolved.

http://codereview.chromium.org/40059/diff/1004/30
File chrome/renderer/media/data_source_impl.cc (right):

http://codereview.chromium.org/40059/diff/1004/30#newcode151
Line 151: if (stream_->Read(reinterpret_cast<char*>(data), size,
&read_callback_) !=
nit (maybe): code might look cleaner if we cast to a separate char* variable
first

also rietveld seems to be showing a trailing whitespace here, might want to
double check

http://codereview.chromium.org/40059/diff/1004/30#newcode154
Line 154: // gets the new pipeline CL checked in..
extra period here

http://codereview.chromium.org/40059/diff/1004/28
File media/base/mock_media_filters.h (right):

http://codereview.chromium.org/40059/diff/1004/28#newcode357
Line 357: explicit MockVideoFrame(size_t video_width,
don't need explicit for constructors with more than one argument

[scherkus (not reviewing), 2009-03-04 19:08:22]

rietveld isn't being super annoying today... LGTM!

[ralphl, 2009-03-04 19:28:04]

Fixed all the nits.  Added a compile-time check to make sure the
reinterpret_cast<> never allows a bug through if someone changes the input
parameters to the OnRead...() function.

When try try servers come back OK I'll commit.

http://codereview.chromium.org/40059/diff/1004/30
File chrome/renderer/media/data_source_impl.cc (right):

http://codereview.chromium.org/40059/diff/1004/30#newcode151
Line 151: if (stream_->Read(reinterpret_cast<char*>(data), size,
&read_callback_) !=
On 2009/03/04 01:10:09, scherkus wrote:
> nit (maybe): code might look cleaner if we cast to a separate char* variable
> first
> 
> also rietveld seems to be showing a trailing whitespace here, might want to
> double check

I agree, and I also put in a compile-time check to for the size of the input.  I
HATE using reinterpret_cast without some compile time check that will break if
the input type changes.

http://codereview.chromium.org/40059/diff/1004/30#newcode154
Line 154: // gets the new pipeline CL checked in..
On 2009/03/04 01:10:09, scherkus wrote:
> extra period here

Done.

http://codereview.chromium.org/40059/diff/1004/28
File media/base/mock_media_filters.h (right):

http://codereview.chromium.org/40059/diff/1004/28#newcode357
Line 357: explicit MockVideoFrame(size_t video_width,
On 2009/03/04 01:10:09, scherkus wrote:
> don't need explicit for constructors with more than one argument

Done.

http://codereview.chromium.org/40059/diff/1004/28#newcode357
Line 357: explicit MockVideoFrame(size_t video_width,
On 2009/03/04 01:10:09, scherkus wrote:
> don't need explicit for constructors with more than one argument

Done.