[{"id":4526,"web_url":"https://patchwork.libcamera.org/comment/4526/","msgid":"<20200426130217.3z7qev2ajxrmmcza@uno.localdomain>","date":"2020-04-26T13:02:17","subject":"Re: [libcamera-devel] [PATCH v5 2/7] libcamera: pipeline: uvcvideo:\n\tAdd support for AeEnable","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Sat, Apr 25, 2020 at 03:45:28AM +0300, Laurent Pinchart wrote:\n> UVC devices support auto-exposure, expose the feature through the\n> AeEnable control.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 42 +++++++++++++++-----\n>  1 file changed, 33 insertions(+), 9 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 92777b9f5fe4..b040f2460b1c 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -244,9 +244,6 @@ void PipelineHandlerUVC::stop(Camera *camera)\n>  int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,\n>  \t\t\t\t       const ControlValue &value)\n>  {\n> -\tif (value.type() != ControlTypeInteger32)\n> -\t\treturn -EINVAL;\n> -\n>  \tuint32_t cid;\n>\n>  \tif (id == controls::Brightness)\n> @@ -255,6 +252,8 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,\n>  \t\tcid = V4L2_CID_CONTRAST;\n>  \telse if (id == controls::Saturation)\n>  \t\tcid = V4L2_CID_SATURATION;\n> +\telse if (id == controls::AeEnable)\n> +\t\tcid = V4L2_CID_EXPOSURE_AUTO;\n>  \telse if (id == controls::ManualExposure)\n>  \t\tcid = V4L2_CID_EXPOSURE_ABSOLUTE;\n>  \telse if (id == controls::ManualGain)\n> @@ -262,12 +261,21 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,\n>  \telse\n>  \t\treturn -EINVAL;\n>\n> -\tint32_t ivalue = value.get<int32_t>();\n> +\tswitch (cid) {\n> +\tcase V4L2_CID_EXPOSURE_AUTO: {\n> +\t\tint32_t ivalue = value.get<bool>()\n> +\t\t\t       ? V4L2_EXPOSURE_APERTURE_PRIORITY\n> +\t\t\t       : V4L2_EXPOSURE_MANUAL;\n> +\t\tcontrols->set(V4L2_CID_EXPOSURE_AUTO, ivalue);\n> +\t\tbreak;\n> +\t}\n>\n> -\tif (cid == V4L2_CID_EXPOSURE_ABSOLUTE)\n> -\t\tcontrols->set(V4L2_CID_EXPOSURE_AUTO, static_cast<int32_t>(1));\n\nWhich one between AeEnable and ManualExposure has priority ? Here it\nseems to me the last processed one overwrites the other...\n\nAnyway, am I wrong or if you receive ManualExposure, AeEnable is not\ndisabled anymore?  And, this comment applies to patch #1 but I'm just\nnoticing it now, if cid == V4L2_CID_EXPOSURE_ABSOLUTE, it means we have\nreceived ManualExposure, shouldn't then EXPOSURE_AUTO be set to 0 ?\n\n> -\n> -\tcontrols->set(cid, ivalue);\n> +\tdefault: {\n> +\t\tint32_t ivalue = value.get<int32_t>();\n> +\t\tcontrols->set(cid, ivalue);\n> +\t\tbreak;\n> +\t}\n> +\t}\n>\n>  \treturn 0;\n>  }\n> @@ -388,7 +396,9 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n>  \t\t\t       ControlInfoMap::Map *ctrls)\n>  {\n>  \tconst ControlId *id;\n> +\tControlInfo info;\n>\n> +\t/* Map the control ID. */\n\nit's fine here, but could have gone in #1. Nitpicking...\n\n>  \tswitch (cid) {\n>  \tcase V4L2_CID_BRIGHTNESS:\n>  \t\tid = &controls::Brightness;\n> @@ -399,6 +409,9 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n>  \tcase V4L2_CID_SATURATION:\n>  \t\tid = &controls::Saturation;\n>  \t\tbreak;\n> +\tcase V4L2_CID_EXPOSURE_AUTO:\n> +\t\tid = &controls::AeEnable;\n> +\t\tbreak;\n>  \tcase V4L2_CID_EXPOSURE_ABSOLUTE:\n>  \t\tid = &controls::ManualExposure;\n>  \t\tbreak;\n> @@ -409,7 +422,18 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n>  \t\treturn;\n>  \t}\n>\n> -\tctrls->emplace(id, v4l2Info);\n> +\t/* Map the control info. */\n> +\tswitch (cid) {\n> +\tcase V4L2_CID_EXPOSURE_AUTO:\n> +\t\tinfo = ControlInfo{ false, true, true };\n\nWhy can't we use the v4l2Info here ? what will it report that needs to\nbe manually contructed ?\n\n> +\t\tbreak;\n> +\n> +\tdefault:\n> +\t\tinfo = v4l2Info;\n> +\t\tbreak;\n> +\t}\n> +\n> +\tctrls->emplace(id, info);\n>  }\n>\n>  void UVCCameraData::bufferReady(FrameBuffer *buffer)\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 relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1E792603FA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 26 Apr 2020 14:59:14 +0200 (CEST)","from uno.localdomain\n\t(host170-48-dynamic.3-87-r.retail.telecomitalia.it [87.3.48.170])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 320241BF20A;\n\tSun, 26 Apr 2020 12:59:11 +0000 (UTC)"],"X-Originating-IP":"87.3.48.170","Date":"Sun, 26 Apr 2020 15:02:17 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200426130217.3z7qev2ajxrmmcza@uno.localdomain>","References":"<20200425004533.26907-1-laurent.pinchart@ideasonboard.com>\n\t<20200425004533.26907-3-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200425004533.26907-3-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 2/7] libcamera: pipeline: uvcvideo:\n\tAdd support for AeEnable","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, 26 Apr 2020 12:59:14 -0000"}},{"id":4536,"web_url":"https://patchwork.libcamera.org/comment/4536/","msgid":"<20200426170427.GD5950@pendragon.ideasonboard.com>","date":"2020-04-26T17:04:27","subject":"Re: [libcamera-devel] [PATCH v5 2/7] libcamera: pipeline: uvcvideo:\n\tAdd support for AeEnable","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, Apr 26, 2020 at 03:02:17PM +0200, Jacopo Mondi wrote:\n> On Sat, Apr 25, 2020 at 03:45:28AM +0300, Laurent Pinchart wrote:\n> > UVC devices support auto-exposure, expose the feature through the\n> > AeEnable control.\n> >\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 42 +++++++++++++++-----\n> >  1 file changed, 33 insertions(+), 9 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > index 92777b9f5fe4..b040f2460b1c 100644\n> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > @@ -244,9 +244,6 @@ void PipelineHandlerUVC::stop(Camera *camera)\n> >  int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,\n> >  \t\t\t\t       const ControlValue &value)\n> >  {\n> > -\tif (value.type() != ControlTypeInteger32)\n> > -\t\treturn -EINVAL;\n> > -\n> >  \tuint32_t cid;\n> >\n> >  \tif (id == controls::Brightness)\n> > @@ -255,6 +252,8 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,\n> >  \t\tcid = V4L2_CID_CONTRAST;\n> >  \telse if (id == controls::Saturation)\n> >  \t\tcid = V4L2_CID_SATURATION;\n> > +\telse if (id == controls::AeEnable)\n> > +\t\tcid = V4L2_CID_EXPOSURE_AUTO;\n> >  \telse if (id == controls::ManualExposure)\n> >  \t\tcid = V4L2_CID_EXPOSURE_ABSOLUTE;\n> >  \telse if (id == controls::ManualGain)\n> > @@ -262,12 +261,21 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,\n> >  \telse\n> >  \t\treturn -EINVAL;\n> >\n> > -\tint32_t ivalue = value.get<int32_t>();\n> > +\tswitch (cid) {\n> > +\tcase V4L2_CID_EXPOSURE_AUTO: {\n> > +\t\tint32_t ivalue = value.get<bool>()\n> > +\t\t\t       ? V4L2_EXPOSURE_APERTURE_PRIORITY\n> > +\t\t\t       : V4L2_EXPOSURE_MANUAL;\n> > +\t\tcontrols->set(V4L2_CID_EXPOSURE_AUTO, ivalue);\n> > +\t\tbreak;\n> > +\t}\n> >\n> > -\tif (cid == V4L2_CID_EXPOSURE_ABSOLUTE)\n> > -\t\tcontrols->set(V4L2_CID_EXPOSURE_AUTO, static_cast<int32_t>(1));\n> \n> Which one between AeEnable and ManualExposure has priority ? Here it\n> seems to me the last processed one overwrites the other...\n\nThe V4L2_CID_EXPOSURE_ABSOLUTE control is ignored by the device when\nV4L2_CID_EXPOSURE_AUTO is set to automatic, which is what we want. We\ncould make it explicit in the code here by skipping the\nV4L2_CID_EXPOSURE_ABSOLUTE control when auto-exposure is enabled, but I\nthink that's a bit overkill as the hardware ignores it anyway.\n\n> Anyway, am I wrong or if you receive ManualExposure, AeEnable is not\n> disabled anymore?  And, this comment applies to patch #1 but I'm just\n> noticing it now, if cid == V4L2_CID_EXPOSURE_ABSOLUTE, it means we have\n> received ManualExposure, shouldn't then EXPOSURE_AUTO be set to 0 ?\n\nAeEnable controls whether the auto-exposure algorithm is enabled or not.\nWhen enabled, manual changes to the exposure time shall be ignored by\nthe pipeline handler. In this specific case the pipeline handler\nimplementation relies on the device ignoring the manual exposure time.\n\n> > -\n> > -\tcontrols->set(cid, ivalue);\n> > +\tdefault: {\n> > +\t\tint32_t ivalue = value.get<int32_t>();\n> > +\t\tcontrols->set(cid, ivalue);\n> > +\t\tbreak;\n> > +\t}\n> > +\t}\n> >\n> >  \treturn 0;\n> >  }\n> > @@ -388,7 +396,9 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n> >  \t\t\t       ControlInfoMap::Map *ctrls)\n> >  {\n> >  \tconst ControlId *id;\n> > +\tControlInfo info;\n> >\n> > +\t/* Map the control ID. */\n> \n> it's fine here, but could have gone in #1. Nitpicking...\n> \n> >  \tswitch (cid) {\n> >  \tcase V4L2_CID_BRIGHTNESS:\n> >  \t\tid = &controls::Brightness;\n> > @@ -399,6 +409,9 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n> >  \tcase V4L2_CID_SATURATION:\n> >  \t\tid = &controls::Saturation;\n> >  \t\tbreak;\n> > +\tcase V4L2_CID_EXPOSURE_AUTO:\n> > +\t\tid = &controls::AeEnable;\n> > +\t\tbreak;\n> >  \tcase V4L2_CID_EXPOSURE_ABSOLUTE:\n> >  \t\tid = &controls::ManualExposure;\n> >  \t\tbreak;\n> > @@ -409,7 +422,18 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n> >  \t\treturn;\n> >  \t}\n> >\n> > -\tctrls->emplace(id, v4l2Info);\n> > +\t/* Map the control info. */\n> > +\tswitch (cid) {\n> > +\tcase V4L2_CID_EXPOSURE_AUTO:\n> > +\t\tinfo = ControlInfo{ false, true, true };\n> \n> Why can't we use the v4l2Info here ? what will it report that needs to\n> be manually contructed ?\n\nV4L2_CID_EXPOSURE_AUTO is a menu control, so v4l2Info contains int32_t\nvalues, while controls::AeEnable is a boolean control.\n\n> > +\t\tbreak;\n> > +\n> > +\tdefault:\n> > +\t\tinfo = v4l2Info;\n> > +\t\tbreak;\n> > +\t}\n> > +\n> > +\tctrls->emplace(id, info);\n> >  }\n> >\n> >  void UVCCameraData::bufferReady(FrameBuffer *buffer)","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 CC77162E55\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 26 Apr 2020 19:04:42 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2E61C4F7;\n\tSun, 26 Apr 2020 19:04:42 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ReL1M/h5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1587920682;\n\tbh=6mfw4iY08VPDqjucz82noa9VxGJGCh9mnW2l6da/sHc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ReL1M/h592DpnfdziM/1+ASpshW50f+6fDRWCP5zk6UZ9uM95rU7CeeyPA4iux6nO\n\tMEaEtflpGK4+D57l27dasj834Pbmxm55PqiTbh2XgQs9e5ZyEuK0qBYk85nJ9OA3oc\n\t6AMG68fKFPFyEzzMz8y7w8OTcYw5vvH/a+DyEf5E=","Date":"Sun, 26 Apr 2020 20:04:27 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200426170427.GD5950@pendragon.ideasonboard.com>","References":"<20200425004533.26907-1-laurent.pinchart@ideasonboard.com>\n\t<20200425004533.26907-3-laurent.pinchart@ideasonboard.com>\n\t<20200426130217.3z7qev2ajxrmmcza@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200426130217.3z7qev2ajxrmmcza@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v5 2/7] libcamera: pipeline: uvcvideo:\n\tAdd support for AeEnable","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, 26 Apr 2020 17:04:43 -0000"}},{"id":4540,"web_url":"https://patchwork.libcamera.org/comment/4540/","msgid":"<20200426193622.le2yhum7d3q5wva2@uno.localdomain>","date":"2020-04-26T19:36:22","subject":"Re: [libcamera-devel] [PATCH v5 2/7] libcamera: pipeline: uvcvideo:\n\tAdd support for AeEnable","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Sun, Apr 26, 2020 at 08:04:27PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Sun, Apr 26, 2020 at 03:02:17PM +0200, Jacopo Mondi wrote:\n> > On Sat, Apr 25, 2020 at 03:45:28AM +0300, Laurent Pinchart wrote:\n> > > UVC devices support auto-exposure, expose the feature through the\n> > > AeEnable control.\n> > >\n> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > > ---\n> > >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 42 +++++++++++++++-----\n> > >  1 file changed, 33 insertions(+), 9 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > index 92777b9f5fe4..b040f2460b1c 100644\n> > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> > > @@ -244,9 +244,6 @@ void PipelineHandlerUVC::stop(Camera *camera)\n> > >  int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,\n> > >  \t\t\t\t       const ControlValue &value)\n> > >  {\n> > > -\tif (value.type() != ControlTypeInteger32)\n> > > -\t\treturn -EINVAL;\n> > > -\n> > >  \tuint32_t cid;\n> > >\n> > >  \tif (id == controls::Brightness)\n> > > @@ -255,6 +252,8 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,\n> > >  \t\tcid = V4L2_CID_CONTRAST;\n> > >  \telse if (id == controls::Saturation)\n> > >  \t\tcid = V4L2_CID_SATURATION;\n> > > +\telse if (id == controls::AeEnable)\n> > > +\t\tcid = V4L2_CID_EXPOSURE_AUTO;\n> > >  \telse if (id == controls::ManualExposure)\n> > >  \t\tcid = V4L2_CID_EXPOSURE_ABSOLUTE;\n> > >  \telse if (id == controls::ManualGain)\n> > > @@ -262,12 +261,21 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,\n> > >  \telse\n> > >  \t\treturn -EINVAL;\n> > >\n> > > -\tint32_t ivalue = value.get<int32_t>();\n> > > +\tswitch (cid) {\n> > > +\tcase V4L2_CID_EXPOSURE_AUTO: {\n> > > +\t\tint32_t ivalue = value.get<bool>()\n> > > +\t\t\t       ? V4L2_EXPOSURE_APERTURE_PRIORITY\n> > > +\t\t\t       : V4L2_EXPOSURE_MANUAL;\n> > > +\t\tcontrols->set(V4L2_CID_EXPOSURE_AUTO, ivalue);\n> > > +\t\tbreak;\n> > > +\t}\n> > >\n> > > -\tif (cid == V4L2_CID_EXPOSURE_ABSOLUTE)\n> > > -\t\tcontrols->set(V4L2_CID_EXPOSURE_AUTO, static_cast<int32_t>(1));\n> >\n> > Which one between AeEnable and ManualExposure has priority ? Here it\n> > seems to me the last processed one overwrites the other...\n>\n> The V4L2_CID_EXPOSURE_ABSOLUTE control is ignored by the device when\n> V4L2_CID_EXPOSURE_AUTO is set to automatic, which is what we want. We\n> could make it explicit in the code here by skipping the\n> V4L2_CID_EXPOSURE_ABSOLUTE control when auto-exposure is enabled, but I\n> think that's a bit overkill as the hardware ignores it anyway.\n>\n> > Anyway, am I wrong or if you receive ManualExposure, AeEnable is not\n> > disabled anymore?  And, this comment applies to patch #1 but I'm just\n> > noticing it now, if cid == V4L2_CID_EXPOSURE_ABSOLUTE, it means we have\n> > received ManualExposure, shouldn't then EXPOSURE_AUTO be set to 0 ?\n>\n> AeEnable controls whether the auto-exposure algorithm is enabled or not.\n> When enabled, manual changes to the exposure time shall be ignored by\n> the pipeline handler. In this specific case the pipeline handler\n> implementation relies on the device ignoring the manual exposure time.\n>\n\nOh, I had no idea, this is fine then.\n\n> > > -\n> > > -\tcontrols->set(cid, ivalue);\n> > > +\tdefault: {\n> > > +\t\tint32_t ivalue = value.get<int32_t>();\n> > > +\t\tcontrols->set(cid, ivalue);\n> > > +\t\tbreak;\n> > > +\t}\n> > > +\t}\n> > >\n> > >  \treturn 0;\n> > >  }\n> > > @@ -388,7 +396,9 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n> > >  \t\t\t       ControlInfoMap::Map *ctrls)\n> > >  {\n> > >  \tconst ControlId *id;\n> > > +\tControlInfo info;\n> > >\n> > > +\t/* Map the control ID. */\n> >\n> > it's fine here, but could have gone in #1. Nitpicking...\n> >\n> > >  \tswitch (cid) {\n> > >  \tcase V4L2_CID_BRIGHTNESS:\n> > >  \t\tid = &controls::Brightness;\n> > > @@ -399,6 +409,9 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n> > >  \tcase V4L2_CID_SATURATION:\n> > >  \t\tid = &controls::Saturation;\n> > >  \t\tbreak;\n> > > +\tcase V4L2_CID_EXPOSURE_AUTO:\n> > > +\t\tid = &controls::AeEnable;\n> > > +\t\tbreak;\n> > >  \tcase V4L2_CID_EXPOSURE_ABSOLUTE:\n> > >  \t\tid = &controls::ManualExposure;\n> > >  \t\tbreak;\n> > > @@ -409,7 +422,18 @@ void UVCCameraData::addControl(uint32_t cid, const ControlInfo &v4l2Info,\n> > >  \t\treturn;\n> > >  \t}\n> > >\n> > > -\tctrls->emplace(id, v4l2Info);\n> > > +\t/* Map the control info. */\n> > > +\tswitch (cid) {\n> > > +\tcase V4L2_CID_EXPOSURE_AUTO:\n> > > +\t\tinfo = ControlInfo{ false, true, true };\n> >\n> > Why can't we use the v4l2Info here ? what will it report that needs to\n> > be manually contructed ?\n>\n> V4L2_CID_EXPOSURE_AUTO is a menu control, so v4l2Info contains int32_t\n> values, while controls::AeEnable is a boolean control.\n>\n\nAck.\n\nWith these questions clarified\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n  j\n\n> > > +\t\tbreak;\n> > > +\n> > > +\tdefault:\n> > > +\t\tinfo = v4l2Info;\n> > > +\t\tbreak;\n> > > +\t}\n> > > +\n> > > +\tctrls->emplace(id, info);\n> > >  }\n> > >\n> > >  void UVCCameraData::bufferReady(FrameBuffer *buffer)\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["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 0987862E55\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 26 Apr 2020 21:33:14 +0200 (CEST)","from uno.localdomain (a-ur1-85.tin.it [212.216.150.148])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 1B6A91C000A;\n\tSun, 26 Apr 2020 19:33:12 +0000 (UTC)"],"X-Originating-IP":"212.216.150.148","Date":"Sun, 26 Apr 2020 21:36:22 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200426193622.le2yhum7d3q5wva2@uno.localdomain>","References":"<20200425004533.26907-1-laurent.pinchart@ideasonboard.com>\n\t<20200425004533.26907-3-laurent.pinchart@ideasonboard.com>\n\t<20200426130217.3z7qev2ajxrmmcza@uno.localdomain>\n\t<20200426170427.GD5950@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200426170427.GD5950@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 2/7] libcamera: pipeline: uvcvideo:\n\tAdd support for AeEnable","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, 26 Apr 2020 19:33:14 -0000"}}]