Patch Detail
Show a patch.
GET /api/1.1/patches/17671/?format=api
{ "id": 17671, "url": "https://patchwork.libcamera.org/api/1.1/patches/17671/?format=api", "web_url": "https://patchwork.libcamera.org/patch/17671/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20221024000356.29521-9-laurent.pinchart@ideasonboard.com>", "date": "2022-10-24T00:03:51", "name": "[libcamera-devel,v3,08/13] libcamera: pipeline_handler: Return unique_ptr from generateConfiguration()", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "66d46f94d3d82bb6e5cbcf202167c541608af8b7", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/1.1/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/17671/mbox/", "series": [ { "id": 3574, "url": "https://patchwork.libcamera.org/api/1.1/series/3574/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3574", "date": "2022-10-24T00:03:43", "name": "Add Bayer format support for RkISP1", "version": 3, "mbox": "https://patchwork.libcamera.org/series/3574/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/17671/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/17671/checks/", "tags": {}, "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 946E7C3289\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Oct 2022 00:04:37 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D33FB62EFD;\n\tMon, 24 Oct 2022 02:04:36 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 64C8F62ED2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Oct 2022 02:04:32 +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 EDB4088F;\n\tMon, 24 Oct 2022 02:04:31 +0200 (CEST)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666569876;\n\tbh=w3agMv/Il0W8IEOQXHr443qEB6P2AARPOi0AzE1asvM=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=ZzIITo4lv70RWso/P9nDz4lk826ywDDVLYCKOy1Sgel57Y818lWWWVlhJVWcWpvJ8\n\tlKFgBo8c8dv+T0ouT1N4cAkEvIrafsQoTcFRtovE8oLDFtAupl9wJW+vuaINocPrQT\n\t1qk+R7BN1i7+i4h1mrqVDTtXwQJE9Ow82gHlC/cENEUVqQM9tuBBYJeTV1hr/PEO7p\n\tgskHTqKsu/ZX3u9P5z9JCuiSqpe2BqqSPdgm6/04vcysRMR9YqoWOhp6OMx2I5GxDx\n\tEOzYL4V1qdCLeMaz025nOi+H4GFCnF1yAUuxzChaPK5B9IRUDnhhs8+aDwskmAqEs/\n\t26ts7lpoUr0EA==", "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1666569872;\n\tbh=w3agMv/Il0W8IEOQXHr443qEB6P2AARPOi0AzE1asvM=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=RqgHJXyhPldfMhE/CzxP5pc1BkTpPz+fhQL8nLFltmm1pwt82b4rfLWoo3wvEm1pY\n\tGrdxwC/qpQvoSifgyCpN6X0HMzFCoEqhY5hQ1w1Q4iobI3jbE7Xnlf7IlMvc0KZLlq\n\t9zAB/YefuoKIsgCDVxRyT4pK5bTal5PEUJtAFupc=" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"RqgHJXyh\"; dkim-atps=neutral", "To": "libcamera-devel@lists.libcamera.org", "Date": "Mon, 24 Oct 2022 03:03:51 +0300", "Message-Id": "<20221024000356.29521-9-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.37.4", "In-Reply-To": "<20221024000356.29521-1-laurent.pinchart@ideasonboard.com>", "References": "<20221024000356.29521-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v3 08/13] libcamera: pipeline_handler:\n\tReturn unique_ptr from generateConfiguration()", "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>", "From": "Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>", "Reply-To": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "The PipelineHandler::generateConfiguration() function allocates a\nCameraConfiguration instance and returns it. The ownership of the\ninstance is transferred to the caller. This is a perfect match for a\nstd::unique_ptr<>, which the Camera::generateConfiguration() function\nalready returns. Update PipelineHandler::generateConfiguration() to\nmatch it. This fixes a memory leak in one of the error return paths in\nthe IPU3 pipeline handler.\n\nWhile at it, update the Camera::generateConfiguration() function\ndocumentation to drop the sentence that describes the ownership\ntransfer, as that is implied by usage of std::unique_ptr<>.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n include/libcamera/internal/pipeline_handler.h | 2 +-\n src/libcamera/camera.cpp | 8 ++++----\n src/libcamera/pipeline/ipu3/ipu3.cpp | 10 +++++-----\n src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 12 ++++++------\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 +++++----\n src/libcamera/pipeline/simple/simple.cpp | 12 ++++++------\n src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 8 +++++---\n src/libcamera/pipeline/vimc/vimc.cpp | 8 +++++---\n src/libcamera/pipeline_handler.cpp | 3 +--\n 9 files changed, 38 insertions(+), 34 deletions(-)", "diff": "diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\nindex b6139a88d421..96aab9d6459e 100644\n--- a/include/libcamera/internal/pipeline_handler.h\n+++ b/include/libcamera/internal/pipeline_handler.h\n@@ -48,7 +48,7 @@ public:\n \tbool acquire();\n \tvoid release();\n \n-\tvirtual CameraConfiguration *generateConfiguration(Camera *camera,\n+\tvirtual std::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n \t\tconst StreamRoles &roles) = 0;\n \tvirtual int configure(Camera *camera, CameraConfiguration *config) = 0;\n \ndiff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\nindex 9fe29ca9dca5..daef77016971 100644\n--- a/src/libcamera/camera.cpp\n+++ b/src/libcamera/camera.cpp\n@@ -934,8 +934,7 @@ const std::set<Stream *> &Camera::streams() const\n * \\context This function is \\threadsafe.\n *\n * \\return A CameraConfiguration if the requested roles can be satisfied, or a\n- * null pointer otherwise. The ownership of the returned configuration is\n- * passed to the caller.\n+ * null pointer otherwise.\n */\n std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamRoles &roles)\n {\n@@ -949,7 +948,8 @@ std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamR\n \tif (roles.size() > streams().size())\n \t\treturn nullptr;\n \n-\tCameraConfiguration *config = d->pipe_->generateConfiguration(this, roles);\n+\tstd::unique_ptr<CameraConfiguration> config =\n+\t\td->pipe_->generateConfiguration(this, roles);\n \tif (!config) {\n \t\tLOG(Camera, Debug)\n \t\t\t<< \"Pipeline handler failed to generate configuration\";\n@@ -966,7 +966,7 @@ std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamR\n \n \tLOG(Camera, Debug) << msg.str();\n \n-\treturn std::unique_ptr<CameraConfiguration>(config);\n+\treturn config;\n }\n \n /**\ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex 3b892d9671c5..e4d79ea44aed 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -136,7 +136,7 @@ public:\n \n \tPipelineHandlerIPU3(CameraManager *manager);\n \n-\tCameraConfiguration *generateConfiguration(Camera *camera,\n+\tstd::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n \t\tconst StreamRoles &roles) override;\n \tint configure(Camera *camera, CameraConfiguration *config) override;\n \n@@ -424,11 +424,12 @@ PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager)\n {\n }\n \n-CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,\n-\t\t\t\t\t\t\t\tconst StreamRoles &roles)\n+std::unique_ptr<CameraConfiguration>\n+PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &roles)\n {\n \tIPU3CameraData *data = cameraData(camera);\n-\tIPU3CameraConfiguration *config = new IPU3CameraConfiguration(data);\n+\tstd::unique_ptr<IPU3CameraConfiguration> config =\n+\t\tstd::make_unique<IPU3CameraConfiguration>(data);\n \n \tif (roles.empty())\n \t\treturn config;\n@@ -494,7 +495,6 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,\n \t\tdefault:\n \t\t\tLOG(IPU3, Error)\n \t\t\t\t<< \"Requested stream role not supported: \" << role;\n-\t\t\tdelete config;\n \t\t\treturn nullptr;\n \t\t}\n \ndiff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\nindex 343f8cb2c7ed..7c54550005fa 100644\n--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n@@ -323,7 +323,8 @@ class PipelineHandlerRPi : public PipelineHandler\n public:\n \tPipelineHandlerRPi(CameraManager *manager);\n \n-\tCameraConfiguration *generateConfiguration(Camera *camera, const StreamRoles &roles) override;\n+\tstd::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n+\t\tconst StreamRoles &roles) override;\n \tint configure(Camera *camera, CameraConfiguration *config) override;\n \n \tint exportFrameBuffers(Camera *camera, Stream *stream,\n@@ -561,11 +562,12 @@ PipelineHandlerRPi::PipelineHandlerRPi(CameraManager *manager)\n {\n }\n \n-CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n-\t\t\t\t\t\t\t const StreamRoles &roles)\n+std::unique_ptr<CameraConfiguration>\n+PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &roles)\n {\n \tRPiCameraData *data = cameraData(camera);\n-\tCameraConfiguration *config = new RPiCameraConfiguration(data);\n+\tstd::unique_ptr<CameraConfiguration> config =\n+\t\tstd::make_unique<RPiCameraConfiguration>(data);\n \tV4L2SubdeviceFormat sensorFormat;\n \tunsigned int bufferCount;\n \tPixelFormat pixelFormat;\n@@ -640,13 +642,11 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n \t\tdefault:\n \t\t\tLOG(RPI, Error) << \"Requested stream role not supported: \"\n \t\t\t\t\t<< role;\n-\t\t\tdelete config;\n \t\t\treturn nullptr;\n \t\t}\n \n \t\tif (rawCount > 1 || outCount > 2) {\n \t\t\tLOG(RPI, Error) << \"Invalid stream roles requested\";\n-\t\t\tdelete config;\n \t\t\treturn nullptr;\n \t\t}\n \ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 1418dc9a47fb..50da92d4d6f8 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -142,7 +142,7 @@ class PipelineHandlerRkISP1 : public PipelineHandler\n public:\n \tPipelineHandlerRkISP1(CameraManager *manager);\n \n-\tCameraConfiguration *generateConfiguration(Camera *camera,\n+\tstd::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n \t\tconst StreamRoles &roles) override;\n \tint configure(Camera *camera, CameraConfiguration *config) override;\n \n@@ -539,7 +539,8 @@ PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)\n * Pipeline Operations\n */\n \n-CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n+std::unique_ptr<CameraConfiguration>\n+PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n \tconst StreamRoles &roles)\n {\n \tRkISP1CameraData *data = cameraData(camera);\n@@ -550,7 +551,8 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n \t\treturn nullptr;\n \t}\n \n-\tCameraConfiguration *config = new RkISP1CameraConfiguration(camera, data);\n+\tstd::unique_ptr<CameraConfiguration> config =\n+\t\tstd::make_unique<RkISP1CameraConfiguration>(camera, data);\n \tif (roles.empty())\n \t\treturn config;\n \n@@ -595,7 +597,6 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n \t\tdefault:\n \t\t\tLOG(RkISP1, Warning)\n \t\t\t\t<< \"Requested stream role not supported: \" << role;\n-\t\t\tdelete config;\n \t\t\treturn nullptr;\n \t\t}\n \ndiff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\nindex 37b3e7acd0a1..a32de7f36e13 100644\n--- a/src/libcamera/pipeline/simple/simple.cpp\n+++ b/src/libcamera/pipeline/simple/simple.cpp\n@@ -311,8 +311,8 @@ class SimplePipelineHandler : public PipelineHandler\n public:\n \tSimplePipelineHandler(CameraManager *manager);\n \n-\tCameraConfiguration *generateConfiguration(Camera *camera,\n-\t\t\t\t\t\t const StreamRoles &roles) override;\n+\tstd::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n+\t\tconst StreamRoles &roles) override;\n \tint configure(Camera *camera, CameraConfiguration *config) override;\n \n \tint exportFrameBuffers(Camera *camera, Stream *stream,\n@@ -1037,12 +1037,12 @@ SimplePipelineHandler::SimplePipelineHandler(CameraManager *manager)\n {\n }\n \n-CameraConfiguration *SimplePipelineHandler::generateConfiguration(Camera *camera,\n-\t\t\t\t\t\t\t\t const StreamRoles &roles)\n+std::unique_ptr<CameraConfiguration>\n+SimplePipelineHandler::generateConfiguration(Camera *camera, const StreamRoles &roles)\n {\n \tSimpleCameraData *data = cameraData(camera);\n-\tCameraConfiguration *config =\n-\t\tnew SimpleCameraConfiguration(camera, data);\n+\tstd::unique_ptr<CameraConfiguration> config =\n+\t\tstd::make_unique<SimpleCameraConfiguration>(camera, data);\n \n \tif (roles.empty())\n \t\treturn config;\ndiff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\nindex a28195450634..277465b72164 100644\n--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n@@ -74,7 +74,7 @@ class PipelineHandlerUVC : public PipelineHandler\n public:\n \tPipelineHandlerUVC(CameraManager *manager);\n \n-\tCameraConfiguration *generateConfiguration(Camera *camera,\n+\tstd::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n \t\tconst StreamRoles &roles) override;\n \tint configure(Camera *camera, CameraConfiguration *config) override;\n \n@@ -178,11 +178,13 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)\n {\n }\n \n-CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,\n+std::unique_ptr<CameraConfiguration>\n+PipelineHandlerUVC::generateConfiguration(Camera *camera,\n \tconst StreamRoles &roles)\n {\n \tUVCCameraData *data = cameraData(camera);\n-\tCameraConfiguration *config = new UVCCameraConfiguration(data);\n+\tstd::unique_ptr<CameraConfiguration> config =\n+\t\tstd::make_unique<UVCCameraConfiguration>(data);\n \n \tif (roles.empty())\n \t\treturn config;\ndiff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\nindex f85d05f77a61..204f5ad73f6d 100644\n--- a/src/libcamera/pipeline/vimc/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc/vimc.cpp\n@@ -84,7 +84,7 @@ class PipelineHandlerVimc : public PipelineHandler\n public:\n \tPipelineHandlerVimc(CameraManager *manager);\n \n-\tCameraConfiguration *generateConfiguration(Camera *camera,\n+\tstd::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n \t\tconst StreamRoles &roles) override;\n \tint configure(Camera *camera, CameraConfiguration *config) override;\n \n@@ -189,11 +189,13 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager)\n {\n }\n \n-CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera,\n+std::unique_ptr<CameraConfiguration>\n+PipelineHandlerVimc::generateConfiguration(Camera *camera,\n \tconst StreamRoles &roles)\n {\n \tVimcCameraData *data = cameraData(camera);\n-\tCameraConfiguration *config = new VimcCameraConfiguration(data);\n+\tstd::unique_ptr<CameraConfiguration> config =\n+\t\tstd::make_unique<VimcCameraConfiguration>(data);\n \n \tif (roles.empty())\n \t\treturn config;\ndiff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\nindex 588a3db30e82..825aff5ac20a 100644\n--- a/src/libcamera/pipeline_handler.cpp\n+++ b/src/libcamera/pipeline_handler.cpp\n@@ -233,8 +233,7 @@ void PipelineHandler::unlockMediaDevices()\n * handler.\n *\n * \\return A valid CameraConfiguration if the requested roles can be satisfied,\n- * or a null pointer otherwise. The ownership of the returned configuration is\n- * passed to the caller.\n+ * or a null pointer otherwise.\n */\n \n /**\n", "prefixes": [ "libcamera-devel", "v3", "08/13" ] }