[libcamera-devel] [PATCH 2/5] libcamera: controls: Add camera location control

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Sat, Aug 17, 2019 at 05:54:24PM +0200, Niklas Söderlund wrote:
> On 2019-08-17 12:59:34 +0200, Jacopo Mondi wrote:
> > Define a new libcamera control used to specify the camera location.
> > Provide an enumeration of possible values to describe the "front",
> > "back" and "external" camera locations.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  include/libcamera/control_ids.h | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/include/libcamera/control_ids.h b/include/libcamera/control_ids.h
> > index 75b6a2d5cafe..97ba11ec5f7b 100644
> > --- a/include/libcamera/control_ids.h
> > +++ b/include/libcamera/control_ids.h
> > @@ -12,6 +12,12 @@
> >  
> >  namespace libcamera {
> >  
> > +enum CameraLocation {
> 
> Do we need a CAMERA_LOCATION_UNKNOWN to handle cases where this 
> information is not available?

That, or move CAMERA_LOCATION_EXTERNAL as the first entry and use it as
a default.

I don't like UNKNOWN much as it gives a way for pipeline handler authors
to be lazy and "fix this later".

> > +	CAMERA_LOCATION_FRONT,
> > +	CAMERA_LOCATION_BACK,
> > +	CAMERA_LOCATION_EXTERNAL,
> > +};

I'm afraid I'll ask you to document CameraLocation and the new Location
property :-)

> > +
> >  enum ControlId {
> >  	AwbEnable,
> >  	Brightness,
> > @@ -19,6 +25,7 @@ enum ControlId {
> >  	Saturation,
> >  	ManualExposure,
> >  	ManualGain,
> > +	Location,

Should we move this to a new PropertyId enum ?

> >  };
> >  
> >  } /* namespace libcamera */

-- 
Regards,

Laurent Pinchart