Remove reference counting from media::DemuxerStream and friends.

media/filters/video_frame_stream.cc

 // Copyright (c) 2013 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/video_frame_stream.h"
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/message_loop_proxy.h"
 #include "media/base/bind_to_loop.h"
 #include "media/base/demuxer_stream.h"
 #include "media/base/video_decoder_config.h"
 #include "media/filters/decrypting_demuxer_stream.h"
 #include "media/filters/video_decoder_selector.h"
 
 namespace media {
 
 VideoFrameStream::VideoFrameStream(
     const scoped_refptr<base::MessageLoopProxy>& message_loop,
     ScopedVector<VideoDecoder> decoders,
     const SetDecryptorReadyCB& set_decryptor_ready_cb)
     : message_loop_(message_loop),
       weak_factory_(this),
       state_(UNINITIALIZED),
-      decoders_(decoders.Pass()),
-      set_decryptor_ready_cb_(set_decryptor_ready_cb) {
+      stream_(NULL),
+      decoder_selector_(new VideoDecoderSelector(
+          message_loop, decoders.Pass(), set_decryptor_ready_cb)) {
 }
 
 VideoFrameStream::~VideoFrameStream() {
   DCHECK(state_ == UNINITIALIZED || state_ == STOPPED) << state_;
 }
 
-void VideoFrameStream::Initialize(const scoped_refptr<DemuxerStream>& stream,
+void VideoFrameStream::Initialize(DemuxerStream* stream,
                                   const StatisticsCB& statistics_cb,
                                   const InitCB& init_cb) {
   DCHECK(message_loop_->BelongsToCurrentThread());
   DCHECK_EQ(state_, UNINITIALIZED);
 
   weak_this_ = weak_factory_.GetWeakPtr();
 
   DCHECK(init_cb_.is_null());
   DCHECK(!init_cb.is_null());
   init_cb_ = init_cb;
   stream_ = stream;
 
-  // TODO(scherkus): Make |decoder_selector| a member variable after
-  // DemuxerStream is no longer refcounted. Today we're forced to do this
-  // because it creates a refcount loop between |this| and |decoder_selector|.
-  scoped_ptr<VideoDecoderSelector> decoder_selector(new VideoDecoderSelector(
-      message_loop_, decoders_.Pass(), set_decryptor_ready_cb_));
-  set_decryptor_ready_cb_.Reset();
-
-  VideoDecoderSelector* decoder_selector_ptr = decoder_selector.get();
-
-  decoder_selector_ptr->SelectVideoDecoder(this, statistics_cb, base::Bind(
-      &VideoFrameStream::OnDecoderSelected, weak_this_,
-      base::Passed(&decoder_selector)));
+  decoder_selector_->SelectVideoDecoder(this, statistics_cb, base::Bind(
+      &VideoFrameStream::OnDecoderSelected, weak_this_));
 }
 
 void VideoFrameStream::ReadFrame(const VideoDecoder::ReadCB& read_cb) {
   DCHECK(message_loop_->BelongsToCurrentThread());
   DCHECK_EQ(state_, NORMAL);
   // No two reads in the flight at any time.
   DCHECK(read_cb_.is_null());
   // No read during resetting or stopping process.
   DCHECK(reset_cb_.is_null());
   DCHECK(stop_cb_.is_null());
@@ skipping 45 matching lines @@
         &VideoFrameStream::StopDecoder, weak_this_));
     return;
   }
 
   if (decoder_) {
     StopDecoder();
     return;
   }
 
   state_ = STOPPED;
-  // Break the ref-count loop so we don't leak objects.
-  // TODO(scherkus): Make DemuxerStream and/or VideoDecoder not ref-counted so
-  // we don't need this here. See: http://crbug.com/173313
-  stream_ = NULL;

[acolwell GONE FROM CHROMIUM, 2013/04/23 03:20:42]

nit: Any harm in keeping this?

[scherkus (not reviewing), 2013/04/23 18:43:40]

Done.

-  decrypting_demuxer_stream_ = NULL;
   decoder_.reset();
+  decrypting_demuxer_stream_.reset();
   message_loop_->PostTask(FROM_HERE, base::ResetAndReturn(&stop_cb_));
 }
 
 bool VideoFrameStream::HasOutputFrameAvailable() const {
   DCHECK(message_loop_->BelongsToCurrentThread());
   return decoder_->HasOutputFrameAvailable();
 }
 
 void VideoFrameStream::Read(const ReadCB& read_cb) {
   DCHECK(message_loop_->BelongsToCurrentThread());
@@ skipping 15 matching lines @@
   DCHECK(message_loop_->BelongsToCurrentThread());
   return VIDEO;
 }
 
 void VideoFrameStream::EnableBitstreamConverter() {
   DCHECK(message_loop_->BelongsToCurrentThread());
   stream_->EnableBitstreamConverter();
 }
 
 void VideoFrameStream::OnDecoderSelected(
-    scoped_ptr<VideoDecoderSelector> decoder_selector,
     scoped_ptr<VideoDecoder> selected_decoder,
-    const scoped_refptr<DecryptingDemuxerStream>& decrypting_demuxer_stream) {
+    scoped_ptr<DecryptingDemuxerStream> decrypting_demuxer_stream) {
   DCHECK(message_loop_->BelongsToCurrentThread());
   DCHECK_EQ(state_, UNINITIALIZED);
   DCHECK(!init_cb_.is_null());
+  decoder_selector_.reset();
 
   if (!selected_decoder) {
     state_ = UNINITIALIZED;
     base::ResetAndReturn(&init_cb_).Run(false, false);
   } else {
     decoder_ = selected_decoder.Pass();
-    decrypting_demuxer_stream_ = decrypting_demuxer_stream;
+    decrypting_demuxer_stream_ = decrypting_demuxer_stream.Pass();
     state_ = NORMAL;
     base::ResetAndReturn(&init_cb_).Run(true, decoder_->HasAlpha());
   }
 
   // Stop() called during initialization.
   if (!stop_cb_.is_null()) {
     Stop(base::ResetAndReturn(&stop_cb_));
     return;
   }
 }
@@ skipping 37 matching lines @@
 void VideoFrameStream::OnDecoderStopped() {
   DCHECK(message_loop_->BelongsToCurrentThread());
   DCHECK_EQ(state_, NORMAL);
   // If Stop() was called during pending read/reset, read/reset callback should
   // be fired before the stop callback is fired.
   DCHECK(read_cb_.is_null());
   DCHECK(reset_cb_.is_null());
   DCHECK(!stop_cb_.is_null());
 
   state_ = STOPPED;
-  // Break the ref-count loop so we don't leak objects.
-  // TODO(scherkus): Make DemuxerStream and/or VideoDecoder not ref-counted so
-  // we don't need this here. See: http://crbug.com/173313
-  stream_ = NULL;

[acolwell GONE FROM CHROMIUM, 2013/04/23 03:20:42]

nit:ditto

[scherkus (not reviewing), 2013/04/23 18:43:40]

Done.

-  decrypting_demuxer_stream_ = NULL;
   decoder_.reset();
+  decrypting_demuxer_stream_.reset();
   base::ResetAndReturn(&stop_cb_).Run();
 }
 
 }  // namespace media