[libcamera-devel] [PATCH 2/3] libcamera: camera_sensor: Set default sensor location to Unknown
Niklas Söderlund
niklas.soderlund at ragnatech.se
Thu Feb 11 23:35:58 CET 2021
Hi Laurent,
On 2021-02-12 00:26:02 +0200, Laurent Pinchart wrote:
> Hi Niklas,
>
> On Thu, Feb 11, 2021 at 10:58:24PM +0100, Niklas Söderlund wrote:
> > On 2021-02-11 23:44:44 +0200, Laurent Pinchart wrote:
> > > 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.
> >
> > I'd say it depends :-)
> >
> > Down the road I envision the camera location to always be mandatory.
> > Either read from firmware or a platform configuration file. If this
> > holds true I think adding a unknown location now is just pushing the
> > problem down the road. On the other hand if we think we will have
> > cameras with an unknown location and see it as a valid use-case I think
> > this patch is correct.
> >
> > I'm however not convinced we have a good use-case for a camera with an
> > unknown location. Do you have an example of such use-case?
>
> How would you describe, for instance, a Raspberry Pi camera sitting on a
> desk ?
I would describe it as external, just like a UVC camera at the end of a
cable. If I then make a product of the RPi/UVC camera and mount it in a
frame I would expect a firmware or other integration work (configuation
file?) addition to describe how I mounted it.
IMHO that way libcamera applications can make a better deductions about
use-cases then if it has to handle the subtle differences between
external and unknown.
>
> > > > 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
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list