| Message ID | 20210423020932.2760-4-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Delegated to: | Laurent Pinchart |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On 23/04/2021 03:09, Laurent Pinchart wrote: > Use the newly introduced utils::enumerate() to replace manual loop > counters. A local variable is needed, as utils::enumerate() requires an > lvalue reference. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/v4l2/v4l2_compat_manager.cpp | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp > index 90c0f0121a32..bf5bf8e3223d 100644 > --- a/src/v4l2/v4l2_compat_manager.cpp > +++ b/src/v4l2/v4l2_compat_manager.cpp > @@ -23,6 +23,7 @@ > #include <libcamera/camera_manager.h> > > #include "libcamera/internal/log.h" > +#include "libcamera/internal/utils.h" > > #include "v4l2_camera_file.h" > > @@ -81,11 +82,11 @@ int V4L2CompatManager::start() > * For each Camera registered in the system, a V4L2CameraProxy gets > * created here to wrap a camera device. > */ > - unsigned int index = 0; > - for (auto &camera : cm_->cameras()) { > - V4L2CameraProxy *proxy = new V4L2CameraProxy(index, camera); > + auto cameras = cm_->cameras(); > + for (const auto camera : utils::enumerate(cameras)) { Is it possible to used named scopes here like: for (const auto [index, camera] : utils::enumerate(cameras)) { V4L2CameraProxy *proxy = new V4L2CameraProxy(index, camera); (note, I'm asking if your series makes this possible, not for you to make this change) Otherwise, I like that the index becomes part of the iteration. I have a slight dislike to treating your iterated item as 'value' ... but that's just the same dislike I have for std::pair, but at least in this case it's not named first and second! Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > + V4L2CameraProxy *proxy = new V4L2CameraProxy(camera.index, > + camera.value); > proxies_.emplace_back(proxy); > - ++index; > } > > return 0; > @@ -117,11 +118,10 @@ int V4L2CompatManager::getCameraIndex(int fd) > if (!target) > return -1; > > - unsigned int index = 0; > - for (auto &camera : cm_->cameras()) { > - if (camera == target) > - return index; > - ++index; > + auto cameras = cm_->cameras(); > + for (const auto camera : utils::enumerate(cameras)) { > + if (camera.value == target) > + return camera.index; > } > > return -1; >
Hi Laurent, Thanks for your patch. On 2021-04-23 05:09:32 +0300, Laurent Pinchart wrote: > Use the newly introduced utils::enumerate() to replace manual loop > counters. A local variable is needed, as utils::enumerate() requires an > lvalue reference. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I would prefers the 1/3 API but for the change itself, Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/v4l2/v4l2_compat_manager.cpp | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp > index 90c0f0121a32..bf5bf8e3223d 100644 > --- a/src/v4l2/v4l2_compat_manager.cpp > +++ b/src/v4l2/v4l2_compat_manager.cpp > @@ -23,6 +23,7 @@ > #include <libcamera/camera_manager.h> > > #include "libcamera/internal/log.h" > +#include "libcamera/internal/utils.h" > > #include "v4l2_camera_file.h" > > @@ -81,11 +82,11 @@ int V4L2CompatManager::start() > * For each Camera registered in the system, a V4L2CameraProxy gets > * created here to wrap a camera device. > */ > - unsigned int index = 0; > - for (auto &camera : cm_->cameras()) { > - V4L2CameraProxy *proxy = new V4L2CameraProxy(index, camera); > + auto cameras = cm_->cameras(); > + for (const auto camera : utils::enumerate(cameras)) { > + V4L2CameraProxy *proxy = new V4L2CameraProxy(camera.index, > + camera.value); > proxies_.emplace_back(proxy); > - ++index; > } > > return 0; > @@ -117,11 +118,10 @@ int V4L2CompatManager::getCameraIndex(int fd) > if (!target) > return -1; > > - unsigned int index = 0; > - for (auto &camera : cm_->cameras()) { > - if (camera == target) > - return index; > - ++index; > + auto cameras = cm_->cameras(); > + for (const auto camera : utils::enumerate(cameras)) { > + if (camera.value == target) > + return camera.index; > } > > return -1; > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Kieran, On Tue, May 11, 2021 at 11:05:43AM +0100, Kieran Bingham wrote: > On 23/04/2021 03:09, Laurent Pinchart wrote: > > Use the newly introduced utils::enumerate() to replace manual loop > > counters. A local variable is needed, as utils::enumerate() requires an > > lvalue reference. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/v4l2/v4l2_compat_manager.cpp | 18 +++++++++--------- > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > > diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp > > index 90c0f0121a32..bf5bf8e3223d 100644 > > --- a/src/v4l2/v4l2_compat_manager.cpp > > +++ b/src/v4l2/v4l2_compat_manager.cpp > > @@ -23,6 +23,7 @@ > > #include <libcamera/camera_manager.h> > > > > #include "libcamera/internal/log.h" > > +#include "libcamera/internal/utils.h" > > > > #include "v4l2_camera_file.h" > > > > @@ -81,11 +82,11 @@ int V4L2CompatManager::start() > > * For each Camera registered in the system, a V4L2CameraProxy gets > > * created here to wrap a camera device. > > */ > > - unsigned int index = 0; > > - for (auto &camera : cm_->cameras()) { > > - V4L2CameraProxy *proxy = new V4L2CameraProxy(index, camera); > > + auto cameras = cm_->cameras(); > > + for (const auto camera : utils::enumerate(cameras)) { > > Is it possible to used named scopes here like: > > for (const auto [index, camera] : utils::enumerate(cameras)) { > V4L2CameraProxy *proxy = new V4L2CameraProxy(index, camera); > > (note, I'm asking if your series makes this possible, not for you to > make this change) If you drop patch 2/3, yes, that's what would happen. Both you and Niklas seem to prefer that option, so that's what I'll do in v2. > Otherwise, I like that the index becomes part of the iteration. > I have a slight dislike to treating your iterated item as 'value' ... > but that's just the same dislike I have for std::pair, but at least in > this case it's not named first and second! > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > + V4L2CameraProxy *proxy = new V4L2CameraProxy(camera.index, > > + camera.value); > > proxies_.emplace_back(proxy); > > - ++index; > > } > > > > return 0; > > @@ -117,11 +118,10 @@ int V4L2CompatManager::getCameraIndex(int fd) > > if (!target) > > return -1; > > > > - unsigned int index = 0; > > - for (auto &camera : cm_->cameras()) { > > - if (camera == target) > > - return index; > > - ++index; > > + auto cameras = cm_->cameras(); > > + for (const auto camera : utils::enumerate(cameras)) { > > + if (camera.value == target) > > + return camera.index; > > } > > > > return -1;
diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp index 90c0f0121a32..bf5bf8e3223d 100644 --- a/src/v4l2/v4l2_compat_manager.cpp +++ b/src/v4l2/v4l2_compat_manager.cpp @@ -23,6 +23,7 @@ #include <libcamera/camera_manager.h> #include "libcamera/internal/log.h" +#include "libcamera/internal/utils.h" #include "v4l2_camera_file.h" @@ -81,11 +82,11 @@ int V4L2CompatManager::start() * For each Camera registered in the system, a V4L2CameraProxy gets * created here to wrap a camera device. */ - unsigned int index = 0; - for (auto &camera : cm_->cameras()) { - V4L2CameraProxy *proxy = new V4L2CameraProxy(index, camera); + auto cameras = cm_->cameras(); + for (const auto camera : utils::enumerate(cameras)) { + V4L2CameraProxy *proxy = new V4L2CameraProxy(camera.index, + camera.value); proxies_.emplace_back(proxy); - ++index; } return 0; @@ -117,11 +118,10 @@ int V4L2CompatManager::getCameraIndex(int fd) if (!target) return -1; - unsigned int index = 0; - for (auto &camera : cm_->cameras()) { - if (camera == target) - return index; - ++index; + auto cameras = cm_->cameras(); + for (const auto camera : utils::enumerate(cameras)) { + if (camera.value == target) + return camera.index; } return -1;
Use the newly introduced utils::enumerate() to replace manual loop counters. A local variable is needed, as utils::enumerate() requires an lvalue reference. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/v4l2/v4l2_compat_manager.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)