media: add video capture device for ARC++ camera HAL v3

media/capture/video/chromeos/camera_hal_delegate.cc

+// Copyright 2017 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 "media/capture/video/chromeos/camera_hal_delegate.h"
+
+#include <fcntl.h>
+#include <sys/uio.h>
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/strings/string_piece.h"
+#include "media/capture/video/chromeos/camera_metadata_utils.h"
+#include "media/capture/video/chromeos/video_capture_device_arc_chromeos.h"
+#include "mojo/edk/embedder/embedder.h"
+#include "mojo/edk/embedder/named_platform_handle.h"
+#include "mojo/edk/embedder/named_platform_handle_utils.h"
+#include "mojo/edk/embedder/pending_process_connection.h"
+#include "mojo/edk/embedder/platform_channel_pair.h"
+#include "mojo/edk/embedder/platform_channel_utils_posix.h"
+#include "mojo/edk/embedder/platform_handle_vector.h"
+
+namespace media {
+
+namespace {
+
+const base::StringPiece kArcCamera3SocketPath("/var/run/camera/camera3.sock");
+
+const base::TimeDelta kEventWaitTimeoutMs =
+    base::TimeDelta::FromMilliseconds(3000);
+
+}  // namespace
+
+CameraHalDelegate::CameraHalDelegate(
+    const scoped_refptr<base::SingleThreadTaskRunner> module_task_runner)
+    : camera_info_updated_(base::WaitableEvent::ResetPolicy::MANUAL,
+                           base::WaitableEvent::InitialState::NOT_SIGNALED),
+      num_cameras_(0),
+      module_callbacks_registered_(false),
+      module_task_runner_(module_task_runner),
+      camera_module_callbacks_(this) {

[wuchengli, 2017/05/02 16:11:42]

note to myself: this is run on browser UI thread.

+  thread_checker_.DetachFromThread();
+}
+
+CameraHalDelegate::~CameraHalDelegate() {

[wuchengli, 2017/05/02 16:11:41]

note to myself: CameraHalDelegate is singleton, created with
VideoCaptureDeviceFactory.

+  base::WaitableEvent interface_closed(
+      base::WaitableEvent::ResetPolicy::MANUAL,
+      base::WaitableEvent::InitialState::NOT_SIGNALED);
+  module_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&CameraHalDelegate::ResetMojoInterfaceOnModuleThread,
+                 base::Unretained(this), base::Unretained(&interface_closed)));
+  interface_closed.Wait();
+}
+
+bool CameraHalDelegate::StartCameraModuleIpc() {

[wuchengli, 2017/05/02 16:11:42]

note to myself: this is called on browser UI thread.

+  // Non-blocking socket handle.
+  mojo::edk::ScopedPlatformHandle socket_handle = mojo::edk::CreateClientHandle(
+      mojo::edk::NamedPlatformHandle(kArcCamera3SocketPath));
+
+  // Set socket to blocking
+  int flags = HANDLE_EINTR(fcntl(socket_handle.get().handle, F_GETFL));
+  if (flags == -1) {
+    LOG(ERROR) << "fcntl(F_GETFL) failed";

[wuchengli, 2017/05/02 16:11:42]

PLOG. Same for other places in this file.

[jcliang, 2017/05/03 04:44:56]

Done.

+    return false;
+  }
+  if (HANDLE_EINTR(fcntl(socket_handle.get().handle, F_SETFL,
+                         flags & ~O_NONBLOCK)) == -1) {
+    LOG(ERROR) << "fcntl(F_SETFL) failed";
+    return false;
+  }
+
+  char token[32] = {};
+  std::deque<mojo::edk::PlatformHandle> platform_handles;
+  ssize_t ret = mojo::edk::PlatformChannelRecvmsg(
+      socket_handle.get(), token, sizeof(token), &platform_handles, true);
+  if (ret == -1) {
+    LOG(ERROR) << "PlatformChannelRecvmsg failed: " << strerror(errno);
+  }
+  if (platform_handles.size() != 1) {
+    LOG(ERROR) << "Unexpected number of handles received, expected 1: "
+               << platform_handles.size();
+    return false;
+  }
+  mojo::edk::ScopedPlatformHandle parent_pipe(platform_handles.back());
+  platform_handles.pop_back();
+  if (!parent_pipe.is_valid()) {
+    LOG(ERROR) << "Invalid parent pipe";
+    return false;
+  }
+  mojo::edk::SetParentPipeHandle(std::move(parent_pipe));
+
+  mojo::ScopedMessagePipeHandle child_pipe =
+      mojo::edk::CreateChildMessagePipe(std::string(token, 32));
+
+  camera_module_ =
+      mojo::MakeProxy(mojo::InterfacePtrInfo<arc::mojom::CameraModule>(
+                          std::move(child_pipe), 0u),
+                      module_task_runner_);
+
+  VLOG(1) << "Camera module IPC connection established";
+
+  return true;
+}
+
+std::unique_ptr<VideoCaptureDevice> CameraHalDelegate::CreateDevice(
+    const scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner,
+    const VideoCaptureDeviceDescriptor& device_descriptor) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  std::unique_ptr<VideoCaptureDevice> capture_device(
+      new VideoCaptureDeviceArcChromeOS(ui_task_runner, device_descriptor,
+                                        this));
+  return capture_device;
+}
+
+void CameraHalDelegate::GetSupportedFormats(
+    const VideoCaptureDeviceDescriptor& device_descriptor,
+    VideoCaptureFormats* supported_formats) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  if (!UpdateCameraInfo()) {
+    return;
+  }
+  int camera_id = std::stoi(device_descriptor.device_id);
+  if (camera_info_.find(camera_id) == camera_info_.end() ||
+      camera_info_[camera_id].is_null()) {
+    // The camera may be removed already or is not ready yet.

[wuchengli, 2017/05/02 16:11:42]

How does the client handle the case that camera is not ready? Does the client
call GetSupportedFormats again later?

[jcliang, 2017/05/03 04:44:56]

I think Chrome would find the camera unusable, and would fail to use the camera
on this failed call. Subsequent attempt to use the camera will query the camera
info again.

+    VLOG(1) << "Invalid camera_id: " << camera_id;

[wuchengli, 2017/05/02 16:11:42]

How can the camera be removed already? If UpdateCameraInfo returns true in line
121, camera_info_ should still have the data (may be obsoleted though). Right?

[jcliang, 2017/05/03 04:44:55]

If the camera is not ready then camera_info_[camera_id] may be null, as
indicated in line 313.

This behavior corresponds to the HAL API specification where get_camera_info()
returns null if the HAL finds the camera not ready to accept any request (e.g.
the camera is plugged in but not hardware initialized yet, or the camera is
removed and the |camera_id| is invalid).

The above scenario is a race condition between the software and user
interaction, so it's not something we can avoid here.

+    return;
+  }
+  const arc::mojom::CameraInfoPtr& camera_info = camera_info_[camera_id];
+
+  const arc::mojom::CameraMetadataEntryPtr* fps_settings =
+      GetMetadataEntry(camera_info->static_camera_characteristics,
+                       arc::mojom::CameraMetadataTag::
+                           ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES);
+  if (!fps_settings) {
+    LOG(ERROR) << "Failed to get available target FPS ranges from camera info";
+    return;
+  }
+  // The available target FPS ranges are stored as pairs of two int32s:
+  // (min_fps, max_fps).
+  float max_fps = 0.0;
+  int32_t* iter = reinterpret_cast<int32_t*>((*fps_settings)->data.data());
+  // There may be multiple FPS ranges. We simply use the maximum FPS of all the

