[libcamera-devel] [PATCH] CameraSensor: Don't fail to add camera with faulty lens

[Umang Jain]

Hello,

On 9/3/22 7:23 PM, Laurent Pinchart via libcamera-devel wrote:
> Hi Kieran,
>
> Thank you for the patch.
>
> On Sat, Sep 03, 2022 at 12:48:03PM +0100, Kieran Bingham via libcamera-devel wrote:
>> If the CameraSensor fails to identify the VCM specified, it could still
>> be possible to continue to operate the sensor. Autofocus support will be
>> disabled, but this would be no different to operating a camera with a
>> fixed focus. While of course the fixed focus position may not be
>> suitable, it would provide a better user experience to be able to
>> continue to operate the camera, while still reporting that the lens is
>> disabled.
>>
>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=146
>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>> ---
>>
>> The Surface devices now have a VCM registered, but they don't
>> successfully identify the device. While that should also be fixed,
>> failing to operate a camera because the VCM isn't operating seems to be
>> a bad user experience.
> I think that's reasonable, while there's clearly a problem on the kernel
> side, this degraded mode is implemented without much overhead here, so
> I'm fine with it.
>
>>   src/libcamera/camera_sensor.cpp | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
>> index d055c16a4885..dcee7b9a0210 100644
>> --- a/src/libcamera/camera_sensor.cpp
>> +++ b/src/libcamera/camera_sensor.cpp
>> @@ -467,8 +467,8 @@ int CameraSensor::discoverAncillaryDevices()
>>   			ret = focusLens_->init();
>>   			if (ret) {
>>   				LOG(CameraSensor, Error)
>> -					<< "CameraLens initialisation failed";
>> -				return ret;
>> +					<< "CameraLens initialisation failed: Lens disabled";
> I'd reset focusLens_ here to avoid exposin a non-working CameraLens
> through focusLens().
>
>> +				break;
> And you can drop the break, there's one just after. The message could
> also be shortened slightly, up to you.
>
> 			ret = focusLens_->init();
> 			if (ret) {
> 				LOG(CameraSensor, Error)
> 					<< "Lens initialisation failed, lens disabled";
> 				focusLens_.reset();
> 			}
> 			break;
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>

>
>>   			}
>>   			break;
>>