Deliver video frames on libjingle worker thread to WebRtcVideoCapturerAdapter.

content/renderer/media/webrtc/webrtc_video_track_adapter.cc

 // Copyright 2014 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 "content/renderer/media/webrtc/webrtc_video_track_adapter.h"
 
 #include "base/strings/utf_string_conversions.h"
 #include "content/common/media/media_stream_options.h"
 #include "content/renderer/media/media_stream_video_source.h"
 #include "content/renderer/media/media_stream_video_track.h"
 
 namespace {
 
 bool ConstraintKeyExists(const blink::WebMediaConstraints& constraints,
                          const blink::WebString& name) {
   blink::WebString value_str;
   return constraints.getMandatoryConstraintValue(name, value_str) ||
       constraints.getOptionalConstraintValue(name, value_str);
 }
 
+// Used to make sure |source| is released on the main render thread.
+void ReleaseWebRtcSourceOnMainRenderThread(
+    webrtc::VideoSourceInterface* source) {
+  source->Release();
+}
+
 }  // anonymouse namespace
 
 namespace content {
 
 // Simple help class used for receiving video frames on the IO-thread from
 // a MediaStreamVideoTrack and forward the frames to a
 // WebRtcVideoCapturerAdapter that implements a video capturer for libjingle.
 class WebRtcVideoTrackAdapter::WebRtcVideoSourceAdapter
     : public base::RefCountedThreadSafe<WebRtcVideoSourceAdapter> {
  public:
   WebRtcVideoSourceAdapter(
+      const scoped_refptr<base::MessageLoopProxy>& libjingle_worker_thread,
       const scoped_refptr<webrtc::VideoSourceInterface>& source,
       WebRtcVideoCapturerAdapter* capture_adapter);
 
   void OnVideoFrameOnIO(const scoped_refptr<media::VideoFrame>& frame,
                         const media::VideoCaptureFormat& format);
 
  private:
+  void OnVideoFrameOnWt(const scoped_refptr<media::VideoFrame>& frame,

[tommi (sloooow) - chröme, 2014/05/12 14:20:41]

OnVideoFrameOnWorkerThread

[perkj_chrome, 2014/05/12 15:39:51]

Done.

+                        const media::VideoCaptureFormat& format);
   friend class base::RefCountedThreadSafe<WebRtcVideoSourceAdapter>;
   virtual ~WebRtcVideoSourceAdapter();
 
+  scoped_refptr<base::MessageLoopProxy> render_thread_message_loop_;
+
   // Used to DCHECK that frames are called on the IO-thread.
   base::ThreadChecker io_thread_checker_;
+
+  // Used for posting frames to libjingles worker thread. Accessed on the

[tommi (sloooow) - chröme, 2014/05/12 14:20:41]

libjingle's

[perkj_chrome, 2014/05/12 15:39:51]

Done.

+  // IO-thread.
+  scoped_refptr<base::MessageLoopProxy> libjingle_worker_thread_;
+
   scoped_refptr<webrtc::VideoSourceInterface> video_source_;
   // |capture_adapter_| is owned by |video_source_|
   WebRtcVideoCapturerAdapter* capture_adapter_;
 };
 
 WebRtcVideoTrackAdapter::WebRtcVideoSourceAdapter::WebRtcVideoSourceAdapter(
+    const scoped_refptr<base::MessageLoopProxy>& libjingle_worker_thread,
     const scoped_refptr<webrtc::VideoSourceInterface>& source,
     WebRtcVideoCapturerAdapter* capture_adapter)
-    : video_source_(source),
+    : render_thread_message_loop_(base::MessageLoopProxy::current()),
+      libjingle_worker_thread_(libjingle_worker_thread),
+      video_source_(source),
       capture_adapter_(capture_adapter) {
   io_thread_checker_.DetachFromThread();
 }
 
 WebRtcVideoTrackAdapter::WebRtcVideoSourceAdapter::~WebRtcVideoSourceAdapter() {
+  // Since frames are posted to the worker thread, this object might be deleted
+  // on that thread. However, since |video_source_| was created on the render
+  // thread, we should release our reference on the render thread. This is not

[tommi (sloooow) - chröme, 2014/05/12 14:20:41]

nit: s/should/must

+  // strictly necessary since both the main render thread and the worker thread
+  // implements libjingle's thread Send method.

[tommi (sloooow) - chröme, 2014/05/12 14:20:41]

nit: implement
(no s)

After reading why this isn't strictly necessary, I still don't understand why it
isn't strictly necessary.  Is it because you have proxy to the source at this
point and the proxy will do the right thing somehow? (I guess I'm not groking
how a couple of threads implementing Send() somehow make this particular case
safe)

[perkj_chrome, 2014/05/12 15:39:51]

ok- I removed the whole comment instead since we don't want to rely on it
anyway. It is ok since each libjingle has a proxy that ensures that all methods
are called on the signaling thread. Including destruction. But for that to work,
the originating thread must implement Send. 

+  if (!render_thread_message_loop_->BelongsToCurrentThread()) {

[tommi (sloooow) - chröme, 2014/05/12 14:20:41]

I think that if at all possible to avoid this, we should.

Having an inconsistent teardown sequence can be very hard to troubleshoot.  I'd
like to keep the object being deleted on the same thread and avoid checks like
these.

[perkj_chrome, 2014/05/12 15:39:51]

Suggestions? We have 3 - threads here. Render thread that sets it all up. IO
thread where video frames are received on. WT where we want to deliver frames
on. 

All objects should be released on the render thread
video_source_ own the capturer that received frames on the WT. So we must make
sure no frames are posted to the WT after
WebRtcVideoTrackAdapter::WebRtcVideoSourceAdapter is destroyed.

+    webrtc::VideoSourceInterface* source = video_source_.get();
+    source->AddRef();
+    video_source_ = NULL;
+    render_thread_message_loop_->PostTask(
+        FROM_HERE,
+        base::Bind(&ReleaseWebRtcSourceOnMainRenderThread,
+                   base::Unretained(source)));
+  }
 }
 
 void WebRtcVideoTrackAdapter::WebRtcVideoSourceAdapter::OnVideoFrameOnIO(
     const scoped_refptr<media::VideoFrame>& frame,
     const media::VideoCaptureFormat& format) {
   DCHECK(io_thread_checker_.CalledOnValidThread());
+  libjingle_worker_thread_->PostTask(

[perkj_chrome, 2014/05/12 15:39:51]

This is what might cause WebRtcVideoTrackAdapter::WebRtcVideoSourceAdapter to be
deleted on the WT.

+      FROM_HERE,
+      base::Bind(&WebRtcVideoSourceAdapter::OnVideoFrameOnWt,
+                 this, frame, format));
+}
+
+void WebRtcVideoTrackAdapter::WebRtcVideoSourceAdapter::OnVideoFrameOnWt(
+    const scoped_refptr<media::VideoFrame>& frame,
+    const media::VideoCaptureFormat& format) {
+  DCHECK(libjingle_worker_thread_->BelongsToCurrentThread());
   capture_adapter_->OnFrameCaptured(frame);
 }
 
 WebRtcVideoTrackAdapter::WebRtcVideoTrackAdapter(
     const blink::WebMediaStreamTrack& track,
     MediaStreamDependencyFactory* factory)
     : web_track_(track) {
   const blink::WebMediaConstraints& constraints =
       MediaStreamVideoTrack::GetVideoTrack(track)->constraints();
 
   bool is_screencast = ConstraintKeyExists(
       constraints, base::UTF8ToUTF16(kMediaStreamSource));
   WebRtcVideoCapturerAdapter* capture_adapter =
       factory->CreateVideoCapturer(is_screencast);
 
   // |video_source| owns |capture_adapter|
   scoped_refptr<webrtc::VideoSourceInterface> video_source(
       factory->CreateVideoSource(capture_adapter,
                                  track.source().constraints()));
 
   video_track_ = factory->CreateLocalVideoTrack(web_track_.id().utf8(),
                                                 video_source.get());
 
   video_track_->set_enabled(web_track_.isEnabled());
 
-  source_adapter_ = new WebRtcVideoSourceAdapter(video_source,
-                                                 capture_adapter);
+  source_adapter_ = new WebRtcVideoSourceAdapter(
+      factory->worker_thread_proxy(),
+      video_source,
+      capture_adapter);
 
   AddToVideoTrack(
       this,
       base::Bind(&WebRtcVideoSourceAdapter::OnVideoFrameOnIO,
                  source_adapter_),
       web_track_);
 
   DVLOG(3) << "WebRtcVideoTrackAdapter ctor() : is_screencast "
            << is_screencast;
 }
 
 WebRtcVideoTrackAdapter::~WebRtcVideoTrackAdapter() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DVLOG(3) << "WebRtcVideoTrackAdapter dtor().";
   RemoveFromVideoTrack(this, web_track_);
 }
 
 void WebRtcVideoTrackAdapter::OnEnabledChanged(bool enabled) {
   DCHECK(thread_checker_.CalledOnValidThread());
   video_track_->set_enabled(enabled);
 }
 
 }  // namespace content