Fix rendering in new PeerConnection API.

Fix MediaStreamHandler to make sure it releases the reference to a renderer when it is no longer needed.
Removes the use of the signaling thread in MediaStreamHandler.

Fix renderering in peerconnection_client_dev. It now uses the reference counted render wrapper.

BUG=
TEST=

Review URL: http://webrtc-codereview.appspot.com/242001

git-svn-id: http://webrtc.googlecode.com/svn/trunk@764 4adac7df-926f-26a2-2b94-8c16560cd09d

third_party_mods/libjingle/source/talk/app/webrtc_dev/mediastreamhandler.cc

@@ -37,24 +37,13 @@
 
 namespace webrtc {
 
-enum {
-  MSG_TRACK_STATECHANGED = 1,
-  MSG_TRACK_RENDERERCHANGED = 2,
-  MSG_TRACK_ENABLEDCHANGED = 3,
-};
-
-typedef talk_base::TypedMessageData<MediaStreamTrackInterface::TrackState>
-    TrackStateMessageData;
-typedef talk_base::TypedMessageData<bool> TrackEnabledMessageData;
-
 VideoTrackHandler::VideoTrackHandler(VideoTrackInterface* track,
                                      MediaProviderInterface* provider)
     : provider_(provider),
       video_track_(track),
       state_(track->state()),
       enabled_(track->enabled()),
-      renderer_(track->GetRenderer()),
-      signaling_thread_(talk_base::Thread::Current()) {
+      renderer_(track->GetRenderer()) {
   video_track_->RegisterObserver(this);
 }
 
@@ -65,40 +54,15 @@ VideoTrackHandler::~VideoTrackHandler() {
 void VideoTrackHandler::OnChanged() {
   if (state_ != video_track_->state()) {
     state_ = video_track_->state();
-    TrackStateMessageData* state_param(new TrackStateMessageData(state_));
-    signaling_thread_->Post(this, MSG_TRACK_STATECHANGED, state_param);
+    OnStateChanged();
   }
   if (renderer_.get() != video_track_->GetRenderer()) {
     renderer_ = video_track_->GetRenderer();
-    signaling_thread_->Post(this, MSG_TRACK_RENDERERCHANGED);
+    OnRendererChanged();
   }
   if (enabled_ != video_track_->enabled()) {
-    enabled_ =video_track_->enabled();
-    TrackEnabledMessageData* enabled_param(
-        new TrackEnabledMessageData(enabled_));
-    signaling_thread_->Post(this, MSG_TRACK_ENABLEDCHANGED, enabled_param);
-  }
-}
-
-void VideoTrackHandler::OnMessage(talk_base::Message* msg) {
-  switch (msg->message_id) {
-    case MSG_TRACK_STATECHANGED: {
-      TrackStateMessageData* data =
-          static_cast<TrackStateMessageData*>(msg->pdata);
-      OnStateChanged(data->data());
-      delete data;
-      break;
-    }
-    case MSG_TRACK_RENDERERCHANGED: {
-      OnRendererChanged();
-      break;
-    }
-    case MSG_TRACK_ENABLEDCHANGED: {
-      TrackEnabledMessageData* data =
-          static_cast<TrackEnabledMessageData*>(msg->pdata);
-      OnEnabledChanged(data->data());
-      break;
-    }
+    enabled_ = video_track_->enabled();
+    OnEnabledChanged();
   }
 }
 
@@ -109,6 +73,12 @@ LocalVideoTrackHandler::LocalVideoTrackHandler(
       local_video_track_(track) {
 }
 
+LocalVideoTrackHandler::~LocalVideoTrackHandler() {
+  // Since cricket::VideoRenderer is not reference counted
+  // we need to remove the renderer before we are deleted.
+  provider_->SetLocalRenderer(video_track_->ssrc(), NULL);
+}
+
 void LocalVideoTrackHandler::OnRendererChanged() {
   VideoRendererWrapperInterface* renderer(video_track_->GetRenderer());
   if (renderer)
@@ -117,9 +87,8 @@ void LocalVideoTrackHandler::OnRendererChanged() {
     provider_->SetLocalRenderer(video_track_->ssrc(), NULL);
 }
 
-void LocalVideoTrackHandler::OnStateChanged(
-    MediaStreamTrackInterface::TrackState state) {
-  if (state == VideoTrackInterface::kLive) {
+void LocalVideoTrackHandler::OnStateChanged() {
+  if (local_video_track_->state() == VideoTrackInterface::kLive) {
     provider_->SetCaptureDevice(local_video_track_->ssrc(),
                                 local_video_track_->GetVideoCapture());
     VideoRendererWrapperInterface* renderer(video_track_->GetRenderer());
@@ -130,7 +99,7 @@ void LocalVideoTrackHandler::OnStateChanged(
   }
 }
 
-void LocalVideoTrackHandler::OnEnabledChanged(bool enabled) {
+void LocalVideoTrackHandler::OnEnabledChanged() {
   // TODO(perkj) What should happen when enabled is changed?
 }
 
@@ -141,6 +110,13 @@ RemoteVideoTrackHandler::RemoteVideoTrackHandler(
       remote_video_track_(track) {
 }
 
+RemoteVideoTrackHandler::~RemoteVideoTrackHandler() {
+  // Since cricket::VideoRenderer is not reference counted
+  // we need to remove the renderer before we are deleted.
+  provider_->SetRemoteRenderer(video_track_->ssrc(), NULL);
+}
+
+
 void RemoteVideoTrackHandler::OnRendererChanged() {
   VideoRendererWrapperInterface* renderer(video_track_->GetRenderer());
   if (renderer)
@@ -149,11 +125,10 @@ void RemoteVideoTrackHandler::OnRendererChanged() {
     provider_->SetRemoteRenderer(video_track_->ssrc(), NULL);
 }
 
-void RemoteVideoTrackHandler::OnStateChanged(
-    MediaStreamTrackInterface::TrackState state) {
+void RemoteVideoTrackHandler::OnStateChanged() {
 }
 
-void RemoteVideoTrackHandler::OnEnabledChanged(bool enabled) {
+void RemoteVideoTrackHandler::OnEnabledChanged() {
   // TODO(perkj): What should happen when enabled is changed?
 }
 

third_party_mods/libjingle/source/talk/app/webrtc_dev/mediastreamhandler.h

@@ -45,8 +45,7 @@ namespace webrtc {
 
 // VideoTrackHandler listen to events on a VideoTrack instance and
 // executes the requested change.
-class VideoTrackHandler : public Observer,
-                          public talk_base::MessageHandler {
+class VideoTrackHandler : public Observer {
  public:
   VideoTrackHandler(VideoTrackInterface* track,
                     MediaProviderInterface* provider);
@@ -54,11 +53,9 @@ class VideoTrackHandler : public Observer,
   virtual void OnChanged();
 
  protected:
-  virtual void OnMessage(talk_base::Message* msg);
-
   virtual void OnRendererChanged() = 0;
-  virtual void OnStateChanged(MediaStreamTrackInterface::TrackState state) = 0;
-  virtual void OnEnabledChanged(bool enabled) = 0;
+  virtual void OnStateChanged() = 0;
+  virtual void OnEnabledChanged() = 0;
 
   MediaProviderInterface* provider_;
   VideoTrackInterface* video_track_;
@@ -67,18 +64,18 @@ class VideoTrackHandler : public Observer,
   MediaStreamTrackInterface::TrackState state_;
   bool enabled_;
   scoped_refptr<VideoRendererWrapperInterface> renderer_;
-  talk_base::Thread* signaling_thread_;
 };
 
 class LocalVideoTrackHandler : public VideoTrackHandler {
  public:
   LocalVideoTrackHandler(LocalVideoTrackInterface* track,
                          MediaProviderInterface* provider);
+  virtual ~LocalVideoTrackHandler();
 
  protected:
   virtual void OnRendererChanged();
-  virtual void OnStateChanged(MediaStreamTrackInterface::TrackState state);
-  virtual void OnEnabledChanged(bool enabled);
+  virtual void OnStateChanged();
+  virtual void OnEnabledChanged();
 
  private:
   scoped_refptr<LocalVideoTrackInterface> local_video_track_;
@@ -88,11 +85,12 @@ class RemoteVideoTrackHandler : public VideoTrackHandler {
  public:
   RemoteVideoTrackHandler(VideoTrackInterface* track,
                           MediaProviderInterface* provider);
+  virtual ~RemoteVideoTrackHandler();
 
  protected:
   virtual void OnRendererChanged();
-  virtual void OnStateChanged(MediaStreamTrackInterface::TrackState state);
-  virtual void OnEnabledChanged(bool enabled);
+  virtual void OnStateChanged();
+  virtual void OnEnabledChanged();
 
  private:
   scoped_refptr<VideoTrackInterface> remote_video_track_;

third_party_mods/libjingle/source/talk/examples/peerconnection_client/conductor.cc

@@ -65,9 +65,9 @@ bool Conductor::InitializePeerConnection() {
 }
 
 void Conductor::DeletePeerConnection() {
-  peer_connection_.release();
+  peer_connection_ = NULL;
   active_streams_.clear();
-  peer_connection_factory_.release();
+  peer_connection_factory_ = NULL;
   peer_id_ = -1;
 }
 
@@ -256,10 +256,7 @@ void Conductor::AddStreams() {
       peer_connection_factory_->CreateLocalVideoTrack(
           kVideoLabel, OpenVideoCaptureDevice()));
 
-  scoped_refptr<webrtc::VideoRendererWrapperInterface> renderer(
-      webrtc::CreateVideoRenderer(
-      main_wnd_->local_renderer()));
-  video_track->SetRenderer(renderer);
+  video_track->SetRenderer(main_wnd_->local_renderer());
 
   scoped_refptr<webrtc::LocalMediaStreamInterface> stream =
       peer_connection_factory_->CreateLocalMediaStream(kStreamLabel);
@@ -348,9 +345,7 @@ void Conductor::UIThreadCallback(int msg_id, void* data) {
       for (size_t i = 0; i < tracks->count(); ++i) {
         webrtc::VideoTrackInterface* track = tracks->at(i);
         LOG(INFO) << "Setting video renderer for track: " << track->label();
-        scoped_refptr<webrtc::VideoRendererWrapperInterface> renderer(
-            webrtc::CreateVideoRenderer(main_wnd_->remote_renderer()));
-        track->SetRenderer(renderer);
+        track->SetRenderer(main_wnd_->remote_renderer());
       }
       // If we haven't shared any streams with this peer (we're the receiver)
       // then do so now.

third_party_mods/libjingle/source/talk/examples/peerconnection_client/linux/main_wnd.cc

@@ -139,16 +139,18 @@ MainWindow::UI GtkMainWnd::current_ui() {
   return STREAMING;
 }
 
-cricket::VideoRenderer* GtkMainWnd::local_renderer() {
-  if (!local_renderer_.get())
-    local_renderer_.reset(new VideoRenderer(this));
-  return local_renderer_.get();
+webrtc::VideoRendererWrapperInterface* GtkMainWnd::local_renderer() {
+  if (!local_renderer_wrapper_.get())
+    local_renderer_wrapper_  =
+        webrtc::CreateVideoRenderer(new VideoRenderer(this));
+  return local_renderer_wrapper_.get();
 }
 
-cricket::VideoRenderer* GtkMainWnd::remote_renderer() {
-  if (!remote_renderer_.get())
-    remote_renderer_.reset(new VideoRenderer(this));
-  return remote_renderer_.get();
+webrtc::VideoRendererWrapperInterface*  GtkMainWnd::remote_renderer() {
+  if (!remote_renderer_wrapper_.get())
+    remote_renderer_wrapper_ =
+        webrtc::CreateVideoRenderer(new VideoRenderer(this));
+  return remote_renderer_wrapper_.get();
 }
 
 void GtkMainWnd::QueueUIThreadCallback(int msg_id, void* data) {
@@ -234,8 +236,8 @@ void GtkMainWnd::SwitchToPeerList(const Peers& peers) {
   LOG(INFO) << __FUNCTION__;
 
   // Clean up buffers from a potential previous session.
-  local_renderer_.reset();
-  remote_renderer_.reset();
+  local_renderer_wrapper_ = NULL;
+  remote_renderer_wrapper_ = NULL;
 
   if (!peer_list_) {
     gtk_container_set_border_width(GTK_CONTAINER(window_), 0);
@@ -349,10 +351,12 @@ void GtkMainWnd::OnRowActivated(GtkTreeView* tree_view, GtkTreePath* path,
 void GtkMainWnd::OnRedraw() {
   gdk_threads_enter();
 
-  if (remote_renderer_.get() && remote_renderer_->image() != NULL &&
+  VideoRenderer* remote_renderer =
+      static_cast<VideoRenderer*>(remote_renderer_wrapper_->renderer());
+  if (remote_renderer && remote_renderer->image() != NULL &&
       draw_area_ != NULL) {
-    int width = remote_renderer_->width();
-    int height = remote_renderer_->height();
+    int width = remote_renderer->width();
+    int height = remote_renderer->height();
 
     if (!draw_buffer_.get()) {
       draw_buffer_size_ = (width * height * 4) * 4;
@@ -361,7 +365,7 @@ void GtkMainWnd::OnRedraw() {
     }
 
     const uint32* image = reinterpret_cast<const uint32*>(
-        remote_renderer_->image());
+        remote_renderer->image());
     uint32* scaled = reinterpret_cast<uint32*>(draw_buffer_.get());
     for (int r = 0; r < height; ++r) {
       for (int c = 0; c < width; ++c) {
@@ -377,22 +381,24 @@ void GtkMainWnd::OnRedraw() {
       scaled += width * 2;
     }
 
-    if (local_renderer_.get() && local_renderer_->image()) {
-      image = reinterpret_cast<const uint32*>(local_renderer_->image());
+    VideoRenderer* local_renderer =
+        static_cast<VideoRenderer*>(local_renderer_wrapper_->renderer());
+    if (local_renderer && local_renderer->image()) {
+      image = reinterpret_cast<const uint32*>(local_renderer->image());
       scaled = reinterpret_cast<uint32*>(draw_buffer_.get());
       // Position the local preview on the right side.
-      scaled += (width * 2) - (local_renderer_->width() / 2);
+      scaled += (width * 2) - (local_renderer->width() / 2);
       // right margin...
       scaled -= 10;
       // ... towards the bottom.
       scaled += (height * width * 4) -
-                ((local_renderer_->height() / 2) *
-                 (local_renderer_->width() / 2) * 4);
+                ((local_renderer->height() / 2) *
+                 (local_renderer->width() / 2) * 4);
       // bottom margin...
       scaled -= (width * 2) * 5;
-      for (int r = 0; r < local_renderer_->height(); r += 2) {
-        for (int c = 0; c < local_renderer_->width(); c += 2) {
-          scaled[c / 2] = image[c + r * local_renderer_->width()];
+      for (int r = 0; r < local_renderer->height(); r += 2) {
+        for (int c = 0; c < local_renderer->width(); c += 2) {
+          scaled[c / 2] = image[c + r * local_renderer->width()];
         }
         scaled += width * 2;
       }

third_party_mods/libjingle/source/talk/examples/peerconnection_client/linux/main_wnd.h

@@ -14,6 +14,7 @@
 
 #include "talk/examples/peerconnection_client/main_wnd.h"
 #include "talk/examples/peerconnection_client/peer_connection_client.h"
+#include "talk/app/webrtc_dev/scoped_refptr.h"
 
 // Forward declarations.
 typedef struct _GtkWidget GtkWidget;
@@ -39,8 +40,9 @@ class GtkMainWnd : public MainWindow {
   virtual void MessageBox(const char* caption, const char* text,
                           bool is_error);
   virtual MainWindow::UI current_ui();
-  virtual cricket::VideoRenderer* local_renderer();
-  virtual cricket::VideoRenderer* remote_renderer();
+  virtual webrtc::VideoRendererWrapperInterface* local_renderer();
+  virtual webrtc::VideoRendererWrapperInterface* remote_renderer();
+
   virtual void QueueUIThreadCallback(int msg_id, void* data);
 
   // Creates and shows the main window with the |Connect UI| enabled.
@@ -63,13 +65,13 @@ class GtkMainWnd : public MainWindow {
   // connection.
   void OnRowActivated(GtkTreeView* tree_view, GtkTreePath* path,
                       GtkTreeViewColumn* column);
-                      
+
   void OnRedraw();
 
  protected:
   class VideoRenderer : public cricket::VideoRenderer {
    public:
-    VideoRenderer(GtkMainWnd* main_wnd);
+    explicit VideoRenderer(GtkMainWnd* main_wnd);
     virtual ~VideoRenderer();
 
     virtual bool SetSize(int width, int height, int reserved);
@@ -83,11 +85,11 @@ class GtkMainWnd : public MainWindow {
     int width() const {
       return width_;
     }
-    
+
     int height() const {
       return height_;
     }
-    
+
    protected:
     talk_base::scoped_array<uint8> image_;
     int width_;
@@ -105,8 +107,8 @@ class GtkMainWnd : public MainWindow {
   MainWndCallback* callback_;
   std::string server_;
   std::string port_;
-  talk_base::scoped_ptr<VideoRenderer> local_renderer_;
-  talk_base::scoped_ptr<VideoRenderer> remote_renderer_;
+  scoped_refptr<webrtc::VideoRendererWrapperInterface> local_renderer_wrapper_;
+  scoped_refptr<webrtc::VideoRendererWrapperInterface> remote_renderer_wrapper_;
   talk_base::scoped_ptr<uint8> draw_buffer_;
   int draw_buffer_size_;
 };

third_party_mods/libjingle/source/talk/examples/peerconnection_client/main_wnd.h

@@ -15,6 +15,7 @@
 #include <map>
 #include <string>
 
+#include "talk/app/webrtc_dev/mediastream.h"
 #include "talk/examples/peerconnection_client/peer_connection_client.h"
 #include "talk/base/win32.h"
 #include "talk/session/phone/mediachannel.h"
@@ -57,8 +58,8 @@ class MainWindow {
   virtual void SwitchToPeerList(const Peers& peers) = 0;
   virtual void SwitchToStreamingUI() = 0;
 
-  virtual cricket::VideoRenderer* local_renderer() = 0;
-  virtual cricket::VideoRenderer* remote_renderer() = 0;
+  virtual webrtc::VideoRendererWrapperInterface* local_renderer() = 0;
+  virtual webrtc::VideoRendererWrapperInterface* remote_renderer() = 0;
 
   virtual void QueueUIThreadCallback(int msg_id, void* data) = 0;
 };