[{"id":2727,"web_url":"https://patchwork.libcamera.org/comment/2727/","msgid":"<20190929101308.6tirybtacyez3do5@uno.localdomain>","date":"2019-09-29T10:13:08","subject":"Re: [libcamera-devel] [PATCH 09/12] libcamera: v4l2_controls: Use\n\tthe ControlRange class for control info","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Sat, Sep 28, 2019 at 06:22:35PM +0300, Laurent Pinchart wrote:\n> Use the ControlRange class to express the range of a V4L2 control,\n> replacing the open-coded minimum and maximum fields in the\n> V4L2ControlInfo class.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  src/libcamera/include/v4l2_controls.h |  6 ++----\n>  src/libcamera/pipeline/uvcvideo.cpp   |  2 +-\n>  src/libcamera/pipeline/vimc.cpp       |  2 +-\n>  src/libcamera/v4l2_controls.cpp       | 21 ++++++++++-----------\n>  4 files changed, 14 insertions(+), 17 deletions(-)\n>\n> diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\n> index f2b67c5d44e1..b39370b2e90e 100644\n> --- a/src/libcamera/include/v4l2_controls.h\n> +++ b/src/libcamera/include/v4l2_controls.h\n> @@ -30,8 +30,7 @@ public:\n>  \tsize_t size() const { return size_; }\n>  \tconst std::string &name() const { return name_; }\n\nSame question as I had on Controls. Do we want to serialize and\ntransport names ?\n\nAlso, not V4L2ControlInfo seems more like to be a V4L2ControlId, but\nthat's just naming, so it's fine\n\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\nThanks\n   j\n\n\n>\n> -\tint64_t min() const { return min_; }\n> -\tint64_t max() const { return max_; }\n> +\tconst ControlRange &range() const { return range_; }\n>\n>  private:\n>  \tunsigned int id_;\n> @@ -39,8 +38,7 @@ private:\n>  \tsize_t size_;\n>  \tstd::string name_;\n>\n> -\tint64_t min_;\n> -\tint64_t max_;\n> +\tControlRange range_;\n>  };\n>\n>  using V4L2ControlInfoMap = std::map<unsigned int, V4L2ControlInfo>;\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index a80f8a43a116..17bbbad15838 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -364,7 +364,7 @@ int UVCCameraData::init(MediaEntity *entity)\n>\n>  \t\tcontrolInfo_.emplace(std::piecewise_construct,\n>  \t\t\t\t     std::forward_as_tuple(id),\n> -\t\t\t\t     std::forward_as_tuple(info.min(), info.max()));\n> +\t\t\t\t     std::forward_as_tuple(info.range()));\n>  \t}\n>\n>  \treturn 0;\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index 5d6909f58cf9..f9a7feb6571a 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -437,7 +437,7 @@ int VimcCameraData::init(MediaDevice *media)\n>\n>  \t\tcontrolInfo_.emplace(std::piecewise_construct,\n>  \t\t\t\t     std::forward_as_tuple(id),\n> -\t\t\t\t     std::forward_as_tuple(info.min(), info.max()));\n> +\t\t\t\t     std::forward_as_tuple(info.range()));\n>  \t}\n>\n>  \treturn 0;\n> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> index 64f0555fff7c..6f5f1578b139 100644\n> --- a/src/libcamera/v4l2_controls.cpp\n> +++ b/src/libcamera/v4l2_controls.cpp\n> @@ -74,8 +74,13 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n>  \ttype_ = ctrl.type;\n>  \tname_ = static_cast<const char *>(ctrl.name);\n>  \tsize_ = ctrl.elem_size * ctrl.elems;\n> -\tmin_ = ctrl.minimum;\n> -\tmax_ = ctrl.maximum;\n> +\n> +\tif (ctrl.type == V4L2_CTRL_TYPE_INTEGER64)\n> +\t\trange_ = ControlRange(static_cast<int64_t>(ctrl.minimum),\n> +\t\t\t\t      static_cast<int64_t>(ctrl.maximum));\n> +\telse\n> +\t\trange_ = ControlRange(static_cast<int32_t>(ctrl.minimum),\n> +\t\t\t\t      static_cast<int32_t>(ctrl.maximum));\n>  }\n>\n>  /**\n> @@ -103,15 +108,9 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n>   */\n>\n>  /**\n> - * \\fn V4L2ControlInfo::min()\n> - * \\brief Retrieve the control minimum value\n> - * \\return The V4L2 control minimum value\n> - */\n> -\n> -/**\n> - * \\fn V4L2ControlInfo::max()\n> - * \\brief Retrieve the control maximum value\n> - * \\return The V4L2 control maximum value\n> + * \\fn V4L2ControlInfo::range()\n> + * \\brief Retrieve the control value range\n> + * \\return The V4L2 control value range\n>   */\n>\n>  /**\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CD96F61654\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 29 Sep 2019 12:11:27 +0200 (CEST)","from uno.localdomain\n\t(host7-199-dynamic.171-212-r.retail.telecomitalia.it [212.171.199.7])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay10.mail.gandi.net (Postfix) with ESMTPSA id 7A73B240004;\n\tSun, 29 Sep 2019 10:11:26 +0000 (UTC)"],"Date":"Sun, 29 Sep 2019 12:13:08 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190929101308.6tirybtacyez3do5@uno.localdomain>","References":"<20190928152238.23752-1-laurent.pinchart@ideasonboard.com>\n\t<20190928152238.23752-10-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"7hjgvp4cjaorbzod\"","Content-Disposition":"inline","In-Reply-To":"<20190928152238.23752-10-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 09/12] libcamera: v4l2_controls: Use\n\tthe ControlRange class for control info","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Sun, 29 Sep 2019 10:11:28 -0000"}},{"id":2735,"web_url":"https://patchwork.libcamera.org/comment/2735/","msgid":"<20190929133409.GG4827@pendragon.ideasonboard.com>","date":"2019-09-29T13:34:09","subject":"Re: [libcamera-devel] [PATCH 09/12] libcamera: v4l2_controls: Use\n\tthe ControlRange class for control info","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Sun, Sep 29, 2019 at 12:13:08PM +0200, Jacopo Mondi wrote:\n> On Sat, Sep 28, 2019 at 06:22:35PM +0300, Laurent Pinchart wrote:\n> > Use the ControlRange class to express the range of a V4L2 control,\n> > replacing the open-coded minimum and maximum fields in the\n> > V4L2ControlInfo class.\n> >\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  src/libcamera/include/v4l2_controls.h |  6 ++----\n> >  src/libcamera/pipeline/uvcvideo.cpp   |  2 +-\n> >  src/libcamera/pipeline/vimc.cpp       |  2 +-\n> >  src/libcamera/v4l2_controls.cpp       | 21 ++++++++++-----------\n> >  4 files changed, 14 insertions(+), 17 deletions(-)\n> >\n> > diff --git a/src/libcamera/include/v4l2_controls.h b/src/libcamera/include/v4l2_controls.h\n> > index f2b67c5d44e1..b39370b2e90e 100644\n> > --- a/src/libcamera/include/v4l2_controls.h\n> > +++ b/src/libcamera/include/v4l2_controls.h\n> > @@ -30,8 +30,7 @@ public:\n> >  \tsize_t size() const { return size_; }\n> >  \tconst std::string &name() const { return name_; }\n> \n> Same question as I had on Controls. Do we want to serialize and\n> transport names ?\n\nNo, there's no need to.\n\n> Also, not V4L2ControlInfo seems more like to be a V4L2ControlId, but\n> that's just naming, so it's fine\n> \n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> \n> > -\tint64_t min() const { return min_; }\n> > -\tint64_t max() const { return max_; }\n> > +\tconst ControlRange &range() const { return range_; }\n> >\n> >  private:\n> >  \tunsigned int id_;\n> > @@ -39,8 +38,7 @@ private:\n> >  \tsize_t size_;\n> >  \tstd::string name_;\n> >\n> > -\tint64_t min_;\n> > -\tint64_t max_;\n> > +\tControlRange range_;\n> >  };\n> >\n> >  using V4L2ControlInfoMap = std::map<unsigned int, V4L2ControlInfo>;\n> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> > index a80f8a43a116..17bbbad15838 100644\n> > --- a/src/libcamera/pipeline/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> > @@ -364,7 +364,7 @@ int UVCCameraData::init(MediaEntity *entity)\n> >\n> >  \t\tcontrolInfo_.emplace(std::piecewise_construct,\n> >  \t\t\t\t     std::forward_as_tuple(id),\n> > -\t\t\t\t     std::forward_as_tuple(info.min(), info.max()));\n> > +\t\t\t\t     std::forward_as_tuple(info.range()));\n> >  \t}\n> >\n> >  \treturn 0;\n> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> > index 5d6909f58cf9..f9a7feb6571a 100644\n> > --- a/src/libcamera/pipeline/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc.cpp\n> > @@ -437,7 +437,7 @@ int VimcCameraData::init(MediaDevice *media)\n> >\n> >  \t\tcontrolInfo_.emplace(std::piecewise_construct,\n> >  \t\t\t\t     std::forward_as_tuple(id),\n> > -\t\t\t\t     std::forward_as_tuple(info.min(), info.max()));\n> > +\t\t\t\t     std::forward_as_tuple(info.range()));\n> >  \t}\n> >\n> >  \treturn 0;\n> > diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp\n> > index 64f0555fff7c..6f5f1578b139 100644\n> > --- a/src/libcamera/v4l2_controls.cpp\n> > +++ b/src/libcamera/v4l2_controls.cpp\n> > @@ -74,8 +74,13 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n> >  \ttype_ = ctrl.type;\n> >  \tname_ = static_cast<const char *>(ctrl.name);\n> >  \tsize_ = ctrl.elem_size * ctrl.elems;\n> > -\tmin_ = ctrl.minimum;\n> > -\tmax_ = ctrl.maximum;\n> > +\n> > +\tif (ctrl.type == V4L2_CTRL_TYPE_INTEGER64)\n> > +\t\trange_ = ControlRange(static_cast<int64_t>(ctrl.minimum),\n> > +\t\t\t\t      static_cast<int64_t>(ctrl.maximum));\n> > +\telse\n> > +\t\trange_ = ControlRange(static_cast<int32_t>(ctrl.minimum),\n> > +\t\t\t\t      static_cast<int32_t>(ctrl.maximum));\n> >  }\n> >\n> >  /**\n> > @@ -103,15 +108,9 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)\n> >   */\n> >\n> >  /**\n> > - * \\fn V4L2ControlInfo::min()\n> > - * \\brief Retrieve the control minimum value\n> > - * \\return The V4L2 control minimum value\n> > - */\n> > -\n> > -/**\n> > - * \\fn V4L2ControlInfo::max()\n> > - * \\brief Retrieve the control maximum value\n> > - * \\return The V4L2 control maximum value\n> > + * \\fn V4L2ControlInfo::range()\n> > + * \\brief Retrieve the control value range\n> > + * \\return The V4L2 control value range\n> >   */\n> >\n> >  /**","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E952061654\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 29 Sep 2019 15:34:20 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5204B320;\n\tSun, 29 Sep 2019 15:34:20 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1569764060;\n\tbh=dGQbObgzQPQmSG5HWJemSzYahkdfW8SrUwXRoSAgq7w=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OMKQX4XuKkzMd740xFn9z/kDob3HqRkTqKlj6zQqJSFDkgPIcVRb5DwUVICdlv6iv\n\tS+X6gnEzpyx5JNXLrG9wkZB3yDa3o/CCkAG1ux3ats9MTo02y9sahpwLi6opc5bpHp\n\tahybI8JsckoTZVM9q7lXbkBdfywDtpEUIZmnVLas=","Date":"Sun, 29 Sep 2019 16:34:09 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190929133409.GG4827@pendragon.ideasonboard.com>","References":"<20190928152238.23752-1-laurent.pinchart@ideasonboard.com>\n\t<20190928152238.23752-10-laurent.pinchart@ideasonboard.com>\n\t<20190929101308.6tirybtacyez3do5@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190929101308.6tirybtacyez3do5@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 09/12] libcamera: v4l2_controls: Use\n\tthe ControlRange class for control info","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Sun, 29 Sep 2019 13:34:21 -0000"}}]