[libcamera-devel] [PATCH v2 4/4] cam: Default camera location to external

[paul.elder at ideasonboard.com]

Hi Jacopo and Kieran,

On Fri, Feb 12, 2021 at 07:22:37PM +0000, Kieran Bingham wrote:
> Hi Paul,
> 
> On 12/02/2021 09:42, Jacopo Mondi wrote:
> > Hi Paul,
> >  I think the patch subject is wrong :/
> > 
> > On Fri, Feb 12, 2021 at 02:48:16PM +0900, Paul Elder wrote:
> >> Now that we default the camera sensor location to Unknown, handle it in
> >> cam the same way external is handled.
> > 
> > I would not call an unknown camera "External".
> > This is also the second different default we assign to "Unknown" in
> > this series. For the HAL it defaults to Front, here it defaults to
> > "External". Isn't this a bit confusing ?

tbh I think it's fine for different applications to handle the unknown
location differently.

> > I would rather name it just "Camera" without reporting
> > external/front/back or anything else.
> > 
> 
> I wouldn't even use Camera, just print the model, and the rest of the
> string:
> 
> I.e.:
> > 1: Camera 'HP Wide Vision FHD Camera: HP W' (\_SB_.PCI0.XHC_.RHUB.HS05-5:1.0-0408:5251)
> 
> 
> vs
> > 2: 'HP Wide Vision FHD Camera: HP I' (\_SB_.PCI0.XHC_.RHUB.HS05-5:1.2-0408:5251)
> 
> Otherwise, as this is cam/test tool - we should possibly report that the
> location is unknown:
> 
> >> 1: Unknown Location 'HP Wide Vision FHD Camera: HP W' (\_SB_.PCI0.XHC_.RHUB.HS05-5:1.0-0408:5251)

But for cam I think this would be the better way.


Thanks,

Paul

> >>
> >> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> >>
> >> ---
> >> New in v2
> >> ---
> >>  src/cam/main.cpp | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> >> index e01be63a..261ed0c8 100644
> >> --- a/src/cam/main.cpp
> >> +++ b/src/cam/main.cpp
> >> @@ -387,6 +387,7 @@ std::string const CamApp::cameraName(const Camera *camera)
> >>  		name = "Internal back camera";
> >>  		break;
> >>  	case properties::CameraLocationExternal:
> >> +	case properties::CameraLocationUnknown:
> >>  		name = "External camera";
> >>  		if (props.contains(properties::Model))
> >>  			name += " '" + props.get(properties::Model) + "'";
> >> --
> >> 2.27.0
> >>
> >> _______________________________________________
> >> libcamera-devel mailing list
> >> libcamera-devel at lists.libcamera.org
> >> https://lists.libcamera.org/listinfo/libcamera-devel
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
> >