Refactor scaling.

webrtc/common_video/include/video_frame_buffer.h

Index: webrtc/common_video/include/video_frame_buffer.h
diff --git a/webrtc/common_video/include/video_frame_buffer.h b/webrtc/common_video/include/video_frame_buffer.h
index 984bd50d674e78d7e7aedfc252fcfef8810b6963..08b914321a944b400f9350575fc5cfa1786547a5 100644
--- a/webrtc/common_video/include/video_frame_buffer.h
+++ b/webrtc/common_video/include/video_frame_buffer.h
@@ -109,6 +109,23 @@ class I420Buffer : public VideoFrameBuffer {
   static rtc::scoped_refptr<I420Buffer> Copy(
       const rtc::scoped_refptr<VideoFrameBuffer>& buffer);
 
+  // Only the most generally useful operations should be methods on
+  // this class, for more specialized operations, use
+  // MutableDataY(,U,V) to get write access.
+
+  // Crop the input buffer, then scale it to the size of this buffer.

[perkj_webrtc, 2016/06/02 09:25:36]

Can we rephrase this to make it easier to understand that this modifies |this|
and not |buffer|? 

[nisse-webrtc, 2016/06/02 12:17:50]

I'm trying rephrasing the comment, and renaming the argument buffer --> src.

Do you want me to rename it to CropAndScaleFrom too?

+  void CropAndScale(const rtc::scoped_refptr<VideoFrameBuffer>& buffer,
+                    int offset_x,
+                    int offset_y,
+                    int crop_width,

[perkj_webrtc, 2016/06/02 09:25:36]

When are these arguments usable? It seems like you use this by creating the
destination buffer first and then crop/scale to dst from src=buffer.
Can we just skip them and have this as CropAndScaleFrom(const
rtc::scoped_refptr<VideoFrameBuffer>& buffer) ? 

[nisse-webrtc, 2016/06/02 12:17:50]

To use this, one first creates a destination buffer of the desired size, using
either the new operator or I420BufferPool::CreateBuffer. And then one calls this
function with the src buffer and cropping parameters, the latter typically
chosen to get the same scale factor in both dimensions. 

We could drop cropping parameters and always to a center crop when needed to
avoid distorting the image. The reason I have them, is that I don't think this
is the right place to decide which portion of the image should be left out.

We discussed this a bit more off-line. It will simplify the interface if we let
this method decide on the appropriate cropping. However, the logic in
VideoAdapter (added by magjed) a few weeks back, is a bit hairy. And after a
second look, it tweaks both crop area size and destination size if needed to get
the scale factor exactly as desired. And this method really can't tweak the
destination size, and any caller using a buffer pool also need to know the
destination size exactly.

So I'd prefer to keep the crop_width and crop_height input arguments.

It is easier to get rid of the offset_x and offset_y, if desired. They can be
implied as (src->width() - crop_width) / 2. To me, it makes sense to have all
the crop parameters decided at the same place, but I don't have a very strong
opinion about that; currently nothing but center crop is ever used.

+                    int crop_height);
+
+  // The common case of a center crop, when needed to maintain the
+  // same aspect ratio.
+  static void CenterCropAndScale(
+      const rtc::scoped_refptr<I420Buffer>& dst,
+      const rtc::scoped_refptr<VideoFrameBuffer>& src);
+
  protected:
   ~I420Buffer() override;
 
@@ -186,13 +203,6 @@ class WrappedI420Buffer : public webrtc::VideoFrameBuffer {
   rtc::Callback0<void> no_longer_used_cb_;
 };
 
-// Helper function to crop |buffer| without making a deep copy. May only be used
-// for non-native frames.
-rtc::scoped_refptr<VideoFrameBuffer> ShallowCenterCrop(
-    const rtc::scoped_refptr<VideoFrameBuffer>& buffer,
-    int cropped_width,
-    int cropped_height);
-
 }  // namespace webrtc
 
 #endif  // WEBRTC_COMMON_VIDEO_INCLUDE_VIDEO_FRAME_BUFFER_H_