Cast: Refactoring Cast API's

chrome/renderer/media/cast_session_delegate.cc

 // Copyright 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 "chrome/renderer/media/cast_session_delegate.h"
 
 #include "base/logging.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "content/public/renderer/p2p_socket_client.h"
 #include "content/public/renderer/render_thread.h"
@@ skipping 51 matching lines @@
 
  private:
   DISALLOW_COPY_AND_ASSIGN(DummyTransport);
 };
 }  // namespace
 
 CastSessionDelegate::CastSessionDelegate()
     : audio_encode_thread_("CastAudioEncodeThread"),
       video_encode_thread_("CastVideoEncodeThread"),
       io_message_loop_proxy_(
-          content::RenderThread::Get()->GetIOMessageLoopProxy()) {
-}
-
-CastSessionDelegate::~CastSessionDelegate() {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
-}
-
-void CastSessionDelegate::Initialize() {
-  if (cast_environment_)
-    return;  // Already initialized.
+          content::RenderThread::Get()->GetIOMessageLoopProxy()),
+      weak_factory_(this) {
 
   cast_transport_.reset(new DummyTransport());
   audio_encode_thread_.Start();
   video_encode_thread_.Start();
 
   // CastSender uses the renderer's IO thread as the main thread. This reduces
   // thread hopping for incoming video frames and outgoing network packets.
   // There's no need to decode so no thread assigned for decoding.
   // Get default logging: All disabled.
   cast_environment_ = new CastEnvironment(
       scoped_ptr<base::TickClock>(new base::DefaultTickClock()).Pass(),
       base::MessageLoopProxy::current(),
       audio_encode_thread_.message_loop_proxy(),
       NULL,
       video_encode_thread_.message_loop_proxy(),
       NULL,
       base::MessageLoopProxy::current(),
       media::cast::GetDefaultCastSenderLoggingConfig());
+  cast_sender_ =
+      CastSender::Create(cast_environment_,
+                         base::Bind(&CastSessionDelegate::InitializationResult,
+                                    weak_factory_.GetWeakPtr()),
+                         cast_transport_.get()));
+}
+
+CastSessionDelegate::~CastSessionDelegate() {
+  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
 }
 
 void CastSessionDelegate::StartAudio(
     const AudioSenderConfig& config,
     const FrameInputAvailableCallback& callback) {
   DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
 
-  audio_config_.reset(new AudioSenderConfig(config));
-  StartSendingInternal(callback, true);
+  cast_sender_->InitializeAudio(config);
+  audio_frame_input_available_callback_.reset(

[Alpha Left Google, 2014/02/18 22:28:13]

This is very odd. We rarely use a scoped_ptr to hold a callback. This should
just be:

audio_frame_input_available_callback_ = callback;

Also the assignment of callback should come before InitializeAudio is called.

[mikhal1, 2014/03/05 21:44:05]

As mentioned before, resolved when rebased.
On 2014/02/18 22:28:13, Alpha wrote:

+      new FrameInputAvailableCallback(callback));
 }
 
 void CastSessionDelegate::StartVideo(
     const VideoSenderConfig& config,
     const FrameInputAvailableCallback& callback) {
   DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
 
-  video_config_.reset(new VideoSenderConfig(config));
-  StartSendingInternal(callback, false);
-}
-
-void CastSessionDelegate::StartSendingInternal(
-    const FrameInputAvailableCallback& callback,
-    bool is_audio) {
-  DCHECK(io_message_loop_proxy_->BelongsToCurrentThread());
-
-  Initialize();
-
-  if (is_audio) {
-    audio_frame_input_available_callback_.reset(
-        new FrameInputAvailableCallback(callback));
-  } else {
-    video_frame_input_available_callback_.reset(
-        new FrameInputAvailableCallback(callback));
-  }
-
-  cast_sender_.reset(CastSender::CreateCastSender(
-      cast_environment_,
-      audio_config_.get(),
-      video_config_.get(),
-      NULL,  // GPU.
-      base::Bind(&CastSessionDelegate::InitializationResult,
-                 base::Unretained(this)),
-      cast_transport_.get()));
+  // Initialize video - set gpu to NULL.

[Alpha Left Google, 2014/02/18 22:28:13]

nit: This comment should be a TODO to pass in a valid
GpuVideoAcceleratorFactories to support hardware video encoding.

[mikhal1, 2014/03/05 21:44:05]

Done.

+  cast_sender_->InitializeVideo(config, NULL);
+  video_frame_input_available_callback_.reset(

[Alpha Left Google, 2014/02/18 22:28:13]

This should just be:
video_frame_input_available_callback_ = callback;
This line should be called before InitializeVideo.

[mikhal1, 2014/03/05 21:44:05]

Done.

+      new FrameInputAvailableCallback(callback));
 }
 
 void CastSessionDelegate::InitializationResult(
     media::cast::CastInitializationStatus result) const {
   DCHECK(cast_sender_);
 
   // TODO(pwestin): handle the error codes.
-  if (result == media::cast::STATUS_INITIALIZED) {
-    if (audio_frame_input_available_callback_) {
+  if (result == media::cast::STATUS_AUDIO_INITIALIZED) {
+    if (audio_frame_input_available_callback_)

[Alpha Left Google, 2014/02/18 22:28:13]

This should be audio_frame_input_available_callback_.is_null().

[mikhal1, 2014/03/05 21:44:05]

Done.

       audio_frame_input_available_callback_->Run(cast_sender_->frame_input());
-    }
-    if (video_frame_input_available_callback_) {
+  } else if (result == media::cast::STATUS_VIDEO_INITIALIZED) {
+    if (video_frame_input_available_callback_)
       video_frame_input_available_callback_->Run(cast_sender_->frame_input());
-    }
   }
 }