[wuchengli, 2017/05/02 16:11:42]

Do we want to use the maximum FPS of all FPS ranges here? The original video
capture device linux hard-codes to 30fps? What if we choose a 60FPS and
rendering is not smooth?

[jcliang, 2017/05/03 04:44:56]

The VideoCaptureDeviceFactoryLinux asks the kernel to enumerate all frame
intervals of a pixel format with VIDIOC_ENUM_FRAMEINTERVALS and reports all the
(pixel_format, frame_rate) combinations.

+  // FPS ranges here.
+  for (size_t i = 0; i < (*fps_settings)->count / sizeof(int32_t); ++i) {
+    float fps = static_cast<float>(*(iter + 1));
+    if (fps > max_fps) {
+      max_fps = fps;
+    }
+    iter += 2;
+  }
+
+  const arc::mojom::CameraMetadataEntryPtr* configurations =
+      GetMetadataEntry(camera_info->static_camera_characteristics,
+                       arc::mojom::CameraMetadataTag::
+                           ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS);
+  if (!configurations) {
+    LOG(ERROR)
+        << "Failed to get available stream configurations from camera info";
+    return;
+  }
+  // The available stream configurations are stored as tuples of four int32s:
+  // (hal_pixel_format, width, height, input_or_output_type).
+  iter = reinterpret_cast<int32_t*>((*configurations)->data.data());
+  for (size_t i = 0; i < (*configurations)->count / sizeof(int32_t); ++i) {
+    arc::mojom::HalPixelFormat format =
+        static_cast<arc::mojom::HalPixelFormat>(*iter);
+    int32_t width = *(iter + 1);
+    int32_t height = *(iter + 2);
+    int32_t type = *(iter + 3);
+    iter += 4;
+    VLOG(1) << "[" << std::hex << format << " " << std::dec << width << " "
+            << height << " " << type << "]";
+    VideoPixelFormat cr_format =
+        VideoCaptureDeviceArcChromeOS::PixFormatHalToChromium(format);
+    if (cr_format == PIXEL_FORMAT_UNKNOWN) {
+      continue;
+    }
+    VLOG(1) << "Supported format: " << width << "x" << height
+            << " fps=" << max_fps << " format=" << cr_format;
+    supported_formats->emplace_back(gfx::Size(width, height), max_fps,
+                                    cr_format);
+  }
+}
+
+void CameraHalDelegate::GetDeviceDescriptors(
+    VideoCaptureDeviceDescriptors* device_descriptors) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  if (!UpdateCameraInfo()) {
+    return;
+  }

