[{"id":2721,"web_url":"https://patchwork.libcamera.org/comment/2721/","msgid":"<20190929090848.gzeodofx3jb7oa5f@uno.localdomain>","date":"2019-09-29T09:08:48","subject":"Re: [libcamera-devel] [PATCH 03/12] libcamera: controls: Use\n\texplicit 32-bit integer types","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:29PM +0300, Laurent Pinchart wrote:\n> Make the control API more explicit when dealing with integer controls by\n> specifying the size. We already do so for 64-bit integers, using int64_t\n> and ControlTypeInteger64, do the same for 32-bit integers.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  include/libcamera/controls.h        |  6 ++---\n>  src/libcamera/controls.cpp          | 36 ++++++++++++++---------------\n>  src/libcamera/pipeline/uvcvideo.cpp | 10 ++++----\n>  src/libcamera/pipeline/vimc.cpp     |  6 ++---\n>  test/controls/control_info.cpp      | 10 ++++----\n>  test/controls/control_list.cpp      | 16 ++++++-------\n>  test/controls/control_value.cpp     |  6 ++---\n>  7 files changed, 46 insertions(+), 44 deletions(-)\n>\n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index 0886370f0901..a3089064c4b5 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -21,7 +21,7 @@ class Camera;\n>  enum ControlType {\n>  \tControlTypeNone,\n>  \tControlTypeBool,\n> -\tControlTypeInteger,\n> +\tControlTypeInteger32,\n>  \tControlTypeInteger64,\n>  };\n>\n> @@ -30,7 +30,7 @@ class ControlValue\n>  public:\n>  \tControlValue();\n>  \tControlValue(bool value);\n> -\tControlValue(int value);\n> +\tControlValue(int32_t value);\n>  \tControlValue(int64_t value);\n>\n>  \tControlType type() const { return type_; };\n> @@ -48,7 +48,7 @@ private:\n>\n>  \tunion {\n>  \t\tbool bool_;\n> -\t\tint integer_;\n> +\t\tint32_t integer32_;\n>  \t\tint64_t integer64_;\n>  \t};\n>  };\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 88aab88db327..295ccd55a622 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -31,8 +31,8 @@ LOG_DEFINE_CATEGORY(Controls)\n>   * Invalid type, for empty values\n>   * \\var ControlTypeBool\n>   * The control stores a boolean value\n> - * \\var ControlTypeInteger\n> - * The control stores an integer value\n> + * \\var ControlTypeInteger32\n> + * The control stores a 32-bit integer value\n>   * \\var ControlTypeInteger64\n>   * The control stores a 64-bit integer value\n>   */\n> @@ -63,8 +63,8 @@ ControlValue::ControlValue(bool value)\n>   * \\brief Construct an integer ControlValue\n>   * \\param[in] value Integer value to store\n>   */\n> -ControlValue::ControlValue(int value)\n> -\t: type_(ControlTypeInteger), integer_(value)\n> +ControlValue::ControlValue(int32_t value)\n> +\t: type_(ControlTypeInteger32), integer32_(value)\n>  {\n>  }\n>\n> @@ -115,17 +115,17 @@ const bool &ControlValue::get<bool>() const\n>  }\n>\n>  template<>\n> -const int &ControlValue::get<int>() const\n> +const int32_t &ControlValue::get<int32_t>() const\n>  {\n> -\tASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64);\n> +\tASSERT(type_ == ControlTypeInteger32 || type_ == ControlTypeInteger64);\n>\n> -\treturn integer_;\n> +\treturn integer32_;\n>  }\n>\n>  template<>\n>  const int64_t &ControlValue::get<int64_t>() const\n>  {\n> -\tASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64);\n> +\tASSERT(type_ == ControlTypeInteger32 || type_ == ControlTypeInteger64);\n>\n>  \treturn integer64_;\n>  }\n> @@ -138,10 +138,10 @@ void ControlValue::set<bool>(const bool &value)\n>  }\n>\n>  template<>\n> -void ControlValue::set<int>(const int &value)\n> +void ControlValue::set<int32_t>(const int32_t &value)\n>  {\n> -\ttype_ = ControlTypeInteger;\n> -\tinteger_ = value;\n> +\ttype_ = ControlTypeInteger32;\n> +\tinteger32_ = value;\n>  }\n>\n>  template<>\n> @@ -163,8 +163,8 @@ std::string ControlValue::toString() const\n>  \t\treturn \"<None>\";\n>  \tcase ControlTypeBool:\n>  \t\treturn bool_ ? \"True\" : \"False\";\n> -\tcase ControlTypeInteger:\n> -\t\treturn std::to_string(integer_);\n> +\tcase ControlTypeInteger32:\n> +\t\treturn std::to_string(integer32_);\n>  \tcase ControlTypeInteger64:\n>  \t\treturn std::to_string(integer64_);\n>  \t}\n> @@ -186,35 +186,35 @@ std::string ControlValue::toString() const\n>\n>  /**\n>   * \\var Brightness\n> - * ControlType: Integer\n> + * ControlType: Integer32\n>   *\n>   * Specify a fixed brightness parameter.\n>   */\n>\n>  /**\n>   * \\var Contrast\n> - * ControlType: Integer\n> + * ControlType: Integer32\n>   *\n>   * Specify a fixed contrast parameter.\n>   */\n>\n>  /**\n>   * \\var Saturation\n> - * ControlType: Integer\n> + * ControlType: Integer32\n>   *\n>   * Specify a fixed saturation parameter.\n>   */\n>\n>  /**\n>   * \\var ManualExposure\n> - * ControlType: Integer\n> + * ControlType: Integer32\n>   *\n>   * Specify a fixed exposure time in milli-seconds\n>   */\n>\n>  /**\n>   * \\var ManualGain\n> - * ControlType: Integer\n> + * ControlType: Integer32\n>   *\n>   * Specify a fixed gain parameter\n>   */\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index 81c548af2c64..0d56758e3e1d 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -235,24 +235,24 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n>\n>  \t\tswitch (ci->id()) {\n>  \t\tcase Brightness:\n> -\t\t\tcontrols.add(V4L2_CID_BRIGHTNESS, value.get<int>());\n> +\t\t\tcontrols.add(V4L2_CID_BRIGHTNESS, value.get<int32_t>());\n\nDue to C++ internal type casting rules (I presume) the previous syntax\nvalue.get<int>() is still legal. I assume the template argument\nsubstitution process matches this with the <int32_t> specialization.\nShouldn't we keep using the shorter one ?\n\nAnyway\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n   j\n\n>  \t\t\tbreak;\n>\n>  \t\tcase Contrast:\n> -\t\t\tcontrols.add(V4L2_CID_CONTRAST, value.get<int>());\n> +\t\t\tcontrols.add(V4L2_CID_CONTRAST, value.get<int32_t>());\n>  \t\t\tbreak;\n>\n>  \t\tcase Saturation:\n> -\t\t\tcontrols.add(V4L2_CID_SATURATION, value.get<int>());\n> +\t\t\tcontrols.add(V4L2_CID_SATURATION, value.get<int32_t>());\n>  \t\t\tbreak;\n>\n>  \t\tcase ManualExposure:\n>  \t\t\tcontrols.add(V4L2_CID_EXPOSURE_AUTO, 1);\n> -\t\t\tcontrols.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get<int>());\n> +\t\t\tcontrols.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get<int32_t>());\n>  \t\t\tbreak;\n>\n>  \t\tcase ManualGain:\n> -\t\t\tcontrols.add(V4L2_CID_GAIN, value.get<int>());\n> +\t\t\tcontrols.add(V4L2_CID_GAIN, value.get<int32_t>());\n>  \t\t\tbreak;\n>\n>  \t\tdefault:\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index 3e34e7a0edbf..e549dc64b996 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -288,15 +288,15 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)\n>\n>  \t\tswitch (ci->id()) {\n>  \t\tcase Brightness:\n> -\t\t\tcontrols.add(V4L2_CID_BRIGHTNESS, value.get<int>());\n> +\t\t\tcontrols.add(V4L2_CID_BRIGHTNESS, value.get<int32_t>());\n>  \t\t\tbreak;\n>\n>  \t\tcase Contrast:\n> -\t\t\tcontrols.add(V4L2_CID_CONTRAST, value.get<int>());\n> +\t\t\tcontrols.add(V4L2_CID_CONTRAST, value.get<int32_t>());\n>  \t\t\tbreak;\n>\n>  \t\tcase Saturation:\n> -\t\t\tcontrols.add(V4L2_CID_SATURATION, value.get<int>());\n> +\t\t\tcontrols.add(V4L2_CID_SATURATION, value.get<int32_t>());\n>  \t\t\tbreak;\n>\n>  \t\tdefault:\n> diff --git a/test/controls/control_info.cpp b/test/controls/control_info.cpp\n> index faefaaa444d9..2aba568a302e 100644\n> --- a/test/controls/control_info.cpp\n> +++ b/test/controls/control_info.cpp\n> @@ -26,13 +26,14 @@ protected:\n>  \t\tControlInfo info(Brightness);\n>\n>  \t\tif (info.id() != Brightness ||\n> -\t\t    info.type() != ControlTypeInteger ||\n> +\t\t    info.type() != ControlTypeInteger32 ||\n>  \t\t    info.name() != std::string(\"Brightness\")) {\n>  \t\t\tcout << \"Invalid control identification for Brightness\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n>\n> -\t\tif (info.min().get<int>() != 0 || info.max().get<int>() != 0) {\n> +\t\tif (info.min().get<int32_t>() != 0 ||\n> +\t\t    info.max().get<int32_t>() != 0) {\n>  \t\t\tcout << \"Invalid control range for Brightness\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n> @@ -44,13 +45,14 @@ protected:\n>  \t\tinfo = ControlInfo(Contrast, 10, 200);\n>\n>  \t\tif (info.id() != Contrast ||\n> -\t\t    info.type() != ControlTypeInteger ||\n> +\t\t    info.type() != ControlTypeInteger32 ||\n>  \t\t    info.name() != std::string(\"Contrast\")) {\n>  \t\t\tcout << \"Invalid control identification for Contrast\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n>\n> -\t\tif (info.min().get<int>() != 10 || info.max().get<int>() != 200) {\n> +\t\tif (info.min().get<int32_t>() != 10 ||\n> +\t\t    info.max().get<int32_t>() != 200) {\n>  \t\t\tcout << \"Invalid control range for Contrast\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n> diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp\n> index 0402e7c23dba..5215840b1c4e 100644\n> --- a/test/controls/control_list.cpp\n> +++ b/test/controls/control_list.cpp\n> @@ -96,7 +96,7 @@ protected:\n>  \t\t\treturn TestFail;\n>  \t\t}\n>\n> -\t\tif (list[Brightness].get<int>() != 255) {\n> +\t\tif (list[Brightness].get<int32_t>() != 255) {\n>  \t\t\tcout << \"Incorrest Brightness control value\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n> @@ -125,8 +125,8 @@ protected:\n>  \t\t/*\n>  \t\t * Test control value retrieval and update through ControlInfo.\n>  \t\t */\n> -\t\tif (list[brightness].get<int>() != 64 ||\n> -\t\t    list[contrast].get<int>() != 128) {\n> +\t\tif (list[brightness].get<int32_t>() != 64 ||\n> +\t\t    list[contrast].get<int32_t>() != 128) {\n>  \t\t\tcout << \"Failed to retrieve control value\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n> @@ -134,8 +134,8 @@ protected:\n>  \t\tlist[brightness] = 10;\n>  \t\tlist[contrast] = 20;\n>\n> -\t\tif (list[brightness].get<int>() != 10 ||\n> -\t\t    list[contrast].get<int>() != 20) {\n> +\t\tif (list[brightness].get<int32_t>() != 10 ||\n> +\t\t    list[contrast].get<int32_t>() != 20) {\n>  \t\t\tcout << \"Failed to update control value\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n> @@ -185,9 +185,9 @@ protected:\n>  \t\t\treturn TestFail;\n>  \t\t}\n>\n> -\t\tif (newList[Brightness].get<int>() != 10 ||\n> -\t\t    newList[Contrast].get<int>() != 20 ||\n> -\t\t    newList[Saturation].get<int>() != 255) {\n> +\t\tif (newList[Brightness].get<int32_t>() != 10 ||\n> +\t\t    newList[Contrast].get<int32_t>() != 20 ||\n> +\t\t    newList[Saturation].get<int32_t>() != 255) {\n>  \t\t\tcout << \"New list contains incorrect values\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n> diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp\n> index 397c43f799ad..a1ffa842f29e 100644\n> --- a/test/controls/control_value.cpp\n> +++ b/test/controls/control_value.cpp\n> @@ -27,7 +27,7 @@ protected:\n>  \t\t     << \" Bool: \" << boolean.toString()\n>  \t\t     << endl;\n>\n> -\t\tif (integer.get<int>() != 1234) {\n> +\t\tif (integer.get<int32_t>() != 1234) {\n>  \t\t\tcerr << \"Failed to get Integer\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n> @@ -56,8 +56,8 @@ protected:\n>  \t\t\treturn TestFail;\n>  \t\t}\n>\n> -\t\tvalue.set<int>(10);\n> -\t\tif (value.get<int>() != 10) {\n> +\t\tvalue.set<int32_t>(10);\n> +\t\tif (value.get<int32_t>() != 10) {\n>  \t\t\tcerr << \"Failed to get Integer\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\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 relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BFA6560BE8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 29 Sep 2019 11:07:07 +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 relay11.mail.gandi.net (Postfix) with ESMTPSA id 80247100009;\n\tSun, 29 Sep 2019 09:07:05 +0000 (UTC)"],"Date":"Sun, 29 Sep 2019 11:08:48 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190929090848.gzeodofx3jb7oa5f@uno.localdomain>","References":"<20190928152238.23752-1-laurent.pinchart@ideasonboard.com>\n\t<20190928152238.23752-4-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"hq36dfxraqu53yj7\"","Content-Disposition":"inline","In-Reply-To":"<20190928152238.23752-4-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 03/12] libcamera: controls: Use\n\texplicit 32-bit integer types","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 09:07:07 -0000"}},{"id":2731,"web_url":"https://patchwork.libcamera.org/comment/2731/","msgid":"<20190929113954.GC4827@pendragon.ideasonboard.com>","date":"2019-09-29T11:39:54","subject":"Re: [libcamera-devel] [PATCH 03/12] libcamera: controls: Use\n\texplicit 32-bit integer types","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 11:08:48AM +0200, Jacopo Mondi wrote:\n> On Sat, Sep 28, 2019 at 06:22:29PM +0300, Laurent Pinchart wrote:\n> > Make the control API more explicit when dealing with integer controls by\n> > specifying the size. We already do so for 64-bit integers, using int64_t\n> > and ControlTypeInteger64, do the same for 32-bit integers.\n> >\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  include/libcamera/controls.h        |  6 ++---\n> >  src/libcamera/controls.cpp          | 36 ++++++++++++++---------------\n> >  src/libcamera/pipeline/uvcvideo.cpp | 10 ++++----\n> >  src/libcamera/pipeline/vimc.cpp     |  6 ++---\n> >  test/controls/control_info.cpp      | 10 ++++----\n> >  test/controls/control_list.cpp      | 16 ++++++-------\n> >  test/controls/control_value.cpp     |  6 ++---\n> >  7 files changed, 46 insertions(+), 44 deletions(-)\n> >\n> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > index 0886370f0901..a3089064c4b5 100644\n> > --- a/include/libcamera/controls.h\n> > +++ b/include/libcamera/controls.h\n> > @@ -21,7 +21,7 @@ class Camera;\n> >  enum ControlType {\n> >  \tControlTypeNone,\n> >  \tControlTypeBool,\n> > -\tControlTypeInteger,\n> > +\tControlTypeInteger32,\n> >  \tControlTypeInteger64,\n> >  };\n> >\n> > @@ -30,7 +30,7 @@ class ControlValue\n> >  public:\n> >  \tControlValue();\n> >  \tControlValue(bool value);\n> > -\tControlValue(int value);\n> > +\tControlValue(int32_t value);\n> >  \tControlValue(int64_t value);\n> >\n> >  \tControlType type() const { return type_; };\n> > @@ -48,7 +48,7 @@ private:\n> >\n> >  \tunion {\n> >  \t\tbool bool_;\n> > -\t\tint integer_;\n> > +\t\tint32_t integer32_;\n> >  \t\tint64_t integer64_;\n> >  \t};\n> >  };\n> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > index 88aab88db327..295ccd55a622 100644\n> > --- a/src/libcamera/controls.cpp\n> > +++ b/src/libcamera/controls.cpp\n> > @@ -31,8 +31,8 @@ LOG_DEFINE_CATEGORY(Controls)\n> >   * Invalid type, for empty values\n> >   * \\var ControlTypeBool\n> >   * The control stores a boolean value\n> > - * \\var ControlTypeInteger\n> > - * The control stores an integer value\n> > + * \\var ControlTypeInteger32\n> > + * The control stores a 32-bit integer value\n> >   * \\var ControlTypeInteger64\n> >   * The control stores a 64-bit integer value\n> >   */\n> > @@ -63,8 +63,8 @@ ControlValue::ControlValue(bool value)\n> >   * \\brief Construct an integer ControlValue\n> >   * \\param[in] value Integer value to store\n> >   */\n> > -ControlValue::ControlValue(int value)\n> > -\t: type_(ControlTypeInteger), integer_(value)\n> > +ControlValue::ControlValue(int32_t value)\n> > +\t: type_(ControlTypeInteger32), integer32_(value)\n> >  {\n> >  }\n> >\n> > @@ -115,17 +115,17 @@ const bool &ControlValue::get<bool>() const\n> >  }\n> >\n> >  template<>\n> > -const int &ControlValue::get<int>() const\n> > +const int32_t &ControlValue::get<int32_t>() const\n> >  {\n> > -\tASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64);\n> > +\tASSERT(type_ == ControlTypeInteger32 || type_ == ControlTypeInteger64);\n> >\n> > -\treturn integer_;\n> > +\treturn integer32_;\n> >  }\n> >\n> >  template<>\n> >  const int64_t &ControlValue::get<int64_t>() const\n> >  {\n> > -\tASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64);\n> > +\tASSERT(type_ == ControlTypeInteger32 || type_ == ControlTypeInteger64);\n> >\n> >  \treturn integer64_;\n> >  }\n> > @@ -138,10 +138,10 @@ void ControlValue::set<bool>(const bool &value)\n> >  }\n> >\n> >  template<>\n> > -void ControlValue::set<int>(const int &value)\n> > +void ControlValue::set<int32_t>(const int32_t &value)\n> >  {\n> > -\ttype_ = ControlTypeInteger;\n> > -\tinteger_ = value;\n> > +\ttype_ = ControlTypeInteger32;\n> > +\tinteger32_ = value;\n> >  }\n> >\n> >  template<>\n> > @@ -163,8 +163,8 @@ std::string ControlValue::toString() const\n> >  \t\treturn \"<None>\";\n> >  \tcase ControlTypeBool:\n> >  \t\treturn bool_ ? \"True\" : \"False\";\n> > -\tcase ControlTypeInteger:\n> > -\t\treturn std::to_string(integer_);\n> > +\tcase ControlTypeInteger32:\n> > +\t\treturn std::to_string(integer32_);\n> >  \tcase ControlTypeInteger64:\n> >  \t\treturn std::to_string(integer64_);\n> >  \t}\n> > @@ -186,35 +186,35 @@ std::string ControlValue::toString() const\n> >\n> >  /**\n> >   * \\var Brightness\n> > - * ControlType: Integer\n> > + * ControlType: Integer32\n> >   *\n> >   * Specify a fixed brightness parameter.\n> >   */\n> >\n> >  /**\n> >   * \\var Contrast\n> > - * ControlType: Integer\n> > + * ControlType: Integer32\n> >   *\n> >   * Specify a fixed contrast parameter.\n> >   */\n> >\n> >  /**\n> >   * \\var Saturation\n> > - * ControlType: Integer\n> > + * ControlType: Integer32\n> >   *\n> >   * Specify a fixed saturation parameter.\n> >   */\n> >\n> >  /**\n> >   * \\var ManualExposure\n> > - * ControlType: Integer\n> > + * ControlType: Integer32\n> >   *\n> >   * Specify a fixed exposure time in milli-seconds\n> >   */\n> >\n> >  /**\n> >   * \\var ManualGain\n> > - * ControlType: Integer\n> > + * ControlType: Integer32\n> >   *\n> >   * Specify a fixed gain parameter\n> >   */\n> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> > index 81c548af2c64..0d56758e3e1d 100644\n> > --- a/src/libcamera/pipeline/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> > @@ -235,24 +235,24 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n> >\n> >  \t\tswitch (ci->id()) {\n> >  \t\tcase Brightness:\n> > -\t\t\tcontrols.add(V4L2_CID_BRIGHTNESS, value.get<int>());\n> > +\t\t\tcontrols.add(V4L2_CID_BRIGHTNESS, value.get<int32_t>());\n> \n> Due to C++ internal type casting rules (I presume) the previous syntax\n> value.get<int>() is still legal. I assume the template argument\n> substitution process matches this with the <int32_t> specialization.\n> Shouldn't we keep using the shorter one ?\n\nIt's still legal, and will still compile, the same way that get<long>\nwill match get<int64_t> on 64-bit platforms. However, get<long long>\nwon't, as long long and long are not the same type, even if they end up\nhaving the same size and behaving exactly the same. On 32-bit platforms,\nit's the other way around, get<long long> will match get<int64_t>, but\nget<long> won't match get<int32_t>.\n\nThe reason that pushed me to go for int32_t is to push authors to\nconsider the data type when writing code, instead of blindly relying on\n\"an integer being good enough\". Down the line I think it would be useful\nto replace V4L2_CID_* with Control instances to get the same type safety\nfor V4L2 controls that we have for libcamera controls, but that will\nrequire manually defining Control instances for all the V4L2 controls,\nwhich isn't very nice. I still need to think about it.\n\n> Anyway\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> \n> >  \t\t\tbreak;\n> >\n> >  \t\tcase Contrast:\n> > -\t\t\tcontrols.add(V4L2_CID_CONTRAST, value.get<int>());\n> > +\t\t\tcontrols.add(V4L2_CID_CONTRAST, value.get<int32_t>());\n> >  \t\t\tbreak;\n> >\n> >  \t\tcase Saturation:\n> > -\t\t\tcontrols.add(V4L2_CID_SATURATION, value.get<int>());\n> > +\t\t\tcontrols.add(V4L2_CID_SATURATION, value.get<int32_t>());\n> >  \t\t\tbreak;\n> >\n> >  \t\tcase ManualExposure:\n> >  \t\t\tcontrols.add(V4L2_CID_EXPOSURE_AUTO, 1);\n> > -\t\t\tcontrols.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get<int>());\n> > +\t\t\tcontrols.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get<int32_t>());\n> >  \t\t\tbreak;\n> >\n> >  \t\tcase ManualGain:\n> > -\t\t\tcontrols.add(V4L2_CID_GAIN, value.get<int>());\n> > +\t\t\tcontrols.add(V4L2_CID_GAIN, value.get<int32_t>());\n> >  \t\t\tbreak;\n> >\n> >  \t\tdefault:\n> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> > index 3e34e7a0edbf..e549dc64b996 100644\n> > --- a/src/libcamera/pipeline/vimc.cpp\n> > +++ b/src/libcamera/pipeline/vimc.cpp\n> > @@ -288,15 +288,15 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)\n> >\n> >  \t\tswitch (ci->id()) {\n> >  \t\tcase Brightness:\n> > -\t\t\tcontrols.add(V4L2_CID_BRIGHTNESS, value.get<int>());\n> > +\t\t\tcontrols.add(V4L2_CID_BRIGHTNESS, value.get<int32_t>());\n> >  \t\t\tbreak;\n> >\n> >  \t\tcase Contrast:\n> > -\t\t\tcontrols.add(V4L2_CID_CONTRAST, value.get<int>());\n> > +\t\t\tcontrols.add(V4L2_CID_CONTRAST, value.get<int32_t>());\n> >  \t\t\tbreak;\n> >\n> >  \t\tcase Saturation:\n> > -\t\t\tcontrols.add(V4L2_CID_SATURATION, value.get<int>());\n> > +\t\t\tcontrols.add(V4L2_CID_SATURATION, value.get<int32_t>());\n> >  \t\t\tbreak;\n> >\n> >  \t\tdefault:\n> > diff --git a/test/controls/control_info.cpp b/test/controls/control_info.cpp\n> > index faefaaa444d9..2aba568a302e 100644\n> > --- a/test/controls/control_info.cpp\n> > +++ b/test/controls/control_info.cpp\n> > @@ -26,13 +26,14 @@ protected:\n> >  \t\tControlInfo info(Brightness);\n> >\n> >  \t\tif (info.id() != Brightness ||\n> > -\t\t    info.type() != ControlTypeInteger ||\n> > +\t\t    info.type() != ControlTypeInteger32 ||\n> >  \t\t    info.name() != std::string(\"Brightness\")) {\n> >  \t\t\tcout << \"Invalid control identification for Brightness\" << endl;\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> >\n> > -\t\tif (info.min().get<int>() != 0 || info.max().get<int>() != 0) {\n> > +\t\tif (info.min().get<int32_t>() != 0 ||\n> > +\t\t    info.max().get<int32_t>() != 0) {\n> >  \t\t\tcout << \"Invalid control range for Brightness\" << endl;\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> > @@ -44,13 +45,14 @@ protected:\n> >  \t\tinfo = ControlInfo(Contrast, 10, 200);\n> >\n> >  \t\tif (info.id() != Contrast ||\n> > -\t\t    info.type() != ControlTypeInteger ||\n> > +\t\t    info.type() != ControlTypeInteger32 ||\n> >  \t\t    info.name() != std::string(\"Contrast\")) {\n> >  \t\t\tcout << \"Invalid control identification for Contrast\" << endl;\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> >\n> > -\t\tif (info.min().get<int>() != 10 || info.max().get<int>() != 200) {\n> > +\t\tif (info.min().get<int32_t>() != 10 ||\n> > +\t\t    info.max().get<int32_t>() != 200) {\n> >  \t\t\tcout << \"Invalid control range for Contrast\" << endl;\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> > diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp\n> > index 0402e7c23dba..5215840b1c4e 100644\n> > --- a/test/controls/control_list.cpp\n> > +++ b/test/controls/control_list.cpp\n> > @@ -96,7 +96,7 @@ protected:\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> >\n> > -\t\tif (list[Brightness].get<int>() != 255) {\n> > +\t\tif (list[Brightness].get<int32_t>() != 255) {\n> >  \t\t\tcout << \"Incorrest Brightness control value\" << endl;\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> > @@ -125,8 +125,8 @@ protected:\n> >  \t\t/*\n> >  \t\t * Test control value retrieval and update through ControlInfo.\n> >  \t\t */\n> > -\t\tif (list[brightness].get<int>() != 64 ||\n> > -\t\t    list[contrast].get<int>() != 128) {\n> > +\t\tif (list[brightness].get<int32_t>() != 64 ||\n> > +\t\t    list[contrast].get<int32_t>() != 128) {\n> >  \t\t\tcout << \"Failed to retrieve control value\" << endl;\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> > @@ -134,8 +134,8 @@ protected:\n> >  \t\tlist[brightness] = 10;\n> >  \t\tlist[contrast] = 20;\n> >\n> > -\t\tif (list[brightness].get<int>() != 10 ||\n> > -\t\t    list[contrast].get<int>() != 20) {\n> > +\t\tif (list[brightness].get<int32_t>() != 10 ||\n> > +\t\t    list[contrast].get<int32_t>() != 20) {\n> >  \t\t\tcout << \"Failed to update control value\" << endl;\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> > @@ -185,9 +185,9 @@ protected:\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> >\n> > -\t\tif (newList[Brightness].get<int>() != 10 ||\n> > -\t\t    newList[Contrast].get<int>() != 20 ||\n> > -\t\t    newList[Saturation].get<int>() != 255) {\n> > +\t\tif (newList[Brightness].get<int32_t>() != 10 ||\n> > +\t\t    newList[Contrast].get<int32_t>() != 20 ||\n> > +\t\t    newList[Saturation].get<int32_t>() != 255) {\n> >  \t\t\tcout << \"New list contains incorrect values\" << endl;\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> > diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp\n> > index 397c43f799ad..a1ffa842f29e 100644\n> > --- a/test/controls/control_value.cpp\n> > +++ b/test/controls/control_value.cpp\n> > @@ -27,7 +27,7 @@ protected:\n> >  \t\t     << \" Bool: \" << boolean.toString()\n> >  \t\t     << endl;\n> >\n> > -\t\tif (integer.get<int>() != 1234) {\n> > +\t\tif (integer.get<int32_t>() != 1234) {\n> >  \t\t\tcerr << \"Failed to get Integer\" << endl;\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> > @@ -56,8 +56,8 @@ protected:\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> >\n> > -\t\tvalue.set<int>(10);\n> > -\t\tif (value.get<int>() != 10) {\n> > +\t\tvalue.set<int32_t>(10);\n> > +\t\tif (value.get<int32_t>() != 10) {\n> >  \t\t\tcerr << \"Failed to get Integer\" << endl;\n> >  \t\t\treturn TestFail;\n> >  \t\t}","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 05C9861654\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 29 Sep 2019 13:40:06 +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 5A412320;\n\tSun, 29 Sep 2019 13:40:05 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1569757205;\n\tbh=Jv5zgrwe9bz0RFJ4j+cGVZY1QMFaYpptJWsmVRcfAQY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iA52d6tkfcG3LDNVUsQ14NDGJM8R2xRgxEHUBt/L9x/wAQxKgHhto+m2uySde/yYA\n\tybmudYr/5E0O6rifzTzA4mCunbzK7EOEq44aTvpe2i/tOcaSPys1ryds17iGQv0rwx\n\tj+WuRFkYD4/H5vI9s3zoHkKJe+yeutA/UUvwFrXw=","Date":"Sun, 29 Sep 2019 14:39:54 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190929113954.GC4827@pendragon.ideasonboard.com>","References":"<20190928152238.23752-1-laurent.pinchart@ideasonboard.com>\n\t<20190928152238.23752-4-laurent.pinchart@ideasonboard.com>\n\t<20190929090848.gzeodofx3jb7oa5f@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190929090848.gzeodofx3jb7oa5f@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 03/12] libcamera: controls: Use\n\texplicit 32-bit integer types","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 11:40:06 -0000"}}]