Late initialize MediaController, for less resource i.e. ProcessThread, usage by PeerConnection.

webrtc/api/peerconnection.cc

Index: webrtc/api/peerconnection.cc
diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc
index eacbeaedc3c8bd2262169362276572d95772ce73..511ae07896fcea869f6ac481d818fa88788b39b5 100644
--- a/webrtc/api/peerconnection.cc
+++ b/webrtc/api/peerconnection.cc
@@ -84,7 +84,6 @@ enum {
   MSG_SET_SESSIONDESCRIPTION_SUCCESS = 0,
   MSG_SET_SESSIONDESCRIPTION_FAILED,
   MSG_CREATE_SESSIONDESCRIPTION_FAILED,
-  MSG_GETSTATS,
   MSG_FREE_DATACHANNELS,
 };
 
@@ -107,15 +106,6 @@ struct CreateSessionDescriptionMsg : public rtc::MessageData {
   std::string error;
 };
 
-struct GetStatsMsg : public rtc::MessageData {
-  GetStatsMsg(webrtc::StatsObserver* observer,
-              webrtc::MediaStreamTrackInterface* track)
-      : observer(observer), track(track) {
-  }
-  rtc::scoped_refptr<webrtc::StatsObserver> observer;
-  rtc::scoped_refptr<webrtc::MediaStreamTrackInterface> track;
-};
-
 // |in_str| should be of format
 // stunURI       = scheme ":" stun-host [ ":" stun-port ]
 // scheme        = "stun" / "stuns"
@@ -617,26 +607,19 @@ bool PeerConnection::Initialize(
   port_allocator_->set_step_delay(cricket::kMinimumStepDelay);
 
   // We rely on default values when constraints aren't found.
-  cricket::MediaConfig media_config;
-
-  media_config.disable_prerenderer_smoothing =
+  media_config_.disable_prerenderer_smoothing =
       configuration.disable_prerenderer_smoothing;
 
   // Find DSCP constraint.
   FindConstraint(constraints, MediaConstraintsInterface::kEnableDscp,
-                 &media_config.enable_dscp, NULL);
+                 &media_config_.enable_dscp, NULL);
   // Find constraints for cpu overuse detection.
   FindConstraint(constraints, MediaConstraintsInterface::kCpuOveruseDetection,
-                 &media_config.enable_cpu_overuse_detection, NULL);
-
-  media_controller_.reset(factory_->CreateMediaController(media_config));
-
-  remote_stream_factory_.reset(new RemoteMediaStreamFactory(
-      factory_->signaling_thread(), media_controller_->channel_manager()));
+                 &media_config_.enable_cpu_overuse_detection, NULL);
 
   session_.reset(
-      new WebRtcSession(media_controller_.get(), factory_->signaling_thread(),
-                        factory_->worker_thread(), port_allocator_.get()));
+      new WebRtcSession(factory_->signaling_thread(), factory_->worker_thread(),
+                        port_allocator_.get()));
   stats_.reset(new StatsCollector(this));
 
   // Initialize the WebRtcSession. It creates transport channels etc.
@@ -742,12 +725,14 @@ rtc::scoped_refptr<RtpSenderInterface> PeerConnection::AddTrack(
         << "Adding a track with two streams is not currently supported.";
     return nullptr;
   }
+
   // TODO(deadbeef): Support adding a track to two different senders.
   if (FindSenderForTrack(track) != senders_.end()) {
     LOG(LS_ERROR) << "Sender for track " << track->id() << " already exists.";
     return nullptr;
   }
 
