From 1305a1d05e520dcef9fab90bfaac921391e4c99d Mon Sep 17 00:00:00 2001 From: "perkj@webrtc.org" Date: Tue, 18 Oct 2011 11:54:46 +0000 Subject: [PATCH] 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 --- .../talk/app/webrtc_dev/mediastreamhandler.cc | 71 ++++++------------- .../talk/app/webrtc_dev/mediastreamhandler.h | 20 +++--- .../peerconnection_client/conductor.cc | 13 ++-- .../peerconnection_client/linux/main_wnd.cc | 50 +++++++------ .../peerconnection_client/linux/main_wnd.h | 18 ++--- .../examples/peerconnection_client/main_wnd.h | 5 +- 6 files changed, 77 insertions(+), 100 deletions(-) diff --git a/third_party_mods/libjingle/source/talk/app/webrtc_dev/mediastreamhandler.cc b/third_party_mods/libjingle/source/talk/app/webrtc_dev/mediastreamhandler.cc index 7280f9fd3..6f9a07358 100644 --- a/third_party_mods/libjingle/source/talk/app/webrtc_dev/mediastreamhandler.cc +++ b/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 - TrackStateMessageData; -typedef talk_base::TypedMessageData 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(msg->pdata); - OnStateChanged(data->data()); - delete data; - break; - } - case MSG_TRACK_RENDERERCHANGED: { - OnRendererChanged(); - break; - } - case MSG_TRACK_ENABLEDCHANGED: { - TrackEnabledMessageData* data = - static_cast(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? } diff --git a/third_party_mods/libjingle/source/talk/app/webrtc_dev/mediastreamhandler.h b/third_party_mods/libjingle/source/talk/app/webrtc_dev/mediastreamhandler.h index d3551a2cb..560a590bd 100644 --- a/third_party_mods/libjingle/source/talk/app/webrtc_dev/mediastreamhandler.h +++ b/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 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 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 remote_video_track_; diff --git a/third_party_mods/libjingle/source/talk/examples/peerconnection_client/conductor.cc b/third_party_mods/libjingle/source/talk/examples/peerconnection_client/conductor.cc index b60c00549..92b995ef0 100644 --- a/third_party_mods/libjingle/source/talk/examples/peerconnection_client/conductor.cc +++ b/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 renderer( - webrtc::CreateVideoRenderer( - main_wnd_->local_renderer())); - video_track->SetRenderer(renderer); + video_track->SetRenderer(main_wnd_->local_renderer()); scoped_refptr 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 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. diff --git a/third_party_mods/libjingle/source/talk/examples/peerconnection_client/linux/main_wnd.cc b/third_party_mods/libjingle/source/talk/examples/peerconnection_client/linux/main_wnd.cc index 9abc762be..61e6e842b 100644 --- a/third_party_mods/libjingle/source/talk/examples/peerconnection_client/linux/main_wnd.cc +++ b/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(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( - remote_renderer_->image()); + remote_renderer->image()); uint32* scaled = reinterpret_cast(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(local_renderer_->image()); + VideoRenderer* local_renderer = + static_cast(local_renderer_wrapper_->renderer()); + if (local_renderer && local_renderer->image()) { + image = reinterpret_cast(local_renderer->image()); scaled = reinterpret_cast(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; } diff --git a/third_party_mods/libjingle/source/talk/examples/peerconnection_client/linux/main_wnd.h b/third_party_mods/libjingle/source/talk/examples/peerconnection_client/linux/main_wnd.h index 64a3eb2b8..ca4e52130 100644 --- a/third_party_mods/libjingle/source/talk/examples/peerconnection_client/linux/main_wnd.h +++ b/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 image_; int width_; @@ -105,8 +107,8 @@ class GtkMainWnd : public MainWindow { MainWndCallback* callback_; std::string server_; std::string port_; - talk_base::scoped_ptr local_renderer_; - talk_base::scoped_ptr remote_renderer_; + scoped_refptr local_renderer_wrapper_; + scoped_refptr remote_renderer_wrapper_; talk_base::scoped_ptr draw_buffer_; int draw_buffer_size_; }; diff --git a/third_party_mods/libjingle/source/talk/examples/peerconnection_client/main_wnd.h b/third_party_mods/libjingle/source/talk/examples/peerconnection_client/main_wnd.h index 02be37160..5b514c08c 100644 --- a/third_party_mods/libjingle/source/talk/examples/peerconnection_client/main_wnd.h +++ b/third_party_mods/libjingle/source/talk/examples/peerconnection_client/main_wnd.h @@ -15,6 +15,7 @@ #include #include +#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; };