<div dir="ltr"><div>Hi Jacopo,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Apr 15, 2021 at 10:51 PM Jacopo Mondi <<a href="mailto:jacopo@jmondi.org">jacopo@jmondi.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Open the HAL configuration file in the Camera HAL manager and get<br>
the camera properties for each created CameraDevice and initialize it<br>
with them.<br>
<br>
Reviewed-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br></blockquote><div> </div>Reviewed-by: Hirokazu Honda <<a href="mailto:hiroh@chromium.org" target="_blank">hiroh@chromium.org</a>><div><br></div><div>When would you merge this series?</div><div><br></div><div>-Hiro </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Signed-off-by: Jacopo Mondi <<a href="mailto:jacopo@jmondi.org" target="_blank">jacopo@jmondi.org</a>><br>
---<br>
src/android/camera_device.cpp | 56 +++++++++++++++++++++++++-----<br>
src/android/camera_device.h | 3 +-<br>
src/android/camera_hal_manager.cpp | 33 +++++++++++++++++-<br>
src/android/camera_hal_manager.h | 3 ++<br>
4 files changed, 85 insertions(+), 10 deletions(-)<br>
<br>
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp<br>
index 89044efa7ebe..50809c6ffbdc 100644<br>
--- a/src/android/camera_device.cpp<br>
+++ b/src/android/camera_device.cpp<br>
@@ -6,6 +6,7 @@<br>
*/<br>
<br>
#include "camera_device.h"<br>
+#include "camera_hal_config.h"<br>
#include "camera_ops.h"<br>
#include "post_processor.h"<br>
<br>
@@ -443,12 +444,25 @@ std::unique_ptr<CameraDevice> CameraDevice::create(unsigned int id,<br>
}<br>
<br>
/*<br>
- * Initialize the camera static information.<br>
+ * Initialize the camera static information retrieved from the<br>
+ * Camera::properties or from the cameraConfigData.<br>
+ *<br>
+ * cameraConfigData is optional for external camera devices and can be<br>
+ * nullptr.<br>
+ *<br>
* This method is called before the camera device is opened.<br>
*/<br>
-int CameraDevice::initialize()<br>
+int CameraDevice::initialize(const CameraConfigData *cameraConfigData)<br>
{<br>
- /* Initialize orientation and facing side of the camera. */<br>
+ /*<br>
+ * Initialize orientation and facing side of the camera.<br>
+ *<br>
+ * If the libcamera::Camera provides those information as retrieved<br>
+ * from firmware use them, otherwise fallback to values parsed from<br>
+ * the configuration file. If the configuration file is not available<br>
+ * the camera is external so its location and rotation can be safely<br>
+ * defaulted.<br>
+ */<br>
const ControlList &properties = camera_->properties();<br>
<br>
if (properties.contains(properties::Location)) {<br>
@@ -464,12 +478,22 @@ int CameraDevice::initialize()<br>
facing_ = CAMERA_FACING_EXTERNAL;<br>
break;<br>
}<br>
+<br>
+ if (cameraConfigData && cameraConfigData->facing != -1 &&<br>
+ facing_ != cameraConfigData->facing) {<br>
+ LOG(HAL, Warning)<br>
+ << "Camera location does not match"<br>
+ << " configuration file. Using " << facing_;<br>
+ }<br>
+ } else if (cameraConfigData) {<br>
+ if (cameraConfigData->facing == -1) {<br>
+ LOG(HAL, Error)<br>
+ << "Camera facing not in configuration file";<br>
+ return -EINVAL;<br>
+ }<br>
+ facing_ = cameraConfigData->facing;<br>
} else {<br>
- /*<br>
- * \todo Retrieve the camera location from configuration file<br>
- * if not available from the library.<br>
- */<br>
- facing_ = CAMERA_FACING_FRONT;<br>
+ facing_ = CAMERA_FACING_EXTERNAL;<br>
}<br>
<br>
/*<br>
@@ -483,8 +507,24 @@ int CameraDevice::initialize()<br>
if (properties.contains(properties::Rotation)) {<br>
int rotation = properties.get(properties::Rotation);<br>
orientation_ = (360 - rotation) % 360;<br>
+ if (cameraConfigData && cameraConfigData->rotation != -1 &&<br>
+ orientation_ != cameraConfigData->rotation) {<br>
+ LOG(HAL, Warning)<br>
+ << "Camera orientation does not match"<br>
+ << " configuration file. Using " << orientation_;<br>
+ }<br>
+ } else if (cameraConfigData) {<br>
+ if (cameraConfigData->rotation == -1) {<br>
+ LOG(HAL, Error)<br>
+ << "Camera rotation not in configuration file";<br>
+ return -EINVAL;<br>
+ }<br>
+ orientation_ = cameraConfigData->rotation;<br>
+ } else {<br>
+ orientation_ = 0;<br>
}<br>
<br>
+ /* Acquire the camera and initialize available stream configurations. */<br>
int ret = camera_->acquire();<br>
if (ret) {<br>
LOG(HAL, Error) << "Failed to temporarily acquire the camera";<br>
diff --git a/src/android/camera_device.h b/src/android/camera_device.h<br>
index 11bdfec8d587..9cc0d4005242 100644<br>
--- a/src/android/camera_device.h<br>
+++ b/src/android/camera_device.h<br>
@@ -29,6 +29,7 @@<br>
#include "camera_worker.h"<br>
#include "jpeg/encoder.h"<br>
<br>
+struct CameraConfigData;<br>
class CameraDevice : protected libcamera::Loggable<br>
{<br>
public:<br>
@@ -36,7 +37,7 @@ public:<br>
std::shared_ptr<libcamera::Camera> cam);<br>
~CameraDevice();<br>
<br>
- int initialize();<br>
+ int initialize(const CameraConfigData *cameraConfigData);<br>
<br>
int open(const hw_module_t *hardwareModule);<br>
void close();<br>
diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp<br>
index bf3fcda75237..f5b86974e8e3 100644<br>
--- a/src/android/camera_hal_manager.cpp<br>
+++ b/src/android/camera_hal_manager.cpp<br>
@@ -41,6 +41,15 @@ int CameraHalManager::init()<br>
{<br>
cameraManager_ = std::make_unique<CameraManager>();<br>
<br>
+ /*<br>
+ * If the configuration file is not available the HAL only supports<br>
+ * external cameras. If it exists but it's not valid then error out.<br>
+ */<br>
+ if (halConfig_.exists() && !halConfig_.isValid()) {<br>
+ LOG(HAL, Error) << "HAL configuration file is not valid";<br>
+ return -EINVAL;<br>
+ }<br>
+<br>
/* Support camera hotplug. */<br>
cameraManager_->cameraAdded.connect(this, &CameraHalManager::cameraAdded);<br>
cameraManager_->cameraRemoved.connect(this, &CameraHalManager::cameraRemoved);<br>
@@ -100,6 +109,8 @@ void CameraHalManager::cameraAdded(std::shared_ptr<Camera> cam)<br>
auto iter = cameraIdsMap_.find(cam->id());<br>
if (iter != cameraIdsMap_.end()) {<br>
id = iter->second;<br>
+ if (id >= firstExternalCameraId_)<br>
+ isCameraExternal = true;<br>
} else {<br>
isCameraNew = true;<br>
<br>
@@ -117,7 +128,27 @@ void CameraHalManager::cameraAdded(std::shared_ptr<Camera> cam)<br>
<br>
/* Create a CameraDevice instance to wrap the libcamera Camera. */<br>
std::unique_ptr<CameraDevice> camera = CameraDevice::create(id, cam);<br>
- int ret = camera->initialize();<br>
+<br>
+ /*<br>
+ * The configuration file must be valid, and contain a corresponding<br>
+ * entry for internal cameras. External cameras can be initialized<br>
+ * without configuration file.<br>
+ */<br>
+ if (!isCameraExternal && !halConfig_.exists()) {<br>
+ LOG(HAL, Error)<br>
+ << "HAL configuration file is mandatory for internal cameras";<br>
+ return;<br>
+ }<br>
+<br>
+ const CameraConfigData *cameraConfigData = halConfig_.cameraConfigData(cam->id());<br>
+ if (!isCameraExternal && !cameraConfigData) {<br>
+ LOG(HAL, Error)<br>
+ << "HAL configuration entry for internal camera "<br>
+ << cam->id() << " is missing";<br>
+ return;<br>
+ }<br>
+<br>
+ int ret = camera->initialize(cameraConfigData);<br>
if (ret) {<br>
LOG(HAL, Error) << "Failed to initialize camera: " << cam->id();<br>
return;<br>
diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h<br>
index d9bf27989965..af1581da6579 100644<br>
--- a/src/android/camera_hal_manager.h<br>
+++ b/src/android/camera_hal_manager.h<br>
@@ -19,6 +19,8 @@<br>
<br>
#include <libcamera/camera_manager.h><br>
<br>
+#include "camera_hal_config.h"<br>
+<br>
class CameraDevice;<br>
<br>
class CameraHalManager<br>
@@ -50,6 +52,7 @@ private:<br>
CameraDevice *cameraDeviceFromHalId(unsigned int id);<br>
<br>
std::unique_ptr<libcamera::CameraManager> cameraManager_;<br>
+ CameraHalConfig halConfig_;<br>
<br>
const camera_module_callbacks_t *callbacks_;<br>
std::vector<std::unique_ptr<CameraDevice>> cameras_;<br>
-- <br>
2.31.1<br>
<br>
_______________________________________________<br>
libcamera-devel mailing list<br>
<a href="mailto:libcamera-devel@lists.libcamera.org" target="_blank">libcamera-devel@lists.libcamera.org</a><br>
<a href="https://lists.libcamera.org/listinfo/libcamera-devel" rel="noreferrer" target="_blank">https://lists.libcamera.org/listinfo/libcamera-devel</a><br>
</blockquote></div></div>