[libcamera-devel] [PATCH 2/2] android: camera_device: Set the camera location to Front if External
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Feb 22 10:27:25 CET 2021
On 21/02/2021 19:21, Laurent Pinchart wrote:
> Hello,
>
> On Fri, Feb 19, 2021 at 11:05:14AM +0000, Kieran Bingham wrote:
>> On 19/02/2021 10:45, Paul Elder wrote:
>>> Our android HAL implementation currently does not support external
>>> cameras, so if the camera location property is external, set it to
>>> front.
>>>
>>> This allows the following CTS test to pass:
>>> - android.hardware.camera2.cts.CameraManagerTest#testCameraManagerGetDeviceIdList
>>>
>>> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>>> ---
>>> src/android/camera_device.cpp | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
>>> index 1e2a5b5f..16cb8c6d 100644
>>> --- a/src/android/camera_device.cpp
>>> +++ b/src/android/camera_device.cpp
>>> @@ -405,7 +405,11 @@ int CameraDevice::initialize()
>>> facing_ = CAMERA_FACING_BACK;
>>> break;
>>> case properties::CameraLocationExternal:
>>> - facing_ = CAMERA_FACING_EXTERNAL;
>>> + /*
>>> + * \todo Set this to EXTERNAL once we support
>>> + * HARDWARE_LEVEL_EXTERNAL
>>> + */
>>> + facing_ = CAMERA_FACING_FRONT;
>>
>> I would be tempted to add a warning log here too, as it's distinctly
>> separate from the setting of the properties::CameraLocationExternal,
>> which can certainly be correctly set to External (on UVC) for example,
>> but which would come through here.
>
> It's a todo item, and we have lots of them for which we don't print any
> warning. I'm not sure to see why this one would be special.
Because this one impacts the actual expected output. An 'External'
camera is being renamed / replaced to being 'Front'.
If this was explicitly set (for example, an RPi, or UVC camera) we're
changing the reported location.
Consider that the user *expected* this camera to be external, and yet
when run through the HAL was always reported 'Front'. - It's that
*change* which makes me think a warning is worth printing. It's not
about having a todo action, it's reporting that something unexpectedly
changed internally.
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
>> I hadn't realised/been aware of this HARDWARE_LEVEL_EXTERNAL issues, I
>> wonder if that was some how related to the difficulties we still had in
>> even getting a Brio YUV UVC camera to work.
>>
>> I suspect it is, as perhaps with HARDWARE_LEVEL_EXTERNAL we can better
>> define the actual capabilities /streams of the external device.
But as I said:
>> Anyway, with or without the warning added, as this is a known workaround:
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>
>>> break;
>>> }
>>> }
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list