Remove reference counting from media::VideoDecoder and friends.

media/filters/gpu_video_decoder.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 "media/filters/gpu_video_decoder.h"
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/cpu.h"
 #include "base/message_loop.h"
 #include "base/stl_util.h"
+#include "base/synchronization/waitable_event.h"
 #include "media/base/bind_to_loop.h"
 #include "media/base/decoder_buffer.h"
 #include "media/base/demuxer_stream.h"
 #include "media/base/pipeline.h"
 #include "media/base/pipeline_status.h"
 #include "media/base/video_decoder_config.h"
 
 namespace media {
 
 // Maximum number of concurrent VDA::Decode() operations GVD will maintain.
@@ skipping 25 matching lines @@
     : bitstream_buffer_id(bbid), timestamp(ts), visible_rect(vr),
       natural_size(ns) {
 }
 
 GpuVideoDecoder::BufferData::~BufferData() {}
 
 GpuVideoDecoder::GpuVideoDecoder(
     const scoped_refptr<base::MessageLoopProxy>& message_loop,
     const scoped_refptr<Factories>& factories)
     : gvd_loop_proxy_(message_loop),
+      weak_factory_(this),
       vda_loop_proxy_(factories->GetMessageLoop()),
       factories_(factories),
       state_(kNormal),
       demuxer_read_in_progress_(false),
       decoder_texture_target_(0),
       next_picture_buffer_id_(0),
       next_bitstream_buffer_id_(0),
       error_occured_(false),
       available_pictures_(-1) {
   DCHECK(factories_);
 }
 
 void GpuVideoDecoder::Reset(const base::Closure& closure)  {
   DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
 
   if (state_ == kDrainingDecoder) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::Reset, this, closure));
+        &GpuVideoDecoder::Reset, weak_this_, closure));
     // NOTE: if we're deferring Reset() until a Flush() completes, return
     // queued pictures to the VDA so they can be used to finish that Flush().
     if (pending_read_cb_.is_null())
       ready_video_frames_.clear();
     return;
   }
 
   // Throw away any already-decoded, not-yet-delivered frames.
   ready_video_frames_.clear();
 
@@ skipping 18 matching lines @@
   DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
   if (vda_.get())
     DestroyVDA();
   BindToCurrentLoop(closure).Run();
 }
 
 void GpuVideoDecoder::Initialize(const scoped_refptr<DemuxerStream>& stream,
                                  const PipelineStatusCB& orig_status_cb,
                                  const StatisticsCB& statistics_cb) {
   DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
+  weak_this_ = weak_factory_.GetWeakPtr();
+
   PipelineStatusCB status_cb = CreateUMAReportingPipelineCB(
       "Media.GpuVideoDecoderInitializeStatus",
       BindToCurrentLoop(orig_status_cb));
   DCHECK(!demuxer_stream_);
 
   if (!stream) {
     status_cb.Run(PIPELINE_ERROR_DECODE);
     return;
   }
 
@@ skipping 33 matching lines @@
 
   if (config.codec() == kCodecH264)
     stream->EnableBitstreamConverter();
 
   demuxer_stream_ = stream;
   statistics_cb_ = statistics_cb;
 
   DVLOG(1) << "GpuVideoDecoder::Initialize() succeeded.";
   vda_loop_proxy_->PostTaskAndReply(
       FROM_HERE,
-      base::Bind(&GpuVideoDecoder::SetVDA, this, vda),
+      base::Bind(&GpuVideoDecoder::SetVDA, weak_this_, vda),
       base::Bind(status_cb, PIPELINE_OK));
 }
 
 void GpuVideoDecoder::SetVDA(VideoDecodeAccelerator* vda) {
   DCHECK(vda_loop_proxy_->BelongsToCurrentThread());
   DCHECK(!vda_.get());
   vda_.reset(vda);
   weak_vda_ = vda->AsWeakPtr();
 }
 
 void GpuVideoDecoder::DestroyTextures() {
   for (std::map<int32, PictureBuffer>::iterator it =
           picture_buffers_in_decoder_.begin();
           it != picture_buffers_in_decoder_.end(); ++it) {
     factories_->DeleteTexture(it->second.texture_id());
   }
   picture_buffers_in_decoder_.clear();
 }
 
+static void DestroyAndSignal(
+    base::WeakPtr<VideoDecodeAccelerator> weak_vda,
+    base::WaitableEvent* event) {
+  // XXX: how else would weak_vda be null?

[scherkus (not reviewing), 2013/03/22 00:04:20]

FYI

[xhwang, 2013/03/22 01:00:32]

Defer this part to fischman :)

[Ami GONE FROM CHROMIUM, 2013/03/22 23:48:01]

VDA unfortunately SupportsWeakPtr, so others can invalidate its weakptrs, and
you might not know about it.
E.g. if an error is signaled from the GPU process the VDA (GVDAH) in the
renderer may be released without telling you.
(I think?)

