Video capture: Fix camera rotation for Windows devices locked in

non-default orientation

Problem: When Windows tablet devices are locked in a non-default
orientation, the video capture stream is incorrectly rotated.

Cause: We always return a camera rotation of 0 when auto-rotation is
turned off. For devices locked in non-default orientations this leads
to incorrect camera rotation.

Fix: This change removes the IsAutoRotationEnabled() check inside
GetCameraRotation(). It also adds a new base::Optional<int> member
camera_rotation_ in video_capture_device_mf_win.cc and
video_capture_device_win.cc.

When a frame is received, if camera_rotation_ doesn't have a value OR
auto rotation is enabled, we call GetCameraRotation() and cache the
non-negative returned value in camera_rotation_. This enables us to
calculate and save the correct camera rotation when the first frame is
received. Further, if auto rotation is turned off during the capture
session, we'll be able to use the cached value to keep the
camera correctly rotated.

Testing: I added a test in video_capture_device_mf_win_unittest.cc to
verify that GetCameraRotation() is called and |camera_rotation_| is
populated when MFVideoCallback::OnSample() is invoked.

I also tested this change with both a naturally landscape and a
naturally portrait Windows tablet device locked in all possible
orientations. In each case the camera was correctly rotated.

Bug: 1134801
Change-Id: Ib1b6f1a4bb388b1390e5a257686c3b8a417a2a9c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2447036Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Rahul Singh <rahsin@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#815223}

media/capture/BUILD.gn

@@ -494,6 +494,8 @@ test("capture_unittests") {
 
     # TODO(jschuh): https://crbug.com/167187 fix size_t to int truncations.
     configs += [ "//build/config/compiler:no_size_t_to_int_warning" ]
+
+    deps += [ "//media/base/win:media_foundation_util" ]
   }
 
   # TODO(https://crbug.com/1043007): use is_linux.

media/capture/video/win/video_capture_device_mf_win.cc

@@ -1362,12 +1362,17 @@ void VideoCaptureDeviceMFWin::OnIncomingCapturedData(
       client_->OnStarted();
     }
 
+    // We always calculate camera rotation for the first frame. We also cache
+    // the latest value to use when AutoRotation is turned off.
+    if (!camera_rotation_.has_value() || IsAutoRotationEnabled())
+      camera_rotation_ = GetCameraRotation(facing_mode_);
+
     // TODO(julien.isorce): retrieve the color space information using Media
     // Foundation api, MFGetAttributeSize/MF_MT_VIDEO_PRIMARIES,in order to
     // build a gfx::ColorSpace. See http://crbug.com/959988.
     client_->OnIncomingCapturedData(
         data, length, selected_video_capability_->supported_format,
-        gfx::ColorSpace(), GetCameraRotation(facing_mode_), false /* flip_y */,
+        gfx::ColorSpace(), camera_rotation_.value(), false /* flip_y */,
         reference_time, timestamp);
   }
 

media/capture/video/win/video_capture_device_mf_win.h

@@ -20,6 +20,7 @@
 
 #include "base/callback_forward.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/sequence_checker.h"
 #include "media/capture/capture_export.h"
 #include "media/capture/video/video_capture_device.h"
@@ -103,6 +104,8 @@ class CAPTURE_EXPORT VideoCaptureDeviceMFWin : public VideoCaptureDevice {
     dxgi_device_manager_ = std::move(dxgi_device_manager);
   }
 
