[{"id":25533,"web_url":"https://patchwork.libcamera.org/comment/25533/","msgid":"<20221024080104.GC3874866@pyrite.rasen.tech>","date":"2022-10-24T08:01:04","subject":"Re: [libcamera-devel] [PATCH v3 08/13] libcamera: pipeline_handler:\n\tReturn unique_ptr from generateConfiguration()","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"On Mon, Oct 24, 2022 at 03:03:51AM +0300, Laurent Pinchart wrote:\n> The PipelineHandler::generateConfiguration() function allocates a\n> CameraConfiguration instance and returns it. The ownership of the\n> instance is transferred to the caller. This is a perfect match for a\n> std::unique_ptr<>, which the Camera::generateConfiguration() function\n> already returns. Update PipelineHandler::generateConfiguration() to\n> match it. This fixes a memory leak in one of the error return paths in\n> the IPU3 pipeline handler.\n> \n> While at it, update the Camera::generateConfiguration() function\n> documentation to drop the sentence that describes the ownership\n> transfer, as that is implied by usage of std::unique_ptr<>.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\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(-)\n> \n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index 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>  \n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 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>  /**\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 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>  \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 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>  \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 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>  \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 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;\n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 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;\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 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;\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 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> -- \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 56FECBD16B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Oct 2022 08:01:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7CFF362EF8;\n\tMon, 24 Oct 2022 10:01:14 +0200 (CEST)","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 48EDF62EBC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Oct 2022 10:01:13 +0200 (CEST)","from pyrite.rasen.tech (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7DF27471;\n\tMon, 24 Oct 2022 10:01:11 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666598474;\n\tbh=hFfuF+5GZcZiTmlNYIDHmC8KwVxOJSxG3WxVq9xPtGw=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=rPJGDj9MYA3K/lYQ69s1qyyNJFrmx22QaETEnkvXEHfCGGkCMmR6BKm6XscByWlKm\n\t7fJgdBdcZd7olFFDKn9UVGcAAE4xYStTuuHo4HxzH4WJQ/31QaYYJdvJB2MS0rH5aN\n\to9BSicUP04JhQ0/HQ21IIc6LXv8pD5bH2pI2BMQakHHRqWVTSFhjPAxMnfqHHMsD7F\n\to6X7iWeLueWE6SWRIIEFYrHa1XTY7/C3RkKmnufYBPinV/GRPfUD/4s0Oynd8vmjO5\n\tYjdEnl5YPc1lkP4hVBURPNRcxkrMQxYVK1l1J7F+95baaasQbtXb9roSn25FPMTz8p\n\tmhzbnYEc0gs4Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1666598472;\n\tbh=hFfuF+5GZcZiTmlNYIDHmC8KwVxOJSxG3WxVq9xPtGw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ue32isjdZDvSl6KIk+HDDQ/mVErGFGqHMOVtj2E4x58eQYzHqObtyt+IEzzTH9IPz\n\t/8iCvdBu0nmdYaciREhtbdfY5LqxbtwgqllRbSDJp03dmO6P+qIIXcl9KuWympDgO2\n\tb2tqzTm0nFxL86c8ts8gZQgFZ/oYVkIpWhbVhx84="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ue32isjd\"; dkim-atps=neutral","Date":"Mon, 24 Oct 2022 17:01:04 +0900","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20221024080104.GC3874866@pyrite.rasen.tech>","References":"<20221024000356.29521-1-laurent.pinchart@ideasonboard.com>\n\t<20221024000356.29521-9-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20221024000356.29521-9-laurent.pinchart@ideasonboard.com>","Subject":"Re: [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":"Paul Elder via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"paul.elder@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":25604,"web_url":"https://patchwork.libcamera.org/comment/25604/","msgid":"<20221026151859.nsnl33w3aov7qkqv@uno.localdomain>","date":"2022-10-26T15:18:59","subject":"Re: [libcamera-devel] [PATCH v3 08/13] libcamera: pipeline_handler:\n\tReturn unique_ptr from generateConfiguration()","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Mon, Oct 24, 2022 at 03:03:51AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> The PipelineHandler::generateConfiguration() function allocates a\n> CameraConfiguration instance and returns it. The ownership of the\n> instance is transferred to the caller. This is a perfect match for a\n> std::unique_ptr<>, which the Camera::generateConfiguration() function\n> already returns. Update PipelineHandler::generateConfiguration() to\n> match it. This fixes a memory leak in one of the error return paths in\n> the IPU3 pipeline handler.\n\nGreat!\n\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n  j\n\n>\n> While at it, update the Camera::generateConfiguration() function\n> documentation to drop the sentence that describes the ownership\n> transfer, as that is implied by usage of std::unique_ptr<>.\n>\n> Signed-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(-)\n>\n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index 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>\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 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>  /**\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 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>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 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>\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 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>\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 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;\n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 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;\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 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;\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 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> --\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 0027BBD16B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 26 Oct 2022 15:19:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 47FC262F5A;\n\tWed, 26 Oct 2022 17:19:02 +0200 (CEST)","from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::222])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A386961F4B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 26 Oct 2022 17:19:01 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 2B6944000C;\n\tWed, 26 Oct 2022 15:19:00 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666797542;\n\tbh=Fel5Q9+cB4RMwsG7r77hsCCvfSURukYL1kmXqepPIUY=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=269PD1wD2sjFqI/m5fVcHIxgxMGsp/tW9vGsXNkXYjVEEneGYEG9beiKjjwHwqkMj\n\t5A/nwR1iJZDIVoaDSVsE2XyV0xZTw7QXP3tdmWnN2bspkD0vOT8jgoXTQbdhMEy+WR\n\tX51Rz/CdgjO13K7V/mMf8MDzak5mFcScRP7xr1d1tO41Z9A1cd85b1k1ku0Yl9z1fm\n\t8yxv1bIs3x/ZjuGFwY7JRfnGrlTe/n4duMsnw1zRotMoY1eMLzP5vlUd0A65qlSo97\n\t624Se45/M0OPOxDeK9b+CizkzSluAO+LI2cUH9oHeUm8SudkPP4e7M+CZWUYoD6rTL\n\tXNtM+Zf7MaL/Q==","Date":"Wed, 26 Oct 2022 17:18:59 +0200","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20221026151859.nsnl33w3aov7qkqv@uno.localdomain>","References":"<20221024000356.29521-1-laurent.pinchart@ideasonboard.com>\n\t<20221024000356.29521-9-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221024000356.29521-9-laurent.pinchart@ideasonboard.com>","Subject":"Re: [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":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":25676,"web_url":"https://patchwork.libcamera.org/comment/25676/","msgid":"<166699474826.404886.1307523535577486529@Monstersaurus>","date":"2022-10-28T22:05:48","subject":"Re: [libcamera-devel] [PATCH v3 08/13] libcamera: pipeline_handler:\n\tReturn unique_ptr from generateConfiguration()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart via libcamera-devel (2022-10-24 01:03:51)\n> The PipelineHandler::generateConfiguration() function allocates a\n> CameraConfiguration instance and returns it. The ownership of the\n> instance is transferred to the caller. This is a perfect match for a\n> std::unique_ptr<>, which the Camera::generateConfiguration() function\n> already returns. Update PipelineHandler::generateConfiguration() to\n> match it. This fixes a memory leak in one of the error return paths in\n> the IPU3 pipeline handler.\n> \n> While at it, update the Camera::generateConfiguration() function\n> documentation to drop the sentence that describes the ownership\n> transfer, as that is implied by usage of std::unique_ptr<>.\n> \n> Signed-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(-)\n> \n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index 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>         bool acquire();\n>         void release();\n>  \n> -       virtual CameraConfiguration *generateConfiguration(Camera *camera,\n> +       virtual std::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n>                 const StreamRoles &roles) = 0;\n>         virtual int configure(Camera *camera, CameraConfiguration *config) = 0;\n>  \n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 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>         if (roles.size() > streams().size())\n>                 return nullptr;\n>  \n> -       CameraConfiguration *config = d->pipe_->generateConfiguration(this, roles);\n> +       std::unique_ptr<CameraConfiguration> config =\n> +               d->pipe_->generateConfiguration(this, roles);\n>         if (!config) {\n>                 LOG(Camera, Debug)\n>                         << \"Pipeline handler failed to generate configuration\";\n> @@ -966,7 +966,7 @@ std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamR\n>  \n>         LOG(Camera, Debug) << msg.str();\n>  \n> -       return std::unique_ptr<CameraConfiguration>(config);\n> +       return config;\n\nOk - so this really makes sense.!\n\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n>  }\n>  \n>  /**\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 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>         PipelineHandlerIPU3(CameraManager *manager);\n>  \n> -       CameraConfiguration *generateConfiguration(Camera *camera,\n> +       std::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n>                 const StreamRoles &roles) override;\n>         int 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> -                                                               const StreamRoles &roles)\n> +std::unique_ptr<CameraConfiguration>\n> +PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &roles)\n>  {\n>         IPU3CameraData *data = cameraData(camera);\n> -       IPU3CameraConfiguration *config = new IPU3CameraConfiguration(data);\n> +       std::unique_ptr<IPU3CameraConfiguration> config =\n> +               std::make_unique<IPU3CameraConfiguration>(data);\n>  \n>         if (roles.empty())\n>                 return config;\n> @@ -494,7 +495,6 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,\n>                 default:\n>                         LOG(IPU3, Error)\n>                                 << \"Requested stream role not supported: \" << role;\n> -                       delete config;\n>                         return nullptr;\n>                 }\n>  \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 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>         PipelineHandlerRPi(CameraManager *manager);\n>  \n> -       CameraConfiguration *generateConfiguration(Camera *camera, const StreamRoles &roles) override;\n> +       std::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n> +               const StreamRoles &roles) override;\n>         int configure(Camera *camera, CameraConfiguration *config) override;\n>  \n>         int exportFrameBuffers(Camera *camera, Stream *stream,\n> @@ -561,11 +562,12 @@ PipelineHandlerRPi::PipelineHandlerRPi(CameraManager *manager)\n>  {\n>  }\n>  \n> -CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> -                                                              const StreamRoles &roles)\n> +std::unique_ptr<CameraConfiguration>\n> +PipelineHandlerRPi::generateConfiguration(Camera *camera, const StreamRoles &roles)\n>  {\n>         RPiCameraData *data = cameraData(camera);\n> -       CameraConfiguration *config = new RPiCameraConfiguration(data);\n> +       std::unique_ptr<CameraConfiguration> config =\n> +               std::make_unique<RPiCameraConfiguration>(data);\n>         V4L2SubdeviceFormat sensorFormat;\n>         unsigned int bufferCount;\n>         PixelFormat pixelFormat;\n> @@ -640,13 +642,11 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n>                 default:\n>                         LOG(RPI, Error) << \"Requested stream role not supported: \"\n>                                         << role;\n> -                       delete config;\n>                         return nullptr;\n>                 }\n>  \n>                 if (rawCount > 1 || outCount > 2) {\n>                         LOG(RPI, Error) << \"Invalid stream roles requested\";\n> -                       delete config;\n>                         return nullptr;\n>                 }\n>  \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 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>         PipelineHandlerRkISP1(CameraManager *manager);\n>  \n> -       CameraConfiguration *generateConfiguration(Camera *camera,\n> +       std::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n>                 const StreamRoles &roles) override;\n>         int 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>         const StreamRoles &roles)\n>  {\n>         RkISP1CameraData *data = cameraData(camera);\n> @@ -550,7 +551,8 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n>                 return nullptr;\n>         }\n>  \n> -       CameraConfiguration *config = new RkISP1CameraConfiguration(camera, data);\n> +       std::unique_ptr<CameraConfiguration> config =\n> +               std::make_unique<RkISP1CameraConfiguration>(camera, data);\n>         if (roles.empty())\n>                 return config;\n>  \n> @@ -595,7 +597,6 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n>                 default:\n>                         LOG(RkISP1, Warning)\n>                                 << \"Requested stream role not supported: \" << role;\n> -                       delete config;\n>                         return nullptr;\n>                 }\n>  \n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 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>         SimplePipelineHandler(CameraManager *manager);\n>  \n> -       CameraConfiguration *generateConfiguration(Camera *camera,\n> -                                                  const StreamRoles &roles) override;\n> +       std::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n> +               const StreamRoles &roles) override;\n>         int configure(Camera *camera, CameraConfiguration *config) override;\n>  \n>         int exportFrameBuffers(Camera *camera, Stream *stream,\n> @@ -1037,12 +1037,12 @@ SimplePipelineHandler::SimplePipelineHandler(CameraManager *manager)\n>  {\n>  }\n>  \n> -CameraConfiguration *SimplePipelineHandler::generateConfiguration(Camera *camera,\n> -                                                                 const StreamRoles &roles)\n> +std::unique_ptr<CameraConfiguration>\n> +SimplePipelineHandler::generateConfiguration(Camera *camera, const StreamRoles &roles)\n>  {\n>         SimpleCameraData *data = cameraData(camera);\n> -       CameraConfiguration *config =\n> -               new SimpleCameraConfiguration(camera, data);\n> +       std::unique_ptr<CameraConfiguration> config =\n> +               std::make_unique<SimpleCameraConfiguration>(camera, data);\n>  \n>         if (roles.empty())\n>                 return config;\n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 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>         PipelineHandlerUVC(CameraManager *manager);\n>  \n> -       CameraConfiguration *generateConfiguration(Camera *camera,\n> +       std::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n>                 const StreamRoles &roles) override;\n>         int 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>         const StreamRoles &roles)\n>  {\n>         UVCCameraData *data = cameraData(camera);\n> -       CameraConfiguration *config = new UVCCameraConfiguration(data);\n> +       std::unique_ptr<CameraConfiguration> config =\n> +               std::make_unique<UVCCameraConfiguration>(data);\n>  \n>         if (roles.empty())\n>                 return config;\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index 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>         PipelineHandlerVimc(CameraManager *manager);\n>  \n> -       CameraConfiguration *generateConfiguration(Camera *camera,\n> +       std::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,\n>                 const StreamRoles &roles) override;\n>         int 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>         const StreamRoles &roles)\n>  {\n>         VimcCameraData *data = cameraData(camera);\n> -       CameraConfiguration *config = new VimcCameraConfiguration(data);\n> +       std::unique_ptr<CameraConfiguration> config =\n> +               std::make_unique<VimcCameraConfiguration>(data);\n>  \n>         if (roles.empty())\n>                 return config;\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 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> -- \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 ECB76BD16B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 28 Oct 2022 22:05:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 54BAE62F89;\n\tSat, 29 Oct 2022 00:05:53 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 11F6562F89\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 29 Oct 2022 00:05:51 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 87BEF440;\n\tSat, 29 Oct 2022 00:05:50 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666994753;\n\tbh=uUToxsdJlmWVl7QQU85pFrN80GiDdynKmEhNPIMIELc=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=jNof/Qjeucvq6aEAUaAO9McHGOboUedgFZpKpGrJAqdW+mhDira4gRZYENTxN02V2\n\t3AsEVeC9nrpEgnpnnUVH5crXo+e+qds858AE3SWxYB/x61Q4Wu+V7xf8rQeR3LKwZw\n\tX3VRKyANCf9CrTbYyheF5ztH3a7fqroVSTwy7A5NKE3YgGvlsClLaGBQa1wNt7MdZQ\n\tilAqZi+997+/k2YMKQlQrvKgfqyvsjyHg1R574NDximxns2B/b4Tef4uCqt0GeMZpV\n\t4rLxZcAlGuc1EHfQNV8uXKrRv3zJlPDCgXh2Hp79w8mvs6eMSj2Efemfl2hoUqGYz2\n\tUD3LXwzfIw3ag==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1666994750;\n\tbh=uUToxsdJlmWVl7QQU85pFrN80GiDdynKmEhNPIMIELc=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=izjnkquvQaaJjJiFKOOmXsDhA07iiICzv+UCQ9t4hgPse5UR5hOAsJ70yR/YrfGdl\n\tzmqKEGA5un4ykBu3bMcm1PLJeg+U4CwGW/yvCmwogCiOi2zL8GnWBn6QlOSrh/DMFy\n\tHWdWbcLKML6cs5nDEwjBuiAUWHSNLxh2Z4DiEJXs="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"izjnkquv\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20221024000356.29521-9-laurent.pinchart@ideasonboard.com>","References":"<20221024000356.29521-1-laurent.pinchart@ideasonboard.com>\n\t<20221024000356.29521-9-laurent.pinchart@ideasonboard.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 28 Oct 2022 23:05:48 +0100","Message-ID":"<166699474826.404886.1307523535577486529@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [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":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]