Move the DXVA decoder off the GPU thread.

content/common/gpu/media/dxva_video_decode_accelerator.cc

 // Copyright (c) 2012 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.
 
 #include "content/common/gpu/media/dxva_video_decode_accelerator.h"
 
 #if !defined(OS_WIN)
 #error This file should only be built on Windows.
 #endif   // !defined(OS_WIN)
 
@@ skipping 470 matching lines @@
 DXVAVideoDecodeAccelerator::DXVAVideoDecodeAccelerator(
     const base::Callback<bool(void)>& make_context_current)
     : client_(NULL),
       dev_manager_reset_token_(0),
       egl_config_(NULL),
       state_(kUninitialized),
       pictures_requested_(false),
       inputs_before_decode_(0),
       make_context_current_(make_context_current),
       weak_this_factory_(this),
-      codec_(media::kUnknownVideoCodec) {
+      codec_(media::kUnknownVideoCodec),
+      decoder_thread_("DXVAVideoDecoderThread") {
   memset(&input_stream_info_, 0, sizeof(input_stream_info_));
   memset(&output_stream_info_, 0, sizeof(output_stream_info_));
 }
 
 DXVAVideoDecodeAccelerator::~DXVAVideoDecodeAccelerator() {
   client_ = NULL;
 }
 
 bool DXVAVideoDecodeAccelerator::Initialize(media::VideoCodecProfile profile,
-                                            Client* client) {
-  DCHECK(CalledOnValidThread());
+                                           Client* client) {
+  client_ = client;
 
-  client_ = client;
+  main_thread_task_runner_ = base::MessageLoop::current()->task_runner();
 
   // Not all versions of Windows 7 and later include Media Foundation DLLs.
   // Instead of crashing while delay loading the DLL when calling MFStartup()
   // below, probe whether we can successfully load the DLL now.
   //
   // See http://crbug.com/339678 for details.
   HMODULE mfplat_dll = ::LoadLibrary(L"MFPlat.dll");
   RETURN_ON_FAILURE(mfplat_dll, "MFPlat.dll is required for decoding", false);
 
   // TODO(ananta)
   // H264PROFILE_HIGH video decoding is janky at times. Needs more
   // investigation. http://crbug.com/426707
   if (profile != media::H264PROFILE_BASELINE &&
       profile != media::H264PROFILE_MAIN &&
       profile != media::VP8PROFILE_ANY &&
       profile != media::VP9PROFILE_ANY) {
     RETURN_AND_NOTIFY_ON_FAILURE(false,
         "Unsupported h.264, vp8, or vp9 profile", PLATFORM_FAILURE, false);
   }
 
   RETURN_AND_NOTIFY_ON_FAILURE(
       gfx::g_driver_egl.ext.b_EGL_ANGLE_surface_d3d_texture_2d_share_handle,
       "EGL_ANGLE_surface_d3d_texture_2d_share_handle unavailable",
       PLATFORM_FAILURE,
       false);
 
-  RETURN_AND_NOTIFY_ON_FAILURE((state_ == kUninitialized),
-      "Initialize: invalid state: " << state_, ILLEGAL_STATE, false);
+  State state = GetState();
+  RETURN_AND_NOTIFY_ON_FAILURE((state == kUninitialized),
+      "Initialize: invalid state: " << state, ILLEGAL_STATE, false);
 
   HRESULT hr = MFStartup(MF_VERSION, MFSTARTUP_FULL);
   RETURN_AND_NOTIFY_ON_HR_FAILURE(hr, "MFStartup failed.", PLATFORM_FAILURE,
       false);
 
   RETURN_AND_NOTIFY_ON_FAILURE(CreateD3DDevManager(),
                                "Failed to initialize D3D device and manager",
                                PLATFORM_FAILURE,
                                false);
 
   RETURN_AND_NOTIFY_ON_FAILURE(InitDecoder(profile),
       "Failed to initialize decoder", PLATFORM_FAILURE, false);
 
   RETURN_AND_NOTIFY_ON_FAILURE(GetStreamsInfoAndBufferReqs(),
       "Failed to get input/output stream info.", PLATFORM_FAILURE, false);
 
   RETURN_AND_NOTIFY_ON_FAILURE(
       SendMFTMessage(MFT_MESSAGE_NOTIFY_BEGIN_STREAMING, 0),
       "Send MFT_MESSAGE_NOTIFY_BEGIN_STREAMING notification failed",
       PLATFORM_FAILURE, false);
 
   RETURN_AND_NOTIFY_ON_FAILURE(
       SendMFTMessage(MFT_MESSAGE_NOTIFY_START_OF_STREAM, 0),
       "Send MFT_MESSAGE_NOTIFY_START_OF_STREAM notification failed",
       PLATFORM_FAILURE, false);
 
-  state_ = kNormal;
+  SetState(kNormal);
+
+  StartDecoderThread();
   return true;
 }
 
 void DXVAVideoDecodeAccelerator::Decode(
     const media::BitstreamBuffer& bitstream_buffer) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(main_thread_task_runner_->BelongsToCurrentThread());
 
-  RETURN_AND_NOTIFY_ON_FAILURE((state_ == kNormal || state_ == kStopped ||
-                                state_ == kFlushing),
-           "Invalid state: " << state_, ILLEGAL_STATE,);
+  State state = GetState();
+  RETURN_AND_NOTIFY_ON_FAILURE((state == kNormal || state == kStopped ||
+                                state == kFlushing),
+           "Invalid state: " << state, ILLEGAL_STATE,);
 
   base::win::ScopedComPtr<IMFSample> sample;
   sample.Attach(CreateSampleFromInputBuffer(bitstream_buffer,
                                             input_stream_info_.cbSize,
                                             input_stream_info_.cbAlignment));
   RETURN_AND_NOTIFY_ON_FAILURE(sample.get(), "Failed to create input sample",
                                PLATFORM_FAILURE, );
 
   RETURN_AND_NOTIFY_ON_HR_FAILURE(sample->SetSampleTime(bitstream_buffer.id()),
       "Failed to associate input buffer id with sample", PLATFORM_FAILURE,);
 
-  DecodeInternal(sample);
+  decoder_thread_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&DXVAVideoDecodeAccelerator::DecodeInternal,
+                 base::Unretained(this), sample));
 }
 
 void DXVAVideoDecodeAccelerator::AssignPictureBuffers(
     const std::vector<media::PictureBuffer>& buffers) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(main_thread_task_runner_->BelongsToCurrentThread());
 
