Remove OmxSink interface because Buffer Allocation will go through Allocator in the future, and all allocation required parameters will be passed to "Allocator factory" during intiailized phase of render/decoder.

Remove OMX_BUFFERHEADERTYPE merge logic, this is based on assumptions that all vendor will use one frame per buffer scheme instead of one slice(multiple rows) per buffer which is specified by OpenMAX IL Spec.

Review URL: http://codereview.chromium.org/1786001

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

10 files changed, 90 insertions, 520 deletions

diff --git a/media/base/video_frame.h b/media/base/video_frame.h
index 5341c16..0ad0628 100644
--- a/media/base/video_frame.h
+++ b/media/base/video_frame.h
@@ -36,6 +36,12 @@ class VideoFrame : public StreamSample {
     EMPTY,       // An empty frame.
   };
 
+  enum BufferType {
+    TYPE_SYSTEM_MEMORY,
+    TYPE_OMX_BUFFER_HEAD,
+    TYPE_EGL_IMAGE,
+  };
+
  public:
   // Creates a new frame with given parameters. Buffers for the frame are
   // allocated but not initialized.
@@ -55,6 +61,8 @@ class VideoFrame : public StreamSample {
   static void CreateBlackFrame(int width, int height,
                                scoped_refptr<VideoFrame>* frame_out);
 
+  virtual BufferType type() const { return TYPE_SYSTEM_MEMORY; }
+
   Format format() const { return format_; }
 
   size_t width() const { return width_; }
@@ -80,7 +88,7 @@ class VideoFrame : public StreamSample {
     repeat_count_ = repeat_count;
   }
 
- private:
+ protected:
   // Clients must use the static CreateFrame() method to create a new frame.
   VideoFrame(Format format,
              size_t video_width,
diff --git a/media/filters/omx_video_decode_engine.cc b/media/filters/omx_video_decode_engine.cc
index e0db841..b8794f3 100644
--- a/media/filters/omx_video_decode_engine.cc
+++ b/media/filters/omx_video_decode_engine.cc
@@ -28,7 +28,6 @@ namespace media {
 
 OmxVideoDecodeEngine::OmxVideoDecodeEngine()
     : state_(kCreated),
-      frame_bytes_(0),
       has_fed_on_eos_(false),
       message_loop_(NULL) {
 }
@@ -45,13 +44,6 @@ void OmxVideoDecodeEngine::Initialize(AVStream* stream, Task* done_cb) {
   width_ = stream->codec->width;
   height_ = stream->codec->height;
 
-  // TODO(ajwong): Extract magic formula to something based on output
-  // pixel format.
-  frame_bytes_ = (width_ * height_ * 3) / 2;
-  y_buffer_.reset(new uint8[width_ * height_]);
-  u_buffer_.reset(new uint8[width_ * height_ / 4]);
-  v_buffer_.reset(new uint8[width_ * height_ / 4]);
-
   // TODO(ajwong): Find the right way to determine the Omx component name.
   OmxConfigurator::MediaFormat input_format, output_format;
   memset(&input_format, 0, sizeof(input_format));
@@ -60,7 +52,7 @@ void OmxVideoDecodeEngine::Initialize(AVStream* stream, Task* done_cb) {
   output_format.codec = OmxConfigurator::kCodecRaw;
   omx_configurator_.reset(
       new OmxDecoderConfigurator(input_format, output_format));
-  omx_codec_->Setup(omx_configurator_.get(), this);
+  omx_codec_->Setup(omx_configurator_.get());
   omx_codec_->SetErrorCallback(
       NewCallback(this, &OmxVideoDecodeEngine::OnHardwareError));
   omx_codec_->SetFormatCallback(
@@ -96,6 +88,10 @@ void OmxVideoDecodeEngine::DecodeFrame(Buffer* buffer,
                                        Task* done_cb) {
   DCHECK_EQ(message_loop_, MessageLoop::current());
   if (state_ != kNormal) {
+    *got_result = false;
+    *video_frame = NULL;
+    done_cb->Run();
+    delete done_cb;
     return;
   }
 
@@ -137,52 +133,10 @@ void OmxVideoDecodeEngine::Stop(Callback0::Type* done_cb) {
   state_ = kStopped;
 }
 
-bool OmxVideoDecodeEngine::AllocateEGLImages(
-    int width, int height, std::vector<EGLImageKHR>* images) {
-  NOTREACHED() << "This method is never used";
-  return false;
-}
-
-void OmxVideoDecodeEngine::ReleaseEGLImages(
-    const std::vector<EGLImageKHR>& images) {
-  NOTREACHED() << "This method is never used";
-}
-
-void OmxVideoDecodeEngine::UseThisBuffer(int buffer_id,
-                                         OMX_BUFFERHEADERTYPE* buffer) {
-  DCHECK_EQ(message_loop_, MessageLoop::current());
-  omx_buffers_.push_back(std::make_pair(buffer_id, buffer));
-}
-
-void OmxVideoDecodeEngine::StopUsingThisBuffer(int id) {
-  DCHECK_EQ(message_loop_, MessageLoop::current());
-  omx_buffers_.erase(FindBuffer(id));
-}
-
-void OmxVideoDecodeEngine::BufferReady(int buffer_id,
-                                       BufferUsedCallback* callback) {
+void OmxVideoDecodeEngine::OnReadComplete(
+  OMX_BUFFERHEADERTYPE* buffer) {
   DCHECK_EQ(message_loop_, MessageLoop::current());
-  CHECK((buffer_id == OmxCodec::kEosBuffer) || callback);
-
-  // Obtain the corresponding OMX_BUFFERHEADERTYPE.
-  OmxBufferList::iterator iter = FindBuffer(buffer_id);
-  if (iter != omx_buffers_.end()) {
-    uint8* buffer = iter->second->pBuffer;
-    int size = iter->second->nFilledLen;
-
-    if ((size_t)size != frame_bytes_) {
-      LOG(ERROR) << "Read completed with weird size: " << size;
-    }
-
-    // Merge the buffer into previous buffers.
-    MergeBytesFrameQueue(buffer, size);
-  }
-
-  // Notify OmxCodec that we have finished using this buffer.
-  if (callback) {
-    callback->Run(buffer_id);
-    delete callback;
-  }
+  DCHECK_EQ(buffer->nFilledLen, width_*height_*3/2 );
 
   // We assume that when we receive a read complete callback from
   // OmxCodec there was a read request made.
@@ -194,73 +148,31 @@ void OmxVideoDecodeEngine::BufferReady(int buffer_id,
   // Notify the read request to this object has been fulfilled.
   AutoTaskRunner done_cb_runner(request.done_cb);
 
-  // Detect if we have received a full decoded frame.
-  if (DecodedFrameAvailable()) {
-    // |frame| carries the decoded frame.
-    scoped_ptr<YuvFrame> frame(yuv_frame_queue_.front());
-    yuv_frame_queue_.pop_front();
-
-    VideoFrame::CreateFrame(GetSurfaceFormat(),
-                            width_, height_,
-                            StreamSample::kInvalidTimestamp,
-                            StreamSample::kInvalidTimestamp,
-                            request.frame);
-    if (!request.frame->get()) {
-      // TODO(jiesun): this is also an error case handled as normal.
-      return;
-    }
-    *request.got_result = true;
-
-    // TODO(jiesun): Assume YUV 420 format.
-    const int pixels = width_ * height_;
-    memcpy((*request.frame)->data(VideoFrame::kYPlane), frame->data, pixels);
-    memcpy((*request.frame)->data(VideoFrame::kUPlane), frame->data + pixels,
-           pixels / 4);
-    memcpy((*request.frame)->data(VideoFrame::kVPlane),
-           frame->data + pixels + pixels /4,
-           pixels / 4);
-  }
-}
-
-void OmxVideoDecodeEngine::OnReadComplete(
-    int buffer_id, OmxOutputSink::BufferUsedCallback* callback) {
-  BufferReady(buffer_id, callback);
-}
-
-bool OmxVideoDecodeEngine::IsFrameComplete(const YuvFrame* frame) {
-  return frame->size == frame_bytes_;
-}
-
-bool OmxVideoDecodeEngine::DecodedFrameAvailable() {
-  return (!yuv_frame_queue_.empty() &&
-          IsFrameComplete(yuv_frame_queue_.front()));
-}
-
-void OmxVideoDecodeEngine::MergeBytesFrameQueue(uint8* buffer, int size) {
-  int amount_left = size;
+  if (!buffer)  // EOF signal by OmxCodec
+    return;
 
-  // TODO(ajwong): Do the swizzle here instead of in DecodeFrame.  This
-  // should be able to avoid 1 memcpy.
-  while (amount_left > 0) {
-    if (yuv_frame_queue_.empty() || IsFrameComplete(yuv_frame_queue_.back())) {
-      yuv_frame_queue_.push_back(new YuvFrame(frame_bytes_));
-    }
-    YuvFrame* frame = yuv_frame_queue_.back();
-    int amount_to_copy = std::min((int)(frame_bytes_ - frame->size), size);
-    frame->size += amount_to_copy;
-    memcpy(frame->data, buffer, amount_to_copy);
-    amount_left -= amount_to_copy;
+  VideoFrame::CreateFrame(GetSurfaceFormat(),
+                          width_, height_,
+                          StreamSample::kInvalidTimestamp,
+                          StreamSample::kInvalidTimestamp,
+                          request.frame);
+  if (!request.frame->get()) {
+    // TODO(jiesun): this is also an error case handled as normal.
+    return;
   }
-}
 
-OmxVideoDecodeEngine::OmxBufferList::iterator
-OmxVideoDecodeEngine::FindBuffer(int buffer_id) {
-  for (OmxVideoDecodeEngine::OmxBufferList::iterator i = omx_buffers_.begin();
-       i != omx_buffers_.end(); ++i) {
-    if (i->first == buffer_id)
-      return i;
-  }
-  return omx_buffers_.end();
+  // Now we had everything ready.
+  *request.got_result = true;
+
+  // TODO(jiesun): Assume YUV 420 format.
+  // TODO(jiesun): We will use VideoFrame to wrap OMX_BUFFERHEADTYPE.
+  const int pixels = width_ * height_;
+  memcpy((*request.frame)->data(VideoFrame::kYPlane), buffer->pBuffer, pixels);
+  memcpy((*request.frame)->data(VideoFrame::kUPlane), buffer->pBuffer + pixels,
+         pixels / 4);
+  memcpy((*request.frame)->data(VideoFrame::kVPlane),
+         buffer->pBuffer + pixels + pixels /4,
+         pixels / 4);
 }
 
 }  // namespace media
diff --git a/media/filters/omx_video_decode_engine.h b/media/filters/omx_video_decode_engine.h
index 273006c..756ccc1 100644
--- a/media/filters/omx_video_decode_engine.h
+++ b/media/filters/omx_video_decode_engine.h
@@ -25,8 +25,7 @@ namespace media {
 
 // TODO(hclam):
 // Extract the OmxOutputSink implementation to a strategy class.
-class OmxVideoDecodeEngine : public VideoDecodeEngine,
-                             public OmxOutputSink {
+class OmxVideoDecodeEngine : public VideoDecodeEngine {
  public:
   OmxVideoDecodeEngine();
   virtual ~OmxVideoDecodeEngine();
@@ -51,35 +50,7 @@ class OmxVideoDecodeEngine : public VideoDecodeEngine,
     message_loop_ = message_loop;
   };
 
-  // Implementation of OmxOutputSink interface.
-  virtual bool ProvidesEGLImages() const { return false; }
-  virtual bool AllocateEGLImages(int width, int height,
-                                 std::vector<EGLImageKHR>* images);
-  virtual void ReleaseEGLImages(const std::vector<EGLImageKHR>& images);
-  virtual void UseThisBuffer(int buffer_id, OMX_BUFFERHEADERTYPE* buffer);
-  virtual void StopUsingThisBuffer(int id);
-  virtual void BufferReady(int buffer_id, BufferUsedCallback* callback);
-
  private:
-  typedef std::pair<int, OMX_BUFFERHEADERTYPE*> OmxOutputUnit;
-  typedef std::vector<OmxOutputUnit> OmxBufferList;
-
-  struct YuvFrame {
-    // TODO(ajwong): Please avoid ever using this class anywhere else until
-    // we've consolidated the buffer struct.
-    YuvFrame(size_t c) {
-      size = 0;
-      capacity = c;
-      data = new unsigned char[capacity];
-    }
-    ~YuvFrame() {
-      delete [] data;
-    }
-    size_t size;
-    size_t capacity;
-    unsigned char* data;
-  };
-
   // A struct to hold parameters of a decode request. Objects pointed by
   // these parameters are owned by the caller.
   struct DecodeRequest {
@@ -96,38 +67,25 @@ class OmxVideoDecodeEngine : public VideoDecodeEngine,
 
   virtual void OnFeedDone(Buffer* buffer);
   virtual void OnHardwareError();
-  virtual void OnReadComplete(
-      int buffer_id, OmxOutputSink::BufferUsedCallback* callback);
+  virtual void OnReadComplete(OMX_BUFFERHEADERTYPE* buffer);
   virtual void OnFormatChange(
       const OmxConfigurator::MediaFormat& input_format,
       const OmxConfigurator::MediaFormat& output_format);
 
-  virtual bool DecodedFrameAvailable();
-  virtual void MergeBytesFrameQueue(uint8* buffer, int size);
-  virtual bool IsFrameComplete(const YuvFrame* frame);
-  virtual OmxBufferList::iterator FindBuffer(int buffer_id);
-
   State state_;
-  size_t frame_bytes_;
   size_t width_;
   size_t height_;
-  scoped_array<uint8> y_buffer_;
-  scoped_array<uint8> u_buffer_;
-  scoped_array<uint8> v_buffer_;
 
   // TODO(hclam): We should let OmxCodec handle this case.
   bool has_fed_on_eos_;  // Used to avoid sending an end of stream to
   // OpenMax twice since OpenMax does not always
   // handle this nicely.
-  std::list<YuvFrame*> yuv_frame_queue_;
   std::list<DecodeRequest> decode_request_queue_;
 
   scoped_refptr<media::OmxCodec> omx_codec_;
   scoped_ptr<media::OmxConfigurator> omx_configurator_;
   MessageLoop* message_loop_;
 
-  OmxBufferList omx_buffers_;
-
   DISALLOW_COPY_AND_ASSIGN(OmxVideoDecodeEngine);
 };
 
diff --git a/media/omx/omx_codec.cc b/media/omx/omx_codec.cc
index 70a5fe8..d47e3d0 100644
--- a/media/omx/omx_codec.cc
+++ b/media/omx/omx_codec.cc
@@ -12,7 +12,6 @@
 #include "base/string_util.h"
 #include "media/omx/omx_codec.h"
 #include "media/base/buffers.h"
-#include "media/omx/omx_output_sink.h"
 
 namespace media {
 
@@ -47,21 +46,15 @@ OmxCodec::~OmxCodec() {
   DCHECK_EQ(0u, input_buffers_.size());
   DCHECK_EQ(0u, output_buffers_.size());
   DCHECK(available_input_buffers_.empty());
-  DCHECK(output_buffers_in_use_.empty());
   DCHECK(pending_input_queue_.empty());
   DCHECK(output_queue_.empty());
   DCHECK(processing_input_queue_.empty());
 }
 
-void OmxCodec::Setup(OmxConfigurator* configurator,
-                     OmxOutputSink* output_sink) {
+void OmxCodec::Setup(OmxConfigurator* configurator) {
   DCHECK_EQ(kEmpty, state_);
-
   CHECK(configurator);
-  CHECK(output_sink);
-
   configurator_ = configurator;
-  output_sink_ = output_sink;
 }
 
 void OmxCodec::SetErrorCallback(Callback* callback) {
@@ -76,7 +69,6 @@ void OmxCodec::SetFormatCallback(FormatCallback* callback) {
 
 void OmxCodec::Start() {
   CHECK(configurator_);
-  CHECK(output_sink_);
 
   message_loop_->PostTask(
       FROM_HERE,
@@ -166,9 +158,7 @@ void OmxCodec::ReadTask(ReadCallback* callback) {
 
   // Don't accept read request on error state.
   if (!CanAcceptOutput()) {
-    callback->Run(
-        kEosBuffer,
-        static_cast<OmxOutputSink::BufferUsedCallback*>(NULL));
+    callback->Run(static_cast<OMX_BUFFERHEADERTYPE*>(NULL));
     delete callback;
     return;
   }
@@ -232,29 +222,14 @@ bool OmxCodec::AllocateInputBuffers() {
 bool OmxCodec::AllocateOutputBuffers() {
   DCHECK_EQ(message_loop_, MessageLoop::current());
 
-  if (output_sink_->ProvidesEGLImages()) {
-    // TODO(hclam): Do the following things here:
-    // 1. Call output_sink_->AllocateEGLImages() to allocate some
-    //    EGL images from the sink component.
-    // 2. Call OMX_UseEGLImage() to assign the images to the output
-    //    port.
-    NOTIMPLEMENTED();
-  } else {
-    for (int i = 0; i < output_buffer_count_; ++i) {
-      OMX_BUFFERHEADERTYPE* buffer;
-      OMX_ERRORTYPE error =
-          OMX_AllocateBuffer(component_handle_, &buffer, output_port_,
-                             NULL, output_buffer_size_);
-      if (error != OMX_ErrorNone)
-        return false;
-      output_buffers_.push_back(buffer);
-    }
-  }
-
-  // Tell the sink component to use the buffers for output.
-  for (size_t i = 0; i < output_buffers_.size(); ++i) {
-    output_sink_->UseThisBuffer(static_cast<int>(i),
-                                output_buffers_[i]);
+  for (int i = 0; i < output_buffer_count_; ++i) {
+    OMX_BUFFERHEADERTYPE* buffer;
+    OMX_ERRORTYPE error =
+        OMX_AllocateBuffer(component_handle_, &buffer, output_port_,
+                           NULL, output_buffer_size_);
+    if (error != OMX_ErrorNone)
+      return false;
+    output_buffers_.push_back(buffer);
   }
 
   return true;
@@ -277,25 +252,10 @@ void OmxCodec::FreeInputBuffers() {
 void OmxCodec::FreeOutputBuffers() {
   DCHECK_EQ(message_loop_, MessageLoop::current());
 
-  // First we need to make sure output sink is not using the buffer so
-  // tell it to stop using our buffer headers.
-  // TODO(hclam): We should make this an asynchronous call so that
-  // we'll wait until all buffers are not used.
-  for (size_t i = 0; i < output_buffers_.size(); ++i)
-    output_sink_->StopUsingThisBuffer(static_cast<int>(i));
-
   // Calls to OMX to free buffers.
   for (size_t i = 0; i < output_buffers_.size(); ++i)
     OMX_FreeBuffer(component_handle_, output_port_, output_buffers_[i]);
   output_buffers_.clear();
-
-  // If we have asked the sink component to provide us EGL images for
-  // output we need to tell it we are done with those images.
-  // TODO(hclam): Implement this correctly.
-  if (output_sink_->ProvidesEGLImages()) {
-    std::vector<EGLImageKHR> images;
-    output_sink_->ReleaseEGLImages(images);
-  }
 }
 
 void OmxCodec::FreeInputQueue() {
@@ -1119,12 +1079,10 @@ void OmxCodec::FulfillOneRead() {
     // If the buffer is real then save it to the in-use list.
     // Otherwise if it is an end-of-stream buffer then just drop it.
     if (buffer_id != kEosBuffer) {
-      output_buffers_in_use_.push_back(buffer_id);
-      callback->Run(buffer_id,
-                    NewCallback(this, &OmxCodec::BufferUsedCallback));
+      callback->Run(output_buffers_[buffer_id]);
+      BufferUsedCallback(buffer_id); //hack, we will change this really soon.
     } else {
-      callback->Run(kEosBuffer,
-                    static_cast<OmxOutputSink::BufferUsedCallback*>(NULL));
+      callback->Run(static_cast<OMX_BUFFERHEADERTYPE*>(NULL));
     }
     delete callback;
   }
@@ -1158,19 +1116,6 @@ void OmxCodec::BufferUsedTask(int buffer_id) {
   // Make sure an end-of-stream buffer id is not received here.
   CHECK(buffer_id != kEosBuffer);
 
-  // The use of |output_buffers_in_use_| is just a precaution. So we can
-  // actually move it to a debugging section.
-  OutputBuffersInUseSet::iterator iter =
-      std::find(output_buffers_in_use_.begin(),
-                output_buffers_in_use_.end(),
-                buffer_id);
-
-  if (iter == output_buffers_in_use_.end()) {
-    LOG(ERROR) << "Received an unknown buffer id: " << buffer_id;
-    StateTransitionTask(kError);
-  }
-  output_buffers_in_use_.erase(iter);
-
   // We'll try to issue more FillThisBuffer() to the decoder.
   // If we can't do it now then just return.
   if (!CanFillBuffer())
diff --git a/media/omx/omx_codec.h b/media/omx/omx_codec.h
index f558827..4de2b78 100644
--- a/media/omx/omx_codec.h
+++ b/media/omx/omx_codec.h
@@ -12,15 +12,12 @@
 //
 // OWNERSHIP
 //
-// The OmxCodec works with two external objects, they are:
-// 1. OmxConfigurator
+// The OmxCodec works with external objects
+// OmxConfigurator
 //    This object is given to OmxCodec to perform port configuration.
-// 2. OmxOutputSink
-//    This object is given to OmxCodec to perform output buffer negotiation.
-//
-// These two external objects are provided and destroyed externally. Their
-// references are given to OmxCodec and client application is responsible
-// for cleaning them.
+//    This object is provided and destroyed externally. Its references
+//    are given to OmxCodec and client application is responsible
+//    for cleaning them.
 //
 // INTERACTION WITH EXTERNAL OBJECTS
 //
@@ -53,9 +50,8 @@
 // output_format.codec = OmxCodec::kCodecRaw;
 // scoped_ptr<OmxConfigurator> configurator(
 //     new OmxDecoderConfigurator(input_format, output_format));
-// scoped_ptr<OmxOutputSink> output_sink(new CustomOutputSink());
 //
-// decoder->Setup(configurator.get(), output_sink.get());
+// decoder->Setup(configurator.get());
 // decoder->SetErrorCallback(NewCallback(this, &Client::ErrorCallback));
 // decoder->SetFormatCallback(NewCallback(this, &Client::FormatCallback));
 //
@@ -142,7 +138,6 @@
 #include "base/callback.h"
 #include "base/scoped_ptr.h"
 #include "media/omx/omx_configurator.h"
-#include "media/omx/omx_output_sink.h"
 #include "third_party/openmax/il/OMX_Component.h"
 #include "third_party/openmax/il/OMX_Core.h"
 #include "third_party/openmax/il/OMX_Video.h"
@@ -160,8 +155,7 @@ class OmxCodec : public base::RefCountedThreadSafe<OmxCodec> {
       const OmxConfigurator::MediaFormat&,
       const OmxConfigurator::MediaFormat&>::Type FormatCallback;
   typedef Callback1<Buffer*>::Type FeedCallback;
-  typedef Callback2<int,
-      OmxOutputSink::BufferUsedCallback*>::Type ReadCallback;
+  typedef Callback1<OMX_BUFFERHEADERTYPE*>::Type ReadCallback;
   typedef Callback0::Type Callback;
 
   // Initialize an OmxCodec object that runs on |message_loop|. It is
@@ -171,7 +165,7 @@ class OmxCodec : public base::RefCountedThreadSafe<OmxCodec> {
 
   // Setup OmxCodec using |configurator|. |configurator| and |output_sink|
   // are not owned by this class and should be cleaned up externally.
-  void Setup(OmxConfigurator* configurator, OmxOutputSink* output_sink);
+  void Setup(OmxConfigurator* configurator);
 
   // Set the error callback. In case of error the callback will be called.
   void SetErrorCallback(Callback* callback);
@@ -373,7 +367,6 @@ class OmxCodec : public base::RefCountedThreadSafe<OmxCodec> {
 
   OMX_COMPONENTTYPE* component_handle_;
   OmxConfigurator* configurator_;
-  OmxOutputSink* output_sink_;
   MessageLoop* message_loop_;
 
   scoped_ptr<FormatCallback> format_callback_;
@@ -404,11 +397,6 @@ class OmxCodec : public base::RefCountedThreadSafe<OmxCodec> {
   // TOOD(hclam): extract it to a separate class.
   std::queue<int> output_buffers_ready_;
 
-  // A set of buffers that are currently in use by the client.
-  // TODO(hclam): extract it to a separate class.
-  typedef std::vector<int> OutputBuffersInUseSet;
-  OutputBuffersInUseSet output_buffers_in_use_;
-
  private:
   DISALLOW_COPY_AND_ASSIGN(OmxCodec);
 };
diff --git a/media/omx/omx_codec_unittest.cc b/media/omx/omx_codec_unittest.cc
index eb4facc..adf8132 100644
--- a/media/omx/omx_codec_unittest.cc
+++ b/media/omx/omx_codec_unittest.cc
@@ -115,26 +115,11 @@ class MockOmxConfigurator : public OmxConfigurator {
                           OMX_PARAM_PORTDEFINITIONTYPE* output_def));
 };
 
-class MockOmxOutputSink : public OmxOutputSink {
- public:
-  MOCK_CONST_METHOD0(ProvidesEGLImages, bool());
-  MOCK_METHOD3(AllocateEGLImages,
-               bool(int width, int height,
-                    std::vector<EGLImageKHR>* images));
-  MOCK_METHOD1(ReleaseEGLImages,
-               void(const std::vector<EGLImageKHR>& images));
-  MOCK_METHOD2(UseThisBuffer, void(int buffer_id,
-                                   OMX_BUFFERHEADERTYPE* buffer));
-  MOCK_METHOD1(StopUsingThisBuffer, void(int buffer_id));
-  MOCK_METHOD2(BufferReady, void(int buffer_id,
-                                 BufferUsedCallback* callback));
-};
-
 class OmxCodecTest : public testing::Test {
  public:
   OmxCodecTest ()
       : omx_codec_(new OmxCodec(&message_loop_)) {
-    omx_codec_->Setup(&mock_configurator_, &mock_output_sink_);
+    omx_codec_->Setup(&mock_configurator_);
   }
 
   ~OmxCodecTest() {
@@ -206,20 +191,11 @@ class OmxCodecTest : public testing::Test {
         .Times(kBufferCount)
         .WillRepeatedly(DoAll(UseBuffer(), Return(OMX_ErrorNone)));
 
-    // Don't support EGL images in this case.
-    EXPECT_CALL(mock_output_sink_, ProvidesEGLImages())
-        .WillOnce(Return(false));
-
     // Expect allocation of output buffers and send command complete.
     EXPECT_CALL(*MockOmx::get(),
                 AllocateBuffer(NotNull(), 1, IsNull(), kBufferSize))
         .Times(kBufferCount)
         .WillRepeatedly(DoAll(AllocateBuffer(), Return(OMX_ErrorNone)));
-
-    // The allocate output buffers will then be passed to output sink.
-    for (int i = 0; i < kBufferCount; ++i) {
-      EXPECT_CALL(mock_output_sink_, UseThisBuffer(i, _));
-    }
   }
 
   void ExpectToExecuting() {
@@ -262,18 +238,9 @@ class OmxCodecTest : public testing::Test {
         .Times(kBufferCount)
         .WillRepeatedly(DoAll(FreeBuffer(), Return(OMX_ErrorNone)));
 
-    // Expect free output buffer.
-    for (int i = 0; i < kBufferCount; ++i) {
-      EXPECT_CALL(mock_output_sink_, StopUsingThisBuffer(i));
-    }
-
     EXPECT_CALL(*MockOmx::get(), FreeBuffer(1, NotNull()))
         .Times(kBufferCount)
         .WillRepeatedly(DoAll(FreeBuffer(), Return(OMX_ErrorNone)));
-
-    // Report that the sink don't provide EGL images.
-    EXPECT_CALL(mock_output_sink_, ProvidesEGLImages())
-        .WillOnce(Return(false));
   }
 
   void ExpectToEmpty() {
@@ -298,9 +265,8 @@ class OmxCodecTest : public testing::Test {
     ExpectToEmpty();
   }
 
-  void ReadCallback(int buffer_id,
-                    OmxOutputSink::BufferUsedCallback* callback) {
-    output_units_.push_back(std::make_pair(buffer_id, callback));
+  void ReadCallback(OMX_BUFFERHEADERTYPE* buffer) {
+    output_units_.push_back(buffer);
   }
 
   void MakeReadRequest() {
@@ -318,13 +284,11 @@ class OmxCodecTest : public testing::Test {
         .RetiresOnSaturation();
   }
 
-  typedef std::pair<int, OmxOutputSink::BufferUsedCallback*> OutputUnit;
-  std::deque<OutputUnit> output_units_;
+  std::deque<OMX_BUFFERHEADERTYPE*> output_units_;
   std::deque<OMX_BUFFERHEADERTYPE*> fill_this_buffer_received_;
 
   MockOmx mock_omx_;
   MockOmxConfigurator mock_configurator_;
-  MockOmxOutputSink mock_output_sink_;
 
   MessageLoop message_loop_;
   scoped_refptr<OmxCodec> omx_codec_;
@@ -358,6 +322,11 @@ TEST_F(OmxCodecTest, EndOfStream) {
   // buffers already.
   EXPECT_EQ(0u, output_units_.size());
   for (int i = 0; i < kBufferCount; ++i) {
+    // Give buffers back to OmxCodec. OmxCodec will make a new
+    // FillThisBuffer() call for each read.
+    EXPECT_CALL(*MockOmx::get(), FillThisBuffer(NotNull()))
+        .WillOnce(DoAll(FillEosBuffer(), Return(OMX_ErrorNone)))
+        .RetiresOnSaturation();
     MakeReadRequest();
   }
   message_loop_.RunAllPending();
@@ -365,19 +334,11 @@ TEST_F(OmxCodecTest, EndOfStream) {
 
   // Make sure buffers received are in order.
   for (int i = 0; i < kBufferCount; ++i) {
-    EXPECT_EQ(i, output_units_[i].first);
-    EXPECT_TRUE(output_units_[i].second != NULL);
+    // TODO(jiesun): How to verify this now?
+    // EXPECT_EQ(i, output_units_[i].first);
+    EXPECT_TRUE(output_units_[i] != NULL);
   }
 
-  // Give buffers back to OmxCodec. OmxCodec will make a new
-  // FillThisBuffer() call for each read.
-  for (int i = kBufferCount - 1; i >= 0; --i) {
-    EXPECT_CALL(*MockOmx::get(), FillThisBuffer(NotNull()))
-        .WillOnce(DoAll(FillEosBuffer(), Return(OMX_ErrorNone)))
-        .RetiresOnSaturation();
-    output_units_[i].second->Run(output_units_[i].first);
-    delete output_units_[i].second;
-  }
   output_units_.clear();
 
   // Make some read requests and make sure end-of-stream buffer id
@@ -390,8 +351,7 @@ TEST_F(OmxCodecTest, EndOfStream) {
   EXPECT_EQ(2 * kBufferCount, static_cast<int>(output_units_.size()));
 
   for (size_t i = 0; i <output_units_.size(); ++i) {
-    EXPECT_EQ(OmxCodec::kEosBuffer, output_units_[i].first);
-    EXPECT_EQ(NULL, output_units_[i].second);
+    EXPECT_EQ(NULL, output_units_[i]);
   }
   output_units_.clear();
 
@@ -413,6 +373,7 @@ TEST_F(OmxCodecTest, OutputFlowControl) {
   // start. Reads issued to OmxCodec will be fulfilled now.
   EXPECT_EQ(0u, output_units_.size());
   for (int i = 0; i < kBufferCount; ++i) {
+    ExpectAndSaveFillThisBuffer();
     MakeReadRequest();
   }
   message_loop_.RunAllPending();
@@ -420,26 +381,22 @@ TEST_F(OmxCodecTest, OutputFlowControl) {
 
   // Make sure buffers received are in order.
   for (int i = 0; i < kBufferCount; ++i) {
-    EXPECT_EQ(i, output_units_[i].first);
-    EXPECT_TRUE(output_units_[i].second != NULL);
+    EXPECT_TRUE(output_units_[i] != NULL);
   }
 
-  // Give the buffer back in reverse order.
-  for (int i = kBufferCount - 1; i >= 0; --i) {
-    ExpectAndSaveFillThisBuffer();
-    output_units_[i].second->Run(output_units_[i].first);
-    delete output_units_[i].second;
-  }
   output_units_.clear();
 
   // In each loop, perform the following actions:
   // 1. Make a read request to OmxCodec.
   // 2. Fake a response for FillBufferDone().
   // 3. Expect read response received.
-  // 4. Give the buffer read back to OmxCodec.
-  // 5. Expect a FillThisBuffer() is called to OpenMAX.
   for (int i = 0; i < kBufferCount; ++i) {
     // 1. First make a read request.
+    // Since a buffer is given back to OmxCodec. A FillThisBuffer() is called
+    // to OmxCodec.
+    EXPECT_CALL(*MockOmx::get(), FillThisBuffer(NotNull()))
+         .WillOnce(Return(OMX_ErrorNone))
+         .RetiresOnSaturation();
     MakeReadRequest();
 
     // 2. Then fake a response from OpenMAX.
@@ -460,17 +417,8 @@ TEST_F(OmxCodecTest, OutputFlowControl) {
     // receive one buffer now. Also expect the buffer id be received in
     // reverse order.
     EXPECT_EQ(1u, output_units_.size());
-    EXPECT_EQ(kBufferCount - i - 1, output_units_.front().first);
-
-    // 4. Since a buffer is given back to OmxCodec. A FillThisBuffer() is called
-    // to OmxCodec.
-    EXPECT_CALL(*MockOmx::get(), FillThisBuffer(NotNull()))
-        .WillOnce(Return(OMX_ErrorNone))
-        .RetiresOnSaturation();
+    //EXPECT_EQ(kBufferCount - i - 1, output_units_.front().first);
 
-    // 5. Give this buffer back to OmxCodec.
-    output_units_.front().second->Run(output_units_.front().first);
-    delete output_units_.front().second;
     output_units_.pop_front();
 
     // Make sure actions are completed.
diff --git a/media/omx/omx_output_sink.h b/media/omx/omx_output_sink.h
deleted file mode 100644
index b4d0768..0000000
--- a/media/omx/omx_output_sink.h
+++ /dev/null
@@ -1,144 +0,0 @@
-// Copyright (c) 2010 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.
-//
-// An abstract class to define the behavior of an output buffer sink for
-// media::OmxCodec. It is responsible for negotiation of buffer allocation
-// for output of media::OmxCodec. It is also responsible for processing buffer
-// output from OpenMAX decoder. It is important to note that implementor must
-// assure usage of this class is safe on the thread where media::OmxCodec
-// lives. Ownership of this object is described in src/media/omx/omx_codec.h.
-//
-// BUFFER NEGOTIATION
-//
-// One of the key function of OmxOutputSink is to negotiate output buffer
-// allocation with OmxCodec. There are two modes of operation:
-//
-// 1. Buffer is provided by OmxCodec.
-//
-// In this case ProvidesEGLImage() will return false and OmxCodec allocates
-// output buffers during initialization. Here's a sample sequence of actions:
-//
-// - OmxCodec allocated buffers during initialization.
-// - OmxOutputSink::UseThisBuffer(id, buffer) is called to assign an output
-//   buffer header to the sink component.
-// - OmxOutputSink maps the output buffer as a texture for rendering.
-// - OmxCodec received dynamic port reconfiguration from the hardware.
-// - OmxOutputSink::StopUsingThisBuffer(id) is called to release the buffer.
-// - OmxOutputSink unmaps the output buffer as texture and make sure there
-//   is not reference to it.
-// - OmxOutputSink::UseThisBuffer(id, buffer) is called to assign a new
-//   output buffer to the sink component.
-// - ...
-//
-// 1. Buffer is provided by OmxCodec as EGL image.
-//
-// - OmxOutputSink::AllocateEGLImages(...) is called to request an EGL image
-//   from the sink component, an associated buffer header is created.
-// - OmxOutputSink::UseThisBuffer(id, buffer) is called to assign an output
-//   buffer header to the sink component.
-// - OmxOutputSink maps the output buffer as a texture for rendering.
-// - OmxCodec received dynamic port reconfiguration from the hardware.
-// - OmxOutputSink::StopUsingThisBuffer(id) is called to release the buffer.
-// - OmxOutputSink unmaps the output buffer as texture and make sure there
-//   is not reference to it.
-// - OmxOutputSink::ReleaseEGLImages() is called to tell the sink component
-//   to release all allocated EGL images.
-// - OmxOutputSink::AllocateEGLImages(...) is called to allocate EGL images.
-// - ...
-//
-// BUFFER READY SIGNALING
-//
-// Another key part of OmxOutputSink is to process the output buffer given
-// by OmxCodec. This is done by signaling the sink component that a
-// particular buffer is ready.
-//
-// This is done through the following sequence:
-//
-// - Owner of this object calls BufferReady(buffer_id, callback).
-// - OmxOutputSink uses the buffer for rendering or other operations
-//   asynchronously.
-// - Callback provided by BufferReady() is called along with a buffer id to
-//   notify the buffer has been consumed.
-//
-// It is up to implementor to decide which thread this callback is executed.
-//
-// THREADING
-//
-// OmxOutputSink::BufferReady(buffer_id) is called from the owner of this
-// object which can be made from any thread. All other methods are made on
-// the thread where OmxCodec lives.
-//
-// When the sink component is notified of buffer ready and the buffer is
-// used BufferUsedCallback is called. There is no gurantee which thread
-// this call is made.
-//
-// OWNERSHIP
-//
-// Described in src/media/omx/omx_codec.h.
-
-#ifndef MEDIA_OMX_OMX_OUTPUT_SINK_
-#define MEDIA_OMX_OMX_OUTPUT_SINK_
-
-#include <vector>
-
-#include "base/callback.h"
-#include "third_party/openmax/il/OMX_Core.h"
-
-// TODO(hclam): This is just to get the build going. Remove this when we
-// include the GLES header.
-typedef void* EGLImageKHR;
-
-namespace media {
-
-class OmxOutputSink {
- public:
-  typedef Callback1<int>::Type BufferUsedCallback;
-  virtual ~OmxOutputSink() {}
-
-  // Returns true if this sink component provides EGL images as output
-  // buffers.
-  virtual bool ProvidesEGLImages() const = 0;
-
-  // Returns a list of EGL images allocated by the sink component. The
-  // EGL images will then be given to the hardware decoder for port
-  // configuration. The amount of EGL images created is controlled by the
-  // sink component. The EGL image allocated is owned by the sink
-  // component.
-  // Returns true if allocate was successful.
-  // TODO(hclam): Improve this API once we know what to do with it.
-  virtual bool AllocateEGLImages(int width, int height,
-                                 std::vector<EGLImageKHR>* images) = 0;
-
-  // Notify the sink component that OmxCodec has done with the EGL
-  // image allocated by AllocateEGLImages(). They can be released by
-  // the sink component any time.
-  virtual void ReleaseEGLImages(
-      const std::vector<EGLImageKHR>& images) = 0;
-
-  // Assign a buffer header to the sink component for output. The sink
-  // component will read from the assicated buffer for the decoded frame.
-  // There is also an associated buffer id to identify the buffer. This id
-  // is used in subsequent steps for identifying the right buffer.
-  // Note that the sink component doesn't own the buffer header.
-  // Note that this method is used to assign buffer only at the
-  // initialization stage and is not used for data delivery.
-  virtual void UseThisBuffer(int buffer_id,
-                             OMX_BUFFERHEADERTYPE* buffer) = 0;
-
-  // Tell the sink component to stop using the buffer identified by the
-  // buffer id.
-  // TODO(hclam): Should accept a callback so notify the operation has
-  // completed.
-  virtual void StopUsingThisBuffer(int id) = 0;
-
-  // Notify the sink component that the buffer identified by buffer id
-  // is ready to be consumed. When the buffer is used, |callback| is
-  // called. Ths callback can be made on any thread.
-  virtual void BufferReady(int buffer_id,
-                           BufferUsedCallback* callback) = 0;
-};
-
-}
-
-#endif  // MEDIA_OMX_OMX_OUTPUT_SINK_
diff --git a/media/tools/omx_test/file_sink.cc b/media/tools/omx_test/file_sink.cc
index abee305..2191980 100644
--- a/media/tools/omx_test/file_sink.cc
+++ b/media/tools/omx_test/file_sink.cc
@@ -10,36 +10,8 @@
 
 namespace media {
 
-bool FileSink::AllocateEGLImages(int width, int height,
-                                 std::vector<EGLImageKHR>* images) {
-  NOTREACHED() << "This method is never used";
-  return false;
-}
-
-void FileSink::ReleaseEGLImages(const std::vector<EGLImageKHR>& images) {
-  NOTREACHED() << "This method is never used";
-}
-
-void FileSink::UseThisBuffer(int buffer_id, OMX_BUFFERHEADERTYPE* buffer) {
-  CHECK(omx_buffers_.find(buffer_id) == omx_buffers_.end());
-  omx_buffers_[buffer_id] = buffer;
-}
-
-void FileSink::StopUsingThisBuffer(int id) {
-  omx_buffers_.erase(id);
-}
-
-void FileSink::BufferReady(int buffer_id, BufferUsedCallback* callback) {
-  CHECK(omx_buffers_.find(buffer_id) != omx_buffers_.end());
-  CHECK(callback);
-
-  OMX_BUFFERHEADERTYPE* omx_buffer = omx_buffers_[buffer_id];
-  uint8* buffer = omx_buffer->pBuffer;
-  int size = omx_buffer->nFilledLen;
-
-  // We never receive an end-of-stream buffer here.
-  CHECK(!(omx_buffer->nFlags & OMX_BUFFERFLAG_EOS));
 
+void FileSink::BufferReady(int size, uint8* buffer) {
   if (size > copy_buf_size_) {
     copy_buf_.reset(new uint8[size]);
     copy_buf_size_ = size;
@@ -62,10 +34,6 @@ void FileSink::BufferReady(int buffer_id, BufferUsedCallback* callback) {
 
   if (output_file_.get())
     fwrite(out_buffer, sizeof(uint8), size, output_file_.get());
-
-  // Always make the callback.
-  callback->Run(buffer_id);
-  delete callback;
 }
 
 bool FileSink::Initialize() {
diff --git a/media/tools/omx_test/file_sink.h b/media/tools/omx_test/file_sink.h
index 58edb04..a20bca1 100644
--- a/media/tools/omx_test/file_sink.h
+++ b/media/tools/omx_test/file_sink.h
@@ -11,13 +11,12 @@
 #include "base/basictypes.h"
 #include "base/scoped_handle.h"
 #include "base/scoped_ptr.h"
-#include "media/omx/omx_output_sink.h"
 
 namespace media {
 
 // This class writes output of a frame decoded by OmxCodec and save it to
 // a file.
-class FileSink : public OmxOutputSink {
+class FileSink {
  public:
   FileSink(std::string output_filename,
            bool simulate_copy,
@@ -31,14 +30,7 @@ class FileSink : public OmxOutputSink {
         csc_buf_size_(0) {
   }
 
-  // OmxOutputSink implementations.
-  virtual bool ProvidesEGLImages() const { return false; }
-  virtual bool AllocateEGLImages(int width, int height,
-                                 std::vector<EGLImageKHR>* images);
-  virtual void ReleaseEGLImages(const std::vector<EGLImageKHR>& images);
-  virtual void UseThisBuffer(int buffer_id, OMX_BUFFERHEADERTYPE* buffer);
-  virtual void StopUsingThisBuffer(int id);
-  virtual void BufferReady(int buffer_id, BufferUsedCallback* callback);
+  virtual void BufferReady(int size, uint8* buffer);
 
   // Initialize this object. Returns true if successful.
   bool Initialize();
@@ -65,8 +57,6 @@ class FileSink : public OmxOutputSink {
   scoped_array<uint8> csc_buf_;
   int csc_buf_size_;
 
-  std::map<int, OMX_BUFFERHEADERTYPE*> omx_buffers_;
-
   DISALLOW_COPY_AND_ASSIGN(FileSink);
 };
 
diff --git a/media/tools/omx_test/omx_test.cc b/media/tools/omx_test/omx_test.cc
index 947147e..284572f 100644
--- a/media/tools/omx_test/omx_test.cc
+++ b/media/tools/omx_test/omx_test.cc
@@ -20,7 +20,6 @@
 #include "media/filters/bitstream_converter.h"
 #include "media/omx/omx_codec.h"
 #include "media/base/data_buffer.h"
-#include "media/omx/omx_output_sink.h"
 #include "media/tools/omx_test/color_space_util.h"
 #include "media/tools/omx_test/file_reader_util.h"
 #include "media/tools/omx_test/file_sink.h"
@@ -34,7 +33,6 @@ using media::OmxCodec;
 using media::OmxConfigurator;
 using media::OmxDecoderConfigurator;
 using media::OmxEncoderConfigurator;
-using media::OmxOutputSink;
 using media::YuvFileReader;
 using media::Buffer;
 using media::DataBuffer;
@@ -111,8 +109,7 @@ class TestApp {
       FeedInputBuffer();
   }
 
-  void ReadCompleteCallback(int buffer,
-                            FileSink::BufferUsedCallback* callback) {
+  void ReadCompleteCallback(OMX_BUFFERHEADERTYPE* buffer) {
     // This callback is received when the decoder has completed a decoding
     // task and given us some output data. The buffer is owned by the decoder.
     if (stopped_ || error_)
@@ -122,7 +119,7 @@ class TestApp {
       first_sample_delivered_time_ = base::TimeTicks::HighResNow();
 
     // If we are readding to the end, then stop.
-    if (buffer == OmxCodec::kEosBuffer) {
+    if (buffer == NULL) {
       codec_->Stop(NewCallback(this, &TestApp::StopCallback));
       return;
     }
@@ -131,7 +128,7 @@ class TestApp {
     codec_->Read(NewCallback(this, &TestApp::ReadCompleteCallback));
 
     if (file_sink_.get())
-      file_sink_->BufferReady(buffer, callback);
+      file_sink_->BufferReady(buffer->nFilledLen, buffer->pBuffer);
 
     // could OMX IL return patial sample for decoder?
     frame_count_++;
@@ -151,7 +148,7 @@ class TestApp {
     // Setup the |codec_| with the message loop of the current thread. Also
     // setup component name, codec format and callbacks.
     codec_ = new OmxCodec(&message_loop_);
-    codec_->Setup(configurator_.get(), file_sink_.get());
+    codec_->Setup(configurator_.get());
     codec_->SetErrorCallback(NewCallback(this, &TestApp::ErrorCallback));
     codec_->SetFormatCallback(NewCallback(this, &TestApp::FormatCallback));