| Message ID | 20210422021809.520675-5-hiroh@chromium.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Hiro, On Thu, Apr 22, 2021 at 11:18:09AM +0900, Hirokazu Honda wrote: > V4L2Device::updateControls() takes two arguments, raw array and > its size, for the v4l2_ext_control values. This replaces it with > libcamera::Span. Now that v4l2Ctrls is an std::vector<>, can't updateControls() take a reference to it, instead of going through a Span<> ? Thanks j > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > include/libcamera/internal/v4l2_device.h | 4 ++-- > src/libcamera/v4l2_device.cpp | 11 ++++------- > 2 files changed, 6 insertions(+), 9 deletions(-) > > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h > index d006bf68..087f07e7 100644 > --- a/include/libcamera/internal/v4l2_device.h > +++ b/include/libcamera/internal/v4l2_device.h > @@ -14,6 +14,7 @@ > #include <linux/videodev2.h> > > #include <libcamera/signal.h> > +#include <libcamera/span.h> > > #include "libcamera/internal/log.h" > #include "libcamera/internal/v4l2_controls.h" > @@ -55,8 +56,7 @@ protected: > private: > void listControls(); > void updateControls(ControlList *ctrls, > - const struct v4l2_ext_control *v4l2Ctrls, > - unsigned int count); > + Span<const v4l2_ext_control> v4l2Ctrls); > > void eventAvailable(EventNotifier *notifier); > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > index 9f71bf0e..58516609 100644 > --- a/src/libcamera/v4l2_device.cpp > +++ b/src/libcamera/v4l2_device.cpp > @@ -244,7 +244,7 @@ ControlList V4L2Device::getControls(const std::vector<uint32_t> &ids) > v4l2Ctrls.resize(errorIdx); > } > > - updateControls(&ctrls, v4l2Ctrls.data(), v4l2Ctrls.size()); > + updateControls(&ctrls, v4l2Ctrls); > > return ctrls; > } > @@ -343,7 +343,7 @@ int V4L2Device::setControls(ControlList *ctrls) > ret = errorIdx; > } > > - updateControls(ctrls, v4l2Ctrls.data(), v4l2Ctrls.size()); > + updateControls(ctrls, v4l2Ctrls); > > return ret; > } > @@ -507,14 +507,11 @@ void V4L2Device::listControls() > * values in \a v4l2Ctrls > * \param[inout] ctrls List of V4L2 controls to update > * \param[in] v4l2Ctrls List of V4L2 extended controls as returned by the driver > - * \param[in] count The number of controls to update > */ > void V4L2Device::updateControls(ControlList *ctrls, > - const struct v4l2_ext_control *v4l2Ctrls, > - unsigned int count) > + Span<const v4l2_ext_control> v4l2Ctrls) > { > - for (unsigned int i = 0; i < count; i++) { > - const struct v4l2_ext_control &v4l2Ctrl = v4l2Ctrls[i]; > + for (const v4l2_ext_control &v4l2Ctrl : v4l2Ctrls) { > const unsigned int id = v4l2Ctrl.id; > if (!ctrls->contains(id)) { > LOG(V4L2, Error) << "ControlList doesn't contain id: " > -- > 2.31.1.368.gbe11c130af-goog > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, On Thu, Apr 22, 2021 at 09:52:21AM +0200, Jacopo Mondi wrote: > On Thu, Apr 22, 2021 at 11:18:09AM +0900, Hirokazu Honda wrote: > > V4L2Device::updateControls() takes two arguments, raw array and > > its size, for the v4l2_ext_control values. This replaces it with > > libcamera::Span. > > Now that v4l2Ctrls is an std::vector<>, can't updateControls() take a > reference to it, instead of going through a Span<> ? That's what v3 did, and I proposed using span... > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > --- > > include/libcamera/internal/v4l2_device.h | 4 ++-- > > src/libcamera/v4l2_device.cpp | 11 ++++------- > > 2 files changed, 6 insertions(+), 9 deletions(-) > > > > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h > > index d006bf68..087f07e7 100644 > > --- a/include/libcamera/internal/v4l2_device.h > > +++ b/include/libcamera/internal/v4l2_device.h > > @@ -14,6 +14,7 @@ > > #include <linux/videodev2.h> > > > > #include <libcamera/signal.h> > > +#include <libcamera/span.h> > > > > #include "libcamera/internal/log.h" > > #include "libcamera/internal/v4l2_controls.h" > > @@ -55,8 +56,7 @@ protected: > > private: > > void listControls(); > > void updateControls(ControlList *ctrls, > > - const struct v4l2_ext_control *v4l2Ctrls, > > - unsigned int count); > > + Span<const v4l2_ext_control> v4l2Ctrls); > > > > void eventAvailable(EventNotifier *notifier); > > > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > > index 9f71bf0e..58516609 100644 > > --- a/src/libcamera/v4l2_device.cpp > > +++ b/src/libcamera/v4l2_device.cpp > > @@ -244,7 +244,7 @@ ControlList V4L2Device::getControls(const std::vector<uint32_t> &ids) > > v4l2Ctrls.resize(errorIdx); > > } > > > > - updateControls(&ctrls, v4l2Ctrls.data(), v4l2Ctrls.size()); > > + updateControls(&ctrls, v4l2Ctrls); > > > > return ctrls; > > } > > @@ -343,7 +343,7 @@ int V4L2Device::setControls(ControlList *ctrls) > > ret = errorIdx; > > } > > > > - updateControls(ctrls, v4l2Ctrls.data(), v4l2Ctrls.size()); > > + updateControls(ctrls, v4l2Ctrls); > > > > return ret; > > } > > @@ -507,14 +507,11 @@ void V4L2Device::listControls() > > * values in \a v4l2Ctrls > > * \param[inout] ctrls List of V4L2 controls to update > > * \param[in] v4l2Ctrls List of V4L2 extended controls as returned by the driver > > - * \param[in] count The number of controls to update > > */ > > void V4L2Device::updateControls(ControlList *ctrls, > > - const struct v4l2_ext_control *v4l2Ctrls, > > - unsigned int count) > > + Span<const v4l2_ext_control> v4l2Ctrls) > > { > > - for (unsigned int i = 0; i < count; i++) { > > - const struct v4l2_ext_control &v4l2Ctrl = v4l2Ctrls[i]; > > + for (const v4l2_ext_control &v4l2Ctrl : v4l2Ctrls) { > > const unsigned int id = v4l2Ctrl.id; > > if (!ctrls->contains(id)) { > > LOG(V4L2, Error) << "ControlList doesn't contain id: "
On Fri, Apr 23, 2021 at 12:03 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Jacopo, > > On Thu, Apr 22, 2021 at 09:52:21AM +0200, Jacopo Mondi wrote: > > On Thu, Apr 22, 2021 at 11:18:09AM +0900, Hirokazu Honda wrote: > > > V4L2Device::updateControls() takes two arguments, raw array and > > > its size, for the v4l2_ext_control values. This replaces it with > > > libcamera::Span. > > > > Now that v4l2Ctrls is an std::vector<>, can't updateControls() take a > > reference to it, instead of going through a Span<> ? > > That's what v3 did, and I proposed using span... > Yep, mail-based review doesn't easily show us the review comments in the previous version. :( That's why I prefer some web app review site like GitHub or Gerrit, which most of you don't like. :p Either Span or vector is fine to me. Please feel free to request me. -Hiro > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > --- > > > include/libcamera/internal/v4l2_device.h | 4 ++-- > > > src/libcamera/v4l2_device.cpp | 11 ++++------- > > > 2 files changed, 6 insertions(+), 9 deletions(-) > > > > > > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h > > > index d006bf68..087f07e7 100644 > > > --- a/include/libcamera/internal/v4l2_device.h > > > +++ b/include/libcamera/internal/v4l2_device.h > > > @@ -14,6 +14,7 @@ > > > #include <linux/videodev2.h> > > > > > > #include <libcamera/signal.h> > > > +#include <libcamera/span.h> > > > > > > #include "libcamera/internal/log.h" > > > #include "libcamera/internal/v4l2_controls.h" > > > @@ -55,8 +56,7 @@ protected: > > > private: > > > void listControls(); > > > void updateControls(ControlList *ctrls, > > > - const struct v4l2_ext_control *v4l2Ctrls, > > > - unsigned int count); > > > + Span<const v4l2_ext_control> v4l2Ctrls); > > > > > > void eventAvailable(EventNotifier *notifier); > > > > > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > > > index 9f71bf0e..58516609 100644 > > > --- a/src/libcamera/v4l2_device.cpp > > > +++ b/src/libcamera/v4l2_device.cpp > > > @@ -244,7 +244,7 @@ ControlList V4L2Device::getControls(const std::vector<uint32_t> &ids) > > > v4l2Ctrls.resize(errorIdx); > > > } > > > > > > - updateControls(&ctrls, v4l2Ctrls.data(), v4l2Ctrls.size()); > > > + updateControls(&ctrls, v4l2Ctrls); > > > > > > return ctrls; > > > } > > > @@ -343,7 +343,7 @@ int V4L2Device::setControls(ControlList *ctrls) > > > ret = errorIdx; > > > } > > > > > > - updateControls(ctrls, v4l2Ctrls.data(), v4l2Ctrls.size()); > > > + updateControls(ctrls, v4l2Ctrls); > > > > > > return ret; > > > } > > > @@ -507,14 +507,11 @@ void V4L2Device::listControls() > > > * values in \a v4l2Ctrls > > > * \param[inout] ctrls List of V4L2 controls to update > > > * \param[in] v4l2Ctrls List of V4L2 extended controls as returned by the driver > > > - * \param[in] count The number of controls to update > > > */ > > > void V4L2Device::updateControls(ControlList *ctrls, > > > - const struct v4l2_ext_control *v4l2Ctrls, > > > - unsigned int count) > > > + Span<const v4l2_ext_control> v4l2Ctrls) > > > { > > > - for (unsigned int i = 0; i < count; i++) { > > > - const struct v4l2_ext_control &v4l2Ctrl = v4l2Ctrls[i]; > > > + for (const v4l2_ext_control &v4l2Ctrl : v4l2Ctrls) { > > > const unsigned int id = v4l2Ctrl.id; > > > if (!ctrls->contains(id)) { > > > LOG(V4L2, Error) << "ControlList doesn't contain id: " > > -- > Regards, > > Laurent Pinchart
Hello, On Fri, Apr 23, 2021 at 01:42:00PM +0900, Hirokazu Honda wrote: > On Fri, Apr 23, 2021 at 12:03 PM Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: > > > > Hi Jacopo, > > > > On Thu, Apr 22, 2021 at 09:52:21AM +0200, Jacopo Mondi wrote: > > > On Thu, Apr 22, 2021 at 11:18:09AM +0900, Hirokazu Honda wrote: > > > > V4L2Device::updateControls() takes two arguments, raw array and > > > > its size, for the v4l2_ext_control values. This replaces it with > > > > libcamera::Span. > > > > > > Now that v4l2Ctrls is an std::vector<>, can't updateControls() take a > > > reference to it, instead of going through a Span<> ? > > > > That's what v3 did, and I proposed using span... > > > > Yep, mail-based review doesn't easily show us the review comments in > the previous version. :( > That's why I prefer some web app review site like GitHub or Gerrit, > which most of you don't like. :p > > Either Span or vector is fine to me. Please feel free to request me. I welcome Span<> when it allows us to wrap a memory location and its size is a convenient way, but what do we gain here where we already have a container ? > > -Hiro > > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > --- > > > > include/libcamera/internal/v4l2_device.h | 4 ++-- > > > > src/libcamera/v4l2_device.cpp | 11 ++++------- > > > > 2 files changed, 6 insertions(+), 9 deletions(-) > > > > > > > > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h > > > > index d006bf68..087f07e7 100644 > > > > --- a/include/libcamera/internal/v4l2_device.h > > > > +++ b/include/libcamera/internal/v4l2_device.h > > > > @@ -14,6 +14,7 @@ > > > > #include <linux/videodev2.h> > > > > > > > > #include <libcamera/signal.h> > > > > +#include <libcamera/span.h> > > > > > > > > #include "libcamera/internal/log.h" > > > > #include "libcamera/internal/v4l2_controls.h" > > > > @@ -55,8 +56,7 @@ protected: > > > > private: > > > > void listControls(); > > > > void updateControls(ControlList *ctrls, > > > > - const struct v4l2_ext_control *v4l2Ctrls, > > > > - unsigned int count); > > > > + Span<const v4l2_ext_control> v4l2Ctrls); > > > > > > > > void eventAvailable(EventNotifier *notifier); > > > > > > > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > > > > index 9f71bf0e..58516609 100644 > > > > --- a/src/libcamera/v4l2_device.cpp > > > > +++ b/src/libcamera/v4l2_device.cpp > > > > @@ -244,7 +244,7 @@ ControlList V4L2Device::getControls(const std::vector<uint32_t> &ids) > > > > v4l2Ctrls.resize(errorIdx); > > > > } > > > > > > > > - updateControls(&ctrls, v4l2Ctrls.data(), v4l2Ctrls.size()); > > > > + updateControls(&ctrls, v4l2Ctrls); > > > > > > > > return ctrls; > > > > } > > > > @@ -343,7 +343,7 @@ int V4L2Device::setControls(ControlList *ctrls) > > > > ret = errorIdx; > > > > } > > > > > > > > - updateControls(ctrls, v4l2Ctrls.data(), v4l2Ctrls.size()); > > > > + updateControls(ctrls, v4l2Ctrls); > > > > > > > > return ret; > > > > } > > > > @@ -507,14 +507,11 @@ void V4L2Device::listControls() > > > > * values in \a v4l2Ctrls > > > > * \param[inout] ctrls List of V4L2 controls to update > > > > * \param[in] v4l2Ctrls List of V4L2 extended controls as returned by the driver > > > > - * \param[in] count The number of controls to update > > > > */ > > > > void V4L2Device::updateControls(ControlList *ctrls, > > > > - const struct v4l2_ext_control *v4l2Ctrls, > > > > - unsigned int count) > > > > + Span<const v4l2_ext_control> v4l2Ctrls) > > > > { > > > > - for (unsigned int i = 0; i < count; i++) { > > > > - const struct v4l2_ext_control &v4l2Ctrl = v4l2Ctrls[i]; > > > > + for (const v4l2_ext_control &v4l2Ctrl : v4l2Ctrls) { > > > > const unsigned int id = v4l2Ctrl.id; > > > > if (!ctrls->contains(id)) { > > > > LOG(V4L2, Error) << "ControlList doesn't contain id: " > > > > -- > > Regards, > > > > Laurent Pinchart
Hi Jacopo, On Fri, Apr 23, 2021 at 09:05:33AM +0200, Jacopo Mondi wrote: > On Fri, Apr 23, 2021 at 01:42:00PM +0900, Hirokazu Honda wrote: > > On Fri, Apr 23, 2021 at 12:03 PM Laurent Pinchart wrote: > > > On Thu, Apr 22, 2021 at 09:52:21AM +0200, Jacopo Mondi wrote: > > > > On Thu, Apr 22, 2021 at 11:18:09AM +0900, Hirokazu Honda wrote: > > > > > V4L2Device::updateControls() takes two arguments, raw array and > > > > > its size, for the v4l2_ext_control values. This replaces it with > > > > > libcamera::Span. > > > > > > > > Now that v4l2Ctrls is an std::vector<>, can't updateControls() take a > > > > reference to it, instead of going through a Span<> ? > > > > > > That's what v3 did, and I proposed using span... > > > > Yep, mail-based review doesn't easily show us the review comments in > > the previous version. :( > > That's why I prefer some web app review site like GitHub or Gerrit, > > which most of you don't like. :p > > > > Either Span or vector is fine to me. Please feel free to request me. > > I welcome Span<> when it allows us to wrap a memory location and its > size is a convenient way, but what do we gain here where we already > have a container ? Generally speaking, when a function gets a contiguous array of elements with a size, Span is more generic, as other containers than std::vector<> could be used. In this case, you're right that it makes no difference. updateControls() is an internal API. Making it not depend on the container being a vector would in theory make the function more generic, but there's very little chance (or risk :-)) we'll extend the usage. > > > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > --- > > > > > include/libcamera/internal/v4l2_device.h | 4 ++-- > > > > > src/libcamera/v4l2_device.cpp | 11 ++++------- > > > > > 2 files changed, 6 insertions(+), 9 deletions(-) > > > > > > > > > > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h > > > > > index d006bf68..087f07e7 100644 > > > > > --- a/include/libcamera/internal/v4l2_device.h > > > > > +++ b/include/libcamera/internal/v4l2_device.h > > > > > @@ -14,6 +14,7 @@ > > > > > #include <linux/videodev2.h> > > > > > > > > > > #include <libcamera/signal.h> > > > > > +#include <libcamera/span.h> > > > > > > > > > > #include "libcamera/internal/log.h" > > > > > #include "libcamera/internal/v4l2_controls.h" > > > > > @@ -55,8 +56,7 @@ protected: > > > > > private: > > > > > void listControls(); > > > > > void updateControls(ControlList *ctrls, > > > > > - const struct v4l2_ext_control *v4l2Ctrls, > > > > > - unsigned int count); > > > > > + Span<const v4l2_ext_control> v4l2Ctrls); > > > > > > > > > > void eventAvailable(EventNotifier *notifier); > > > > > > > > > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > > > > > index 9f71bf0e..58516609 100644 > > > > > --- a/src/libcamera/v4l2_device.cpp > > > > > +++ b/src/libcamera/v4l2_device.cpp > > > > > @@ -244,7 +244,7 @@ ControlList V4L2Device::getControls(const std::vector<uint32_t> &ids) > > > > > v4l2Ctrls.resize(errorIdx); > > > > > } > > > > > > > > > > - updateControls(&ctrls, v4l2Ctrls.data(), v4l2Ctrls.size()); > > > > > + updateControls(&ctrls, v4l2Ctrls); > > > > > > > > > > return ctrls; > > > > > } > > > > > @@ -343,7 +343,7 @@ int V4L2Device::setControls(ControlList *ctrls) > > > > > ret = errorIdx; > > > > > } > > > > > > > > > > - updateControls(ctrls, v4l2Ctrls.data(), v4l2Ctrls.size()); > > > > > + updateControls(ctrls, v4l2Ctrls); > > > > > > > > > > return ret; > > > > > } > > > > > @@ -507,14 +507,11 @@ void V4L2Device::listControls() > > > > > * values in \a v4l2Ctrls > > > > > * \param[inout] ctrls List of V4L2 controls to update > > > > > * \param[in] v4l2Ctrls List of V4L2 extended controls as returned by the driver > > > > > - * \param[in] count The number of controls to update > > > > > */ > > > > > void V4L2Device::updateControls(ControlList *ctrls, > > > > > - const struct v4l2_ext_control *v4l2Ctrls, > > > > > - unsigned int count) > > > > > + Span<const v4l2_ext_control> v4l2Ctrls) > > > > > { > > > > > - for (unsigned int i = 0; i < count; i++) { > > > > > - const struct v4l2_ext_control &v4l2Ctrl = v4l2Ctrls[i]; > > > > > + for (const v4l2_ext_control &v4l2Ctrl : v4l2Ctrls) { > > > > > const unsigned int id = v4l2Ctrl.id; > > > > > if (!ctrls->contains(id)) { > > > > > LOG(V4L2, Error) << "ControlList doesn't contain id: "
Hi Hiro, On Fri, Apr 23, 2021 at 01:42:00PM +0900, Hirokazu Honda wrote: > On Fri, Apr 23, 2021 at 12:03 PM Laurent Pinchart wrote: > > On Thu, Apr 22, 2021 at 09:52:21AM +0200, Jacopo Mondi wrote: > > > On Thu, Apr 22, 2021 at 11:18:09AM +0900, Hirokazu Honda wrote: > > > > V4L2Device::updateControls() takes two arguments, raw array and > > > > its size, for the v4l2_ext_control values. This replaces it with > > > > libcamera::Span. > > > > > > Now that v4l2Ctrls is an std::vector<>, can't updateControls() take a > > > reference to it, instead of going through a Span<> ? > > > > That's what v3 did, and I proposed using span... > > Yep, mail-based review doesn't easily show us the review comments in > the previous version. :( > That's why I prefer some web app review site like GitHub or Gerrit, > which most of you don't like. :p You're right that this is a useful feature of gerrit. There are more drawbacks than advantages in gerrit compared to e-mails in my opinion (and that's only my opinion, with my use cases, my workflow, and my bias), but I have to recognize that there are useful features in gerrit. As for github (and gitlab), that's not comparable, they're just unusable for review :-) > Either Span or vector is fine to me. Please feel free to request me. Overall, I think Span is more generic, and I have a preference for using it, as a coding guideline. In this specific case, however, std::vector<> would work too, it's an internal API, it doesn't matter much. If you or Jacopo think that std::vector<> would be better, I'm fine with that. > > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > --- > > > > include/libcamera/internal/v4l2_device.h | 4 ++-- > > > > src/libcamera/v4l2_device.cpp | 11 ++++------- > > > > 2 files changed, 6 insertions(+), 9 deletions(-) > > > > > > > > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h > > > > index d006bf68..087f07e7 100644 > > > > --- a/include/libcamera/internal/v4l2_device.h > > > > +++ b/include/libcamera/internal/v4l2_device.h > > > > @@ -14,6 +14,7 @@ > > > > #include <linux/videodev2.h> > > > > > > > > #include <libcamera/signal.h> > > > > +#include <libcamera/span.h> > > > > > > > > #include "libcamera/internal/log.h" > > > > #include "libcamera/internal/v4l2_controls.h" > > > > @@ -55,8 +56,7 @@ protected: > > > > private: > > > > void listControls(); > > > > void updateControls(ControlList *ctrls, > > > > - const struct v4l2_ext_control *v4l2Ctrls, > > > > - unsigned int count); > > > > + Span<const v4l2_ext_control> v4l2Ctrls); > > > > > > > > void eventAvailable(EventNotifier *notifier); > > > > > > > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > > > > index 9f71bf0e..58516609 100644 > > > > --- a/src/libcamera/v4l2_device.cpp > > > > +++ b/src/libcamera/v4l2_device.cpp > > > > @@ -244,7 +244,7 @@ ControlList V4L2Device::getControls(const std::vector<uint32_t> &ids) > > > > v4l2Ctrls.resize(errorIdx); > > > > } > > > > > > > > - updateControls(&ctrls, v4l2Ctrls.data(), v4l2Ctrls.size()); > > > > + updateControls(&ctrls, v4l2Ctrls); > > > > > > > > return ctrls; > > > > } > > > > @@ -343,7 +343,7 @@ int V4L2Device::setControls(ControlList *ctrls) > > > > ret = errorIdx; > > > > } > > > > > > > > - updateControls(ctrls, v4l2Ctrls.data(), v4l2Ctrls.size()); > > > > + updateControls(ctrls, v4l2Ctrls); > > > > > > > > return ret; > > > > } > > > > @@ -507,14 +507,11 @@ void V4L2Device::listControls() > > > > * values in \a v4l2Ctrls > > > > * \param[inout] ctrls List of V4L2 controls to update > > > > * \param[in] v4l2Ctrls List of V4L2 extended controls as returned by the driver > > > > - * \param[in] count The number of controls to update > > > > */ > > > > void V4L2Device::updateControls(ControlList *ctrls, > > > > - const struct v4l2_ext_control *v4l2Ctrls, > > > > - unsigned int count) > > > > + Span<const v4l2_ext_control> v4l2Ctrls) > > > > { > > > > - for (unsigned int i = 0; i < count; i++) { > > > > - const struct v4l2_ext_control &v4l2Ctrl = v4l2Ctrls[i]; > > > > + for (const v4l2_ext_control &v4l2Ctrl : v4l2Ctrls) { > > > > const unsigned int id = v4l2Ctrl.id; > > > > if (!ctrls->contains(id)) { > > > > LOG(V4L2, Error) << "ControlList doesn't contain id: "
diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h index d006bf68..087f07e7 100644 --- a/include/libcamera/internal/v4l2_device.h +++ b/include/libcamera/internal/v4l2_device.h @@ -14,6 +14,7 @@ #include <linux/videodev2.h> #include <libcamera/signal.h> +#include <libcamera/span.h> #include "libcamera/internal/log.h" #include "libcamera/internal/v4l2_controls.h" @@ -55,8 +56,7 @@ protected: private: void listControls(); void updateControls(ControlList *ctrls, - const struct v4l2_ext_control *v4l2Ctrls, - unsigned int count); + Span<const v4l2_ext_control> v4l2Ctrls); void eventAvailable(EventNotifier *notifier); diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 9f71bf0e..58516609 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -244,7 +244,7 @@ ControlList V4L2Device::getControls(const std::vector<uint32_t> &ids) v4l2Ctrls.resize(errorIdx); } - updateControls(&ctrls, v4l2Ctrls.data(), v4l2Ctrls.size()); + updateControls(&ctrls, v4l2Ctrls); return ctrls; } @@ -343,7 +343,7 @@ int V4L2Device::setControls(ControlList *ctrls) ret = errorIdx; } - updateControls(ctrls, v4l2Ctrls.data(), v4l2Ctrls.size()); + updateControls(ctrls, v4l2Ctrls); return ret; } @@ -507,14 +507,11 @@ void V4L2Device::listControls() * values in \a v4l2Ctrls * \param[inout] ctrls List of V4L2 controls to update * \param[in] v4l2Ctrls List of V4L2 extended controls as returned by the driver - * \param[in] count The number of controls to update */ void V4L2Device::updateControls(ControlList *ctrls, - const struct v4l2_ext_control *v4l2Ctrls, - unsigned int count) + Span<const v4l2_ext_control> v4l2Ctrls) { - for (unsigned int i = 0; i < count; i++) { - const struct v4l2_ext_control &v4l2Ctrl = v4l2Ctrls[i]; + for (const v4l2_ext_control &v4l2Ctrl : v4l2Ctrls) { const unsigned int id = v4l2Ctrl.id; if (!ctrls->contains(id)) { LOG(V4L2, Error) << "ControlList doesn't contain id: "