[libcamera-devel] [PATCH] android: nautilus: Add camera HAL configuration

[Umang Jain]

Hi Jacopo

On 5/26/21 3:27 PM, Jacopo Mondi wrote:
> Hi Umang,
>
> On Wed, May 26, 2021 at 02:51:19PM +0530, Umang Jain wrote:
>> Hi Jacopo
>>
>> On 5/26/21 1:35 PM, Jacopo Mondi wrote:
>>> Hi Umang,
>>>
>>> On Wed, May 26, 2021 at 12:45:42PM +0530, Umang Jain wrote:
>>>> nautilus has two internal cameras, one UVC and one for the IPU3.
>>>> However, libcamera assumes all UVC cameras as 'external' hence, mark
>>>> the location of UVC camera in HAL configuration as 'external' too.
>>>>
>>>> Note that the presence of UVC camera in camera HAL will complain:
>>>>
>>>>> ERROR HAL camera_device.cpp:701 '\_SB_.PCI0.XHCI.RHUB.HS09-9:1.0-04f2:b647':
>>>>     Failed to map mandatory Android format IMPLEMENTATION_DEFINED (0x00000022): aborting
>>>>> ERROR HAL camera_hal_manager.cpp:153 Failed to initialize camera:
>>>>     \_SB_.PCI0.XHCI.RHUB.HS09-9:1.0-04f2:b647
>>>>
>>>> This is related to a known issue of UVC cameras on HAL which do not
>>>> provide NV12 by default.
>>> Does it work without configuration file support ?
>> No, it won't, since it's UVC. And cannot work until there's a
>> format-convertor in place that can convert the stream to NV12 provided by
>> that camera, I suppose.
> Right, it was not working before the configuration file series went
> in, right ? What I'm interested about is that there's no regression
> due to the series :)
Oh yea, no regression. I just went and wrote a verbose commit message 
about what's "fine"

>
>
>>>> Also, if we don't mention the UVC camera in the config:
>>>>
>>>>> ERROR HALConfig camera_hal_config.cpp:393 Camera
>>>>     '\_SB_.PCI0.XHCI.RHUB.HS09-9:1.0-04f2:b647' not described in the HAL
>>>>     configuration file
> This is "fine".. If you look at the caller of
> CameraHalConfig::cameraConfigData() in camera_hal_manager.cpp
>
> 	const CameraConfigData *cameraConfigData = halConfig_.cameraConfigData(cam->id());
> 	if (!isCameraExternal && !cameraConfigData) {
> 		LOG(HAL, Error)
> 			<< "HAL configuration entry for internal camera "
> 			<< cam->id() << " is missing";
> 		return;
> 	}
>
> You'll see that the configuration entry for external cameras is not
> mandatory, and the UVC camera should be correctly identified as
> EXTERNAL.
>
> We spam the log enough with worrying errors which are not really
> errors, so we might want to suppress this one for external cameras ?
Won't supress it entirely, but maybe not keep it as ERROR. WARN or DEBUG 
maybe?

Thanks!
>
> Thanks
>     j
>
>
>>>>> ERROR HAL camera_device.cpp:701 '\_SB_.PCI0.XHCI.RHUB.HS09-9:1.0-04f2:b647':
>>>>     Failed to map mandatory Android format IMPLEMENTATION_DEFINED (0x00000022): aborting
>>>>> ERROR HAL camera_hal_manager.cpp:153 Failed to initialize camera:
>>>>     \_SB_.PCI0.XHCI.RHUB.HS09-9:1.0-04f2:b647
>>>>
>>>> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
>>>> ---
>>>>    src/android/data/nautilus/camera_hal.yaml | 8 ++++++++
>>>>    1 file changed, 8 insertions(+)
>>>>    create mode 100644 src/android/data/nautilus/camera_hal.yaml
>>>>
>>>> diff --git a/src/android/data/nautilus/camera_hal.yaml b/src/android/data/nautilus/camera_hal.yaml
>>>> new file mode 100644
>>>> index 00000000..f2d31e1c
>>>> --- /dev/null
>>>> +++ b/src/android/data/nautilus/camera_hal.yaml
>>>> @@ -0,0 +1,8 @@
>>>> +cameras:
>>>> +  "\\_SB_.PCI0.I2C2.CAM0":
>>>> +    location: back
>>>> +    rotation: 0
>>>> +
>>>> +  "\\_SB_.PCI0.XHCI.RHUB.HS09-9:1.0-04f2:b647":
>>>> +    location: external
>>>> +    rotation: 0
>>>> --
>>>> 2.26.2
>>>>