1. make omx engine the driving force to read bit stream

2. update omx_codec_unittest and omx_test correspondingly
3. fix copy&paste error in shader code

BUG=none
TEST=dev platform

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

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

7 files changed, 86 insertions, 99 deletions

diff --git a/media/filters/omx_video_decode_engine.cc b/media/filters/omx_video_decode_engine.cc
index 68b1ac4..4d56f56 100644
--- a/media/filters/omx_video_decode_engine.cc
+++ b/media/filters/omx_video_decode_engine.cc
@@ -37,6 +37,7 @@ OmxVideoDecodeEngine::OmxVideoDecodeEngine()
       input_buffer_count_(0),
       input_buffer_size_(0),
       input_port_(0),
+      input_buffers_at_component_(0),
       input_queue_has_eos_(false),
       input_has_fed_eos_(false),
       output_buffer_count_(0),
@@ -47,6 +48,7 @@ OmxVideoDecodeEngine::OmxVideoDecodeEngine()
       expected_il_state_(kIlNone),
       client_state_(kClientNotInitialized),
       component_handle_(NULL),
+      need_free_input_buffers_(false),
       output_frames_allocated_(false),
       need_setup_output_port_(false) {
   // TODO(wjia): change uses_egl_image_ to runtime setup
@@ -64,17 +66,14 @@ OmxVideoDecodeEngine::~OmxVideoDecodeEngine() {
   DCHECK_EQ(il_state_, kIlNone);
   DCHECK_EQ(0u, input_buffers_.size());
   DCHECK_EQ(0u, output_buffers_.size());
+  DCHECK(free_input_buffers_.empty());
   DCHECK(available_input_buffers_.empty());
-  DCHECK(pending_input_queue_.empty());
+  DCHECK_EQ(0, input_buffers_at_component_);
   DCHECK(output_frames_.empty());
   DCHECK(available_output_frames_.empty());
   DCHECK(output_frames_ready_.empty());
 }
 
-#if !defined(COMPILER_MSVC)
-int const OmxVideoDecodeEngine::kEosBuffer;
-#endif
-
 template <typename T>
 static void ResetParamHeader(const OmxVideoDecodeEngine& dec, T* param) {
   memset(param, 0, sizeof(T));
@@ -114,6 +113,7 @@ void OmxVideoDecodeEngine::Initialize(
 // This method handles only input buffer, without coupling with output
 void OmxVideoDecodeEngine::EmptyThisBuffer(scoped_refptr<Buffer> buffer) {
   DCHECK_EQ(message_loop_, MessageLoop::current());
+  DCHECK(!free_input_buffers_.empty());
 
   if (!CanAcceptInput()) {
     FinishEmptyBuffer(buffer);
@@ -125,8 +125,18 @@ void OmxVideoDecodeEngine::EmptyThisBuffer(scoped_refptr<Buffer> buffer) {
     input_queue_has_eos_ = true;
   }
 
-  // Queue this input buffer.
-  pending_input_queue_.push(buffer);
+  OMX_BUFFERHEADERTYPE* omx_buffer = free_input_buffers_.front();
+  free_input_buffers_.pop();
+
+  // setup |omx_buffer|.
+  omx_buffer->pBuffer = const_cast<OMX_U8*>(buffer->GetData());
+  omx_buffer->nFilledLen = buffer->GetDataSize();
+  omx_buffer->nAllocLen = omx_buffer->nFilledLen;
+  omx_buffer->nFlags |= input_queue_has_eos_ ? OMX_BUFFERFLAG_EOS : 0;
+  omx_buffer->nTimeStamp = buffer->GetTimestamp().InMicroseconds();
+  omx_buffer->pAppPrivate = buffer.get();
+  buffer->AddRef();
+  available_input_buffers_.push(omx_buffer);
 
   // Try to feed buffers into the decoder.
   EmptyBufferTask();
@@ -478,6 +488,9 @@ void OmxVideoDecodeEngine::DoneSetStateIdle(OMX_STATETYPE state) {
   DLOG(INFO) << "OMX video decode engine is in Idle";
 
   il_state_ = kIlIdle;
+
+  // start reading bit stream
+  InitialReadBuffer();
   OnStateSetEventFunc = &OmxVideoDecodeEngine::DoneSetStateExecuting;
   if (!TransitionToState(OMX_StateExecuting)) {
     StopOnError();
@@ -496,10 +509,6 @@ void OmxVideoDecodeEngine::DoneSetStateExecuting(OMX_STATETYPE state) {
   il_state_ = kIlExecuting;
   client_state_ = kClientRunning;
   OnStateSetEventFunc = NULL;
-  // TODO(wjia): The engine should be the driving force to read input buffers.
-  // After video decoder decouples input and output handling, this initial
-  // filling should be changed to call empty_buffer_callback with NULL to pull
-  // bitstream from decoder.
   EmptyBufferTask();
   InitialFillBuffer();
   if (kClientError == client_state_) {
@@ -678,8 +687,6 @@ void OmxVideoDecodeEngine::StopTask(Callback* callback) {
     return;
   }
 
-  FreeInputQueue();
-
   // TODO(wjia): add more state checking
   if (kClientRunning == client_state_) {
     client_state_ = kClientStopping;
@@ -706,7 +713,11 @@ void OmxVideoDecodeEngine::DeinitFromIdle(OMX_STATETYPE state) {
   TransitionToState(OMX_StateLoaded);
   expected_il_state_ = kIlLoaded;
 
-  FreeInputBuffers();
+  if (!input_buffers_at_component_)
+    FreeInputBuffers();
+  else
+    need_free_input_buffers_ = true;
+
   FreeOutputBuffers();
 }
 
@@ -734,7 +745,6 @@ void OmxVideoDecodeEngine::StopOnError() {
   DCHECK_EQ(message_loop_, MessageLoop::current());
 
   client_state_ = kClientStopping;
-  FreeInputQueue();
 
   if (kIlExecuting == expected_il_state_) {
     DeinitFromExecuting(OMX_StateExecuting);
@@ -764,7 +774,7 @@ bool OmxVideoDecodeEngine::AllocateInputBuffers() {
     buffer->nOffset = 0;
     buffer->nFlags = 0;
     input_buffers_.push_back(buffer);
-    available_input_buffers_.push(buffer);
+    free_input_buffers_.push(buffer);
   }
   return true;
 }
@@ -810,15 +820,23 @@ bool OmxVideoDecodeEngine::AllocateOutputBuffers() {
 void OmxVideoDecodeEngine::FreeInputBuffers() {
   DCHECK_EQ(message_loop_, MessageLoop::current());
 
-  // Calls to OMX to free buffers.
-  for (size_t i = 0; i < input_buffers_.size(); ++i)
-    OMX_FreeBuffer(component_handle_, input_port_, input_buffers_[i]);
-  input_buffers_.clear();
-
   // Empty available buffer queue.
+  while (!free_input_buffers_.empty()) {
+    free_input_buffers_.pop();
+  }
+
   while (!available_input_buffers_.empty()) {
+    OMX_BUFFERHEADERTYPE* omx_buffer = available_input_buffers_.front();
     available_input_buffers_.pop();
+    Buffer* stored_buffer = static_cast<Buffer*>(omx_buffer->pAppPrivate);
+    FinishEmptyBuffer(stored_buffer);
+    stored_buffer->Release();
   }
+
+  // Calls to OMX to free buffers.
+  for (size_t i = 0; i < input_buffers_.size(); ++i)
+    OMX_FreeBuffer(component_handle_, input_port_, input_buffers_[i]);
+  input_buffers_.clear();
 }
 
 void OmxVideoDecodeEngine::FreeOutputBuffers() {
@@ -840,16 +858,6 @@ void OmxVideoDecodeEngine::FreeOutputBuffers() {
   }
 }
 
-void OmxVideoDecodeEngine::FreeInputQueue() {
-  DCHECK_EQ(message_loop_, MessageLoop::current());
-
-  while (!pending_input_queue_.empty()) {
-    scoped_refptr<Buffer> buffer = pending_input_queue_.front();
-    FinishEmptyBuffer(buffer);
-    pending_input_queue_.pop();
-  }
-}
-
 bool OmxVideoDecodeEngine::ConfigureIOPorts() {
   OMX_PARAM_PORTDEFINITIONTYPE input_port_def, output_port_def;
   OMX_ERRORTYPE omxresult = OMX_ErrorNone;
@@ -931,30 +939,18 @@ void OmxVideoDecodeEngine::EmptyBufferTask() {
 
   // Loop for all available input data and input buffer for the
   // decoder. When input has reached EOS  we need to stop.
-  while (!pending_input_queue_.empty() &&
-         !available_input_buffers_.empty() &&
+  while (!available_input_buffers_.empty() &&
          !input_has_fed_eos_) {
-    scoped_refptr<Buffer> buffer = pending_input_queue_.front();
-    pending_input_queue_.pop();
-    buffer->AddRef();
-
     OMX_BUFFERHEADERTYPE* omx_buffer = available_input_buffers_.front();
     available_input_buffers_.pop();
 
-    input_has_fed_eos_ = buffer->IsEndOfStream();
+    input_has_fed_eos_ = omx_buffer->nFlags & OMX_BUFFERFLAG_EOS;
     if (input_has_fed_eos_) {
       DLOG(INFO) << "Input has fed EOS";
     }
 
-    // setup |omx_buffer|.
-    omx_buffer->pBuffer = const_cast<OMX_U8*>(buffer.get()->GetData());
-    omx_buffer->nFilledLen = buffer.get()->GetDataSize();
-    omx_buffer->nAllocLen = omx_buffer->nFilledLen;
-    omx_buffer->nFlags |= input_has_fed_eos_ ? OMX_BUFFERFLAG_EOS : 0;
-    omx_buffer->nTimeStamp = buffer->GetTimestamp().InMicroseconds();
-    omx_buffer->pAppPrivate = buffer.get();
-
     // Give this buffer to OMX.
+    input_buffers_at_component_++;
     OMX_ERRORTYPE ret = OMX_EmptyThisBuffer(component_handle_, omx_buffer);
     if (ret != OMX_ErrorNone) {
       LOG(ERROR) << "OMX_EmptyThisBuffer() failed with result " << ret;
@@ -984,6 +980,13 @@ void OmxVideoDecodeEngine::FulfillOneRead() {
   }
 }
 
+void OmxVideoDecodeEngine::InitialReadBuffer() {
+  DCHECK_EQ(message_loop_, MessageLoop::current());
+
+  for (size_t i = 0; i < free_input_buffers_.size(); i++)
+    FinishEmptyBuffer(NULL);
+}
+
 void OmxVideoDecodeEngine::InitialFillBuffer() {
   DCHECK_EQ(message_loop_, MessageLoop::current());
   // DCHECK(output_frames_allocated_);
@@ -1091,14 +1094,19 @@ bool OmxVideoDecodeEngine::TransitionToState(OMX_STATETYPE new_state) {
 
 void OmxVideoDecodeEngine::EmptyBufferDoneTask(OMX_BUFFERHEADERTYPE* buffer) {
   DCHECK_EQ(message_loop_, MessageLoop::current());
+  DCHECK_GT(input_buffers_at_component_, 0);
 
   Buffer* stored_buffer = static_cast<Buffer*>(buffer->pAppPrivate);
   buffer->pAppPrivate = NULL;
   FinishEmptyBuffer(stored_buffer);
   stored_buffer->Release();
 
-  // Enqueue the available buffer beacuse the decoder has consumed it.
-  available_input_buffers_.push(buffer);
+  // Enqueue the available buffer because the decoder has consumed it.
+  free_input_buffers_.push(buffer);
+  input_buffers_at_component_--;
+
+  if (need_free_input_buffers_ && !input_buffers_at_component_)
+    FreeInputBuffers();
 
   // Try to feed more data into the decoder.
   EmptyBufferTask();
diff --git a/media/filters/omx_video_decode_engine.h b/media/filters/omx_video_decode_engine.h
index 9c3d386..6cdfe73 100644
--- a/media/filters/omx_video_decode_engine.h
+++ b/media/filters/omx_video_decode_engine.h
@@ -59,8 +59,6 @@ class OmxVideoDecodeEngine :
   virtual int current_omx_spec_version() const { return 0x00000101; }
 
  private:
-  static const int kEosBuffer = -1;
-
   enum OmxIlState {
     kIlNone,
     kIlLoaded,
@@ -146,7 +144,10 @@ class OmxVideoDecodeEngine :
   // Take one decoded buffer to fulfill one read request.
   void FulfillOneRead();
 
-  // Method doing initial reads to kick start the decoding process.
+  // Method doing initial reads to get bit stream from demuxer.
+  void InitialReadBuffer();
+
+  // Method doing initial fills to kick start the decoding process.
   void InitialFillBuffer();
 
   // helper functions
@@ -199,6 +200,7 @@ class OmxVideoDecodeEngine :
   int input_buffer_count_;
   int input_buffer_size_;
   int input_port_;
+  int input_buffers_at_component_;
   bool input_queue_has_eos_;
   bool input_has_fed_eos_;
 
@@ -226,16 +228,17 @@ class OmxVideoDecodeEngine :
 
   scoped_ptr<Callback> stop_callback_;
 
-  typedef scoped_refptr<Buffer> InputUnit;
-
-  // Input queue for encoded data.
-  // The input buffers will be sent to OMX component via OMX_EmptyThisBuffer()
-  std::queue<InputUnit> pending_input_queue_;
+  // Free input OpenMAX buffers that can be used to take input bitstream from
+  // demuxer.
+  std::queue<OMX_BUFFERHEADERTYPE*> free_input_buffers_;
 
   // Available input OpenMAX buffers that we can use to issue
   // OMX_EmptyThisBuffer() call.
   std::queue<OMX_BUFFERHEADERTYPE*> available_input_buffers_;
 
+  // flag for freeing input buffers
+  bool need_free_input_buffers_;
+
   // For output buffer recycling cases.
   typedef std::pair<scoped_refptr<VideoFrame>,
                     OMX_BUFFERHEADERTYPE*> OutputFrame;
@@ -243,8 +246,10 @@ class OmxVideoDecodeEngine :
   std::queue<OMX_BUFFERHEADERTYPE*> available_output_frames_;
   std::queue<OMX_BUFFERHEADERTYPE*> output_frames_ready_;
   bool output_frames_allocated_;
-  bool need_setup_output_port_;
+
+  // port related
   bool input_port_enabled_;
+  bool need_setup_output_port_;
   OmxIlPortState output_port_state_;
 
   DISALLOW_COPY_AND_ASSIGN(OmxVideoDecodeEngine);
diff --git a/media/filters/omx_video_decoder.cc b/media/filters/omx_video_decoder.cc
index f144fe8..d969cb6 100644
--- a/media/filters/omx_video_decoder.cc
+++ b/media/filters/omx_video_decoder.cc
@@ -144,8 +144,6 @@ void OmxVideoDecoder::InitCompleteTask(FilterCallback* callback) {
   // Check the status of the decode engine.
   if (omx_engine_->state() == VideoDecodeEngine::kError)
     host()->SetError(PIPELINE_ERROR_DECODE);
-  else
-    InitialDemux();
 
   callback->Run();
   delete callback;
@@ -161,19 +159,4 @@ void OmxVideoDecoder::DemuxCompleteTask(Buffer* buffer) {
                         &OmxVideoDecodeEngine::EmptyThisBuffer, ref_buffer));
 }
 
-void OmxVideoDecoder::InitialDemux() {
-  DCHECK_EQ(message_loop(), MessageLoop::current());
-
-  // This is the right time to issue read to the demuxer. The first thing we
-  // need to do here is to determine how many we should read from the
-  // demuxer.
-  // TODO(hclam): Query this number from |omx_engine_|.
-  const int kDemuxPackets = 2;
-  DCHECK(demuxer_stream_);
-  for (int i = 0; i < kDemuxPackets; ++i) {
-    demuxer_stream_->Read(
-        NewCallback(this, &OmxVideoDecoder::DemuxCompleteTask));
-  }
-}
-
 }  // namespace media
diff --git a/media/filters/omx_video_decoder.h b/media/filters/omx_video_decoder.h
index 3f93f3a..18c8b6b 100644
--- a/media/filters/omx_video_decoder.h
+++ b/media/filters/omx_video_decoder.h
@@ -21,8 +21,6 @@ class VideoFrame;
 
 class OmxVideoDecoder : public VideoDecoder {
  public:
-  typedef Callback1<VideoFrame*>::Type ReadCallback;
-
   static FilterFactory* CreateFactory();
   static bool IsMediaFormatSupported(const MediaFormat& media_format);
 
@@ -51,9 +49,6 @@ class OmxVideoDecoder : public VideoDecoder {
   // Calls |omx_engine_|'s EmptyThisBuffer() method on the right thread.
   void EmptyBufferTask(scoped_refptr<Buffer> buffer);
 
-  // Helper method to do the initial demuxing.
-  void InitialDemux();
-
   DemuxerStream* demuxer_stream_;
   bool supports_egl_image_;
   scoped_refptr<OmxVideoDecodeEngine> omx_engine_;
diff --git a/media/omx/omx_codec_unittest.cc b/media/omx/omx_codec_unittest.cc
index 92201d4..71a9ca7 100644
--- a/media/omx/omx_codec_unittest.cc
+++ b/media/omx/omx_codec_unittest.cc
@@ -149,7 +149,8 @@ class TestBuffer : public media::Buffer {
 class OmxCodecTest : public testing::Test {
  public:
   OmxCodecTest ()
-      : got_eos_(false),
+      : input_buffer_count_(0),
+        got_eos_(false),
         omx_engine_(new OmxVideoDecodeEngine()) {
     av_stream_.codec = &av_codec_context_;
     av_codec_context_.width = 16;
@@ -302,7 +303,13 @@ class OmxCodecTest : public testing::Test {
   }
 
   void EmptyBufferDoneCallback(scoped_refptr<Buffer> buffer) {
-    input_units_.push_back(buffer);
+    if (buffer.get()) {
+      input_units_.push_back(buffer);
+    } else {
+      input_buffer_count_++;
+      scoped_refptr<Buffer> buffer_ref = new TestBuffer(input_buffer_count_);
+      input_units_.push_back(buffer_ref);
+    }
   }
 
   void FillBufferDoneCallback(scoped_refptr<VideoFrame> frame) {
@@ -311,13 +318,6 @@ class OmxCodecTest : public testing::Test {
       got_eos_ = true;
   }
 
-  void InitializeInputBuffers(int count) {
-    for (int i = 0; i < count; ++i) {
-      scoped_refptr<Buffer> buffer_ref = new TestBuffer(i + 1);
-      input_units_.push_back(buffer_ref);
-    }
-  }
-
   void MakeEmptyBufferRequest() {
     scoped_refptr<Buffer> buffer = input_units_.front();
     input_units_.pop_front();
@@ -335,6 +335,8 @@ class OmxCodecTest : public testing::Test {
     message_loop_.RunAllPending();
   }
 
+  int input_buffer_count_;
+
   std::deque<scoped_refptr<Buffer> > input_units_;
   std::deque<scoped_refptr<VideoFrame> > output_units_;
   std::deque<OMX_BUFFERHEADERTYPE*> fill_this_buffer_received_;
@@ -369,6 +371,7 @@ TEST_F(OmxCodecTest, SimpleStartAndStop) {
                           init_done_cb_task_.CreateTask());
   message_loop_.RunAllPending();
 
+  EXPECT_EQ(kBufferCount, input_buffer_count_);
   EXPECT_EQ(VideoDecodeEngine::kNormal, omx_engine_->state());
 
   ExpectStop();
@@ -386,11 +389,9 @@ TEST_F(OmxCodecTest, NormalFlow) {
                           init_done_cb_task_.CreateTask());
   message_loop_.RunAllPending();
 
+  EXPECT_EQ(kBufferCount, input_buffer_count_);
   EXPECT_EQ(VideoDecodeEngine::kNormal, omx_engine_->state());
 
-  // Allocate bitstream buffers.
-  InitializeInputBuffers(kBufferCount);
-
   // Make emptybuffer requests.
   EXPECT_EQ(0u, output_units_.size());
   int count = output_pool_.size();
@@ -435,11 +436,9 @@ TEST_F(OmxCodecTest, RecycleInputBuffers) {
                           init_done_cb_task_.CreateTask());
   message_loop_.RunAllPending();
 
+  EXPECT_EQ(kBufferCount, input_buffer_count_);
   EXPECT_EQ(VideoDecodeEngine::kNormal, omx_engine_->state());
 
-  // Allocate bitstream buffers.
-  InitializeInputBuffers(kBufferCount);
-
   // Make emptybuffer requests, also recycle input buffers
   EXPECT_EQ(0u, output_units_.size());
   int count = output_pool_.size();
diff --git a/media/tools/omx_test/omx_test.cc b/media/tools/omx_test/omx_test.cc
index 2deddeb..2b39fee 100644
--- a/media/tools/omx_test/omx_test.cc
+++ b/media/tools/omx_test/omx_test.cc
@@ -112,7 +112,7 @@ class TestApp : public base::RefCountedThreadSafe<TestApp> {
     // There are some conditions we don't want to enqueue, for example when
     // the last buffer is an end-of-stream buffer, when we have stopped, and
     // when we have received an error.
-    bool eos = buffer->IsEndOfStream();
+    bool eos = buffer.get() && buffer->IsEndOfStream();
     if (!eos && !stopped_ && !error_)
       FeedInputBuffer();
   }
@@ -164,9 +164,6 @@ class TestApp : public base::RefCountedThreadSafe<TestApp> {
                         NewRunnableMethod(this,
                                           &TestApp::InitializeDoneCallback));
 
-    for (int i = 0; i < 20; ++i)
-      FeedInputBuffer();
-
     // Execute the message loop so that we can run tasks on it. This call
     // will return when we call message_loop_.Quit().
     message_loop_.Run();
diff --git a/media/tools/player_x11/gles_video_renderer.cc b/media/tools/player_x11/gles_video_renderer.cc
index ef02d75..4d14fee 100644
--- a/media/tools/player_x11/gles_video_renderer.cc
+++ b/media/tools/player_x11/gles_video_renderer.cc
@@ -483,7 +483,7 @@ void GlesVideoRenderer::CreateTextureAndProgramYuv2Rgb() {
   CreateShader(program, GL_VERTEX_SHADER,
                kVertexShader, sizeof(kVertexShader));
   CreateShader(program, GL_FRAGMENT_SHADER,
-               kFragmentShaderEgl, sizeof(kFragmentShader));
+               kFragmentShader, sizeof(kFragmentShader));
   LinkProgram(program);
 
   // Bind parameters.