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.  
Review URL: http://codereview.chromium.org/40059

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

8 files changed, 107 insertions, 63 deletions

diff --git a/chrome/renderer/media/data_source_impl.cc b/chrome/renderer/media/data_source_impl.cc
index 3b8af52..5e97a80 100644
--- a/chrome/renderer/media/data_source_impl.cc
+++ b/chrome/renderer/media/data_source_impl.cc
@@ -45,7 +45,7 @@ void DataSourceImpl::Stop() {
   // Post a close file stream task to IO message loop, it will signal the read
   // event.
   io_loop_->PostTask(
-      FROM_HERE,NewRunnableMethod(this, &DataSourceImpl::OnCloseFileStream));
+      FROM_HERE, NewRunnableMethod(this, &DataSourceImpl::OnCloseFileStream));
 
   // Wait for close to finish for FileStream.
   close_event_.Wait();
@@ -61,14 +61,14 @@ bool DataSourceImpl::Initialize(const std::string& url) {
   // We should get a call back at OnReceivedResponse().
   media_format_.SetAsString(media::MediaFormat::kMimeType,
                             media::mime_type::kApplicationOctetStream);
-  media_format_.SetAsString(media::MediaFormat::kURL, url); 
+  media_format_.SetAsString(media::MediaFormat::kURL, url);
   return true;
 }
 