-  RETURN_AND_NOTIFY_ON_FAILURE((state_ != kUninitialized),
-      "Invalid state: " << state_, ILLEGAL_STATE,);
+  State state = GetState();
+  RETURN_AND_NOTIFY_ON_FAILURE((state != kUninitialized),
+      "Invalid state: " << state, ILLEGAL_STATE,);
   RETURN_AND_NOTIFY_ON_FAILURE((kNumPictureBuffers == buffers.size()),
       "Failed to provide requested picture buffers. (Got " << buffers.size() <<
       ", requested " << kNumPictureBuffers << ")", INVALID_ARGUMENT,);
 
   // Copy the picture buffers provided by the client to the available list,
   // and mark these buffers as available for use.
   for (size_t buffer_index = 0; buffer_index < buffers.size();
        ++buffer_index) {
     linked_ptr<DXVAPictureBuffer> picture_buffer =
         DXVAPictureBuffer::Create(*this, buffers[buffer_index], egl_config_);
     RETURN_AND_NOTIFY_ON_FAILURE(picture_buffer.get(),
         "Failed to allocate picture buffer", PLATFORM_FAILURE,);
 
     bool inserted = output_picture_buffers_.insert(std::make_pair(
         buffers[buffer_index].id(), picture_buffer)).second;
     DCHECK(inserted);
   }
   ProcessPendingSamples();
-  if (state_ == kFlushing && pending_output_samples_.empty())
+
+  if (state == kFlushing)
     FlushInternal();
 }
 
 void DXVAVideoDecodeAccelerator::ReusePictureBuffer(
     int32 picture_buffer_id) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(main_thread_task_runner_->BelongsToCurrentThread());
 
-  RETURN_AND_NOTIFY_ON_FAILURE((state_ != kUninitialized),
-      "Invalid state: " << state_, ILLEGAL_STATE,);
+  State state = GetState();
+  RETURN_AND_NOTIFY_ON_FAILURE((state != kUninitialized),
+      "Invalid state: " << state, ILLEGAL_STATE,);
 
   if (output_picture_buffers_.empty() && stale_output_picture_buffers_.empty())
     return;
 
   OutputBuffers::iterator it = output_picture_buffers_.find(picture_buffer_id);
   // If we didn't find the picture id in the |output_picture_buffers_| map we
   // try the |stale_output_picture_buffers_| map, as this may have been an
   // output picture buffer from before a resolution change, that at resolution
   // change time had yet to be displayed. The client is calling us back to tell
   // us that we can now recycle this picture buffer, so if we were waiting to
   // dispose of it we now can.
   if (it == output_picture_buffers_.end()) {
     it = stale_output_picture_buffers_.find(picture_buffer_id);
     RETURN_AND_NOTIFY_ON_FAILURE(it != stale_output_picture_buffers_.end(),
         "Invalid picture id: " << picture_buffer_id, INVALID_ARGUMENT,);
-    base::MessageLoop::current()->PostTask(FROM_HERE,
+    main_thread_task_runner_->PostTask(
+        FROM_HERE,
         base::Bind(&DXVAVideoDecodeAccelerator::DeferredDismissStaleBuffer,
-            weak_this_factory_.GetWeakPtr(), picture_buffer_id));
+                   weak_this_factory_.GetWeakPtr(), picture_buffer_id));
     return;
   }
 
   it->second->ReusePictureBuffer();
   ProcessPendingSamples();
 