+  base::Optional<int> camera_rotation() const { return camera_rotation_; }
+
  private:
   HRESULT ExecuteHresultCallbackWithRetries(
       base::RepeatingCallback<HRESULT()> callback,
@@ -159,6 +162,7 @@ class CAPTURE_EXPORT VideoCaptureDeviceMFWin : public VideoCaptureDevice {
   base::WaitableEvent capture_initialize_;
   base::WaitableEvent capture_error_;
   scoped_refptr<VideoCaptureDXGIDeviceManager> dxgi_device_manager_;
+  base::Optional<int> camera_rotation_;
 
   SEQUENCE_CHECKER(sequence_checker_);
 

media/capture/video/win/video_capture_device_mf_win_unittest.cc

@@ -14,6 +14,7 @@
 #include "base/bind_helpers.h"
 #include "base/test/task_environment.h"
 #include "base/win/windows_version.h"
+#include "media/base/win/mf_helpers.h"
 #include "media/capture/video/win/sink_filter_win.h"
 #include "media/capture/video/win/video_capture_device_factory_win.h"
 #include "media/capture/video/win/video_capture_device_mf_win.h"
@@ -44,6 +45,8 @@ constexpr long kVideoProcAmpMinBase = -50;
 constexpr long kVideoProcAmpMaxBase = 50;
 constexpr long kVideoProcAmpStep = 1;
 
+constexpr uint32_t kMFSampleBufferLength = 1;
+
 class MockClient : public VideoCaptureDevice::Client {
  public:
   void OnIncomingCapturedData(const uint8_t* data,
@@ -1229,6 +1232,24 @@ TEST_F(VideoCaptureDeviceMFWinTest, StartPreviewOnAllocateAndStart) {
   device_->StopAndDeAllocate();
 }
 
+// Expects device's |camera_rotation_| to be populated after first OnSample().
+TEST_F(VideoCaptureDeviceMFWinTest, PopulateCameraRotationOnSample) {
+  if (ShouldSkipTest())
+    return;
+
+  PrepareMFDeviceWithOneVideoStream(MFVideoFormat_MJPG);
+
+  EXPECT_CALL(*(engine_.Get()), OnStartPreview());
+  EXPECT_CALL(*client_, OnStarted());
+
+  device_->AllocateAndStart(VideoCaptureParams(), std::move(client_));
+  // Create a valid IMFSample to use with the callback.
+  Microsoft::WRL::ComPtr<IMFSample> test_sample =
+      CreateEmptySampleWithBuffer(kMFSampleBufferLength, 0);
+  capture_preview_sink_->sample_callback->OnSample(test_sample.Get());
+  EXPECT_TRUE(device_->camera_rotation().has_value());
+}
+
 // Expects OnError() to be called on an errored IMFMediaEvent
 TEST_F(VideoCaptureDeviceMFWinTest, CallClientOnErrorMediaEvent) {
   if (ShouldSkipTest())

media/capture/video/win/video_capture_device_utils_win.cc

@@ -82,10 +82,6 @@ double PlatformExposureTimeToCaptureStep(long log_step,
 int GetCameraRotation(VideoFacingMode facing) {
   int rotation = 0;
 
-  if (!IsAutoRotationEnabled()) {
-    return rotation;
-  }
-
   // Before Win10, we can't distinguish if the selected camera is an internal or
   // external one. So we assume it's internal and do the frame rotation if the
   // auto rotation is enabled to cover most user cases.

media/capture/video/win/video_capture_device_win.cc

@@ -857,6 +857,11 @@ void VideoCaptureDeviceWin::FrameReceived(const uint8_t* buffer,
   if (timestamp == kNoTimestamp)
     timestamp = base::TimeTicks::Now() - first_ref_time_;
 
+  // We always calculate camera rotation for the first frame. We also cache the
+  // latest value to use when AutoRotation is turned off.
+  if (!camera_rotation_.has_value() || IsAutoRotationEnabled())
+    camera_rotation_ = GetCameraRotation(device_descriptor_.facing);
+
   // TODO(julien.isorce): retrieve the color space information using the
   // DirectShow api, AM_MEDIA_TYPE::VIDEOINFOHEADER2::dwControlFlags. If
   // AMCONTROL_COLORINFO_PRESENT, then reinterpret dwControlFlags as a
@@ -864,8 +869,8 @@ void VideoCaptureDeviceWin::FrameReceived(const uint8_t* buffer,
   // DXVA_VideoTransferMatrix, DXVA_VideoTransferFunction and
   // DXVA_NominalRangeto build a gfx::ColorSpace. See http://crbug.com/959992.
   client_->OnIncomingCapturedData(buffer, length, format, gfx::ColorSpace(),
-                                  GetCameraRotation(device_descriptor_.facing),
-                                  flip_y, base::TimeTicks::Now(), timestamp);
+                                  camera_rotation_.value(), flip_y,
+                                  base::TimeTicks::Now(), timestamp);
 
   while (!take_photo_callbacks_.empty()) {
     TakePhotoCallback cb = std::move(take_photo_callbacks_.front());

media/capture/video/win/video_capture_device_win.h

@@ -20,6 +20,7 @@
 
 #include "base/containers/queue.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/threading/thread_checker.h"
 #include "media/capture/video/video_capture_device.h"
 #include "media/capture/video/win/capability_list_win.h"
@@ -155,6 +156,8 @@ class VideoCaptureDeviceWin : public VideoCaptureDevice,
 
   bool enable_get_photo_state_;
 
+  base::Optional<int> camera_rotation_;
+
   DISALLOW_IMPLICIT_CONSTRUCTORS(VideoCaptureDeviceWin);
 };