Rotate frames according to camera orientation

media/capture/video/linux/camera_config_chromeos.cc

Index: media/capture/video/linux/camera_config_chromeos.cc
diff --git a/media/capture/video/linux/camera_facing_chromeos.cc b/media/capture/video/linux/camera_config_chromeos.cc
similarity index 78%
rename from media/capture/video/linux/camera_facing_chromeos.cc
rename to media/capture/video/linux/camera_config_chromeos.cc
index ad54ed4185e757836431ca31c057b91fb4c3df1b..f4fa409bdacad7a0bf3146141ce46d867dd40ac6 100644
--- a/media/capture/video/linux/camera_facing_chromeos.cc
+++ b/media/capture/video/linux/camera_config_chromeos.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "camera_facing_chromeos.h"
+#include "camera_config_chromeos.h"
 
 #include <base/files/file_util.h>
 #include <base/logging.h>
@@ -20,7 +20,7 @@ namespace {
 // /etc/camera/camera_characteristics.conf.
 enum LensFacing { FRONT = 0, BACK = 1 };
 
-bool GetCameraId(const base::StringPiece& sub_key, int* camera_id) {
+bool ParseCameraId(const base::StringPiece& sub_key, int* camera_id) {
   const base::StringPiece camera_id_prefix = "camera";
   if (!sub_key.starts_with(camera_id_prefix))
     return false;
@@ -33,51 +33,72 @@ bool GetCameraId(const base::StringPiece& sub_key, int* camera_id) {
 static const char kCameraCharacteristicsConfigFile[] =
     "/etc/camera/camera_characteristics.conf";
 static const char kLensFacing[] = "lens_facing";
+static const char kSensorOrientation[] = "sensor_orientation";
 static const char kUsbVidPid[] = "usb_vid_pid";
 static const char kUsbPath[] = "usb_path";
+static const int kOrientationDefault = 0;
+static const int kCameraIdNotFound = -1;
 
-CameraFacingChromeOS::CameraFacingChromeOS() {
+CameraConfigChromeOS::CameraConfigChromeOS() {
   InitializeDeviceInfo(std::string(kCameraCharacteristicsConfigFile));
 }
 
-CameraFacingChromeOS::CameraFacingChromeOS(
+CameraConfigChromeOS::CameraConfigChromeOS(
     const std::string& config_file_path) {
   InitializeDeviceInfo(config_file_path);
 }
 
-CameraFacingChromeOS::~CameraFacingChromeOS() {}
+CameraConfigChromeOS::~CameraConfigChromeOS() {}
 
-VideoFacingMode CameraFacingChromeOS::GetCameraFacing(
+VideoFacingMode CameraConfigChromeOS::GetCameraFacing(
     const std::string& device_id,
     const std::string& model_id) const {
+  int camera_id = GetCameraId(device_id, model_id);
+  const auto& camera_id_to_facing_const = camera_id_to_facing_;
+  auto camera_id_iter = camera_id_to_facing_const.find(camera_id);
+  if (camera_id_iter == camera_id_to_facing_const.end()) {
+    DLOG(ERROR) << "Can't find lens_facing of camera ID " << camera_id
+                << " in config file";
+    return kLensFacingDefault;
+  }
+  return camera_id_iter->second;
+}
+
+int CameraConfigChromeOS::GetOrientation(const std::string& device_id,
+                                         const std::string& model_id) const {
+  int camera_id = GetCameraId(device_id, model_id);
+  const auto& camera_id_to_orientation_const = camera_id_to_orientation_;
+  auto camera_id_iter = camera_id_to_orientation_const.find(camera_id);

[mcasas, 2017/01/23 21:44:57]

You shouldn't encode the type of the variable
or its const-ness in the name, so just
s/camera_id_iter/camera_id/ and
s/camera_id_to_orientation_const/camera_id_to_orientation/

(that's usual in Win programming, but not in Chromium)

[shenghao, 2017/01/24 14:29:52]

Done.

+  if (camera_id_iter == camera_id_to_orientation_const.end()) {
+    DLOG(ERROR) << "Can't find sensor_orientation of camera ID " << camera_id
+                << " in config file";
+    return kOrientationDefault;
+  }
+  return camera_id_iter->second;
+}
+
+int CameraConfigChromeOS::GetCameraId(const std::string& device_id,
+                                      const std::string& model_id) const {
   std::string usb_id = GetUsbId(device_id);
   const auto& usb_id_to_camera_id_const = usb_id_to_camera_id_;
   const auto& model_id_to_camera_id_const = model_id_to_camera_id_;
-  const auto& camera_id_to_facing_const = camera_id_to_facing_;
   auto usb_id_iter = usb_id_to_camera_id_const.find(usb_id);
-  int camera_id;
+  int camera_id = 0;
   if (usb_id_iter == usb_id_to_camera_id_const.end()) {
     // Can't find Usb ID. Fall back to use model_id.

[mcasas, 2017/01/23 21:44:57]

s/model_id/|model_id|/

[shenghao, 2017/01/24 14:29:52]

Done.

     auto model_id_iter = model_id_to_camera_id_const.find(model_id);
     if (model_id_iter == model_id_to_camera_id_const.end()) {
       DLOG(ERROR) << "Can't find model ID in config file: " << model_id;
-      return kLensFacingDefault;
+      return kCameraIdNotFound;
     }
     camera_id = model_id_iter->second;
   } else {
     camera_id = usb_id_iter->second;
   }
-
-  auto camera_id_iter = camera_id_to_facing_const.find(camera_id);
-  if (camera_id_iter == camera_id_to_facing_const.end()) {
-    DLOG(ERROR) << "Can't find lens_facing of camera ID " << camera_id
-                << " in config file";
-    return kLensFacingDefault;
-  }
-  return camera_id_iter->second;
+  return camera_id;

[mcasas, 2017/01/23 21:44:57]

The code in this and GetOrientation() methods is hard
to read due to the nested if()s. Prefer early return and
do not define aliases just for constness, e.g.:

  const auto& usb_id =...
  if (usb_id != usb_id_to_camera_id_.end())
    return usb_id->second;

  const model_id = ...
  if (model_id != model_id_to_camera_id_.end())
    return usb_id->second;

  DLOG(ERROR) << "..."
  return kCameraIdNotFound;

[shenghao, 2017/01/24 14:29:52]

Done, except that I have to keep the const temporary variable. Otherwise find()
will become a non-const method, which conflicts the constness of GetCameraId()

 }
 
-std::string CameraFacingChromeOS::GetUsbId(const std::string& device_id) const {
+std::string CameraConfigChromeOS::GetUsbId(const std::string& device_id) const {
   // |device_id| is of the form "/dev/video2".  We want to retrieve "video2"
   // into |file_name|.
   const std::string device_dir = "/dev/";
@@ -113,7 +134,7 @@ std::string CameraFacingChromeOS::GetUsbId(const std::string& device_id) const {
   return usb_id_pieces[0].as_string();
 }
 
-void CameraFacingChromeOS::InitializeDeviceInfo(
+void CameraConfigChromeOS::InitializeDeviceInfo(
     const std::string& config_file_path) {
   const base::FilePath path(config_file_path);
   std::string content;
@@ -147,7 +168,7 @@ void CameraFacingChromeOS::InitializeDeviceInfo(
       continue;
     }
     int camera_id = 0;
-    if (!GetCameraId(sub_keys[0], &camera_id)) {
+    if (!ParseCameraId(sub_keys[0], &camera_id)) {
       DLOG(ERROR) << "Invalid sub key for camera id: " << sub_keys[0];
       continue;
     }
@@ -171,6 +192,13 @@ void CameraFacingChromeOS::InitializeDeviceInfo(
           DLOG(ERROR) << "Invalid value for lens_facing: " << lens_facing;
           continue;
       }
+    } else if (sub_keys.size() == 2 && sub_keys[1] == kSensorOrientation) {
+      int orientation = 0;
+      if (!base::StringToInt(value, &orientation)) {
+        DLOG(ERROR) << "Invalid value for sensor_orientation: " << value;
+        continue;
+      }
+      camera_id_to_orientation_[camera_id] = orientation;
     } else if (sub_keys.size() == 3 && sub_keys[2] == kUsbVidPid) {
       if (value.empty()) {
         DLOG(ERROR) << "model_id is empty";