Move VideoDestinationHandler processing to the IO-thread.

content/renderer/media/webrtc/video_destination_handler.cc

 // Copyright (c) 2013 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/video_destination_handler.h"
 
 #include <string>
 
 #include "base/base64.h"
 #include "base/debug/trace_event.h"
@@ skipping 11 matching lines @@
 #include "third_party/WebKit/public/web/WebMediaStreamRegistry.h"
 #include "third_party/libyuv/include/libyuv/convert.h"
 #include "url/gurl.h"
 
 namespace content {
 
 class PpFrameWriter::FrameWriterDelegate
     : public base::RefCountedThreadSafe<FrameWriterDelegate> {
  public:
   FrameWriterDelegate(
-      const scoped_refptr<base::MessageLoopProxy>& io_message_loop_proxy,
-      const VideoCaptureDeliverFrameCB& new_frame_callback);
+      const scoped_refptr<base::MessageLoopProxy>& io_message_loop_proxy);
 
-  void DeliverFrame(const scoped_refptr<media::VideoFrame>& frame,
-                    const media::VideoCaptureFormat& format);
+  // Starts forwarding frames to |frame_callback| on the  IO-thread that are
+  // delivered to this class by calling DeliverFrame on the main render thread.
+  void StartDeliver(const VideoCaptureDeliverFrameCB& frame_callback);
+  void StopDeliver();
+
+  void DeliverFrame(const scoped_refptr<PPB_ImageData_Impl>& image_data,
+                    int64 time_stamp_ns);
+
  private:
   friend class base::RefCountedThreadSafe<FrameWriterDelegate>;
   virtual ~FrameWriterDelegate();
 
-  void DeliverFrameOnIO(const scoped_refptr<media::VideoFrame>& frame,
-                        const media::VideoCaptureFormat& format);
+  void DeliverFrameOnIO(const VideoCaptureDeliverFrameCB& frame_callback,
+                        uint8* data, int stride, int width, int height,
+                        int64 time_stamp_ns);
+  void FrameDelivered(const scoped_refptr<PPB_ImageData_Impl>& image_data);
 
   scoped_refptr<base::MessageLoopProxy> io_message_loop_;
   VideoCaptureDeliverFrameCB new_frame_callback_;
+
+  // |frame_pool_| is only used on the IO-thread.
+  media::VideoFramePool frame_pool_;
+
+  // Used to DCHECK that we are called on the main render thread.
+  base::ThreadChecker thread_checker_;
 };
 
 PpFrameWriter::FrameWriterDelegate::FrameWriterDelegate(
-    const scoped_refptr<base::MessageLoopProxy>& io_message_loop_proxy,
-    const VideoCaptureDeliverFrameCB& new_frame_callback)
-    : io_message_loop_(io_message_loop_proxy),
-      new_frame_callback_(new_frame_callback) {
+    const scoped_refptr<base::MessageLoopProxy>& io_message_loop_proxy)
+    : io_message_loop_(io_message_loop_proxy) {
 }
 
 PpFrameWriter::FrameWriterDelegate::~FrameWriterDelegate() {
 }
 
