[Video Capture, ChromeOS] Fix wrong rotation when starting capture with non-zero rotation

When starting video capture with a device that is rotated, e.g. a
convertible in portrait orientation, the rotation of the captured video
intermittently (most of the time) comes out wrong.

The root cause is that rotation events are ignored before capturing is
started but may end up arriving before.

This CL fixes this by storing the rotation in case rotation events are
received while capturing is not started.

Bug: 944215
Change-Id: I1346772341044c92be754d11d17a519397598df9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1540459Reviewed-by: Ricky Liang <jcliang@chromium.org>
Reviewed-by: Sheng-hao Tsao <shenghao@chromium.org>
Commit-Queue: Christian Fremerey <chfremer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#644783}

media/capture/video/linux/v4l2_capture_delegate.cc

@@ -234,7 +234,8 @@ V4L2CaptureDelegate::V4L2CaptureDelegate(
     V4L2CaptureDevice* v4l2,
     const VideoCaptureDeviceDescriptor& device_descriptor,
     const scoped_refptr<base::SingleThreadTaskRunner>& v4l2_task_runner,
-    int power_line_frequency)
+    int power_line_frequency,
+    int rotation)
     : v4l2_(v4l2),
       v4l2_task_runner_(v4l2_task_runner),
       device_descriptor_(device_descriptor),
@@ -242,7 +243,7 @@ V4L2CaptureDelegate::V4L2CaptureDelegate(
       device_fd_(v4l2),
       is_capturing_(false),
       timeout_count_(0),
-      rotation_(0),
+      rotation_(rotation),
       weak_factory_(this) {}
 
 void V4L2CaptureDelegate::AllocateAndStart(

media/capture/video/linux/v4l2_capture_delegate.h

@@ -51,7 +51,8 @@ class CAPTURE_EXPORT V4L2CaptureDelegate final {
       V4L2CaptureDevice* v4l2,
       const VideoCaptureDeviceDescriptor& device_descriptor,
       const scoped_refptr<base::SingleThreadTaskRunner>& v4l2_task_runner,
-      int power_line_frequency);
+      int power_line_frequency,
+      int rotation);
   ~V4L2CaptureDelegate();
 
   // Forward-to versions of VideoCaptureDevice virtual methods.

media/capture/video/linux/v4l2_capture_delegate_unittest.cc

@@ -182,7 +182,8 @@ class V4L2CaptureDelegateTest : public ::testing::Test {
             v4l2_.get(),
             device_descriptor_,
             base::ThreadTaskRunnerHandle::Get(),
-            50)) {}
+            50,
+            0)) {}
   ~V4L2CaptureDelegateTest() override = default;
 
   base::test::ScopedTaskEnvironment scoped_task_environment_;

media/capture/video/linux/video_capture_device_chromeos.cc

@@ -28,10 +28,12 @@ VideoCaptureDeviceChromeOS::VideoCaptureDeviceChromeOS(
           ScreenObserverDelegate::Create(this, ui_task_runner)) {}
 
 VideoCaptureDeviceChromeOS::~VideoCaptureDeviceChromeOS() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   screen_observer_delegate_->RemoveObserver();
 }
 
 void VideoCaptureDeviceChromeOS::SetRotation(int rotation) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (!camera_config_.rotates_with_device) {
     rotation = 0;
   } else if (camera_config_.lens_facing ==
@@ -77,6 +79,7 @@ void VideoCaptureDeviceChromeOS::SetRotation(int rotation) {
 
 void VideoCaptureDeviceChromeOS::SetDisplayRotation(
     const display::Display& display) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (display.IsInternal())
     SetRotation(display.rotation() * 90);
 }

media/capture/video/linux/video_capture_device_chromeos.h

@@ -51,8 +51,12 @@ class VideoCaptureDeviceChromeOS : public VideoCaptureDeviceLinux,
  private:
   // DisplayRotationObserver implementation.
   void SetDisplayRotation(const display::Display& display) override;
+
   const ChromeOSDeviceCameraConfig camera_config_;
   scoped_refptr<ScreenObserverDelegate> screen_observer_delegate_;
+
+  SEQUENCE_CHECKER(sequence_checker_);
+
   DISALLOW_IMPLICIT_CONSTRUCTORS(VideoCaptureDeviceChromeOS);
 };
 