-size_t DataSourceImpl::Read(char* data, size_t size) {
+size_t DataSourceImpl::Read(uint8* data, size_t size) {
   DCHECK(stream_.get());
   // Wait until we have downloaded the requested bytes.
-  while(!stopped_) {
+  while (!stopped_) {
     {
       AutoLock auto_lock(lock_);
       if (position_ + size <= downloaded_bytes_)
@@ -99,7 +99,7 @@ bool DataSourceImpl::GetPosition(int64* position_out) {
 
 bool DataSourceImpl::SetPosition(int64 position) {
   DCHECK(stream_.get());
-  while(!stopped_) {
+  while (!stopped_) {
     {
       AutoLock auto_lock(lock_);
       if (position < downloaded_bytes_)
@@ -142,17 +142,19 @@ void DataSourceImpl::OnCreateFileStream(base::PlatformFile file) {
   host_->InitializationComplete();
 }
 
-void DataSourceImpl::OnReadFileStream(char* data, size_t size) {
+void DataSourceImpl::OnReadFileStream(uint8* data, size_t size) {
   if (!stopped_ && stream_.get()) {
+    // net::FileStream::Read wants a char*, not uint8*.
+    char* c_data = reinterpret_cast<char*>(data);
+    COMPILE_ASSERT(sizeof(*c_data) == sizeof(*data), data_not_sizeof_char);
+
     // This method IO operation is asynchronous, it is expected to return
     // ERROR_IO_PENDING, when the operation is done, OnDidFileStreamRead() will
-    // be called.
-    int rv = stream_->Read(data, size, &read_callback_);
-
-    // Since the file handle is asynchronous, return value other than
-    // ERROR_IO_PENDING is an error.
-    if (rv != net::ERR_IO_PENDING) {
-      // TODO(hclam): using something like PipelineError::PIPELINE_READ_ERROR.
+    // be called.  Since the file handle is asynchronous, return value other
+    // than ERROR_IO_PENDING is an error.
+    if (stream_->Read(c_data, size, &read_callback_) != net::ERR_IO_PENDING) {
+      // TODO(hclam): change to PipelineError::PIPELINE_ERROR_READ once ralphl
+      // gets the new pipeline CL checked in.
       host_->Error(media::PIPELINE_ERROR_NETWORK);
     }
   }
diff --git a/chrome/renderer/media/data_source_impl.h b/chrome/renderer/media/data_source_impl.h
index e62bcd9..95e8090 100644
--- a/chrome/renderer/media/data_source_impl.h
+++ b/chrome/renderer/media/data_source_impl.h
@@ -102,7 +102,7 @@ class DataSourceImpl : public media::DataSource {
 
   // Methods called from demuxer thread ---------------------------------------
   // media::DataSource implementation.
-  virtual size_t Read(char* data, size_t size);
+  virtual size_t Read(uint8* data, size_t size);
   virtual bool GetPosition(int64* position_out);
   virtual bool SetPosition(int64 position);
   virtual bool GetSize(int64* size_out);
@@ -120,7 +120,7 @@ class DataSourceImpl : public media::DataSource {
   // Methods called from IO thread --------------------------------------------
   // Handlers for file reading.
   void OnCreateFileStream(base::PlatformFile file);
-  void OnReadFileStream(char* data, size_t size);
+  void OnReadFileStream(uint8* data, size_t size);
   void OnCloseFileStream();
   void OnSeekFileStream(net::Whence whence, int64 position);
   void OnDidFileStreamRead(int size);
diff --git a/media/base/buffers.h b/media/base/buffers.h
index 9cd8c96..972a124 100644
--- a/media/base/buffers.h
+++ b/media/base/buffers.h
@@ -150,7 +150,7 @@ struct VideoSurface {
   size_t strides[kMaxPlanes];
 
   // Array of data pointers to each plane.
-  char* data[kMaxPlanes];
+  uint8* data[kMaxPlanes];
 };
 
 
diff --git a/media/base/filters.h b/media/base/filters.h
index ea4a585..3776c72 100644
--- a/media/base/filters.h
+++ b/media/base/filters.h
@@ -113,7 +113,7 @@ class DataSource : public MediaFilter {
 
   // Read the given amount of bytes into data, returns the number of bytes read
   // if successful, kReadError otherwise.
-  virtual size_t Read(char* data, size_t size) = 0;
+  virtual size_t Read(uint8* data, size_t size) = 0;
 
   // Returns true and the current file position for this file, false if the
   // file position could not be retrieved.
diff --git a/media/base/mock_media_filters.h b/media/base/mock_media_filters.h
index ef89021..ec324ed 100644
--- a/media/base/mock_media_filters.h
+++ b/media/base/mock_media_filters.h
@@ -41,12 +41,14 @@ struct MockFilterConfig {
         video_width(1280u),
         video_height(720u),
         video_surface_format(VideoSurface::YV12),
+        has_audio(true),
         compressed_audio_mime_type(mime_type::kAACAudio),
         uncompressed_audio_mime_type(mime_type::kUncompressedAudio),
         compressed_video_mime_type(mime_type::kH264AnnexB),
         uncompressed_video_mime_type(mime_type::kUncompressedVideo),
         frame_duration(base::TimeDelta::FromMicroseconds(33333)),
-        media_duration(base::TimeDelta::FromSeconds(5)) {
+        media_duration(base::TimeDelta::FromSeconds(5)),
+        media_total_bytes(media_duration.InMilliseconds() * 250) {
   }
 
   MockDataSourceBehavior data_source_behavior;
@@ -54,12 +56,14 @@ struct MockFilterConfig {
   size_t video_width;
   size_t video_height;
   VideoSurface::Format video_surface_format;
+  bool has_audio;
   std::string compressed_audio_mime_type;
   std::string uncompressed_audio_mime_type;
   std::string compressed_video_mime_type;
   std::string uncompressed_video_mime_type;
   base::TimeDelta frame_duration;
   base::TimeDelta media_duration;
+  int64 media_total_bytes;
 };
 
 
@@ -78,7 +82,8 @@ class MockDataSource : public DataSource {
     media_format_.SetAsString(MediaFormat::kMimeType,
                               mime_type::kApplicationOctetStream);
     media_format_.SetAsString(MediaFormat::kURL, url);
-    switch (behavior_) {
+    host_->SetTotalBytes(config_->media_total_bytes);
+    switch (config_->data_source_behavior) {
       case MOCK_DATA_SOURCE_NORMAL_INIT:
         host_->InitializationComplete();
         return true;
@@ -106,21 +111,32 @@ class MockDataSource : public DataSource {
     return &media_format_;
   }
 
-  virtual size_t Read(char* data, size_t size) {
-    return 0;
+  virtual size_t Read(uint8* data, size_t size) {
+    size_t read = static_cast<size_t>(config_->media_total_bytes - position_);
+    if (size < read) {
+      read = size;
+    }
+    memset(data, 0, read);
+    return read;
   }
 
   virtual bool GetPosition(int64* position_out) {
-    *position_out = 0;
-    return false;
+    *position_out = position_;
+    return true;
   }
 
   virtual bool SetPosition(int64 position) {
+    EXPECT_GE(position, 0u);
+    EXPECT_LE(position, config_->media_total_bytes);
+    if (position < 0u || position > config_->media_total_bytes) {
+      return false;
+    }
+    position_ = position;
     return true;
   }
 
   virtual bool GetSize(int64* size_out) {
-    *size_out = 0;
+    *size_out = config_->media_total_bytes;
     return false;
   }
 
@@ -129,13 +145,14 @@ class MockDataSource : public DataSource {
                                   const MockFilterConfig*>;
 
   explicit MockDataSource(const MockFilterConfig* config)
-      : behavior_(config->data_source_behavior) {
+      : config_(config),
+        position_(0) {
   }
 
   virtual ~MockDataSource() {}
 
   void TaskBehavior() {
-    switch (behavior_) {
+    switch (config_->data_source_behavior) {
       case MOCK_DATA_SOURCE_TASK_ERROR_POST_INIT:
       case MOCK_DATA_SOURCE_TASK_ERROR_PRE_INIT:
         host_->Error(PIPELINE_ERROR_NETWORK);
@@ -148,7 +165,8 @@ class MockDataSource : public DataSource {
     }
   }
 
-  MockDataSourceBehavior behavior_;
+  const MockFilterConfig* config_;
+  int64 position_;
   MediaFormat media_format_;
 
   DISALLOW_COPY_AND_ASSIGN(MockDataSource);
@@ -173,25 +191,33 @@ class MockDemuxer : public Demuxer {
   }
 
   virtual size_t GetNumberOfStreams() {
+    size_t num_streams = 0;
+    if (config_->has_audio) {
+      ++num_streams;
+    }
     if (config_->has_video) {
-      return 2;
+      ++num_streams;
     }
-    return 1;
+    return num_streams;
   }
 
   virtual DemuxerStream* GetStream(int stream_id) {
     switch (stream_id) {
       case 0:
-        return &mock_audio_stream_;
+        if (config_->has_audio) {
+          return &mock_audio_stream_;
+        } else if (config_->has_video) {
+          return &mock_video_stream_;
+        }
+        break;
       case 1:
-        if (config_->has_video) {
+        if (config_->has_audio && config_->has_video) {
           return &mock_video_stream_;
         }
-        // Fall-through is correct if no video.
-      default:
-        ADD_FAILURE();
-        return NULL;
+        break;
     }
+    ADD_FAILURE();
+    return NULL;
   }
 
  private:
@@ -199,8 +225,8 @@ class MockDemuxer : public Demuxer {
 
   explicit MockDemuxer(const MockFilterConfig* config)
       : config_(config),
-        mock_audio_stream_(config->compressed_audio_mime_type),
-        mock_video_stream_(config->compressed_video_mime_type) {
+        mock_audio_stream_(config, true),
+        mock_video_stream_(config, false) {
   }
 
   virtual ~MockDemuxer() {}
@@ -208,8 +234,16 @@ class MockDemuxer : public Demuxer {
   // Internal class implements DemuxerStream interface.
   class MockDemuxerStream : public DemuxerStream {
    public:
-    explicit MockDemuxerStream(const std::string& mime_type) {
-      media_format_.SetAsString(MediaFormat::kMimeType, mime_type);
+    MockDemuxerStream(const MockFilterConfig* config, bool is_audio) {
+      if (is_audio) {
+        media_format_.SetAsString(MediaFormat::kMimeType,
+                                  config->compressed_audio_mime_type);
+      } else {
+        media_format_.SetAsString(MediaFormat::kMimeType,
+                                  config->compressed_video_mime_type);
+        media_format_.SetAsInteger(MediaFormat::kWidth, config->video_width);
+        media_format_.SetAsInteger(MediaFormat::kHeight, config->video_height);
+      }
     }
 
     virtual ~MockDemuxerStream() {}
@@ -320,30 +354,32 @@ class MockAudioRenderer : public AudioRenderer {
 
 class MockVideoFrame : public VideoFrame {
  public:
-  explicit MockVideoFrame(const MockFilterConfig* config,
-                          base::TimeDelta timestamp)
-      : config_(config),
-        surface_locked_(false) {
+  MockVideoFrame(size_t video_width,
+                 size_t video_height,
+                 VideoSurface::Format video_surface_format,
+                 base::TimeDelta timestamp,
+                 base::TimeDelta duration,
+                 double ratio_white_to_black) {
+    surface_locked_ = false;
     SetTimestamp(timestamp);
-    SetDuration(config->frame_duration);
-    size_t y_byte_count = config_->video_width * config_->video_height;
+    SetDuration(duration);
+    size_t y_byte_count = video_width * video_height;
     size_t uv_byte_count = y_byte_count / 4;
-    surface_.format = config_->video_surface_format;
-    surface_.width = config_->video_width;
-    surface_.height = config_->video_height;
+    surface_.format = video_surface_format;
+    surface_.width = video_width;
+    surface_.height = video_height;
     surface_.planes = 3;
-    surface_.data[0] = new char[y_byte_count];
-    surface_.data[1] = new char[uv_byte_count];
-    surface_.data[2] = new char[uv_byte_count];
-    surface_.strides[0] = config_->video_width;
-    surface_.strides[1] = config_->video_width / 2;
-    surface_.strides[2] = config_->video_width / 2;
+    surface_.data[0] = new uint8[y_byte_count];
+    surface_.data[1] = new uint8[uv_byte_count];
+    surface_.data[2] = new uint8[uv_byte_count];
+    surface_.strides[0] = video_width;
+    surface_.strides[1] = video_width / 2;
+    surface_.strides[2] = video_width / 2;
     memset(surface_.data[0], 0, y_byte_count);
     memset(surface_.data[1], 0x80, uv_byte_count);
     memset(surface_.data[2], 0x80, uv_byte_count);
-    int64 num_white_pixels = y_byte_count *
-                             timestamp.InMicroseconds() /
-                             config_->media_duration.InMicroseconds();
+    int64 num_white_pixels = static_cast<int64>(y_byte_count *
+                                                ratio_white_to_black);
     if (num_white_pixels > y_byte_count) {
       ADD_FAILURE();
       num_white_pixels = y_byte_count;
@@ -368,7 +404,7 @@ class MockVideoFrame : public VideoFrame {
       return false;
     }
     surface_locked_ = true;
-    DCHECK(sizeof(*surface) == sizeof(surface_));
+    COMPILE_ASSERT(sizeof(*surface) == sizeof(surface_), surface_size_mismatch);
     memcpy(surface, &surface_, sizeof(*surface));
     return true;
   }
@@ -379,7 +415,6 @@ class MockVideoFrame : public VideoFrame {
   }
 
  private:
-  const MockFilterConfig* config_;
   bool surface_locked_;
   VideoSurface surface_;
 
@@ -428,7 +463,14 @@ class MockVideoDecoder : public VideoDecoder {
 
   void DoRead(Assignable<VideoFrame>* buffer) {
     if (mock_frame_time_ < config_->media_duration) {
-      VideoFrame* frame = new MockVideoFrame(config_, mock_frame_time_);
+      VideoFrame* frame = new MockVideoFrame(
+          config_->video_width,
+          config_->video_height,
+          config_->video_surface_format,
+          mock_frame_time_,
+          config_->frame_duration,
+          (mock_frame_time_.InSecondsF() /
+              config_->media_duration.InSecondsF()));
       mock_frame_time_ += config_->frame_duration;
       if (mock_frame_time_ >= config_->media_duration) {
         frame->SetEndOfStream(true);
diff --git a/media/filters/file_data_source.cc b/media/filters/file_data_source.cc
index 95b0ec0..9e8f54b 100644
--- a/media/filters/file_data_source.cc
+++ b/media/filters/file_data_source.cc
@@ -57,7 +57,7 @@ const MediaFormat* FileDataSource::GetMediaFormat() {
   return &media_format_;
 }
 
-size_t FileDataSource::Read(char* data, size_t size) {
+size_t FileDataSource::Read(uint8* data, size_t size) {
   DCHECK(file_);
   AutoLock l(lock_);
   if (file_) {
diff --git a/media/filters/file_data_source.h b/media/filters/file_data_source.h
index 9caa435..1c03cf0 100644
--- a/media/filters/file_data_source.h
+++ b/media/filters/file_data_source.h
@@ -30,7 +30,7 @@ class FileDataSource : public DataSource {
   // Implementation of DataSource.
   virtual bool Initialize(const std::string& url);
   virtual const MediaFormat* GetMediaFormat();
-  virtual size_t Read(char* data, size_t size);
+  virtual size_t Read(uint8* data, size_t size);
   virtual bool GetPosition(int64* position_out);
   virtual bool SetPosition(int64 position);
   virtual bool GetSize(int64* size_out);
diff --git a/media/filters/file_data_source_unittest.cc b/media/filters/file_data_source_unittest.cc
index 7555cb9..3eb5262 100644
--- a/media/filters/file_data_source_unittest.cc
+++ b/media/filters/file_data_source_unittest.cc
@@ -80,7 +80,7 @@ TEST(FileDataSourceTest, ReadData) {
   MediaFormat url_format;
   int64 position;
   int64 size;
-  char ten_bytes[10];
+  uint8 ten_bytes[10];
   std::string url = TestFileURL();
   url_format.SetAsString(MediaFormat::kMimeType, media::mime_type::kURL);
   url_format.SetAsString(MediaFormat::kURL, url);