Use incoming frame's rotation while converting it to I420

content/renderer/media_recorder/video_track_recorder.cc

 // Copyright 2015 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_recorder/video_track_recorder.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/logging.h"
@@ skipping 34 matching lines @@
 
 using media::VideoFrame;
 using media::VideoFrameMetadata;
 using video_track_recorder::kVEAEncoderMinResolutionWidth;
 using video_track_recorder::kVEAEncoderMinResolutionHeight;
 
 namespace content {
 
 namespace {
 
+libyuv::RotationMode MediaVideoRotationToRotationMode(
+    media::VideoRotation rotation) {
+  switch (rotation) {
+    case media::VIDEO_ROTATION_0:
+      return libyuv::kRotate0;
+    case media::VIDEO_ROTATION_90:
+      return libyuv::kRotate90;
+    case media::VIDEO_ROTATION_180:
+      return libyuv::kRotate180;
+    case media::VIDEO_ROTATION_270:
+      return libyuv::kRotate270;
+  }

[emircan, 2017/04/07 19:50:55]

Add "NOTREACHED();". It makes sure that we will be notified if it ever comes
this section.

[Chandan, 2017/04/10 10:36:15]

Done.

+  return libyuv::kRotate0;
+}
+
 // HW encoders expect a nonzero bitrate, so |kVEADefaultBitratePerPixel| is used
 // to estimate bits per second for ~30 fps with ~1/16 compression rate.
 const int kVEADefaultBitratePerPixel = 2;
 // Number of output buffers used to copy the encoded data coming from HW
 // encoders.
 const int kVEAEncoderOutputBufferCount = 4;
 
 using CodecId = VideoTrackRecorder::CodecId;
 
 static const struct {
@@ skipping 231 matching lines @@
     // Send black frames (yuv = {0, 127, 127}).
     frame = media::VideoFrame::CreateColorFrame(
         video_frame->visible_rect().size(), 0u, 0x80, 0x80,
         video_frame->timestamp());
   } else {
     // Accelerated decoders produce ARGB/ABGR texture-backed frames (see
     // https://crbug.com/585242), fetch them using a SkCanvasVideoRenderer.
     DCHECK(video_frame->HasTextures());
     DCHECK_EQ(media::PIXEL_FORMAT_ARGB, video_frame->format());
 
-    frame = media::VideoFrame::CreateFrame(
-        media::PIXEL_FORMAT_I420, video_frame->coded_size(),
-        video_frame->visible_rect(), video_frame->natural_size(),
-        video_frame->timestamp());
+    gfx::Size coded_size = video_frame->coded_size(),
+              natural_size = video_frame->natural_size();
+    gfx::Rect visible_rect = video_frame->visible_rect();

[emircan, 2017/04/07 19:50:55]

You can do all these calculations based on visible rectangle size.

const gfx::Rect& old_visible_size = video_frame->visible_rect().size();
gfx::Rect new_visible_size = old_visible_size;
...
new_visible_size.set_width(old_visible_size.height());
new_visible_size.set_height(old_visible_size.width());
... 
frame = media::VideoFrame::CreateFrame(media::PIXEL_FORMAT_I420,
new_visible_size, gfx::Rect(new_visible_size), new_visible_size,
video_frame->timestamp());

[Chandan, 2017/04/10 10:36:15]

Done.

+    media::VideoRotation video_rotation = media::VIDEO_ROTATION_0;
+    if (video_frame->metadata()->GetRotation(
+            media::VideoFrameMetadata::ROTATION, &video_rotation)) {

[emircan, 2017/04/07 19:50:55]

Merge two if statements with &&.

[Chandan, 2017/04/10 10:36:15]

Done.

+      if (video_rotation == media::VIDEO_ROTATION_90 ||
+          video_rotation == media::VIDEO_ROTATION_270) {
+        coded_size.SetSize(coded_size.height(), coded_size.width());
+        natural_size.SetSize(natural_size.height(), natural_size.width());
+        visible_rect.set_width(visible_rect.height());
+        visible_rect.set_height(visible_rect.width());

[Chandan, 2017/04/07 13:21:55]

I am not too confident of this change. Please suggest.

[emircan, 2017/04/07 19:50:55]

I can totally understand, sorry for my late response. Here is a summary of what
those sizes stand for:
- coded_size: The memory footprint of the frame. 
- visible_rect: Actual size of the frame that is a subset of coded_size. For
instance, for a 571x265 frame, we might align memory to 572x266 for performance.
Then 572x266 would be the coded_size and 0x0,571x265 would be visible_rect.
- natural_size: visible_rect's size with aspect ratio adjustment.
https://cs.chromium.org/chromium/src/media/base/video_frame.h?rcl=9c785e56c8f...

In this particular case, we are only copying visible_rect's size portion of the
texture into an SKImage, see l.340. So, that is what we are interested in. 

[Chandan, 2017/04/10 10:36:15]

Thanks!

+      }
+    }
+
+    frame = media::VideoFrame::CreateFrame(media::PIXEL_FORMAT_I420, coded_size,
+                                           visible_rect, natural_size,
+                                           video_frame->timestamp());
 
     const SkImageInfo info = SkImageInfo::MakeN32(
         frame->visible_rect().width(), frame->visible_rect().height(),
         kOpaque_SkAlphaType);
 
     // Create |surface_| if it doesn't exist or incoming resolution has changed.
     if (!canvas_ || canvas_->imageInfo().width() != info.width() ||
         canvas_->imageInfo().height() != info.height()) {
       bitmap_.allocPixels(info);
       canvas_ = base::MakeUnique<cc::SkiaPaintCanvas>(bitmap_);
     }
     if (!video_renderer_)
       video_renderer_.reset(new media::SkCanvasVideoRenderer);
 
     DCHECK(context_provider->ContextGL());
     video_renderer_->Copy(video_frame.get(), canvas_.get(),
                           media::Context3D(context_provider->ContextGL(),
                                            context_provider->GrContext()));
 
     SkPixmap pixmap;
     if (!bitmap_.peekPixels(&pixmap)) {
       DLOG(ERROR) << "Error trying to map PaintSurface's pixels";
       return;
     }
-    // TODO(mcasas): Use the incoming frame's rotation when
-    // https://bugs.chromium.org/p/webrtc/issues/detail?id=6069 is closed.
-    const libyuv::RotationMode source_rotation = libyuv::kRotate0;
+
+    libyuv::RotationMode source_rotation = libyuv::kRotate0;
+    if (video_rotation != media::VIDEO_ROTATION_0)
+      source_rotation = MediaVideoRotationToRotationMode(video_rotation);
+
     const uint32 source_pixel_format =
         (kN32_SkColorType == kRGBA_8888_SkColorType) ? libyuv::FOURCC_ABGR
                                                      : libyuv::FOURCC_ARGB;
     if (libyuv::ConvertToI420(static_cast<uint8*>(pixmap.writable_addr()),
                               pixmap.getSafeSize(),
                               frame->visible_data(media::VideoFrame::kYPlane),
                               frame->stride(media::VideoFrame::kYPlane),
                               frame->visible_data(media::VideoFrame::kUPlane),
                               frame->stride(media::VideoFrame::kUPlane),
                               frame->visible_data(media::VideoFrame::kVPlane),
                               frame->stride(media::VideoFrame::kVPlane),
                               0 /* crop_x */, 0 /* crop_y */,
                               pixmap.width(), pixmap.height(),
                               frame->visible_rect().width(),

[emircan, 2017/04/07 19:50:55]

Use |old_visible_rect| here as well.

[Chandan, 2017/04/10 10:36:15]

Done.

                               frame->visible_rect().height(),
                               source_rotation,
                               source_pixel_format) != 0) {
       DLOG(ERROR) << "Error converting frame to I420";
       return;
     }
   }
 
   encoding_task_runner_->PostTask(
       FROM_HERE, base::Bind(&Encoder::EncodeOnEncodingTaskRunner, this, frame,
@@ skipping 1004 matching lines @@
                                           false /*allow_vea_encoder*/)),
       false);
 }
 
 bool VideoTrackRecorder::CanEncodeAlphaChannelForTesting() {
   DCHECK(encoder_);
   return encoder_->CanEncodeAlphaChannel();
 }
 
 }  // namespace content