media/capture/video/linux/video_capture_device_linux.cc

@@ -56,9 +56,11 @@ VideoCaptureDeviceLinux::VideoCaptureDeviceLinux(
     const VideoCaptureDeviceDescriptor& device_descriptor)
     : device_descriptor_(device_descriptor),
       v4l2_(std::move(v4l2)),
-      v4l2_thread_("V4L2CaptureThread") {}
+      v4l2_thread_("V4L2CaptureThread"),
+      rotation_(0) {}
 
 VideoCaptureDeviceLinux::~VideoCaptureDeviceLinux() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   // Check if the thread is running.
   // This means that the device has not been StopAndDeAllocate()d properly.
   DCHECK(!v4l2_thread_.IsRunning());
@@ -68,6 +70,7 @@ VideoCaptureDeviceLinux::~VideoCaptureDeviceLinux() {
 void VideoCaptureDeviceLinux::AllocateAndStart(
     const VideoCaptureParams& params,
     std::unique_ptr<VideoCaptureDevice::Client> client) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(!capture_impl_);
   if (v4l2_thread_.IsRunning())
     return;  // Wrong state.
@@ -77,7 +80,7 @@ void VideoCaptureDeviceLinux::AllocateAndStart(
       TranslatePowerLineFrequencyToV4L2(GetPowerLineFrequency(params));
   capture_impl_ = std::make_unique<V4L2CaptureDelegate>(
       v4l2_.get(), device_descriptor_, v4l2_thread_.task_runner(),
-      line_frequency);
+      line_frequency, rotation_);
   if (!capture_impl_) {
     client->OnError(VideoCaptureError::
                         kDeviceCaptureLinuxFailedToCreateVideoCaptureDelegate,
@@ -98,6 +101,7 @@ void VideoCaptureDeviceLinux::AllocateAndStart(
 }
 
 void VideoCaptureDeviceLinux::StopAndDeAllocate() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (!v4l2_thread_.IsRunning())
     return;  // Wrong state.
   v4l2_thread_.task_runner()->PostTask(
@@ -110,6 +114,7 @@ void VideoCaptureDeviceLinux::StopAndDeAllocate() {
 }
 
 void VideoCaptureDeviceLinux::TakePhoto(TakePhotoCallback callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(capture_impl_);
   auto functor =
       base::BindOnce(&V4L2CaptureDelegate::TakePhoto,
@@ -123,6 +128,7 @@ void VideoCaptureDeviceLinux::TakePhoto(TakePhotoCallback callback) {
 }
 
 void VideoCaptureDeviceLinux::GetPhotoState(GetPhotoStateCallback callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   auto functor =
       base::BindOnce(&V4L2CaptureDelegate::GetPhotoState,
                      capture_impl_->GetWeakPtr(), std::move(callback));
@@ -137,6 +143,7 @@ void VideoCaptureDeviceLinux::GetPhotoState(GetPhotoStateCallback callback) {
 void VideoCaptureDeviceLinux::SetPhotoOptions(
     mojom::PhotoSettingsPtr settings,
     SetPhotoOptionsCallback callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   auto functor = base::BindOnce(&V4L2CaptureDelegate::SetPhotoOptions,
                                 capture_impl_->GetWeakPtr(),
                                 std::move(settings), std::move(callback));
@@ -149,6 +156,8 @@ void VideoCaptureDeviceLinux::SetPhotoOptions(
 }
 
 void VideoCaptureDeviceLinux::SetRotation(int rotation) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  rotation_ = rotation;
   if (v4l2_thread_.IsRunning()) {
     v4l2_thread_.task_runner()->PostTask(
         FROM_HERE, base::BindOnce(&V4L2CaptureDelegate::SetRotation,

media/capture/video/linux/video_capture_device_linux.h

@@ -67,6 +67,13 @@ class VideoCaptureDeviceLinux : public VideoCaptureDevice {
 
   base::Thread v4l2_thread_;  // Thread used for reading data from the device.
 
+  // SetRotation() may get called even when the device is not started. When that
+  // is the case we remember the value here and use it as soon as the device
+  // gets started.
+  int rotation_;
+
+  SEQUENCE_CHECKER(sequence_checker_);
+
   DISALLOW_IMPLICIT_CONSTRUCTORS(VideoCaptureDeviceLinux);
 };