-  if (state_ == kFlushing && pending_output_samples_.empty())
+  if (state == kFlushing)
     FlushInternal();
 }
 
 void DXVAVideoDecodeAccelerator::Flush() {
-  DCHECK(CalledOnValidThread());
+  DCHECK(main_thread_task_runner_->BelongsToCurrentThread());
 
   DVLOG(1) << "DXVAVideoDecodeAccelerator::Flush";
 
-  RETURN_AND_NOTIFY_ON_FAILURE((state_ == kNormal || state_ == kStopped),
-      "Unexpected decoder state: " << state_, ILLEGAL_STATE,);
+  State state = GetState();
+  RETURN_AND_NOTIFY_ON_FAILURE((state == kNormal || state == kStopped),
+      "Unexpected decoder state: " << state, ILLEGAL_STATE,);
 
-  state_ = kFlushing;
+  SetState(kFlushing);
 
   RETURN_AND_NOTIFY_ON_FAILURE(SendMFTMessage(MFT_MESSAGE_COMMAND_DRAIN, 0),
       "Failed to send drain message", PLATFORM_FAILURE,);
 
-  if (!pending_output_samples_.empty())
-    return;
-
   FlushInternal();
 }
 
 void DXVAVideoDecodeAccelerator::Reset() {
-  DCHECK(CalledOnValidThread());
+  DCHECK(main_thread_task_runner_->BelongsToCurrentThread());
 
   DVLOG(1) << "DXVAVideoDecodeAccelerator::Reset";
 
-  RETURN_AND_NOTIFY_ON_FAILURE((state_ == kNormal || state_ == kStopped),
-      "Reset: invalid state: " << state_, ILLEGAL_STATE,);
+  State state = GetState();
+  RETURN_AND_NOTIFY_ON_FAILURE((state == kNormal || state == kStopped),
+      "Reset: invalid state: " << state, ILLEGAL_STATE,);
 
-  state_ = kResetting;
+  decoder_thread_.Stop();
+
+  SetState(kResetting);
 
   pending_output_samples_.clear();
 
   NotifyInputBuffersDropped();
 
   RETURN_AND_NOTIFY_ON_FAILURE(SendMFTMessage(MFT_MESSAGE_COMMAND_FLUSH, 0),
       "Reset: Failed to send message.", PLATFORM_FAILURE,);
 
-  base::MessageLoop::current()->PostTask(
+  main_thread_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&DXVAVideoDecodeAccelerator::NotifyResetDone,
                  weak_this_factory_.GetWeakPtr()));
 
