[libcamera-devel] [PATCH v3 3/4] android: Override camera as "Internal" provided found in HAL config

[Umang Jain]

Hi Paul,

On 8/2/21 2:16 PM, paul.elder at ideasonboard.com wrote:
> Hi Umang,
>
> On Fri, Jul 30, 2021 at 04:31:53PM +0530, Umang Jain wrote:
>> Currently, all UVC cameras are reported with CameraLocationExternal [1]
>> by libcamera-core, since there is no universal information or standard,
> s/,$//
>
>> to know the location of these cameras. However, in the libcamera HAL
>> layer, we can make an informed decision whether its external or
> s/its/it's/
>
>> internal, simply by checking the presence of it in the HAL
> s/the presence of it/its presence/
>
>> configuration file.
>>
>> If the camera is found to be present in the HAL configuration file,
> s/to be present//
>
>> treat it as internal. CameraHalManager will now assign the numerical id
>> of the camera accordingly, based on which the facing of the camera is set
> I didn't think the facing of the camera affects the id?

Well, it should be kept in sync. If the camera is internal (id < 1000), 
but it's facing is set to be external (as per checks in CameraDevice), 
then these are not in sync. We need to take care of that.

I don't think anything bad will happen if these are out of sync. But why 
should we allow it to be out-of-sync if we handle it gracefully?

>
>> as well (as per the HAL config file) while initializing the CameraDevice
>> wrapper.
> I don't quite understand this last part.

So in the  CameraDevice::initialize(), the facing of the camera is set. 
If properties::Location doesn't match the location in the HAL config, 
CameraDevice complains a warning.

But, we know that for UVC properties:::Location won't be matching the 
HAL config. In that case we rely on id assigned by CameraManager to see 
if its a  internal camera or not. If yes, we just assign facing whatever 
is mentioned in the HAL config file :)
>
>
> Thanks,
>
> Paul
>
>> [1] 76809320bb1a ("libcamera: pipeline: uvcvideo: Treat all UVC cameras
>>                     as external")
>>
>> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
>> ---
>>   src/android/camera_device.cpp      | 13 ++++++++++++-
>>   src/android/camera_hal_manager.cpp | 17 +++++++++++++++++
>>   2 files changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
>> index 678cde23..394ebc84 100644
>> --- a/src/android/camera_device.cpp
>> +++ b/src/android/camera_device.cpp
>> @@ -330,7 +330,18 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)
>>   			facing_ = CAMERA_FACING_BACK;
>>   			break;
>>   		case properties::CameraLocationExternal:
>> -			facing_ = CAMERA_FACING_EXTERNAL;
>> +			/*
>> +			 * If the camera is reported as external, but
>> +			 * the CameraHalManager has overriden it, use
>> +			 * what is reported in the configuration file.
>> +			 * This typically happens for UVC cameras
>> +			 * reported as 'External' by libcamera but
>> +			 * installed in fixed position on the device.
>> +			 */
>> +			if (id_ < 1000 && cameraConfigData->facing != -1)
>> +			       facing_ = cameraConfigData->facing;
>> +			else
>> +			       facing_ = CAMERA_FACING_EXTERNAL;
>>   			break;
>>   		}
>>   
>> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
>> index b364f62a..cce98fad 100644
>> --- a/src/android/camera_hal_manager.cpp
>> +++ b/src/android/camera_hal_manager.cpp
>> @@ -145,6 +145,23 @@ void CameraHalManager::cameraAdded(std::shared_ptr<Camera> cam)
>>   	}
>>   
>>   	const CameraConfigData *cameraConfigData = halConfig_.cameraConfigData(cam->id());
>> +
>> +	/*
>> +	 * Some cameras whose location is reported by libcamera as external may
>> +	 * actually be internal to the device. This is common with UVC cameras
>> +	 * that are integrated in a laptop. In that case the real location
>> +	 * should be specified in the configuration file.
>> +	 *
>> +	 * If the camera location is external and a configuration
>> +	 * entry exists for it, override its location.
>> +	 */
>> +	if (isCameraNew && isCameraExternal) {
>> +		if (cameraConfigData && cameraConfigData->facing != -1) {
>> +			isCameraExternal = false;
>> +			id = numInternalCameras_;
>> +		}
>> +	}
>> +
>>   	if (!isCameraExternal && !cameraConfigData) {
>>   		LOG(HAL, Error)
>>   			<< "HAL configuration entry for internal camera "
>> -- 
>> 2.31.0
>>