Rotate frames correctly for back camera

media/capture/video/linux/camera_characteristics.cc

+// Copyright 2016 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 "camera_characteristics.h"
+
+#include <base/files/file_util.h>
+#include <base/logging.h>
+#include <base/strings/string_piece.h>
+#include <base/strings/string_split.h>
+
+namespace media {
+
+// /etc/camera/camera_characteristics.conf contains camera information which
+// driver cannot provide.
+static const char kCameraCharacteristicsConfigFile[] =
+    "/etc/camera/camera_characteristics.conf";
+static const char kLensFacing[] = "lens_facing";
+static const char kUsbVidPid[] = "usb_vid_pid";
+
+static const struct DeviceInfo kDefaultCharacteristics = {

[wuchengli, 2016/11/22 03:47:06]

not used?

[shenghao, 2016/11/24 07:13:03]

Done.

+    "",  // vid_pid
+    0,   // lens_facing
+};
+
+CameraCharacteristics::CameraCharacteristics() {}
+
+CameraCharacteristics::~CameraCharacteristics() {}
+
+int CameraCharacteristics::GetCameraFacing(const std::string& model_id) {
+  std::unordered_map<std::string, DeviceInfo> model_id_to_info =
+      GetDeviceInfoFromFile();
+  auto iter = model_id_to_info.find(model_id);
+  if (iter != model_id_to_info.end()) {
+    return iter->second.lens_facing;
+  }
+  return DeviceInfo::LENS_FACING_FRONT;
+}
+
+std::unordered_map<std::string, DeviceInfo>
+CameraCharacteristics::GetDeviceInfoFromFile() {
+  const base::FilePath path(kCameraCharacteristicsConfigFile);

[wuchengli, 2016/11/22 03:47:06]

We should not read the file every time a camera is opened. Make
CameraCharacteristics a LazyInstance and do GetDeviceInfoFromFile in the
constructor.

[shenghao, 2016/11/24 07:13:04]

Done.

+  std::string content;
+  if (!base::ReadFileToString(path, &content)) {
+    DLOG(ERROR) << "Config file bigger than buffer size.";

[wuchengli, 2016/11/22 07:59:33]

If the file doesn't exist, ReadFileToString also returns false. We should return
here.

if (!base::ReadFileToString(path, &content)) {
  DVLOG(1) << "ReadFileToString fails";
  return;
}

[shenghao, 2016/11/24 07:13:03]

Done.

+  }
+  std::vector<base::StringPiece> lines = base::SplitStringPiece(
+      base::StringPiece(content), base::StringPiece("\n"),

[kcwu, 2016/11/21 07:59:24]

IIRC, StringPiece will do casting implicitly, so you can just write
base::SplitStringPiece(content, "\n");

[shenghao, 2016/11/24 07:13:03]

Done.

+      base::WhitespaceHandling::TRIM_WHITESPACE,

[wuchengli, 2016/11/22 07:59:34]

Check if TRIM_WHITESPACE will remove "\r" if the config file uses \r\n as
newline.

[shenghao, 2016/11/24 07:13:03]

Yes.
https://cs.chromium.org/chromium/src/base/strings/string_util_constants.cc?gs...

+      base::SplitResult::SPLIT_WANT_NONEMPTY);
+
+  int current_lens_facing = DeviceInfo::LENS_FACING_FRONT;
+  std::unordered_map<std::string, DeviceInfo> device_info;
+  for (base::StringPiece line : lines) {

[wuchengli, 2016/11/22 07:59:34]

// Ignore the comment that starts with "#".

[shenghao, 2016/11/24 07:13:04]

Done.

+    if (line.starts_with(base::StringPiece("#"))) {
+      continue;
+    }
+    std::vector<base::StringPiece> key_value = base::SplitStringPiece(
+        line, base::StringPiece("="), base::WhitespaceHandling::TRIM_WHITESPACE,
+        base::SplitResult::SPLIT_WANT_ALL);
+    if (key_value.size() != 2) {
+      DLOG(ERROR) << "Invalid line in config file: " << line;

[wuchengli, 2016/11/22 07:59:33]

LOG(ERROR) because this is an actual error. We need to fix it when it happens.

[shenghao, 2016/11/24 07:13:03]

Done.

+      continue;
+    }
+    base::StringPiece key = key_value[0];

[kcwu, 2016/11/21 07:59:24]

const auto& key = key_value[0];
const auto& value = key_value[1];

[shenghao, 2016/11/24 07:13:03]

Done.

+    base::StringPiece value = key_value[1];
+    std::vector<base::StringPiece> sub_keys = base::SplitStringPiece(

[kcwu, 2016/11/21 07:59:24]

the value of sub_keys[0] is ignored?

[shenghao, 2016/11/24 07:13:04]

deleted

+        key, base::StringPiece("."), base::WhitespaceHandling::TRIM_WHITESPACE,
+        base::SplitResult::SPLIT_WANT_ALL);
+    if (sub_keys.size() >= 2 && sub_keys[1].compare(kLensFacing) == 0) {

[kcwu, 2016/11/21 07:59:24]

1. why ">=" instead of "==" ? Do you intent to ignore sub_keys[2:] silently?
2. Could it use "==" instead of compare() ?

+      current_lens_facing = strtol(value.data(), NULL, 10);

[kcwu, 2016/11/21 07:59:24]

base::StringToUint or the like?

[shenghao, 2016/11/24 07:13:03]

Done.

+      continue;
+    }

[wuchengli, 2016/11/22 07:59:33]

else if (sub_keys.size() >= 3 && sub_keys[2].compare(kUsbVidPid) == 0) {

[shenghao, 2016/11/24 07:13:03]

Done.

+    if (sub_keys.size() >= 3 && sub_keys[2].compare(kUsbVidPid) == 0) {

[kcwu, 2016/11/21 07:59:24]

ditto

[shenghao, 2016/11/24 07:13:03]

Done.

+      std::string vid_pid = value.as_string();
+      DeviceInfo info;
+      info.vid_pid = vid_pid;
+      info.lens_facing = current_lens_facing;

[kcwu, 2016/11/21 07:59:24]

You need to reset current_lens_facing when parsing different model. For example,

camera0.lens_facing=1
camera0.module0.usb_vid_pic=foo:1
camera1.module0.usb_vid_pic=foo:2

foo:2 will have the same current_lens_facing carrying from foo:1.

[shenghao, 2016/11/24 07:13:03]

Done.

+      device_info[vid_pid] = info;
+    }
+  }

[kcwu, 2016/11/21 07:59:24]

what about else? DLOG(ERROR) ?

[wuchengli, 2016/11/22 07:59:33]

No need to log. We ignore keys like lens_info_available_focal_lengths.

[shenghao, 2016/11/24 07:13:04]

Acknowledged.

+
+  return device_info;
+}
+
+}  // namespace media