[{"id":17294,"web_url":"https://patchwork.libcamera.org/comment/17294/","msgid":"<20210526212012.4qczqjikynhmhr7v@uno.localdomain>","date":"2021-05-26T21:20:12","subject":"Re: [libcamera-devel] [PATCH v5 4/6] libcamera: CameraSensor:\n\tEnable retrieving supported test pattern modes","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hiro\n\nOn Wed, May 19, 2021 at 04:59:39PM +0900, Hirokazu Honda wrote:\n> This enables retrieving supported test pattern modes through\n> CameraSensorInfo.\n>\n> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>\n> ---\n>  include/libcamera/internal/camera_sensor.h |  7 +++++\n>  src/libcamera/camera_sensor.cpp            | 34 ++++++++++++++++++++++\n>  2 files changed, 41 insertions(+)\n>\n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index 2a5c51a1..59d1188f 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -58,6 +58,10 @@ public:\n>  \tV4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes,\n>  \t\t\t\t      const Size &size) const;\n>  \tint setFormat(V4L2SubdeviceFormat *format);\n> +\tconst std::vector<uint8_t> &testPatternModes() const\n> +\t{\n> +\t\treturn testPatternModes_;\n> +\t}\n\nI would move this just below resolution().\nIt is not related to get/setFormat()\n\n>\n>  \tconst ControlInfoMap &controls() const;\n>  \tControlList getControls(const std::vector<uint32_t> &ids);\n> @@ -81,6 +85,8 @@ private:\n>  \tvoid initVimcDefaultProperties();\n>  \tvoid initStaticProperties();\n>  \tint initProperties();\n> +\tvoid initTestPatternModes(\n> +\t\tconst std::map<uint8_t, uint8_t> &testPatternModeMap);\n>\n>  \tconst MediaEntity *entity_;\n>  \tstd::unique_ptr<V4L2Subdevice> subdev_;\n> @@ -92,6 +98,7 @@ private:\n>  \tV4L2Subdevice::Formats formats_;\n>  \tstd::vector<unsigned int> mbusCodes_;\n>  \tstd::vector<Size> sizes_;\n> +\tstd::vector<uint8_t> testPatternModes_;\n>\n>  \tSize pixelArraySize_;\n>  \tRectangle activeArea_;\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index eb84d9eb..a6aed417 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -408,6 +408,32 @@ void CameraSensor::initVimcDefaultProperties()\n>  \tactiveArea_ = Rectangle(pixelArraySize_);\n>  }\n>\n> +void CameraSensor::initTestPatternModes(\n> +\tconst std::map<uint8_t, uint8_t> &testPatternModeMap)\n> +{\n> +\tconst auto &v4l2TestPattern = controls().find(V4L2_CID_TEST_PATTERN);\n> +\tif (v4l2TestPattern == controls().end()) {\n> +\t\tLOG(CameraSensor, Debug) << \"No static test pattern map for \\'\"\n> +\t\t\t\t\t << model() << \"\\'\";\n> +\t\treturn;\n> +\t}\n> +\n> +\tfor (const ControlValue &value : v4l2TestPattern->second.values()) {\n\nWe have the index-control association in the sensor db, and the list of\nindexes in the sensor's controls.\n\nI would be fine with just using the map in the sensor db, without\ngoing through indexes to be honest.\n\n> +\t\tconst int32_t index = value.get<int32_t>();\n> +\n> +\t\tconst auto it = testPatternModeMap.find(index);\n> +\t\tif (it != testPatternModeMap.end()) {\n> +\t\t\tLOG(CameraSensor, Debug) << \"Test pattern mode (index=\"\n> +\t\t\t\t\t\t << index << \") ignored\";\n\nBut if you feel it's safer to cross-check between the two, maybe it is\nworth to report this issue ( I would s/\\(index=\\)// )\nas it might indicate the sensor driver and the sensor db diverged\n\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tLOG(CameraSensor, Debug) << \"Test pattern mode (index=\"\n> +\t\t\t\t\t << index << \") added\";\n\nBut this one has no real value at run-time\n\n> +\t\ttestPatternModes_.push_back(it->second);\n> +\t}\n> +}\n> +\n>  void CameraSensor::initStaticProperties()\n>  {\n>  \tconst CameraSensorProperties *props = CameraSensorProperties::get(model_);\n> @@ -416,6 +442,8 @@ void CameraSensor::initStaticProperties()\n>\n>  \t/* Register the properties retrieved from the sensor database. */\n>  \tproperties_.set(properties::UnitCellSize, props->unitCellSize);\n> +\n> +\tinitTestPatternModes(props->testPatternModeMap);\n>  }\n>\n>  int CameraSensor::initProperties()\n> @@ -767,6 +795,12 @@ int CameraSensor::setControls(ControlList *ctrls)\n>   * \\return The list of camera sensor properties\n>   */\n>\n> +/**\n> + * \\fn CameraSensor::testPatternModes()\n> + * \\brief Retrieve all the supported test pattern modes of the camera sensor\n> + * \\return The list of test pattern modes.\n\nNo full stop please.\n\nMostly minors\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n  j\n\n\n> + */\n> +\n>  /**\n>   * \\brief Assemble and return the camera sensor info\n>   * \\param[out] info The camera sensor info\n> --\n> 2.31.1.751.gd2f1c929bd-goog\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 645F5C3203\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 26 May 2021 21:19:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BB01268923;\n\tWed, 26 May 2021 23:19:29 +0200 (CEST)","from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3C18E602B1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 26 May 2021 23:19:28 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 52D151C0007;\n\tWed, 26 May 2021 21:19:26 +0000 (UTC)"],"Date":"Wed, 26 May 2021 23:20:12 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Hirokazu Honda <hiroh@chromium.org>","Message-ID":"<20210526212012.4qczqjikynhmhr7v@uno.localdomain>","References":"<20210519075941.1337388-1-hiroh@chromium.org>\n\t<20210519075941.1337388-4-hiroh@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210519075941.1337388-4-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [PATCH v5 4/6] libcamera: CameraSensor:\n\tEnable retrieving supported test pattern modes","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>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":17308,"web_url":"https://patchwork.libcamera.org/comment/17308/","msgid":"<CAO5uPHN0JPsPihu+X-pVN_JAargxLTtA1GKrbFPfq1t4J57g6w@mail.gmail.com>","date":"2021-05-27T06:34:58","subject":"Re: [libcamera-devel] [PATCH v5 4/6] libcamera: CameraSensor:\n\tEnable retrieving supported test pattern modes","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Jacopo, thank you for reviewing.\n\nOn Thu, May 27, 2021 at 6:19 AM Jacopo Mondi <jacopo@jmondi.org> wrote:\n\n> Hi Hiro\n>\n> On Wed, May 19, 2021 at 04:59:39PM +0900, Hirokazu Honda wrote:\n> > This enables retrieving supported test pattern modes through\n> > CameraSensorInfo.\n> >\n> > Signed-off-by: Hirokazu Honda <hiroh@chromium.org>\n> > ---\n> >  include/libcamera/internal/camera_sensor.h |  7 +++++\n> >  src/libcamera/camera_sensor.cpp            | 34 ++++++++++++++++++++++\n> >  2 files changed, 41 insertions(+)\n> >\n> > diff --git a/include/libcamera/internal/camera_sensor.h\n> b/include/libcamera/internal/camera_sensor.h\n> > index 2a5c51a1..59d1188f 100644\n> > --- a/include/libcamera/internal/camera_sensor.h\n> > +++ b/include/libcamera/internal/camera_sensor.h\n> > @@ -58,6 +58,10 @@ public:\n> >       V4L2SubdeviceFormat getFormat(const std::vector<unsigned int>\n> &mbusCodes,\n> >                                     const Size &size) const;\n> >       int setFormat(V4L2SubdeviceFormat *format);\n> > +     const std::vector<uint8_t> &testPatternModes() const\n> > +     {\n> > +             return testPatternModes_;\n> > +     }\n>\n> I would move this just below resolution().\n> It is not related to get/setFormat()\n>\n> >\n> >       const ControlInfoMap &controls() const;\n> >       ControlList getControls(const std::vector<uint32_t> &ids);\n> > @@ -81,6 +85,8 @@ private:\n> >       void initVimcDefaultProperties();\n> >       void initStaticProperties();\n> >       int initProperties();\n> > +     void initTestPatternModes(\n> > +             const std::map<uint8_t, uint8_t> &testPatternModeMap);\n> >\n> >       const MediaEntity *entity_;\n> >       std::unique_ptr<V4L2Subdevice> subdev_;\n> > @@ -92,6 +98,7 @@ private:\n> >       V4L2Subdevice::Formats formats_;\n> >       std::vector<unsigned int> mbusCodes_;\n> >       std::vector<Size> sizes_;\n> > +     std::vector<uint8_t> testPatternModes_;\n> >\n> >       Size pixelArraySize_;\n> >       Rectangle activeArea_;\n> > diff --git a/src/libcamera/camera_sensor.cpp\n> b/src/libcamera/camera_sensor.cpp\n> > index eb84d9eb..a6aed417 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -408,6 +408,32 @@ void CameraSensor::initVimcDefaultProperties()\n> >       activeArea_ = Rectangle(pixelArraySize_);\n> >  }\n> >\n> > +void CameraSensor::initTestPatternModes(\n> > +     const std::map<uint8_t, uint8_t> &testPatternModeMap)\n> > +{\n> > +     const auto &v4l2TestPattern =\n> controls().find(V4L2_CID_TEST_PATTERN);\n> > +     if (v4l2TestPattern == controls().end()) {\n> > +             LOG(CameraSensor, Debug) << \"No static test pattern map\n> for \\'\"\n> > +                                      << model() << \"\\'\";\n> > +             return;\n> > +     }\n> > +\n> > +     for (const ControlValue &value : v4l2TestPattern->second.values())\n> {\n>\n> We have the index-control association in the sensor db, and the list of\n> indexes in the sensor's controls.\n>\n> I would be fine with just using the map in the sensor db, without\n> going through indexes to be honest.\n>\n> > +             const int32_t index = value.get<int32_t>();\n> > +\n> > +             const auto it = testPatternModeMap.find(index);\n> > +             if (it != testPatternModeMap.end()) {\n> > +                     LOG(CameraSensor, Debug) << \"Test pattern mode\n> (index=\"\n> > +                                              << index << \") ignored\";\n>\n> But if you feel it's safer to cross-check between the two, maybe it is\n> worth to report this issue ( I would s/\\(index=\\)// )\n> as it might indicate the sensor driver and the sensor db diverged\n>\n>\nI see. I promote the log level to Warning.\n-Hiro\n\n\n> > +                     continue;\n> > +             }\n> > +\n> > +             LOG(CameraSensor, Debug) << \"Test pattern mode (index=\"\n> > +                                      << index << \") added\";\n>\n> But this one has no real value at run-time\n>\n> > +             testPatternModes_.push_back(it->second);\n> > +     }\n> > +}\n> > +\n> >  void CameraSensor::initStaticProperties()\n> >  {\n> >       const CameraSensorProperties *props =\n> CameraSensorProperties::get(model_);\n> > @@ -416,6 +442,8 @@ void CameraSensor::initStaticProperties()\n> >\n> >       /* Register the properties retrieved from the sensor database. */\n> >       properties_.set(properties::UnitCellSize, props->unitCellSize);\n> > +\n> > +     initTestPatternModes(props->testPatternModeMap);\n> >  }\n> >\n> >  int CameraSensor::initProperties()\n> > @@ -767,6 +795,12 @@ int CameraSensor::setControls(ControlList *ctrls)\n> >   * \\return The list of camera sensor properties\n> >   */\n> >\n> > +/**\n> > + * \\fn CameraSensor::testPatternModes()\n> > + * \\brief Retrieve all the supported test pattern modes of the camera\n> sensor\n> > + * \\return The list of test pattern modes.\n>\n> No full stop please.\n>\n> Mostly minors\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n>\n> Thanks\n>   j\n>\n>\n> > + */\n> > +\n> >  /**\n> >   * \\brief Assemble and return the camera sensor info\n> >   * \\param[out] info The camera sensor info\n> > --\n> > 2.31.1.751.gd2f1c929bd-goog\n> >\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 40099C3203\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 27 May 2021 06:35:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8418668926;\n\tThu, 27 May 2021 08:35:11 +0200 (CEST)","from mail-ej1-x62e.google.com (mail-ej1-x62e.google.com\n\t[IPv6:2a00:1450:4864:20::62e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D0CED6891F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 27 May 2021 08:35:09 +0200 (CEST)","by mail-ej1-x62e.google.com with SMTP id h20so4843337ejg.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 26 May 2021 23:35:09 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"h7pDzbU6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=70frh6zENfsJ7Y7OAqFSEindsRUzoGNiT/rYboNifrA=;\n\tb=h7pDzbU6PHE3+tLgGtLMPoOSs5+I0uQvzFEiP7+XBLKMdafCr7LifEOYTeXAOoUPP4\n\t7PABYPwXhUMXhd+9jRi3vAHq2P9l+XcVjdl2ZQZTseWPyF4O9JpgSeQg6Uh6SU+RCDaS\n\ttIBPZn/W81M3KJCY4VUOfYnlQPbPT5QYNe8P0=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=70frh6zENfsJ7Y7OAqFSEindsRUzoGNiT/rYboNifrA=;\n\tb=UmNNFTFoN5yCiDy+2bfDfZvI08LTetsm0nouJXSZvJNQwQe0l6NOxjO9Cr8cKstjq3\n\tGkR4/JBHkRd40SmNHYQJXQi4pEX2PY7uIqGJvW+cZyVSfl0lMGFs6nNGumRKLHp/W2o7\n\t00uKwbm1s7rZdYMA3BwIj+OHijCrZ9qZMdXbGtxYMIXaX4bmRXH+NPaj7mz81z5cqZqR\n\tDWGMdg+gyIzKWH3eiOuD9C1tCUpqWIWHf++Gs5YcIp5daer0gztxkMHScypQ93aOepph\n\tym4Hfe14E/IwqnHJwUXWXGR2I0fhEKbQGiqb+B02g+XGb8qcpdkfDjwsnBuSgLJxTfM+\n\tBhvg==","X-Gm-Message-State":"AOAM531XmCt0tVuF+EuCjaOyvVSfFB+p16WkgnH9B73TNgo+T4PHI2ow\n\tnWCwI6IX/gVqqHlHTLDNlevzp1BuaL1UFU0f2u9dWg==","X-Google-Smtp-Source":"ABdhPJzZmmjL5o+Se3prisaoK2VEql7FYhkSymAt/wjWYys1FZPJaZOBWpEtd84W9G8fT2XLXhOwgfgBwhmTfcE+VAU=","X-Received":"by 2002:a17:907:1b06:: with SMTP id\n\tmp6mr2220718ejc.292.1622097309467; \n\tWed, 26 May 2021 23:35:09 -0700 (PDT)","MIME-Version":"1.0","References":"<20210519075941.1337388-1-hiroh@chromium.org>\n\t<20210519075941.1337388-4-hiroh@chromium.org>\n\t<20210526212012.4qczqjikynhmhr7v@uno.localdomain>","In-Reply-To":"<20210526212012.4qczqjikynhmhr7v@uno.localdomain>","From":"Hirokazu Honda <hiroh@chromium.org>","Date":"Thu, 27 May 2021 15:34:58 +0900","Message-ID":"<CAO5uPHN0JPsPihu+X-pVN_JAargxLTtA1GKrbFPfq1t4J57g6w@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Content-Type":"multipart/alternative; boundary=\"000000000000ec1e4a05c349f56b\"","Subject":"Re: [libcamera-devel] [PATCH v5 4/6] libcamera: CameraSensor:\n\tEnable retrieving supported test pattern modes","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>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":17338,"web_url":"https://patchwork.libcamera.org/comment/17338/","msgid":"<CAO5uPHM+=nG1DidJTO3qKk6ierTvZy1RVrAUugh733U1ZNY5GA@mail.gmail.com>","date":"2021-05-28T02:45:55","subject":"Re: [libcamera-devel] [PATCH v5 4/6] libcamera: CameraSensor:\n\tEnable retrieving supported test pattern modes","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"On Thu, May 27, 2021 at 3:34 PM Hirokazu Honda <hiroh@chromium.org> wrote:\n\n> Hi Jacopo, thank you for reviewing.\n>\n> On Thu, May 27, 2021 at 6:19 AM Jacopo Mondi <jacopo@jmondi.org> wrote:\n>\n>> Hi Hiro\n>>\n>> On Wed, May 19, 2021 at 04:59:39PM +0900, Hirokazu Honda wrote:\n>> > This enables retrieving supported test pattern modes through\n>> > CameraSensorInfo.\n>> >\n>> > Signed-off-by: Hirokazu Honda <hiroh@chromium.org>\n>> > ---\n>> >  include/libcamera/internal/camera_sensor.h |  7 +++++\n>> >  src/libcamera/camera_sensor.cpp            | 34 ++++++++++++++++++++++\n>> >  2 files changed, 41 insertions(+)\n>> >\n>> > diff --git a/include/libcamera/internal/camera_sensor.h\n>> b/include/libcamera/internal/camera_sensor.h\n>> > index 2a5c51a1..59d1188f 100644\n>> > --- a/include/libcamera/internal/camera_sensor.h\n>> > +++ b/include/libcamera/internal/camera_sensor.h\n>> > @@ -58,6 +58,10 @@ public:\n>> >       V4L2SubdeviceFormat getFormat(const std::vector<unsigned int>\n>> &mbusCodes,\n>> >                                     const Size &size) const;\n>> >       int setFormat(V4L2SubdeviceFormat *format);\n>> > +     const std::vector<uint8_t> &testPatternModes() const\n>> > +     {\n>> > +             return testPatternModes_;\n>> > +     }\n>>\n>> I would move this just below resolution().\n>> It is not related to get/setFormat()\n>>\n>> >\n>> >       const ControlInfoMap &controls() const;\n>> >       ControlList getControls(const std::vector<uint32_t> &ids);\n>> > @@ -81,6 +85,8 @@ private:\n>> >       void initVimcDefaultProperties();\n>> >       void initStaticProperties();\n>> >       int initProperties();\n>> > +     void initTestPatternModes(\n>> > +             const std::map<uint8_t, uint8_t> &testPatternModeMap);\n>> >\n>> >       const MediaEntity *entity_;\n>> >       std::unique_ptr<V4L2Subdevice> subdev_;\n>> > @@ -92,6 +98,7 @@ private:\n>> >       V4L2Subdevice::Formats formats_;\n>> >       std::vector<unsigned int> mbusCodes_;\n>> >       std::vector<Size> sizes_;\n>> > +     std::vector<uint8_t> testPatternModes_;\n>> >\n>> >       Size pixelArraySize_;\n>> >       Rectangle activeArea_;\n>> > diff --git a/src/libcamera/camera_sensor.cpp\n>> b/src/libcamera/camera_sensor.cpp\n>> > index eb84d9eb..a6aed417 100644\n>> > --- a/src/libcamera/camera_sensor.cpp\n>> > +++ b/src/libcamera/camera_sensor.cpp\n>> > @@ -408,6 +408,32 @@ void CameraSensor::initVimcDefaultProperties()\n>> >       activeArea_ = Rectangle(pixelArraySize_);\n>> >  }\n>> >\n>> > +void CameraSensor::initTestPatternModes(\n>> > +     const std::map<uint8_t, uint8_t> &testPatternModeMap)\n>> > +{\n>> > +     const auto &v4l2TestPattern =\n>> controls().find(V4L2_CID_TEST_PATTERN);\n>> > +     if (v4l2TestPattern == controls().end()) {\n>> > +             LOG(CameraSensor, Debug) << \"No static test pattern map\n>> for \\'\"\n>> > +                                      << model() << \"\\'\";\n>> > +             return;\n>> > +     }\n>> > +\n>> > +     for (const ControlValue &value :\n>> v4l2TestPattern->second.values()) {\n>>\n>> We have the index-control association in the sensor db, and the list of\n>> indexes in the sensor's controls.\n>>\n>> I would be fine with just using the map in the sensor db, without\n>> going through indexes to be honest.\n>>\n>> > +             const int32_t index = value.get<int32_t>();\n>> > +\n>> > +             const auto it = testPatternModeMap.find(index);\n>> > +             if (it != testPatternModeMap.end()) {\n>> > +                     LOG(CameraSensor, Debug) << \"Test pattern mode\n>> (index=\"\n>> > +                                              << index << \") ignored\";\n>>\n>> But if you feel it's safer to cross-check between the two, maybe it is\n>> worth to report this issue ( I would s/\\(index=\\)// )\n>> as it might indicate the sensor driver and the sensor db diverged\n>>\n>>\n> I see. I promote the log level to Warning.\n>\n\nI found ov5693 has test pattern values that no test pattern mode\ncorresponds to and they will hit this Warning.\nThen logging them in warning here is misleading. So I set this log level to\nDebug again.\n\n\n> -Hiro\n>\n>\n>> > +                     continue;\n>> > +             }\n>> > +\n>> > +             LOG(CameraSensor, Debug) << \"Test pattern mode (index=\"\n>> > +                                      << index << \") added\";\n>>\n>> But this one has no real value at run-time\n>>\n>> > +             testPatternModes_.push_back(it->second);\n>> > +     }\n>> > +}\n>> > +\n>> >  void CameraSensor::initStaticProperties()\n>> >  {\n>> >       const CameraSensorProperties *props =\n>> CameraSensorProperties::get(model_);\n>> > @@ -416,6 +442,8 @@ void CameraSensor::initStaticProperties()\n>> >\n>> >       /* Register the properties retrieved from the sensor database. */\n>> >       properties_.set(properties::UnitCellSize, props->unitCellSize);\n>> > +\n>> > +     initTestPatternModes(props->testPatternModeMap);\n>> >  }\n>> >\n>> >  int CameraSensor::initProperties()\n>> > @@ -767,6 +795,12 @@ int CameraSensor::setControls(ControlList *ctrls)\n>> >   * \\return The list of camera sensor properties\n>> >   */\n>> >\n>> > +/**\n>> > + * \\fn CameraSensor::testPatternModes()\n>> > + * \\brief Retrieve all the supported test pattern modes of the camera\n>> sensor\n>> > + * \\return The list of test pattern modes.\n>>\n>> No full stop please.\n>>\n>> Mostly minors\n>> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n>>\n>> Thanks\n>>   j\n>>\n>>\n>> > + */\n>> > +\n>> >  /**\n>> >   * \\brief Assemble and return the camera sensor info\n>> >   * \\param[out] info The camera sensor info\n>> > --\n>> > 2.31.1.751.gd2f1c929bd-goog\n>> >\n>>\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B5942C3206\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 28 May 2021 02:46:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 02DCD68920;\n\tFri, 28 May 2021 04:46:07 +0200 (CEST)","from mail-ej1-x62c.google.com (mail-ej1-x62c.google.com\n\t[IPv6:2a00:1450:4864:20::62c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C0D78602A8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 28 May 2021 04:46:05 +0200 (CEST)","by mail-ej1-x62c.google.com with SMTP id gb17so3106709ejc.8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 27 May 2021 19:46:05 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"D8fSUIba\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=e64bv5BS4+QXCotw99zW2/uBswdYBYkhOyLAefKCzPs=;\n\tb=D8fSUIbausEzZWvRdnH1BSapvUd/2C/JIGVCERd4A+nMZpdz+llTXcKJEZO5BJdlBf\n\tnOvFnBR57Vli64PXfT/12X1P9Z/9kONxlv7ZFLxEjv17AfTqb8NvaQaFqhDc+Q9ocJmL\n\tfJxS+39kjsvy51ZDVsAgf5dHEm+1VEM68aB14=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=e64bv5BS4+QXCotw99zW2/uBswdYBYkhOyLAefKCzPs=;\n\tb=dlLjyfyTxSRIijWgyULBIiayYoWyuizPFbIu8W8uDZVK1qeVTePe1As2+n7aP2RMVm\n\t4SSZwwe5FLNM6JDW6Mi6WlmF34xZWm1Gk7N8gEzfLpuu6xE8IRX9VpCyhicJFbxhUEdh\n\tqwaRvjVb0aXSY2QphLVRv4akmHoRTk1Ldxdhh/SvwWhshJNVAR9SN+CiwSLDKyWaeRl8\n\tFjOvn7vJrdiOwn1viNCZFdzVfJK0eGYKxjj+bDpzbLFZ+HjlX0DFrMfIvbW+8XzfetIV\n\tZJJonO/HG4zGj/WUs9iGc4O+4+Nx2/d+FQliwOMg4LOHZaerIHnVOEYpew1OktDQgAh2\n\tN3PA==","X-Gm-Message-State":"AOAM531eYVDGZ9FpZd+J3I3veobBNjpvnIzM94jarOXrmH1Rf5gVPctm\n\t/lBfQ7rCHpIM8vk3KY1AQLoVonAha9G5E/OrmHoqGM9bssw=","X-Google-Smtp-Source":"ABdhPJzI0uPNoUDgQjbuY7Fwf944NmA9jahSADiowhBZzVq+y/QcMvaKLnslSR9WP9AhSnHJidO41OtVX1lDljyUPtw=","X-Received":"by 2002:a17:906:8249:: with SMTP id\n\tf9mr6134155ejx.243.1622169965343; \n\tThu, 27 May 2021 19:46:05 -0700 (PDT)","MIME-Version":"1.0","References":"<20210519075941.1337388-1-hiroh@chromium.org>\n\t<20210519075941.1337388-4-hiroh@chromium.org>\n\t<20210526212012.4qczqjikynhmhr7v@uno.localdomain>\n\t<CAO5uPHN0JPsPihu+X-pVN_JAargxLTtA1GKrbFPfq1t4J57g6w@mail.gmail.com>","In-Reply-To":"<CAO5uPHN0JPsPihu+X-pVN_JAargxLTtA1GKrbFPfq1t4J57g6w@mail.gmail.com>","From":"Hirokazu Honda <hiroh@chromium.org>","Date":"Fri, 28 May 2021 11:45:55 +0900","Message-ID":"<CAO5uPHM+=nG1DidJTO3qKk6ierTvZy1RVrAUugh733U1ZNY5GA@mail.gmail.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Content-Type":"multipart/alternative; boundary=\"0000000000008ccddd05c35ae0fa\"","Subject":"Re: [libcamera-devel] [PATCH v5 4/6] libcamera: CameraSensor:\n\tEnable retrieving supported test pattern modes","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>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]