+  if (weak_vda)
+    weak_vda->Destroy();
+  event->Signal();
+}
+
 void GpuVideoDecoder::DestroyVDA() {
   DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
   VideoDecodeAccelerator* vda ALLOW_UNUSED = vda_.release();
-  // Tricky: |this| needs to stay alive until after VDA::Destroy is actually
-  // called, not just posted, so we take an artificial ref to |this| and release
-  // it as |reply| after VDA::Destroy() returns.
-  AddRef();
-  vda_loop_proxy_->PostTaskAndReply(
-      FROM_HERE,
-      base::Bind(&VideoDecodeAccelerator::Destroy, weak_vda_),
-      base::Bind(&GpuVideoDecoder::Release, this));
+
+  // Wait until VideoDecodeAccelerator::Destroy() completes.

[scherkus (not reviewing), 2013/03/22 00:04:20]

fischman:
http://blog.operationreality.org/wp-content/uploads/2012/03/chemistry-dog-no-...

+  base::WaitableEvent event(false, false);
+  vda_loop_proxy_->PostTask(FROM_HERE, base::Bind(
+      &DestroyAndSignal, weak_vda_, &event));
+  event.Wait();

[Ami GONE FROM CHROMIUM, 2013/03/22 23:48:01]

This looks like a deadlock: Stop() calls this function, so the render thread is
blocked waiting for this to return, so DestroyAndSignal won't get a chance to
run.

 
   DestroyTextures();
 }
 
 void GpuVideoDecoder::Read(const ReadCB& read_cb) {
   DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
   DCHECK(pending_reset_cb_.is_null());
   DCHECK(pending_read_cb_.is_null());
   pending_read_cb_ = BindToCurrentLoop(read_cb);
 
@@ skipping 31 matching lines @@
   return bitstream_buffers_in_decoder_.size() < kMaxInFlightDecodes;
 }
 
 void GpuVideoDecoder::RequestBufferDecode(
     DemuxerStream::Status status,
     const scoped_refptr<DecoderBuffer>& buffer) {
   DCHECK_EQ(status != DemuxerStream::kOk, !buffer) << status;
 
   if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::RequestBufferDecode, this, status, buffer));
+        &GpuVideoDecoder::RequestBufferDecode, weak_this_, status, buffer));
     return;
   }
   demuxer_read_in_progress_ = false;
 
   if (status != DemuxerStream::kOk) {
     if (pending_read_cb_.is_null())
       return;
 
     // TODO(acolwell): Add support for reinitializing the decoder when
     // |status| == kConfigChanged. For now we just trigger a decode error.
@@ skipping 85 matching lines @@
 
 void GpuVideoDecoder::NotifyInitializeDone() {
   NOTREACHED() << "GpuVideoDecodeAcceleratorHost::Initialize is synchronous!";
 }
 
 void GpuVideoDecoder::ProvidePictureBuffers(uint32 count,
                                             const gfx::Size& size,
                                             uint32 texture_target) {
   if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::ProvidePictureBuffers, this, count, size,
+        &GpuVideoDecoder::ProvidePictureBuffers, weak_this_, count, size,
         texture_target));
     return;
   }
 
   std::vector<uint32> texture_ids;
   decoder_texture_target_ = texture_target;
   if (!factories_->CreateTextures(
       count, size, &texture_ids, decoder_texture_target_)) {
     NotifyError(VideoDecodeAccelerator::PLATFORM_FAILURE);
     return;
@@ skipping 14 matching lines @@
     DCHECK(inserted);
   }
   vda_loop_proxy_->PostTask(FROM_HERE, base::Bind(
       &VideoDecodeAccelerator::AssignPictureBuffers, weak_vda_,
       picture_buffers));
 }
 
 void GpuVideoDecoder::DismissPictureBuffer(int32 id) {
   if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::DismissPictureBuffer, this, id));
+        &GpuVideoDecoder::DismissPictureBuffer, weak_this_, id));
     return;
   }
   std::map<int32, PictureBuffer>::iterator it =
       picture_buffers_in_decoder_.find(id);
   if (it == picture_buffers_in_decoder_.end()) {
     NOTREACHED() << "Missing picture buffer: " << id;
     return;
   }
   factories_->DeleteTexture(it->second.texture_id());
   picture_buffers_in_decoder_.erase(it);
 }
 
 void GpuVideoDecoder::PictureReady(const media::Picture& picture) {
   if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::PictureReady, this, picture));
