[{"id":1630,"web_url":"https://patchwork.libcamera.org/comment/1630/","msgid":"<20190519204438.GK25081@bigcity.dyn.berto.se>","date":"2019-05-19T20:44:38","subject":"Re: [libcamera-devel] [PATCH v2 4/6] libcamera: camera: Return a\n\tpointer from generateConfiguration()","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2019-05-19 18:00:45 +0300, Laurent Pinchart wrote:\n> To prepare for specialising the CameraConfiguration class in pipeline\n> handlers, return a pointer to a camera configuration instead of a\n> reference from Camera::generateConfiguration(). The camera configuration\n> always needs to be allocated from the pipeline handler, and its\n> ownership is passed to the application.\n> \n> For symmetry, change Camera::configure() to take a CameraConfiguration\n> pointer instead of a reference. This aligns with our coding practice of\n> passing parameters that are modified by the callee by pointer.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  include/libcamera/camera.h               |  4 +--\n>  src/cam/main.cpp                         | 40 ++++++++++-----------\n>  src/libcamera/camera.cpp                 | 31 +++++++++--------\n>  src/libcamera/include/pipeline_handler.h |  6 ++--\n>  src/libcamera/pipeline/ipu3/ipu3.cpp     | 31 +++++++++--------\n>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 25 ++++++++------\n>  src/libcamera/pipeline/uvcvideo.cpp      | 30 ++++++++--------\n>  src/libcamera/pipeline/vimc.cpp          | 30 ++++++++--------\n>  src/libcamera/pipeline_handler.cpp       |  3 +-\n>  src/qcam/main_window.cpp                 |  5 ++-\n>  src/qcam/main_window.h                   |  2 +-\n>  test/camera/capture.cpp                  | 38 +++++++++++++++-----\n>  test/camera/configuration_default.cpp    | 14 +++++---\n>  test/camera/configuration_set.cpp        | 44 +++++++++++++++++-------\n>  test/camera/statemachine.cpp             | 22 ++++++++++--\n>  15 files changed, 202 insertions(+), 123 deletions(-)\n> \n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 284e5276a055..144133c5de9f 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> +\tCameraConfiguration *generateConfiguration(const StreamRoles &roles);\n> +\tint configure(CameraConfiguration *config);\n>  \n>  \tint allocateBuffers();\n>  \tint freeBuffers();\n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index cd165bea34cd..7550ae4f3428 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 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,23 @@ 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> +\tCameraConfiguration *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\tdelete config;\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 +140,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,25 +189,25 @@ 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> +\tCameraConfiguration *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>  \tif (ret < 0) {\n>  \t\tstd::cout << \"Failed to configure camera\" << std::endl;\n> +\t\tdelete config;\n>  \t\treturn ret;\n>  \t}\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> @@ -217,6 +215,7 @@ static int capture()\n>  \tif (ret) {\n>  \t\tstd::cerr << \"Failed to allocate buffers\"\n>  \t\t\t  << std::endl;\n> +\t\tdelete config;\n>  \t\treturn ret;\n>  \t}\n>  \n> @@ -224,7 +223,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 +243,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}\n> @@ -280,6 +279,7 @@ static int capture()\n>  \t\tstd::cout << \"Failed to stop capture\" << std::endl;\n>  out:\n>  \tcamera->freeBuffers();\n> +\tdelete config;\n>  \n>  \treturn ret;\n>  }\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 5848330f639a..0e80691ed862 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -542,21 +542,24 @@ const std::set<Stream *> &Camera::streams() const\n>   * specifies a list of stream roles and the camera returns a configuration\n>   * containing suitable streams and their suggested default configurations.\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> +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> @@ -590,7 +593,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 +603,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 +611,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 +624,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)\n> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> index 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;\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 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}\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index ec6980b0943a..01ba50f09db1 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> -\tStreamConfiguration cfg{};\n> +\tCameraConfiguration *config = new CameraConfiguration();\n>  \n> -\tcfg.pixelFormat = V4L2_PIX_FMT_NV12;\n> -\tcfg.size = data->sensor_->resolution();\n> -\tcfg.bufferCount = RKISP1_BUFFER_COUNT;\n> +\tif (!roles.empty()) {\n\nI would invert this check and reduce the indentation level,\n\n    if (roles.empty())\n        return config;\n\nSame for other generateConfiguration() bellow.\n\nWith this fixed,\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> +\t\tStreamConfiguration cfg{};\n>  \n> -\tconfig.addConfiguration(cfg);\n> +\t\tcfg.pixelFormat = V4L2_PIX_FMT_NV12;\n> +\t\tcfg.size = data->sensor_->resolution();\n> +\t\tcfg.bufferCount = RKISP1_BUFFER_COUNT;\n> +\n> +\t\tconfig->addConfiguration(cfg);\n> +\t}\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>  \n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index 5dcc868b2fc9..91b4065c250b 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> -\tcfg.pixelFormat = V4L2_PIX_FMT_YUYV;\n> -\tcfg.size = { 640, 480 };\n> -\tcfg.bufferCount = 4;\n> +\tif (!roles.empty()) {\n> +\t\tStreamConfiguration cfg;\n>  \n> -\tconfig.addConfiguration(cfg);\n> +\t\tcfg.pixelFormat = V4L2_PIX_FMT_YUYV;\n> +\t\tcfg.size = { 640, 480 };\n> +\t\tcfg.bufferCount = 4;\n> +\n> +\t\tconfig->addConfiguration(cfg);\n> +\t}\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 = {};\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index af6b6f21e3c5..510e6c082f13 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> -\tcfg.pixelFormat = V4L2_PIX_FMT_RGB24;\n> -\tcfg.size = { 640, 480 };\n> -\tcfg.bufferCount = 4;\n> +\tif (!roles.empty()) {\n> +\t\tStreamConfiguration cfg;\n>  \n> -\tconfig.addConfiguration(cfg);\n> +\t\tcfg.pixelFormat = V4L2_PIX_FMT_RGB24;\n> +\t\tcfg.size = { 640, 480 };\n> +\t\tcfg.bufferCount = 4;\n> +\n> +\t\tconfig->addConfiguration(cfg);\n> +\t}\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 = {};\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 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>  /**\n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index 06ae2985f80d..3ae5aae9cc76 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -104,7 +104,7 @@ int MainWindow::startCapture()\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,9 @@ void MainWindow::stopCapture()\n>  \n>  \tcamera_->freeBuffers();\n>  \tisCapturing_ = false;\n> +\n> +\tdelete config_;\n> +\tconfig_ = nullptr;\n>  }\n>  \n>  void MainWindow::requestComplete(Request *request,\n> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h\n> index 143b5b08a5a0..866866e93d23 100644\n> --- a/src/qcam/main_window.h\n> +++ b/src/qcam/main_window.h\n> @@ -45,7 +45,7 @@ private:\n>  \n>  \tstd::shared_ptr<Camera> camera_;\n>  \tbool isCapturing_;\n> -\tCameraConfiguration config_;\n> +\tCameraConfiguration *config_;\n>  \n>  \tViewFinder *viewfinder_;\n>  };\n> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp\n> index bfd11eefedcf..137aa649a505 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_)) {\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,14 @@ protected:\n>  \n>  \t\treturn TestPass;\n>  \t}\n> +\n> +\tvoid cleanup() override\n> +\t{\n> +\t\tdelete config_;\n> +\t\tCameraTest::cleanup();\n> +\t}\n> +\n> +\tCameraConfiguration *config_;\n>  };\n>  \n>  } /* namespace */\n> diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp\n> index 8c4a03db498a..d5cefc1127c9 100644\n> --- a/test/camera/configuration_default.cpp\n> +++ b/test/camera/configuration_default.cpp\n> @@ -18,26 +18,32 @@ class ConfigurationDefault : public CameraTest\n>  protected:\n>  \tint run()\n>  \t{\n> -\t\tCameraConfiguration config;\n> +\t\tCameraConfiguration *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\tdelete config;\n>  \t\t\treturn TestFail;\n>  \t\t}\n>  \n> +\t\tdelete config;\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\tdelete config;\n>  \t\t\treturn TestFail;\n>  \t\t}\n>  \n> +\t\tdelete config;\n> +\n>  \t\treturn TestPass;\n>  \t}\n>  };\n> diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp\n> index 25b5db67103a..23c611a93355 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_)) {\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_)) {\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_)) {\n>  \t\t\tcout << \"Failed to set modified configuration\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n> @@ -74,14 +86,22 @@ 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_)) {\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> +\tvoid cleanup() override\n> +\t{\n> +\t\tdelete config_;\n> +\t\tCameraTest::cleanup();\n> +\t}\n> +\n> +\tCameraConfiguration *config_;\n>  };\n>  \n>  } /* namespace */\n> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp\n> index 7a74cd85a37a..a662e869fadf 100644\n> --- a/test/camera/statemachine.cpp\n> +++ b/test/camera/statemachine.cpp\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,13 @@ protected:\n>  \t\treturn TestPass;\n>  \t}\n>  \n> -\tCameraConfiguration defconf_;\n> +\tvoid cleanup() override\n> +\t{\n> +\t\tdelete defconf_;\n> +\t\tCameraTest::cleanup();\n> +\t}\n> +\n> +\tCameraConfiguration *defconf_;\n>  };\n>  \n>  } /* namespace */\n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lf1-x143.google.com (mail-lf1-x143.google.com\n\t[IPv6:2a00:1450:4864:20::143])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 41C7760E4C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 19 May 2019 22:44:41 +0200 (CEST)","by mail-lf1-x143.google.com with SMTP id v18so8802041lfi.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 19 May 2019 13:44:41 -0700 (PDT)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\th14sm3392363ljj.11.2019.05.19.13.44.38\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSun, 19 May 2019 13:44:39 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=TTi29Is/kkg6b4yPhrYCevxw8KO9A6Ygn00GRGsOIqI=;\n\tb=eSzmmr9VWRM3QhaLaBTf//2xg4e6PNQZy3ypMWm005hHgo1w9czScToS0LEL+u80Ii\n\txj19g+XKLY4Cq91lxrdijR2aTHQnxSSrIPRgpqGdOQ3W44kCRc014bLnqqz6CSFSXysN\n\t/55vUtB1jfqUelsyf8mEGvgt5so0pzAzTgnDRRwRze42KfKxMSF/8ccCMnAzBAhFdN4k\n\tFsi1ApcBTvQziOYTCaacyeDjAwrfNLz3IvFNCXzChHbibXZ/BjSAu/BIh2er2uA8+Vub\n\tcnx5qKU519yzhFVWy6ypYfJfNYIuRg71o9+xnVii8JGzyBjEs1ARAKO4SDRy6N4QLk4h\n\tv9iw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=TTi29Is/kkg6b4yPhrYCevxw8KO9A6Ygn00GRGsOIqI=;\n\tb=PY0zdthJNTCuJ1d1mehOTMIlS94a30geiqfyjfpJT5G88n3llAVkFdCtFmhso1hTup\n\t/BYdsUrL/XMmia2MMbyxO0Hkuf7d4y1Fw4R1Sv9cbUMmMwFuf6wHKk/XT/LmKpqpn96l\n\txKd9en3m7T6h9nvyzRmRnS6qIrkrX7AzOGrpULjkvE9o8azj/PqsSFLbbQKVMCifP3aQ\n\tsVSCNGOMfVBE8ZgkzFufDWpLTDCU42JWWOyMANzs2zo4iHJ1u+S1c2xPMZoi5CoXH8BT\n\tlxdAo1tQAB0OTGOfI1zBvxtZJLJDJPXQurCjL8r6pxBQ9hHKuy9mD0XDKVnTo3XiyWCk\n\tn8hA==","X-Gm-Message-State":"APjAAAVgbIQ5qBq+f2ECH9hvsL5ybZFdc4I5V7Wn9uBIn6R7jSM+zl+H\n\tdDMSLFUbhImuXQ/3RdIbm3EPwg==","X-Google-Smtp-Source":"APXvYqwkLBsh1v/Rb6cs7RISbSS99PTM0lfVRScf10pPM/Iq9wt0YWgLps1WZvvaocjRzq5voQhP/A==","X-Received":"by 2002:ac2:4989:: with SMTP id f9mr35091115lfl.12.1558298680315;\n\tSun, 19 May 2019 13:44:40 -0700 (PDT)","Date":"Sun, 19 May 2019 22:44:38 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190519204438.GK25081@bigcity.dyn.berto.se>","References":"<20190519150047.12444-1-laurent.pinchart@ideasonboard.com>\n\t<20190519150047.12444-5-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190519150047.12444-5-laurent.pinchart@ideasonboard.com>","User-Agent":"Mutt/1.11.3 (2019-02-01)","Subject":"Re: [libcamera-devel] [PATCH v2 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":"Sun, 19 May 2019 20:44:41 -0000"}},{"id":1637,"web_url":"https://patchwork.libcamera.org/comment/1637/","msgid":"<20190521095243.qdmovfzv2au2w6p2@uno.localdomain>","date":"2019-05-21T09:52:44","subject":"Re: [libcamera-devel] [PATCH v2 4/6] libcamera: camera: Return a\n\tpointer 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 Sun, May 19, 2019 at 06:00:45PM +0300, Laurent Pinchart wrote:\n> To prepare for specialising the CameraConfiguration class in pipeline\n> handlers, return a pointer to a camera configuration instead of a\n> reference from Camera::generateConfiguration(). The camera configuration\n> always needs to be allocated from the pipeline handler, and its\n> ownership is passed to the application.\n\nHave you considered enforcing this returning CameraConfiguration as a\nunique_ptr<> to applications ? Ownership is passed to them at\ngenerateConfiguration() time, and then applications would pass a\nreference accessing the unique_ptr<> at configure() time.\n\nAfter all, there is no other way from an application to get back the\nsame CameraConfiguration from the Camera, so once it has been\nreturned, it's responsability of the application to keep a valid\nreference to it. Also, to make it easier for app to deal with that,\nwe could abstract it with a d-pointer later (if appropriate)\n>\n> For symmetry, change Camera::configure() to take a CameraConfiguration\n> pointer instead of a reference. This aligns with our coding practice of\n> passing parameters that are modified by the callee by pointer.\n>\n\nWith unique_ptr this would be Camera::configure(config.get())\n\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  include/libcamera/camera.h               |  4 +--\n>  src/cam/main.cpp                         | 40 ++++++++++-----------\n>  src/libcamera/camera.cpp                 | 31 +++++++++--------\n>  src/libcamera/include/pipeline_handler.h |  6 ++--\n>  src/libcamera/pipeline/ipu3/ipu3.cpp     | 31 +++++++++--------\n>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 25 ++++++++------\n>  src/libcamera/pipeline/uvcvideo.cpp      | 30 ++++++++--------\n>  src/libcamera/pipeline/vimc.cpp          | 30 ++++++++--------\n>  src/libcamera/pipeline_handler.cpp       |  3 +-\n>  src/qcam/main_window.cpp                 |  5 ++-\n>  src/qcam/main_window.h                   |  2 +-\n>  test/camera/capture.cpp                  | 38 +++++++++++++++-----\n>  test/camera/configuration_default.cpp    | 14 +++++---\n>  test/camera/configuration_set.cpp        | 44 +++++++++++++++++-------\n>  test/camera/statemachine.cpp             | 22 ++++++++++--\n>  15 files changed, 202 insertions(+), 123 deletions(-)\n>\n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 284e5276a055..144133c5de9f 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> +\tCameraConfiguration *generateConfiguration(const StreamRoles &roles);\n> +\tint configure(CameraConfiguration *config);\n>\n>  \tint allocateBuffers();\n>  \tint freeBuffers();\n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index cd165bea34cd..7550ae4f3428 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 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,23 @@ 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> +\tCameraConfiguration *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\tdelete config;\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\nI curious to know why at() here? Just for style reasons ?\n\n>\n>  \t\tif (conf.isSet(\"width\"))\n>  \t\t\tcfg.size.width = conf[\"width\"];\n> @@ -142,7 +140,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,25 +189,25 @@ 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> +\tCameraConfiguration *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>  \tif (ret < 0) {\n>  \t\tstd::cout << \"Failed to configure camera\" << std::endl;\n> +\t\tdelete config;\n>  \t\treturn ret;\n>  \t}\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> @@ -217,6 +215,7 @@ static int capture()\n>  \tif (ret) {\n>  \t\tstd::cerr << \"Failed to allocate buffers\"\n>  \t\t\t  << std::endl;\n> +\t\tdelete config;\n\nusing unique_ptr<> would avoid application having to do this. It comes\nat a cost of a more verbose API indeed.\n\n>  \t\treturn ret;\n>  \t}\n>\n> @@ -224,7 +223,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 +243,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}\n> @@ -280,6 +279,7 @@ static int capture()\n>  \t\tstd::cout << \"Failed to stop capture\" << std::endl;\n>  out:\n>  \tcamera->freeBuffers();\n> +\tdelete config;\n>\n>  \treturn ret;\n>  }\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 5848330f639a..0e80691ed862 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -542,21 +542,24 @@ const std::set<Stream *> &Camera::streams() const\n>   * specifies a list of stream roles and the camera returns a configuration\n>   * containing suitable streams and their suggested default configurations.\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> +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\nI might have missed it, but I would document that we accept empty\nStreamRoles to generate an empty CameraConfiguration to be filled by\napplications.\n\nI'm not sure but if that's the case, we could check here and return an\nhere instantiated empty CameraConfiguration without even going to the\npipeline handlers.\n\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> @@ -590,7 +593,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 +603,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 +611,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 +624,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)\n> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> index 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;\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 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\nnit: I would keep this aligned with the first parameter\n\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}\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index ec6980b0943a..01ba50f09db1 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> -\tStreamConfiguration cfg{};\n> +\tCameraConfiguration *config = new CameraConfiguration();\n>\n> -\tcfg.pixelFormat = V4L2_PIX_FMT_NV12;\n> -\tcfg.size = data->sensor_->resolution();\n> -\tcfg.bufferCount = RKISP1_BUFFER_COUNT;\n> +\tif (!roles.empty()) {\n> +\t\tStreamConfiguration cfg{};\n\nSame comment as Niklas here, with the additional point of moving this\ncheck to Camera::generateConfiguration()\n\nThanks\n   j\n>\n> -\tconfig.addConfiguration(cfg);\n> +\t\tcfg.pixelFormat = V4L2_PIX_FMT_NV12;\n> +\t\tcfg.size = data->sensor_->resolution();\n> +\t\tcfg.bufferCount = RKISP1_BUFFER_COUNT;\n> +\n> +\t\tconfig->addConfiguration(cfg);\n> +\t}\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>\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index 5dcc868b2fc9..91b4065c250b 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> -\tcfg.pixelFormat = V4L2_PIX_FMT_YUYV;\n> -\tcfg.size = { 640, 480 };\n> -\tcfg.bufferCount = 4;\n> +\tif (!roles.empty()) {\n> +\t\tStreamConfiguration cfg;\n>\n> -\tconfig.addConfiguration(cfg);\n> +\t\tcfg.pixelFormat = V4L2_PIX_FMT_YUYV;\n> +\t\tcfg.size = { 640, 480 };\n> +\t\tcfg.bufferCount = 4;\n> +\n> +\t\tconfig->addConfiguration(cfg);\n> +\t}\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 = {};\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index af6b6f21e3c5..510e6c082f13 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> -\tcfg.pixelFormat = V4L2_PIX_FMT_RGB24;\n> -\tcfg.size = { 640, 480 };\n> -\tcfg.bufferCount = 4;\n> +\tif (!roles.empty()) {\n> +\t\tStreamConfiguration cfg;\n>\n> -\tconfig.addConfiguration(cfg);\n> +\t\tcfg.pixelFormat = V4L2_PIX_FMT_RGB24;\n> +\t\tcfg.size = { 640, 480 };\n> +\t\tcfg.bufferCount = 4;\n> +\n> +\t\tconfig->addConfiguration(cfg);\n> +\t}\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 = {};\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 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>  /**\n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index 06ae2985f80d..3ae5aae9cc76 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -104,7 +104,7 @@ int MainWindow::startCapture()\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,9 @@ void MainWindow::stopCapture()\n>\n>  \tcamera_->freeBuffers();\n>  \tisCapturing_ = false;\n> +\n> +\tdelete config_;\n> +\tconfig_ = nullptr;\n>  }\n>\n>  void MainWindow::requestComplete(Request *request,\n> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h\n> index 143b5b08a5a0..866866e93d23 100644\n> --- a/src/qcam/main_window.h\n> +++ b/src/qcam/main_window.h\n> @@ -45,7 +45,7 @@ private:\n>\n>  \tstd::shared_ptr<Camera> camera_;\n>  \tbool isCapturing_;\n> -\tCameraConfiguration config_;\n> +\tCameraConfiguration *config_;\n>\n>  \tViewFinder *viewfinder_;\n>  };\n> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp\n> index bfd11eefedcf..137aa649a505 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_)) {\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,14 @@ protected:\n>\n>  \t\treturn TestPass;\n>  \t}\n> +\n> +\tvoid cleanup() override\n> +\t{\n> +\t\tdelete config_;\n> +\t\tCameraTest::cleanup();\n> +\t}\n> +\n> +\tCameraConfiguration *config_;\n>  };\n>\n>  } /* namespace */\n> diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp\n> index 8c4a03db498a..d5cefc1127c9 100644\n> --- a/test/camera/configuration_default.cpp\n> +++ b/test/camera/configuration_default.cpp\n> @@ -18,26 +18,32 @@ class ConfigurationDefault : public CameraTest\n>  protected:\n>  \tint run()\n>  \t{\n> -\t\tCameraConfiguration config;\n> +\t\tCameraConfiguration *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\tdelete config;\n>  \t\t\treturn TestFail;\n>  \t\t}\n>\n> +\t\tdelete config;\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\tdelete config;\n>  \t\t\treturn TestFail;\n>  \t\t}\n>\n> +\t\tdelete config;\n> +\n>  \t\treturn TestPass;\n>  \t}\n>  };\n> diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp\n> index 25b5db67103a..23c611a93355 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_)) {\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_)) {\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_)) {\n>  \t\t\tcout << \"Failed to set modified configuration\" << endl;\n>  \t\t\treturn TestFail;\n>  \t\t}\n> @@ -74,14 +86,22 @@ 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_)) {\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> +\tvoid cleanup() override\n> +\t{\n> +\t\tdelete config_;\n> +\t\tCameraTest::cleanup();\n> +\t}\n> +\n> +\tCameraConfiguration *config_;\n>  };\n>\n>  } /* namespace */\n> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp\n> index 7a74cd85a37a..a662e869fadf 100644\n> --- a/test/camera/statemachine.cpp\n> +++ b/test/camera/statemachine.cpp\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,13 @@ protected:\n>  \t\treturn TestPass;\n>  \t}\n>\n> -\tCameraConfiguration defconf_;\n> +\tvoid cleanup() override\n> +\t{\n> +\t\tdelete defconf_;\n> +\t\tCameraTest::cleanup();\n> +\t}\n> +\n> +\tCameraConfiguration *defconf_;\n>  };\n>\n>  } /* namespace */\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AF15860C1D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 21 May 2019 11:51:38 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id E7C9A4000C;\n\tTue, 21 May 2019 09:51:37 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Tue, 21 May 2019 11:52:44 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190521095243.qdmovfzv2au2w6p2@uno.localdomain>","References":"<20190519150047.12444-1-laurent.pinchart@ideasonboard.com>\n\t<20190519150047.12444-5-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"6ukbzr3t226hhuvz\"","Content-Disposition":"inline","In-Reply-To":"<20190519150047.12444-5-laurent.pinchart@ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 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 09:51:38 -0000"}},{"id":1644,"web_url":"https://patchwork.libcamera.org/comment/1644/","msgid":"<20190521141545.GG5674@pendragon.ideasonboard.com>","date":"2019-05-21T14:15:45","subject":"Re: [libcamera-devel] [PATCH v2 4/6] libcamera: camera: Return a\n\tpointer from generateConfiguration()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Tue, May 21, 2019 at 11:52:44AM +0200, Jacopo Mondi wrote:\n> On Sun, May 19, 2019 at 06:00:45PM +0300, Laurent Pinchart wrote:\n> > To prepare for specialising the CameraConfiguration class in pipeline\n> > handlers, return a pointer to a camera configuration instead of a\n> > reference from Camera::generateConfiguration(). The camera configuration\n> > always needs to be allocated from the pipeline handler, and its\n> > ownership is passed to the application.\n> \n> Have you considered enforcing this returning CameraConfiguration as a\n> unique_ptr<> to applications ? Ownership is passed to them at\n> generateConfiguration() time, and then applications would pass a\n> reference accessing the unique_ptr<> at configure() time.\n\nIt's a good idea, I hadn't thought about it. The code looks cleaner with\nthat change (9 files changed, 27 insertions(+), 55 deletions(-)).\n\n> After all, there is no other way from an application to get back the\n> same CameraConfiguration from the Camera, so once it has been\n> returned, it's responsability of the application to keep a valid\n> reference to it. Also, to make it easier for app to deal with that,\n> we could abstract it with a d-pointer later (if appropriate)\n>\n> > For symmetry, change Camera::configure() to take a CameraConfiguration\n> > pointer instead of a reference. This aligns with our coding practice of\n> > passing parameters that are modified by the callee by pointer.\n> \n> With unique_ptr this would be Camera::configure(config.get())\n> \n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  include/libcamera/camera.h               |  4 +--\n> >  src/cam/main.cpp                         | 40 ++++++++++-----------\n> >  src/libcamera/camera.cpp                 | 31 +++++++++--------\n> >  src/libcamera/include/pipeline_handler.h |  6 ++--\n> >  src/libcamera/pipeline/ipu3/ipu3.cpp     | 31 +++++++++--------\n> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 25 ++++++++------\n> >  src/libcamera/pipeline/uvcvideo.cpp      | 30 ++++++++--------\n> >  src/libcamera/pipeline/vimc.cpp          | 30 ++++++++--------\n> >  src/libcamera/pipeline_handler.cpp       |  3 +-\n> >  src/qcam/main_window.cpp                 |  5 ++-\n> >  src/qcam/main_window.h                   |  2 +-\n> >  test/camera/capture.cpp                  | 38 +++++++++++++++-----\n> >  test/camera/configuration_default.cpp    | 14 +++++---\n> >  test/camera/configuration_set.cpp        | 44 +++++++++++++++++-------\n> >  test/camera/statemachine.cpp             | 22 ++++++++++--\n> >  15 files changed, 202 insertions(+), 123 deletions(-)\n> >\n> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > index 284e5276a055..144133c5de9f 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> > +\tCameraConfiguration *generateConfiguration(const StreamRoles &roles);\n> > +\tint configure(CameraConfiguration *config);\n> >\n> >  \tint allocateBuffers();\n> >  \tint freeBuffers();\n> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > index cd165bea34cd..7550ae4f3428 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 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,23 @@ 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> > +\tCameraConfiguration *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\tdelete config;\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> I curious to know why at() here? Just for style reasons ?\n\nYes, just for style reason. If you convince Niklas that (*config)[] is\nfine, I'll switch back to it and drop the at() method.\n\n> >\n> >  \t\tif (conf.isSet(\"width\"))\n> >  \t\t\tcfg.size.width = conf[\"width\"];\n> > @@ -142,7 +140,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,25 +189,25 @@ 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> > +\tCameraConfiguration *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> >  \tif (ret < 0) {\n> >  \t\tstd::cout << \"Failed to configure camera\" << std::endl;\n> > +\t\tdelete config;\n> >  \t\treturn ret;\n> >  \t}\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> > @@ -217,6 +215,7 @@ static int capture()\n> >  \tif (ret) {\n> >  \t\tstd::cerr << \"Failed to allocate buffers\"\n> >  \t\t\t  << std::endl;\n> > +\t\tdelete config;\n> \n> using unique_ptr<> would avoid application having to do this. It comes\n> at a cost of a more verbose API indeed.\n> \n> >  \t\treturn ret;\n> >  \t}\n> >\n> > @@ -224,7 +223,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 +243,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}\n> > @@ -280,6 +279,7 @@ static int capture()\n> >  \t\tstd::cout << \"Failed to stop capture\" << std::endl;\n> >  out:\n> >  \tcamera->freeBuffers();\n> > +\tdelete config;\n> >\n> >  \treturn ret;\n> >  }\n> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > index 5848330f639a..0e80691ed862 100644\n> > --- a/src/libcamera/camera.cpp\n> > +++ b/src/libcamera/camera.cpp\n> > @@ -542,21 +542,24 @@ const std::set<Stream *> &Camera::streams() const\n> >   * specifies a list of stream roles and the camera returns a configuration\n> >   * containing suitable streams and their suggested default configurations.\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> > +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> \n> I might have missed it, but I would document that we accept empty\n> StreamRoles to generate an empty CameraConfiguration to be filled by\n> applications.\n\nI'll fix that.\n\n> I'm not sure but if that's the case, we could check here and return an\n> here instantiated empty CameraConfiguration without even going to the\n> pipeline handlers.\n\nWe can't, as that empty configuration should get filled by applications,\nand then validated. We need a specialised instance of\nCameraConfiguration for that.\n\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> > @@ -590,7 +593,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 +603,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 +611,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 +624,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)\n> > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> > index 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;\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 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> nit: I would keep this aligned with the first parameter\n\nI'm also in two minds about that, when the method name is long, all\nparameters are pushed back far to the right. I don't think the style\nused here is perfect, but it has the advantage of being consistently\nusable regardless of how long the method name is.\n\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}\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index ec6980b0943a..01ba50f09db1 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> > -\tStreamConfiguration cfg{};\n> > +\tCameraConfiguration *config = new CameraConfiguration();\n> >\n> > -\tcfg.pixelFormat = V4L2_PIX_FMT_NV12;\n> > -\tcfg.size = data->sensor_->resolution();\n> > -\tcfg.bufferCount = RKISP1_BUFFER_COUNT;\n> > +\tif (!roles.empty()) {\n> > +\t\tStreamConfiguration cfg{};\n> \n> Same comment as Niklas here, with the additional point of moving this\n> check to Camera::generateConfiguration()\n> \n> > -\tconfig.addConfiguration(cfg);\n> > +\t\tcfg.pixelFormat = V4L2_PIX_FMT_NV12;\n> > +\t\tcfg.size = data->sensor_->resolution();\n> > +\t\tcfg.bufferCount = RKISP1_BUFFER_COUNT;\n> > +\n> > +\t\tconfig->addConfiguration(cfg);\n> > +\t}\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> >\n> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> > index 5dcc868b2fc9..91b4065c250b 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> > -\tcfg.pixelFormat = V4L2_PIX_FMT_YUYV;\n> > -\tcfg.size = { 640, 480 };\n> > -\tcfg.bufferCount = 4;\n> > +\tif (!roles.empty()) {\n> > +\t\tStreamConfiguration cfg;\n> >\n> > -\tconfig.addConfiguration(cfg);\n> > +\t\tcfg.pixelFormat = V4L2_PIX_FMT_YUYV;\n> > +\t\tcfg.size = { 640, 480 };\n> > +\t\tcfg.bufferCount = 4;\n> > +\n> > +\t\tconfig->addConfiguration(cfg);\n> > +\t}\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 = {};\n> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> > index af6b6f21e3c5..510e6c082f13 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> > -\tcfg.pixelFormat = V4L2_PIX_FMT_RGB24;\n> > -\tcfg.size = { 640, 480 };\n> > -\tcfg.bufferCount = 4;\n> > +\tif (!roles.empty()) {\n> > +\t\tStreamConfiguration cfg;\n> >\n> > -\tconfig.addConfiguration(cfg);\n> > +\t\tcfg.pixelFormat = V4L2_PIX_FMT_RGB24;\n> > +\t\tcfg.size = { 640, 480 };\n> > +\t\tcfg.bufferCount = 4;\n> > +\n> > +\t\tconfig->addConfiguration(cfg);\n> > +\t}\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 = {};\n> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > index 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> >  /**\n> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> > index 06ae2985f80d..3ae5aae9cc76 100644\n> > --- a/src/qcam/main_window.cpp\n> > +++ b/src/qcam/main_window.cpp\n> > @@ -104,7 +104,7 @@ int MainWindow::startCapture()\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,9 @@ void MainWindow::stopCapture()\n> >\n> >  \tcamera_->freeBuffers();\n> >  \tisCapturing_ = false;\n> > +\n> > +\tdelete config_;\n> > +\tconfig_ = nullptr;\n> >  }\n> >\n> >  void MainWindow::requestComplete(Request *request,\n> > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h\n> > index 143b5b08a5a0..866866e93d23 100644\n> > --- a/src/qcam/main_window.h\n> > +++ b/src/qcam/main_window.h\n> > @@ -45,7 +45,7 @@ private:\n> >\n> >  \tstd::shared_ptr<Camera> camera_;\n> >  \tbool isCapturing_;\n> > -\tCameraConfiguration config_;\n> > +\tCameraConfiguration *config_;\n> >\n> >  \tViewFinder *viewfinder_;\n> >  };\n> > diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp\n> > index bfd11eefedcf..137aa649a505 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_)) {\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,14 @@ protected:\n> >\n> >  \t\treturn TestPass;\n> >  \t}\n> > +\n> > +\tvoid cleanup() override\n> > +\t{\n> > +\t\tdelete config_;\n> > +\t\tCameraTest::cleanup();\n> > +\t}\n> > +\n> > +\tCameraConfiguration *config_;\n> >  };\n> >\n> >  } /* namespace */\n> > diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp\n> > index 8c4a03db498a..d5cefc1127c9 100644\n> > --- a/test/camera/configuration_default.cpp\n> > +++ b/test/camera/configuration_default.cpp\n> > @@ -18,26 +18,32 @@ class ConfigurationDefault : public CameraTest\n> >  protected:\n> >  \tint run()\n> >  \t{\n> > -\t\tCameraConfiguration config;\n> > +\t\tCameraConfiguration *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\tdelete config;\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> >\n> > +\t\tdelete config;\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\tdelete config;\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> >\n> > +\t\tdelete config;\n> > +\n> >  \t\treturn TestPass;\n> >  \t}\n> >  };\n> > diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp\n> > index 25b5db67103a..23c611a93355 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_)) {\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_)) {\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_)) {\n> >  \t\t\tcout << \"Failed to set modified configuration\" << endl;\n> >  \t\t\treturn TestFail;\n> >  \t\t}\n> > @@ -74,14 +86,22 @@ 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_)) {\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> > +\tvoid cleanup() override\n> > +\t{\n> > +\t\tdelete config_;\n> > +\t\tCameraTest::cleanup();\n> > +\t}\n> > +\n> > +\tCameraConfiguration *config_;\n> >  };\n> >\n> >  } /* namespace */\n> > diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp\n> > index 7a74cd85a37a..a662e869fadf 100644\n> > --- a/test/camera/statemachine.cpp\n> > +++ b/test/camera/statemachine.cpp\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,13 @@ protected:\n> >  \t\treturn TestPass;\n> >  \t}\n> >\n> > -\tCameraConfiguration defconf_;\n> > +\tvoid cleanup() override\n> > +\t{\n> > +\t\tdelete defconf_;\n> > +\t\tCameraTest::cleanup();\n> > +\t}\n> > +\n> > +\tCameraConfiguration *defconf_;\n> >  };\n> >\n> >  } /* namespace */","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BDB8360C27\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 21 May 2019 16:16:02 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 10EFB52C;\n\tTue, 21 May 2019 16:16:02 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1558448162;\n\tbh=48qU7nwP6maAcd5J8IYOYSiHaToOXfQYfydp0w9ZN9o=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=t4ik0ohby22V60alIIGFNU1PjQbXelyvxZ/KFuGWBdO0bdQZblcvk/8FJWUdGCOEZ\n\tWv+TnejwipHabcqfU00s4EJ+6WW/sYA4LjzpylKPLHiCPu+hKJNsKYyhtZTH3ZB81G\n\tEW47KsL8mu3rYnapEok2vVzFjflS0LdAVjGiwc5g=","Date":"Tue, 21 May 2019 17:15:45 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190521141545.GG5674@pendragon.ideasonboard.com>","References":"<20190519150047.12444-1-laurent.pinchart@ideasonboard.com>\n\t<20190519150047.12444-5-laurent.pinchart@ideasonboard.com>\n\t<20190521095243.qdmovfzv2au2w6p2@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190521095243.qdmovfzv2au2w6p2@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 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 14:16:03 -0000"}}]