[libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set default sensor location to Unknown

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Thu, Feb 11, 2021 at 02:24:19PM +0100, Niklas Söderlund wrote:
> On 2021-02-11 17:55:26 +0900, Paul Elder wrote:
> > Instead of choosing some arbitrary location for the sensor when its
> > location is unknown, set it explicitly to unknown.
> > 
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > ---
> >  src/libcamera/camera_sensor.cpp | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> > index c9e8d49b..474055ba 100644
> > --- a/src/libcamera/camera_sensor.cpp
> > +++ b/src/libcamera/camera_sensor.cpp
> > @@ -446,7 +446,7 @@ int CameraSensor::initProperties()
> >  			break;
> >  		}
> >  	} else {
> > -		propertyValue = properties::CameraLocationExternal;
> > +		propertyValue = properties::CameraLocationUnknown;
> 
> I wonder if it would not make more sens to just set the location to 
> front here? What additional use-case do we cover by adding the unkown 
> location?
>
> If we want to highlight we don't know where a camera is would it not be 
> better to LOG() that we don't know but assume front. I'm thinking from 
> an application point of view is it not kind of messy to have to deal 
> with a firmware description that is incomplete? I guess all users will 
> do what you do in this series for the HAL and default it to something 
> else.

Isn't it better to let the application decide though, instead of
pretending we know ? The application could then decide how to deal with
the situation depending on its use cases, which are not known to
libcamera.

> If you do opt to keep the addition of CameraLocationUnknown you should 
> also update cam utility to handle the new location value.

Yes, that should be part of this series.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> >  	}
> >  	properties_.set(properties::Location, propertyValue);
> >  

-- 
Regards,

Laurent Pinchart