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

[Laurent Pinchart]

Hi Umang,

On Wed, May 26, 2021 at 03:42:53PM +0530, Umang Jain wrote:
> On 5/26/21 3:27 PM, Jacopo Mondi wrote:
> > On Wed, May 26, 2021 at 02:51:19PM +0530, Umang Jain wrote:
> >> On 5/26/21 1:35 PM, Jacopo Mondi wrote:
> >>> 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?

Given that the UVC camera is internal, what happens if you set its
location to front in the configuration file ?

> >>>>> 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

-- 
Regards,

Laurent Pinchart