-  state_ = DXVAVideoDecodeAccelerator::kNormal;
+  StartDecoderThread();
+  SetState(kNormal);
 }
 
 void DXVAVideoDecodeAccelerator::Destroy() {
-  DCHECK(CalledOnValidThread());
+  DCHECK(main_thread_task_runner_->BelongsToCurrentThread());
   Invalidate();
   delete this;
 }
 
 bool DXVAVideoDecodeAccelerator::CanDecodeOnIOThread() {
   return false;
 }
 
 bool DXVAVideoDecodeAccelerator::InitDecoder(media::VideoCodecProfile profile) {
   HMODULE decoder_dll = NULL;
@@ skipping 220 matching lines @@
           << std::hex << std::showbase << output_stream_info_.dwFlags;
   if (codec_ == media::kCodecH264) {
     CHECK_EQ(output_stream_info_.dwFlags, 0x107u);
   }
   DVLOG(1) << "Min buffer size: " << output_stream_info_.cbSize;
   DVLOG(1) << "Alignment: " << output_stream_info_.cbAlignment;
   return true;
 }
 
 void DXVAVideoDecodeAccelerator::DoDecode() {
+  DCHECK(decoder_thread_task_runner_->BelongsToCurrentThread());
   // This function is also called from FlushInternal in a loop which could
   // result in the state transitioning to kStopped due to no decoded output.
+  State state = GetState();
   RETURN_AND_NOTIFY_ON_FAILURE(
-      (state_ == kNormal || state_ == kFlushing ||
-       state_ == kStopped || state_ == kFlushingPendingInputBuffers),
+      (state == kNormal || state == kFlushing ||
+       state == kStopped || state == kFlushingPendingInputBuffers),
           "DoDecode: not in normal/flushing/stopped state", ILLEGAL_STATE,);
 
   MFT_OUTPUT_DATA_BUFFER output_data_buffer = {0};
   DWORD status = 0;
 
   HRESULT hr = decoder_->ProcessOutput(0,  // No flags
                                        1,  // # of out streams to pull from
                                        &output_data_buffer,
                                        &status);
   IMFCollection* events = output_data_buffer.pEvents;
   if (events != NULL) {
     DVLOG(1) << "Got events from ProcessOuput, but discarding";
     events->Release();
   }
   if (FAILED(hr)) {
     // A stream change needs further ProcessInput calls to get back decoder
     // output which is why we need to set the state to stopped.
     if (hr == MF_E_TRANSFORM_STREAM_CHANGE) {
       if (!SetDecoderOutputMediaType(MFVideoFormat_NV12)) {
         // Decoder didn't let us set NV12 output format. Not sure as to why
         // this can happen. Give up in disgust.
         NOTREACHED() << "Failed to set decoder output media type to NV12";
-        state_ = kStopped;
+        SetState(kStopped);
       } else {
         DVLOG(1) << "Received output format change from the decoder."
                     " Recursively invoking DoDecode";
         DoDecode();
       }
       return;
     } else if (hr == MF_E_TRANSFORM_NEED_MORE_INPUT) {
       // No more output from the decoder. Stop playback.
-      if (state_ != kFlushingPendingInputBuffers)
-        state_ = kStopped;
+      if (state != kFlushingPendingInputBuffers)
+        SetState(kStopped);
       return;
     } else {
       NOTREACHED() << "Unhandled error in DoDecode()";
       return;
     }
   }
   TRACE_EVENT_END_ETW("DXVAVideoDecodeAccelerator.Decoding", this, "");
 
   TRACE_COUNTER1("DXVA Decoding", "TotalPacketsBeforeDecode",
                  inputs_before_decode_);
@@ skipping 15 matching lines @@
   hr = MFGetService(output_buffer.get(), MR_BUFFER_SERVICE,
                     IID_PPV_ARGS(surface.Receive()));
   RETURN_ON_HR_FAILURE(hr, "Failed to get D3D surface from output sample",
                        false);
 
   LONGLONG input_buffer_id = 0;
   RETURN_ON_HR_FAILURE(sample->GetSampleTime(&input_buffer_id),
                        "Failed to get input buffer id associated with sample",
                        false);
 
+  decoder_lock_.Acquire();

[DaleCurtis, 2014/12/04 01:54:27]

It's generally preferred to use an AutoLock here with scoped {} 

[ananta, 2014/12/04 08:04:50]

Done.

   pending_output_samples_.push_back(
       PendingSampleInfo(input_buffer_id, sample));
+  decoder_lock_.Release();
 
-  // If we have available picture buffers to copy the output data then use the
-  // first one and then flag it as not being available for use.
-  if (output_picture_buffers_.size()) {
-    ProcessPendingSamples();
-    return true;
-  }
   if (pictures_requested_) {
     DVLOG(1) << "Waiting for picture slots from the client.";
+    main_thread_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&DXVAVideoDecodeAccelerator::ProcessPendingSamples,
+                   weak_this_factory_.GetWeakPtr()));
     return true;
   }
 
   // We only read the surface description, which contains its width/height when
   // we need the picture buffers from the client. Once we have those, then they
   // are reused.
   D3DSURFACE_DESC surface_desc;
   hr = surface->GetDesc(&surface_desc);
   RETURN_ON_HR_FAILURE(hr, "Failed to get surface description", false);
 
   // Go ahead and request picture buffers.
