[{"id":1004,"web_url":"https://patchwork.libcamera.org/comment/1004/","msgid":"<20190302210444.GF4682@pendragon.ideasonboard.com>","date":"2019-03-02T21:04:44","subject":"Re: [libcamera-devel] [PATCH 01/10] libcamera: ipu3: Group CIO2\n\tdevices","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 Thu, Feb 28, 2019 at 09:04:01PM +0100, Jacopo Mondi wrote:\n> Group CIO2 devices (cio2, csi2 and image sensor) in a structure\n> associated with the CameraData, to ease management of the CIO2 devices.\n> \n> Update the IPU3 pipeline handler implementation to avoid name clashes\n> and break-out IPU3CameraData from the pipeline handler class for\n> clarity.\n\nI'm not sure that really brings clarity :-) I would have kept the\nIPU3CameraData structure where it was, but preferences differ of course.\n\n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  src/libcamera/pipeline/ipu3/ipu3.cpp | 251 ++++++++++++++-------------\n>  1 file changed, 133 insertions(+), 118 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 9694d0ce51ab..d3f1d9a95f81 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -24,6 +24,30 @@ namespace libcamera {\n>  \n>  LOG_DEFINE_CATEGORY(IPU3)\n>  \n> +struct Cio2Device {\n\nShould this be called CIO2Device ?\n\n> +\tCio2Device()\n> +\t\t: output(nullptr), csi2(nullptr), sensor(nullptr) {}\n\nI'd open the {} on the next two lines for consistency, up to you.\n\n> +\n> +\t~Cio2Device()\n> +\t{\n> +\t\tdelete output;\n> +\t\tdelete csi2;\n> +\t\tdelete sensor;\n> +\t}\n\nI'm not very fond on having part of the logic in the PipelineHandlerIPU3\nclass (the initCio2() function - I know I asked for it to be moved\nthere) part here in the destructor :-/ Splitting it in two doesn't feel\nreally right, I'd move these three delete statements to the\nIPU3CameraData destructor. What do you think ?\n\n> +\n> +\tV4L2Device *output;\n> +\tV4L2Subdevice *csi2;\n> +\tV4L2Subdevice *sensor;\n> +};\n> +\n> +class IPU3CameraData : public CameraData\n> +{\n> +public:\n> +\tCio2Device cio2;\n> +\n> +\tStream stream_;\n> +};\n> +\n>  class PipelineHandlerIPU3 : public PipelineHandler\n>  {\n>  public:\n> @@ -47,65 +71,47 @@ public:\n>  \tbool match(DeviceEnumerator *enumerator);\n>  \n>  private:\n> -\tclass IPU3CameraData : public CameraData\n> -\t{\n> -\tpublic:\n> -\t\tIPU3CameraData()\n> -\t\t\t: cio2_(nullptr), csi2_(nullptr), sensor_(nullptr) {}\n> -\n> -\t\t~IPU3CameraData()\n> -\t\t{\n> -\t\t\tdelete cio2_;\n> -\t\t\tdelete csi2_;\n> -\t\t\tdelete sensor_;\n> -\t\t}\n> -\n> -\t\tV4L2Device *cio2_;\n> -\t\tV4L2Subdevice *csi2_;\n> -\t\tV4L2Subdevice *sensor_;\n> -\n> -\t\tStream stream_;\n> -\t};\n> -\n>  \tIPU3CameraData *cameraData(const Camera *camera)\n>  \t{\n>  \t\treturn static_cast<IPU3CameraData *>(\n>  \t\t\tPipelineHandler::cameraData(camera));\n>  \t}\n>  \n> +\tint initCio2(unsigned int index, Cio2Device *cio2);\n\ninitCIO2() ?\n\n>  \tvoid registerCameras();\n>  \n> -\tstd::shared_ptr<MediaDevice> cio2_;\n> -\tstd::shared_ptr<MediaDevice> imgu_;\n> +\tstd::shared_ptr<MediaDevice> cio2MediaDev_;\n> +\tstd::shared_ptr<MediaDevice> imguMediaDev_;\n\nAre these renames really needed ?\n\n>  };\n>  \n>  PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager)\n> -\t: PipelineHandler(manager), cio2_(nullptr), imgu_(nullptr)\n> +\t: PipelineHandler(manager), cio2MediaDev_(nullptr), imguMediaDev_(nullptr)\n>  {\n>  }\n>  \n>  PipelineHandlerIPU3::~PipelineHandlerIPU3()\n>  {\n> -\tif (cio2_)\n> -\t\tcio2_->release();\n> +\tif (cio2MediaDev_)\n> +\t\tcio2MediaDev_->release();\n>  \n> -\tif (imgu_)\n> -\t\timgu_->release();\n> +\tif (imguMediaDev_)\n> +\t\timguMediaDev_->release();\n>  }\n>  \n>  std::map<Stream *, StreamConfiguration>\n>  PipelineHandlerIPU3::streamConfiguration(Camera *camera,\n>  \t\t\t\t\t std::vector<Stream *> &streams)\n>  {\n> -\tIPU3CameraData *data = cameraData(camera);\n>  \tstd::map<Stream *, StreamConfiguration> configs;\n> +\tIPU3CameraData *data = cameraData(camera);\n> +\tV4L2Subdevice *sensor = data->cio2.sensor;\n>  \tV4L2SubdeviceFormat format = {};\n>  \n>  \t/*\n>  \t * FIXME: As of now, return the image format reported by the sensor.\n>  \t * In future good defaults should be provided for each stream.\n>  \t */\n> -\tif (data->sensor_->getFormat(0, &format)) {\n> +\tif (sensor->getFormat(0, &format)) {\n>  \t\tLOG(IPU3, Error) << \"Failed to create stream configurations\";\n>  \t\treturn configs;\n>  \t}\n> @@ -126,9 +132,9 @@ int PipelineHandlerIPU3::configureStreams(Camera *camera,\n>  {\n>  \tIPU3CameraData *data = cameraData(camera);\n>  \tStreamConfiguration *cfg = &config[&data->stream_];\n> -\tV4L2Subdevice *sensor = data->sensor_;\n> -\tV4L2Subdevice *csi2 = data->csi2_;\n> -\tV4L2Device *cio2 = data->cio2_;\n> +\tV4L2Subdevice *sensor = data->cio2.sensor;\n> +\tV4L2Subdevice *csi2 = data->cio2.csi2;\n> +\tV4L2Device *cio2 = data->cio2.output;\n>  \tV4L2SubdeviceFormat subdevFormat = {};\n>  \tV4L2DeviceFormat devFormat = {};\n>  \tint ret;\n> @@ -185,13 +191,14 @@ int PipelineHandlerIPU3::configureStreams(Camera *camera,\n>  \n>  int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)\n>  {\n> -\tIPU3CameraData *data = cameraData(camera);\n>  \tconst StreamConfiguration &cfg = stream->configuration();\n> +\tIPU3CameraData *data = cameraData(camera);\n> +\tV4L2Device *cio2 = data->cio2.output;\n>  \n>  \tif (!cfg.bufferCount)\n>  \t\treturn -EINVAL;\n>  \n> -\tint ret = data->cio2_->exportBuffers(&stream->bufferPool());\n> +\tint ret = cio2->exportBuffers(&stream->bufferPool());\n>  \tif (ret) {\n>  \t\tLOG(IPU3, Error) << \"Failed to request memory\";\n>  \t\treturn ret;\n> @@ -203,8 +210,9 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)\n>  int PipelineHandlerIPU3::freeBuffers(Camera *camera, Stream *stream)\n>  {\n>  \tIPU3CameraData *data = cameraData(camera);\n> +\tV4L2Device *cio2 = data->cio2.output;\n>  \n> -\tint ret = data->cio2_->releaseBuffers();\n> +\tint ret = cio2->releaseBuffers();\n>  \tif (ret) {\n>  \t\tLOG(IPU3, Error) << \"Failed to release memory\";\n>  \t\treturn ret;\n> @@ -216,9 +224,10 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera, Stream *stream)\n>  int PipelineHandlerIPU3::start(const Camera *camera)\n>  {\n>  \tIPU3CameraData *data = cameraData(camera);\n> +\tV4L2Device *cio2 = data->cio2.output;\n>  \tint ret;\n>  \n> -\tret = data->cio2_->streamOn();\n> +\tret = cio2->streamOn();\n>  \tif (ret) {\n>  \t\tLOG(IPU3, Info) << \"Failed to start camera \" << camera->name();\n>  \t\treturn ret;\n> @@ -230,14 +239,16 @@ int PipelineHandlerIPU3::start(const Camera *camera)\n>  void PipelineHandlerIPU3::stop(const Camera *camera)\n>  {\n>  \tIPU3CameraData *data = cameraData(camera);\n> +\tV4L2Device *cio2 = data->cio2.output;\n>  \n> -\tif (data->cio2_->streamOff())\n> +\tif (cio2->streamOff())\n>  \t\tLOG(IPU3, Info) << \"Failed to stop camera \" << camera->name();\n>  }\n>  \n>  int PipelineHandlerIPU3::queueRequest(const Camera *camera, Request *request)\n>  {\n>  \tIPU3CameraData *data = cameraData(camera);\n> +\tV4L2Device *cio2 = data->cio2.output;\n>  \tStream *stream = &data->stream_;\n>  \n>  \tBuffer *buffer = request->findBuffer(stream);\n> @@ -247,7 +258,7 @@ int PipelineHandlerIPU3::queueRequest(const Camera *camera, Request *request)\n>  \t\treturn -ENOENT;\n>  \t}\n>  \n> -\tdata->cio2_->queueBuffer(buffer);\n> +\tcio2->queueBuffer(buffer);\n>  \n>  \treturn 0;\n>  }\n> @@ -278,20 +289,20 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n>  \timgu_dm.add(\"ipu3-imgu 1 viewfinder\");\n>  \timgu_dm.add(\"ipu3-imgu 1 3a stat\");\n>  \n> -\tcio2_ = enumerator->search(cio2_dm);\n> -\tif (!cio2_)\n> +\tcio2MediaDev_ = enumerator->search(cio2_dm);\n> +\tif (!cio2MediaDev_)\n>  \t\treturn false;\n>  \n> -\timgu_ = enumerator->search(imgu_dm);\n> -\tif (!imgu_)\n> +\timguMediaDev_ = enumerator->search(imgu_dm);\n> +\tif (!imguMediaDev_)\n>  \t\treturn false;\n>  \n>  \t/*\n>  \t * It is safe to acquire both media devices at this point as\n>  \t * DeviceEnumerator::search() skips the busy ones for us.\n>  \t */\n> -\tcio2_->acquire();\n> -\timgu_->acquire();\n> +\tcio2MediaDev_->acquire();\n> +\timguMediaDev_->acquire();\n>  \n>  \t/*\n>  \t * Disable all links that are enabled by default on CIO2, as camera\n> @@ -300,28 +311,90 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n>  \t * Close the CIO2 media device after, as links are enabled and should\n>  \t * not need to be changed after.\n>  \t */\n> -\tif (cio2_->open())\n> +\tif (cio2MediaDev_->open())\n>  \t\tgoto error_release_mdev;\n>  \n> -\tif (cio2_->disableLinks())\n> +\tif (cio2MediaDev_->disableLinks())\n>  \t\tgoto error_close_cio2;\n>  \n>  \tregisterCameras();\n>  \n> -\tcio2_->close();\n> +\tcio2MediaDev_->close();\n>  \n>  \treturn true;\n>  \n>  error_close_cio2:\n> -\tcio2_->close();\n> +\tcio2MediaDev_->close();\n>  \n>  error_release_mdev:\n> -\tcio2_->release();\n> -\timgu_->release();\n> +\tcio2MediaDev_->release();\n> +\timguMediaDev_->release();\n>  \n>  \treturn false;\n>  }\n>  \n\nThis pipeline handler will likely be used as a reference implementation\nin the future, could you add more documentation, even for private\nfunctions ?\n\n> +int PipelineHandlerIPU3::initCio2(unsigned int index, Cio2Device *cio2)\n> +{\n> +\tint ret;\n> +\n> +\t/* Verify a sensor subdevice is connected to this CIO2 instance. */\n> +\tstd::string csi2Name = \"ipu3-csi2 \" + std::to_string(index);\n> +\tMediaEntity *entity = cio2MediaDev_->getEntityByName(csi2Name);\n> +\tif (!entity) {\n> +\t\tLOG(IPU3, Error)\n> +\t\t\t<< \"Failed to get entity '\" << csi2Name << \"'\";\n> +\t\treturn -ENODEV;\n> +\t}\n> +\n> +\tconst std::vector<MediaPad *> &pads = entity->pads();\n> +\tif (pads.empty())\n> +\t\treturn -EINVAL;\n> +\n> +\t/* IPU3 CSI-2 receivers have a single sink pad at index 0. */\n> +\tMediaPad *sink = pads[0];\n> +\tconst std::vector<MediaLink *> &links = sink->links();\n> +\tif (links.empty())\n> +\t\treturn -EINVAL;\n> +\n> +\tMediaLink *link = links[0];\n> +\tMediaEntity *sensorEntity = link->source()->entity();\n> +\tif (sensorEntity->function() != MEDIA_ENT_F_CAM_SENSOR)\n> +\t\treturn -ENODEV;\n> +\n> +\tret = link->setEnabled(true);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\t/*\n> +\t * Now that we're sure a sensor subdevice is connected, create and open\n> +\t * video devices and subdevices associated with this CIO2 unit.\n> +\t */\n> +\tcio2->sensor = new V4L2Subdevice(sensorEntity);\n> +\tret = cio2->sensor->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tcio2->csi2 = new V4L2Subdevice(entity);\n> +\tret = cio2->csi2->open();\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tstd::string cio2Name = \"ipu3-cio2 \" + std::to_string(index);\n> +\tentity = cio2MediaDev_->getEntityByName(cio2Name);\n> +\tif (!entity) {\n> +\t\tLOG(IPU3, Error)\n> +\t\t\t<< \"Failed to get entity '\" << cio2Name << \"'\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tcio2->output = new V4L2Device(entity);\n> +\tret = cio2->output->open();\n\nNote for later, we shouldn't keep device nodes open when they're not\nneeded. Depending on the kernel drivers, it may consume more power.\nCould you add a \\todo ?\n\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\treturn 0;\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> @@ -329,85 +402,27 @@ error_release_mdev:\n>   */\n>  void PipelineHandlerIPU3::registerCameras()\n>  {\n> +\tint 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.\n>  \t */\n>  \tunsigned int numCameras = 0;\n>  \tfor (unsigned int id = 0; id < 4; ++id) {\n> -\t\tstd::string csi2Name = \"ipu3-csi2 \" + std::to_string(id);\n> -\t\tMediaEntity *csi2 = cio2_->getEntityByName(csi2Name);\n> -\t\tint ret;\n\nYou could keep ret within the loop.\n\n> -\n> -\t\t/*\n> -\t\t * This shall not happen, as the device enumerator matched\n> -\t\t * all entities described in the cio2_dm DeviceMatch.\n> -\t\t *\n> -\t\t * As this check is basically free, better stay safe than sorry.\n> -\t\t */\n> -\t\tif (!csi2)\n> -\t\t\tcontinue;\n> -\n> -\t\tconst std::vector<MediaPad *> &pads = csi2->pads();\n> -\t\tif (pads.empty())\n> -\t\t\tcontinue;\n> -\n> -\t\t/* IPU3 CSI-2 receivers have a single sink pad at index 0. */\n> -\t\tMediaPad *sink = pads[0];\n> -\t\tconst std::vector<MediaLink *> &links = sink->links();\n> -\t\tif (links.empty())\n> -\t\t\tcontinue;\n> -\n> -\t\t/*\n> -\t\t * Verify that the receiver is connected to a sensor, enable\n> -\t\t * the media link between the two, and create a Camera with\n> -\t\t * a unique name.\n> -\t\t */\n> -\t\tMediaLink *link = links[0];\n> -\t\tMediaEntity *sensor = link->source()->entity();\n> -\t\tif (sensor->function() != MEDIA_ENT_F_CAM_SENSOR)\n> -\t\t\tcontinue;\n> -\n> -\t\tif (link->setEnabled(true))\n> -\t\t\tcontinue;\n> -\n> -\t\tstd::unique_ptr<IPU3CameraData> data = utils::make_unique<IPU3CameraData>();\n> -\n> -\t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> +\t\tstd::unique_ptr<IPU3CameraData> data =\n> +\t\t\tutils::make_unique<IPU3CameraData>();\n>  \t\tstd::vector<Stream *> streams{ &data->stream_ };\n> -\t\tstd::shared_ptr<Camera> camera = Camera::create(this, cameraName, streams);\n> -\n> -\t\t/*\n> -\t\t * Create and open video devices and subdevices associated with\n> -\t\t * the camera.\n> -\t\t *\n> -\t\t * If any of these operations fails, the Camera instance won't\n> -\t\t * be registered. The 'camera' shared pointer and the 'data'\n> -\t\t * unique pointers go out of scope and delete the objects they\n> -\t\t * manage.\n> -\t\t */\n> -\t\tstd::string cio2Name = \"ipu3-cio2 \" + std::to_string(id);\n> -\t\tMediaEntity *cio2 = cio2_->getEntityByName(cio2Name);\n> -\t\tif (!cio2) {\n> -\t\t\tLOG(IPU3, Error)\n> -\t\t\t\t<< \"Failed to get entity '\" << cio2Name << \"'\";\n> -\t\t\tcontinue;\n> -\t\t}\n> -\n> -\t\tdata->cio2_ = new V4L2Device(cio2);\n> -\t\tret = data->cio2_->open();\n> -\t\tif (ret)\n> -\t\t\tcontinue;\n> +\t\tCio2Device *cio2 = &data->cio2;\n>  \n\nNo need for a blank line here.\n\n> -\t\tdata->sensor_ = new V4L2Subdevice(sensor);\n> -\t\tret = data->sensor_->open();\n> +\t\tret = initCio2(id, cio2);\n>  \t\tif (ret)\n>  \t\t\tcontinue;\n>  \n> -\t\tdata->csi2_ = new V4L2Subdevice(csi2);\n> -\t\tret = data->csi2_->open();\n> -\t\tif (ret)\n> -\t\t\tcontinue;\n> +\t\tstd::string cameraName = cio2->sensor->deviceName() + \" \"\n> +\t\t\t\t       + std::to_string(id);\n> +\t\tstd::shared_ptr<Camera> camera =\n> +\t\t\tCamera::create(this, cameraName, streams);\n>  \n>  \t\tsetCameraData(camera.get(), std::move(data));\n>  \t\tregisterCamera(std::move(camera));","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","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 62FB3610BF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat,  2 Mar 2019 22:04:51 +0100 (CET)","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 A400254E;\n\tSat,  2 Mar 2019 22:04:50 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1551560690;\n\tbh=4u7mqeZ1RUgRMKibQ72tnqGgL764n7uS6XTugMa7jk4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dOwpstDlGvKXu9qwsCpOnw4QF9QQ+Mow1hz9YQ9d1GS2+ILEBOkY5sDta9X00iyV7\n\t5fKDxOqDM9t4YE8FPTPllfIdwslQEdatSE9lFQL27hMVqH9Wk4DfZs/U6o5h6RjCH4\n\tkHosqsrLSsinnW5BTUuSQfHW5En9lPDmkGdGzMDI=","Date":"Sat, 2 Mar 2019 23:04:44 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190302210444.GF4682@pendragon.ideasonboard.com>","References":"<20190228200410.3022-1-jacopo@jmondi.org>\n\t<20190228200410.3022-2-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190228200410.3022-2-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 01/10] libcamera: ipu3: Group CIO2\n\tdevices","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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":"Sat, 02 Mar 2019 21:04:51 -0000"}},{"id":1019,"web_url":"https://patchwork.libcamera.org/comment/1019/","msgid":"<20190309203448.qyldtwse7bzlsqe2@uno.localdomain>","date":"2019-03-09T20:34:48","subject":"Re: [libcamera-devel] [PATCH 01/10] libcamera: ipu3: Group CIO2\n\tdevices","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Sat, Mar 02, 2019 at 11:04:44PM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Thu, Feb 28, 2019 at 09:04:01PM +0100, Jacopo Mondi wrote:\n> > Group CIO2 devices (cio2, csi2 and image sensor) in a structure\n> > associated with the CameraData, to ease management of the CIO2 devices.\n> >\n> > Update the IPU3 pipeline handler implementation to avoid name clashes\n> > and break-out IPU3CameraData from the pipeline handler class for\n> > clarity.\n>\n> I'm not sure that really brings clarity :-) I would have kept the\n> IPU3CameraData structure where it was, but preferences differ of course.\n>\n\nThe IPU3CameraData definition is not exported anyhow, so it would not\nmake much difference if it is declared inside the IPU3PipelineHandler\nclass. If you're not strongly against this, I would break it out.\n\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> > ---\n> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 251 ++++++++++++++-------------\n> >  1 file changed, 133 insertions(+), 118 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 9694d0ce51ab..d3f1d9a95f81 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -24,6 +24,30 @@ namespace libcamera {\n> >\n> >  LOG_DEFINE_CATEGORY(IPU3)\n> >\n> > +struct Cio2Device {\n>\n> Should this be called CIO2Device ?\n>\n\nack\n\n> > +\tCio2Device()\n> > +\t\t: output(nullptr), csi2(nullptr), sensor(nullptr) {}\n>\n> I'd open the {} on the next two lines for consistency, up to you.\n>\n\nack\n\n> > +\n> > +\t~Cio2Device()\n> > +\t{\n> > +\t\tdelete output;\n> > +\t\tdelete csi2;\n> > +\t\tdelete sensor;\n> > +\t}\n>\n> I'm not very fond on having part of the logic in the PipelineHandlerIPU3\n> class (the initCio2() function - I know I asked for it to be moved\n> there) part here in the destructor :-/ Splitting it in two doesn't feel\n> really right, I'd move these three delete statements to the\n> IPU3CameraData destructor. What do you think ?\n\nRe-looking at the series, I'm still in favor of aggregate all the\nCIO2-related components (csi2, sensor and output) in a structure, as\nan instance will always be associated with a Camera, and grouping them\ncompacts the code a bit.\n\nI understand your concerns, but the CIO2 intialization routine would\nnot just initialize components, but would actually make sure if a\nsensor is connected to the CIO2 instance, and it is used to decide if\na camera has to be created or not. True we could make a class out the\nCIO2Device structure and add an init() method, whose result would be\nused to decide if to procede with creating the camera or not, but you\nwhere against this, and I would tend to agree as then would be\ntempting to put more and more methods in that class.\n\nTo sum up, I agree with your first comment to not make a class out of\nthis, that compared to the ImgUDevice one is trivally associated with\na camera, and its components (sensor in particular) have to be\ninteracted with by the IPU3PipelineHandler, but at the same time, I\nthink it is nice to have a CIO2Device in the per-Camera\nIPU3CameraData, and centralize the component destruction in a single\nplace.\n\nImgUDevice is different, as we will assign them dynamically to\nCameras, so they can't be destroyed when the CameraData is destroyed,\nbut when the pipeline handler is.\n\nAck for keeping this the way it is?\n\n>\n> > +\n> > +\tV4L2Device *output;\n> > +\tV4L2Subdevice *csi2;\n> > +\tV4L2Subdevice *sensor;\n> > +};\n> > +\n> > +class IPU3CameraData : public CameraData\n> > +{\n> > +public:\n> > +\tCio2Device cio2;\n> > +\n> > +\tStream stream_;\n> > +};\n> > +\n> >  class PipelineHandlerIPU3 : public PipelineHandler\n> >  {\n> >  public:\n> > @@ -47,65 +71,47 @@ public:\n> >  \tbool match(DeviceEnumerator *enumerator);\n> >\n> >  private:\n> > -\tclass IPU3CameraData : public CameraData\n> > -\t{\n> > -\tpublic:\n> > -\t\tIPU3CameraData()\n> > -\t\t\t: cio2_(nullptr), csi2_(nullptr), sensor_(nullptr) {}\n> > -\n> > -\t\t~IPU3CameraData()\n> > -\t\t{\n> > -\t\t\tdelete cio2_;\n> > -\t\t\tdelete csi2_;\n> > -\t\t\tdelete sensor_;\n> > -\t\t}\n> > -\n> > -\t\tV4L2Device *cio2_;\n> > -\t\tV4L2Subdevice *csi2_;\n> > -\t\tV4L2Subdevice *sensor_;\n> > -\n> > -\t\tStream stream_;\n> > -\t};\n> > -\n> >  \tIPU3CameraData *cameraData(const Camera *camera)\n> >  \t{\n> >  \t\treturn static_cast<IPU3CameraData *>(\n> >  \t\t\tPipelineHandler::cameraData(camera));\n> >  \t}\n> >\n> > +\tint initCio2(unsigned int index, Cio2Device *cio2);\n>\n> initCIO2() ?\n>\n> >  \tvoid registerCameras();\n> >\n> > -\tstd::shared_ptr<MediaDevice> cio2_;\n> > -\tstd::shared_ptr<MediaDevice> imgu_;\n> > +\tstd::shared_ptr<MediaDevice> cio2MediaDev_;\n> > +\tstd::shared_ptr<MediaDevice> imguMediaDev_;\n>\n> Are these renames really needed ?\n\nI have way too many CIO2s and ImgUs names around, clarify which one is\nthe media device instance is helpfull. Got suggestions for better\nnames?\n\n>\n> >  };\n> >\n> >  PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager)\n> > -\t: PipelineHandler(manager), cio2_(nullptr), imgu_(nullptr)\n> > +\t: PipelineHandler(manager), cio2MediaDev_(nullptr), imguMediaDev_(nullptr)\n> >  {\n> >  }\n> >\n> >  PipelineHandlerIPU3::~PipelineHandlerIPU3()\n> >  {\n> > -\tif (cio2_)\n> > -\t\tcio2_->release();\n> > +\tif (cio2MediaDev_)\n> > +\t\tcio2MediaDev_->release();\n> >\n> > -\tif (imgu_)\n> > -\t\timgu_->release();\n> > +\tif (imguMediaDev_)\n> > +\t\timguMediaDev_->release();\n> >  }\n> >\n> >  std::map<Stream *, StreamConfiguration>\n> >  PipelineHandlerIPU3::streamConfiguration(Camera *camera,\n> >  \t\t\t\t\t std::vector<Stream *> &streams)\n> >  {\n> > -\tIPU3CameraData *data = cameraData(camera);\n> >  \tstd::map<Stream *, StreamConfiguration> configs;\n> > +\tIPU3CameraData *data = cameraData(camera);\n> > +\tV4L2Subdevice *sensor = data->cio2.sensor;\n> >  \tV4L2SubdeviceFormat format = {};\n> >\n> >  \t/*\n> >  \t * FIXME: As of now, return the image format reported by the sensor.\n> >  \t * In future good defaults should be provided for each stream.\n> >  \t */\n> > -\tif (data->sensor_->getFormat(0, &format)) {\n> > +\tif (sensor->getFormat(0, &format)) {\n> >  \t\tLOG(IPU3, Error) << \"Failed to create stream configurations\";\n> >  \t\treturn configs;\n> >  \t}\n> > @@ -126,9 +132,9 @@ int PipelineHandlerIPU3::configureStreams(Camera *camera,\n> >  {\n> >  \tIPU3CameraData *data = cameraData(camera);\n> >  \tStreamConfiguration *cfg = &config[&data->stream_];\n> > -\tV4L2Subdevice *sensor = data->sensor_;\n> > -\tV4L2Subdevice *csi2 = data->csi2_;\n> > -\tV4L2Device *cio2 = data->cio2_;\n> > +\tV4L2Subdevice *sensor = data->cio2.sensor;\n> > +\tV4L2Subdevice *csi2 = data->cio2.csi2;\n> > +\tV4L2Device *cio2 = data->cio2.output;\n> >  \tV4L2SubdeviceFormat subdevFormat = {};\n> >  \tV4L2DeviceFormat devFormat = {};\n> >  \tint ret;\n> > @@ -185,13 +191,14 @@ int PipelineHandlerIPU3::configureStreams(Camera *camera,\n> >\n> >  int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)\n> >  {\n> > -\tIPU3CameraData *data = cameraData(camera);\n> >  \tconst StreamConfiguration &cfg = stream->configuration();\n> > +\tIPU3CameraData *data = cameraData(camera);\n> > +\tV4L2Device *cio2 = data->cio2.output;\n> >\n> >  \tif (!cfg.bufferCount)\n> >  \t\treturn -EINVAL;\n> >\n> > -\tint ret = data->cio2_->exportBuffers(&stream->bufferPool());\n> > +\tint ret = cio2->exportBuffers(&stream->bufferPool());\n> >  \tif (ret) {\n> >  \t\tLOG(IPU3, Error) << \"Failed to request memory\";\n> >  \t\treturn ret;\n> > @@ -203,8 +210,9 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)\n> >  int PipelineHandlerIPU3::freeBuffers(Camera *camera, Stream *stream)\n> >  {\n> >  \tIPU3CameraData *data = cameraData(camera);\n> > +\tV4L2Device *cio2 = data->cio2.output;\n> >\n> > -\tint ret = data->cio2_->releaseBuffers();\n> > +\tint ret = cio2->releaseBuffers();\n> >  \tif (ret) {\n> >  \t\tLOG(IPU3, Error) << \"Failed to release memory\";\n> >  \t\treturn ret;\n> > @@ -216,9 +224,10 @@ int PipelineHandlerIPU3::freeBuffers(Camera *camera, Stream *stream)\n> >  int PipelineHandlerIPU3::start(const Camera *camera)\n> >  {\n> >  \tIPU3CameraData *data = cameraData(camera);\n> > +\tV4L2Device *cio2 = data->cio2.output;\n> >  \tint ret;\n> >\n> > -\tret = data->cio2_->streamOn();\n> > +\tret = cio2->streamOn();\n> >  \tif (ret) {\n> >  \t\tLOG(IPU3, Info) << \"Failed to start camera \" << camera->name();\n> >  \t\treturn ret;\n> > @@ -230,14 +239,16 @@ int PipelineHandlerIPU3::start(const Camera *camera)\n> >  void PipelineHandlerIPU3::stop(const Camera *camera)\n> >  {\n> >  \tIPU3CameraData *data = cameraData(camera);\n> > +\tV4L2Device *cio2 = data->cio2.output;\n> >\n> > -\tif (data->cio2_->streamOff())\n> > +\tif (cio2->streamOff())\n> >  \t\tLOG(IPU3, Info) << \"Failed to stop camera \" << camera->name();\n> >  }\n> >\n> >  int PipelineHandlerIPU3::queueRequest(const Camera *camera, Request *request)\n> >  {\n> >  \tIPU3CameraData *data = cameraData(camera);\n> > +\tV4L2Device *cio2 = data->cio2.output;\n> >  \tStream *stream = &data->stream_;\n> >\n> >  \tBuffer *buffer = request->findBuffer(stream);\n> > @@ -247,7 +258,7 @@ int PipelineHandlerIPU3::queueRequest(const Camera *camera, Request *request)\n> >  \t\treturn -ENOENT;\n> >  \t}\n> >\n> > -\tdata->cio2_->queueBuffer(buffer);\n> > +\tcio2->queueBuffer(buffer);\n> >\n> >  \treturn 0;\n> >  }\n> > @@ -278,20 +289,20 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> >  \timgu_dm.add(\"ipu3-imgu 1 viewfinder\");\n> >  \timgu_dm.add(\"ipu3-imgu 1 3a stat\");\n> >\n> > -\tcio2_ = enumerator->search(cio2_dm);\n> > -\tif (!cio2_)\n> > +\tcio2MediaDev_ = enumerator->search(cio2_dm);\n> > +\tif (!cio2MediaDev_)\n> >  \t\treturn false;\n> >\n> > -\timgu_ = enumerator->search(imgu_dm);\n> > -\tif (!imgu_)\n> > +\timguMediaDev_ = enumerator->search(imgu_dm);\n> > +\tif (!imguMediaDev_)\n> >  \t\treturn false;\n> >\n> >  \t/*\n> >  \t * It is safe to acquire both media devices at this point as\n> >  \t * DeviceEnumerator::search() skips the busy ones for us.\n> >  \t */\n> > -\tcio2_->acquire();\n> > -\timgu_->acquire();\n> > +\tcio2MediaDev_->acquire();\n> > +\timguMediaDev_->acquire();\n> >\n> >  \t/*\n> >  \t * Disable all links that are enabled by default on CIO2, as camera\n> > @@ -300,28 +311,90 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)\n> >  \t * Close the CIO2 media device after, as links are enabled and should\n> >  \t * not need to be changed after.\n> >  \t */\n> > -\tif (cio2_->open())\n> > +\tif (cio2MediaDev_->open())\n> >  \t\tgoto error_release_mdev;\n> >\n> > -\tif (cio2_->disableLinks())\n> > +\tif (cio2MediaDev_->disableLinks())\n> >  \t\tgoto error_close_cio2;\n> >\n> >  \tregisterCameras();\n> >\n> > -\tcio2_->close();\n> > +\tcio2MediaDev_->close();\n> >\n> >  \treturn true;\n> >\n> >  error_close_cio2:\n> > -\tcio2_->close();\n> > +\tcio2MediaDev_->close();\n> >\n> >  error_release_mdev:\n> > -\tcio2_->release();\n> > -\timgu_->release();\n> > +\tcio2MediaDev_->release();\n> > +\timguMediaDev_->release();\n> >\n> >  \treturn false;\n> >  }\n> >\n>\n> This pipeline handler will likely be used as a reference implementation\n> in the future, could you add more documentation, even for private\n> functions ?\n>\n\nYes, will use doxygen style (minor thing: doxygen won't produce\ndocumentation for private memebers but should I use the /** comment\nblock starter anyway for consistency ? )\n\n> > +int PipelineHandlerIPU3::initCio2(unsigned int index, Cio2Device *cio2)\n> > +{\n> > +\tint ret;\n> > +\n> > +\t/* Verify a sensor subdevice is connected to this CIO2 instance. */\n> > +\tstd::string csi2Name = \"ipu3-csi2 \" + std::to_string(index);\n> > +\tMediaEntity *entity = cio2MediaDev_->getEntityByName(csi2Name);\n> > +\tif (!entity) {\n> > +\t\tLOG(IPU3, Error)\n> > +\t\t\t<< \"Failed to get entity '\" << csi2Name << \"'\";\n> > +\t\treturn -ENODEV;\n> > +\t}\n> > +\n> > +\tconst std::vector<MediaPad *> &pads = entity->pads();\n> > +\tif (pads.empty())\n> > +\t\treturn -EINVAL;\n> > +\n> > +\t/* IPU3 CSI-2 receivers have a single sink pad at index 0. */\n> > +\tMediaPad *sink = pads[0];\n> > +\tconst std::vector<MediaLink *> &links = sink->links();\n> > +\tif (links.empty())\n> > +\t\treturn -EINVAL;\n> > +\n> > +\tMediaLink *link = links[0];\n> > +\tMediaEntity *sensorEntity = link->source()->entity();\n> > +\tif (sensorEntity->function() != MEDIA_ENT_F_CAM_SENSOR)\n> > +\t\treturn -ENODEV;\n> > +\n> > +\tret = link->setEnabled(true);\n> > +\tif (ret)\n> > +\t\treturn ret;\n> > +\n> > +\t/*\n> > +\t * Now that we're sure a sensor subdevice is connected, create and open\n> > +\t * video devices and subdevices associated with this CIO2 unit.\n> > +\t */\n> > +\tcio2->sensor = new V4L2Subdevice(sensorEntity);\n> > +\tret = cio2->sensor->open();\n> > +\tif (ret)\n> > +\t\treturn ret;\n> > +\n> > +\tcio2->csi2 = new V4L2Subdevice(entity);\n> > +\tret = cio2->csi2->open();\n> > +\tif (ret)\n> > +\t\treturn ret;\n> > +\n> > +\tstd::string cio2Name = \"ipu3-cio2 \" + std::to_string(index);\n> > +\tentity = cio2MediaDev_->getEntityByName(cio2Name);\n> > +\tif (!entity) {\n> > +\t\tLOG(IPU3, Error)\n> > +\t\t\t<< \"Failed to get entity '\" << cio2Name << \"'\";\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > +\tcio2->output = new V4L2Device(entity);\n> > +\tret = cio2->output->open();\n>\n> Note for later, we shouldn't keep device nodes open when they're not\n> needed. Depending on the kernel drivers, it may consume more power.\n> Could you add a \\todo ?\n>\n\nYes indeed.\n\n> > +\tif (ret)\n> > +\t\treturn ret;\n> > +\n> > +\treturn 0;\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> > @@ -329,85 +402,27 @@ error_release_mdev:\n> >   */\n> >  void PipelineHandlerIPU3::registerCameras()\n> >  {\n> > +\tint 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.\n> >  \t */\n> >  \tunsigned int numCameras = 0;\n> >  \tfor (unsigned int id = 0; id < 4; ++id) {\n> > -\t\tstd::string csi2Name = \"ipu3-csi2 \" + std::to_string(id);\n> > -\t\tMediaEntity *csi2 = cio2_->getEntityByName(csi2Name);\n> > -\t\tint ret;\n>\n> You could keep ret within the loop.\n>\n\nack\n\n> > -\n> > -\t\t/*\n> > -\t\t * This shall not happen, as the device enumerator matched\n> > -\t\t * all entities described in the cio2_dm DeviceMatch.\n> > -\t\t *\n> > -\t\t * As this check is basically free, better stay safe than sorry.\n> > -\t\t */\n> > -\t\tif (!csi2)\n> > -\t\t\tcontinue;\n> > -\n> > -\t\tconst std::vector<MediaPad *> &pads = csi2->pads();\n> > -\t\tif (pads.empty())\n> > -\t\t\tcontinue;\n> > -\n> > -\t\t/* IPU3 CSI-2 receivers have a single sink pad at index 0. */\n> > -\t\tMediaPad *sink = pads[0];\n> > -\t\tconst std::vector<MediaLink *> &links = sink->links();\n> > -\t\tif (links.empty())\n> > -\t\t\tcontinue;\n> > -\n> > -\t\t/*\n> > -\t\t * Verify that the receiver is connected to a sensor, enable\n> > -\t\t * the media link between the two, and create a Camera with\n> > -\t\t * a unique name.\n> > -\t\t */\n> > -\t\tMediaLink *link = links[0];\n> > -\t\tMediaEntity *sensor = link->source()->entity();\n> > -\t\tif (sensor->function() != MEDIA_ENT_F_CAM_SENSOR)\n> > -\t\t\tcontinue;\n> > -\n> > -\t\tif (link->setEnabled(true))\n> > -\t\t\tcontinue;\n> > -\n> > -\t\tstd::unique_ptr<IPU3CameraData> data = utils::make_unique<IPU3CameraData>();\n> > -\n> > -\t\tstd::string cameraName = sensor->name() + \" \" + std::to_string(id);\n> > +\t\tstd::unique_ptr<IPU3CameraData> data =\n> > +\t\t\tutils::make_unique<IPU3CameraData>();\n> >  \t\tstd::vector<Stream *> streams{ &data->stream_ };\n> > -\t\tstd::shared_ptr<Camera> camera = Camera::create(this, cameraName, streams);\n> > -\n> > -\t\t/*\n> > -\t\t * Create and open video devices and subdevices associated with\n> > -\t\t * the camera.\n> > -\t\t *\n> > -\t\t * If any of these operations fails, the Camera instance won't\n> > -\t\t * be registered. The 'camera' shared pointer and the 'data'\n> > -\t\t * unique pointers go out of scope and delete the objects they\n> > -\t\t * manage.\n> > -\t\t */\n> > -\t\tstd::string cio2Name = \"ipu3-cio2 \" + std::to_string(id);\n> > -\t\tMediaEntity *cio2 = cio2_->getEntityByName(cio2Name);\n> > -\t\tif (!cio2) {\n> > -\t\t\tLOG(IPU3, Error)\n> > -\t\t\t\t<< \"Failed to get entity '\" << cio2Name << \"'\";\n> > -\t\t\tcontinue;\n> > -\t\t}\n> > -\n> > -\t\tdata->cio2_ = new V4L2Device(cio2);\n> > -\t\tret = data->cio2_->open();\n> > -\t\tif (ret)\n> > -\t\t\tcontinue;\n> > +\t\tCio2Device *cio2 = &data->cio2;\n> >\n>\n> No need for a blank line here.\n>\n\nI'll fix.\n\nThanks\n  j\n\n> > -\t\tdata->sensor_ = new V4L2Subdevice(sensor);\n> > -\t\tret = data->sensor_->open();\n> > +\t\tret = initCio2(id, cio2);\n> >  \t\tif (ret)\n> >  \t\t\tcontinue;\n> >\n> > -\t\tdata->csi2_ = new V4L2Subdevice(csi2);\n> > -\t\tret = data->csi2_->open();\n> > -\t\tif (ret)\n> > -\t\t\tcontinue;\n> > +\t\tstd::string cameraName = cio2->sensor->deviceName() + \" \"\n> > +\t\t\t\t       + std::to_string(id);\n> > +\t\tstd::shared_ptr<Camera> camera =\n> > +\t\t\tCamera::create(this, cameraName, streams);\n> >\n> >  \t\tsetCameraData(camera.get(), std::move(data));\n> >  \t\tregisterCamera(std::move(camera));\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"<jacopo@jmondi.org>","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 CD06E600F9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat,  9 Mar 2019 21:34:19 +0100 (CET)","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 B1E8340004;\n\tSat,  9 Mar 2019 20:34:18 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Sat, 9 Mar 2019 21:34:48 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190309203448.qyldtwse7bzlsqe2@uno.localdomain>","References":"<20190228200410.3022-1-jacopo@jmondi.org>\n\t<20190228200410.3022-2-jacopo@jmondi.org>\n\t<20190302210444.GF4682@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"g7sqzxmffhztydim\"","Content-Disposition":"inline","In-Reply-To":"<20190302210444.GF4682@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 01/10] libcamera: ipu3: Group CIO2\n\tdevices","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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":"Sat, 09 Mar 2019 20:34:20 -0000"}}]