-void PpFrameWriter::FrameWriterDelegate::DeliverFrame(
-    const scoped_refptr<media::VideoFrame>& frame,
-    const media::VideoCaptureFormat& format) {
-  io_message_loop_->PostTask(
-      FROM_HERE,
-      base::Bind(&FrameWriterDelegate::DeliverFrameOnIO,
-                 this, frame, format));
+void PpFrameWriter::FrameWriterDelegate::StartDeliver(
+    const VideoCaptureDeliverFrameCB& frame_callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  new_frame_callback_ = frame_callback;
 }
 
-void PpFrameWriter::FrameWriterDelegate::DeliverFrameOnIO(
-     const scoped_refptr<media::VideoFrame>& frame,
-     const media::VideoCaptureFormat& format) {
-  DCHECK(io_message_loop_->BelongsToCurrentThread());
-  // The local time when this frame is generated is unknown so give a null
-  // value to |estimated_capture_time|.
-  new_frame_callback_.Run(frame, format, base::TimeTicks());
+void PpFrameWriter::FrameWriterDelegate::StopDeliver() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  new_frame_callback_.Reset();
 }
 
-PpFrameWriter::PpFrameWriter() {
-  DVLOG(3) << "PpFrameWriter ctor";
-}
-
-PpFrameWriter::~PpFrameWriter() {
-  DVLOG(3) << "PpFrameWriter dtor";
-}
-
-void PpFrameWriter::GetCurrentSupportedFormats(
-    int max_requested_width,
-    int max_requested_height,
-    double max_requested_frame_rate,
-    const VideoCaptureDeviceFormatsCB& callback) {
-  DCHECK(CalledOnValidThread());
-  DVLOG(3) << "PpFrameWriter::GetCurrentSupportedFormats()";
-  // Since the input is free to change the resolution at any point in time
-  // the supported formats are unknown.
-  media::VideoCaptureFormats formats;
-  callback.Run(formats);
-}
-
-void PpFrameWriter::StartSourceImpl(
-    const media::VideoCaptureFormat& format,
-    const VideoCaptureDeliverFrameCB& frame_callback) {
-  DCHECK(CalledOnValidThread());
-  DCHECK(!delegate_.get());
-  DVLOG(3) << "PpFrameWriter::StartSourceImpl()";
-  delegate_ = new FrameWriterDelegate(io_message_loop(), frame_callback);
-  OnStartDone(MEDIA_DEVICE_OK);
-}
-
-void PpFrameWriter::StopSourceImpl() {
-  DCHECK(CalledOnValidThread());
-}
-
-void PpFrameWriter::PutFrame(PPB_ImageData_Impl* image_data,
-                             int64 time_stamp_ns) {
-  DCHECK(CalledOnValidThread());
-  TRACE_EVENT0("video", "PpFrameWriter::PutFrame");
-  DVLOG(3) << "PpFrameWriter::PutFrame()";
-
-  if (!image_data) {
-    LOG(ERROR) << "PpFrameWriter::PutFrame - Called with NULL image_data.";
+void PpFrameWriter::FrameWriterDelegate::DeliverFrame(
+    const scoped_refptr<PPB_ImageData_Impl>& image_data,
+    int64 time_stamp_ns) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  TRACE_EVENT0("video", "PpFrameWriter::FrameWriterDelegate::DeliverFrame");
+  if (new_frame_callback_.is_null())
     return;
-  }
-  ImageDataAutoMapper mapper(image_data);
-  if (!mapper.is_valid()) {
+  if (!image_data->Map()) {
     LOG(ERROR) << "PpFrameWriter::PutFrame - "
                << "The image could not be mapped and is unusable.";
     return;
   }
+
   const SkBitmap* bitmap = image_data->GetMappedBitmap();
   if (!bitmap) {
     LOG(ERROR) << "PpFrameWriter::PutFrame - "
                << "The image_data's mapped bitmap is NULL.";
     return;
   }
+  io_message_loop_->PostTaskAndReply(
+      FROM_HERE,
+      base::Bind(&FrameWriterDelegate::DeliverFrameOnIO, this,
+                 new_frame_callback_,

[tommi (sloooow) - chröme, 2014/10/08 09:11:24]

Are you sending a pointer to the callback to the IO thread?
It looks like the callback is otherwise only accessed on this thread
(thread_checker_) and there's no synchronization, so we would need to send a
copy here I think unless there's some magic happening here that I'm missing.

Alternatively, could the callback be touched only on the IO thread?

[perkj_chrome, 2014/10/08 12:20:18]

Changed so that StartDeliver post the cb to the io thread. 

+                 static_cast<uint8*>(bitmap->getPixels()),
+                 bitmap->rowBytes(),
+                 bitmap->width(),
+                 bitmap->height(),
+                 time_stamp_ns),
+      base::Bind(&FrameWriterDelegate::FrameDelivered, this,
+                 image_data));
+}
 
-  const gfx::Size frame_size(bitmap->width(), bitmap->height());
+void PpFrameWriter::FrameWriterDelegate::FrameDelivered(
+    const scoped_refptr<PPB_ImageData_Impl>& image_data) {
+  image_data->Unmap();

[tommi (sloooow) - chröme, 2014/10/08 09:11:24]

should there be a thread check in this function?

[perkj_chrome, 2014/10/08 12:20:18]

Done.

+}
 
-  if (state() != MediaStreamVideoSource::STARTED)
-    return;
+void PpFrameWriter::FrameWriterDelegate::DeliverFrameOnIO(
+    const VideoCaptureDeliverFrameCB& frame_callback,
+    uint8* data, int stride, int width, int height, int64 time_stamp_ns) {
+  DCHECK(io_message_loop_->BelongsToCurrentThread());
+  TRACE_EVENT0("video", "PpFrameWriter::FrameWriterDelegate::DeliverFrameOnIO");
+  const gfx::Size frame_size(width, height);
 
   const base::TimeDelta timestamp = base::TimeDelta::FromMicroseconds(
       time_stamp_ns / base::Time::kNanosecondsPerMicrosecond);
 
   // TODO(perkj): It would be more efficient to use I420 here. Using YV12 will
   // force a copy into a tightly packed I420 frame in
   // WebRtcVideoCapturerAdapter before the frame is delivered to libJingle.
   // crbug/359587.
   scoped_refptr<media::VideoFrame> new_frame =
       frame_pool_.CreateFrame(media::VideoFrame::YV12, frame_size,
                               gfx::Rect(frame_size), frame_size, timestamp);
   media::VideoCaptureFormat format(
       frame_size,
       MediaStreamVideoSource::kUnknownFrameRate,
       media::PIXEL_FORMAT_YV12);
 
-  libyuv::BGRAToI420(reinterpret_cast<uint8*>(bitmap->getPixels()),
-                     bitmap->rowBytes(),
+  libyuv::BGRAToI420(data,
+                     stride,
                      new_frame->data(media::VideoFrame::kYPlane),
                      new_frame->stride(media::VideoFrame::kYPlane),
                      new_frame->data(media::VideoFrame::kUPlane),
                      new_frame->stride(media::VideoFrame::kUPlane),
                      new_frame->data(media::VideoFrame::kVPlane),
                      new_frame->stride(media::VideoFrame::kVPlane),
                      frame_size.width(), frame_size.height());
 
-  delegate_->DeliverFrame(new_frame, format);
+  // The local time when this frame is generated is unknown so give a null
+  // value to |estimated_capture_time|.
+  frame_callback.Run(new_frame, format, base::TimeTicks());
 }
 
-// PpFrameWriterProxy is a helper class to make sure the user won't use
-// PpFrameWriter after it is released (IOW its owner - WebMediaStreamSource -
-// is released).
-class PpFrameWriterProxy : public FrameWriterInterface {
- public:
-  explicit PpFrameWriterProxy(const base::WeakPtr<PpFrameWriter>& writer)
-      : writer_(writer) {
-    DCHECK(writer_ != NULL);
-  }
+PpFrameWriter::PpFrameWriter() {
+  DVLOG(3) << "PpFrameWriter ctor";
+  delegate_ = new FrameWriterDelegate(io_message_loop());

[tommi (sloooow) - chröme, 2014/10/08 09:11:24]

initializer list?

[perkj_chrome, 2014/10/08 12:20:18]

I guess I can but I have a vague memory that you didnt like calling a base class
method in an initializer list?    io_message_loop()

[tommi (sloooow) - chröme, 2014/10/08 14:50:36]

oh, delegate_ is a base class member.  If so, leave as is.

+}
 
-  virtual ~PpFrameWriterProxy() {}
+PpFrameWriter::~PpFrameWriter() {
+  DVLOG(3) << "PpFrameWriter dtor";
+}
 
-  virtual void PutFrame(PPB_ImageData_Impl* image_data,
-                        int64 time_stamp_ns) OVERRIDE {
-    writer_->PutFrame(image_data, time_stamp_ns);
-  }
+VideoDestinationHandler::FrameWriterCallback
+PpFrameWriter::GetFrameWriterCallback() {
+  DCHECK(CalledOnValidThread());
+  return base::Bind(&PpFrameWriter::FrameWriterDelegate::DeliverFrame,
+                    delegate_);
+}
 
- private:
-  base::WeakPtr<PpFrameWriter> writer_;
+void PpFrameWriter::GetCurrentSupportedFormats(
+    int max_requested_width,
+    int max_requested_height,
+    double max_requested_frame_rate,
+    const VideoCaptureDeviceFormatsCB& callback) {
+  DCHECK(CalledOnValidThread());
+  DVLOG(3) << "PpFrameWriter::GetCurrentSupportedFormats()";
+  // Since the input is free to change the resolution at any point in time
+  // the supported formats are unknown.
+  media::VideoCaptureFormats formats;
+  callback.Run(formats);
+}
 
-  DISALLOW_COPY_AND_ASSIGN(PpFrameWriterProxy);
-};
+void PpFrameWriter::StartSourceImpl(
+    const media::VideoCaptureFormat& format,
+    const VideoCaptureDeliverFrameCB& frame_callback) {
+  DCHECK(CalledOnValidThread());
+  DVLOG(3) << "PpFrameWriter::StartSourceImpl()";
+  delegate_->StartDeliver(frame_callback);
+  OnStartDone(MEDIA_DEVICE_OK);
+}
+
+void PpFrameWriter::StopSourceImpl() {
+  DCHECK(CalledOnValidThread());
+  delegate_->StopDeliver();
+}
 
 bool VideoDestinationHandler::Open(
     MediaStreamRegistryInterface* registry,
     const std::string& url,
-    FrameWriterInterface** frame_writer) {
+    FrameWriterCallback* frame_writer) {
   DVLOG(3) << "VideoDestinationHandler::Open";
   blink::WebMediaStream stream;
   if (registry) {
     stream = registry->GetMediaStream(url);
   } else {
     stream =
         blink::WebMediaStreamRegistry::lookupMediaStreamDescriptor(GURL(url));
   }
   if (stream.isNull()) {
     LOG(ERROR) << "VideoDestinationHandler::Open - invalid url: " << url;
     return false;
   }
 
   // Create a new native video track and add it to |stream|.
   std::string track_id;
   // According to spec, a media stream source's id should be unique per
   // application. There's no easy way to strictly achieve that. The id
   // generated with this method should be unique for most of the cases but
   // theoretically it's possible we can get an id that's duplicated with the
   // existing sources.
   base::Base64Encode(base::RandBytesAsString(64), &track_id);
 
   PpFrameWriter* writer = new PpFrameWriter();
+  *frame_writer = writer->GetFrameWriterCallback();
 
   // Create a new webkit video track.
   blink::WebMediaStreamSource webkit_source;
   blink::WebMediaStreamSource::Type type =
       blink::WebMediaStreamSource::TypeVideo;
   blink::WebString webkit_track_id = base::UTF8ToUTF16(track_id);
   webkit_source.initialize(webkit_track_id, type, webkit_track_id);
   webkit_source.setExtraData(writer);
 
   blink::WebMediaConstraints constraints;
   constraints.initialize();
   bool track_enabled = true;
 
   stream.addTrack(MediaStreamVideoTrack::CreateVideoTrack(
       writer, constraints, MediaStreamVideoSource::ConstraintsCallback(),
       track_enabled));
 
-  *frame_writer = new PpFrameWriterProxy(writer->AsWeakPtr());
   return true;
 }
 
 }  // namespace content