+  LateInitialize();
   // TODO(deadbeef): Support adding a track to multiple streams.
   rtc::scoped_refptr<RtpSenderInterface> new_sender;
   if (track->kind() == MediaStreamTrackInterface::kAudioKind) {
@@ -816,6 +801,7 @@ rtc::scoped_refptr<DtmfSenderInterface> PeerConnection::CreateDtmfSender(
     return NULL;
   }
 
+  LateInitialize();
   rtc::scoped_refptr<DtmfSenderInterface> sender(
       DtmfSender::Create(track, signaling_thread(), session_.get()));
   if (!sender.get()) {
@@ -829,6 +815,7 @@ rtc::scoped_refptr<RtpSenderInterface> PeerConnection::CreateSender(
     const std::string& kind,
     const std::string& stream_id) {
   TRACE_EVENT0("webrtc", "PeerConnection::CreateSender");
+  LateInitialize();
   rtc::scoped_refptr<RtpSenderInterface> new_sender;
   if (kind == MediaStreamTrackInterface::kAudioKind) {
     new_sender = RtpSenderProxy::Create(
@@ -866,10 +853,10 @@ bool PeerConnection::GetStats(StatsObserver* observer,
     LOG(LS_ERROR) << "GetStats - observer is NULL.";
     return false;
   }
-
   stats_->UpdateStats(level);
-  signaling_thread()->Post(this, MSG_GETSTATS,
-                           new GetStatsMsg(observer, track));
+  StatsReports reports;
+  stats_->GetStats(track, &reports);

[tommi, 2016/02/22 15:22:05]

Is this change necessary for the goal of the CL?

I worry that this could have some unwanted side effects. Today, OnComplete() is
called after GetStats() has executed, but this forces it to be _while_ GetStats
is executing.  GetStats is meant to be async, so I'd prefer to keep it working
like that and give us an opportunity to improve things such as this:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/webrtc...
This is btw a TODO I put down for myself after restructuring that code quite a
bit and make it obvious that it's not truly async, while it should be.

[the sun, 2016/02/23 14:14:03]

Duh! I thought I remembered that a Post() to current thread would be executed
synchronously, but it seems I was wrong. This wasn't strictly required for the
CL at hand so I've reverted. Thanks!

+  observer->OnComplete(reports);
   return true;
 }
 
@@ -908,6 +895,7 @@ PeerConnection::CreateDataChannel(
     return nullptr;
   }
 
+  LateInitialize();
   // Trigger the onRenegotiationNeeded event for every new RTP DataChannel, or
   // the first SCTP DataChannel.
   if (session_->data_channel_type() == cricket::DCT_RTP || first_datachannel) {
@@ -977,6 +965,7 @@ void PeerConnection::CreateOffer(CreateSessionDescriptionObserver* observer,
     return;
   }
 
+  LateInitialize();
   cricket::MediaSessionOptions session_options;
   if (!GetOptionsForOffer(options, &session_options)) {
     std::string error = "CreateOffer called with invalid options.";
@@ -997,6 +986,7 @@ void PeerConnection::CreateAnswer(
     return;
   }
 
+  LateInitialize();
   cricket::MediaSessionOptions session_options;
   if (!GetOptionsForAnswer(constraints, &session_options)) {
     std::string error = "CreateAnswer called with invalid constraints.";
@@ -1020,6 +1010,7 @@ void PeerConnection::SetLocalDescription(
     PostSetSessionDescriptionFailure(observer, "SessionDescription is NULL.");
     return;
   }
+  LateInitialize();
   // Update stats here so that we have the most recent stats for tracks and
   // streams that might be removed by updating the session description.
   stats_->UpdateStats(kStatsOutputLevelStandard);
@@ -1098,6 +1089,7 @@ void PeerConnection::SetRemoteDescription(
     PostSetSessionDescriptionFailure(observer, "SessionDescription is NULL.");
     return;
   }
+  LateInitialize();
   // Update stats here so that we have the most recent stats for tracks and
   // streams that might be removed by updating the session description.
   stats_->UpdateStats(kStatsOutputLevelStandard);
@@ -1243,8 +1235,9 @@ void PeerConnection::Close() {
   // Update stats here so that we have the most recent stats for tracks and
   // streams before the channels are closed.
   stats_->UpdateStats(kStatsOutputLevelStandard);
-
   session_->Close();
+  media_controller_.reset(nullptr);
+  remote_stream_factory_.reset(nullptr);
 }
 
 void PeerConnection::OnSessionStateChange(WebRtcSession* /*session*/,
@@ -1299,14 +1292,6 @@ void PeerConnection::OnMessage(rtc::Message* msg) {
       delete param;
       break;
     }
-    case MSG_GETSTATS: {
-      GetStatsMsg* param = static_cast<GetStatsMsg*>(msg->pdata);
-      StatsReports reports;
-      stats_->GetStats(param->track, &reports);
-      param->observer->OnComplete(reports);
-      delete param;
-      break;
-    }
     case MSG_FREE_DATACHANNELS: {
       sctp_data_channels_to_free_.clear();
       break;
@@ -1320,6 +1305,7 @@ void PeerConnection::OnMessage(rtc::Message* msg) {
 void PeerConnection::CreateAudioReceiver(MediaStreamInterface* stream,
                                          AudioTrackInterface* audio_track,
                                          uint32_t ssrc) {
+  LateInitialize();
   receivers_.push_back(RtpReceiverProxy::Create(
       signaling_thread(),
       new AudioRtpReceiver(audio_track, ssrc, session_.get())));
@@ -1328,6 +1314,7 @@ void PeerConnection::CreateAudioReceiver(MediaStreamInterface* stream,
 void PeerConnection::CreateVideoReceiver(MediaStreamInterface* stream,
                                          VideoTrackInterface* video_track,
                                          uint32_t ssrc) {
+  LateInitialize();
   receivers_.push_back(RtpReceiverProxy::Create(
       signaling_thread(),
       new VideoRtpReceiver(video_track, ssrc, session_.get())));
@@ -1415,6 +1402,7 @@ void PeerConnection::OnAudioTrackAdded(AudioTrackInterface* track,
     return;
   }
 
+  LateInitialize();
   // Normal case; we've never seen this track before.
   rtc::scoped_refptr<RtpSenderInterface> new_sender = RtpSenderProxy::Create(
       signaling_thread(),
@@ -1457,6 +1445,7 @@ void PeerConnection::OnVideoTrackAdded(VideoTrackInterface* track,
     return;
   }
 
+  LateInitialize();
   // Normal case; we've never seen this track before.
   rtc::scoped_refptr<RtpSenderInterface> new_sender = RtpSenderProxy::Create(
       signaling_thread(),
@@ -1497,6 +1486,16 @@ void PeerConnection::PostCreateSessionDescriptionFailure(
   signaling_thread()->Post(this, MSG_CREATE_SESSIONDESCRIPTION_FAILED, msg);
 }
 
+void PeerConnection::LateInitialize() {
+  if (!media_controller_) {
+    RTC_DCHECK(!remote_stream_factory_);
+    media_controller_.reset(factory_->CreateMediaController(media_config_));
+    session_->LateInitialize(media_controller_.get());

[tommi, 2016/02/22 15:22:05]

Would it make sense to media_controller_ be owned by session_? (and no
|media_controller_| member variable in PeerConnection)

[the sun, 2016/02/23 14:14:03]

It used to be owned by WebRtcSession: https://codereview.webrtc.org/1269863005/

I don't know the exact reason why the relationship has changed, but I would
assume it has to do with the longer term goal of merging PeerConnection and
WebRtcSession in terms of functionality provided. pthatcher@? 

+    remote_stream_factory_.reset(new RemoteMediaStreamFactory(
+        factory_->signaling_thread(), media_controller_->channel_manager()));
+  }
+}
+
 bool PeerConnection::GetOptionsForOffer(
     const PeerConnectionInterface::RTCOfferAnswerOptions& rtc_options,
     cricket::MediaSessionOptions* session_options) {
@@ -1504,6 +1503,7 @@ bool PeerConnection::GetOptionsForOffer(
     return false;
   }
 
+  LateInitialize();
   AddSendStreams(session_options, senders_, rtp_data_channels_);
   // Offer to receive audio/video if the constraint is not set and there are
   // send streams, or we're currently receiving.
@@ -1537,6 +1537,7 @@ bool PeerConnection::GetOptionsForAnswer(
     return false;
   }
 
+  LateInitialize();
   AddSendStreams(session_options, senders_, rtp_data_channels_);
   session_options->bundle_enabled =
       session_options->bundle_enabled &&
@@ -1638,6 +1639,7 @@ void PeerConnection::OnRemoteTrackSeen(const std::string& stream_label,
                                        cricket::MediaType media_type) {
   MediaStreamInterface* stream = remote_streams_->find(stream_label);
 
+  LateInitialize();
   if (media_type == cricket::MEDIA_TYPE_AUDIO) {
     AudioTrackInterface* audio_track = remote_stream_factory_->AddAudioTrack(
         ssrc, session_.get(), stream, track_id);
@@ -1894,6 +1896,7 @@ rtc::scoped_refptr<DataChannel> PeerConnection::InternalCreateDataChannel(
   if (IsClosed()) {
     return nullptr;
   }
+  LateInitialize();
   if (session_->data_channel_type() == cricket::DCT_NONE) {
     LOG(LS_ERROR)
         << "InternalCreateDataChannel: Data is not supported in this call.";