[{"id":1854,"web_url":"https://patchwork.libcamera.org/comment/1854/","msgid":"<87e4cac9-f409-b51a-3240-ee2014826d2b@ideasonboard.com>","date":"2019-06-11T14:09:51","subject":"Re: [libcamera-devel] [PATCH v2 6/6] [HACK] ipu3: Set and get a few\n\tsensor controls","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 10/06/2019 17:40, Jacopo Mondi wrote:\n> Not to merge patch to demonstrate how to set controls on the image\n> sensor device.\n> \n> Not-Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 80 ++++++++++++++++++++++++++++\n> 1 file changed, 80 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index b3e7fb0e9c64..59c1fe3c56fd 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -688,6 +688,86 @@ int PipelineHandlerIPU3::start(Camera *camera)\n> \tImgUDevice *imgu = data->imgu_;\n> \tint ret;\n> \n> +\t/* --- Get control values --- */\n> +\tstd::vector controlIds = {\n> +\t\tV4L2_CID_EXPOSURE, V4L2_CID_ANALOGUE_GAIN,\n> +\t};\n> +\tV4L2Controls controls;\n> +\tret = cio2->sensor_->dev()->getControls(controlIds, &controls);\n> +\tif (ret) {\n> +\t\tLOG(Error) << \"Failed to get control values\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n\nMoving some logic for printing the values (or working out how to use\nControlValue class in a V4L2Control) would simplify any debug prints\ngreatly:\n\nfor (V4L2Control *ctrl : controls) {\n\tLOG(Error) << \"Control : \" << id\n\t\t << \" - value: \" << ctrl->value();\n}\n\n\n> +\tfor (V4L2Control *ctrl : controls) {\n> +\t\tunsigned int id = ctrl->id();\n> +\n> +\t\tswitch(ctrl->type()) {\n> +\t\tcase V4L2_CTRL_TYPE_INTEGER:\n> +\t\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> +\t\t{\n> +\t\t\tuint32_t val = controls.getInt(id);\n> +\t\t\tLOG(Error) << \"Control : \" << id\n> +\t\t\t\t << \" - value: \" << val;\n> +\t\t}\n> +\t\t\tbreak;\n> +\t\tcase V4L2_CTRL_TYPE_INTEGER64:\n> +\t\t{\n> +\t\t\tuint64_t val = controls.getInt64(id);\n> +\t\t\tLOG(Error) << \"Control : \" << id\n> +\t\t\t\t << \" - value: \" << val;\n> +\t\t}\n> +\t\t\tbreak;\n> +\t\tdefault:\n> +\t\t\tLOG(Error) << \"Unsupported type: \" << ctrl->type();\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\t}\n> +\n\n\n\n> +\t/* --- Set control values --- */\n> +\tV4L2Controls setControls;\n> +\tsetControls.set(V4L2_CID_EXPOSURE, 2046);\n> +\tsetControls.set(V4L2_CID_ANALOGUE_GAIN, 1024);\n> +\n> +\tret = cio2->sensor_->dev()->setControls(setControls);\n> +\tif (ret) {\n> +\t\tLOG(IPU3, Error) << \"Failed to set controls\";\n> +\t\treturn ret;\n> +\t}\n> +\n> +\t/* --- Get control values back again and verify they have changed --- */\n> +\tV4L2Controls newcontrols;\n> +\tret = cio2->sensor_->dev()->getControls(controlIds, &newcontrols);\n> +\tif (ret) {\n> +\t\tLOG(Error) << \"Failed to get control values\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tfor (V4L2Control *ctrl : newcontrols) {\n> +\t\tunsigned int id = ctrl->id();\n> +\n> +\t\tswitch(ctrl->type()) {\n> +\t\tcase V4L2_CTRL_TYPE_INTEGER:\n> +\t\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> +\t\t{\n> +\t\t\tuint32_t val = newcontrols.getInt(id);\n\nEeep, Here we are iterating new controls to print them (I get that this\nis just a print) - but we are then 'getting' the int from the list\ncontainer instead of the V4L2Control, which is involving 'finding' the\ncontrol again, and iterating the 'newcontrols' container for each print?\n\nIterating the list twice seems painful to me... but perhaps that's just\nbecause of this particular demo use case, and it might not be such an\nissue in real usage.\n\n\n> +\t\t\tLOG(Error) << \"Control : \" << id\n> +\t\t\t\t << \" - value: \" << val;\n> +\t\t}\n> +\t\t\tbreak;\n> +\t\tcase V4L2_CTRL_TYPE_INTEGER64:\n> +\t\t{\n> +\t\t\tuint64_t val = newcontrols.getInt64(id);\n> +\t\t\tLOG(Error) << \"Control : \" << id\n> +\t\t\t\t << \" - value: \" << val;\n> +\t\t}\n> +\t\t\tbreak;\n> +\t\tdefault:\n> +\t\t\tLOG(Error) << \"Unsupported type: \" << ctrl->type();\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\t}\n> +\n> \t/*\n> \t * Start the ImgU video devices, buffers will be queued to the\n> \t * ImgU output and viewfinder when requests will be queued.\n>","headers":{"Return-Path":"","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 2681162F75\n\tfor ;\n\tTue, 11 Jun 2019 16:09:56 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A14AAFA0;\n\tTue, 11 Jun 2019 16:09:54 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1560262194;\n\tbh=jamBB7XgR4KNZxTj2Y5+kvnaMqIMqLHOp2khuKfTJws=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=JlwkUkb6H2k53ZcEd+PmErCNSzF6DwOa+G6T8LjiRTugc9/63EqoyDAvqwo7Udv9r\n\tgctD2iCu6+O6NNxEREW7olw3ut9IplUhzm/BXmn/JZtFSFvCggkaAGT4lOMBte+Yz6\n\tyM9zc1EnwOtAL0ZklXe2YC2iK7ZaWquGCtBxXBsI=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","References":"<20190610164052.30741-1-jacopo@jmondi.org>\n\t<20190610164052.30741-7-jacopo@jmondi.org>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<87e4cac9-f409-b51a-3240-ee2014826d2b@ideasonboard.com>","Date":"Tue, 11 Jun 2019 15:09:51 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.0","MIME-Version":"1.0","In-Reply-To":"<20190610164052.30741-7-jacopo@jmondi.org>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 6/6] [HACK] ipu3: Set and get a few\n\tsensor controls","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 11 Jun 2019 14:09:56 -0000"}},{"id":1859,"web_url":"https://patchwork.libcamera.org/comment/1859/","msgid":"<20190611150541.2kjeagupko7kijcj@uno.localdomain>","date":"2019-06-11T15:05:41","subject":"Re: [libcamera-devel] [PATCH v2 6/6] [HACK] ipu3: Set and get a few\n\tsensor controls","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kieran,\n\nOn Tue, Jun 11, 2019 at 03:09:51PM +0100, Kieran Bingham wrote:\n> Hi Jacopo,\n>\n> On 10/06/2019 17:40, Jacopo Mondi wrote:\n> > Not to merge patch to demonstrate how to set controls on the image\n> > sensor device.\n> >\n> > Not-Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 80 ++++++++++++++++++++++++++++\n> > 1 file changed, 80 insertions(+)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index b3e7fb0e9c64..59c1fe3c56fd 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -688,6 +688,86 @@ int PipelineHandlerIPU3::start(Camera *camera)\n> > \tImgUDevice *imgu = data->imgu_;\n> > \tint ret;\n> >\n> > +\t/* --- Get control values --- */\n> > +\tstd::vector controlIds = {\n> > +\t\tV4L2_CID_EXPOSURE, V4L2_CID_ANALOGUE_GAIN,\n> > +\t};\n> > +\tV4L2Controls controls;\n> > +\tret = cio2->sensor_->dev()->getControls(controlIds, &controls);\n> > +\tif (ret) {\n> > +\t\tLOG(Error) << \"Failed to get control values\";\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n>\n> Moving some logic for printing the values (or working out how to use\n> ControlValue class in a V4L2Control) would simplify any debug prints\n> greatly:\n>\n> for (V4L2Control *ctrl : controls) {\n> \tLOG(Error) << \"Control : \" << id\n> \t\t << \" - value: \" << ctrl->value();\n> }\n>\n\nSorry, I might have missed how that would change using ControlValue.\nThat class has getInt, getBool etc, right?\n\n>\n> > +\tfor (V4L2Control *ctrl : controls) {\n> > +\t\tunsigned int id = ctrl->id();\n> > +\n> > +\t\tswitch(ctrl->type()) {\n> > +\t\tcase V4L2_CTRL_TYPE_INTEGER:\n> > +\t\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> > +\t\t{\n> > +\t\t\tuint32_t val = controls.getInt(id);\n> > +\t\t\tLOG(Error) << \"Control : \" << id\n> > +\t\t\t\t << \" - value: \" << val;\n> > +\t\t}\n> > +\t\t\tbreak;\n> > +\t\tcase V4L2_CTRL_TYPE_INTEGER64:\n> > +\t\t{\n> > +\t\t\tuint64_t val = controls.getInt64(id);\n> > +\t\t\tLOG(Error) << \"Control : \" << id\n> > +\t\t\t\t << \" - value: \" << val;\n> > +\t\t}\n> > +\t\t\tbreak;\n> > +\t\tdefault:\n> > +\t\t\tLOG(Error) << \"Unsupported type: \" << ctrl->type();\n> > +\t\t\treturn -EINVAL;\n> > +\t\t}\n> > +\t}\n> > +\n>\n>\n>\n> > +\t/* --- Set control values --- */\n> > +\tV4L2Controls setControls;\n> > +\tsetControls.set(V4L2_CID_EXPOSURE, 2046);\n> > +\tsetControls.set(V4L2_CID_ANALOGUE_GAIN, 1024);\n> > +\n> > +\tret = cio2->sensor_->dev()->setControls(setControls);\n> > +\tif (ret) {\n> > +\t\tLOG(IPU3, Error) << \"Failed to set controls\";\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > +\t/* --- Get control values back again and verify they have changed --- */\n> > +\tV4L2Controls newcontrols;\n> > +\tret = cio2->sensor_->dev()->getControls(controlIds, &newcontrols);\n> > +\tif (ret) {\n> > +\t\tLOG(Error) << \"Failed to get control values\";\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > +\tfor (V4L2Control *ctrl : newcontrols) {\n> > +\t\tunsigned int id = ctrl->id();\n> > +\n> > +\t\tswitch(ctrl->type()) {\n> > +\t\tcase V4L2_CTRL_TYPE_INTEGER:\n> > +\t\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> > +\t\t{\n> > +\t\t\tuint32_t val = newcontrols.getInt(id);\n>\n> Eeep, Here we are iterating new controls to print them (I get that this\n> is just a print) - but we are then 'getting' the int from the list\n> container instead of the V4L2Control, which is involving 'finding' the\n> control again, and iterating the 'newcontrols' container for each print?\n>\n> Iterating the list twice seems painful to me... but perhaps that's just\n> because of this particular demo use case, and it might not be such an\n> issue in real usage.\n\nAccessing the V4L2Control value would require the user to cast the\nV4L2Control * to the appropriate specialized derived class, something\nwe don't want.\n\nIn this case yes, using a ControlValue object which has a field for\nall possible value types and provides a getInt() getBool() etc would\nremove the casting requirement and would allow to access a control\nvalue with a single call (provided the user knows the control value\ntype, but we have the same issue here, where user should call the\nopportune get$TYPE() anyhow.\n\n\n>\n>\n> > +\t\t\tLOG(Error) << \"Control : \" << id\n> > +\t\t\t\t << \" - value: \" << val;\n> > +\t\t}\n> > +\t\t\tbreak;\n> > +\t\tcase V4L2_CTRL_TYPE_INTEGER64:\n> > +\t\t{\n> > +\t\t\tuint64_t val = newcontrols.getInt64(id);\n> > +\t\t\tLOG(Error) << \"Control : \" << id\n> > +\t\t\t\t << \" - value: \" << val;\n> > +\t\t}\n> > +\t\t\tbreak;\n> > +\t\tdefault:\n> > +\t\t\tLOG(Error) << \"Unsupported type: \" << ctrl->type();\n> > +\t\t\treturn -EINVAL;\n> > +\t\t}\n> > +\t}\n> > +\n> > \t/*\n> > \t * Start the ImgU video devices, buffers will be queued to the\n> > \t * ImgU output and viewfinder when requests will be queued.\n> >\n>\n> --\n> Regards\n> --\n> Kieran","headers":{"Return-Path":"","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 02B2162F5C\n\tfor ;\n\tTue, 11 Jun 2019 17:04:29 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 8FAFC1C0005;\n\tTue, 11 Jun 2019 15:04:28 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Tue, 11 Jun 2019 17:05:41 +0200","From":"Jacopo Mondi ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190611150541.2kjeagupko7kijcj@uno.localdomain>","References":"<20190610164052.30741-1-jacopo@jmondi.org>\n\t<20190610164052.30741-7-jacopo@jmondi.org>\n\t<87e4cac9-f409-b51a-3240-ee2014826d2b@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"mouofwaoqypz7mgq\"","Content-Disposition":"inline","In-Reply-To":"<87e4cac9-f409-b51a-3240-ee2014826d2b@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 6/6] [HACK] ipu3: Set and get a few\n\tsensor controls","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 11 Jun 2019 15:04:29 -0000"}},{"id":1865,"web_url":"https://patchwork.libcamera.org/comment/1865/","msgid":"<119880fb-62b2-f2c6-d7fc-df51c8492881@ideasonboard.com>","date":"2019-06-11T22:53:19","subject":"Re: [libcamera-devel] [PATCH v2 6/6] [HACK] ipu3: Set and get a few\n\tsensor controls","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn 11/06/2019 16:05, Jacopo Mondi wrote:\n> Hi Kieran,\n> \n> On Tue, Jun 11, 2019 at 03:09:51PM +0100, Kieran Bingham wrote:\n>> Hi Jacopo,\n>>\n>> On 10/06/2019 17:40, Jacopo Mondi wrote:\n>>> Not to merge patch to demonstrate how to set controls on the image\n>>> sensor device.\n>>>\n>>> Not-Signed-off-by: Jacopo Mondi \n>>> ---\n>>> src/libcamera/pipeline/ipu3/ipu3.cpp | 80 ++++++++++++++++++++++++++++\n>>> 1 file changed, 80 insertions(+)\n>>>\n>>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> index b3e7fb0e9c64..59c1fe3c56fd 100644\n>>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> @@ -688,6 +688,86 @@ int PipelineHandlerIPU3::start(Camera *camera)\n>>> \tImgUDevice *imgu = data->imgu_;\n>>> \tint ret;\n>>>\n>>> +\t/* --- Get control values --- */\n>>> +\tstd::vector controlIds = {\n>>> +\t\tV4L2_CID_EXPOSURE, V4L2_CID_ANALOGUE_GAIN,\n>>> +\t};\n>>> +\tV4L2Controls controls;\n>>> +\tret = cio2->sensor_->dev()->getControls(controlIds, &controls);\n>>> +\tif (ret) {\n>>> +\t\tLOG(Error) << \"Failed to get control values\";\n>>> +\t\treturn -EINVAL;\n>>> +\t}\n>>> +\n>>\n>> Moving some logic for printing the values (or working out how to use\n>> ControlValue class in a V4L2Control) would simplify any debug prints\n>> greatly:\n>>\n>> for (V4L2Control *ctrl : controls) {\n>> \tLOG(Error) << \"Control : \" << id\n>> \t\t << \" - value: \" << ctrl->value();\n>> }\n>>\n> \n> Sorry, I might have missed how that would change using ControlValue.\n> That class has getInt, getBool etc, right?\n\nThe class ControlValue provides toString() and << overrides, so you can\ndo a direct << on a ControlValue.\n\nIt simplifies debug prints at least. Re-reading my statement, it looks\nlike I hadn't completed my sentence fully. I was trynig to say, \"If you\nmove the logic for printing the values into the V4L2Control class or\nV4L2ControlValue or such ...\"\n\n\n>>\n>>> +\tfor (V4L2Control *ctrl : controls) {\n>>> +\t\tunsigned int id = ctrl->id();\n>>> +\n>>> +\t\tswitch(ctrl->type()) {\n>>> +\t\tcase V4L2_CTRL_TYPE_INTEGER:\n>>> +\t\tcase V4L2_CTRL_TYPE_BOOLEAN:\n>>> +\t\t{\n>>> +\t\t\tuint32_t val = controls.getInt(id);\n>>> +\t\t\tLOG(Error) << \"Control : \" << id\n>>> +\t\t\t\t << \" - value: \" << val;\n>>> +\t\t}\n>>> +\t\t\tbreak;\n>>> +\t\tcase V4L2_CTRL_TYPE_INTEGER64:\n>>> +\t\t{\n>>> +\t\t\tuint64_t val = controls.getInt64(id);\n>>> +\t\t\tLOG(Error) << \"Control : \" << id\n>>> +\t\t\t\t << \" - value: \" << val;\n>>> +\t\t}\n>>> +\t\t\tbreak;\n>>> +\t\tdefault:\n>>> +\t\t\tLOG(Error) << \"Unsupported type: \" << ctrl->type();\n>>> +\t\t\treturn -EINVAL;\n>>> +\t\t}\n>>> +\t}\n>>> +\n>>\n>>\n>>\n>>> +\t/* --- Set control values --- */\n>>> +\tV4L2Controls setControls;\n>>> +\tsetControls.set(V4L2_CID_EXPOSURE, 2046);\n>>> +\tsetControls.set(V4L2_CID_ANALOGUE_GAIN, 1024);\n>>> +\n>>> +\tret = cio2->sensor_->dev()->setControls(setControls);\n>>> +\tif (ret) {\n>>> +\t\tLOG(IPU3, Error) << \"Failed to set controls\";\n>>> +\t\treturn ret;\n>>> +\t}\n>>> +\n>>> +\t/* --- Get control values back again and verify they have changed --- */\n>>> +\tV4L2Controls newcontrols;\n>>> +\tret = cio2->sensor_->dev()->getControls(controlIds, &newcontrols);\n>>> +\tif (ret) {\n>>> +\t\tLOG(Error) << \"Failed to get control values\";\n>>> +\t\treturn -EINVAL;\n>>> +\t}\n>>> +\n>>> +\tfor (V4L2Control *ctrl : newcontrols) {\n>>> +\t\tunsigned int id = ctrl->id();\n>>> +\n>>> +\t\tswitch(ctrl->type()) {\n>>> +\t\tcase V4L2_CTRL_TYPE_INTEGER:\n>>> +\t\tcase V4L2_CTRL_TYPE_BOOLEAN:\n>>> +\t\t{\n>>> +\t\t\tuint32_t val = newcontrols.getInt(id);\n>>\n>> Eeep, Here we are iterating new controls to print them (I get that this\n>> is just a print) - but we are then 'getting' the int from the list\n>> container instead of the V4L2Control, which is involving 'finding' the\n>> control again, and iterating the 'newcontrols' container for each print?\n>>\n>> Iterating the list twice seems painful to me... but perhaps that's just\n>> because of this particular demo use case, and it might not be such an\n>> issue in real usage.\n> \n> Accessing the V4L2Control value would require the user to cast the\n> V4L2Control * to the appropriate specialized derived class, something\n> we don't want.\n> \n> In this case yes, using a ControlValue object which has a field for\n> all possible value types and provides a getInt() getBool() etc would\n> remove the casting requirement and would allow to access a control\n> value with a single call (provided the user knows the control value\n> type, but we have the same issue here, where user should call the\n> opportune get$TYPE() anyhow.\n\nYes, that will depend on where the type is needed to be stored.\n\nBut one thing it does do is allow value exchange without caring about\nthe type.\n\nOne reason why I'd really like to see this class be common (i.e. use\nControlValue) is because then pipeline handlers do not need to care\nabout the types in the most part. They could do direct copies or stores\nof the ControlValue object, or have that object (from the application)\nfilled in directly.\n\nThe application must already know about the Type for dealing with\nControlValue class (which is application facing).\n\nPipelines could potentially find themselves dealing with lists of\ncontrols to either get or set them, and they won't care about the\ndata-type; they will just be the bridge between the Application and the\nV4L2 layer.\n\nThe application however, will know that if it is looking at a Brightness\ncontrol it is an integer, and if it's dealing with an Enable control\nit's boolean.\n\nPerhaps we might even enable signals and slots on controls so that the\napplication gets updated directly when a control is updated, allowing it\nto update the GUI or such.\n\n\n\n>>\n>>> +\t\t\tLOG(Error) << \"Control : \" << id\n>>> +\t\t\t\t << \" - value: \" << val;\n>>> +\t\t}\n>>> +\t\t\tbreak;\n>>> +\t\tcase V4L2_CTRL_TYPE_INTEGER64:\n>>> +\t\t{\n>>> +\t\t\tuint64_t val = newcontrols.getInt64(id);\n>>> +\t\t\tLOG(Error) << \"Control : \" << id\n>>> +\t\t\t\t << \" - value: \" << val;\n>>> +\t\t}\n>>> +\t\t\tbreak;\n>>> +\t\tdefault:\n>>> +\t\t\tLOG(Error) << \"Unsupported type: \" << ctrl->type();\n>>> +\t\t\treturn -EINVAL;\n>>> +\t\t}\n>>> +\t}\n>>> +\n>>> \t/*\n>>> \t * Start the ImgU video devices, buffers will be queued to the\n>>> \t * ImgU output and viewfinder when requests will be queued.\n>>>\n>>\n>> --\n>> Regards\n>> --\n>> Kieran","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5B5FF61F79\n\tfor ;\n\tWed, 12 Jun 2019 00:53:23 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7BC55FA0;\n\tWed, 12 Jun 2019 00:53:22 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1560293602;\n\tbh=VRIyqCLOY4zqXoGC3eKnTQLxmDIkysv2E03uM4d0RRE=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=CoVIWJbkFJHR/qNGI6HfUjVbkxW5x+u/43G62vL7uC7DPhhV2o1cH2RRdwL7Y+/li\n\tuHskSauvLNmStcFWi/k9s3R0kStf6EbNT2tufHK9HM5vS1J3E/lo3SHuZ+BIILdeiF\n\tYUD23nQX1iysvq3rQCC6g6/hugCBNgHYn9L63oUg=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20190610164052.30741-1-jacopo@jmondi.org>\n\t<20190610164052.30741-7-jacopo@jmondi.org>\n\t<87e4cac9-f409-b51a-3240-ee2014826d2b@ideasonboard.com>\n\t<20190611150541.2kjeagupko7kijcj@uno.localdomain>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<119880fb-62b2-f2c6-d7fc-df51c8492881@ideasonboard.com>","Date":"Tue, 11 Jun 2019 23:53:19 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.0","MIME-Version":"1.0","In-Reply-To":"<20190611150541.2kjeagupko7kijcj@uno.localdomain>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 6/6] [HACK] ipu3: Set and get a few\n\tsensor controls","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 11 Jun 2019 22:53:23 -0000"}}]