-  base::MessageLoop::current()->PostTask(
+  main_thread_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&DXVAVideoDecodeAccelerator::RequestPictureBuffers,
                  weak_this_factory_.GetWeakPtr(),
                  surface_desc.Width,
                  surface_desc.Height));
 
   pictures_requested_ = true;
   return true;
 }
 
 void DXVAVideoDecodeAccelerator::ProcessPendingSamples() {
+  DCHECK(main_thread_task_runner_->BelongsToCurrentThread());
+
+  if (!output_picture_buffers_.size())
+    return;
+
   RETURN_AND_NOTIFY_ON_FAILURE(make_context_current_.Run(),
       "Failed to make context current", PLATFORM_FAILURE,);
 
   OutputBuffers::iterator index;
 
+  base::AutoLock lock(decoder_lock_);

[DaleCurtis, 2014/12/04 01:54:27]

Notably since this section may take a while to run, you shouldn't hold the lock
except when checking if pending_output_samples is empty and when running
pop_front().

[ananta, 2014/12/04 08:04:51]

Done.

+
   for (index = output_picture_buffers_.begin();
        index != output_picture_buffers_.end() &&
        !pending_output_samples_.empty();
        ++index) {
     if (index->second->available()) {
       PendingSampleInfo sample_info = pending_output_samples_.front();
 
       base::win::ScopedComPtr<IMFMediaBuffer> output_buffer;
       HRESULT hr = sample_info.output_sample->GetBufferByIndex(
           0, output_buffer.Receive());
@@ skipping 21 matching lines @@
       }
 
       RETURN_AND_NOTIFY_ON_FAILURE(
           index->second->CopyOutputSampleDataToPictureBuffer(*this,
                                                              surface.get()),
           "Failed to copy output sample", PLATFORM_FAILURE, );
 
       media::Picture output_picture(index->second->id(),
                                     sample_info.input_buffer_id,
                                     gfx::Rect(index->second->size()));
-      base::MessageLoop::current()->PostTask(
+      main_thread_task_runner_->PostTask(
           FROM_HERE,
           base::Bind(&DXVAVideoDecodeAccelerator::NotifyPictureReady,
                      weak_this_factory_.GetWeakPtr(),
                      output_picture));
 
       index->second->set_available(false);
       pending_output_samples_.pop_front();
     }
   }
-
-  if (!pending_input_buffers_.empty() && pending_output_samples_.empty()) {
-    base::MessageLoop::current()->PostTask(
+  if (GetState() == kFlushing) {
+    decoder_thread_task_runner_->PostTask(

[DaleCurtis, 2014/12/04 01:54:27]

Seems you can just early return after this PostTask? FlushInternal will run
DecodePendingInputBuffers() if necessary.

[ananta, 2014/12/04 08:04:51]

The state transitions in Flush are a touch complicated at the moment. Returning
early here causes problems in Flush completing early leading to issues with
hangouts not working correctly. I will look into this in the next patch.

         FROM_HERE,
-        base::Bind(&DXVAVideoDecodeAccelerator::DecodePendingInputBuffers,
-                   weak_this_factory_.GetWeakPtr()));
+        base::Bind(&DXVAVideoDecodeAccelerator::FlushInternal,
+                   base::Unretained(this)));
   }
+  decoder_thread_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&DXVAVideoDecodeAccelerator::DecodePendingInputBuffers,
+                 base::Unretained(this)));
 }
 
 void DXVAVideoDecodeAccelerator::StopOnError(
   media::VideoDecodeAccelerator::Error error) {
-  DCHECK(CalledOnValidThread());
+  if (!main_thread_task_runner_->BelongsToCurrentThread()) {
+    main_thread_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&DXVAVideoDecodeAccelerator::StopOnError,
+                   weak_this_factory_.GetWeakPtr(),
+                   error));
+    return;
+  }
 
   if (client_)
     client_->NotifyError(error);
   client_ = NULL;
 
