Patch Detail
Show a patch.
GET /api/patches/1247/?format=api
{ "id": 1247, "url": "https://patchwork.libcamera.org/api/patches/1247/?format=api", "web_url": "https://patchwork.libcamera.org/patch/1247/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/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": "<20190521192740.28112-5-laurent.pinchart@ideasonboard.com>", "date": "2019-05-21T19:27:38", "name": "[libcamera-devel,v3,4/6] libcamera: camera: Return a pointer from generateConfiguration()", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "82984b1256229ac97746a744c7ad4313debe9a16", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/1247/mbox/", "series": [ { "id": 313, "url": "https://patchwork.libcamera.org/api/series/313/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=313", "date": "2019-05-21T19:27:34", "name": "Rework camera configuration to introduce negotiation of parameters", "version": 3, "mbox": "https://patchwork.libcamera.org/series/313/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/1247/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/1247/checks/", "tags": {}, "headers": { "Return-Path": "<laurent.pinchart@ideasonboard.com>", "Received": [ "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E315B60E4A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 21 May 2019 21:28:04 +0200 (CEST)", "from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1C72F52C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 21 May 2019 21:28:04 +0200 (CEST)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1558466884;\n\tbh=QW83BWw3knb8zhdnVSNSlcZ4egd8D/TFBO33GoK2YPw=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=JJYg6fGHhVtmTwN+5ucpN+1jQB3VID+b9eZAMJEavL9wr5+QtnMROpQJMyY+5PLih\n\tBcWaWh7QRX1JdCckvOVNiAZHNABxL/OG6zfIuthrpFoUz2sCWnROPJ2y3c+fHtd5L6\n\tYO5M73REpFAqz7sbXw9MMtqmhnyEsT4rohF9KBz4=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Tue, 21 May 2019 22:27:38 +0300", "Message-Id": "<20190521192740.28112-5-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.21.0", "In-Reply-To": "<20190521192740.28112-1-laurent.pinchart@ideasonboard.com>", "References": "<20190521192740.28112-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=UTF-8", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v3 4/6] libcamera: camera: Return a\n\tpointer from generateConfiguration()", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.23", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "X-List-Received-Date": "Tue, 21 May 2019 19:28:05 -0000" }, "content": "To prepare for specialising the CameraConfiguration class in pipeline\nhandlers, return a pointer to a camera configuration instead of a\nreference from Camera::generateConfiguration(). The camera configuration\nalways needs to be allocated from the pipeline handler, and its\nownership is passed to the application.\n\nFor symmetry, change Camera::configure() to take a CameraConfiguration\npointer instead of a reference. This aligns with our coding practice of\npassing parameters that are modified by the callee by pointer.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n---\nChanges since v2:\n\n- Return an std::unique_ptr<> from Camera::generateConfiguration()\n- Document that Camera::generateConfiguration() can take an empty list\n of roles\n---\n include/libcamera/camera.h | 4 +--\n src/cam/main.cpp | 38 +++++++++++-------------\n src/libcamera/camera.cpp | 37 +++++++++++++----------\n src/libcamera/include/pipeline_handler.h | 6 ++--\n src/libcamera/pipeline/ipu3/ipu3.cpp | 31 +++++++++----------\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 19 +++++++-----\n src/libcamera/pipeline/uvcvideo.cpp | 24 ++++++++-------\n src/libcamera/pipeline/vimc.cpp | 24 ++++++++-------\n src/libcamera/pipeline_handler.cpp | 3 +-\n src/qcam/main_window.cpp | 6 ++--\n src/qcam/main_window.h | 3 +-\n test/camera/capture.cpp | 32 ++++++++++++++------\n test/camera/configuration_default.cpp | 8 ++---\n test/camera/configuration_set.cpp | 38 ++++++++++++++++--------\n test/camera/statemachine.cpp | 30 +++++++++++++------\n 15 files changed, 178 insertions(+), 125 deletions(-)", "diff": "diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\nindex 284e5276a055..a3a7289a7aa7 100644\n--- a/include/libcamera/camera.h\n+++ b/include/libcamera/camera.h\n@@ -77,8 +77,8 @@ public:\n \tint release();\n \n \tconst std::set<Stream *> &streams() const;\n-\tCameraConfiguration generateConfiguration(const StreamRoles &roles);\n-\tint configure(CameraConfiguration &config);\n+\tstd::unique_ptr<CameraConfiguration> generateConfiguration(const StreamRoles &roles);\n+\tint configure(CameraConfiguration *config);\n \n \tint allocateBuffers();\n \tint freeBuffers();\ndiff --git a/src/cam/main.cpp b/src/cam/main.cpp\nindex cd165bea34cd..535c2420893e 100644\n--- a/src/cam/main.cpp\n+++ b/src/cam/main.cpp\n@@ -85,15 +85,13 @@ static int parseOptions(int argc, char *argv[])\n \treturn 0;\n }\n \n-static int prepareCameraConfig(CameraConfiguration *config)\n+static std::unique_ptr<CameraConfiguration> prepareCameraConfig()\n {\n \tStreamRoles roles;\n \n \t/* If no configuration is provided assume a single video stream. */\n-\tif (!options.isSet(OptStream)) {\n-\t\t*config = camera->generateConfiguration({ StreamRole::VideoRecording });\n-\t\treturn 0;\n-\t}\n+\tif (!options.isSet(OptStream))\n+\t\treturn camera->generateConfiguration({ StreamRole::VideoRecording });\n \n \tconst std::vector<OptionValue> &streamOptions =\n \t\toptions[OptStream].toArray();\n@@ -113,23 +111,22 @@ static int prepareCameraConfig(CameraConfiguration *config)\n \t\t} else {\n \t\t\tstd::cerr << \"Unknown stream role \"\n \t\t\t\t << conf[\"role\"].toString() << std::endl;\n-\t\t\treturn -EINVAL;\n+\t\t\treturn nullptr;\n \t\t}\n \t}\n \n-\t*config = camera->generateConfiguration(roles);\n-\n-\tif (!config->isValid()) {\n+\tstd::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles);\n+\tif (!config || !config->isValid()) {\n \t\tstd::cerr << \"Failed to get default stream configuration\"\n \t\t\t << std::endl;\n-\t\treturn -EINVAL;\n+\t\treturn nullptr;\n \t}\n \n \t/* Apply configuration explicitly requested. */\n \tunsigned int i = 0;\n \tfor (auto const &value : streamOptions) {\n \t\tKeyValueParser::Options conf = value.toKeyValues();\n-\t\tStreamConfiguration &cfg = (*config)[i++];\n+\t\tStreamConfiguration &cfg = config->at(i++);\n \n \t\tif (conf.isSet(\"width\"))\n \t\t\tcfg.size.width = conf[\"width\"];\n@@ -142,7 +139,7 @@ static int prepareCameraConfig(CameraConfiguration *config)\n \t\t\tcfg.pixelFormat = conf[\"pixelformat\"];\n \t}\n \n-\treturn 0;\n+\treturn config;\n }\n \n static void requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers)\n@@ -191,16 +188,15 @@ static void requestComplete(Request *request, const std::map<Stream *, Buffer *>\n \n static int capture()\n {\n-\tCameraConfiguration config;\n \tint ret;\n \n-\tret = prepareCameraConfig(&config);\n-\tif (ret) {\n+\tstd::unique_ptr<CameraConfiguration> config = prepareCameraConfig();\n+\tif (!config) {\n \t\tstd::cout << \"Failed to prepare camera configuration\" << std::endl;\n-\t\treturn ret;\n+\t\treturn -EINVAL;\n \t}\n \n-\tret = camera->configure(config);\n+\tret = camera->configure(config.get());\n \tif (ret < 0) {\n \t\tstd::cout << \"Failed to configure camera\" << std::endl;\n \t\treturn ret;\n@@ -208,8 +204,8 @@ static int capture()\n \n \tstreamInfo.clear();\n \n-\tfor (unsigned int index = 0; index < config.size(); ++index) {\n-\t\tStreamConfiguration &cfg = config[index];\n+\tfor (unsigned int index = 0; index < config->size(); ++index) {\n+\t\tStreamConfiguration &cfg = config->at(index);\n \t\tstreamInfo[cfg.stream()] = \"stream\" + std::to_string(index);\n \t}\n \n@@ -224,7 +220,7 @@ static int capture()\n \n \t/* Identify the stream with the least number of buffers. */\n \tunsigned int nbuffers = UINT_MAX;\n-\tfor (StreamConfiguration &cfg : config) {\n+\tfor (StreamConfiguration &cfg : *config) {\n \t\tStream *stream = cfg.stream();\n \t\tnbuffers = std::min(nbuffers, stream->bufferPool().count());\n \t}\n@@ -244,7 +240,7 @@ static int capture()\n \t\t}\n \n \t\tstd::map<Stream *, Buffer *> map;\n-\t\tfor (StreamConfiguration &cfg : config) {\n+\t\tfor (StreamConfiguration &cfg : *config) {\n \t\t\tStream *stream = cfg.stream();\n \t\t\tmap[stream] = &stream->bufferPool().buffers()[i];\n \t\t}\ndiff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\nindex 5848330f639a..0e5fd7f57271 100644\n--- a/src/libcamera/camera.cpp\n+++ b/src/libcamera/camera.cpp\n@@ -540,27 +540,32 @@ const std::set<Stream *> &Camera::streams() const\n *\n * Generate a camera configuration for a set of desired stream roles. The caller\n * specifies a list of stream roles and the camera returns a configuration\n- * containing suitable streams and their suggested default configurations.\n+ * containing suitable streams and their suggested default configurations. An\n+ * empty list of roles is valid, and will generate an empty configuration that\n+ * can be filled by the caller.\n *\n- * \\return A valid CameraConfiguration if the requested roles can be satisfied,\n- * or a invalid one otherwise\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 */\n-CameraConfiguration\n-Camera::generateConfiguration(const StreamRoles &roles)\n+std::unique_ptr<CameraConfiguration> Camera::generateConfiguration(const StreamRoles &roles)\n {\n-\tif (disconnected_ || !roles.size() || roles.size() > streams_.size())\n-\t\treturn CameraConfiguration();\n+\tif (disconnected_ || roles.size() > streams_.size())\n+\t\treturn nullptr;\n \n-\tCameraConfiguration config = pipe_->generateConfiguration(this, roles);\n+\tCameraConfiguration *config = pipe_->generateConfiguration(this, roles);\n \n \tstd::ostringstream msg(\"streams configuration:\", std::ios_base::ate);\n \n-\tfor (unsigned int index = 0; index < config.size(); ++index)\n-\t\tmsg << \" (\" << index << \") \" << config[index].toString();\n+\tif (config->empty())\n+\t\tmsg << \" empty\";\n+\n+\tfor (unsigned int index = 0; index < config->size(); ++index)\n+\t\tmsg << \" (\" << index << \") \" << config->at(index).toString();\n \n \tLOG(Camera, Debug) << msg.str();\n \n-\treturn config;\n+\treturn std::unique_ptr<CameraConfiguration>(config);\n }\n \n /**\n@@ -590,7 +595,7 @@ Camera::generateConfiguration(const StreamRoles &roles)\n * \\retval -EACCES The camera is not in a state where it can be configured\n * \\retval -EINVAL The configuration is not valid\n */\n-int Camera::configure(CameraConfiguration &config)\n+int Camera::configure(CameraConfiguration *config)\n {\n \tint ret;\n \n@@ -600,7 +605,7 @@ int Camera::configure(CameraConfiguration &config)\n \tif (!stateBetween(CameraAcquired, CameraConfigured))\n \t\treturn -EACCES;\n \n-\tif (!config.isValid()) {\n+\tif (!config->isValid()) {\n \t\tLOG(Camera, Error)\n \t\t\t<< \"Can't configure camera with invalid configuration\";\n \t\treturn -EINVAL;\n@@ -608,8 +613,8 @@ int Camera::configure(CameraConfiguration &config)\n \n \tstd::ostringstream msg(\"configuring streams:\", std::ios_base::ate);\n \n-\tfor (unsigned int index = 0; index < config.size(); ++index) {\n-\t\tStreamConfiguration &cfg = config[index];\n+\tfor (unsigned int index = 0; index < config->size(); ++index) {\n+\t\tStreamConfiguration &cfg = config->at(index);\n \t\tcfg.setStream(nullptr);\n \t\tmsg << \" (\" << index << \") \" << cfg.toString();\n \t}\n@@ -621,7 +626,7 @@ int Camera::configure(CameraConfiguration &config)\n \t\treturn ret;\n \n \tactiveStreams_.clear();\n-\tfor (const StreamConfiguration &cfg : config) {\n+\tfor (const StreamConfiguration &cfg : *config) {\n \t\tStream *stream = cfg.stream();\n \t\tif (!stream)\n \t\t\tLOG(Camera, Fatal)\ndiff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\nindex a025905ab68f..7da6df1ab2a0 100644\n--- a/src/libcamera/include/pipeline_handler.h\n+++ b/src/libcamera/include/pipeline_handler.h\n@@ -60,9 +60,9 @@ public:\n \tbool lock();\n \tvoid unlock();\n \n-\tvirtual CameraConfiguration\n-\tgenerateConfiguration(Camera *camera, const StreamRoles &roles) = 0;\n-\tvirtual int configure(Camera *camera, CameraConfiguration &config) = 0;\n+\tvirtual CameraConfiguration *generateConfiguration(Camera *camera,\n+\t\tconst StreamRoles &roles) = 0;\n+\tvirtual int configure(Camera *camera, CameraConfiguration *config) = 0;\n \n \tvirtual int allocateBuffers(Camera *camera,\n \t\t\t\t const std::set<Stream *> &streams) = 0;\ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex ed0ef69de1d1..3acf82ff4665 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -150,9 +150,9 @@ class PipelineHandlerIPU3 : public PipelineHandler\n public:\n \tPipelineHandlerIPU3(CameraManager *manager);\n \n-\tCameraConfiguration\n-\tgenerateConfiguration(Camera *camera, const StreamRoles &roles) override;\n-\tint configure(Camera *camera, CameraConfiguration &config) override;\n+\tCameraConfiguration *generateConfiguration(Camera *camera,\n+\t\tconst StreamRoles &roles) override;\n+\tint configure(Camera *camera, CameraConfiguration *config) override;\n \n \tint allocateBuffers(Camera *camera,\n \t\t\t const std::set<Stream *> &streams) override;\n@@ -207,12 +207,11 @@ PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager)\n {\n }\n \n-CameraConfiguration\n-PipelineHandlerIPU3::generateConfiguration(Camera *camera,\n-\t\t\t\t\t const StreamRoles &roles)\n+CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,\n+\tconst StreamRoles &roles)\n {\n \tIPU3CameraData *data = cameraData(camera);\n-\tCameraConfiguration config = {};\n+\tCameraConfiguration *config = new CameraConfiguration();\n \tstd::set<IPU3Stream *> streams = {\n \t\t&data->outStream_,\n \t\t&data->vfStream_,\n@@ -290,21 +289,23 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera,\n \t\t\tbreak;\n \t\t}\n \n-\t\tif (!stream)\n-\t\t\treturn CameraConfiguration{};\n+\t\tif (!stream) {\n+\t\t\tdelete config;\n+\t\t\treturn nullptr;\n+\t\t}\n \n \t\tstreams.erase(stream);\n \n \t\tcfg.pixelFormat = V4L2_PIX_FMT_NV12;\n \t\tcfg.bufferCount = IPU3_BUFFER_COUNT;\n \n-\t\tconfig.addConfiguration(cfg);\n+\t\tconfig->addConfiguration(cfg);\n \t}\n \n \treturn config;\n }\n \n-int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config)\n+int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *config)\n {\n \tIPU3CameraData *data = cameraData(camera);\n \tIPU3Stream *outStream = &data->outStream_;\n@@ -316,7 +317,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config)\n \n \toutStream->active_ = false;\n \tvfStream->active_ = false;\n-\tfor (StreamConfiguration &cfg : config) {\n+\tfor (StreamConfiguration &cfg : *config) {\n \t\t/*\n \t\t * Pick a stream for the configuration entry.\n \t\t * \\todo: This is a naive temporary implementation that will be\n@@ -382,7 +383,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config)\n \t\treturn ret;\n \n \t/* Apply the format to the configured streams output devices. */\n-\tfor (StreamConfiguration &cfg : config) {\n+\tfor (StreamConfiguration &cfg : *config) {\n \t\tIPU3Stream *stream = static_cast<IPU3Stream *>(cfg.stream());\n \t\tret = imgu->configureOutput(stream->device_, cfg);\n \t\tif (ret)\n@@ -395,13 +396,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config)\n \t * be at least one active stream in the configuration request).\n \t */\n \tif (!outStream->active_) {\n-\t\tret = imgu->configureOutput(outStream->device_, config[0]);\n+\t\tret = imgu->configureOutput(outStream->device_, config->at(0));\n \t\tif (ret)\n \t\t\treturn ret;\n \t}\n \n \tif (!vfStream->active_) {\n-\t\tret = imgu->configureOutput(vfStream->device_, config[0]);\n+\t\tret = imgu->configureOutput(vfStream->device_, config->at(0));\n \t\tif (ret)\n \t\t\treturn ret;\n \t}\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex ec6980b0943a..c8d217abdca8 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -34,9 +34,9 @@ public:\n \tPipelineHandlerRkISP1(CameraManager *manager);\n \t~PipelineHandlerRkISP1();\n \n-\tCameraConfiguration generateConfiguration(Camera *camera,\n+\tCameraConfiguration *generateConfiguration(Camera *camera,\n \t\tconst StreamRoles &roles) override;\n-\tint configure(Camera *camera, CameraConfiguration &config) override;\n+\tint configure(Camera *camera, CameraConfiguration *config) override;\n \n \tint allocateBuffers(Camera *camera,\n \t\tconst std::set<Stream *> &streams) override;\n@@ -105,26 +105,29 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1()\n * Pipeline Operations\n */\n \n-CameraConfiguration PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n+CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n \tconst StreamRoles &roles)\n {\n \tRkISP1CameraData *data = cameraData(camera);\n-\tCameraConfiguration config;\n+\tCameraConfiguration *config = new CameraConfiguration();\n+\n+\tif (roles.empty())\n+\t\treturn config;\n+\n \tStreamConfiguration cfg{};\n-\n \tcfg.pixelFormat = V4L2_PIX_FMT_NV12;\n \tcfg.size = data->sensor_->resolution();\n \tcfg.bufferCount = RKISP1_BUFFER_COUNT;\n \n-\tconfig.addConfiguration(cfg);\n+\tconfig->addConfiguration(cfg);\n \n \treturn config;\n }\n \n-int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration &config)\n+int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *config)\n {\n \tRkISP1CameraData *data = cameraData(camera);\n-\tStreamConfiguration &cfg = config[0];\n+\tStreamConfiguration &cfg = config->at(0);\n \tCameraSensor *sensor = data->sensor_;\n \tint ret;\n \ndiff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\nindex 5dcc868b2fc9..120d8d3a1522 100644\n--- a/src/libcamera/pipeline/uvcvideo.cpp\n+++ b/src/libcamera/pipeline/uvcvideo.cpp\n@@ -25,9 +25,9 @@ class PipelineHandlerUVC : public PipelineHandler\n public:\n \tPipelineHandlerUVC(CameraManager *manager);\n \n-\tCameraConfiguration\n-\tgenerateConfiguration(Camera *camera, const StreamRoles &roles) override;\n-\tint configure(Camera *camera, CameraConfiguration &config) override;\n+\tCameraConfiguration *generateConfiguration(Camera *camera,\n+\t\tconst StreamRoles &roles) override;\n+\tint configure(Camera *camera, CameraConfiguration *config) override;\n \n \tint allocateBuffers(Camera *camera,\n \t\t\t const std::set<Stream *> &streams) override;\n@@ -73,26 +73,28 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)\n {\n }\n \n-CameraConfiguration\n-PipelineHandlerUVC::generateConfiguration(Camera *camera,\n-\t\t\t\t\t const StreamRoles &roles)\n+CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,\n+\tconst StreamRoles &roles)\n {\n-\tCameraConfiguration config;\n-\tStreamConfiguration cfg;\n+\tCameraConfiguration *config = new CameraConfiguration();\n \n+\tif (roles.empty())\n+\t\treturn config;\n+\n+\tStreamConfiguration cfg{};\n \tcfg.pixelFormat = V4L2_PIX_FMT_YUYV;\n \tcfg.size = { 640, 480 };\n \tcfg.bufferCount = 4;\n \n-\tconfig.addConfiguration(cfg);\n+\tconfig->addConfiguration(cfg);\n \n \treturn config;\n }\n \n-int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration &config)\n+int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)\n {\n \tUVCCameraData *data = cameraData(camera);\n-\tStreamConfiguration &cfg = config[0];\n+\tStreamConfiguration &cfg = config->at(0);\n \tint ret;\n \n \tV4L2DeviceFormat format = {};\ndiff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\nindex af6b6f21e3c5..f6aa32683e5e 100644\n--- a/src/libcamera/pipeline/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc.cpp\n@@ -25,9 +25,9 @@ class PipelineHandlerVimc : public PipelineHandler\n public:\n \tPipelineHandlerVimc(CameraManager *manager);\n \n-\tCameraConfiguration\n-\tgenerateConfiguration(Camera *camera, const StreamRoles &roles) override;\n-\tint configure(Camera *camera, CameraConfiguration &config) override;\n+\tCameraConfiguration *generateConfiguration(Camera *camera,\n+\t\tconst StreamRoles &roles) override;\n+\tint configure(Camera *camera, CameraConfiguration *config) override;\n \n \tint allocateBuffers(Camera *camera,\n \t\t\t const std::set<Stream *> &streams) override;\n@@ -73,26 +73,28 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager)\n {\n }\n \n-CameraConfiguration\n-PipelineHandlerVimc::generateConfiguration(Camera *camera,\n-\t\t\t\t\t const StreamRoles &roles)\n+CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera,\n+\tconst StreamRoles &roles)\n {\n-\tCameraConfiguration config;\n-\tStreamConfiguration cfg;\n+\tCameraConfiguration *config = new CameraConfiguration();\n \n+\tif (roles.empty())\n+\t\treturn config;\n+\n+\tStreamConfiguration cfg{};\n \tcfg.pixelFormat = V4L2_PIX_FMT_RGB24;\n \tcfg.size = { 640, 480 };\n \tcfg.bufferCount = 4;\n \n-\tconfig.addConfiguration(cfg);\n+\tconfig->addConfiguration(cfg);\n \n \treturn config;\n }\n \n-int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration &config)\n+int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n {\n \tVimcCameraData *data = cameraData(camera);\n-\tStreamConfiguration &cfg = config[0];\n+\tStreamConfiguration &cfg = config->at(0);\n \tint ret;\n \n \tV4L2DeviceFormat format = {};\ndiff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\nindex 4185e3557dcb..de46e98880a2 100644\n--- a/src/libcamera/pipeline_handler.cpp\n+++ b/src/libcamera/pipeline_handler.cpp\n@@ -233,7 +233,8 @@ void PipelineHandler::unlock()\n * the group of streams parameters.\n *\n * \\return A valid CameraConfiguration if the requested roles can be satisfied,\n- * or a invalid configuration otherwise\n+ * or a null pointer otherwise. The ownership of the returned configuration is\n+ * passed to the caller.\n */\n \n /**\ndiff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\nindex 06ae2985f80d..16b123132dd9 100644\n--- a/src/qcam/main_window.cpp\n+++ b/src/qcam/main_window.cpp\n@@ -98,13 +98,13 @@ int MainWindow::startCapture()\n \tint ret;\n \n \tconfig_ = camera_->generateConfiguration({ StreamRole::VideoRecording });\n-\tret = camera_->configure(config_);\n+\tret = camera_->configure(config_.get());\n \tif (ret < 0) {\n \t\tstd::cout << \"Failed to configure camera\" << std::endl;\n \t\treturn ret;\n \t}\n \n-\tconst StreamConfiguration &cfg = config_[0];\n+\tconst StreamConfiguration &cfg = config_->at(0);\n \tStream *stream = cfg.stream();\n \tret = viewfinder_->setFormat(cfg.pixelFormat, cfg.size.width,\n \t\t\t\t cfg.size.height);\n@@ -180,6 +180,8 @@ void MainWindow::stopCapture()\n \n \tcamera_->freeBuffers();\n \tisCapturing_ = false;\n+\n+\tconfig_.reset();\n }\n \n void MainWindow::requestComplete(Request *request,\ndiff --git a/src/qcam/main_window.h b/src/qcam/main_window.h\nindex 143b5b08a5a0..fe565cbcb460 100644\n--- a/src/qcam/main_window.h\n+++ b/src/qcam/main_window.h\n@@ -8,6 +8,7 @@\n #define __QCAM_MAIN_WINDOW_H__\n \n #include <map>\n+#include <memory>\n \n #include <QMainWindow>\n \n@@ -45,7 +46,7 @@ private:\n \n \tstd::shared_ptr<Camera> camera_;\n \tbool isCapturing_;\n-\tCameraConfiguration config_;\n+\tstd::unique_ptr<CameraConfiguration> config_;\n \n \tViewFinder *viewfinder_;\n };\ndiff --git a/test/camera/capture.cpp b/test/camera/capture.cpp\nindex bfd11eefedcf..bb7d380cdc1a 100644\n--- a/test/camera/capture.cpp\n+++ b/test/camera/capture.cpp\n@@ -40,13 +40,25 @@ protected:\n \t\tcamera_->queueRequest(request);\n \t}\n \n-\tint run()\n+\tint init() override\n \t{\n-\t\tCameraConfiguration config =\n-\t\t\tcamera_->generateConfiguration({ StreamRole::VideoRecording });\n-\t\tStreamConfiguration *cfg = &config[0];\n+\t\tCameraTest::init();\n \n-\t\tif (!config.isValid()) {\n+\t\tconfig_ = camera_->generateConfiguration({ StreamRole::VideoRecording });\n+\t\tif (!config_) {\n+\t\t\tcout << \"Failed to generate default configuration\" << endl;\n+\t\t\tCameraTest::cleanup();\n+\t\t\treturn TestFail;\n+\t\t}\n+\n+\t\treturn TestPass;\n+\t}\n+\n+\tint run() override\n+\t{\n+\t\tStreamConfiguration &cfg = config_->at(0);\n+\n+\t\tif (!config_->isValid()) {\n \t\t\tcout << \"Failed to read default configuration\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n@@ -56,7 +68,7 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tif (camera_->configure(config)) {\n+\t\tif (camera_->configure(config_.get())) {\n \t\t\tcout << \"Failed to set default configuration\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n@@ -66,7 +78,7 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tStream *stream = cfg->stream();\n+\t\tStream *stream = cfg.stream();\n \t\tBufferPool &pool = stream->bufferPool();\n \t\tstd::vector<Request *> requests;\n \t\tfor (Buffer &buffer : pool.buffers()) {\n@@ -110,10 +122,10 @@ protected:\n \t\twhile (timer.isRunning())\n \t\t\tdispatcher->processEvents();\n \n-\t\tif (completeRequestsCount_ <= cfg->bufferCount * 2) {\n+\t\tif (completeRequestsCount_ <= cfg.bufferCount * 2) {\n \t\t\tcout << \"Failed to capture enough frames (got \"\n \t\t\t << completeRequestsCount_ << \" expected at least \"\n-\t\t\t << cfg->bufferCount * 2 << \")\" << endl;\n+\t\t\t << cfg.bufferCount * 2 << \")\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n \n@@ -134,6 +146,8 @@ protected:\n \n \t\treturn TestPass;\n \t}\n+\n+\tstd::unique_ptr<CameraConfiguration> config_;\n };\n \n } /* namespace */\ndiff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp\nindex 8c4a03db498a..8a767d2321e0 100644\n--- a/test/camera/configuration_default.cpp\n+++ b/test/camera/configuration_default.cpp\n@@ -18,21 +18,21 @@ class ConfigurationDefault : public CameraTest\n protected:\n \tint run()\n \t{\n-\t\tCameraConfiguration config;\n+\t\tstd::unique_ptr<CameraConfiguration> config;\n \n \t\t/* Test asking for configuration for a video stream. */\n \t\tconfig = camera_->generateConfiguration({ StreamRole::VideoRecording });\n-\t\tif (!config.isValid()) {\n+\t\tif (!config || !config->isValid()) {\n \t\t\tcout << \"Default configuration invalid\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n \n \t\t/*\n \t\t * Test that asking for configuration for an empty array of\n-\t\t * stream roles returns an empty list of configurations.\n+\t\t * stream roles returns an empty camera configuration.\n \t\t */\n \t\tconfig = camera_->generateConfiguration({});\n-\t\tif (config.isValid()) {\n+\t\tif (!config || config->isValid()) {\n \t\t\tcout << \"Failed to retrieve configuration for empty roles list\"\n \t\t\t << endl;\n \t\t\treturn TestFail;\ndiff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp\nindex 25b5db67103a..ece987c7752a 100644\n--- a/test/camera/configuration_set.cpp\n+++ b/test/camera/configuration_set.cpp\n@@ -16,13 +16,25 @@ namespace {\n class ConfigurationSet : public CameraTest\n {\n protected:\n-\tint run()\n+\tint init() override\n \t{\n-\t\tCameraConfiguration config =\n-\t\t\tcamera_->generateConfiguration({ StreamRole::VideoRecording });\n-\t\tStreamConfiguration *cfg = &config[0];\n+\t\tCameraTest::init();\n \n-\t\tif (!config.isValid()) {\n+\t\tconfig_ = camera_->generateConfiguration({ StreamRole::VideoRecording });\n+\t\tif (!config_) {\n+\t\t\tcout << \"Failed to generate default configuration\" << endl;\n+\t\t\tCameraTest::cleanup();\n+\t\t\treturn TestFail;\n+\t\t}\n+\n+\t\treturn TestPass;\n+\t}\n+\n+\tint run() override\n+\t{\n+\t\tStreamConfiguration &cfg = config_->at(0);\n+\n+\t\tif (!config_->isValid()) {\n \t\t\tcout << \"Failed to read default configuration\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n@@ -33,7 +45,7 @@ protected:\n \t\t}\n \n \t\t/* Test that setting the default configuration works. */\n-\t\tif (camera_->configure(config)) {\n+\t\tif (camera_->configure(config_.get())) {\n \t\t\tcout << \"Failed to set default configuration\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n@@ -48,7 +60,7 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tif (!camera_->configure(config)) {\n+\t\tif (!camera_->configure(config_.get())) {\n \t\t\tcout << \"Setting configuration on a camera not acquired succeeded when it should have failed\"\n \t\t\t << endl;\n \t\t\treturn TestFail;\n@@ -64,9 +76,9 @@ protected:\n \t\t * the default configuration of the VIMC camera is known to\n \t\t * work.\n \t\t */\n-\t\tcfg->size.width *= 2;\n-\t\tcfg->size.height *= 2;\n-\t\tif (camera_->configure(config)) {\n+\t\tcfg.size.width *= 2;\n+\t\tcfg.size.height *= 2;\n+\t\tif (camera_->configure(config_.get())) {\n \t\t\tcout << \"Failed to set modified configuration\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n@@ -74,14 +86,16 @@ protected:\n \t\t/*\n \t\t * Test that setting an invalid configuration fails.\n \t\t */\n-\t\tcfg->size = { 0, 0 };\n-\t\tif (!camera_->configure(config)) {\n+\t\tcfg.size = { 0, 0 };\n+\t\tif (!camera_->configure(config_.get())) {\n \t\t\tcout << \"Invalid configuration incorrectly accepted\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n \n \t\treturn TestPass;\n \t}\n+\n+\tstd::unique_ptr<CameraConfiguration> config_;\n };\n \n } /* namespace */\ndiff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp\nindex 7a74cd85a37a..d489f197e402 100644\n--- a/test/camera/statemachine.cpp\n+++ b/test/camera/statemachine.cpp\n@@ -19,7 +19,7 @@ protected:\n \tint testAvailable()\n \t{\n \t\t/* Test operations which should fail. */\n-\t\tif (camera_->configure(defconf_) != -EACCES)\n+\t\tif (camera_->configure(defconf_.get()) != -EACCES)\n \t\t\treturn TestFail;\n \n \t\tif (camera_->allocateBuffers() != -EACCES)\n@@ -84,7 +84,7 @@ protected:\n \t\tif (camera_->acquire())\n \t\t\treturn TestFail;\n \n-\t\tif (camera_->configure(defconf_))\n+\t\tif (camera_->configure(defconf_.get()))\n \t\t\treturn TestFail;\n \n \t\treturn TestPass;\n@@ -113,7 +113,7 @@ protected:\n \t\t\treturn TestFail;\n \n \t\t/* Test operations which should pass. */\n-\t\tif (camera_->configure(defconf_))\n+\t\tif (camera_->configure(defconf_.get()))\n \t\t\treturn TestFail;\n \n \t\t/* Test valid state transitions, end in Prepared state. */\n@@ -123,7 +123,7 @@ protected:\n \t\tif (camera_->acquire())\n \t\t\treturn TestFail;\n \n-\t\tif (camera_->configure(defconf_))\n+\t\tif (camera_->configure(defconf_.get()))\n \t\t\treturn TestFail;\n \n \t\tif (camera_->allocateBuffers())\n@@ -141,7 +141,7 @@ protected:\n \t\tif (camera_->release() != -EBUSY)\n \t\t\treturn TestFail;\n \n-\t\tif (camera_->configure(defconf_) != -EACCES)\n+\t\tif (camera_->configure(defconf_.get()) != -EACCES)\n \t\t\treturn TestFail;\n \n \t\tif (camera_->allocateBuffers() != -EACCES)\n@@ -172,7 +172,7 @@ protected:\n \t\tif (camera_->acquire())\n \t\t\treturn TestFail;\n \n-\t\tif (camera_->configure(defconf_))\n+\t\tif (camera_->configure(defconf_.get()))\n \t\t\treturn TestFail;\n \n \t\tif (camera_->allocateBuffers())\n@@ -193,7 +193,7 @@ protected:\n \t\tif (camera_->release() != -EBUSY)\n \t\t\treturn TestFail;\n \n-\t\tif (camera_->configure(defconf_) != -EACCES)\n+\t\tif (camera_->configure(defconf_.get()) != -EACCES)\n \t\t\treturn TestFail;\n \n \t\tif (camera_->allocateBuffers() != -EACCES)\n@@ -233,10 +233,22 @@ protected:\n \t\treturn TestPass;\n \t}\n \n-\tint run()\n+\tint init() override\n \t{\n+\t\tCameraTest::init();\n+\n \t\tdefconf_ = camera_->generateConfiguration({ StreamRole::VideoRecording });\n+\t\tif (!defconf_) {\n+\t\t\tcout << \"Failed to generate default configuration\" << endl;\n+\t\t\tCameraTest::cleanup();\n+\t\t\treturn TestFail;\n+\t\t}\n \n+\t\treturn TestPass;\n+\t}\n+\n+\tint run() override\n+\t{\n \t\tif (testAvailable() != TestPass) {\n \t\t\tcout << \"State machine in Available state failed\" << endl;\n \t\t\treturn TestFail;\n@@ -265,7 +277,7 @@ protected:\n \t\treturn TestPass;\n \t}\n \n-\tCameraConfiguration defconf_;\n+\tstd::unique_ptr<CameraConfiguration> defconf_;\n };\n \n } /* namespace */\n", "prefixes": [ "libcamera-devel", "v3", "4/6" ] }