| Message ID | 20210730110154.181370-3-umang.jain@ideasonboard.com |
|---|---|
| State | Superseded |
| Delegated to: | Umang Jain |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, Thank you for the patch. On Fri, Jul 30, 2021 at 04:31:52PM +0530, Umang Jain wrote: > Error out on any camera's location if set to "external", > in the HAL configuration file. The HAL configuration file > is only meant for the integrated cameras present on the system. This makes sense, but I'd change the rationale a little bit. We may want to use the HAL configuration file in the future to also specify information for external cameras (not sure what the use cases would be, but someone attaching an external camera to an Android device may want to tweak properties through the file). However, in that case, libcamera will report the location as external already, so there's no need for the configuration file to override the location. Overriding the location to external would only be needed for cameras that libcamera report as front or back, and this wouldn't make sense. You could write the commit message as "Error out on any camera's location if set to "external", in the HAL configuration file. The HAL configuration file is meant to override the location property, and overriding an internal camera location to external doesn't make sense." > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/android/camera_hal_config.cpp | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp > index 833cf4ba..7126aba4 100644 > --- a/src/android/camera_hal_config.cpp > +++ b/src/android/camera_hal_config.cpp > @@ -127,8 +127,6 @@ int CameraHalConfig::Private::parseCameraLocation(CameraConfigData *cameraConfig > cameraConfigData->facing = CAMERA_FACING_FRONT; > else if (location == "back") > cameraConfigData->facing = CAMERA_FACING_BACK; > - else if (location == "external") > - cameraConfigData->facing = CAMERA_FACING_EXTERNAL; > else > return -EINVAL; >
Hi Umang, On Fri, Jul 30, 2021 at 04:31:52PM +0530, Umang Jain wrote: > Error out on any camera's location if set to "external", > in the HAL configuration file. The HAL configuration file > is only meant for the integrated cameras present on the system. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/android/camera_hal_config.cpp | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp > index 833cf4ba..7126aba4 100644 > --- a/src/android/camera_hal_config.cpp > +++ b/src/android/camera_hal_config.cpp > @@ -127,8 +127,6 @@ int CameraHalConfig::Private::parseCameraLocation(CameraConfigData *cameraConfig > cameraConfigData->facing = CAMERA_FACING_FRONT; > else if (location == "back") > cameraConfigData->facing = CAMERA_FACING_BACK; > - else if (location == "external") > - cameraConfigData->facing = CAMERA_FACING_EXTERNAL; > else > return -EINVAL; > > -- > 2.31.0 >
Hi Umang, On Fri, Jul 30, 2021 at 04:31:52PM +0530, Umang Jain wrote: > Error out on any camera's location if set to "external", > in the HAL configuration file. The HAL configuration file > is only meant for the integrated cameras present on the system. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> I think that's fair for the time being Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > --- > src/android/camera_hal_config.cpp | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp > index 833cf4ba..7126aba4 100644 > --- a/src/android/camera_hal_config.cpp > +++ b/src/android/camera_hal_config.cpp > @@ -127,8 +127,6 @@ int CameraHalConfig::Private::parseCameraLocation(CameraConfigData *cameraConfig > cameraConfigData->facing = CAMERA_FACING_FRONT; > else if (location == "back") > cameraConfigData->facing = CAMERA_FACING_BACK; > - else if (location == "external") > - cameraConfigData->facing = CAMERA_FACING_EXTERNAL; > else > return -EINVAL; > > -- > 2.31.0 >
diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp index 833cf4ba..7126aba4 100644 --- a/src/android/camera_hal_config.cpp +++ b/src/android/camera_hal_config.cpp @@ -127,8 +127,6 @@ int CameraHalConfig::Private::parseCameraLocation(CameraConfigData *cameraConfig cameraConfigData->facing = CAMERA_FACING_FRONT; else if (location == "back") cameraConfigData->facing = CAMERA_FACING_BACK; - else if (location == "external") - cameraConfigData->facing = CAMERA_FACING_EXTERNAL; else return -EINVAL;
Error out on any camera's location if set to "external", in the HAL configuration file. The HAL configuration file is only meant for the integrated cameras present on the system. Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- src/android/camera_hal_config.cpp | 2 -- 1 file changed, 2 deletions(-)