Fix crop bug in WebRtcVideoCapturerAdapter

content/renderer/media/webrtc/webrtc_video_capturer_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_capturer_adapter.h"
 
 #include "base/bind.h"
 #include "base/memory/aligned_memory.h"
 #include "base/trace_event/trace_event.h"
 #include "content/renderer/media/webrtc/webrtc_video_frame_adapter.h"
@@ skipping 118 matching lines @@
                   &crop_width, &crop_height, &crop_x, &crop_y,
                   &translated_camera_time_us)) {
     return;
   }
 
   // Return |frame| directly if it is texture backed, because there is no
   // cropping support for texture yet. See http://crbug/503653.
   // Return |frame| directly if it is GpuMemoryBuffer backed, as we want to
   // keep the frame on native buffers.
   if (frame->HasTextures() ||
-      frame->storage_type() ==
-      media::VideoFrame::STORAGE_GPU_MEMORY_BUFFERS) {
+      frame->storage_type() == media::VideoFrame::STORAGE_GPU_MEMORY_BUFFERS) {
     OnFrame(cricket::WebRtcVideoFrame(
                 new rtc::RefCountedObject<WebRtcVideoFrameAdapter>(frame),
                 webrtc::kVideoRotation_0, translated_camera_time_us),
             orig_width, orig_height);
     return;
   }
 
-  // Create a centered cropped visible rect that preservers aspect ratio for
-  // cropped natural size.
-  gfx::Rect visible_rect = frame->visible_rect();
-  visible_rect.ClampToCenteredSize(gfx::Size(
-      visible_rect.width() * adapted_width / orig_width,
-      visible_rect.height() * adapted_height / orig_height));
+  // Translate crop rectangle from natural size to visible size.
+  gfx::Rect cropped_visible_rect(
+      frame->visible_rect().x() +
+          crop_x * frame->visible_rect().width() / orig_width,
+      frame->visible_rect().y() +
+          crop_y * frame->visible_rect().height() / orig_height,
+      crop_width * frame->visible_rect().width() / orig_width,
+      crop_height * frame->visible_rect().height() / orig_height);
 
   const gfx::Size adapted_size(adapted_width, adapted_height);
   scoped_refptr<media::VideoFrame> video_frame =

[nisse-chromium (ooo August 14), 2016/09/09 09:06:57]

It would more readable to write

  frame = media::VideoFrame::WrapVideoFrame(frame, ...)

instead of introducing a new variable video_frame. But there are two uses of the
original value of frame below (frame->timestamp() and
Bind(&ReleaseOriginalFrame, frame), so perhaps that's not so easy.

[magjed_chromium, 2016/09/09 10:26:32]

I agree it's confusing with multiple frames, but I don't dare to touch this code
more than necessary for this fix :)

       media::VideoFrame::WrapVideoFrame(frame, frame->format(),
-                                        visible_rect, adapted_size);
+                                        cropped_visible_rect, adapted_size);
   if (!video_frame)
     return;
 
   video_frame->AddDestructionObserver(base::Bind(&ReleaseOriginalFrame, frame));
 
   // If no scaling is needed, return a wrapped version of |frame| directly.
   if (video_frame->natural_size() == video_frame->visible_rect().size()) {
     OnFrame(cricket::WebRtcVideoFrame(
                 new rtc::RefCountedObject<WebRtcVideoFrameAdapter>(video_frame),
                 webrtc::kVideoRotation_0, translated_camera_time_us),
@@ skipping 22 matching lines @@
                     scaled_frame->stride(media::VideoFrame::kVPlane),
                     adapted_width, adapted_height, libyuv::kFilterBilinear);
 
   OnFrame(cricket::WebRtcVideoFrame(
               new rtc::RefCountedObject<WebRtcVideoFrameAdapter>(scaled_frame),
               webrtc::kVideoRotation_0, translated_camera_time_us),
           orig_width, orig_height);
 }
 
 }  // namespace content