-  if (state_ != kUninitialized) {
+  if (GetState() != kUninitialized) {
     Invalidate();
   }
 }
 
 void DXVAVideoDecodeAccelerator::Invalidate() {
-  if (state_ == kUninitialized)
+  if (GetState() == kUninitialized)
     return;
+  decoder_thread_.Stop();
   weak_this_factory_.InvalidateWeakPtrs();
   output_picture_buffers_.clear();
   stale_output_picture_buffers_.clear();
   pending_output_samples_.clear();
   pending_input_buffers_.clear();
   decoder_.Release();
   MFShutdown();
-  state_ = kUninitialized;
+  SetState(kUninitialized);
 }
 
 void DXVAVideoDecodeAccelerator::NotifyInputBufferRead(int input_buffer_id) {
+  DCHECK(main_thread_task_runner_->BelongsToCurrentThread());
   if (client_)
     client_->NotifyEndOfBitstreamBuffer(input_buffer_id);
 }
 
 void DXVAVideoDecodeAccelerator::NotifyFlushDone() {
-  if (client_)
+  DCHECK(main_thread_task_runner_->BelongsToCurrentThread());
+  if (client_ && GetState() == kStopped)
     client_->NotifyFlushDone();
 }
 
 void DXVAVideoDecodeAccelerator::NotifyResetDone() {
+  DCHECK(main_thread_task_runner_->BelongsToCurrentThread());
   if (client_)
     client_->NotifyResetDone();
 }
 
 void DXVAVideoDecodeAccelerator::RequestPictureBuffers(int width, int height) {
+  DCHECK(main_thread_task_runner_->BelongsToCurrentThread());
   // This task could execute after the decoder has been torn down.
-  if (state_ != kUninitialized && client_) {
+  if (GetState() != kUninitialized && client_) {
     client_->ProvidePictureBuffers(
         kNumPictureBuffers,
         gfx::Size(width, height),
         GL_TEXTURE_2D);
   }
 }
 
 void DXVAVideoDecodeAccelerator::NotifyPictureReady(
     const media::Picture& picture) {
+  DCHECK(main_thread_task_runner_->BelongsToCurrentThread());
   // This task could execute after the decoder has been torn down.
-  if (state_ != kUninitialized && client_)
+  if (GetState() != kUninitialized && client_)
     client_->PictureReady(picture);
 }
 
 void DXVAVideoDecodeAccelerator::NotifyInputBuffersDropped() {
-  if (!client_ || !pending_output_samples_.empty())
+  DCHECK(main_thread_task_runner_->BelongsToCurrentThread());
+  if (!client_)
     return;
 
   for (PendingInputs::iterator it = pending_input_buffers_.begin();
        it != pending_input_buffers_.end(); ++it) {
     LONGLONG input_buffer_id = 0;
     RETURN_ON_HR_FAILURE((*it)->GetSampleTime(&input_buffer_id),
                          "Failed to get buffer id associated with sample",);
     client_->NotifyEndOfBitstreamBuffer(input_buffer_id);
   }
   pending_input_buffers_.clear();
 }
 
 void DXVAVideoDecodeAccelerator::DecodePendingInputBuffers() {
-  RETURN_AND_NOTIFY_ON_FAILURE((state_ != kUninitialized),
-      "Invalid state: " << state_, ILLEGAL_STATE,);
+  DCHECK(decoder_thread_task_runner_->BelongsToCurrentThread());
+  State state = GetState();
+  RETURN_AND_NOTIFY_ON_FAILURE((state != kUninitialized),
+      "Invalid state: " << state, ILLEGAL_STATE,);
 
-  if (pending_input_buffers_.empty() || !pending_output_samples_.empty())
+  if (pending_input_buffers_.empty() || OutputSamplesPresent())

[DaleCurtis, 2014/12/04 01:54:27]

Is it necessary to check OutputSamplesPresent() here? Won't it just end up
queuing a few more input buffers for decoding which will end up blocked until
picture buffers are available? It seems if you remove this restriction you could
remove OutputSamplesPresent() everywhere.

[ananta, 2014/12/04 08:04:51]

Yes. The Media foundation decoder has a pool of surfaces it uses for decoding.
If we exhaust that pool it overwrites the frames. We impose a restriction that
we can have only one pending output frame at any given point.

     return;
 
   PendingInputs pending_input_buffers_copy;
   std::swap(pending_input_buffers_, pending_input_buffers_copy);
 
   for (PendingInputs::iterator it = pending_input_buffers_copy.begin();
        it != pending_input_buffers_copy.end(); ++it) {
     DecodeInternal(*it);
   }
 
-  if (state_ != kFlushingPendingInputBuffers)
+  if (state != kFlushingPendingInputBuffers)
     return;
 
   // If we are scheduled during a flush operation then mark the flush as

[DaleCurtis, 2014/12/04 19:21:34]

Does the bug this resolved no longer happen?

[ananta, 2014/12/04 19:48:55]

This code is not needed with ProcessPendingSamples posting the FlushInternal
task. The bug with hangouts does not happen anymore. I verified that

   // complete if we have no pending input and pending output frames.
   // If we don't have available output slots then this function will be
   // scheduled again by the ProcessPendingSamples function once slots become
   // available.
-  if (pending_input_buffers_.empty() && pending_output_samples_.empty()) {
-    state_ = kNormal;
-    base::MessageLoop::current()->PostTask(
+  if (pending_input_buffers_.empty() && !OutputSamplesPresent()) {
+    SetState(kNormal);
+    main_thread_task_runner_->PostTask(
         FROM_HERE,
         base::Bind(&DXVAVideoDecodeAccelerator::NotifyFlushDone,
-                    weak_this_factory_.GetWeakPtr()));
+                   weak_this_factory_.GetWeakPtr()));
   }
 }
 
 void DXVAVideoDecodeAccelerator::FlushInternal() {
+  if (!decoder_thread_task_runner_->BelongsToCurrentThread()) {

[DaleCurtis, 2014/12/04 01:54:27]

There's only a couple callers of this, so it'd be more readable to avoid the
trampoline and have the callers explicitly PostTask.

[ananta, 2014/12/04 08:04:51]

Done.

+    decoder_thread_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&DXVAVideoDecodeAccelerator::FlushInternal,
+                   base::Unretained(this)));
+    return;
+  }
+
+  // We allow only one output frame to be present at any given time. If we have
+  // an output frame, then we cannot complete the flush at this time.
+  if (OutputSamplesPresent())
+    return;
+
   // The DoDecode function sets the state to kStopped when the decoder returns
   // MF_E_TRANSFORM_NEED_MORE_INPUT.
   // The MFT decoder can buffer upto 30 frames worth of input before returning
   // an output frame. This loop here attempts to retrieve as many output frames
   // as possible from the buffered set.
