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

webrtc/api/peerconnection.cc

 /*
  *  Copyright 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 599 matching lines @@
   if (configuration.tcp_candidate_policy == kTcpCandidatePolicyDisabled) {
     portallocator_flags |= cricket::PORTALLOCATOR_DISABLE_TCP;
     LOG(LS_INFO) << "TCP candidates are disabled.";
   }
 
   port_allocator_->set_flags(portallocator_flags);
   // No step delay is used while allocating ports.
   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.
   if (!session_->Initialize(factory_->options(), constraints,
                             std::move(dtls_identity_store), configuration)) {
     return false;
   }
 
   // Register PeerConnection as receiver of local ice candidates.
   // All the callbacks will be posted to the application from PeerConnection.
@@ skipping 85 matching lines @@
     std::vector<MediaStreamInterface*> streams) {
   TRACE_EVENT0("webrtc", "PeerConnection::AddTrack");
   if (IsClosed()) {
     return nullptr;
   }
   if (streams.size() >= 2) {
     LOG(LS_ERROR)
         << "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();

[pthatcher1, 2016/02/24 23:10:09]

Instead of calling LateInitialize everywhere (which is really error-prone), why
not just have the media_controller created on demand in the media_controller()
method?

[the sun, 2016/02/25 13:15:39]

That's a good suggestion, but unfortunately it doesn't apply here as this
function does not reference media_controller_ directly. LateInitialize() is
called here because of how we use WebRtcSession; in essence it is there to
manage the dual states of WebRtcSession.

I guess another option would be to add an internal fully_initialized_session()
helper, which would be slightly better explicit calls to LateInitialize(), but
still very error prone.

   // TODO(deadbeef): Support adding a track to multiple streams.
   rtc::scoped_refptr<RtpSenderInterface> new_sender;
   if (track->kind() == MediaStreamTrackInterface::kAudioKind) {
     new_sender = RtpSenderProxy::Create(
         signaling_thread(),
         new AudioRtpSender(static_cast<AudioTrackInterface*>(track),
                            session_.get(), stats_.get()));
     if (!streams.empty()) {
       new_sender->set_stream_id(streams[0]->label());
     }
@@ skipping 48 matching lines @@
   TRACE_EVENT0("webrtc", "PeerConnection::CreateDtmfSender");
   if (!track) {
     LOG(LS_ERROR) << "CreateDtmfSender - track is NULL.";
     return NULL;
   }
   if (!local_streams_->FindAudioTrack(track->id())) {
     LOG(LS_ERROR) << "CreateDtmfSender is called with a non local audio track.";
     return NULL;
   }
 
+  LateInitialize();
   rtc::scoped_refptr<DtmfSenderInterface> sender(
       DtmfSender::Create(track, signaling_thread(), session_.get()));
   if (!sender.get()) {
     LOG(LS_ERROR) << "CreateDtmfSender failed on DtmfSender::Create.";
     return NULL;
   }
   return DtmfSenderProxy::Create(signaling_thread(), sender.get());
 }
 
 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(
         signaling_thread(), new AudioRtpSender(session_.get(), stats_.get()));
   } else if (kind == MediaStreamTrackInterface::kVideoKind) {
     new_sender = RtpSenderProxy::Create(signaling_thread(),
                                         new VideoRtpSender(session_.get()));
   } else {
     LOG(LS_ERROR) << "CreateSender called with invalid kind: " << kind;
     return new_sender;
@@ skipping 59 matching lines @@
   rtc::scoped_ptr<InternalDataChannelInit> internal_config;
   if (config) {
     internal_config.reset(new InternalDataChannelInit(*config));
   }
   rtc::scoped_refptr<DataChannelInterface> channel(
       InternalCreateDataChannel(label, internal_config.get()));
   if (!channel.get()) {
     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) {
     observer_->OnRenegotiationNeeded();
   }
 
   return DataChannelProxy::Create(signaling_thread(), channel.get());
 }
 
 void PeerConnection::CreateOffer(CreateSessionDescriptionObserver* observer,
@@ skipping 49 matching lines @@
 }
 
 void PeerConnection::CreateOffer(CreateSessionDescriptionObserver* observer,
                                  const RTCOfferAnswerOptions& options) {
   TRACE_EVENT0("webrtc", "PeerConnection::CreateOffer");
   if (!VERIFY(observer != nullptr)) {
     LOG(LS_ERROR) << "CreateOffer - observer is NULL.";
     return;
   }
 
+  LateInitialize();
   cricket::MediaSessionOptions session_options;
   if (!GetOptionsForOffer(options, &session_options)) {
     std::string error = "CreateOffer called with invalid options.";
     LOG(LS_ERROR) << error;
     PostCreateSessionDescriptionFailure(observer, error);
     return;
   }
 
   session_->CreateOffer(observer, options, session_options);
 }
 
 void PeerConnection::CreateAnswer(
     CreateSessionDescriptionObserver* observer,
     const MediaConstraintsInterface* constraints) {
   TRACE_EVENT0("webrtc", "PeerConnection::CreateAnswer");
   if (!VERIFY(observer != nullptr)) {
     LOG(LS_ERROR) << "CreateAnswer - observer is NULL.";
     return;
   }
 
+  LateInitialize();
   cricket::MediaSessionOptions session_options;
   if (!GetOptionsForAnswer(constraints, &session_options)) {
     std::string error = "CreateAnswer called with invalid constraints.";
     LOG(LS_ERROR) << error;
     PostCreateSessionDescriptionFailure(observer, error);
     return;
   }
 
   session_->CreateAnswer(observer, constraints, session_options);
 }
 
 void PeerConnection::SetLocalDescription(
     SetSessionDescriptionObserver* observer,
     SessionDescriptionInterface* desc) {
   TRACE_EVENT0("webrtc", "PeerConnection::SetLocalDescription");
   if (!VERIFY(observer != nullptr)) {
     LOG(LS_ERROR) << "SetLocalDescription - observer is NULL.";
     return;
   }
   if (!desc) {
     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);
   std::string error;
   if (!session_->SetLocalDescription(desc, &error)) {
     PostSetSessionDescriptionFailure(observer, error);
     return;
   }
 
   // If setting the description decided our SSL role, allocate any necessary
@@ skipping 58 matching lines @@
     SessionDescriptionInterface* desc) {
   TRACE_EVENT0("webrtc", "PeerConnection::SetRemoteDescription");
   if (!VERIFY(observer != nullptr)) {
     LOG(LS_ERROR) << "SetRemoteDescription - observer is NULL.";
     return;
   }
   if (!desc) {
     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);
   std::string error;
   if (!session_->SetRemoteDescription(desc, &error)) {
     PostSetSessionDescriptionFailure(observer, error);
     return;
   }
 
   // If setting the description decided our SSL role, allocate any necessary
@@ skipping 125 matching lines @@
 
 const SessionDescriptionInterface* PeerConnection::remote_description() const {
   return session_->remote_description();
 }
 
 void PeerConnection::Close() {
   TRACE_EVENT0("webrtc", "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*/,
                                           WebRtcSession::State state) {
   switch (state) {
     case WebRtcSession::STATE_INIT:
       ChangeSignalingState(PeerConnectionInterface::kStable);
       break;
     case WebRtcSession::STATE_SENTOFFER:
       ChangeSignalingState(PeerConnectionInterface::kHaveLocalOffer);
@@ skipping 55 matching lines @@
     }
     default:
       RTC_DCHECK(false && "Not implemented");
       break;
   }
 }
 
 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())));
 }
 
 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())));
 }
 
 // TODO(deadbeef): Keep RtpReceivers around even if track goes away in remote
 // description.
 void PeerConnection::DestroyAudioReceiver(MediaStreamInterface* stream,
                                           AudioTrackInterface* audio_track) {
   auto it = FindReceiverForTrack(audio_track);
@@ skipping 67 matching lines @@
 void PeerConnection::OnAudioTrackAdded(AudioTrackInterface* track,
                                        MediaStreamInterface* stream) {
   auto sender = FindSenderForTrack(track);
   if (sender != senders_.end()) {
     // We already have a sender for this track, so just change the stream_id
     // so that it's correct in the next call to CreateOffer.
     (*sender)->set_stream_id(stream->label());
     return;
   }
 
+  LateInitialize();
   // Normal case; we've never seen this track before.
   rtc::scoped_refptr<RtpSenderInterface> new_sender = RtpSenderProxy::Create(
       signaling_thread(),
       new AudioRtpSender(track, stream->label(), session_.get(), stats_.get()));
   senders_.push_back(new_sender);
   // If the sender has already been configured in SDP, we call SetSsrc,
   // which will connect the sender to the underlying transport. This can
   // occur if a local session description that contains the ID of the sender
   // is set before AddStream is called. It can also occur if the local
   // session description is not changed and RemoveStream is called, and
@@ skipping 22 matching lines @@
 void PeerConnection::OnVideoTrackAdded(VideoTrackInterface* track,
                                        MediaStreamInterface* stream) {
   auto sender = FindSenderForTrack(track);
   if (sender != senders_.end()) {
     // We already have a sender for this track, so just change the stream_id
     // so that it's correct in the next call to CreateOffer.
     (*sender)->set_stream_id(stream->label());
     return;
   }
 
+  LateInitialize();
   // Normal case; we've never seen this track before.
   rtc::scoped_refptr<RtpSenderInterface> new_sender = RtpSenderProxy::Create(
       signaling_thread(),
       new VideoRtpSender(track, stream->label(), session_.get()));
   senders_.push_back(new_sender);
   const TrackInfo* track_info =
       FindTrackInfo(local_video_tracks_, stream->label(), track->id());
   if (track_info) {
     new_sender->SetSsrc(track_info->ssrc);
   }
@@ skipping 20 matching lines @@
 }
 
 void PeerConnection::PostCreateSessionDescriptionFailure(
     CreateSessionDescriptionObserver* observer,
     const std::string& error) {
   CreateSessionDescriptionMsg* msg = new CreateSessionDescriptionMsg(observer);
   msg->error = error;
   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());

[pthatcher1, 2016/02/24 23:10:09]

This would make a lot more sense if WebrtcSession and PeerConnection were one
class.  And it's our intention to merge them, we just haven't done it yet.  If
you'd like to do this "lazy load" thing, I think it would be much better to
merge the two classes first to avoid having to initialize methods in
WebRtcSession.

[the sun, 2016/02/25 13:15:39]

Agree. Like I noted in an initial comment, that is not an option if we are to
land a fix for crbug:582441 for the M50 cut. So the question is whether we want
to land this interim fix or not. Myself, I'm torn.

[pthatcher1, 2016/02/26 21:51:20]

I appreciate that we'd like to fix the bug quickly, but this is would be quite a
nasty temporary fix as-is, and it seems like the bug is a very rare one.  

Maybe if we can fix this CL enough to be not so nasty....

[the sun, 2016/02/29 15:58:22]

Acknowledged.

+    remote_stream_factory_.reset(new RemoteMediaStreamFactory(
+        factory_->signaling_thread(), media_controller_->channel_manager()));

[pthatcher1, 2016/02/24 23:10:09]

I looked into this, and found that the ChannelManager here is only needed for
the SignalVideoCaptureStateChange event, which isn't really needed.  I think we
can remove the signal and remove the need to pass the ChannelManager into the
RemoteMedisStreamFactory.

More info here:
https://docs.google.com/document/d/12LMKcZlcLnw62ijmIiLzjJ6GW_cDhGtOUgKiCVKw9...

[the sun, 2016/02/25 13:15:39]

Wow, thanks for taking such a deep dive!

What about the calls from VideoSource to channel_manager_->AddVideoSink() and
channel_manager_->RemoveVideoSink()? Or the calls from
ChannelManager::StartVideoCapture() onto CaptureManager::StartVideoCapture()
(and similarly for Stop)?

[pthatcher1, 2016/02/26 21:51:20]

ChannelManager::StartVideoCapture() is only called by
VideoSource::Initialize/Create and VideoSource::Restart.

ChannelManager::StopVideoCapture() is only called by VideoSource::Stop() and
VideoSource::~VideoSource.

ChannelManager::AddVideoSink is only called by VideoSource::AddSink.

ChannelManager::RemoveVideoSink is only called by VideoSource::RemoveSink.


In all of these cases, VideoSource knows that it changed something. So I think
it could just do SetState(GetReadyState(capture_state)) at the end of the method
instead of having the change trigger an event which does that.

[the sun, 2016/02/29 15:58:22]

Acknowledged.

+  }
+}
+
 bool PeerConnection::GetOptionsForOffer(
     const PeerConnectionInterface::RTCOfferAnswerOptions& rtc_options,
     cricket::MediaSessionOptions* session_options) {
   if (!ConvertRtcOptionsForOffer(rtc_options, session_options)) {
     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.
   if (rtc_options.offer_to_receive_audio == RTCOfferAnswerOptions::kUndefined) {
     session_options->recv_audio =
         session_options->HasSendMediaStream(cricket::MEDIA_TYPE_AUDIO) ||
         !remote_audio_tracks_.empty();
   }
   if (rtc_options.offer_to_receive_video == RTCOfferAnswerOptions::kUndefined) {
     session_options->recv_video =
@@ skipping 13 matching lines @@
 
 bool PeerConnection::GetOptionsForAnswer(
     const MediaConstraintsInterface* constraints,
     cricket::MediaSessionOptions* session_options) {
   session_options->recv_audio = false;
   session_options->recv_video = false;
   if (!ParseConstraintsForAnswer(constraints, session_options)) {
     return false;
   }
 
+  LateInitialize();
   AddSendStreams(session_options, senders_, rtp_data_channels_);
   session_options->bundle_enabled =
       session_options->bundle_enabled &&
       (session_options->has_audio() || session_options->has_video() ||
        session_options->has_data());
 
   // RTP data channel is handled in MediaSessionOptions::AddStream. SCTP streams
   // are not signaled in the SDP so does not go through that path and must be
   // handled here.
   if (session_->data_channel_type() == cricket::DCT_SCTP) {
@@ skipping 81 matching lines @@
     }
   }
 }
 
 void PeerConnection::OnRemoteTrackSeen(const std::string& stream_label,
                                        const std::string& track_id,
                                        uint32_t ssrc,
                                        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);
     CreateAudioReceiver(stream, audio_track, ssrc);
   } else if (media_type == cricket::MEDIA_TYPE_VIDEO) {
     VideoTrackInterface* video_track =
         remote_stream_factory_->AddVideoTrack(stream, track_id);
     CreateVideoReceiver(stream, video_track, ssrc);
   } else {
     RTC_DCHECK(false && "Invalid media type");
@@ skipping 236 matching lines @@
   observer_->OnDataChannel(
       DataChannelProxy::Create(signaling_thread(), channel));
 }
 
 rtc::scoped_refptr<DataChannel> PeerConnection::InternalCreateDataChannel(
     const std::string& label,
     const InternalDataChannelInit* config) {
   if (IsClosed()) {
     return nullptr;
   }
+  LateInitialize();
   if (session_->data_channel_type() == cricket::DCT_NONE) {
     LOG(LS_ERROR)
         << "InternalCreateDataChannel: Data is not supported in this call.";
     return nullptr;
   }
   InternalDataChannelInit new_config =
       config ? (*config) : InternalDataChannelInit();
   if (session_->data_channel_type() == cricket::DCT_SCTP) {
     if (new_config.id < 0) {
       rtc::SSLRole role;
@@ skipping 171 matching lines @@
 DataChannel* PeerConnection::FindDataChannelBySid(int sid) const {
   for (const auto& channel : sctp_data_channels_) {
     if (channel->id() == sid) {
       return channel;
     }
   }
   return nullptr;
 }
 
 }  // namespace webrtc