[{"id":1178,"web_url":"https://patchwork.libcamera.org/comment/1178/","msgid":"<20190402092613.GG4805@pendragon.ideasonboard.com>","date":"2019-04-02T09:26:13","subject":"Re: [libcamera-devel] [PATCH v5 11/19] libcamera: ipu3: Create\n\tImgUDevice class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Tue, Mar 26, 2019 at 09:38:54AM +0100, Jacopo Mondi wrote:\n> Group ImgU components in a class associated with a camera at camera\n> registration time and provide an intialization method to create and open\n> all video devices and subdevices of the ImgU.\n> \n> Statically assign imgu0 to the first camera and imgu1 to the second one\n> and limit support to two cameras. This will have to be revised in the future.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 154 +++++++++++++++++++++++++--\n> 1 file changed, 145 insertions(+), 9 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 21205b39afee..0a059b75cadf 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -44,6 +44,43 @@ static int mediaBusToCIO2Format(unsigned int code)\n> \t}\n> }\n> \n> +class ImgUDevice\n> +{\n> +public:\n> +\tstatic constexpr unsigned int PAD_INPUT = 0;\n> +\tstatic constexpr unsigned int PAD_OUTPUT = 2;\n> +\tstatic constexpr unsigned int PAD_VF = 3;\n> +\tstatic constexpr unsigned int PAD_STAT = 4;\n> +\n> +\tImgUDevice()\n> +\t\t: imgu_(nullptr), input_(nullptr), output_(nullptr),\n> +\t\t viewfinder_(nullptr), stat_(nullptr)\n> +\t{\n> +\t}\n> +\n> +\t~ImgUDevice()\n> +\t{\n> +\t\tdelete imgu_;\n> +\t\tdelete input_;\n> +\t\tdelete output_;\n> +\t\tdelete viewfinder_;\n> +\t\tdelete stat_;\n> +\t}\n> +\n> +\tint init(MediaDevice *media, unsigned int index);\n> +\n> +\tunsigned int index_;\n> +\tstd::string name_;\n> +\tMediaDevice *media_;\n> +\n> +\tV4L2Subdevice *imgu_;\n> +\tV4L2Device *input_;\n> +\tV4L2Device *output_;\n> +\tV4L2Device *viewfinder_;\n> +\tV4L2Device *stat_;\n> +\t/* \\todo Add param video device for 3A tuning */\n> +};\n> +\n> class CIO2Device\n> {\n> public:\n> @@ -106,6 +143,7 @@ private:\n> \t\tvoid bufferReady(Buffer *buffer);\n> \n> \t\tCIO2Device cio2_;\n> +\t\tImgUDevice *imgu_;\n> \n> \t\tStream stream_;\n> \t};\n> @@ -116,8 +154,10 @@ private:\n> \t\t\tPipelineHandler::cameraData(camera));\n> \t}\n> \n> -\tvoid registerCameras();\n> +\tint registerCameras();\n> \n> +\tImgUDevice imgu0_;\n> +\tImgUDevice imgu1_;\n> \tstd::shared_ptr cio2MediaDev_;\n> \tstd::shared_ptr imguMediaDev_;\n> };\n> @@ -303,6 +343,8 @@ int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)\n> \n> bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> {\n> +\tint ret;\n> +\n> \tDeviceMatch cio2_dm(\"ipu3-cio2\");\n> \tcio2_dm.add(\"ipu3-csi2 0\");\n> \tcio2_dm.add(\"ipu3-cio2 0\");\n> @@ -358,36 +400,75 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> \t\treturn false;\n> \t}\n> \n> -\tregisterCameras();\n> +\tif (imguMediaDev_->open()) {\n> +\t\tcio2MediaDev_->close();\n> +\t\treturn false;\n> +\t}\n> +\n> +\tif (imguMediaDev_->disableLinks())\n> +\t\tgoto error;\n> +\n> +\tret = registerCameras();\n> +\tif (ret)\n> +\t\tgoto error;\n> \n> \tcio2MediaDev_->close();\n> +\timguMediaDev_->close();\n> \n> \treturn true;\n> +\n> +error:\n> +\tcio2MediaDev_->close();\n> +\timguMediaDev_->close();\n> +\n> +\treturn false;\n> }\n> \n> -/*\n> - * Cameras are created associating an image sensor (represented by a\n> - * media entity with function MEDIA_ENT_F_CAM_SENSOR) to one of the four\n> - * CIO2 CSI-2 receivers.\n> +/**\n> + * \\brief Initialize ImgU devices and CIO2 ones associated with cameras\n\ns/ImgU devices and CIO2 ones/the ImgU and CIO2 devices/\n\nI'm sure Kieran would also tell you to s/Initialize/Initialise/ :-)\n\n> + *\n> + * Initialize the two ImgU instances and create cameras with an associated\n> + * CIO2 device instance.\n> + *\n> + * \\return 0 on success or a negative error code for error or if no camera\n> + * has been created\n> + * \\retval -ENODEV no camera has been created\n> */\n> -void PipelineHandlerIPU3::registerCameras()\n> +int PipelineHandlerIPU3::registerCameras()\n> {\n> +\tint ret;\n> +\n> +\tret = imgu0_.init(imguMediaDev_.get(), 0);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tret = imgu1_.init(imguMediaDev_.get(), 1);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> \t/*\n> \t * For each CSI-2 receiver on the IPU3, create a Camera if an\n> \t * image sensor is connected to it and it can produce images in\n> \t * a compatible format.\n> \t */\n> \tunsigned int numCameras = 0;\n> -\tfor (unsigned int id = 0; id < 4; ++id) {\n> +\tfor (unsigned int id = 0; id < 4 && numCameras < 2; ++id) {\n> \t\tstd::unique_ptr data =\n> \t\t\tutils::make_unique(this);\n> \t\tstd::set streams{ &data->stream_ };\n> \t\tCIO2Device *cio2 = &data->cio2_;\n> \n> -\t\tint ret = cio2->init(cio2MediaDev_.get(), id);\n> +\t\tret = cio2->init(cio2MediaDev_.get(), id);\n> \t\tif (ret)\n> \t\t\tcontinue;\n> \n> +\t\t/**\n> +\t\t * \\todo Dynamically assign ImgU devices; as of now, limit\n> +\t\t * support to two cameras only, and assign imgu0 to the first\n> +\t\t * one and imgu1 to the second.\n> +\t\t */\n> +\t\tdata->imgu_ = numCameras ? &imgu1_ : &imgu0_;\n> +\n> \t\tstd::string cameraName = cio2->name() + \" \"\n> \t\t\t\t + std::to_string(id);\n> \t\tstd::shared_ptr camera = Camera::create(this,\n> @@ -406,6 +487,8 @@ void PipelineHandlerIPU3::registerCameras()\n> \n> \t\tnumCameras++;\n> \t}\n> +\n> +\treturn numCameras ? 0 : -ENODEV;\n> }\n> \n> void PipelineHandlerIPU3::IPU3CameraData::bufferReady(Buffer *buffer)\n> @@ -416,6 +499,59 @@ void PipelineHandlerIPU3::IPU3CameraData::bufferReady(Buffer *buffer)\n> \tpipe_->completeRequest(camera_, request);\n> }\n> \n> +/* -----------------------------------------------------------------------------\n> + * ImgU Device\n> + */\n> +\n> +/**\n> + * \\brief Initialize components of the ImgU instance\n> + * \\param[in] mediaDevice The ImgU instance media device\n> + * \\param[in] index The ImgU instance index\n> + *\n> + * Create and open the V4L2 devices and subdevices of the ImgU instance\n> + * with \\a index.\n> + *\n> + * In case of errors the created V4L2Device and V4L2Subdevice instances\n> + * are destroyed at pipeline handler delete time.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int ImgUDevice::init(MediaDevice *media, unsigned int index)\n\nIt could make sense to pass the media device pointer and index to the\nconstructor, in order to avoid leaving the media_ and index_ members\nunitialized. The initialization of course belongs here. This would imply\ncreating the ImgUDevice instances dynamically. What do you think ? This\ncomment also applies to the previous patch.\n\n> +{\n> +\tint ret;\n> +\n> +\tindex_ = index;\n> +\tname_ = \"ipu3-imgu \" + std::to_string(index_);\n> +\tmedia_ = media;\n> +\n> +\timgu_ = V4L2Subdevice::fromEntityName(media, name_);\n\nShouldn't test if this (and the other V4L2Device instances below) is\nnull before trying to open it on the next line ?\n\nWith these small issues fixed,\n\nReviewed-by: Laurent Pinchart \n\n> +\tret = imgu_->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tinput_ = V4L2Device::fromEntityName(media, name_ + \" input\");\n> +\tret = input_->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\toutput_ = V4L2Device::fromEntityName(media, name_ + \" output\");\n> +\tret = output_->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tviewfinder_ = V4L2Device::fromEntityName(media, name_ + \" viewfinder\");\n> +\tret = viewfinder_->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tstat_ = V4L2Device::fromEntityName(media, name_ + \" 3a stat\");\n> +\tret = stat_->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\treturn 0;\n> +}\n> +\n> /*------------------------------------------------------------------------------\n> * CIO2 Device\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 28413600FD\n\tfor ;\n\tTue, 2 Apr 2019 11:26:24 +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 86CEA2F9;\n\tTue, 2 Apr 2019 11:26:23 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1554197183;\n\tbh=OiqawepCGwDPKs13FWzRi/Nd8Dux7df0fE43LJzymG4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pZCQEEPgeMlhPRnOVKiFT+8j3Ob1xKWLKJV7ZZSBHwVJnlUJOiH+w8lC6bFoSLEjQ\n\tYMuEbbOL0kawBoeHnQC90pDwAsTzcfwSh62W+iOVUMy3iY1WFG6Ec/heqnHtvK3O1Z\n\tL2jj65+ZFkODbt5qVV9USp6BNU0mRYvtibKkFY7o=","Date":"Tue, 2 Apr 2019 12:26:13 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190402092613.GG4805@pendragon.ideasonboard.com>","References":"<20190326083902.26121-1-jacopo@jmondi.org>\n\t<20190326083902.26121-12-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190326083902.26121-12-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v5 11/19] libcamera: ipu3: Create\n\tImgUDevice class","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, 02 Apr 2019 09:26:24 -0000"}},{"id":1184,"web_url":"https://patchwork.libcamera.org/comment/1184/","msgid":"<20190402103732.ekmnj7zngufrgvtm@uno.localdomain>","date":"2019-04-02T10:37:32","subject":"Re: [libcamera-devel] [PATCH v5 11/19] libcamera: ipu3: Create\n\tImgUDevice class","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Tue, Apr 02, 2019 at 12:26:13PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Tue, Mar 26, 2019 at 09:38:54AM +0100, Jacopo Mondi wrote:\n> > Group ImgU components in a class associated with a camera at camera\n> > registration time and provide an intialization method to create and open\n> > all video devices and subdevices of the ImgU.\n> >\n> > Statically assign imgu0 to the first camera and imgu1 to the second one\n> > and limit support to two cameras. This will have to be revised in the future.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 154 +++++++++++++++++++++++++--\n> > 1 file changed, 145 insertions(+), 9 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 21205b39afee..0a059b75cadf 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -44,6 +44,43 @@ static int mediaBusToCIO2Format(unsigned int code)\n> > \t}\n> > }\n> >\n> > +class ImgUDevice\n> > +{\n> > +public:\n> > +\tstatic constexpr unsigned int PAD_INPUT = 0;\n> > +\tstatic constexpr unsigned int PAD_OUTPUT = 2;\n> > +\tstatic constexpr unsigned int PAD_VF = 3;\n> > +\tstatic constexpr unsigned int PAD_STAT = 4;\n> > +\n> > +\tImgUDevice()\n> > +\t\t: imgu_(nullptr), input_(nullptr), output_(nullptr),\n> > +\t\t viewfinder_(nullptr), stat_(nullptr)\n> > +\t{\n> > +\t}\n> > +\n> > +\t~ImgUDevice()\n> > +\t{\n> > +\t\tdelete imgu_;\n> > +\t\tdelete input_;\n> > +\t\tdelete output_;\n> > +\t\tdelete viewfinder_;\n> > +\t\tdelete stat_;\n> > +\t}\n> > +\n> > +\tint init(MediaDevice *media, unsigned int index);\n> > +\n> > +\tunsigned int index_;\n> > +\tstd::string name_;\n> > +\tMediaDevice *media_;\n> > +\n> > +\tV4L2Subdevice *imgu_;\n> > +\tV4L2Device *input_;\n> > +\tV4L2Device *output_;\n> > +\tV4L2Device *viewfinder_;\n> > +\tV4L2Device *stat_;\n> > +\t/* \\todo Add param video device for 3A tuning */\n> > +};\n> > +\n> > class CIO2Device\n> > {\n> > public:\n> > @@ -106,6 +143,7 @@ private:\n> > \t\tvoid bufferReady(Buffer *buffer);\n> >\n> > \t\tCIO2Device cio2_;\n> > +\t\tImgUDevice *imgu_;\n> >\n> > \t\tStream stream_;\n> > \t};\n> > @@ -116,8 +154,10 @@ private:\n> > \t\t\tPipelineHandler::cameraData(camera));\n> > \t}\n> >\n> > -\tvoid registerCameras();\n> > +\tint registerCameras();\n> >\n> > +\tImgUDevice imgu0_;\n> > +\tImgUDevice imgu1_;\n> > \tstd::shared_ptr cio2MediaDev_;\n> > \tstd::shared_ptr imguMediaDev_;\n> > };\n> > @@ -303,6 +343,8 @@ int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)\n> >\n> > bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> > {\n> > +\tint ret;\n> > +\n> > \tDeviceMatch cio2_dm(\"ipu3-cio2\");\n> > \tcio2_dm.add(\"ipu3-csi2 0\");\n> > \tcio2_dm.add(\"ipu3-cio2 0\");\n> > @@ -358,36 +400,75 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> > \t\treturn false;\n> > \t}\n> >\n> > -\tregisterCameras();\n> > +\tif (imguMediaDev_->open()) {\n> > +\t\tcio2MediaDev_->close();\n> > +\t\treturn false;\n> > +\t}\n> > +\n> > +\tif (imguMediaDev_->disableLinks())\n> > +\t\tgoto error;\n> > +\n> > +\tret = registerCameras();\n> > +\tif (ret)\n> > +\t\tgoto error;\n> >\n> > \tcio2MediaDev_->close();\n> > +\timguMediaDev_->close();\n> >\n> > \treturn true;\n> > +\n> > +error:\n> > +\tcio2MediaDev_->close();\n> > +\timguMediaDev_->close();\n> > +\n> > +\treturn false;\n> > }\n> >\n> > -/*\n> > - * Cameras are created associating an image sensor (represented by a\n> > - * media entity with function MEDIA_ENT_F_CAM_SENSOR) to one of the four\n> > - * CIO2 CSI-2 receivers.\n> > +/**\n> > + * \\brief Initialize ImgU devices and CIO2 ones associated with cameras\n>\n> s/ImgU devices and CIO2 ones/the ImgU and CIO2 devices/\n>\n> I'm sure Kieran would also tell you to s/Initialize/Initialise/ :-)\n>\n\nIs this a british vs american english thing ? My spell checker does\nnot report 'initialize' as wrong :)\n\n> > + *\n> > + * Initialize the two ImgU instances and create cameras with an associated\n> > + * CIO2 device instance.\n> > + *\n> > + * \\return 0 on success or a negative error code for error or if no camera\n> > + * has been created\n> > + * \\retval -ENODEV no camera has been created\n> > */\n> > -void PipelineHandlerIPU3::registerCameras()\n> > +int PipelineHandlerIPU3::registerCameras()\n> > {\n> > +\tint ret;\n> > +\n> > +\tret = imgu0_.init(imguMediaDev_.get(), 0);\n> > +\tif (ret)\n> > +\t\treturn ret;\n> > +\n> > +\tret = imgu1_.init(imguMediaDev_.get(), 1);\n> > +\tif (ret)\n> > +\t\treturn ret;\n> > +\n> > \t/*\n> > \t * For each CSI-2 receiver on the IPU3, create a Camera if an\n> > \t * image sensor is connected to it and it can produce images in\n> > \t * a compatible format.\n> > \t */\n> > \tunsigned int numCameras = 0;\n> > -\tfor (unsigned int id = 0; id < 4; ++id) {\n> > +\tfor (unsigned int id = 0; id < 4 && numCameras < 2; ++id) {\n> > \t\tstd::unique_ptr data =\n> > \t\t\tutils::make_unique(this);\n> > \t\tstd::set streams{ &data->stream_ };\n> > \t\tCIO2Device *cio2 = &data->cio2_;\n> >\n> > -\t\tint ret = cio2->init(cio2MediaDev_.get(), id);\n> > +\t\tret = cio2->init(cio2MediaDev_.get(), id);\n> > \t\tif (ret)\n> > \t\t\tcontinue;\n> >\n> > +\t\t/**\n> > +\t\t * \\todo Dynamically assign ImgU devices; as of now, limit\n> > +\t\t * support to two cameras only, and assign imgu0 to the first\n> > +\t\t * one and imgu1 to the second.\n> > +\t\t */\n> > +\t\tdata->imgu_ = numCameras ? &imgu1_ : &imgu0_;\n> > +\n> > \t\tstd::string cameraName = cio2->name() + \" \"\n> > \t\t\t\t + std::to_string(id);\n> > \t\tstd::shared_ptr camera = Camera::create(this,\n> > @@ -406,6 +487,8 @@ void PipelineHandlerIPU3::registerCameras()\n> >\n> > \t\tnumCameras++;\n> > \t}\n> > +\n> > +\treturn numCameras ? 0 : -ENODEV;\n> > }\n> >\n> > void PipelineHandlerIPU3::IPU3CameraData::bufferReady(Buffer *buffer)\n> > @@ -416,6 +499,59 @@ void PipelineHandlerIPU3::IPU3CameraData::bufferReady(Buffer *buffer)\n> > \tpipe_->completeRequest(camera_, request);\n> > }\n> >\n> > +/* -----------------------------------------------------------------------------\n> > + * ImgU Device\n> > + */\n> > +\n> > +/**\n> > + * \\brief Initialize components of the ImgU instance\n> > + * \\param[in] mediaDevice The ImgU instance media device\n> > + * \\param[in] index The ImgU instance index\n> > + *\n> > + * Create and open the V4L2 devices and subdevices of the ImgU instance\n> > + * with \\a index.\n> > + *\n> > + * In case of errors the created V4L2Device and V4L2Subdevice instances\n> > + * are destroyed at pipeline handler delete time.\n> > + *\n> > + * \\return 0 on success or a negative error code otherwise\n> > + */\n> > +int ImgUDevice::init(MediaDevice *media, unsigned int index)\n>\n> It could make sense to pass the media device pointer and index to the\n> constructor, in order to avoid leaving the media_ and index_ members\n> unitialized. The initialization of course belongs here. This would imply\n> creating the ImgUDevice instances dynamically. What do you think ? This\n> comment also applies to the previous patch.\n\nThe ImgU units initialization happens as first thing at\nregisterCameras() time, if any of two fails, the match() function\nfails (registerCameras() now returns an int).\n\nI'm not sure where it would be risky to have those two member\nuninitialized, but I understand the concern.\n\nI usually try to minimize run-time allocations and I like the fact the\nImgU and CIO2 devices lifecycle is automatically tight to the\ncameraData's one, but it shouldn't be hard to handle them in the\ndestructor. If the change is worth in your opinion, I'll address it.\n\n>\n> > +{\n> > +\tint ret;\n> > +\n> > +\tindex_ = index;\n> > +\tname_ = \"ipu3-imgu \" + std::to_string(index_);\n> > +\tmedia_ = media;\n> > +\n> > +\timgu_ = V4L2Subdevice::fromEntityName(media, name_);\n>\n> Shouldn't test if this (and the other V4L2Device instances below) is\n> null before trying to open it on the next line ?\n\nThe match function has validated that the entity is present in the\nmedia device, the only reason this could fail is if we run out of\nmemory and the 'new V4L2Subdevice()' call fails. Is this worth\nchecking?\n\nThanks\n j\n\n>\n> With these small issues fixed,\n>\n> Reviewed-by: Laurent Pinchart \n>\n> > +\tret = imgu_->open();\n> > +\tif (ret)\n> > +\t\treturn ret;\n> > +\n> > +\tinput_ = V4L2Device::fromEntityName(media, name_ + \" input\");\n> > +\tret = input_->open();\n> > +\tif (ret)\n> > +\t\treturn ret;\n> > +\n> > +\toutput_ = V4L2Device::fromEntityName(media, name_ + \" output\");\n> > +\tret = output_->open();\n> > +\tif (ret)\n> > +\t\treturn ret;\n> > +\n> > +\tviewfinder_ = V4L2Device::fromEntityName(media, name_ + \" viewfinder\");\n> > +\tret = viewfinder_->open();\n> > +\tif (ret)\n> > +\t\treturn ret;\n> > +\n> > +\tstat_ = V4L2Device::fromEntityName(media, name_ + \" 3a stat\");\n> > +\tret = stat_->open();\n> > +\tif (ret)\n> > +\t\treturn ret;\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > /*------------------------------------------------------------------------------\n> > * CIO2 Device\n> > */\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","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 47272600FB\n\tfor ;\n\tTue, 2 Apr 2019 12:36:48 +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 relay8-d.mail.gandi.net (Postfix) with ESMTPSA id C075B1BF213;\n\tTue, 2 Apr 2019 10:36:47 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Tue, 2 Apr 2019 12:37:32 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190402103732.ekmnj7zngufrgvtm@uno.localdomain>","References":"<20190326083902.26121-1-jacopo@jmondi.org>\n\t<20190326083902.26121-12-jacopo@jmondi.org>\n\t<20190402092613.GG4805@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"2if52bfuhbor7kiy\"","Content-Disposition":"inline","In-Reply-To":"<20190402092613.GG4805@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v5 11/19] libcamera: ipu3: Create\n\tImgUDevice class","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, 02 Apr 2019 10:36:48 -0000"}},{"id":1185,"web_url":"https://patchwork.libcamera.org/comment/1185/","msgid":"<20190402104317.GB19747@pendragon.ideasonboard.com>","date":"2019-04-02T10:43:17","subject":"Re: [libcamera-devel] [PATCH v5 11/19] libcamera: ipu3: Create\n\tImgUDevice class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Tue, Apr 02, 2019 at 12:37:32PM +0200, Jacopo Mondi wrote:\n> On Tue, Apr 02, 2019 at 12:26:13PM +0300, Laurent Pinchart wrote:\n> > On Tue, Mar 26, 2019 at 09:38:54AM +0100, Jacopo Mondi wrote:\n> >> Group ImgU components in a class associated with a camera at camera\n> >> registration time and provide an intialization method to create and open\n> >> all video devices and subdevices of the ImgU.\n> >>\n> >> Statically assign imgu0 to the first camera and imgu1 to the second one\n> >> and limit support to two cameras. This will have to be revised in the future.\n> >>\n> >> Signed-off-by: Jacopo Mondi \n> >> ---\n> >> src/libcamera/pipeline/ipu3/ipu3.cpp | 154 +++++++++++++++++++++++++--\n> >> 1 file changed, 145 insertions(+), 9 deletions(-)\n> >>\n> >> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >> index 21205b39afee..0a059b75cadf 100644\n> >> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >> @@ -44,6 +44,43 @@ static int mediaBusToCIO2Format(unsigned int code)\n> >> \t}\n> >> }\n> >>\n> >> +class ImgUDevice\n> >> +{\n> >> +public:\n> >> +\tstatic constexpr unsigned int PAD_INPUT = 0;\n> >> +\tstatic constexpr unsigned int PAD_OUTPUT = 2;\n> >> +\tstatic constexpr unsigned int PAD_VF = 3;\n> >> +\tstatic constexpr unsigned int PAD_STAT = 4;\n> >> +\n> >> +\tImgUDevice()\n> >> +\t\t: imgu_(nullptr), input_(nullptr), output_(nullptr),\n> >> +\t\t viewfinder_(nullptr), stat_(nullptr)\n> >> +\t{\n> >> +\t}\n> >> +\n> >> +\t~ImgUDevice()\n> >> +\t{\n> >> +\t\tdelete imgu_;\n> >> +\t\tdelete input_;\n> >> +\t\tdelete output_;\n> >> +\t\tdelete viewfinder_;\n> >> +\t\tdelete stat_;\n> >> +\t}\n> >> +\n> >> +\tint init(MediaDevice *media, unsigned int index);\n> >> +\n> >> +\tunsigned int index_;\n> >> +\tstd::string name_;\n> >> +\tMediaDevice *media_;\n> >> +\n> >> +\tV4L2Subdevice *imgu_;\n> >> +\tV4L2Device *input_;\n> >> +\tV4L2Device *output_;\n> >> +\tV4L2Device *viewfinder_;\n> >> +\tV4L2Device *stat_;\n> >> +\t/* \\todo Add param video device for 3A tuning */\n> >> +};\n> >> +\n> >> class CIO2Device\n> >> {\n> >> public:\n> >> @@ -106,6 +143,7 @@ private:\n> >> \t\tvoid bufferReady(Buffer *buffer);\n> >>\n> >> \t\tCIO2Device cio2_;\n> >> +\t\tImgUDevice *imgu_;\n> >>\n> >> \t\tStream stream_;\n> >> \t};\n> >> @@ -116,8 +154,10 @@ private:\n> >> \t\t\tPipelineHandler::cameraData(camera));\n> >> \t}\n> >>\n> >> -\tvoid registerCameras();\n> >> +\tint registerCameras();\n> >>\n> >> +\tImgUDevice imgu0_;\n> >> +\tImgUDevice imgu1_;\n> >> \tstd::shared_ptr cio2MediaDev_;\n> >> \tstd::shared_ptr imguMediaDev_;\n> >> };\n> >> @@ -303,6 +343,8 @@ int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)\n> >>\n> >> bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> >> {\n> >> +\tint ret;\n> >> +\n> >> \tDeviceMatch cio2_dm(\"ipu3-cio2\");\n> >> \tcio2_dm.add(\"ipu3-csi2 0\");\n> >> \tcio2_dm.add(\"ipu3-cio2 0\");\n> >> @@ -358,36 +400,75 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> >> \t\treturn false;\n> >> \t}\n> >>\n> >> -\tregisterCameras();\n> >> +\tif (imguMediaDev_->open()) {\n> >> +\t\tcio2MediaDev_->close();\n> >> +\t\treturn false;\n> >> +\t}\n> >> +\n> >> +\tif (imguMediaDev_->disableLinks())\n> >> +\t\tgoto error;\n> >> +\n> >> +\tret = registerCameras();\n> >> +\tif (ret)\n> >> +\t\tgoto error;\n> >>\n> >> \tcio2MediaDev_->close();\n> >> +\timguMediaDev_->close();\n> >>\n> >> \treturn true;\n> >> +\n> >> +error:\n> >> +\tcio2MediaDev_->close();\n> >> +\timguMediaDev_->close();\n> >> +\n> >> +\treturn false;\n> >> }\n> >>\n> >> -/*\n> >> - * Cameras are created associating an image sensor (represented by a\n> >> - * media entity with function MEDIA_ENT_F_CAM_SENSOR) to one of the four\n> >> - * CIO2 CSI-2 receivers.\n> >> +/**\n> >> + * \\brief Initialize ImgU devices and CIO2 ones associated with cameras\n> >\n> > s/ImgU devices and CIO2 ones/the ImgU and CIO2 devices/\n> >\n> > I'm sure Kieran would also tell you to s/Initialize/Initialise/ :-)\n> >\n> \n> Is this a british vs american english thing ? My spell checker does\n> not report 'initialize' as wrong :)\n\nCorrect. Your spell checker likely only reports the use of Incorrect\nEnglish (https://lwn.net/Articles/636286/).\n\n> >> + *\n> >> + * Initialize the two ImgU instances and create cameras with an associated\n> >> + * CIO2 device instance.\n> >> + *\n> >> + * \\return 0 on success or a negative error code for error or if no camera\n> >> + * has been created\n> >> + * \\retval -ENODEV no camera has been created\n> >> */\n> >> -void PipelineHandlerIPU3::registerCameras()\n> >> +int PipelineHandlerIPU3::registerCameras()\n> >> {\n> >> +\tint ret;\n> >> +\n> >> +\tret = imgu0_.init(imguMediaDev_.get(), 0);\n> >> +\tif (ret)\n> >> +\t\treturn ret;\n> >> +\n> >> +\tret = imgu1_.init(imguMediaDev_.get(), 1);\n> >> +\tif (ret)\n> >> +\t\treturn ret;\n> >> +\n> >> \t/*\n> >> \t * For each CSI-2 receiver on the IPU3, create a Camera if an\n> >> \t * image sensor is connected to it and it can produce images in\n> >> \t * a compatible format.\n> >> \t */\n> >> \tunsigned int numCameras = 0;\n> >> -\tfor (unsigned int id = 0; id < 4; ++id) {\n> >> +\tfor (unsigned int id = 0; id < 4 && numCameras < 2; ++id) {\n> >> \t\tstd::unique_ptr data =\n> >> \t\t\tutils::make_unique(this);\n> >> \t\tstd::set streams{ &data->stream_ };\n> >> \t\tCIO2Device *cio2 = &data->cio2_;\n> >>\n> >> -\t\tint ret = cio2->init(cio2MediaDev_.get(), id);\n> >> +\t\tret = cio2->init(cio2MediaDev_.get(), id);\n> >> \t\tif (ret)\n> >> \t\t\tcontinue;\n> >>\n> >> +\t\t/**\n> >> +\t\t * \\todo Dynamically assign ImgU devices; as of now, limit\n> >> +\t\t * support to two cameras only, and assign imgu0 to the first\n> >> +\t\t * one and imgu1 to the second.\n> >> +\t\t */\n> >> +\t\tdata->imgu_ = numCameras ? &imgu1_ : &imgu0_;\n> >> +\n> >> \t\tstd::string cameraName = cio2->name() + \" \"\n> >> \t\t\t\t + std::to_string(id);\n> >> \t\tstd::shared_ptr camera = Camera::create(this,\n> >> @@ -406,6 +487,8 @@ void PipelineHandlerIPU3::registerCameras()\n> >>\n> >> \t\tnumCameras++;\n> >> \t}\n> >> +\n> >> +\treturn numCameras ? 0 : -ENODEV;\n> >> }\n> >>\n> >> void PipelineHandlerIPU3::IPU3CameraData::bufferReady(Buffer *buffer)\n> >> @@ -416,6 +499,59 @@ void PipelineHandlerIPU3::IPU3CameraData::bufferReady(Buffer *buffer)\n> >> \tpipe_->completeRequest(camera_, request);\n> >> }\n> >>\n> >> +/* -----------------------------------------------------------------------------\n> >> + * ImgU Device\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\brief Initialize components of the ImgU instance\n> >> + * \\param[in] mediaDevice The ImgU instance media device\n> >> + * \\param[in] index The ImgU instance index\n> >> + *\n> >> + * Create and open the V4L2 devices and subdevices of the ImgU instance\n> >> + * with \\a index.\n> >> + *\n> >> + * In case of errors the created V4L2Device and V4L2Subdevice instances\n> >> + * are destroyed at pipeline handler delete time.\n> >> + *\n> >> + * \\return 0 on success or a negative error code otherwise\n> >> + */\n> >> +int ImgUDevice::init(MediaDevice *media, unsigned int index)\n> >\n> > It could make sense to pass the media device pointer and index to the\n> > constructor, in order to avoid leaving the media_ and index_ members\n> > unitialized. The initialization of course belongs here. This would imply\n> > creating the ImgUDevice instances dynamically. What do you think ? This\n> > comment also applies to the previous patch.\n> \n> The ImgU units initialization happens as first thing at\n> registerCameras() time, if any of two fails, the match() function\n> fails (registerCameras() now returns an int).\n> \n> I'm not sure where it would be risky to have those two member\n> uninitialized, but I understand the concern.\n> \n> I usually try to minimize run-time allocations and I like the fact the\n> ImgU and CIO2 devices lifecycle is automatically tight to the\n> cameraData's one, but it shouldn't be hard to handle them in the\n> destructor. If the change is worth in your opinion, I'll address it.\n\nThis is internal code so it shouldn't be too dangerous (would be another\nstory if the object was exposed to applications, or even just within\nlibcamera). I would have a slight preference for correctness myself, but\nI agree that not having to destroy the objects is a good argument\nagainst this change. Pros and cons, I'll let you decide :-)\n\n> >> +{\n> >> +\tint ret;\n> >> +\n> >> +\tindex_ = index;\n> >> +\tname_ = \"ipu3-imgu \" + std::to_string(index_);\n> >> +\tmedia_ = media;\n> >> +\n> >> +\timgu_ = V4L2Subdevice::fromEntityName(media, name_);\n> >\n> > Shouldn't test if this (and the other V4L2Device instances below) is\n> > null before trying to open it on the next line ?\n> \n> The match function has validated that the entity is present in the\n> media device, the only reason this could fail is if we run out of\n> memory and the 'new V4L2Subdevice()' call fails. Is this worth\n> checking?\n\nPossibly not. If you don't check this, please add a comment that\nexplains why the check is not needed.\n\n> > With these small issues fixed,\n> >\n> > Reviewed-by: Laurent Pinchart \n> >\n> >> +\tret = imgu_->open();\n> >> +\tif (ret)\n> >> +\t\treturn ret;\n> >> +\n> >> +\tinput_ = V4L2Device::fromEntityName(media, name_ + \" input\");\n> >> +\tret = input_->open();\n> >> +\tif (ret)\n> >> +\t\treturn ret;\n> >> +\n> >> +\toutput_ = V4L2Device::fromEntityName(media, name_ + \" output\");\n> >> +\tret = output_->open();\n> >> +\tif (ret)\n> >> +\t\treturn ret;\n> >> +\n> >> +\tviewfinder_ = V4L2Device::fromEntityName(media, name_ + \" viewfinder\");\n> >> +\tret = viewfinder_->open();\n> >> +\tif (ret)\n> >> +\t\treturn ret;\n> >> +\n> >> +\tstat_ = V4L2Device::fromEntityName(media, name_ + \" 3a stat\");\n> >> +\tret = stat_->open();\n> >> +\tif (ret)\n> >> +\t\treturn ret;\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >> /*------------------------------------------------------------------------------\n> >> * CIO2 Device\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 EC797600FB\n\tfor ;\n\tTue, 2 Apr 2019 12:43:28 +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 586242F9;\n\tTue, 2 Apr 2019 12:43:28 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1554201808;\n\tbh=T2ULJiP8H3E2bqCTFqqf2UmjMXAgmrqym0fvmrbn5jg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RRdrZ8AL8+xDJvobjYbDOOlXv4VLRcHTFLWnC43adSOL5Tg6X+cLwpptKo9c+GF7w\n\tGwfm1EqT4hOhParpwCkLpHoI+3h3tLxfFsxC0otORnPjRdgmYNeLe/ST51DCZB2YC/\n\tAOsswt1EFQZyf/2YsnSVVOy1aV+cZeNquIklt5BY=","Date":"Tue, 2 Apr 2019 13:43:17 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190402104317.GB19747@pendragon.ideasonboard.com>","References":"<20190326083902.26121-1-jacopo@jmondi.org>\n\t<20190326083902.26121-12-jacopo@jmondi.org>\n\t<20190402092613.GG4805@pendragon.ideasonboard.com>\n\t<20190402103732.ekmnj7zngufrgvtm@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190402103732.ekmnj7zngufrgvtm@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v5 11/19] libcamera: ipu3: Create\n\tImgUDevice class","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, 02 Apr 2019 10:43:29 -0000"}},{"id":1189,"web_url":"https://patchwork.libcamera.org/comment/1189/","msgid":"<20190402105354.ud2lm7xkya6is3wl@uno.localdomain>","date":"2019-04-02T10:53:54","subject":"Re: [libcamera-devel] [PATCH v5 11/19] libcamera: ipu3: Create\n\tImgUDevice class","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Tue, Apr 02, 2019 at 01:43:17PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Tue, Apr 02, 2019 at 12:37:32PM +0200, Jacopo Mondi wrote:\n> > On Tue, Apr 02, 2019 at 12:26:13PM +0300, Laurent Pinchart wrote:\n> > > On Tue, Mar 26, 2019 at 09:38:54AM +0100, Jacopo Mondi wrote:\n> > >> Group ImgU components in a class associated with a camera at camera\n> > >> registration time and provide an intialization method to create and open\n> > >> all video devices and subdevices of the ImgU.\n> > >>\n> > >> Statically assign imgu0 to the first camera and imgu1 to the second one\n> > >> and limit support to two cameras. This will have to be revised in the future.\n> > >>\n> > >> Signed-off-by: Jacopo Mondi \n> > >> ---\n> > >> src/libcamera/pipeline/ipu3/ipu3.cpp | 154 +++++++++++++++++++++++++--\n> > >> 1 file changed, 145 insertions(+), 9 deletions(-)\n> > >>\n> > >> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > >> index 21205b39afee..0a059b75cadf 100644\n> > >> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > >> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > >> @@ -44,6 +44,43 @@ static int mediaBusToCIO2Format(unsigned int code)\n> > >> \t}\n> > >> }\n> > >>\n> > >> +class ImgUDevice\n> > >> +{\n> > >> +public:\n> > >> +\tstatic constexpr unsigned int PAD_INPUT = 0;\n> > >> +\tstatic constexpr unsigned int PAD_OUTPUT = 2;\n> > >> +\tstatic constexpr unsigned int PAD_VF = 3;\n> > >> +\tstatic constexpr unsigned int PAD_STAT = 4;\n> > >> +\n> > >> +\tImgUDevice()\n> > >> +\t\t: imgu_(nullptr), input_(nullptr), output_(nullptr),\n> > >> +\t\t viewfinder_(nullptr), stat_(nullptr)\n> > >> +\t{\n> > >> +\t}\n> > >> +\n> > >> +\t~ImgUDevice()\n> > >> +\t{\n> > >> +\t\tdelete imgu_;\n> > >> +\t\tdelete input_;\n> > >> +\t\tdelete output_;\n> > >> +\t\tdelete viewfinder_;\n> > >> +\t\tdelete stat_;\n> > >> +\t}\n> > >> +\n> > >> +\tint init(MediaDevice *media, unsigned int index);\n> > >> +\n> > >> +\tunsigned int index_;\n> > >> +\tstd::string name_;\n> > >> +\tMediaDevice *media_;\n> > >> +\n> > >> +\tV4L2Subdevice *imgu_;\n> > >> +\tV4L2Device *input_;\n> > >> +\tV4L2Device *output_;\n> > >> +\tV4L2Device *viewfinder_;\n> > >> +\tV4L2Device *stat_;\n> > >> +\t/* \\todo Add param video device for 3A tuning */\n> > >> +};\n> > >> +\n> > >> class CIO2Device\n> > >> {\n> > >> public:\n> > >> @@ -106,6 +143,7 @@ private:\n> > >> \t\tvoid bufferReady(Buffer *buffer);\n> > >>\n> > >> \t\tCIO2Device cio2_;\n> > >> +\t\tImgUDevice *imgu_;\n> > >>\n> > >> \t\tStream stream_;\n> > >> \t};\n> > >> @@ -116,8 +154,10 @@ private:\n> > >> \t\t\tPipelineHandler::cameraData(camera));\n> > >> \t}\n> > >>\n> > >> -\tvoid registerCameras();\n> > >> +\tint registerCameras();\n> > >>\n> > >> +\tImgUDevice imgu0_;\n> > >> +\tImgUDevice imgu1_;\n> > >> \tstd::shared_ptr cio2MediaDev_;\n> > >> \tstd::shared_ptr imguMediaDev_;\n> > >> };\n> > >> @@ -303,6 +343,8 @@ int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)\n> > >>\n> > >> bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> > >> {\n> > >> +\tint ret;\n> > >> +\n> > >> \tDeviceMatch cio2_dm(\"ipu3-cio2\");\n> > >> \tcio2_dm.add(\"ipu3-csi2 0\");\n> > >> \tcio2_dm.add(\"ipu3-cio2 0\");\n> > >> @@ -358,36 +400,75 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> > >> \t\treturn false;\n> > >> \t}\n> > >>\n> > >> -\tregisterCameras();\n> > >> +\tif (imguMediaDev_->open()) {\n> > >> +\t\tcio2MediaDev_->close();\n> > >> +\t\treturn false;\n> > >> +\t}\n> > >> +\n> > >> +\tif (imguMediaDev_->disableLinks())\n> > >> +\t\tgoto error;\n> > >> +\n> > >> +\tret = registerCameras();\n> > >> +\tif (ret)\n> > >> +\t\tgoto error;\n> > >>\n> > >> \tcio2MediaDev_->close();\n> > >> +\timguMediaDev_->close();\n> > >>\n> > >> \treturn true;\n> > >> +\n> > >> +error:\n> > >> +\tcio2MediaDev_->close();\n> > >> +\timguMediaDev_->close();\n> > >> +\n> > >> +\treturn false;\n> > >> }\n> > >>\n> > >> -/*\n> > >> - * Cameras are created associating an image sensor (represented by a\n> > >> - * media entity with function MEDIA_ENT_F_CAM_SENSOR) to one of the four\n> > >> - * CIO2 CSI-2 receivers.\n> > >> +/**\n> > >> + * \\brief Initialize ImgU devices and CIO2 ones associated with cameras\n> > >\n> > > s/ImgU devices and CIO2 ones/the ImgU and CIO2 devices/\n> > >\n> > > I'm sure Kieran would also tell you to s/Initialize/Initialise/ :-)\n> > >\n> >\n> > Is this a british vs american english thing ? My spell checker does\n> > not report 'initialize' as wrong :)\n>\n> Correct. Your spell checker likely only reports the use of Incorrect\n> English (https://lwn.net/Articles/636286/).\n>\n\nThat's certainly the version of English I know best.\n\n> > >> + *\n> > >> + * Initialize the two ImgU instances and create cameras with an associated\n> > >> + * CIO2 device instance.\n> > >> + *\n> > >> + * \\return 0 on success or a negative error code for error or if no camera\n> > >> + * has been created\n> > >> + * \\retval -ENODEV no camera has been created\n> > >> */\n> > >> -void PipelineHandlerIPU3::registerCameras()\n> > >> +int PipelineHandlerIPU3::registerCameras()\n> > >> {\n> > >> +\tint ret;\n> > >> +\n> > >> +\tret = imgu0_.init(imguMediaDev_.get(), 0);\n> > >> +\tif (ret)\n> > >> +\t\treturn ret;\n> > >> +\n> > >> +\tret = imgu1_.init(imguMediaDev_.get(), 1);\n> > >> +\tif (ret)\n> > >> +\t\treturn ret;\n> > >> +\n> > >> \t/*\n> > >> \t * For each CSI-2 receiver on the IPU3, create a Camera if an\n> > >> \t * image sensor is connected to it and it can produce images in\n> > >> \t * a compatible format.\n> > >> \t */\n> > >> \tunsigned int numCameras = 0;\n> > >> -\tfor (unsigned int id = 0; id < 4; ++id) {\n> > >> +\tfor (unsigned int id = 0; id < 4 && numCameras < 2; ++id) {\n> > >> \t\tstd::unique_ptr data =\n> > >> \t\t\tutils::make_unique(this);\n> > >> \t\tstd::set streams{ &data->stream_ };\n> > >> \t\tCIO2Device *cio2 = &data->cio2_;\n> > >>\n> > >> -\t\tint ret = cio2->init(cio2MediaDev_.get(), id);\n> > >> +\t\tret = cio2->init(cio2MediaDev_.get(), id);\n> > >> \t\tif (ret)\n> > >> \t\t\tcontinue;\n> > >>\n> > >> +\t\t/**\n> > >> +\t\t * \\todo Dynamically assign ImgU devices; as of now, limit\n> > >> +\t\t * support to two cameras only, and assign imgu0 to the first\n> > >> +\t\t * one and imgu1 to the second.\n> > >> +\t\t */\n> > >> +\t\tdata->imgu_ = numCameras ? &imgu1_ : &imgu0_;\n> > >> +\n> > >> \t\tstd::string cameraName = cio2->name() + \" \"\n> > >> \t\t\t\t + std::to_string(id);\n> > >> \t\tstd::shared_ptr camera = Camera::create(this,\n> > >> @@ -406,6 +487,8 @@ void PipelineHandlerIPU3::registerCameras()\n> > >>\n> > >> \t\tnumCameras++;\n> > >> \t}\n> > >> +\n> > >> +\treturn numCameras ? 0 : -ENODEV;\n> > >> }\n> > >>\n> > >> void PipelineHandlerIPU3::IPU3CameraData::bufferReady(Buffer *buffer)\n> > >> @@ -416,6 +499,59 @@ void PipelineHandlerIPU3::IPU3CameraData::bufferReady(Buffer *buffer)\n> > >> \tpipe_->completeRequest(camera_, request);\n> > >> }\n> > >>\n> > >> +/* -----------------------------------------------------------------------------\n> > >> + * ImgU Device\n> > >> + */\n> > >> +\n> > >> +/**\n> > >> + * \\brief Initialize components of the ImgU instance\n> > >> + * \\param[in] mediaDevice The ImgU instance media device\n> > >> + * \\param[in] index The ImgU instance index\n> > >> + *\n> > >> + * Create and open the V4L2 devices and subdevices of the ImgU instance\n> > >> + * with \\a index.\n> > >> + *\n> > >> + * In case of errors the created V4L2Device and V4L2Subdevice instances\n> > >> + * are destroyed at pipeline handler delete time.\n> > >> + *\n> > >> + * \\return 0 on success or a negative error code otherwise\n> > >> + */\n> > >> +int ImgUDevice::init(MediaDevice *media, unsigned int index)\n> > >\n> > > It could make sense to pass the media device pointer and index to the\n> > > constructor, in order to avoid leaving the media_ and index_ members\n> > > unitialized. The initialization of course belongs here. This would imply\n> > > creating the ImgUDevice instances dynamically. What do you think ? This\n> > > comment also applies to the previous patch.\n> >\n> > The ImgU units initialization happens as first thing at\n> > registerCameras() time, if any of two fails, the match() function\n> > fails (registerCameras() now returns an int).\n> >\n> > I'm not sure where it would be risky to have those two member\n> > uninitialized, but I understand the concern.\n> >\n> > I usually try to minimize run-time allocations and I like the fact the\n> > ImgU and CIO2 devices lifecycle is automatically tight to the\n> > cameraData's one, but it shouldn't be hard to handle them in the\n> > destructor. If the change is worth in your opinion, I'll address it.\n>\n> This is internal code so it shouldn't be too dangerous (would be another\n> story if the object was exposed to applications, or even just within\n> libcamera). I would have a slight preference for correctness myself, but\n> I agree that not having to destroy the objects is a good argument\n> against this change. Pros and cons, I'll let you decide :-)\n>\n\nI would keep them statically allocated if that's fine with you, as you\nsaid, this is internal code after all...\n\n> > >> +{\n> > >> +\tint ret;\n> > >> +\n> > >> +\tindex_ = index;\n> > >> +\tname_ = \"ipu3-imgu \" + std::to_string(index_);\n> > >> +\tmedia_ = media;\n> > >> +\n> > >> +\timgu_ = V4L2Subdevice::fromEntityName(media, name_);\n> > >\n> > > Shouldn't test if this (and the other V4L2Device instances below) is\n> > > null before trying to open it on the next line ?\n> >\n> > The match function has validated that the entity is present in the\n> > media device, the only reason this could fail is if we run out of\n> > memory and the 'new V4L2Subdevice()' call fails. Is this worth\n> > checking?\n>\n> Possibly not. If you don't check this, please add a comment that\n> explains why the check is not needed.\n>\n\nSure thing.\n\n> > > With these small issues fixed,\n> > >\n> > > Reviewed-by: Laurent Pinchart \n\nI'll take your tag in with the above clarified.\n\nThanks\n j\n\n> > >\n> > >> +\tret = imgu_->open();\n> > >> +\tif (ret)\n> > >> +\t\treturn ret;\n> > >> +\n> > >> +\tinput_ = V4L2Device::fromEntityName(media, name_ + \" input\");\n> > >> +\tret = input_->open();\n> > >> +\tif (ret)\n> > >> +\t\treturn ret;\n> > >> +\n> > >> +\toutput_ = V4L2Device::fromEntityName(media, name_ + \" output\");\n> > >> +\tret = output_->open();\n> > >> +\tif (ret)\n> > >> +\t\treturn ret;\n> > >> +\n> > >> +\tviewfinder_ = V4L2Device::fromEntityName(media, name_ + \" viewfinder\");\n> > >> +\tret = viewfinder_->open();\n> > >> +\tif (ret)\n> > >> +\t\treturn ret;\n> > >> +\n> > >> +\tstat_ = V4L2Device::fromEntityName(media, name_ + \" 3a stat\");\n> > >> +\tret = stat_->open();\n> > >> +\tif (ret)\n> > >> +\t\treturn ret;\n> > >> +\n> > >> +\treturn 0;\n> > >> +}\n> > >> +\n> > >> /*------------------------------------------------------------------------------\n> > >> * CIO2 Device\n> > >> */\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 96F39600FB\n\tfor ;\n\tTue, 2 Apr 2019 12:53:09 +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 relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 2AF7B40005;\n\tTue, 2 Apr 2019 10:53:08 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Tue, 2 Apr 2019 12:53:54 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190402105354.ud2lm7xkya6is3wl@uno.localdomain>","References":"<20190326083902.26121-1-jacopo@jmondi.org>\n\t<20190326083902.26121-12-jacopo@jmondi.org>\n\t<20190402092613.GG4805@pendragon.ideasonboard.com>\n\t<20190402103732.ekmnj7zngufrgvtm@uno.localdomain>\n\t<20190402104317.GB19747@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"rvfayai45ywjltnl\"","Content-Disposition":"inline","In-Reply-To":"<20190402104317.GB19747@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v5 11/19] libcamera: ipu3: Create\n\tImgUDevice class","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, 02 Apr 2019 10:53:09 -0000"}},{"id":1194,"web_url":"https://patchwork.libcamera.org/comment/1194/","msgid":"<20190402112848.GN23466@bigcity.dyn.berto.se>","date":"2019-04-02T11:28:48","subject":"Re: [libcamera-devel] [PATCH v5 11/19] libcamera: ipu3: Create\n\tImgUDevice class","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your work.\n\nOn 2019-03-26 09:38:54 +0100, Jacopo Mondi wrote:\n> Group ImgU components in a class associated with a camera at camera\n> registration time and provide an intialization method to create and open\n> all video devices and subdevices of the ImgU.\n> \n> Statically assign imgu0 to the first camera and imgu1 to the second one\n> and limit support to two cameras. This will have to be revised in the future.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 154 +++++++++++++++++++++++++--\n> 1 file changed, 145 insertions(+), 9 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 21205b39afee..0a059b75cadf 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -44,6 +44,43 @@ static int mediaBusToCIO2Format(unsigned int code)\n> \t}\n> }\n> \n> +class ImgUDevice\n> +{\n> +public:\n> +\tstatic constexpr unsigned int PAD_INPUT = 0;\n> +\tstatic constexpr unsigned int PAD_OUTPUT = 2;\n> +\tstatic constexpr unsigned int PAD_VF = 3;\n> +\tstatic constexpr unsigned int PAD_STAT = 4;\n> +\n> +\tImgUDevice()\n> +\t\t: imgu_(nullptr), input_(nullptr), output_(nullptr),\n> +\t\t viewfinder_(nullptr), stat_(nullptr)\n> +\t{\n> +\t}\n> +\n> +\t~ImgUDevice()\n> +\t{\n> +\t\tdelete imgu_;\n> +\t\tdelete input_;\n> +\t\tdelete output_;\n> +\t\tdelete viewfinder_;\n> +\t\tdelete stat_;\n> +\t}\n> +\n> +\tint init(MediaDevice *media, unsigned int index);\n> +\n> +\tunsigned int index_;\n> +\tstd::string name_;\n> +\tMediaDevice *media_;\n> +\n> +\tV4L2Subdevice *imgu_;\n> +\tV4L2Device *input_;\n> +\tV4L2Device *output_;\n> +\tV4L2Device *viewfinder_;\n> +\tV4L2Device *stat_;\n> +\t/* \\todo Add param video device for 3A tuning */\n> +};\n> +\n> class CIO2Device\n> {\n> public:\n> @@ -106,6 +143,7 @@ private:\n> \t\tvoid bufferReady(Buffer *buffer);\n> \n> \t\tCIO2Device cio2_;\n> +\t\tImgUDevice *imgu_;\n> \n> \t\tStream stream_;\n> \t};\n> @@ -116,8 +154,10 @@ private:\n> \t\t\tPipelineHandler::cameraData(camera));\n> \t}\n> \n> -\tvoid registerCameras();\n> +\tint registerCameras();\n> \n> +\tImgUDevice imgu0_;\n> +\tImgUDevice imgu1_;\n> \tstd::shared_ptr cio2MediaDev_;\n> \tstd::shared_ptr imguMediaDev_;\n> };\n> @@ -303,6 +343,8 @@ int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)\n> \n> bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> {\n> +\tint ret;\n> +\n> \tDeviceMatch cio2_dm(\"ipu3-cio2\");\n> \tcio2_dm.add(\"ipu3-csi2 0\");\n> \tcio2_dm.add(\"ipu3-cio2 0\");\n> @@ -358,36 +400,75 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> \t\treturn false;\n> \t}\n> \n> -\tregisterCameras();\n> +\tif (imguMediaDev_->open()) {\n> +\t\tcio2MediaDev_->close();\n> +\t\treturn false;\n> +\t}\n> +\n> +\tif (imguMediaDev_->disableLinks())\n> +\t\tgoto error;\n> +\n> +\tret = registerCameras();\n> +\tif (ret)\n> +\t\tgoto error;\n> \n> \tcio2MediaDev_->close();\n> +\timguMediaDev_->close();\n> \n> \treturn true;\n> +\n> +error:\n> +\tcio2MediaDev_->close();\n> +\timguMediaDev_->close();\n> +\n> +\treturn false;\n\nYou could make this a bit more neat,\n\nerror:\n cio2MediaDev_->close();\n imguMediaDev_->close();\n\n return ret == 0;\n\nIt would align the error and normal error path, with Laurents comments \naddressed and with or without this fixed,\n\nReviewed-by: Niklas Söderlund \n\n> }\n> \n> -/*\n> - * Cameras are created associating an image sensor (represented by a\n> - * media entity with function MEDIA_ENT_F_CAM_SENSOR) to one of the four\n> - * CIO2 CSI-2 receivers.\n> +/**\n> + * \\brief Initialize ImgU devices and CIO2 ones associated with cameras\n> + *\n> + * Initialize the two ImgU instances and create cameras with an associated\n> + * CIO2 device instance.\n> + *\n> + * \\return 0 on success or a negative error code for error or if no camera\n> + * has been created\n> + * \\retval -ENODEV no camera has been created\n> */\n> -void PipelineHandlerIPU3::registerCameras()\n> +int PipelineHandlerIPU3::registerCameras()\n> {\n> +\tint ret;\n> +\n> +\tret = imgu0_.init(imguMediaDev_.get(), 0);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tret = imgu1_.init(imguMediaDev_.get(), 1);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> \t/*\n> \t * For each CSI-2 receiver on the IPU3, create a Camera if an\n> \t * image sensor is connected to it and it can produce images in\n> \t * a compatible format.\n> \t */\n> \tunsigned int numCameras = 0;\n> -\tfor (unsigned int id = 0; id < 4; ++id) {\n> +\tfor (unsigned int id = 0; id < 4 && numCameras < 2; ++id) {\n> \t\tstd::unique_ptr data =\n> \t\t\tutils::make_unique(this);\n> \t\tstd::set streams{ &data->stream_ };\n> \t\tCIO2Device *cio2 = &data->cio2_;\n> \n> -\t\tint ret = cio2->init(cio2MediaDev_.get(), id);\n> +\t\tret = cio2->init(cio2MediaDev_.get(), id);\n> \t\tif (ret)\n> \t\t\tcontinue;\n> \n> +\t\t/**\n> +\t\t * \\todo Dynamically assign ImgU devices; as of now, limit\n> +\t\t * support to two cameras only, and assign imgu0 to the first\n> +\t\t * one and imgu1 to the second.\n> +\t\t */\n> +\t\tdata->imgu_ = numCameras ? &imgu1_ : &imgu0_;\n> +\n> \t\tstd::string cameraName = cio2->name() + \" \"\n> \t\t\t\t + std::to_string(id);\n> \t\tstd::shared_ptr camera = Camera::create(this,\n> @@ -406,6 +487,8 @@ void PipelineHandlerIPU3::registerCameras()\n> \n> \t\tnumCameras++;\n> \t}\n> +\n> +\treturn numCameras ? 0 : -ENODEV;\n> }\n> \n> void PipelineHandlerIPU3::IPU3CameraData::bufferReady(Buffer *buffer)\n> @@ -416,6 +499,59 @@ void PipelineHandlerIPU3::IPU3CameraData::bufferReady(Buffer *buffer)\n> \tpipe_->completeRequest(camera_, request);\n> }\n> \n> +/* -----------------------------------------------------------------------------\n> + * ImgU Device\n> + */\n> +\n> +/**\n> + * \\brief Initialize components of the ImgU instance\n> + * \\param[in] mediaDevice The ImgU instance media device\n> + * \\param[in] index The ImgU instance index\n> + *\n> + * Create and open the V4L2 devices and subdevices of the ImgU instance\n> + * with \\a index.\n> + *\n> + * In case of errors the created V4L2Device and V4L2Subdevice instances\n> + * are destroyed at pipeline handler delete time.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int ImgUDevice::init(MediaDevice *media, unsigned int index)\n> +{\n> +\tint ret;\n> +\n> +\tindex_ = index;\n> +\tname_ = \"ipu3-imgu \" + std::to_string(index_);\n> +\tmedia_ = media;\n> +\n> +\timgu_ = V4L2Subdevice::fromEntityName(media, name_);\n> +\tret = imgu_->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tinput_ = V4L2Device::fromEntityName(media, name_ + \" input\");\n> +\tret = input_->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\toutput_ = V4L2Device::fromEntityName(media, name_ + \" output\");\n> +\tret = output_->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tviewfinder_ = V4L2Device::fromEntityName(media, name_ + \" viewfinder\");\n> +\tret = viewfinder_->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tstat_ = V4L2Device::fromEntityName(media, name_ + \" 3a stat\");\n> +\tret = stat_->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\treturn 0;\n> +}\n> +\n> /*------------------------------------------------------------------------------\n> * CIO2 Device\n> */\n> -- \n> 2.21.0\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D4C6E60DB3\n\tfor ;\n\tTue, 2 Apr 2019 13:28:49 +0200 (CEST)","by mail-lj1-x241.google.com with SMTP id q66so11222573ljq.7\n\tfor ;\n\tTue, 02 Apr 2019 04:28:49 -0700 (PDT)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tu17sm2629602lfu.63.2019.04.02.04.28.48\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tTue, 02 Apr 2019 04:28:48 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=iacJffsB5on8/sC9EqwMtQxfyB6s0DGfazPLyfHVq0g=;\n\tb=XuzBmdnX5JS2HG3YvP+aPZiTIqgPxYZ67cba/ZaNeNZHDuJdz2YXKE4L7Noyic2357\n\tocFzBIYAs0jL12IymHHEogKAYPsDCdnVOc7R/NwtGz7I+123T9NFlqgiDyg6LubfUAbD\n\tl8W+xfOGuFdRamduMpcaDPBj0GlYqRomfQnRQfCfj1bGsFDQTmUDjqqXPUUl+I42SbTe\n\tkD64xPfHeRihU0Y1uhRoVPijFgymAwgKFY6yNr1jTmpnGjMKa7EgWz5ZeFKeMYNGVMOQ\n\tPgUKqrNP/4HgyKFyIgYMOo2u53IR3KOA1xpRYnlr/KdTtZZ/1RaUnJKCikuN8yQ+Bw8r\n\tpCPQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=iacJffsB5on8/sC9EqwMtQxfyB6s0DGfazPLyfHVq0g=;\n\tb=rxuNDAybB3Os0R1VlM8HuF9aJug6PFbLvb6skD6nHzdnDRAsUAaERtEzQY7GV5xOc5\n\tVCh/c5skMnkbibInLRlqFPdlBpOm+R15fDyVS1kAmstB15xi/UFH+DGh4TomvIskKSYf\n\t6xsz5rJhTwPTGX4maeeV/LsaWKtg1e0auVOVuaFYbE5Ivx/nQWBsx+l8s7wT2FcBox0y\n\txfx54F1O5jQFONc1PQXvMJpEyK+/dtlAXaKCXNtdObiAxNQ9ONFw8Adl34Mj58w1QpQk\n\tNR/sustzFhxu3ws0/nLzsC64xDXf2FRQQEDIU0+W6BcgbiQHOMz7AgW5Ccbfj8veJl/D\n\tkqaw==","X-Gm-Message-State":"APjAAAWSo+rq14lojxTPIefQOj6mXVzhum1yxjS6nW9oiLET9rY2CK1i\n\ta/LtKxAdIiW+skdu9EZU24pbclWobq4=","X-Google-Smtp-Source":"APXvYqzTStFcnT0mlUzjhT1y+nNdMSVtrSlFhRAS0iV1bfW5gSw7KiG4n9Br+M3/gGyrjOcSX4Yv5w==","X-Received":"by 2002:a2e:8991:: with SMTP id\n\tc17mr20755785lji.83.1554204529244; \n\tTue, 02 Apr 2019 04:28:49 -0700 (PDT)","Date":"Tue, 2 Apr 2019 13:28:48 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190402112848.GN23466@bigcity.dyn.berto.se>","References":"<20190326083902.26121-1-jacopo@jmondi.org>\n\t<20190326083902.26121-12-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190326083902.26121-12-jacopo@jmondi.org>","User-Agent":"Mutt/1.11.3 (2019-02-01)","Subject":"Re: [libcamera-devel] [PATCH v5 11/19] libcamera: ipu3: Create\n\tImgUDevice class","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, 02 Apr 2019 11:28:50 -0000"}}]