[{"id":18311,"web_url":"https://patchwork.libcamera.org/comment/18311/","msgid":"<YPvBAD/IuyyTFv0j@oden.dyn.berto.se>","date":"2021-07-24T07:28:00","subject":"Re: [libcamera-devel] [RFC PATCH 15/17] libcamera: pipeline: ipu3:\n\tMigrate to Camera::Private","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2021-07-23 07:00:34 +0300, Laurent Pinchart wrote:\n> As part of the effort to remove the CameraData class, migrate the\n> pipeline handler-specific camera data from CameraData to the\n> Camera::Private class.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  src/libcamera/pipeline/ipu3/ipu3.cpp | 38 +++++++++++++---------------\n>  1 file changed, 18 insertions(+), 20 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 15d6cca609ff..6d097ac91d2e 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -53,11 +53,11 @@ static const ControlInfoMap::Map IPU3Controls = {\n>  \t{ &controls::draft::PipelineDepth, ControlInfo(2, 3) },\n>  };\n>  \n> -class IPU3CameraData : public CameraData\n> +class IPU3CameraData : public Camera::Private\n>  {\n>  public:\n>  \tIPU3CameraData(PipelineHandler *pipe)\n> -\t\t: CameraData(pipe), exposureTime_(0), supportsFlips_(false)\n> +\t\t: Camera::Private(pipe), exposureTime_(0), supportsFlips_(false)\n>  \t{\n>  \t}\n>  \n> @@ -146,10 +146,9 @@ public:\n>  \tbool match(DeviceEnumerator *enumerator) override;\n>  \n>  private:\n> -\tIPU3CameraData *cameraData(const Camera *camera)\n> +\tIPU3CameraData *cameraData(Camera *camera)\n>  \t{\n> -\t\treturn static_cast<IPU3CameraData *>(\n> -\t\t\tPipelineHandler::cameraData(camera));\n> +\t\treturn static_cast<IPU3CameraData *>(camera->_d());\n>  \t}\n>  \n>  \tint initControls(IPU3CameraData *data);\n> @@ -800,10 +799,10 @@ void IPU3CameraData::cancelPendingRequests()\n>  \t\tfor (auto it : request->buffers()) {\n>  \t\t\tFrameBuffer *buffer = it.second;\n>  \t\t\tbuffer->cancel();\n> -\t\t\tpipe_->completeBuffer(request, buffer);\n> +\t\t\tpipe()->completeBuffer(request, buffer);\n>  \t\t}\n>  \n> -\t\tpipe_->completeRequest(request);\n> +\t\tpipe()->completeRequest(request);\n>  \t\tpendingRequests_.pop();\n>  \t}\n>  }\n> @@ -1184,12 +1183,11 @@ int PipelineHandlerIPU3::registerCameras()\n>  \t\t\t\t\t&IPU3CameraData::statBufferReady);\n>  \n>  \t\t/* Create and register the Camera instance. */\n> -\t\tstd::string cameraId = cio2->sensor()->id();\n> +\t\tconst std::string &cameraId = cio2->sensor()->id();\n>  \t\tstd::shared_ptr<Camera> camera =\n> -\t\t\tCamera::create(new Camera::Private(this), cameraId,\n> -\t\t\t\t       streams);\n> +\t\t\tCamera::create(data.release(), cameraId, streams);\n>  \n> -\t\tregisterCamera(std::move(camera), std::move(data));\n> +\t\tregisterCamera(std::move(camera), nullptr);\n>  \n>  \t\tLOG(IPU3, Info)\n>  \t\t\t<< \"Registered Camera[\" << numCameras << \"] \\\"\"\n> @@ -1204,7 +1202,7 @@ int PipelineHandlerIPU3::registerCameras()\n>  \n>  int IPU3CameraData::loadIPA()\n>  {\n> -\tipa_ = IPAManager::createIPA<ipa::ipu3::IPAProxyIPU3>(pipe_, 1, 1);\n> +\tipa_ = IPAManager::createIPA<ipa::ipu3::IPAProxyIPU3>(pipe(), 1, 1);\n>  \tif (!ipa_)\n>  \t\treturn -ENOENT;\n>  \n> @@ -1261,7 +1259,7 @@ void IPU3CameraData::queueFrameAction(unsigned int id,\n>  \n>  \t\tinfo->metadataProcessed = true;\n>  \t\tif (frameInfos_.tryComplete(info))\n> -\t\t\tpipe_->completeRequest(request);\n> +\t\t\tpipe()->completeRequest(request);\n>  \n>  \t\tbreak;\n>  \t}\n> @@ -1289,7 +1287,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n>  \n>  \tRequest *request = info->request;\n>  \n> -\tpipe_->completeBuffer(request, buffer);\n> +\tpipe()->completeBuffer(request, buffer);\n>  \n>  \trequest->metadata().set(controls::draft::PipelineDepth, 3);\n>  \t/* \\todo Move the ExposureTime control to the IPA. */\n> @@ -1300,7 +1298,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n>  \trequest->metadata().set(controls::ScalerCrop, cropRegion_);\n>  \n>  \tif (frameInfos_.tryComplete(info))\n> -\t\tpipe_->completeRequest(request);\n> +\t\tpipe()->completeRequest(request);\n>  }\n>  \n>  /**\n> @@ -1332,16 +1330,16 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n>  \t\tfor (auto it : request->buffers()) {\n>  \t\t\tFrameBuffer *b = it.second;\n>  \t\t\tb->cancel();\n> -\t\t\tpipe_->completeBuffer(request, b);\n> +\t\t\tpipe()->completeBuffer(request, b);\n>  \t\t}\n>  \n>  \t\tframeInfos_.remove(info);\n> -\t\tpipe_->completeRequest(request);\n> +\t\tpipe()->completeRequest(request);\n>  \t\treturn;\n>  \t}\n>  \n>  \tif (request->findBuffer(&rawStream_))\n> -\t\tpipe_->completeBuffer(request, buffer);\n> +\t\tpipe()->completeBuffer(request, buffer);\n>  \n>  \tipa::ipu3::IPU3Event ev;\n>  \tev.op = ipa::ipu3::EventFillParams;\n> @@ -1367,7 +1365,7 @@ void IPU3CameraData::paramBufferReady(FrameBuffer *buffer)\n>  \tRequest *request = info->request;\n>  \n>  \tif (frameInfos_.tryComplete(info))\n> -\t\tpipe_->completeRequest(request);\n> +\t\tpipe()->completeRequest(request);\n>  }\n>  \n>  void IPU3CameraData::statBufferReady(FrameBuffer *buffer)\n> @@ -1386,7 +1384,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)\n>  \t\t * In that event, we must have obtained the Request before hand.\n>  \t\t */\n>  \t\tif (frameInfos_.tryComplete(info))\n> -\t\t\tpipe_->completeRequest(request);\n> +\t\t\tpipe()->completeRequest(request);\n>  \n>  \t\treturn;\n>  \t}\n> -- \n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 65F81C0109\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 24 Jul 2021 07:28:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1E1AC687A9;\n\tSat, 24 Jul 2021 09:28:04 +0200 (CEST)","from mail-lf1-x131.google.com (mail-lf1-x131.google.com\n\t[IPv6:2a00:1450:4864:20::131])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 956FB68540\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 24 Jul 2021 09:28:02 +0200 (CEST)","by mail-lf1-x131.google.com with SMTP id d18so5855555lfb.6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 24 Jul 2021 00:28:02 -0700 (PDT)","from localhost (h-46-59-88-219.A463.priv.bahnhof.se.\n\t[46.59.88.219]) by smtp.gmail.com with ESMTPSA id\n\tm11sm1518971lji.8.2021.07.24.00.28.01\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 24 Jul 2021 00:28:01 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"zNbsQ2Yg\"; dkim-atps=neutral","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\tbh=kIRVMjTdtDLCQsOxV6zsoAY5+wk59fYeITWDlt70Vmk=;\n\tb=zNbsQ2YgvnNFUCSZrLklcQSJG8z2pfWui5yhVYVOrY6JHSR/n7S6Qhm8OtnH0WrIeb\n\tsF6Ax9bVhMBbHIEClS0QbC+91xv+nOJvxFaFrNOA9JHeJuG1Eg9u/Wbeuu1j9EaY7UCv\n\tzK3+RtZfIHHPUnptm5/inzF6ttwT2yhFI1NPmDG5RAYESwu5zwrzVATRpaiZ/4SH9R0b\n\t/C5y6hPOWJtmrssnK6VAW9I9dx5KgKfXpil3ETuq5vbxMmHW6oB5pc5+Y8b9tCty4lyR\n\tkqWr1b6ouDF4AeUwpl0t4nkFaAcbxaSaW1xzHpt+2taOrj5CTR+NbXjMGQspTCgOkj/l\n\tXJOA==","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;\n\tbh=kIRVMjTdtDLCQsOxV6zsoAY5+wk59fYeITWDlt70Vmk=;\n\tb=PlbxRJL0N0qMQxMpf7mQ+LSr4BJ6tHuxbfaQ3t9MMy54v2DgHZKnbPdIioEXAMlgKK\n\tiPTffsh47Z+6Ws5uwpDYoFWDWTN3BEBwpSsAKtIwNuz8KatI1g8mWQDOSnkZqR461D+5\n\tA1T4CgGRUoQWkS5DSJDUutKBuDkUZGXP9nJG9rzFvtNXUT9Nvz6Qc4IjB1/UkLMc/hLT\n\t6j68nJDW2z3HuI3IGz6WnHQOpveG2lAJpk9u/QEMFuNAVHtDa8BB5LwpvyALGrokuvLQ\n\tw9yEnI24/GY+XCFkw5XQ7trdCpgKNOm/P3arjZ6fPRQ7ex+jCo2eg/ImeGV1bOwGQyOB\n\tJoWA==","X-Gm-Message-State":"AOAM531+rVl0req7sIWRqqN+eGB/LCx/U2u27bzZLWO6f77zck029HFt\n\t4NSFOn+UAoMJS41Wu43vvTbFNUTuVOqp7Q==","X-Google-Smtp-Source":"ABdhPJwERYQO8UXS3zj+hIr9Xb1fvkXf7mGt56SgsDQnjPuLFN9e2fV/2/UnE1si5fDlb4/ntZQP1g==","X-Received":"by 2002:ac2:4d56:: with SMTP id 22mr5563643lfp.463.1627111681994;\n\tSat, 24 Jul 2021 00:28:01 -0700 (PDT)","Date":"Sat, 24 Jul 2021 09:28:00 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<YPvBAD/IuyyTFv0j@oden.dyn.berto.se>","References":"<20210723040036.32346-1-laurent.pinchart@ideasonboard.com>\n\t<20210723040036.32346-16-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20210723040036.32346-16-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH 15/17] libcamera: pipeline: ipu3:\n\tMigrate to Camera::Private","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":18431,"web_url":"https://patchwork.libcamera.org/comment/18431/","msgid":"<20210729211459.vtgyhwhygv23ncgh@uno.localdomain>","date":"2021-07-29T21:14:59","subject":"Re: [libcamera-devel] [RFC PATCH 15/17] libcamera: pipeline: ipu3:\n\tMigrate to Camera::Private","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n`\nOn Fri, Jul 23, 2021 at 07:00:34AM +0300, Laurent Pinchart wrote:\n> As part of the effort to remove the CameraData class, migrate the\n> pipeline handler-specific camera data from CameraData to the\n> Camera::Private class.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  src/libcamera/pipeline/ipu3/ipu3.cpp | 38 +++++++++++++---------------\n>  1 file changed, 18 insertions(+), 20 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 15d6cca609ff..6d097ac91d2e 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -53,11 +53,11 @@ static const ControlInfoMap::Map IPU3Controls = {\n>  \t{ &controls::draft::PipelineDepth, ControlInfo(2, 3) },\n>  };\n>\n> -class IPU3CameraData : public CameraData\n>+class IPU3CameraData : public Camera::Private\n\nA very trivial comment, shouldn't this class now just be IPU3Camera ?\n\nthanks\n   j\n\n>  {\n>  public:\n>  \tIPU3CameraData(PipelineHandler *pipe)\n> -\t\t: CameraData(pipe), exposureTime_(0), supportsFlips_(false)\n> +\t\t: Camera::Private(pipe), exposureTime_(0), supportsFlips_(false)\n>  \t{\n>  \t}\n>\n> @@ -146,10 +146,9 @@ public:\n>  \tbool match(DeviceEnumerator *enumerator) override;\n>\n>  private:\n> -\tIPU3CameraData *cameraData(const Camera *camera)\n> +\tIPU3CameraData *cameraData(Camera *camera)\n>  \t{\n> -\t\treturn static_cast<IPU3CameraData *>(\n> -\t\t\tPipelineHandler::cameraData(camera));\n> +\t\treturn static_cast<IPU3CameraData *>(camera->_d());\n>  \t}\n>\n>  \tint initControls(IPU3CameraData *data);\n> @@ -800,10 +799,10 @@ void IPU3CameraData::cancelPendingRequests()\n>  \t\tfor (auto it : request->buffers()) {\n>  \t\t\tFrameBuffer *buffer = it.second;\n>  \t\t\tbuffer->cancel();\n> -\t\t\tpipe_->completeBuffer(request, buffer);\n> +\t\t\tpipe()->completeBuffer(request, buffer);\n>  \t\t}\n>\n> -\t\tpipe_->completeRequest(request);\n> +\t\tpipe()->completeRequest(request);\n>  \t\tpendingRequests_.pop();\n>  \t}\n>  }\n> @@ -1184,12 +1183,11 @@ int PipelineHandlerIPU3::registerCameras()\n>  \t\t\t\t\t&IPU3CameraData::statBufferReady);\n>\n>  \t\t/* Create and register the Camera instance. */\n> -\t\tstd::string cameraId = cio2->sensor()->id();\n> +\t\tconst std::string &cameraId = cio2->sensor()->id();\n>  \t\tstd::shared_ptr<Camera> camera =\n> -\t\t\tCamera::create(new Camera::Private(this), cameraId,\n> -\t\t\t\t       streams);\n> +\t\t\tCamera::create(data.release(), cameraId, streams);\n>\n> -\t\tregisterCamera(std::move(camera), std::move(data));\n> +\t\tregisterCamera(std::move(camera), nullptr);\n>\n>  \t\tLOG(IPU3, Info)\n>  \t\t\t<< \"Registered Camera[\" << numCameras << \"] \\\"\"\n> @@ -1204,7 +1202,7 @@ int PipelineHandlerIPU3::registerCameras()\n>\n>  int IPU3CameraData::loadIPA()\n>  {\n> -\tipa_ = IPAManager::createIPA<ipa::ipu3::IPAProxyIPU3>(pipe_, 1, 1);\n> +\tipa_ = IPAManager::createIPA<ipa::ipu3::IPAProxyIPU3>(pipe(), 1, 1);\n>  \tif (!ipa_)\n>  \t\treturn -ENOENT;\n>\n> @@ -1261,7 +1259,7 @@ void IPU3CameraData::queueFrameAction(unsigned int id,\n>\n>  \t\tinfo->metadataProcessed = true;\n>  \t\tif (frameInfos_.tryComplete(info))\n> -\t\t\tpipe_->completeRequest(request);\n> +\t\t\tpipe()->completeRequest(request);\n>\n>  \t\tbreak;\n>  \t}\n> @@ -1289,7 +1287,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n>\n>  \tRequest *request = info->request;\n>\n> -\tpipe_->completeBuffer(request, buffer);\n> +\tpipe()->completeBuffer(request, buffer);\n>\n>  \trequest->metadata().set(controls::draft::PipelineDepth, 3);\n>  \t/* \\todo Move the ExposureTime control to the IPA. */\n> @@ -1300,7 +1298,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n>  \trequest->metadata().set(controls::ScalerCrop, cropRegion_);\n>\n>  \tif (frameInfos_.tryComplete(info))\n> -\t\tpipe_->completeRequest(request);\n> +\t\tpipe()->completeRequest(request);\n>  }\n>\n>  /**\n> @@ -1332,16 +1330,16 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n>  \t\tfor (auto it : request->buffers()) {\n>  \t\t\tFrameBuffer *b = it.second;\n>  \t\t\tb->cancel();\n> -\t\t\tpipe_->completeBuffer(request, b);\n> +\t\t\tpipe()->completeBuffer(request, b);\n>  \t\t}\n>\n>  \t\tframeInfos_.remove(info);\n> -\t\tpipe_->completeRequest(request);\n> +\t\tpipe()->completeRequest(request);\n>  \t\treturn;\n>  \t}\n>\n>  \tif (request->findBuffer(&rawStream_))\n> -\t\tpipe_->completeBuffer(request, buffer);\n> +\t\tpipe()->completeBuffer(request, buffer);\n>\n>  \tipa::ipu3::IPU3Event ev;\n>  \tev.op = ipa::ipu3::EventFillParams;\n> @@ -1367,7 +1365,7 @@ void IPU3CameraData::paramBufferReady(FrameBuffer *buffer)\n>  \tRequest *request = info->request;\n>\n>  \tif (frameInfos_.tryComplete(info))\n> -\t\tpipe_->completeRequest(request);\n> +\t\tpipe()->completeRequest(request);\n>  }\n>\n>  void IPU3CameraData::statBufferReady(FrameBuffer *buffer)\n> @@ -1386,7 +1384,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)\n>  \t\t * In that event, we must have obtained the Request before hand.\n>  \t\t */\n>  \t\tif (frameInfos_.tryComplete(info))\n> -\t\t\tpipe_->completeRequest(request);\n> +\t\t\tpipe()->completeRequest(request);\n>\n>  \t\treturn;\n>  \t}\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 96C57C322E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 29 Jul 2021 21:14:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 033D0687C5;\n\tThu, 29 Jul 2021 23:14:15 +0200 (CEST)","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 57633687BA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 29 Jul 2021 23:14:13 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 95B7840008;\n\tThu, 29 Jul 2021 21:14:12 +0000 (UTC)"],"Date":"Thu, 29 Jul 2021 23:14:59 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20210729211459.vtgyhwhygv23ncgh@uno.localdomain>","References":"<20210723040036.32346-1-laurent.pinchart@ideasonboard.com>\n\t<20210723040036.32346-16-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210723040036.32346-16-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH 15/17] libcamera: pipeline: ipu3:\n\tMigrate to Camera::Private","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":18437,"web_url":"https://patchwork.libcamera.org/comment/18437/","msgid":"<YQNpgG6XnCqb2XKr@pendragon.ideasonboard.com>","date":"2021-07-30T02:52:48","subject":"Re: [libcamera-devel] [RFC PATCH 15/17] libcamera: pipeline: ipu3:\n\tMigrate to Camera::Private","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Thu, Jul 29, 2021 at 11:14:59PM +0200, Jacopo Mondi wrote:\n> On Fri, Jul 23, 2021 at 07:00:34AM +0300, Laurent Pinchart wrote:\n> > As part of the effort to remove the CameraData class, migrate the\n> > pipeline handler-specific camera data from CameraData to the\n> > Camera::Private class.\n> >\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 38 +++++++++++++---------------\n> >  1 file changed, 18 insertions(+), 20 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 15d6cca609ff..6d097ac91d2e 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -53,11 +53,11 @@ static const ControlInfoMap::Map IPU3Controls = {\n> >  \t{ &controls::draft::PipelineDepth, ControlInfo(2, 3) },\n> >  };\n> >\n> > -class IPU3CameraData : public CameraData\n> >+class IPU3CameraData : public Camera::Private\n> \n> A very trivial comment, shouldn't this class now just be IPU3Camera ?\n\nI think that would be a bit misleading, as the name implies (for me at\nleast) that it would be a subclass of Camera.\n\nThis being said, I'm open to considering letting pipeline handlers\nsubclass the Camera class. We haven't allowed this up to now in order\nmainly to avoid exposing the Camera constructor as a public function of\nthe Camera class, but it's not an issue anymore as applications can't\nconstruct a Camera::Private instance to pass to the Camera constructor.\nI haven't however considered all the implications, and Niklas didn't\nseem to be thrilled by the idea, so I haven't really investigated it.\n\n> >  {\n> >  public:\n> >  \tIPU3CameraData(PipelineHandler *pipe)\n> > -\t\t: CameraData(pipe), exposureTime_(0), supportsFlips_(false)\n> > +\t\t: Camera::Private(pipe), exposureTime_(0), supportsFlips_(false)\n> >  \t{\n> >  \t}\n> >\n> > @@ -146,10 +146,9 @@ public:\n> >  \tbool match(DeviceEnumerator *enumerator) override;\n> >\n> >  private:\n> > -\tIPU3CameraData *cameraData(const Camera *camera)\n> > +\tIPU3CameraData *cameraData(Camera *camera)\n> >  \t{\n> > -\t\treturn static_cast<IPU3CameraData *>(\n> > -\t\t\tPipelineHandler::cameraData(camera));\n> > +\t\treturn static_cast<IPU3CameraData *>(camera->_d());\n> >  \t}\n> >\n> >  \tint initControls(IPU3CameraData *data);\n> > @@ -800,10 +799,10 @@ void IPU3CameraData::cancelPendingRequests()\n> >  \t\tfor (auto it : request->buffers()) {\n> >  \t\t\tFrameBuffer *buffer = it.second;\n> >  \t\t\tbuffer->cancel();\n> > -\t\t\tpipe_->completeBuffer(request, buffer);\n> > +\t\t\tpipe()->completeBuffer(request, buffer);\n> >  \t\t}\n> >\n> > -\t\tpipe_->completeRequest(request);\n> > +\t\tpipe()->completeRequest(request);\n> >  \t\tpendingRequests_.pop();\n> >  \t}\n> >  }\n> > @@ -1184,12 +1183,11 @@ int PipelineHandlerIPU3::registerCameras()\n> >  \t\t\t\t\t&IPU3CameraData::statBufferReady);\n> >\n> >  \t\t/* Create and register the Camera instance. */\n> > -\t\tstd::string cameraId = cio2->sensor()->id();\n> > +\t\tconst std::string &cameraId = cio2->sensor()->id();\n> >  \t\tstd::shared_ptr<Camera> camera =\n> > -\t\t\tCamera::create(new Camera::Private(this), cameraId,\n> > -\t\t\t\t       streams);\n> > +\t\t\tCamera::create(data.release(), cameraId, streams);\n> >\n> > -\t\tregisterCamera(std::move(camera), std::move(data));\n> > +\t\tregisterCamera(std::move(camera), nullptr);\n> >\n> >  \t\tLOG(IPU3, Info)\n> >  \t\t\t<< \"Registered Camera[\" << numCameras << \"] \\\"\"\n> > @@ -1204,7 +1202,7 @@ int PipelineHandlerIPU3::registerCameras()\n> >\n> >  int IPU3CameraData::loadIPA()\n> >  {\n> > -\tipa_ = IPAManager::createIPA<ipa::ipu3::IPAProxyIPU3>(pipe_, 1, 1);\n> > +\tipa_ = IPAManager::createIPA<ipa::ipu3::IPAProxyIPU3>(pipe(), 1, 1);\n> >  \tif (!ipa_)\n> >  \t\treturn -ENOENT;\n> >\n> > @@ -1261,7 +1259,7 @@ void IPU3CameraData::queueFrameAction(unsigned int id,\n> >\n> >  \t\tinfo->metadataProcessed = true;\n> >  \t\tif (frameInfos_.tryComplete(info))\n> > -\t\t\tpipe_->completeRequest(request);\n> > +\t\t\tpipe()->completeRequest(request);\n> >\n> >  \t\tbreak;\n> >  \t}\n> > @@ -1289,7 +1287,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n> >\n> >  \tRequest *request = info->request;\n> >\n> > -\tpipe_->completeBuffer(request, buffer);\n> > +\tpipe()->completeBuffer(request, buffer);\n> >\n> >  \trequest->metadata().set(controls::draft::PipelineDepth, 3);\n> >  \t/* \\todo Move the ExposureTime control to the IPA. */\n> > @@ -1300,7 +1298,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n> >  \trequest->metadata().set(controls::ScalerCrop, cropRegion_);\n> >\n> >  \tif (frameInfos_.tryComplete(info))\n> > -\t\tpipe_->completeRequest(request);\n> > +\t\tpipe()->completeRequest(request);\n> >  }\n> >\n> >  /**\n> > @@ -1332,16 +1330,16 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n> >  \t\tfor (auto it : request->buffers()) {\n> >  \t\t\tFrameBuffer *b = it.second;\n> >  \t\t\tb->cancel();\n> > -\t\t\tpipe_->completeBuffer(request, b);\n> > +\t\t\tpipe()->completeBuffer(request, b);\n> >  \t\t}\n> >\n> >  \t\tframeInfos_.remove(info);\n> > -\t\tpipe_->completeRequest(request);\n> > +\t\tpipe()->completeRequest(request);\n> >  \t\treturn;\n> >  \t}\n> >\n> >  \tif (request->findBuffer(&rawStream_))\n> > -\t\tpipe_->completeBuffer(request, buffer);\n> > +\t\tpipe()->completeBuffer(request, buffer);\n> >\n> >  \tipa::ipu3::IPU3Event ev;\n> >  \tev.op = ipa::ipu3::EventFillParams;\n> > @@ -1367,7 +1365,7 @@ void IPU3CameraData::paramBufferReady(FrameBuffer *buffer)\n> >  \tRequest *request = info->request;\n> >\n> >  \tif (frameInfos_.tryComplete(info))\n> > -\t\tpipe_->completeRequest(request);\n> > +\t\tpipe()->completeRequest(request);\n> >  }\n> >\n> >  void IPU3CameraData::statBufferReady(FrameBuffer *buffer)\n> > @@ -1386,7 +1384,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)\n> >  \t\t * In that event, we must have obtained the Request before hand.\n> >  \t\t */\n> >  \t\tif (frameInfos_.tryComplete(info))\n> > -\t\t\tpipe_->completeRequest(request);\n> > +\t\t\tpipe()->completeRequest(request);\n> >\n> >  \t\treturn;\n> >  \t}","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2C136C322E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 30 Jul 2021 02:52:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 94C10687CB;\n\tFri, 30 Jul 2021 04:52:58 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B378B687BD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 30 Jul 2021 04:52:56 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 32B3389B;\n\tFri, 30 Jul 2021 04:52:56 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"CAVzkaYv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1627613576;\n\tbh=GjNK/+I44N/fzlSqmtFdneMMFjvSBImNNWmnc0VnKqg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=CAVzkaYvtFisxpULxDnSGJcOiKUzZcrufLrnmVpkzwi2eTlgMmgcM1KXmdFROhIA9\n\tmTBjJcMQhXUInFipKH3c5/6L4o1KCTNkqr+0VLhG9cRiIvCNey6U+5tyKkekDaLvYS\n\tHciHz37N7QoYNPtgfJ2wkOfynI1TJ2PoN9DQZNFM=","Date":"Fri, 30 Jul 2021 05:52:48 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<YQNpgG6XnCqb2XKr@pendragon.ideasonboard.com>","References":"<20210723040036.32346-1-laurent.pinchart@ideasonboard.com>\n\t<20210723040036.32346-16-laurent.pinchart@ideasonboard.com>\n\t<20210729211459.vtgyhwhygv23ncgh@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210729211459.vtgyhwhygv23ncgh@uno.localdomain>","Subject":"Re: [libcamera-devel] [RFC PATCH 15/17] libcamera: pipeline: ipu3:\n\tMigrate to Camera::Private","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]