Change MediaStreamVideoSource to output different resolutions to different tracks depending on the …

content/renderer/media/video_track_adapter.cc

Index: content/renderer/media/video_track_adapter.cc
diff --git a/content/renderer/media/video_track_adapter.cc b/content/renderer/media/video_track_adapter.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d9fa99a3a02b38e4d11171b22e1a2e5c5a9053ce
--- /dev/null
+++ b/content/renderer/media/video_track_adapter.cc
@@ -0,0 +1,292 @@
+// 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/video_track_adapter.h"
+
+#include <algorithm>
+#include <limits>
+#include <utility>
+
+#include "base/bind.h"
+#include "base/debug/trace_event.h"
+#include "base/location.h"
+#include "media/base/video_util.h"
+
+namespace {
+
+// Empty method used for keeping a reference to the original media::VideoFrame
+// in VideoFrameResolutionAdapter::DeliverFrame if cropping is needed.
+// The reference to |frame| is kept in the closure that calls this method.
+void ReleaseOriginalFrame(
+    const scoped_refptr<media::VideoFrame>& frame) {
+}
+
+}  // anonymous namespace
+
+namespace content {
+
+// VideoFrameResolutionAdapter is created on the main
+// render thread but operates on the IO-thread. It does the resolution
+// adaptation and delivers frames to all registered tracks on the IO-thread.

[mcasas, 2014/05/07 14:10:43]

This threading detail should go in the header file.

[perkj_chrome, 2014/05/08 11:29:47]

This class only exist as an internal class. So documentation related to this
class should be here. There is also documentation in the header file where I
forward declare this.

[mcasas, 2014/05/09 07:46:24]

After offline discussion, I buy the classes' division, but suggest
you could add a comment line on the multiplicity, i.e. "per MSVSource
there is at most one VideoTrackAdapter, and that contains up to
#MSVideoTracks objects of class VTA::VFResolutionAdapter's, depending
on the Constraints..." ?

+// All method calls except creation, must be  on the IO-thread.

[mcasas, 2014/05/07 14:10:43]

nit: double space after "must be"

[perkj_chrome, 2014/05/08 11:29:47]

Done.

+class VideoTrackAdapter::VideoFrameResolutionAdapter
+    : public base::RefCountedThreadSafe<VideoFrameResolutionAdapter> {
+ public:
+  VideoFrameResolutionAdapter(
+      int max_width,
+      int max_height,
+      double min_aspect_ratio,
+      double max_aspect_ratio);
+
+  // Add |callback| to receive video frames on the IO-thread.
+  void AddCallback(MediaStreamVideoTrack* track,
+                   const VideoCaptureDeliverFrameCB& callback);
+
+  // Removes |callback| associated with |id| from receiving video frames if
+  // |track| has been added. It is ok to call RemoveCallback even if the |id|
+  // has not been added.
+  void RemoveCallback(MediaStreamVideoTrack* track);
+
+  virtual void DeliverFrame(
+      const scoped_refptr<media::VideoFrame>& frame,
+      const media::VideoCaptureFormat& format);
+
+  // Returns true if all arguments match with the output of this adapter.
+  bool ConstraintsMatch(int max_width,
+                        int max_height,
+                        double min_aspect_ratio,
+                        double max_aspect_ratio) const;
+
+  bool IsEmpty() const { return callbacks_.empty(); }
+
+ private:
+  virtual ~VideoFrameResolutionAdapter();
+  friend class base::RefCountedThreadSafe<VideoFrameResolutionAdapter>;
+
+  virtual void DoDeliverFrame(
+      const scoped_refptr<media::VideoFrame>& frame,
+      const media::VideoCaptureFormat& format);
+
+  // Bound to the IO-thread.
+  base::ThreadChecker io_thread_checker_;
+
+  gfx::Size max_frame_size_;
+  double min_aspect_ratio_;
+  double max_aspect_ratio_;
+
+  typedef std::pair<void*, VideoCaptureDeliverFrameCB> VideoIdCallbackPair;
+  std::vector<VideoIdCallbackPair> callbacks_;

[mcasas, 2014/05/07 14:10:43]

std::list? Only used for push_back() and iterator-based search.

[perkj_chrome, 2014/05/08 11:29:47]

dito. 

+
+  DISALLOW_COPY_AND_ASSIGN(VideoFrameResolutionAdapter);
+};
+
+VideoTrackAdapter::
+VideoFrameResolutionAdapter::VideoFrameResolutionAdapter(
+    int max_width,
+    int max_height,
+    double min_aspect_ratio,
+    double max_aspect_ratio)
+    : max_frame_size_(max_width, max_height),
+      min_aspect_ratio_(min_aspect_ratio),
+      max_aspect_ratio_(max_aspect_ratio) {
+
+  DVLOG(3) << "VideoFrameResolutionAdapter("
+          << "{ max_width =" << max_width << "}, "
+          << "{ max_height =" << max_height << "}, "
+          << "{ min_aspect_ratio =" << min_aspect_ratio << "}, "
+          << "{ max_aspect_ratio_ =" << max_aspect_ratio_ << "}) ";
+}
+
+VideoTrackAdapter::
+VideoFrameResolutionAdapter::~VideoFrameResolutionAdapter() {
+  DCHECK(callbacks_.empty());
+}
+
+void VideoTrackAdapter::VideoFrameResolutionAdapter::DeliverFrame(
+    const scoped_refptr<media::VideoFrame>& frame,
+    const media::VideoCaptureFormat& format) {
+  DCHECK(io_thread_checker_.CalledOnValidThread());
+  // TODO(perkj): Allow cropping / scaling of textures once
+  // http://crbug/362521 is fixed.
+  if (frame->format() == media::VideoFrame::NATIVE_TEXTURE) {
+    DoDeliverFrame(frame, format);
+    return;
+  }
+  scoped_refptr<media::VideoFrame> video_frame(frame);

[mcasas, 2014/05/07 14:10:43]

Move to l.152?

[perkj_chrome, 2014/05/08 11:29:47]

line 169  DoDeliverFrame(video_frame, format) should be done regardless of the
if statement.

+  double input_ratio =
+      static_cast<double>(frame->natural_size().width()) /
+      frame->natural_size().height();
+
+  if (frame->natural_size().width() > max_frame_size_.width() ||

[mcasas, 2014/05/07 14:10:43]

Perhaps add comment here? Like: "If the input |frame|'s dimensions or aspect
ratio are out of the defined range then..."

[perkj_chrome, 2014/05/08 11:29:47]

Done.

+      frame->natural_size().height() > max_frame_size_.height() ||
+      input_ratio > max_aspect_ratio_ ||
+      input_ratio < min_aspect_ratio_) {
+    int desired_width = std::min(max_frame_size_.width(),

[mcasas, 2014/05/07 14:10:43]

I'd love to see some human-parseable comment on the micro algorithm used here,
like: Choose output letterbox dimensions such that both the aspect ratio range
and the max dimensions are respected blabla.

[perkj_chrome, 2014/05/08 11:29:47]

Done.

+                                 frame->natural_size().width());
+    int desired_height = std::min(max_frame_size_.height(),
+                                  frame->natural_size().height());
+
+    double resulting_ratio =
+        static_cast<double>(desired_width) / desired_height;
+    double requested_ratio = resulting_ratio;
+
+    if (requested_ratio > max_aspect_ratio_)
+      requested_ratio = max_aspect_ratio_;
+
+    if (requested_ratio < min_aspect_ratio_)

[mcasas, 2014/05/07 14:10:43]

else if? |requested_ratio_| should not be larger than max and smaller than min,
right?

Interesting to add somewhere:
DCHECK((max_aspect_ratio_ - min_aspect_ratio_) > 1E-1); ?

[perkj_chrome, 2014/05/08 11:29:47]

done in ctor

+      requested_ratio = min_aspect_ratio_;
+
+    if (resulting_ratio < requested_ratio) {
+      desired_height = static_cast<int>((desired_height * resulting_ratio) /
+                                        requested_ratio) + 1 & ~1;
+    } else if (resulting_ratio > requested_ratio) {
+      desired_width = static_cast<int>((desired_width * requested_ratio) /
+                                       resulting_ratio) + 1 & ~1;
+    }
+
+    gfx::Size desired_size(desired_width, desired_height);
+    gfx::Rect region_in_frame =
+          media::ComputeLetterboxRegion(frame->visible_rect(), desired_size);
+
+    video_frame = media::VideoFrame::WrapVideoFrame(
+        frame,
+        region_in_frame,
+        desired_size,
+        base::Bind(&ReleaseOriginalFrame, frame));
+
+    DVLOG(3) << "desired width " << desired_width
+             << " desired height " << desired_height
+             << " output natural width "
+             << video_frame->natural_size().width()

[mcasas, 2014/05/07 14:10:43]

nit: video_frame->natural_size() has a ToString() method that prints something
like axb.

[perkj_chrome, 2014/05/08 11:29:47]

Done.

+             << " output natural height "
+             << video_frame->natural_size().height()
+             << " output visible rect width "
+             << video_frame->visible_rect().width()
+             << " output visible rect height "
+             << video_frame->visible_rect().height();
+  }
+  DoDeliverFrame(video_frame, format);
+}
+
+void VideoTrackAdapter::
+VideoFrameResolutionAdapter::DoDeliverFrame(
+    const scoped_refptr<media::VideoFrame>& frame,
+    const media::VideoCaptureFormat& format) {
+  for (std::vector<VideoIdCallbackPair>::iterator it = callbacks_.begin();

[mcasas, 2014/05/07 14:10:43]

const_iterator?

[perkj_chrome, 2014/05/08 11:29:47]

Done.

+       it != callbacks_.end(); ++it) {
+    it->second.Run(frame, format);
+  }
+}
+
+void VideoTrackAdapter::VideoFrameResolutionAdapter::AddCallback(
+    MediaStreamVideoTrack* track,
+    const VideoCaptureDeliverFrameCB& callback) {
+  DCHECK(io_thread_checker_.CalledOnValidThread());
+  callbacks_.push_back(std::make_pair(track, callback));
+}
+
+void VideoTrackAdapter::VideoFrameResolutionAdapter::RemoveCallback(
+    MediaStreamVideoTrack* track) {
+  DCHECK(io_thread_checker_.CalledOnValidThread());
+  std::vector<VideoIdCallbackPair>::iterator it = callbacks_.begin();
+  for (; it != callbacks_.end(); ++it) {
+    if (it->first == track) {
+      callbacks_.erase(it);
+      return;
+    }
+  }
+}
+
+bool VideoTrackAdapter::VideoFrameResolutionAdapter::ConstraintsMatch(
+    int max_width,
+    int max_height,
+    double min_aspect_ratio,
+    double max_aspect_ratio) const {
+  return max_frame_size_.width() == max_width &&
+      max_frame_size_.height() == max_height &&
+      min_aspect_ratio_ == min_aspect_ratio &&
+      max_aspect_ratio_ == max_aspect_ratio;
+}
+
+VideoTrackAdapter::VideoTrackAdapter(
+    const scoped_refptr<base::MessageLoopProxy>& io_message_loop)
+    : io_message_loop_(io_message_loop) {
+}
+
+VideoTrackAdapter::~VideoTrackAdapter() {
+  DCHECK(adapters_.empty());
+}
+
+void VideoTrackAdapter::AddTrack(MediaStreamVideoTrack* track,

[mcasas, 2014/05/07 14:10:43]

The naked pointer stands out. Could we at least qualify it |const| ?

[perkj_chrome, 2014/05/08 11:29:47]

Done.

+                                 VideoCaptureDeliverFrameCB frame_callback,
+                                 int max_width,
+                                 int max_height,
+                                 double min_aspect_ratio,
+                                 double max_aspect_ratio) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  io_message_loop_->PostTask(
+      FROM_HERE,
+      base::Bind(&VideoTrackAdapter::AddTrackOnIO,
+                 this, track, frame_callback, max_width, max_height,
+                 min_aspect_ratio, max_aspect_ratio));
+}
+
+void VideoTrackAdapter::AddTrackOnIO(MediaStreamVideoTrack* track,
+                                     VideoCaptureDeliverFrameCB frame_callback,
+                                     int max_width,
+                                     int max_height,
+                                     double min_aspect_ratio,
+                                     double max_aspect_ratio) {
+  DCHECK(io_message_loop_->BelongsToCurrentThread());
+  scoped_refptr<VideoFrameResolutionAdapter> adapter;
+  for (FrameAdapters::iterator it = adapters_.begin(); it != adapters_.end();

[mcasas, 2014/05/07 14:10:43]

const_iterator?

[perkj_chrome, 2014/05/08 11:29:47]

Done.

[perkj_chrome, 2014/05/08 11:29:47]

Done.

+       ++it) {
+    if ((*it)->ConstraintsMatch(max_width, max_height, min_aspect_ratio,
+                                max_aspect_ratio)) {
+      adapter = it->get();
+      break;
+    }
+  }
+  if (!adapter) {
+    adapter = new VideoFrameResolutionAdapter(max_width,
+                                              max_height,
+                                              min_aspect_ratio,
+                                              max_aspect_ratio);
+    adapters_.push_back(adapter);
+  }
+
+  adapter->AddCallback(track, frame_callback);
+}
+
+void VideoTrackAdapter::RemoveTrack(MediaStreamVideoTrack* track) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  io_message_loop_->PostTask(
+      FROM_HERE,
+      base::Bind(&VideoTrackAdapter::RemoveTrackOnIO, this, track));
+}
+
+void VideoTrackAdapter::RemoveTrackOnIO(MediaStreamVideoTrack* track) {
+  DCHECK(io_message_loop_->BelongsToCurrentThread());
+  for (FrameAdapters::iterator it = adapters_.begin();
+       it != adapters_.end(); ++it) {
+    (*it)->RemoveCallback(track);
+    if ((*it)->IsEmpty()) {
+      adapters_.erase(it);
+      break;
+    }
+  }
+}
+
+void VideoTrackAdapter::DeliverFrameOnIO(
+    const scoped_refptr<media::VideoFrame>& frame,
+    const media::VideoCaptureFormat& format) {
+  DCHECK(io_message_loop_->BelongsToCurrentThread());
+  TRACE_EVENT0("video", "VideoTrackAdapter::DeliverFrameOnIO");
+  for (FrameAdapters::iterator it = adapters_.begin();
+       it != adapters_.end(); ++it) {
+    (*it)->DeliverFrame(frame, format);
+  }
+}
+
+}  // namespace content