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 52 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.
 
   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_.reset(CastSender::CreateCastSender(
+      cast_environment_,
+      base::Bind(&CastSessionDelegate::InitializationResult,
+                 base::Unretained(this)),

[Ami GONE FROM CHROMIUM, 2014/02/13 18:02:14]

What makes this Unretained safe?
Is it that this bound callback is only invoked on the IO thread and only as a
result of calling Initialize{Audio,Video} on the IO thread, and that
synchronously, and the dtor DCHECKing that this is only deleted on the IO
thread?
Because if the answer is yeah, the above, it might be worth describing in a note
:)

[Alpha Left Google, 2014/02/13 19:53:59]

I too this this needs an explanation.

Please make sure the callback is called on the cast main thread. If it happens
asynchronously this should be a weak pointer.

[mikhal1, 2014/02/14 18:03:16]

Switching to a weak ptr.
On 2014/02/13 19:53:59, Alpha wrote:

+      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(
+        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(
+  // Initialize video - set gpu to NULL.
+  cast_sender_->InitializeVideo(config, NULL);
+  video_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()));
 }
 
 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_)
       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());
-    }
   }
 }