[wuchengli, 2017/05/02 16:11:42]

Explain if a device is unplugged at this time, the client will get the old
device descriptor. The next CreateDevice should fail.

[jcliang, 2017/05/03 04:44:55]

Done.

+  for (size_t camera_id = 0; camera_id < num_cameras_; ++camera_id) {
+    VideoCaptureDeviceDescriptor desc;
+    arc::mojom::CameraInfoPtr& camera_info = camera_info_[camera_id];
+    if (!camera_info) {
+      continue;
+    }
+    desc.device_id = std::to_string(camera_id);
+    desc.capture_api = VideoCaptureApi::ANDROID_API2_LIMITED;
+    desc.transport_type = VideoCaptureTransportType::OTHER_TRANSPORT;
+    switch (camera_info->facing) {
+      case arc::mojom::CameraFacing::CAMERA_FACING_BACK:
+        desc.facing = VideoFacingMode::MEDIA_VIDEO_FACING_ENVIRONMENT;
+        desc.display_name = std::string("Back Camera");
+        break;
+      case arc::mojom::CameraFacing::CAMERA_FACING_FRONT:
+        desc.facing = VideoFacingMode::MEDIA_VIDEO_FACING_USER;
+        desc.display_name = std::string("Front Camera");
+        break;
+      case arc::mojom::CameraFacing::CAMERA_FACING_EXTERNAL:
+        desc.facing = VideoFacingMode::MEDIA_VIDEO_FACING_NONE;
+        desc.display_name = std::string("External Camera");
+        break;
+    }
+    device_descriptors->push_back(desc);
+  }
+}
+
+void CameraHalDelegate::GetCameraInfo(int32_t camera_id,
+                                      const GetCameraInfoCallback& callback) {
+  // This method may be called on any thread.  Currently this method is used by
+  // VideoCaptureDeviceArcChromeOS to query camera info.
+  module_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&CameraHalDelegate::GetCameraInfoOnModuleThread,
+                            this, camera_id, callback));
+}
+
+void CameraHalDelegate::OpenDevice(int32_t camera_id,
+                                   const OpenDeviceCallback& callback) {
+  // This method may be called on any thread.  Currently this method is used by
+  // VideoCaptureDeviceArcChromeOS to open a camera device.
+  module_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&CameraHalDelegate::OpenDeviceOnModuleThread, this,
+                            camera_id, callback));
+}
+
+void CameraHalDelegate::ResetMojoInterfaceOnModuleThread(
+    base::WaitableEvent* interface_closed) {
+  DCHECK(module_task_runner_->BelongsToCurrentThread());
+  camera_module_.reset();
+  if (camera_module_callbacks_.is_bound()) {
+    camera_module_callbacks_.Unbind();
+  }
+  interface_closed->Signal();
+}
+
+bool CameraHalDelegate::UpdateCameraInfo() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  if (camera_info_updated_.IsSignaled()) {
+    return true;
+  }
+  module_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&CameraHalDelegate::UpdateCameraInfoOnModuleThread, this));
+  if (!camera_info_updated_.TimedWait(kEventWaitTimeoutMs)) {
+    LOG(ERROR) << "Timed out getting camera info";
+    return false;
+  }
+  return true;
+}
+
+void CameraHalDelegate::UpdateCameraInfoOnModuleThread() {
+  DCHECK(module_task_runner_->BelongsToCurrentThread());
+  // We clear all existing camera info here and ask again. The camera info
+  // should remain unchanged for internal cameras, but for external cameras
+  // the info may change.
+  camera_info_.clear();

[wuchengli, 2017/05/02 16:11:42]

Should we set num_cameras_ to 0 here? It's more consistent to clear them both at
the same time. On the other hand, it's hard to understand we write to
num_cameras_ on two different threads.

[jcliang, 2017/05/03 04:44:56]

Yes we should set |num_cameras_| to 0 here to be consistent. |num_cameras_|
should only be modified on |module_task_runner_| so that it's protected by
|camera_info_updated_| as well. I added a DCHECK in OnSetCallbacksOnModuleThread
to make sure it runs on |module_task_runner_|.

+  camera_module_->GetNumberOfCameras(
+      base::Bind(&CameraHalDelegate::OnGotNumberOfCamerasOnModuleThread, this));
+}
+
+void CameraHalDelegate::OnGotNumberOfCamerasOnModuleThread(
+    int32_t num_cameras) {
+  DCHECK(module_task_runner_->BelongsToCurrentThread());
+  num_cameras_ = num_cameras;
+  if (!module_callbacks_registered_) {
+    // Per camera HAL v3 specification SetCallbacks() should be called after the
+    // first time GetNumberOfCameras() is called, and before other CameraModule
+    // functions are called.
+    SetCallbacksOnModuleThread();
+  } else {
+    for (ssize_t camera_id = 0; camera_id < num_cameras; ++camera_id) {
+      GetCameraInfoOnModuleThread(
+          camera_id,
+          base::Bind(&CameraHalDelegate::OnGotCameraInfoOnModuleThread, this,
+                     camera_id));
+    }
+  }
+}
+
+void CameraHalDelegate::GetCameraInfoOnModuleThread(
+    int32_t camera_id,
+    const GetCameraInfoCallback& callback) {
+  DCHECK(module_task_runner_->BelongsToCurrentThread());
+  camera_module_->GetCameraInfo(camera_id, callback);
+}
+
+void CameraHalDelegate::OnGotCameraInfoOnModuleThread(
+    int32_t camera_id,
+    int32_t result,
+    arc::mojom::CameraInfoPtr camera_info) {
+  DCHECK(module_task_runner_->BelongsToCurrentThread());
+  DCHECK(!camera_info_updated_.IsSignaled());

[wuchengli, 2017/05/02 16:11:41]

Should we move DCHECK after line 311? I'm thinking about this call flow. Can
this happen?
- Suppose there are two cameras and camera_info_ have been initialized.
- GetCameraInfo and GetCameraInfoOnModuleThread are called.
- One camera is unplugged.
- CameraDeviceStatusChange is called and camera_info_updated_ is reset.
- OnGotCameraInfoOnModuleThread is called.

[jcliang, 2017/05/03 04:44:55]

This won't happen for now as we only deal with built-in cameras. Hot-plugging
external cameras will be handled in another CL.

+  if (result) {
+    // The call to get_camera_info may fail if the device is still in the
+    // enumerating state or is no longer present.

[wuchengli, 2017/05/02 16:11:41]

If the device is no longer present, shouldn't we update num_cameras_?

[jcliang, 2017/05/03 04:44:56]

Currently I'm following the HAL API spec:
https://chromium.googlesource.com/aosp/platform/hardware/libhardware/+/master...

If we get here then it means the camera device is at the ENUMERATING state:
https://chromium.googlesource.com/aosp/platform/hardware/libhardware/+/master...

If the device is no longer present then we would get an empty |camera_info|
here. We still keep the same |num_cameras_| and insert the empty |camera_info|
into |camera_info_|. GetSupportedFormats and GetDeviceDescriptors above would
ignore the empty camera info. This should simply things as we only rely on the
CameraDeviceStatusChange callback from HAL below to trigger an update to
|camera_info_| once the ENUMERATING camera device is PRESENT or NOT_PRESENT.

+    VLOG(1) << "Failed to get camera info. Device id: " << camera_id;

[wuchengli, 2017/05/02 16:11:42]

If get_camera_info fails, should we return here?

[jcliang, 2017/05/03 04:44:55]

We continue here because of the reason above.

+  }
+  // In case of failure |camera_info| is empty.
+  camera_info_[camera_id] = std::move(camera_info);

[wuchengli, 2017/05/02 16:11:42]

VideoCaptureDeviceArcChromeOS can call GetCameraInfo and pass a
GetCameraInfoCallback. Does VideoCaptureDeviceArcChromeOS also need to set
camera_info_ and camera_info_updated_ in its callback?

[jcliang, 2017/05/03 04:44:55]

|camera_info_| is only used internally by CameraHalDelegate, and
|camera_info_updated_| is used to serialize the access to |camera_info_|
internally in CameraHalDelegate.

GetCameraInfo is meant to be called by external user and thus it carries its own
|callback| in the parameters. When GetCameraInfo is called with a |camera_id| we
IPC to HAL to call get_camera_info directly without using |camera_info_| at all.
When the IPC returns the supplied |callback| would be called and it would not go
through here.

+  if (camera_info_.size() == num_cameras_) {
+    camera_info_updated_.Signal();

[wuchengli, 2017/05/02 16:11:42]

Is it possible we get CameraDeviceStatusChange before
OnGotCameraInfoOnModuleThread? Will camera_info_updated_ be reset first and get
signaled?

[jcliang, 2017/05/03 04:44:56]

The code is refactored to only handle built-in cameras and this case wouldn't
happen for built-in cameras. We'll need locking for handling the external
cameras. I'm trying to understand how the external camera status change event
works as the camera HAL v3 API is not very clear about how the signaling of
external camera works.

I'll handle external camera in another CL.

+  }
+}
+
+void CameraHalDelegate::OpenDeviceOnModuleThread(
+    int32_t camera_id,
+    const OpenDeviceCallback& callback) {
+  DCHECK(module_task_runner_->BelongsToCurrentThread());
+  camera_module_->OpenDevice(camera_id, callback);
+}
+
+void CameraHalDelegate::SetCallbacksOnModuleThread() {
+  DCHECK(module_task_runner_->BelongsToCurrentThread());
+  camera_module_->SetCallbacks(
+      camera_module_callbacks_.CreateInterfacePtrAndBind(),
+      base::Bind(&CameraHalDelegate::OnSetCallbacksOnModuleThread, this));
+}
+
+void CameraHalDelegate::OnSetCallbacksOnModuleThread(int32_t result) {

[wuchengli, 2017/05/02 16:11:42]

What thread runs this function?

[jcliang, 2017/05/03 04:44:56]

Added a DCHECK to make sure this runs on |module_task_runner_|.

+  if (result) {
+    LOG(ERROR) << "Failed to set camera module callbacks";

[wuchengli, 2017/05/02 16:11:42]

I think we should set num_camera_ to 0, clear camera_info_, and signal
camera_info_updated_ to unblock the client. Otherwise the client will be blocked
forever.

[jcliang, 2017/05/03 04:44:56]

|camera_info_| is empty here. I've set |num_cameras_| to 0 and signaled
|camera_info_updated_|. I'm using TimedWait on |camera_info_updated_| above so
it should time out eventually, but we can explicitly signal here to reducing
latency.

+    return;
+  }
+  module_callbacks_registered_ = true;
+  for (size_t camera_id = 0; camera_id < num_cameras_; ++camera_id) {
+    GetCameraInfoOnModuleThread(
+        camera_id, base::Bind(&CameraHalDelegate::OnGotCameraInfoOnModuleThread,
+                              this, camera_id));
+  }
+}
+
+// CameraModuleCallbacks implementations.
+void CameraHalDelegate::CameraDeviceStatusChange(
+    int32_t camera_id,
+    arc::mojom::CameraDeviceStatus new_status) {
+  DCHECK(module_task_runner_->BelongsToCurrentThread());
+  // Here we just reset |camera_info_updated_| instead of trigger an update of
+  // camera info. This is to make sure that changes to |num_cameras_| and
+  // |camera_info_| are only triggered by UpdateCameraInfo to avoid race
+  // conditions.
+  if (new_status ==
+      arc::mojom::CameraDeviceStatus::CAMERA_DEVICE_STATUS_NOT_PRESENT) {

[wuchengli, 2017/05/02 16:11:41]

Probably we won't be able to enable this for USB and MIPI camera HALs at the
same time. Enabling MIPI has higher priority over USB. For MIPI, we don't need
this yet because they are all built-in cameras. There are many review comments
about camera_info_updated_.Reset in this file. I'd prefer making this a TODO so
the CL is smaller and we get unblock the vendor sooner. 

[jcliang, 2017/05/03 04:44:56]

Done.

+    if (camera_info_.find(camera_id) != camera_info_.end()) {
+      camera_info_updated_.Reset();
+    }
+  } else if (new_status ==
+             arc::mojom::CameraDeviceStatus::CAMERA_DEVICE_STATUS_PRESENT) {
+    if (camera_info_.find(camera_id) == camera_info_.end() ||
+        camera_info_[camera_id].is_null()) {
+      camera_info_updated_.Reset();

[wuchengli, 2017/05/02 16:11:41]

As discussed, how do we fix the issue the client may accidentally use a
different camera after status change? This sounds serious. What if one camera is
pointing at a scene that users do not want other people to see?

I'm curious how chrome handles two external USB cameras with the same pid/vid.
Does it have the same problem?

+    }
+  }

[wuchengli, 2017/05/02 16:11:42]

What about CAMERA_DEVICE_STATUS_ENUMERATING? I think we should also reset
camera_info_updated_. Is it simpler just always reset camera_info_updated_
without checking |new_status| and |camera_info_|?

[jcliang, 2017/05/03 04:44:56]

We can simply ignore the ENUMERATING state, since when a device is in this state
querying its info will return error and null info. The ENUMERATING will
transition to PRESENT or NOT_PRESENT eventually and we can handle the event
accordingly like above.

+}
+
+}  // namespace media