[libcamera-devel] [PATCH v2 1/4] libcamera: pipeline: uvcvideo: Treat all UVC cameras as external

[Laurent Pinchart]

Hi Kieran,

On Thu, Aug 13, 2020 at 08:11:18PM +0100, Kieran Bingham wrote:
> On 13/08/2020 11:31, Niklas Söderlund wrote:
> > On 2020-08-10 12:04:11 +0000, Umang Jain wrote:
> >> Treat all UVC cameras as external for android framework to know that
> >> they are external.
> > 
> > nit: I would rewrite this to describe that as we currently have no way 
> > to determine the location if a UVC camera mark it External to have a 
> > starting point for the property.
> > 
> > With or without this fixed but with the comments by Kieran and Laurent 
> > fixed,
> > 
> > Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> 
> So, having seen the conflict this might cause with camera naming, I
> wonder if we should introduce a new property called "hotpluggable" or
> "removable" or such in a separate patch, and then set that property here.

I'm not sure to follow you, what's the conflict ?

> I'm not entirely sure yet, because I fear android expects the difference
> to be 'internal/external' rather than 'hotpluggable/not-hotpluggable'.

Android expects all internal cameras to not be hotpluggable, and all
external cameras to be hotpluggable.

> What do you think ?
> 
> >> Signed-off-by: Umang Jain <email at uajain.com>
> >> ---
> >>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> >> index bc892ec..ed61d12 100644
> >> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> >> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> >> @@ -14,6 +14,7 @@
> >>  #include <libcamera/camera.h>
> >>  #include <libcamera/control_ids.h>
> >>  #include <libcamera/controls.h>
> >> +#include <libcamera/property_ids.h>
> >>  #include <libcamera/request.h>
> >>  #include <libcamera/stream.h>
> >>  
> >> @@ -500,6 +501,9 @@ int UVCCameraData::init(MediaEntity *entity)
> >>  
> >>  	video_->bufferReady.connect(this, &UVCCameraData::bufferReady);
> >>  
> >> +	/* Initialise Camera Property. Treat all UVC cameras as external. */
> >> +	properties_.set(properties::Location, properties::CameraLocationExternal);
> >> +
> >>  	/* Initialise the supported controls. */
> >>  	ControlInfoMap::Map ctrls;
> >>  

-- 
Regards,

Laurent Pinchart