[{"id":1242,"web_url":"https://patchwork.libcamera.org/comment/1242/","msgid":"<20190403070326.GE4813@pendragon.ideasonboard.com>","date":"2019-04-03T07:04:02","subject":"Re: [libcamera-devel] [PATCH 5/5] libcamera: camera: Add support\n\tfor stream roles","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Apr 03, 2019 at 03:12:21AM +0200, Niklas Söderlund wrote:\n> Instead of requesting the default configuration for a set of streams\n> where the application has to figure out which streams provided by the\n> camera is best suited for its intended usage, have the library figure\n> this out by using stream roles.\n> \n> The application asks the library for a list of streams and a suggested\n> default configuration for them by supplying a list of stream roles. Once\n> the list is retrieved the application can try and fine-tune the returned\n> configuration and then try to apply it to the camera.\n> \n> Currently no pipeline handler is prepared to handle stream roles but nor\n> did it make use of the list of Stream IDs which was the previous\n> interface. The main reason for this is that all cameras currently only\n> provide one stream each. This will still be the case but the API will be\n> prepared to expand both pipeline handlers and applications to support\n> streams roles.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> include/libcamera/camera.h | 3 ++-\n> src/cam/main.cpp | 2 +-\n> src/libcamera/camera.cpp | 30 ++++++++----------------\n> src/libcamera/include/pipeline_handler.h | 2 +-\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--\n> src/libcamera/pipeline/uvcvideo.cpp | 6 ++---\n> src/libcamera/pipeline/vimc.cpp | 6 ++---\n> src/libcamera/pipeline_handler.cpp | 8 +++----\n> src/qcam/main_window.cpp | 5 ++--\n> test/camera/capture.cpp | 5 ++--\n> test/camera/configuration_default.cpp | 17 +++++---------\n> test/camera/configuration_set.cpp | 3 +--\n> test/camera/statemachine.cpp | 4 +---\n> 13 files changed, 36 insertions(+), 59 deletions(-)\n> \n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 77e4cd1ee14c2c61..cafc9673db457148 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -22,6 +22,7 @@ class PipelineHandler;\n> class Request;\n> class Stream;\n> class StreamConfiguration;\n> +class StreamRole;\n> \n> class Camera final\n> {\n> @@ -44,7 +45,7 @@ public:\n> \n> \tconst std::set &streams() const;\n> \tstd::map\n> -\tstreamConfiguration(std::set &streams);\n> +\tstreamConfiguration(const std::vector &roles);\n> \tint configureStreams(std::map &config);\n> \n> \tint allocateBuffers();\n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index b5895fae85699b26..030003f081bdddda 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -81,7 +81,7 @@ static int parseOptions(int argc, char *argv[])\n> static int prepareCameraConfig(std::map *config)\n> {\n> \tstd::set streams = camera->streams();\n> -\t*config = camera->streamConfiguration(streams);\n> +\t*config = camera->streamConfiguration({ Stream::VideoRecording() });\n> \tStream *stream = config->begin()->first;\n> \n> \tif (options.isSet(OptFormat)) {\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 8ee9cc0866167ae1..1f613590b579360c 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -345,33 +345,23 @@ const std::set &Camera::streams() const\n> }\n> \n> /**\n> - * \\brief Retrieve a group of stream configurations\n> - * \\param[in] streams A map of stream IDs and configurations to setup\n> + * \\brief Retrieve a group of stream configurations according to stream roles\n> + * \\param[in] roles A list of stream roles\n> *\n> - * Retrieve the camera's configuration for a specified group of streams. The\n> - * caller can specifies which of the camera's streams to retrieve configuration\n> - * from by populating \\a streams.\n> + * Retrieve configuration for a set of desired usages. The caller specifies a\n> + * list of stream roles and the camera returns a map of suitable streams and\n> + * their suggested default configurations.\n> *\n> - * The easiest way to populate the array of streams to fetch configuration from\n> - * is to first retrieve the camera's full array of stream with streams() and\n> - * then potentially trim it down to only contain the streams the caller\n> - * are interested in.\n> - *\n> - * \\return A map of successfully retrieved stream IDs and configurations or an\n> - * empty list on error.\n> + * \\return A map of streams to configurations if the requested roles can be\n> + * satisfied, or an empty map otherwise\n> */\n> std::map\n> -Camera::streamConfiguration(std::set &streams)\n> +Camera::streamConfiguration(const std::vector &roles)\n> {\n> -\tif (disconnected_ || !streams.size())\n> +\tif (disconnected_ || !roles.size() || roles.size() > streams_.size())\n> \t\treturn std::map{};\n> \n> -\tfor (Stream *stream : streams) {\n> -\t\tif (streams_.find(stream) == streams_.end())\n> -\t\t\treturn std::map{};\n> -\t}\n> -\n> -\treturn pipe_->streamConfiguration(this, streams);\n> +\treturn pipe_->streamConfiguration(this, roles);\n> }\n> \n> /**\n> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> index acb376e07030ae03..cfb0a1147e0764c8 100644\n> --- a/src/libcamera/include/pipeline_handler.h\n> +++ b/src/libcamera/include/pipeline_handler.h\n> @@ -53,7 +53,7 @@ public:\n> \tvirtual bool match(DeviceEnumerator *enumerator) = 0;\n> \n> \tvirtual std::map\n> -\tstreamConfiguration(Camera *camera, std::set &streams) = 0;\n> +\tstreamConfiguration(Camera *camera, const std::vector &roles) = 0;\n> \tvirtual int configureStreams(Camera *camera,\n> \t\t\t\t std::map &config) = 0;\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 55489c31df2d0382..a707aefbc0f9b956 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -32,7 +32,7 @@ public:\n> \n> \tstd::map\n> \tstreamConfiguration(Camera *camera,\n> -\t\t\t std::set &streams) override;\n> +\t\t\t const std::vector &roles) override;\n> \tint configureStreams(Camera *camera,\n> \t\t\t std::map &config) override;\n> \n> @@ -100,7 +100,7 @@ PipelineHandlerIPU3::~PipelineHandlerIPU3()\n> \n> std::map\n> PipelineHandlerIPU3::streamConfiguration(Camera *camera,\n> -\t\t\t\t\t std::set &streams)\n> +\t\t\t\t\t const std::vector &roles)\n> {\n> \tIPU3CameraData *data = cameraData(camera);\n> \tstd::map configs;\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index cc3e0cd9afab38b9..cf0581e5be3e0a91 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -28,7 +28,7 @@ public:\n> \n> \tstd::map\n> \tstreamConfiguration(Camera *camera,\n> -\t\t\t std::set &streams) override;\n> +\t\t\t const std::vector &roles) override;\n> \tint configureStreams(Camera *camera,\n> \t\t\t std::map &config) override;\n> \n> @@ -84,14 +84,12 @@ PipelineHandlerUVC::~PipelineHandlerUVC()\n> \n> std::map\n> PipelineHandlerUVC::streamConfiguration(Camera *camera,\n> -\t\t\t\t\tstd::set &streams)\n> +\t\t\t\t\tconst std::vector &roles)\n> {\n> \tUVCCameraData *data = cameraData(camera);\n> -\n> \tstd::map configs;\n> \tStreamConfiguration config{};\n> \n> -\tLOG(UVC, Debug) << \"Retrieving default format\";\n> \tconfig.width = 640;\n> \tconfig.height = 480;\n> \tconfig.pixelFormat = V4L2_PIX_FMT_YUYV;\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index 2e8c26fb7c0b2708..721f2f3f4f39b689 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -28,7 +28,7 @@ public:\n> \n> \tstd::map\n> \tstreamConfiguration(Camera *camera,\n> -\t\t\t std::set &streams) override;\n> +\t\t\t const std::vector &roles) override;\n> \tint configureStreams(Camera *camera,\n> \t\t\t std::map &config) override;\n> \n> @@ -84,14 +84,12 @@ PipelineHandlerVimc::~PipelineHandlerVimc()\n> \n> std::map\n> PipelineHandlerVimc::streamConfiguration(Camera *camera,\n> -\t\t\t\t\t std::set &streams)\n> +\t\t\t\t\t const std::vector &roles)\n> {\n> \tVimcCameraData *data = cameraData(camera);\n> \tstd::map configs;\n> -\n> \tStreamConfiguration config{};\n> \n> -\tLOG(VIMC, Debug) << \"Retrieving default format\";\n> \tconfig.width = 640;\n> \tconfig.height = 480;\n> \tconfig.pixelFormat = V4L2_PIX_FMT_RGB24;\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 1a858f2638ceac5f..738f87b8a22fc151 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -152,12 +152,12 @@ PipelineHandler::~PipelineHandler()\n> * \\fn PipelineHandler::streamConfiguration()\n> * \\brief Retrieve a group of stream configurations for a specified camera\n> * \\param[in] camera The camera to fetch default configuration from\n> - * \\param[in] streams An array of streams to fetch information about\n> + * \\param[in] roles A list of stream roles\n> *\n> * Retrieve the species camera's default configuration for a specified group of\n> - * streams. The caller shall populate the \\a streams array with the streams it\n> - * wish to fetch the configuration from. The map of streams and configuration\n> - * returned can then be examined by the caller to learn about the defualt\n> + * use-cases. The caller shall populate the \\a roles array with the use-cases it\n> + * wishes to fetch the configuration for. The map of streams and configuration\n\ns/configuration/configurations/\n\nReviewed-by: Laurent Pinchart \n\nassuming you will send patches to fix the stream role to stream\nconfiguration match issue.\n\n> + * returned can then be examined by the caller to learn about the default\n> * parameters for the specified streams.\n> *\n> * The intended companion to this is \\a configureStreams() which can be used to\n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index fea701422015ce67..faa3bc5739dd8453 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -97,9 +97,8 @@ int MainWindow::startCapture()\n> {\n> \tint ret;\n> \n> -\tStream *stream = *camera_->streams().begin();\n> -\tstd::set streams{ stream };\n> -\tconfig_ = camera_->streamConfiguration(streams);\n> +\tconfig_ = camera_->streamConfiguration({ Stream::VideoRecording() });\n> +\tStream *stream = config_.begin()->first;\n> \tret = camera_->configureStreams(config_);\n> \tif (ret < 0) {\n> \t\tstd::cout << \"Failed to configure camera\" << std::endl;\n> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp\n> index f6932b7505571712..b8dbdb62f9a50a33 100644\n> --- a/test/camera/capture.cpp\n> +++ b/test/camera/capture.cpp\n> @@ -42,10 +42,9 @@ protected:\n> \n> \tint run()\n> \t{\n> -\t\tStream *stream = *camera_->streams().begin();\n> -\t\tstd::set streams = { stream };\n> \t\tstd::map conf =\n> -\t\t\tcamera_->streamConfiguration(streams);\n> +\t\t\tcamera_->streamConfiguration({ Stream::VideoRecording() });\n> +\t\tStream *stream = conf.begin()->first;\n> \t\tStreamConfiguration *sconf = &conf.begin()->second;\n> \n> \t\tif (!configurationValid(conf)) {\n> diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp\n> index 856cd415f7a6aec1..2d7bfc2e227c689f 100644\n> --- a/test/camera/configuration_default.cpp\n> +++ b/test/camera/configuration_default.cpp\n> @@ -20,14 +20,10 @@ protected:\n> \t{\n> \t\tstd::map conf;\n> \n> -\t\t/*\n> -\t\t * Test that asking for default configuration for a valid\n> -\t\t * array of streams returns something valid.\n> -\t\t */\n> -\t\tstd::set streams = { *camera_->streams().begin() };\n> -\t\tconf = camera_->streamConfiguration(streams);\n> +\t\t/* Test asking for configuration for a video stream. */\n> +\t\tconf = camera_->streamConfiguration({ Stream::VideoRecording() });\n> \t\tif (conf.empty()) {\n> -\t\t\tcout << \"Failed to retrieve configuration for valid streams\"\n> +\t\t\tcout << \"Failed to retrieve configuration for video streams\"\n> \t\t\t << endl;\n> \t\t\treturn TestFail;\n> \t\t}\n> @@ -39,12 +35,11 @@ protected:\n> \n> \t\t/*\n> \t\t * Test that asking for configuration for an empty array of\n> -\t\t * streams returns an empty list of configurations.\n> +\t\t * stream roles returns an empty list of configurations.\n> \t\t */\n> -\t\tstd::set streams_empty = {};\n> -\t\tconf = camera_->streamConfiguration(streams_empty);\n> +\t\tconf = camera_->streamConfiguration({});\n> \t\tif (!conf.empty()) {\n> -\t\t\tcout << \"Failed to retrieve configuration for empty streams\"\n> +\t\t\tcout << \"Failed to retrieve configuration for empty usage list\"\n> \t\t\t << endl;\n> \t\t\treturn TestFail;\n> \t\t}\n> diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp\n> index cac1da959f241bc5..1bc01e66625eedf0 100644\n> --- a/test/camera/configuration_set.cpp\n> +++ b/test/camera/configuration_set.cpp\n> @@ -18,9 +18,8 @@ class ConfigurationSet : public CameraTest\n> protected:\n> \tint run()\n> \t{\n> -\t\tstd::set streams = { *camera_->streams().begin() };\n> \t\tstd::map conf =\n> -\t\t\tcamera_->streamConfiguration(streams);\n> +\t\t\tcamera_->streamConfiguration({ Stream::VideoRecording() });\n> \t\tStreamConfiguration *sconf = &conf.begin()->second;\n> \n> \t\tif (!configurationValid(conf)) {\n> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp\n> index f4395f2b1bfed698..ab3c6fb5bea38c36 100644\n> --- a/test/camera/statemachine.cpp\n> +++ b/test/camera/statemachine.cpp\n> @@ -235,9 +235,7 @@ protected:\n> \n> \tint run()\n> \t{\n> -\t\tStream *stream = *camera_->streams().begin();\n> -\t\tstd::set streams = { stream };\n> -\t\tdefconf_ = camera_->streamConfiguration(streams);\n> +\t\tdefconf_ = camera_->streamConfiguration({ Stream::VideoRecording() });\n> \n> \t\tif (testAvailable() != TestPass) {\n> \t\t\tcout << \"State machine in Available state failed\" << endl;","headers":{"Return-Path":"","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 9D09F60DB2\n\tfor ;\n\tWed, 3 Apr 2019 09:04:13 +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 E9E4B323;\n\tWed, 3 Apr 2019 09:04:12 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1554275053;\n\tbh=9QW1CtryhELDCgUM6WJKBBmbmFurEhF47Tr/6b5jSfA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=vWAI864eG2g7R9G5O90FXZR1JIuhgzroP8JUyxvRqiPcS3Cp2f7+lysSlO220HoJe\n\t0ldNEgm4i2gpHOofyIYWmx6W5zeJBEPGk9EggekXLoVqt1Q4sbp7X8h0Ccf0XHprzY\n\tSvGoEOYA+xBFpWqVRsPB48tAghHlKjTzKjS67ofM=","Date":"Wed, 3 Apr 2019 10:04:02 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190403070326.GE4813@pendragon.ideasonboard.com>","References":"<20190403011221.12711-1-niklas.soderlund@ragnatech.se>\n\t<20190403011221.12711-6-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190403011221.12711-6-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 5/5] libcamera: camera: Add support\n\tfor stream roles","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 03 Apr 2019 07:04:13 -0000"}},{"id":1251,"web_url":"https://patchwork.libcamera.org/comment/1251/","msgid":"<20190403132004.xnh362cq34bysp2x@uno.localdomain>","date":"2019-04-03T13:20:04","subject":"Re: [libcamera-devel] [PATCH 5/5] libcamera: camera: Add support\n\tfor stream roles","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Wed, Apr 03, 2019 at 03:12:21AM +0200, Niklas Söderlund wrote:\n> Instead of requesting the default configuration for a set of streams\n> where the application has to figure out which streams provided by the\n> camera is best suited for its intended usage, have the library figure\n> this out by using stream roles.\n>\n> The application asks the library for a list of streams and a suggested\n> default configuration for them by supplying a list of stream roles. Once\n> the list is retrieved the application can try and fine-tune the returned\ns/can try and/can/ ?\n> configuration and then try to apply it to the camera.\n>\n> Currently no pipeline handler is prepared to handle stream roles but nor\n> did it make use of the list of Stream IDs which was the previous\n> interface. The main reason for this is that all cameras currently only\n> provide one stream each. This will still be the case but the API will be\n> prepared to expand both pipeline handlers and applications to support\n> streams roles.\n\nLet's see what goes in first, but you might want to change this if\nIPU3 multi-stream support gets in first.\n\n>\n> Signed-off-by: Niklas Söderlund \n> ---\n> include/libcamera/camera.h | 3 ++-\n> src/cam/main.cpp | 2 +-\n> src/libcamera/camera.cpp | 30 ++++++++----------------\n> src/libcamera/include/pipeline_handler.h | 2 +-\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--\n> src/libcamera/pipeline/uvcvideo.cpp | 6 ++---\n> src/libcamera/pipeline/vimc.cpp | 6 ++---\n> src/libcamera/pipeline_handler.cpp | 8 +++----\n> src/qcam/main_window.cpp | 5 ++--\n> test/camera/capture.cpp | 5 ++--\n> test/camera/configuration_default.cpp | 17 +++++---------\n> test/camera/configuration_set.cpp | 3 +--\n> test/camera/statemachine.cpp | 4 +---\n> 13 files changed, 36 insertions(+), 59 deletions(-)\n>\n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 77e4cd1ee14c2c61..cafc9673db457148 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -22,6 +22,7 @@ class PipelineHandler;\n> class Request;\n> class Stream;\n> class StreamConfiguration;\n> +class StreamRole;\n>\n> class Camera final\n> {\n> @@ -44,7 +45,7 @@ public:\n>\n> \tconst std::set &streams() const;\n> \tstd::map\n> -\tstreamConfiguration(std::set &streams);\n> +\tstreamConfiguration(const std::vector &roles);\n> \tint configureStreams(std::map &config);\n>\n> \tint allocateBuffers();\n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index b5895fae85699b26..030003f081bdddda 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -81,7 +81,7 @@ static int parseOptions(int argc, char *argv[])\n> static int prepareCameraConfig(std::map *config)\n> {\n> \tstd::set streams = camera->streams();\n> -\t*config = camera->streamConfiguration(streams);\n> +\t*config = camera->streamConfiguration({ Stream::VideoRecording() });\n> \tStream *stream = config->begin()->first;\n>\n> \tif (options.isSet(OptFormat)) {\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 8ee9cc0866167ae1..1f613590b579360c 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -345,33 +345,23 @@ const std::set &Camera::streams() const\n> }\n>\n> /**\n> - * \\brief Retrieve a group of stream configurations\n> - * \\param[in] streams A map of stream IDs and configurations to setup\n> + * \\brief Retrieve a group of stream configurations according to stream roles\n\nI would\n\"Associate a stream and its default configurations to each provided\nstream role in the \\a roles list.\n\n> + * \\param[in] roles A list of stream roles\n\nNot on this set specifically but, as per the first/third person thing,\nthis could be \"A list\" or \"The list\". We have both, we should pick one\ngoing forward.\n\n> *\n> - * Retrieve the camera's configuration for a specified group of streams. The\n> - * caller can specifies which of the camera's streams to retrieve configuration\n> - * from by populating \\a streams.\n> + * Retrieve configuration for a set of desired usages. The caller specifies a\n> + * list of stream roles and the camera returns a map of suitable streams and\n> + * their suggested default configurations.\n> *\n> - * The easiest way to populate the array of streams to fetch configuration from\n> - * is to first retrieve the camera's full array of stream with streams() and\n> - * then potentially trim it down to only contain the streams the caller\n> - * are interested in.\n> - *\n> - * \\return A map of successfully retrieved stream IDs and configurations or an\n> - * empty list on error.\n> + * \\return A map of streams to configurations if the requested roles can be\n> + * satisfied, or an empty map otherwise\n> */\n> std::map\n> -Camera::streamConfiguration(std::set &streams)\n> +Camera::streamConfiguration(const std::vector &roles)\n> {\n> -\tif (disconnected_ || !streams.size())\n> +\tif (disconnected_ || !roles.size() || roles.size() > streams_.size())\n> \t\treturn std::map{};\n>\n> -\tfor (Stream *stream : streams) {\n> -\t\tif (streams_.find(stream) == streams_.end())\n> -\t\t\treturn std::map{};\n> -\t}\n> -\n> -\treturn pipe_->streamConfiguration(this, streams);\n> +\treturn pipe_->streamConfiguration(this, roles);\n> }\n>\n> /**\n> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h\n> index acb376e07030ae03..cfb0a1147e0764c8 100644\n> --- a/src/libcamera/include/pipeline_handler.h\n> +++ b/src/libcamera/include/pipeline_handler.h\n> @@ -53,7 +53,7 @@ public:\n> \tvirtual bool match(DeviceEnumerator *enumerator) = 0;\n>\n> \tvirtual std::map\n> -\tstreamConfiguration(Camera *camera, std::set &streams) = 0;\n> +\tstreamConfiguration(Camera *camera, const std::vector &roles) = 0;\n> \tvirtual int configureStreams(Camera *camera,\n> \t\t\t\t std::map &config) = 0;\n>\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 55489c31df2d0382..a707aefbc0f9b956 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -32,7 +32,7 @@ public:\n>\n> \tstd::map\n> \tstreamConfiguration(Camera *camera,\n> -\t\t\t std::set &streams) override;\n> +\t\t\t const std::vector &roles) override;\n> \tint configureStreams(Camera *camera,\n> \t\t\t std::map &config) override;\n>\n> @@ -100,7 +100,7 @@ PipelineHandlerIPU3::~PipelineHandlerIPU3()\n>\n> std::map\n> PipelineHandlerIPU3::streamConfiguration(Camera *camera,\n> -\t\t\t\t\t std::set &streams)\n> +\t\t\t\t\t const std::vector &roles)\n> {\n> \tIPU3CameraData *data = cameraData(camera);\n> \tstd::map configs;\n> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\n> index cc3e0cd9afab38b9..cf0581e5be3e0a91 100644\n> --- a/src/libcamera/pipeline/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo.cpp\n> @@ -28,7 +28,7 @@ public:\n>\n> \tstd::map\n> \tstreamConfiguration(Camera *camera,\n> -\t\t\t std::set &streams) override;\n> +\t\t\t const std::vector &roles) override;\n> \tint configureStreams(Camera *camera,\n> \t\t\t std::map &config) override;\n>\n> @@ -84,14 +84,12 @@ PipelineHandlerUVC::~PipelineHandlerUVC()\n>\n> std::map\n> PipelineHandlerUVC::streamConfiguration(Camera *camera,\n> -\t\t\t\t\tstd::set &streams)\n> +\t\t\t\t\tconst std::vector &roles)\n> {\n> \tUVCCameraData *data = cameraData(camera);\n> -\n> \tstd::map configs;\n> \tStreamConfiguration config{};\n>\n> -\tLOG(UVC, Debug) << \"Retrieving default format\";\n\nunrelated but welcome :)\n\n> \tconfig.width = 640;\n> \tconfig.height = 480;\n> \tconfig.pixelFormat = V4L2_PIX_FMT_YUYV;\n> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\n> index 2e8c26fb7c0b2708..721f2f3f4f39b689 100644\n> --- a/src/libcamera/pipeline/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc.cpp\n> @@ -28,7 +28,7 @@ public:\n>\n> \tstd::map\n> \tstreamConfiguration(Camera *camera,\n> -\t\t\t std::set &streams) override;\n> +\t\t\t const std::vector &roles) override;\n> \tint configureStreams(Camera *camera,\n> \t\t\t std::map &config) override;\n>\n> @@ -84,14 +84,12 @@ PipelineHandlerVimc::~PipelineHandlerVimc()\n>\n> std::map\n> PipelineHandlerVimc::streamConfiguration(Camera *camera,\n> -\t\t\t\t\t std::set &streams)\n> +\t\t\t\t\t const std::vector &roles)\n> {\n> \tVimcCameraData *data = cameraData(camera);\n> \tstd::map configs;\n> -\n> \tStreamConfiguration config{};\n>\n> -\tLOG(VIMC, Debug) << \"Retrieving default format\";\n> \tconfig.width = 640;\n> \tconfig.height = 480;\n> \tconfig.pixelFormat = V4L2_PIX_FMT_RGB24;\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 1a858f2638ceac5f..738f87b8a22fc151 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -152,12 +152,12 @@ PipelineHandler::~PipelineHandler()\n> * \\fn PipelineHandler::streamConfiguration()\n> * \\brief Retrieve a group of stream configurations for a specified camera\n> * \\param[in] camera The camera to fetch default configuration from\n> - * \\param[in] streams An array of streams to fetch information about\n> + * \\param[in] roles A list of stream roles\n> *\n> * Retrieve the species camera's default configuration for a specified group of\n\nUnrelated to this patch, but: what's \"species\" ?\n\n> - * streams. The caller shall populate the \\a streams array with the streams it\n> - * wish to fetch the configuration from. The map of streams and configuration\n> - * returned can then be examined by the caller to learn about the defualt\n> + * use-cases. The caller shall populate the \\a roles array with the use-cases it\n> + * wishes to fetch the configuration for. The map of streams and configuration\n\ns/to fecth the configuration for/to fetch a default configuration for/ ?\n\nThanks\n j\n\n> + * returned can then be examined by the caller to learn about the default\n> * parameters for the specified streams.\n> *\n> * The intended companion to this is \\a configureStreams() which can be used to\n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index fea701422015ce67..faa3bc5739dd8453 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -97,9 +97,8 @@ int MainWindow::startCapture()\n> {\n> \tint ret;\n>\n> -\tStream *stream = *camera_->streams().begin();\n> -\tstd::set streams{ stream };\n> -\tconfig_ = camera_->streamConfiguration(streams);\n> +\tconfig_ = camera_->streamConfiguration({ Stream::VideoRecording() });\n> +\tStream *stream = config_.begin()->first;\n> \tret = camera_->configureStreams(config_);\n> \tif (ret < 0) {\n> \t\tstd::cout << \"Failed to configure camera\" << std::endl;\n> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp\n> index f6932b7505571712..b8dbdb62f9a50a33 100644\n> --- a/test/camera/capture.cpp\n> +++ b/test/camera/capture.cpp\n> @@ -42,10 +42,9 @@ protected:\n>\n> \tint run()\n> \t{\n> -\t\tStream *stream = *camera_->streams().begin();\n> -\t\tstd::set streams = { stream };\n> \t\tstd::map conf =\n> -\t\t\tcamera_->streamConfiguration(streams);\n> +\t\t\tcamera_->streamConfiguration({ Stream::VideoRecording() });\n> +\t\tStream *stream = conf.begin()->first;\n> \t\tStreamConfiguration *sconf = &conf.begin()->second;\n>\n> \t\tif (!configurationValid(conf)) {\n> diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp\n> index 856cd415f7a6aec1..2d7bfc2e227c689f 100644\n> --- a/test/camera/configuration_default.cpp\n> +++ b/test/camera/configuration_default.cpp\n> @@ -20,14 +20,10 @@ protected:\n> \t{\n> \t\tstd::map conf;\n>\n> -\t\t/*\n> -\t\t * Test that asking for default configuration for a valid\n> -\t\t * array of streams returns something valid.\n> -\t\t */\n> -\t\tstd::set streams = { *camera_->streams().begin() };\n> -\t\tconf = camera_->streamConfiguration(streams);\n> +\t\t/* Test asking for configuration for a video stream. */\n> +\t\tconf = camera_->streamConfiguration({ Stream::VideoRecording() });\n> \t\tif (conf.empty()) {\n> -\t\t\tcout << \"Failed to retrieve configuration for valid streams\"\n> +\t\t\tcout << \"Failed to retrieve configuration for video streams\"\n> \t\t\t << endl;\n> \t\t\treturn TestFail;\n> \t\t}\n> @@ -39,12 +35,11 @@ protected:\n>\n> \t\t/*\n> \t\t * Test that asking for configuration for an empty array of\n> -\t\t * streams returns an empty list of configurations.\n> +\t\t * stream roles returns an empty list of configurations.\n> \t\t */\n> -\t\tstd::set streams_empty = {};\n> -\t\tconf = camera_->streamConfiguration(streams_empty);\n> +\t\tconf = camera_->streamConfiguration({});\n> \t\tif (!conf.empty()) {\n> -\t\t\tcout << \"Failed to retrieve configuration for empty streams\"\n> +\t\t\tcout << \"Failed to retrieve configuration for empty usage list\"\n> \t\t\t << endl;\n> \t\t\treturn TestFail;\n> \t\t}\n> diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp\n> index cac1da959f241bc5..1bc01e66625eedf0 100644\n> --- a/test/camera/configuration_set.cpp\n> +++ b/test/camera/configuration_set.cpp\n> @@ -18,9 +18,8 @@ class ConfigurationSet : public CameraTest\n> protected:\n> \tint run()\n> \t{\n> -\t\tstd::set streams = { *camera_->streams().begin() };\n> \t\tstd::map conf =\n> -\t\t\tcamera_->streamConfiguration(streams);\n> +\t\t\tcamera_->streamConfiguration({ Stream::VideoRecording() });\n> \t\tStreamConfiguration *sconf = &conf.begin()->second;\n>\n> \t\tif (!configurationValid(conf)) {\n> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp\n> index f4395f2b1bfed698..ab3c6fb5bea38c36 100644\n> --- a/test/camera/statemachine.cpp\n> +++ b/test/camera/statemachine.cpp\n> @@ -235,9 +235,7 @@ protected:\n>\n> \tint run()\n> \t{\n> -\t\tStream *stream = *camera_->streams().begin();\n> -\t\tstd::set streams = { stream };\n> -\t\tdefconf_ = camera_->streamConfiguration(streams);\n> +\t\tdefconf_ = camera_->streamConfiguration({ Stream::VideoRecording() });\n>\n> \t\tif (testAvailable() != TestPass) {\n> \t\t\tcout << \"State machine in Available state failed\" << endl;\n> --\n> 2.21.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CE89B600FB\n\tfor ;\n\tWed, 3 Apr 2019 15:19:18 +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 relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 41D74C001E;\n\tWed, 3 Apr 2019 13:19:17 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Wed, 3 Apr 2019 15:20:04 +0200","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190403132004.xnh362cq34bysp2x@uno.localdomain>","References":"<20190403011221.12711-1-niklas.soderlund@ragnatech.se>\n\t<20190403011221.12711-6-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"sbluuyo7v2mal7cn\"","Content-Disposition":"inline","In-Reply-To":"<20190403011221.12711-6-niklas.soderlund@ragnatech.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 5/5] libcamera: camera: Add support\n\tfor stream roles","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 03 Apr 2019 13:19:19 -0000"}}]