[libcamera-devel] [PATCH 1/3] libcamera: properties: Add Unknown value to camera Location

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Thu, Feb 11, 2021 at 01:50:37PM +0000, Kieran Bingham wrote:
> On 11/02/2021 08:55, Paul Elder wrote:
> > If a camera's location is unknown, it should be set so, and not
> > defaulted to another location. Add such a value to the Location property
> > enum.
> > 
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > ---
> >  src/libcamera/property_ids.yaml | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> > index 104e9aaf..66deaa84 100644
> > --- a/src/libcamera/property_ids.yaml
> > +++ b/src/libcamera/property_ids.yaml
> > @@ -25,6 +25,10 @@ controls:
> >            description: |
> >              The camera is attached to the device in a way that allows it to
> >              be moved freely
> > +        - name: CameraLocationUnknown
> > +          value: 3
> 
> Would it make sense to set the Unknown value as 0 (and put it first in
> the list?).
> 
> Of course that would mean adjusting the other valuees in this enum too,
> but while we do not have a stable ABI, I don't see that as being a problem.
> 
> That would mean that if this property were read as uninitialised (or
> defaulted?) it would be read as 'unknown' rather than 'Front' ?
> 
> It probably doesn't matter and doesn't make a difference as if it's set,
> it would be expected to be set correctly though.

I think it's a good idea.

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

> Whichever you decide:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> 
> > +          description: |
> > +            The camera location is unknown
> >  
> >    - Rotation:
> >        type: int32_t
> > 

-- 
Regards,

Laurent Pinchart