Patch Detail
Show a patch.
GET /api/1.1/patches/1221/?format=api
{ "id": 1221, "url": "https://patchwork.libcamera.org/api/1.1/patches/1221/?format=api", "web_url": "https://patchwork.libcamera.org/patch/1221/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20190517230621.24668-5-laurent.pinchart@ideasonboard.com>", "date": "2019-05-17T23:06:13", "name": "[libcamera-devel,PATCH/RFC,04/12] libcamera: Refactor the camera configuration storage and API", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "8ef6295243a6eb4075d02c4a712c2e6d1e66e869", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/1.1/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/1221/mbox/", "series": [ { "id": 307, "url": "https://patchwork.libcamera.org/api/1.1/series/307/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=307", "date": "2019-05-17T23:06:09", "name": "Rework camera configuration to introduce negotiation of parameters", "version": 1, "mbox": "https://patchwork.libcamera.org/series/307/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/1221/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/1221/checks/", "tags": {}, "headers": { "Return-Path": "<laurent.pinchart@ideasonboard.com>", "Received": [ "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 52E9860E4C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 18 May 2019 01:06:44 +0200 (CEST)", "from pendragon.bb.dnainternet.fi\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DB9E5336\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 18 May 2019 01:06:43 +0200 (CEST)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1558134404;\n\tbh=n+rBe2oFZrtdlzRe/k94cKvG9e0y4QFu4/tYQpJRPkE=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=eHx2jvDp2kw2tAZWp0DRYxAuXOcvS133hcCtl5LtDduaYRCAChpV8hNLi8tiU4/Wk\n\t3kNHIwk2+3V2vUPlNtdWQv7B+6u+qj2DtmxXtESnTXVEsmOkHHs44cDNjAvRu7OfzF\n\t3lceMWU9N+tr50SMR0TucKWJbLiFWsPUFaAVrdYE=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Sat, 18 May 2019 02:06:13 +0300", "Message-Id": "<20190517230621.24668-5-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.21.0", "In-Reply-To": "<20190517230621.24668-1-laurent.pinchart@ideasonboard.com>", "References": "<20190517230621.24668-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH/RFC 04/12] libcamera: Refactor the camera\n\tconfiguration storage and API", "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": "Fri, 17 May 2019 23:06:44 -0000" }, "content": "Refactor the CameraConfiguration structure to not rely on Stream\ninstances. This is a step towards making the camera configuration object\nmore powerful with configuration validation using \"try\" semantics.\n\nAs applications need access to the Stream instances associated with the\nconfiguration entries in order to associate buffers with streams when\ncreating requests, expose the stream selected by the pipeline handler\nthrough a new StreamConfiguration::stream().\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n include/libcamera/camera.h | 19 ++-\n include/libcamera/stream.h | 7 +\n src/cam/main.cpp | 35 ++---\n src/libcamera/camera.cpp | 174 +++++++++--------------\n src/libcamera/include/pipeline_handler.h | 2 +-\n src/libcamera/pipeline/ipu3/ipu3.cpp | 32 ++---\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 +-\n src/libcamera/pipeline/uvcvideo.cpp | 23 ++-\n src/libcamera/pipeline/vimc.cpp | 23 ++-\n src/libcamera/pipeline_handler.cpp | 4 +\n src/libcamera/stream.cpp | 22 +++\n src/qcam/main_window.cpp | 4 +-\n test/camera/capture.cpp | 4 +-\n test/camera/configuration_set.cpp | 2 +-\n 14 files changed, 179 insertions(+), 184 deletions(-)", "diff": "diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\nindex 42ba5201eabc..e2759405f497 100644\n--- a/include/libcamera/camera.h\n+++ b/include/libcamera/camera.h\n@@ -25,11 +25,13 @@ class Request;\n class CameraConfiguration\n {\n public:\n-\tusing iterator = std::vector<Stream *>::iterator;\n-\tusing const_iterator = std::vector<Stream *>::const_iterator;\n+\tusing iterator = std::vector<StreamConfiguration>::iterator;\n+\tusing const_iterator = std::vector<StreamConfiguration>::const_iterator;\n \n \tCameraConfiguration();\n \n+\tvoid addConfiguration(const StreamConfiguration &cfg);\n+\n \titerator begin();\n \titerator end();\n \tconst_iterator begin() const;\n@@ -39,16 +41,11 @@ public:\n \tbool isEmpty() const;\n \tstd::size_t size() const;\n \n-\tStream *front();\n-\tconst Stream *front() const;\n-\n-\tStream *operator[](unsigned int index) const;\n-\tStreamConfiguration &operator[](Stream *stream);\n-\tconst StreamConfiguration &operator[](Stream *stream) const;\n+\tStreamConfiguration &operator[](unsigned int index);\n+\tconst StreamConfiguration &operator[](unsigned int index) const;\n \n private:\n-\tstd::vector<Stream *> order_;\n-\tstd::map<Stream *, StreamConfiguration> config_;\n+\tstd::vector<StreamConfiguration> config_;\n };\n \n class Camera final\n@@ -72,7 +69,7 @@ public:\n \n \tconst std::set<Stream *> &streams() const;\n \tCameraConfiguration generateConfiguration(const StreamRoles &roles);\n-\tint configure(const CameraConfiguration &config);\n+\tint configure(CameraConfiguration &config);\n \n \tint allocateBuffers();\n \tint freeBuffers();\ndiff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\nindex 59bdf217eb31..47c007ed52e2 100644\n--- a/include/libcamera/stream.h\n+++ b/include/libcamera/stream.h\n@@ -16,6 +16,7 @@\n namespace libcamera {\n \n class Camera;\n+class Stream;\n \n struct StreamConfiguration {\n \tunsigned int pixelFormat;\n@@ -23,7 +24,13 @@ struct StreamConfiguration {\n \n \tunsigned int bufferCount;\n \n+\tStream *stream() const { return stream_; }\n+\tvoid setStream(Stream *stream) { stream_ = stream; }\n+\n \tstd::string toString() const;\n+\n+private:\n+\tStream *stream_;\n };\n \n enum StreamRole {\ndiff --git a/src/cam/main.cpp b/src/cam/main.cpp\nindex d603228c0116..cd165bea34cd 100644\n--- a/src/cam/main.cpp\n+++ b/src/cam/main.cpp\n@@ -89,12 +89,9 @@ static int prepareCameraConfig(CameraConfiguration *config)\n {\n \tStreamRoles roles;\n \n-\tstreamInfo.clear();\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\tstreamInfo[config->front()] = \"stream0\";\n \t\treturn 0;\n \t}\n \n@@ -129,27 +126,20 @@ static int prepareCameraConfig(CameraConfiguration *config)\n \t}\n \n \t/* Apply configuration explicitly requested. */\n-\tCameraConfiguration::iterator it = config->begin();\n+\tunsigned int i = 0;\n \tfor (auto const &value : streamOptions) {\n \t\tKeyValueParser::Options conf = value.toKeyValues();\n-\t\tStream *stream = *it;\n-\t\tit++;\n+\t\tStreamConfiguration &cfg = (*config)[i++];\n \n \t\tif (conf.isSet(\"width\"))\n-\t\t\t(*config)[stream].size.width = conf[\"width\"];\n+\t\t\tcfg.size.width = conf[\"width\"];\n \n \t\tif (conf.isSet(\"height\"))\n-\t\t\t(*config)[stream].size.height = conf[\"height\"];\n+\t\t\tcfg.size.height = conf[\"height\"];\n \n \t\t/* TODO: Translate 4CC string to ID. */\n \t\tif (conf.isSet(\"pixelformat\"))\n-\t\t\t(*config)[stream].pixelFormat = conf[\"pixelformat\"];\n-\t}\n-\n-\tunsigned int index = 0;\n-\tfor (Stream *stream : *config) {\n-\t\tstreamInfo[stream] = \"stream\" + std::to_string(index);\n-\t\tindex++;\n+\t\t\tcfg.pixelFormat = conf[\"pixelformat\"];\n \t}\n \n \treturn 0;\n@@ -216,6 +206,13 @@ static int capture()\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+\t\tstreamInfo[cfg.stream()] = \"stream\" + std::to_string(index);\n+\t}\n+\n \tret = camera->allocateBuffers();\n \tif (ret) {\n \t\tstd::cerr << \"Failed to allocate buffers\"\n@@ -227,8 +224,10 @@ static int capture()\n \n \t/* Identify the stream with the least number of buffers. */\n \tunsigned int nbuffers = UINT_MAX;\n-\tfor (Stream *stream : config)\n+\tfor (StreamConfiguration &cfg : config) {\n+\t\tStream *stream = cfg.stream();\n \t\tnbuffers = std::min(nbuffers, stream->bufferPool().count());\n+\t}\n \n \t/*\n \t * TODO: make cam tool smarter to support still capture by for\n@@ -245,8 +244,10 @@ static int capture()\n \t\t}\n \n \t\tstd::map<Stream *, Buffer *> map;\n-\t\tfor (Stream *stream : 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 \n \t\tret = request->setBuffers(map);\n \t\tif (ret < 0) {\ndiff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\nindex a3921f91f1c9..f8a4946b4a6a 100644\n--- a/src/libcamera/camera.cpp\n+++ b/src/libcamera/camera.cpp\n@@ -46,72 +46,81 @@ LOG_DECLARE_CATEGORY(Camera)\n * \\class CameraConfiguration\n * \\brief Hold configuration for streams of the camera\n \n- * The CameraConfiguration holds an ordered list of streams and their associated\n- * StreamConfiguration. From a data storage point of view, the class operates as\n- * a map of Stream pointers to StreamConfiguration, with entries accessed with\n- * operator[](Stream *). Accessing an entry for a Stream pointer not yet stored\n- * in the configuration inserts a new empty entry.\n- *\n- * The class also suppors iterators, and from that point of view operates as a\n- * vector of Stream pointers. The streams are iterated in insertion order, and\n- * the operator[](int) returns the Stream pointer based on its insertion index.\n- * Accessing a stream with an invalid index returns a null pointer.\n+ * The CameraConfiguration holds an ordered list of stream configurations. It\n+ * supports iterators and operates as a vector of StreamConfiguration instances.\n+ * The stream configurations are inserted by addConfiguration(), and the\n+ * operator[](int) returns a reference to the StreamConfiguration based on its\n+ * insertion index. Accessing a stream configuration with an invalid index\n+ * results in undefined behaviour.\n */\n \n /**\n * \\typedef CameraConfiguration::iterator\n- * \\brief Iterator for the streams in the configuration\n+ * \\brief Iterator for the stream configurations in the camera configuration\n */\n \n /**\n * \\typedef CameraConfiguration::const_iterator\n- * \\brief Const iterator for the streams in the configuration\n+ * \\brief Const iterator for the stream configuration in the camera\n+ * configuration\n */\n \n /**\n * \\brief Create an empty camera configuration\n */\n CameraConfiguration::CameraConfiguration()\n-\t: order_({}), config_({})\n+\t: config_({})\n {\n }\n \n /**\n- * \\brief Retrieve an iterator to the first stream in the sequence\n- * \\return An iterator to the first stream\n+ * \\brief Add a stream configuration to the camera configuration\n+ * \\param[in] cfg The stream configuration\n */\n-std::vector<Stream *>::iterator CameraConfiguration::begin()\n+void CameraConfiguration::addConfiguration(const StreamConfiguration &cfg)\n {\n-\treturn order_.begin();\n+\tconfig_.push_back(cfg);\n }\n \n /**\n- * \\brief Retrieve an iterator pointing to the past-the-end stream in the\n+ * \\brief Retrieve an iterator to the first stream configuration in the\n * sequence\n- * \\return An iterator to the element following the last stream\n+ * \\return An iterator to the first stream configuration\n */\n-std::vector<Stream *>::iterator CameraConfiguration::end()\n+CameraConfiguration::iterator CameraConfiguration::begin()\n {\n-\treturn order_.end();\n+\treturn config_.begin();\n }\n \n /**\n- * \\brief Retrieve a const iterator to the first element of the streams\n- * \\return A const iterator to the first stream\n+ * \\brief Retrieve an iterator pointing to the past-the-end stream\n+ * configuration in the sequence\n+ * \\return An iterator to the element following the last stream configuration\n */\n-std::vector<Stream *>::const_iterator CameraConfiguration::begin() const\n+CameraConfiguration::iterator CameraConfiguration::end()\n {\n-\treturn order_.begin();\n+\treturn config_.end();\n }\n \n /**\n- * \\brief Retrieve a const iterator pointing to the past-the-end stream in the\n- * sequence\n+ * \\brief Retrieve a const iterator to the first element of the stream\n+ * configurations\n+ * \\return A const iterator to the first stream configuration\n+ */\n+CameraConfiguration::const_iterator CameraConfiguration::begin() const\n+{\n+\treturn config_.begin();\n+}\n+\n+/**\n+ * \\brief Retrieve a const iterator pointing to the past-the-end stream\n+ * configuration in the sequence\n * \\return A const iterator to the element following the last stream\n+ * configuration\n */\n-std::vector<Stream *>::const_iterator CameraConfiguration::end() const\n+CameraConfiguration::const_iterator CameraConfiguration::end() const\n {\n-\treturn order_.end();\n+\treturn config_.end();\n }\n \n /**\n@@ -128,9 +137,7 @@ bool CameraConfiguration::isValid() const\n \tif (isEmpty())\n \t\treturn false;\n \n-\tfor (auto const &it : config_) {\n-\t\tconst StreamConfiguration &cfg = it.second;\n-\n+\tfor (const StreamConfiguration &cfg : config_) {\n \t\tif (cfg.size.width == 0 || cfg.size.height == 0 ||\n \t\t cfg.pixelFormat == 0 || cfg.bufferCount == 0)\n \t\t\treturn false;\n@@ -145,7 +152,7 @@ bool CameraConfiguration::isValid() const\n */\n bool CameraConfiguration::isEmpty() const\n {\n-\treturn order_.empty();\n+\treturn config_.empty();\n }\n \n /**\n@@ -154,75 +161,37 @@ bool CameraConfiguration::isEmpty() const\n */\n std::size_t CameraConfiguration::size() const\n {\n-\treturn order_.size();\n-}\n-\n-/**\n- * \\brief Access the first stream in the configuration\n- * \\return The first stream in the configuration\n- */\n-Stream *CameraConfiguration::front()\n-{\n-\treturn order_.front();\n-}\n-\n-/**\n- * \\brief Access the first stream in the configuration\n- * \\return The first const stream pointer in the configuration\n- */\n-const Stream *CameraConfiguration::front() const\n-{\n-\treturn order_.front();\n-}\n-\n-/**\n- * \\brief Retrieve a stream pointer from index\n- * \\param[in] index Numerical index\n- *\n- * The \\a index represents the zero based insertion order of stream and stream\n- * configuration into the camera configuration.\n- *\n- * \\return The stream pointer at index, or a nullptr if the index is out of\n- * bounds\n- */\n-Stream *CameraConfiguration::operator[](unsigned int index) const\n-{\n-\tif (index >= order_.size())\n-\t\treturn nullptr;\n-\n-\treturn order_.at(index);\n+\treturn config_.size();\n }\n \n /**\n * \\brief Retrieve a reference to a stream configuration\n- * \\param[in] stream Stream to retrieve configuration for\n+ * \\param[in] index Numerical index\n *\n- * If the camera configuration does not yet contain a configuration for\n- * the requested stream, create and return an empty stream configuration.\n+ * The \\a index represents the zero based insertion order of stream\n+ * configuration into the camera configuration with addConfiguration(). Calling\n+ * this method with an invalid index results in undefined behaviour.\n *\n- * \\return The configuration for the stream\n+ * \\return The stream configuration\n */\n-StreamConfiguration &CameraConfiguration::operator[](Stream *stream)\n+StreamConfiguration &CameraConfiguration::operator[](unsigned int index)\n {\n-\tif (config_.find(stream) == config_.end())\n-\t\torder_.push_back(stream);\n-\n-\treturn config_[stream];\n+\treturn config_[index];\n }\n \n /**\n * \\brief Retrieve a const reference to a stream configuration\n- * \\param[in] stream Stream to retrieve configuration for\n+ * \\param[in] index Numerical index\n *\n- * No new stream configuration is created if called with \\a stream that is not\n- * already part of the camera configuration, doing so is an invalid operation\n- * and results in undefined behaviour.\n+ * The \\a index represents the zero based insertion order of stream\n+ * configuration into the camera configuration with addConfiguration(). Calling\n+ * this method with an invalid index results in undefined behaviour.\n *\n- * \\return The configuration for the stream\n+ * \\return The stream configuration\n */\n-const StreamConfiguration &CameraConfiguration::operator[](Stream *stream) const\n+const StreamConfiguration &CameraConfiguration::operator[](unsigned int index) const\n {\n-\treturn config_.at(stream);\n+\treturn config_[index];\n }\n \n /**\n@@ -561,13 +530,9 @@ Camera::generateConfiguration(const StreamRoles &roles)\n \tCameraConfiguration config = pipe_->generateConfiguration(this, roles);\n \n \tstd::ostringstream msg(\"streams configuration:\", std::ios_base::ate);\n-\tunsigned int index = 0;\n \n-\tfor (Stream *stream : config) {\n-\t\tconst StreamConfiguration &cfg = config[stream];\n-\t\tmsg << \" (\" << index << \") \" << cfg.toString();\n-\t\tindex++;\n-\t}\n+\tfor (unsigned int index = 0; index < config.size(); ++index)\n+\t\tmsg << \" (\" << index << \") \" << config[index].toString();\n \n \tLOG(Camera, Debug) << msg.str();\n \n@@ -593,12 +558,15 @@ Camera::generateConfiguration(const StreamRoles &roles)\n *\n * This function affects the state of the camera, see \\ref camera_operation.\n *\n+ * Upon return the StreamConfiguration entries in \\a config are associated with\n+ * Stream instances which can be retrieved with StreamConfiguration::stream().\n+ *\n * \\return 0 on success or a negative error code otherwise\n * \\retval -ENODEV The camera has been disconnected from the system\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(const CameraConfiguration &config)\n+int Camera::configure(CameraConfiguration &config)\n {\n \tint ret;\n \n@@ -615,16 +583,11 @@ int Camera::configure(const CameraConfiguration &config)\n \t}\n \n \tstd::ostringstream msg(\"configuring streams:\", std::ios_base::ate);\n-\tunsigned int index = 0;\n \n-\tfor (Stream *stream : config) {\n-\t\tif (streams_.find(stream) == streams_.end())\n-\t\t\treturn -EINVAL;\n-\n-\t\tconst StreamConfiguration &cfg = config[stream];\n-\t\tmsg << std::dec << \" (\" << index << \") \" << cfg.toString();\n-\n-\t\tindex++;\n+\tfor (unsigned int index = 0; index < config.size(); ++index) {\n+\t\tStreamConfiguration &cfg = config[index];\n+\t\tcfg.setStream(nullptr);\n+\t\tmsg << \" (\" << index << \") \" << cfg.toString();\n \t}\n \n \tLOG(Camera, Info) << msg.str();\n@@ -634,8 +597,11 @@ int Camera::configure(const CameraConfiguration &config)\n \t\treturn ret;\n \n \tactiveStreams_.clear();\n-\tfor (Stream *stream : config) {\n-\t\tconst StreamConfiguration &cfg = config[stream];\n+\tfor (const StreamConfiguration &cfg : config) {\n+\t\tStream *stream = cfg.stream();\n+\t\tif (!stream)\n+\t\t\tLOG(Camera, Fatal)\n+\t\t\t\t<< \"Pipeline handler failed to update stream configuration\";\n \n \t\tstream->configuration_ = cfg;\n \t\tactiveStreams_.insert(stream);\ndiff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\nindex 3352cb0e5bc9..a025905ab68f 100644\n--- a/src/libcamera/include/pipeline_handler.h\n+++ b/src/libcamera/include/pipeline_handler.h\n@@ -62,7 +62,7 @@ public:\n \n \tvirtual CameraConfiguration\n \tgenerateConfiguration(Camera *camera, const StreamRoles &roles) = 0;\n-\tvirtual int configure(Camera *camera, const CameraConfiguration &config) = 0;\n+\tvirtual int configure(Camera *camera, CameraConfiguration &config) = 0;\n \n \tvirtual int allocateBuffers(Camera *camera,\n \t\t\t\t const std::set<Stream *> &streams) = 0;\ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex d234a8ac5289..ed0ef69de1d1 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -152,8 +152,7 @@ public:\n \n \tCameraConfiguration\n \tgenerateConfiguration(Camera *camera, const StreamRoles &roles) override;\n-\tint configure(Camera *camera,\n-\t\t const CameraConfiguration &config) 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@@ -299,14 +298,13 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera,\n \t\tcfg.pixelFormat = V4L2_PIX_FMT_NV12;\n \t\tcfg.bufferCount = IPU3_BUFFER_COUNT;\n \n-\t\tconfig[stream] = cfg;\n+\t\tconfig.addConfiguration(cfg);\n \t}\n \n \treturn config;\n }\n \n-int PipelineHandlerIPU3::configure(Camera *camera,\n-\t\t\t\t const CameraConfiguration &config)\n+int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config)\n {\n \tIPU3CameraData *data = cameraData(camera);\n \tIPU3Stream *outStream = &data->outStream_;\n@@ -318,9 +316,13 @@ int PipelineHandlerIPU3::configure(Camera *camera,\n \n \toutStream->active_ = false;\n \tvfStream->active_ = false;\n-\tfor (Stream *s : config) {\n-\t\tIPU3Stream *stream = static_cast<IPU3Stream *>(s);\n-\t\tconst StreamConfiguration &cfg = config[stream];\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+\t\t * reworked when implementing camera configuration validation.\n+\t\t */\n+\t\tIPU3Stream *stream = vfStream->active_ ? outStream : vfStream;\n \n \t\t/*\n \t\t * Verify that the requested size respects the IPU3 alignment\n@@ -355,6 +357,7 @@ int PipelineHandlerIPU3::configure(Camera *camera,\n \t\t\tsensorSize.height = cfg.size.height;\n \n \t\tstream->active_ = true;\n+\t\tcfg.setStream(stream);\n \t}\n \n \t/*\n@@ -379,10 +382,9 @@ int PipelineHandlerIPU3::configure(Camera *camera,\n \t\treturn ret;\n \n \t/* Apply the format to the configured streams output devices. */\n-\tfor (Stream *s : config) {\n-\t\tIPU3Stream *stream = static_cast<IPU3Stream *>(s);\n-\n-\t\tret = imgu->configureOutput(stream->device_, config[stream]);\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 \t\t\treturn ret;\n \t}\n@@ -393,15 +395,13 @@ int PipelineHandlerIPU3::configure(Camera *camera,\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_,\n-\t\t\t\t\t config[vfStream]);\n+\t\tret = imgu->configureOutput(outStream->device_, config[0]);\n \t\tif (ret)\n \t\t\treturn ret;\n \t}\n \n \tif (!vfStream->active_) {\n-\t\tret = imgu->configureOutput(vfStream->device_,\n-\t\t\t\t\t config[outStream]);\n+\t\tret = imgu->configureOutput(vfStream->device_, config[0]);\n \t\tif (ret)\n \t\t\treturn ret;\n \t}\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 05dd517846d6..6b35aa3fe7c3 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -36,8 +36,7 @@ public:\n \n \tCameraConfiguration generateConfiguration(Camera *camera,\n \t\tconst StreamRoles &roles) override;\n-\tint configure(Camera *camera,\n-\t\tconst 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@@ -117,16 +116,15 @@ CameraConfiguration PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n \tcfg.size = data->sensor_->resolution();\n \tcfg.bufferCount = RKISP1_BUFFER_COUNT;\n \n-\tconfig[&data->stream_] = cfg;\n+\tconfig.addConfiguration(cfg);\n \n \treturn config;\n }\n \n-int PipelineHandlerRkISP1::configure(Camera *camera,\n-\t\t\t\t const CameraConfiguration &config)\n+int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration &config)\n {\n \tRkISP1CameraData *data = cameraData(camera);\n-\tconst StreamConfiguration &cfg = config[&data->stream_];\n+\tStreamConfiguration &cfg = config[0];\n \tCameraSensor *sensor = data->sensor_;\n \tint ret;\n \n@@ -220,6 +218,8 @@ int PipelineHandlerRkISP1::configure(Camera *camera,\n \t\treturn -EINVAL;\n \t}\n \n+\tcfg.setStream(&data->stream_);\n+\n \treturn 0;\n }\n \ndiff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\nindex d2e1f7d4e5b2..5dcc868b2fc9 100644\n--- a/src/libcamera/pipeline/uvcvideo.cpp\n+++ b/src/libcamera/pipeline/uvcvideo.cpp\n@@ -27,8 +27,7 @@ public:\n \n \tCameraConfiguration\n \tgenerateConfiguration(Camera *camera, const StreamRoles &roles) override;\n-\tint configure(Camera *camera,\n-\t\t const CameraConfiguration &config) 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@@ -78,38 +77,38 @@ CameraConfiguration\n PipelineHandlerUVC::generateConfiguration(Camera *camera,\n \t\t\t\t\t const StreamRoles &roles)\n {\n-\tUVCCameraData *data = cameraData(camera);\n \tCameraConfiguration config;\n-\tStreamConfiguration cfg{};\n+\tStreamConfiguration cfg;\n \n \tcfg.pixelFormat = V4L2_PIX_FMT_YUYV;\n \tcfg.size = { 640, 480 };\n \tcfg.bufferCount = 4;\n \n-\tconfig[&data->stream_] = cfg;\n+\tconfig.addConfiguration(cfg);\n \n \treturn config;\n }\n \n-int PipelineHandlerUVC::configure(Camera *camera,\n-\t\t\t\t const CameraConfiguration &config)\n+int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration &config)\n {\n \tUVCCameraData *data = cameraData(camera);\n-\tconst StreamConfiguration *cfg = &config[&data->stream_];\n+\tStreamConfiguration &cfg = config[0];\n \tint ret;\n \n \tV4L2DeviceFormat format = {};\n-\tformat.fourcc = cfg->pixelFormat;\n-\tformat.size = cfg->size;\n+\tformat.fourcc = cfg.pixelFormat;\n+\tformat.size = cfg.size;\n \n \tret = data->video_->setFormat(&format);\n \tif (ret)\n \t\treturn ret;\n \n-\tif (format.size != cfg->size ||\n-\t format.fourcc != cfg->pixelFormat)\n+\tif (format.size != cfg.size ||\n+\t format.fourcc != cfg.pixelFormat)\n \t\treturn -EINVAL;\n \n+\tcfg.setStream(&data->stream_);\n+\n \treturn 0;\n }\n \ndiff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\nindex 17e2491e5c27..af6b6f21e3c5 100644\n--- a/src/libcamera/pipeline/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc.cpp\n@@ -27,8 +27,7 @@ public:\n \n \tCameraConfiguration\n \tgenerateConfiguration(Camera *camera, const StreamRoles &roles) override;\n-\tint configure(Camera *camera,\n-\t\t const CameraConfiguration &config) 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@@ -78,38 +77,38 @@ CameraConfiguration\n PipelineHandlerVimc::generateConfiguration(Camera *camera,\n \t\t\t\t\t const StreamRoles &roles)\n {\n-\tVimcCameraData *data = cameraData(camera);\n \tCameraConfiguration config;\n-\tStreamConfiguration cfg{};\n+\tStreamConfiguration cfg;\n \n \tcfg.pixelFormat = V4L2_PIX_FMT_RGB24;\n \tcfg.size = { 640, 480 };\n \tcfg.bufferCount = 4;\n \n-\tconfig[&data->stream_] = cfg;\n+\tconfig.addConfiguration(cfg);\n \n \treturn config;\n }\n \n-int PipelineHandlerVimc::configure(Camera *camera,\n-\t\t\t\t const CameraConfiguration &config)\n+int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration &config)\n {\n \tVimcCameraData *data = cameraData(camera);\n-\tconst StreamConfiguration *cfg = &config[&data->stream_];\n+\tStreamConfiguration &cfg = config[0];\n \tint ret;\n \n \tV4L2DeviceFormat format = {};\n-\tformat.fourcc = cfg->pixelFormat;\n-\tformat.size = cfg->size;\n+\tformat.fourcc = cfg.pixelFormat;\n+\tformat.size = cfg.size;\n \n \tret = data->video_->setFormat(&format);\n \tif (ret)\n \t\treturn ret;\n \n-\tif (format.size != cfg->size ||\n-\t format.fourcc != cfg->pixelFormat)\n+\tif (format.size != cfg.size ||\n+\t format.fourcc != cfg.pixelFormat)\n \t\treturn -EINVAL;\n \n+\tcfg.setStream(&data->stream_);\n+\n \treturn 0;\n }\n \ndiff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\nindex 81c11149c9fe..4185e3557dcb 100644\n--- a/src/libcamera/pipeline_handler.cpp\n+++ b/src/libcamera/pipeline_handler.cpp\n@@ -255,6 +255,10 @@ void PipelineHandler::unlock()\n * configuration of a subset of the streams can't be satisfied, the\n * whole configuration is considered invalid.\n *\n+ * Once the configuration is validated and the camera configured, the pipeline\n+ * handler shall associate a Stream instance to each StreamConfiguration entry\n+ * in the CameraConfiguration with the StreamConfiguration::setStream() method.\n+ *\n * \\return 0 on success or a negative error code otherwise\n */\n \ndiff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\nindex fe4c4ecf4150..0c59a31a3a05 100644\n--- a/src/libcamera/stream.cpp\n+++ b/src/libcamera/stream.cpp\n@@ -58,6 +58,28 @@ namespace libcamera {\n * \\brief Requested number of buffers to allocate for the stream\n */\n \n+/**\n+ * \\fn StreamConfiguration::stream()\n+ * \\brief Retrieve the stream associated with the configuration\n+ *\n+ * When a camera is configured with Camera::configure() Stream instances are\n+ * associated with each stream configuration entry. This method retrieves the\n+ * associated Stream, which remains valid until the next call to\n+ * Camera::configure() or Camera::release().\n+ *\n+ * \\return The stream associated with the configuration\n+ */\n+\n+/**\n+ * \\fn StreamConfiguration::setStream()\n+ * \\brief Associate a stream with a configuration\n+ *\n+ * This method is meant for the PipelineHandler::configure() method and shall\n+ * not be called by applications.\n+ *\n+ * \\param[in] stream The stream\n+ */\n+\n /**\n * \\brief Assemble and return a string describing the configuration\n *\ndiff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\nindex a984aaca764f..06ae2985f80d 100644\n--- a/src/qcam/main_window.cpp\n+++ b/src/qcam/main_window.cpp\n@@ -98,14 +98,14 @@ int MainWindow::startCapture()\n \tint ret;\n \n \tconfig_ = camera_->generateConfiguration({ StreamRole::VideoRecording });\n-\tStream *stream = config_.front();\n \tret = camera_->configure(config_);\n \tif (ret < 0) {\n \t\tstd::cout << \"Failed to configure camera\" << std::endl;\n \t\treturn ret;\n \t}\n \n-\tconst StreamConfiguration &cfg = config_[stream];\n+\tconst StreamConfiguration &cfg = config_[0];\n+\tStream *stream = cfg.stream();\n \tret = viewfinder_->setFormat(cfg.pixelFormat, cfg.size.width,\n \t\t\t\t cfg.size.height);\n \tif (ret < 0) {\ndiff --git a/test/camera/capture.cpp b/test/camera/capture.cpp\nindex e7e6438203b9..bfd11eefedcf 100644\n--- a/test/camera/capture.cpp\n+++ b/test/camera/capture.cpp\n@@ -44,8 +44,7 @@ protected:\n \t{\n \t\tCameraConfiguration config =\n \t\t\tcamera_->generateConfiguration({ StreamRole::VideoRecording });\n-\t\tStream *stream = config.front();\n-\t\tStreamConfiguration *cfg = &config[stream];\n+\t\tStreamConfiguration *cfg = &config[0];\n \n \t\tif (!config.isValid()) {\n \t\t\tcout << \"Failed to read default configuration\" << endl;\n@@ -67,6 +66,7 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \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()) {\ndiff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp\nindex 76d8bc3e40a4..25b5db67103a 100644\n--- a/test/camera/configuration_set.cpp\n+++ b/test/camera/configuration_set.cpp\n@@ -20,7 +20,7 @@ protected:\n \t{\n \t\tCameraConfiguration config =\n \t\t\tcamera_->generateConfiguration({ StreamRole::VideoRecording });\n-\t\tStreamConfiguration *cfg = &config[config.front()];\n+\t\tStreamConfiguration *cfg = &config[0];\n \n \t\tif (!config.isValid()) {\n \t\t\tcout << \"Failed to read default configuration\" << endl;\n", "prefixes": [ "libcamera-devel", "PATCH/RFC", "04/12" ] }