| Message ID | 20191223072620.13022-4-paul.elder@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, Thank you for the patch. On Mon, Dec 23, 2019 at 01:26:17AM -0600, Paul Elder wrote: > Register all UVC Cameras along with its device number, to eventually s/its device number/their device numbers/ > allow the V4L2 compatibility layer to match against it. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > New in v3 > --- > src/libcamera/pipeline/uvcvideo.cpp | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > index 3043366b..096c1c09 100644 > --- a/src/libcamera/pipeline/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo.cpp > @@ -7,6 +7,7 @@ > > #include <algorithm> > #include <iomanip> > +#include <sys/sysmacros.h> > #include <tuple> > > #include <libcamera/camera.h> > @@ -294,17 +295,19 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) > return false; > > std::unique_ptr<UVCCameraData> data = utils::make_unique<UVCCameraData>(this); > + dev_t devnum = 0; > > /* Locate and initialise the camera data with the default video node. */ > for (MediaEntity *entity : media->entities()) { > if (entity->flags() & MEDIA_ENT_FL_DEFAULT) { > if (data->init(entity)) > return false; > + devnum = makedev(entity->deviceMajor(), entity->deviceMinor()); You can move this line after the loop, and merge it with the devnum variable declaration. I would actually move it after the "Could not find a default video device" check and make it a separate check with a dedicated error message. Or even drop the check completely as it really shouldn't happen (in which case you can move the devnum calculation to the registration below). With this fixed, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > break; > } > } > > - if (!data) { > + if (!data || !devnum) { > LOG(UVC, Error) << "Could not find a default video device"; > return false; > } > @@ -312,7 +315,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) > /* Create and register the camera. */ > std::set<Stream *> streams{ &data->stream_ }; > std::shared_ptr<Camera> camera = Camera::create(this, media->model(), streams); > - registerCamera(std::move(camera), std::move(data)); > + registerCamera(std::move(camera), std::move(data), devnum); > > /* Enable hot-unplug notifications. */ > hotplugMediaDevice(media);
Hi Laurent, Thank you for the review. On Fri, Dec 27, 2019 at 04:48:51PM +0200, Laurent Pinchart wrote: > Hi Paul, > > Thank you for the patch. > > On Mon, Dec 23, 2019 at 01:26:17AM -0600, Paul Elder wrote: > > Register all UVC Cameras along with its device number, to eventually > > s/its device number/their device numbers/ ack > > allow the V4L2 compatibility layer to match against it. > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > > > --- > > New in v3 > > --- > > src/libcamera/pipeline/uvcvideo.cpp | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > > index 3043366b..096c1c09 100644 > > --- a/src/libcamera/pipeline/uvcvideo.cpp > > +++ b/src/libcamera/pipeline/uvcvideo.cpp > > @@ -7,6 +7,7 @@ > > > > #include <algorithm> > > #include <iomanip> > > +#include <sys/sysmacros.h> > > #include <tuple> > > > > #include <libcamera/camera.h> > > @@ -294,17 +295,19 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) > > return false; > > > > std::unique_ptr<UVCCameraData> data = utils::make_unique<UVCCameraData>(this); > > + dev_t devnum = 0; > > > > /* Locate and initialise the camera data with the default video node. */ > > for (MediaEntity *entity : media->entities()) { > > if (entity->flags() & MEDIA_ENT_FL_DEFAULT) { > > if (data->init(entity)) > > return false; > > + devnum = makedev(entity->deviceMajor(), entity->deviceMinor()); > > You can move this line after the loop, and merge it with the devnum > variable declaration. I would actually move it after the "Could not find > a default video device" check and make it a separate check with a > dedicated error message. Or even drop the check completely as it really > shouldn't happen (in which case you can move the devnum calculation to > the registration below). I don't think I can, because entity is only available inside the loop. I would have to save it, in which case I might as well just extract the devnum here. > With this fixed, > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > break; > > } > > } > > > > - if (!data) { > > + if (!data || !devnum) { > > LOG(UVC, Error) << "Could not find a default video device"; > > return false; > > } > > @@ -312,7 +315,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) > > /* Create and register the camera. */ > > std::set<Stream *> streams{ &data->stream_ }; > > std::shared_ptr<Camera> camera = Camera::create(this, media->model(), streams); > > - registerCamera(std::move(camera), std::move(data)); > > + registerCamera(std::move(camera), std::move(data), devnum); > > > > /* Enable hot-unplug notifications. */ > > hotplugMediaDevice(media); Thanks, Paul
diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 3043366b..096c1c09 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -7,6 +7,7 @@ #include <algorithm> #include <iomanip> +#include <sys/sysmacros.h> #include <tuple> #include <libcamera/camera.h> @@ -294,17 +295,19 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) return false; std::unique_ptr<UVCCameraData> data = utils::make_unique<UVCCameraData>(this); + dev_t devnum = 0; /* Locate and initialise the camera data with the default video node. */ for (MediaEntity *entity : media->entities()) { if (entity->flags() & MEDIA_ENT_FL_DEFAULT) { if (data->init(entity)) return false; + devnum = makedev(entity->deviceMajor(), entity->deviceMinor()); break; } } - if (!data) { + if (!data || !devnum) { LOG(UVC, Error) << "Could not find a default video device"; return false; } @@ -312,7 +315,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) /* Create and register the camera. */ std::set<Stream *> streams{ &data->stream_ }; std::shared_ptr<Camera> camera = Camera::create(this, media->model(), streams); - registerCamera(std::move(camera), std::move(data)); + registerCamera(std::move(camera), std::move(data), devnum); /* Enable hot-unplug notifications. */ hotplugMediaDevice(media);
Register all UVC Cameras along with its device number, to eventually allow the V4L2 compatibility layer to match against it. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- New in v3 --- src/libcamera/pipeline/uvcvideo.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)