+        &GpuVideoDecoder::PictureReady, weak_this_, picture));
     return;
   }
   std::map<int32, PictureBuffer>::iterator it =
       picture_buffers_in_decoder_.find(picture.picture_buffer_id());
   if (it == picture_buffers_in_decoder_.end()) {
     NOTREACHED() << "Missing picture buffer: " << picture.picture_buffer_id();
     NotifyError(VideoDecodeAccelerator::PLATFORM_FAILURE);
     return;
   }
   const PictureBuffer& pb = it->second;
 
   // Update frame's timestamp.
   base::TimeDelta timestamp;
   gfx::Rect visible_rect;
   gfx::Size natural_size;
   GetBufferData(picture.bitstream_buffer_id(), &timestamp, &visible_rect,
                 &natural_size);
   DCHECK(decoder_texture_target_);
   scoped_refptr<VideoFrame> frame(
       VideoFrame::WrapNativeTexture(
           pb.texture_id(), decoder_texture_target_, pb.size(), visible_rect,
           natural_size, timestamp,
           base::Bind(&Factories::ReadPixels, factories_, pb.texture_id(),
                      decoder_texture_target_,
                      gfx::Size(visible_rect.width(), visible_rect.height())),
-          base::Bind(&GpuVideoDecoder::ReusePictureBuffer, this,
+          base::Bind(&GpuVideoDecoder::ReusePictureBuffer, weak_this_,
                      picture.picture_buffer_id())));
   CHECK_GT(available_pictures_, 0);
   available_pictures_--;
 
   EnqueueFrameAndTriggerFrameDelivery(frame);
 }
 
 void GpuVideoDecoder::EnqueueFrameAndTriggerFrameDelivery(
     const scoped_refptr<VideoFrame>& frame) {
   DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
@@ skipping 11 matching lines @@
   if (pending_read_cb_.is_null())
     return;
 
   base::ResetAndReturn(&pending_read_cb_).Run(kOk, ready_video_frames_.front());
   ready_video_frames_.pop_front();
 }
 
 void GpuVideoDecoder::ReusePictureBuffer(int64 picture_buffer_id) {
   if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::ReusePictureBuffer, this, picture_buffer_id));
+        &GpuVideoDecoder::ReusePictureBuffer, weak_this_, picture_buffer_id));
     return;
   }
   CHECK_GE(available_pictures_, 0);
   available_pictures_++;
 
   if (!vda_.get())
     return;
   vda_loop_proxy_->PostTask(FROM_HERE, base::Bind(
       &VideoDecodeAccelerator::ReusePictureBuffer, weak_vda_,
       picture_buffer_id));
@@ skipping 14 matching lines @@
 }
 
 void GpuVideoDecoder::PutSHM(SHMBuffer* shm_buffer) {
   DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
   available_shm_segments_.push_back(shm_buffer);
 }
 
 void GpuVideoDecoder::NotifyEndOfBitstreamBuffer(int32 id) {
   if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::NotifyEndOfBitstreamBuffer, this, id));
+        &GpuVideoDecoder::NotifyEndOfBitstreamBuffer, weak_this_, id));
     return;
   }
 
   std::map<int32, BufferPair>::iterator it =
       bitstream_buffers_in_decoder_.find(id);
   if (it == bitstream_buffers_in_decoder_.end()) {
     NotifyError(VideoDecodeAccelerator::PLATFORM_FAILURE);
     NOTREACHED() << "Missing bitstream buffer: " << id;
     return;
   }
@@ skipping 31 matching lines @@
   DestroyTextures();
 }
 
 void GpuVideoDecoder::EnsureDemuxOrDecode() {
   DCHECK(gvd_loop_proxy_->BelongsToCurrentThread());
   if (demuxer_read_in_progress_)
     return;
   demuxer_read_in_progress_ = true;
   gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
       &DemuxerStream::Read, demuxer_stream_.get(),
-      base::Bind(&GpuVideoDecoder::RequestBufferDecode, this)));
+      base::Bind(&GpuVideoDecoder::RequestBufferDecode, weak_this_)));
 }
 
 void GpuVideoDecoder::NotifyFlushDone() {
   if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::NotifyFlushDone, this));
+        &GpuVideoDecoder::NotifyFlushDone, weak_this_));
     return;
   }
   DCHECK_EQ(state_, kDrainingDecoder);
   state_ = kDecoderDrained;
   EnqueueFrameAndTriggerFrameDelivery(VideoFrame::CreateEmptyFrame());
 }
 
 void GpuVideoDecoder::NotifyResetDone() {
   if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::NotifyResetDone, this));
+        &GpuVideoDecoder::NotifyResetDone, weak_this_));
     return;
   }
 
   if (!vda_.get())
     return;
 
   DCHECK(ready_video_frames_.empty());
 
   // This needs to happen after the Reset() on vda_ is done to ensure pictures
   // delivered during the reset can find their time data.
   input_buffer_data_.clear();
 
   if (!pending_reset_cb_.is_null())
     base::ResetAndReturn(&pending_reset_cb_).Run();
 
   if (!pending_read_cb_.is_null())
     EnqueueFrameAndTriggerFrameDelivery(VideoFrame::CreateEmptyFrame());
 }
 
 void GpuVideoDecoder::NotifyError(media::VideoDecodeAccelerator::Error error) {
   if (!gvd_loop_proxy_->BelongsToCurrentThread()) {
     gvd_loop_proxy_->PostTask(FROM_HERE, base::Bind(
-        &GpuVideoDecoder::NotifyError, this, error));
+        &GpuVideoDecoder::NotifyError, weak_this_, error));
     return;
   }
   if (!vda_.get())
     return;
 
   DLOG(ERROR) << "VDA Error: " << error;
   DestroyVDA();
 
   error_occured_ = true;
 
   if (!pending_read_cb_.is_null()) {
     base::ResetAndReturn(&pending_read_cb_).Run(kDecodeError, NULL);
     return;
   }
 }
 
 }  // namespace media