-  while (state_ != kStopped) {
+  while (GetState() != kStopped) {
     DoDecode();
-    if (!pending_output_samples_.empty())
+    if (OutputSamplesPresent())
       return;
   }
 
   // TODO(ananta)
   // Look into whether we can simplify this function by combining the while
   // above and the code below into a single block which achieves both. The Flush
   // transitions in the decoder are a touch intertwined with other portions of
   // the code like AssignPictureBuffers, ReusePictureBuffers etc.
   if (!pending_input_buffers_.empty()) {
-    state_ = kFlushingPendingInputBuffers;
-    base::MessageLoop::current()->PostTask(
+    SetState(kFlushingPendingInputBuffers);
+    decoder_thread_task_runner_->PostTask(
         FROM_HERE,
         base::Bind(&DXVAVideoDecodeAccelerator::DecodePendingInputBuffers,
-                    weak_this_factory_.GetWeakPtr()));
+                   base::Unretained(this)));
     return;
   }
 
-  base::MessageLoop::current()->PostTask(
+  main_thread_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&DXVAVideoDecodeAccelerator::NotifyFlushDone,
                  weak_this_factory_.GetWeakPtr()));
 
-  state_ = kNormal;
+  SetState(kNormal);
 }
 
 void DXVAVideoDecodeAccelerator::DecodeInternal(
     const base::win::ScopedComPtr<IMFSample>& sample) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(decoder_thread_task_runner_->BelongsToCurrentThread());
 
-  if (state_ == kUninitialized)
+  if (GetState() == kUninitialized)
     return;
 
-  if (!pending_output_samples_.empty() || !pending_input_buffers_.empty()) {
+  if (OutputSamplesPresent() || !pending_input_buffers_.empty()) {
     pending_input_buffers_.push_back(sample);
     return;
   }
 
   if (!inputs_before_decode_) {
     TRACE_EVENT_BEGIN_ETW("DXVAVideoDecodeAccelerator.Decoding", this, "");
   }
   inputs_before_decode_++;
 
   HRESULT hr = decoder_->ProcessInput(0, sample.get(), 0);
   // As per msdn if the decoder returns MF_E_NOTACCEPTING then it means that it
   // has enough data to produce one or more output samples. In this case the
   // recommended options are to
   // 1. Generate new output by calling IMFTransform::ProcessOutput until it
   //    returns MF_E_TRANSFORM_NEED_MORE_INPUT.
   // 2. Flush the input data
   // We implement the first option, i.e to retrieve the output sample and then
   // process the input again. Failure in either of these steps is treated as a
   // decoder failure.
   if (hr == MF_E_NOTACCEPTING) {
     DoDecode();
-    RETURN_AND_NOTIFY_ON_FAILURE((state_ == kStopped || state_ == kNormal),
-        "Failed to process output. Unexpected decoder state: " << state_,
+    State state = GetState();
+    RETURN_AND_NOTIFY_ON_FAILURE((state == kStopped || state == kNormal ||
+                                  state == kFlushing),
+        "Failed to process output. Unexpected decoder state: " << state,
         PLATFORM_FAILURE,);
     hr = decoder_->ProcessInput(0, sample.get(), 0);
     // If we continue to get the MF_E_NOTACCEPTING error we do the following:-
     // 1. Add the input sample to the pending queue.
     // 2. If we don't have any output samples we post the
     //    DecodePendingInputBuffers task to process the pending input samples.
     //    If we have an output sample then the above task is posted when the
     //    output samples are sent to the client.
     // This is because we only support 1 pending output sample at any
     // given time due to the limitation with the Microsoft media foundation
     // decoder where it recycles the output Decoder surfaces.
     if (hr == MF_E_NOTACCEPTING) {
       pending_input_buffers_.push_back(sample);
-      if (pending_output_samples_.empty()) {
-        base::MessageLoop::current()->PostTask(
+      if (!OutputSamplesPresent()) {

[DaleCurtis, 2014/12/04 01:54:27]

Why not just always call this? GpuVideoDecoder itself will rate limit, no?

[ananta, 2014/12/04 08:04:50]

Done.

+        decoder_thread_task_runner_->PostTask(
             FROM_HERE,
             base::Bind(&DXVAVideoDecodeAccelerator::DecodePendingInputBuffers,
-                       weak_this_factory_.GetWeakPtr()));
+                       base::Unretained(this)));
       }
       return;
     }
   }
   RETURN_AND_NOTIFY_ON_HR_FAILURE(hr, "Failed to process input sample",
       PLATFORM_FAILURE,);
 
   DoDecode();
 
-  RETURN_AND_NOTIFY_ON_FAILURE((state_ == kStopped || state_ == kNormal ||
-                                state_ == kFlushingPendingInputBuffers),
-      "Failed to process output. Unexpected decoder state: " << state_,
+  State state = GetState();
+  RETURN_AND_NOTIFY_ON_FAILURE((state == kStopped || state == kNormal ||
+                                state == kFlushingPendingInputBuffers ||
+                                state == kFlushing),
+      "Failed to process output. Unexpected decoder state: " << state,
       ILLEGAL_STATE,);
 
   LONGLONG input_buffer_id = 0;
   RETURN_ON_HR_FAILURE(sample->GetSampleTime(&input_buffer_id),
                        "Failed to get input buffer id associated with sample",);
   // The Microsoft Media foundation decoder internally buffers up to 30 frames
   // before returning a decoded frame. We need to inform the client that this
   // input buffer is processed as it may stop sending us further input.
   // Note: This may break clients which expect every input buffer to be
   // associated with a decoded output buffer.
   // TODO(ananta)
   // Do some more investigation into whether it is possible to get the MFT
   // decoder to emit an output packet for every input packet.
   // http://code.google.com/p/chromium/issues/detail?id=108121
   // http://code.google.com/p/chromium/issues/detail?id=150925
-  base::MessageLoop::current()->PostTask(
+  main_thread_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&DXVAVideoDecodeAccelerator::NotifyInputBufferRead,
                  weak_this_factory_.GetWeakPtr(),
                  input_buffer_id));
 }
 
 void DXVAVideoDecodeAccelerator::HandleResolutionChanged(int width,
                                                          int height) {
-  base::MessageLoop::current()->PostTask(
+  main_thread_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&DXVAVideoDecodeAccelerator::DismissStaleBuffers,
                  weak_this_factory_.GetWeakPtr()));
 
-  base::MessageLoop::current()->PostTask(
+  main_thread_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&DXVAVideoDecodeAccelerator::RequestPictureBuffers,
                  weak_this_factory_.GetWeakPtr(),
                  width,
                  height));
 }
 
 void DXVAVideoDecodeAccelerator::DismissStaleBuffers() {
   OutputBuffers::iterator index;
 
@@ skipping 16 matching lines @@
 void DXVAVideoDecodeAccelerator::DeferredDismissStaleBuffer(
     int32 picture_buffer_id) {
   OutputBuffers::iterator it = stale_output_picture_buffers_.find(
       picture_buffer_id);
   DCHECK(it != stale_output_picture_buffers_.end());
   DVLOG(1) << "Dismissing picture id: " << it->second->id();
   client_->DismissPictureBuffer(it->second->id());
   stale_output_picture_buffers_.erase(it);
 }
 
+DXVAVideoDecodeAccelerator::State
+DXVAVideoDecodeAccelerator::GetState() const {
+  State state = kUninitialized;
+  ::InterlockedExchange(reinterpret_cast<long*>(&state),
+                        state_);
+  return state;
+}
+
+void DXVAVideoDecodeAccelerator::SetState(State new_state) {
+  if (!main_thread_task_runner_->BelongsToCurrentThread()) {
+    main_thread_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&DXVAVideoDecodeAccelerator::SetState,
+                   weak_this_factory_.GetWeakPtr(),
+                   new_state));
+    return;
+  }
+  ::InterlockedCompareExchange(reinterpret_cast<long*>(&state_),

[DaleCurtis, 2014/12/04 01:54:27]

Formatting is wrong. Also you should at least DCHECK() that this succeeds.

[ananta, 2014/12/04 08:04:51]

Done.

+      new_state, state_);
+}
+
+void DXVAVideoDecodeAccelerator::StartDecoderThread() {
+  decoder_thread_.init_com_with_mta(false);
+  decoder_thread_.Start();
+  decoder_thread_task_runner_ = decoder_thread_.task_runner();
+}
+
+bool DXVAVideoDecodeAccelerator::OutputSamplesPresent() {
+  base::AutoLock lock(decoder_lock_);
+  return !pending_output_samples_.empty();
+}
+
 }  // namespace content