[libcamera-devel] [PATCH 5/5] libcamera: camera: Add support for stream roles
Jacopo Mondi
jacopo at jmondi.org
Wed Apr 3 15:20:04 CEST 2019
Hi Niklas,
On Wed, Apr 03, 2019 at 03:12:21AM +0200, Niklas Söderlund wrote:
> Instead of requesting the default configuration for a set of streams
> where the application has to figure out which streams provided by the
> camera is best suited for its intended usage, have the library figure
> this out by using stream roles.
>
> The application asks the library for a list of streams and a suggested
> default configuration for them by supplying a list of stream roles. Once
> the list is retrieved the application can try and fine-tune the returned
s/can try and/can/ ?
> configuration and then try to apply it to the camera.
>
> Currently no pipeline handler is prepared to handle stream roles but nor
> did it make use of the list of Stream IDs which was the previous
> interface. The main reason for this is that all cameras currently only
> provide one stream each. This will still be the case but the API will be
> prepared to expand both pipeline handlers and applications to support
> streams roles.
Let's see what goes in first, but you might want to change this if
IPU3 multi-stream support gets in first.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> include/libcamera/camera.h | 3 ++-
> src/cam/main.cpp | 2 +-
> src/libcamera/camera.cpp | 30 ++++++++----------------
> src/libcamera/include/pipeline_handler.h | 2 +-
> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--
> src/libcamera/pipeline/uvcvideo.cpp | 6 ++---
> src/libcamera/pipeline/vimc.cpp | 6 ++---
> src/libcamera/pipeline_handler.cpp | 8 +++----
> src/qcam/main_window.cpp | 5 ++--
> test/camera/capture.cpp | 5 ++--
> test/camera/configuration_default.cpp | 17 +++++---------
> test/camera/configuration_set.cpp | 3 +--
> test/camera/statemachine.cpp | 4 +---
> 13 files changed, 36 insertions(+), 59 deletions(-)
>
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 77e4cd1ee14c2c61..cafc9673db457148 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -22,6 +22,7 @@ class PipelineHandler;
> class Request;
> class Stream;
> class StreamConfiguration;
> +class StreamRole;
>
> class Camera final
> {
> @@ -44,7 +45,7 @@ public:
>
> const std::set<Stream *> &streams() const;
> std::map<Stream *, StreamConfiguration>
> - streamConfiguration(std::set<Stream *> &streams);
> + streamConfiguration(const std::vector<StreamRole> &roles);
> int configureStreams(std::map<Stream *, StreamConfiguration> &config);
>
> int allocateBuffers();
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index b5895fae85699b26..030003f081bdddda 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -81,7 +81,7 @@ static int parseOptions(int argc, char *argv[])
> static int prepareCameraConfig(std::map<Stream *, StreamConfiguration> *config)
> {
> std::set<Stream *> streams = camera->streams();
> - *config = camera->streamConfiguration(streams);
> + *config = camera->streamConfiguration({ Stream::VideoRecording() });
> Stream *stream = config->begin()->first;
>
> if (options.isSet(OptFormat)) {
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 8ee9cc0866167ae1..1f613590b579360c 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -345,33 +345,23 @@ const std::set<Stream *> &Camera::streams() const
> }
>
> /**
> - * \brief Retrieve a group of stream configurations
> - * \param[in] streams A map of stream IDs and configurations to setup
> + * \brief Retrieve a group of stream configurations according to stream roles
I would
"Associate a stream and its default configurations to each provided
stream role in the \a roles list.
> + * \param[in] roles A list of stream roles
Not on this set specifically but, as per the first/third person thing,
this could be "A list" or "The list". We have both, we should pick one
going forward.
> *
> - * Retrieve the camera's configuration for a specified group of streams. The
> - * caller can specifies which of the camera's streams to retrieve configuration
> - * from by populating \a streams.
> + * Retrieve configuration for a set of desired usages. The caller specifies a
> + * list of stream roles and the camera returns a map of suitable streams and
> + * their suggested default configurations.
> *
> - * The easiest way to populate the array of streams to fetch configuration from
> - * is to first retrieve the camera's full array of stream with streams() and
> - * then potentially trim it down to only contain the streams the caller
> - * are interested in.
> - *
> - * \return A map of successfully retrieved stream IDs and configurations or an
> - * empty list on error.
> + * \return A map of streams to configurations if the requested roles can be
> + * satisfied, or an empty map otherwise
> */
> std::map<Stream *, StreamConfiguration>
> -Camera::streamConfiguration(std::set<Stream *> &streams)
> +Camera::streamConfiguration(const std::vector<StreamRole> &roles)
> {
> - if (disconnected_ || !streams.size())
> + if (disconnected_ || !roles.size() || roles.size() > streams_.size())
> return std::map<Stream *, StreamConfiguration>{};
>
> - for (Stream *stream : streams) {
> - if (streams_.find(stream) == streams_.end())
> - return std::map<Stream *, StreamConfiguration>{};
> - }
> -
> - return pipe_->streamConfiguration(this, streams);
> + return pipe_->streamConfiguration(this, roles);
> }
>
> /**
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index acb376e07030ae03..cfb0a1147e0764c8 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -53,7 +53,7 @@ public:
> virtual bool match(DeviceEnumerator *enumerator) = 0;
>
> virtual std::map<Stream *, StreamConfiguration>
> - streamConfiguration(Camera *camera, std::set<Stream *> &streams) = 0;
> + streamConfiguration(Camera *camera, const std::vector<StreamRole> &roles) = 0;
> virtual int configureStreams(Camera *camera,
> std::map<Stream *, StreamConfiguration> &config) = 0;
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 55489c31df2d0382..a707aefbc0f9b956 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -32,7 +32,7 @@ public:
>
> std::map<Stream *, StreamConfiguration>
> streamConfiguration(Camera *camera,
> - std::set<Stream *> &streams) override;
> + const std::vector<StreamRole> &roles) override;
> int configureStreams(Camera *camera,
> std::map<Stream *, StreamConfiguration> &config) override;
>
> @@ -100,7 +100,7 @@ PipelineHandlerIPU3::~PipelineHandlerIPU3()
>
> std::map<Stream *, StreamConfiguration>
> PipelineHandlerIPU3::streamConfiguration(Camera *camera,
> - std::set<Stream *> &streams)
> + const std::vector<StreamRole> &roles)
> {
> IPU3CameraData *data = cameraData(camera);
> std::map<Stream *, StreamConfiguration> configs;
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index cc3e0cd9afab38b9..cf0581e5be3e0a91 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -28,7 +28,7 @@ public:
>
> std::map<Stream *, StreamConfiguration>
> streamConfiguration(Camera *camera,
> - std::set<Stream *> &streams) override;
> + const std::vector<StreamRole> &roles) override;
> int configureStreams(Camera *camera,
> std::map<Stream *, StreamConfiguration> &config) override;
>
> @@ -84,14 +84,12 @@ PipelineHandlerUVC::~PipelineHandlerUVC()
>
> std::map<Stream *, StreamConfiguration>
> PipelineHandlerUVC::streamConfiguration(Camera *camera,
> - std::set<Stream *> &streams)
> + const std::vector<StreamRole> &roles)
> {
> UVCCameraData *data = cameraData(camera);
> -
> std::map<Stream *, StreamConfiguration> configs;
> StreamConfiguration config{};
>
> - LOG(UVC, Debug) << "Retrieving default format";
unrelated but welcome :)
> config.width = 640;
> config.height = 480;
> config.pixelFormat = V4L2_PIX_FMT_YUYV;
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index 2e8c26fb7c0b2708..721f2f3f4f39b689 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -28,7 +28,7 @@ public:
>
> std::map<Stream *, StreamConfiguration>
> streamConfiguration(Camera *camera,
> - std::set<Stream *> &streams) override;
> + const std::vector<StreamRole> &roles) override;
> int configureStreams(Camera *camera,
> std::map<Stream *, StreamConfiguration> &config) override;
>
> @@ -84,14 +84,12 @@ PipelineHandlerVimc::~PipelineHandlerVimc()
>
> std::map<Stream *, StreamConfiguration>
> PipelineHandlerVimc::streamConfiguration(Camera *camera,
> - std::set<Stream *> &streams)
> + const std::vector<StreamRole> &roles)
> {
> VimcCameraData *data = cameraData(camera);
> std::map<Stream *, StreamConfiguration> configs;
> -
> StreamConfiguration config{};
>
> - LOG(VIMC, Debug) << "Retrieving default format";
> config.width = 640;
> config.height = 480;
> config.pixelFormat = V4L2_PIX_FMT_RGB24;
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 1a858f2638ceac5f..738f87b8a22fc151 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -152,12 +152,12 @@ PipelineHandler::~PipelineHandler()
> * \fn PipelineHandler::streamConfiguration()
> * \brief Retrieve a group of stream configurations for a specified camera
> * \param[in] camera The camera to fetch default configuration from
> - * \param[in] streams An array of streams to fetch information about
> + * \param[in] roles A list of stream roles
> *
> * Retrieve the species camera's default configuration for a specified group of
Unrelated to this patch, but: what's "species" ?
> - * streams. The caller shall populate the \a streams array with the streams it
> - * wish to fetch the configuration from. The map of streams and configuration
> - * returned can then be examined by the caller to learn about the defualt
> + * use-cases. The caller shall populate the \a roles array with the use-cases it
> + * wishes to fetch the configuration for. The map of streams and configuration
s/to fecth the configuration for/to fetch a default configuration for/ ?
Thanks
j
> + * returned can then be examined by the caller to learn about the default
> * parameters for the specified streams.
> *
> * The intended companion to this is \a configureStreams() which can be used to
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index fea701422015ce67..faa3bc5739dd8453 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -97,9 +97,8 @@ int MainWindow::startCapture()
> {
> int ret;
>
> - Stream *stream = *camera_->streams().begin();
> - std::set<Stream *> streams{ stream };
> - config_ = camera_->streamConfiguration(streams);
> + config_ = camera_->streamConfiguration({ Stream::VideoRecording() });
> + Stream *stream = config_.begin()->first;
> ret = camera_->configureStreams(config_);
> if (ret < 0) {
> std::cout << "Failed to configure camera" << std::endl;
> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> index f6932b7505571712..b8dbdb62f9a50a33 100644
> --- a/test/camera/capture.cpp
> +++ b/test/camera/capture.cpp
> @@ -42,10 +42,9 @@ protected:
>
> int run()
> {
> - Stream *stream = *camera_->streams().begin();
> - std::set<Stream *> streams = { stream };
> std::map<Stream *, StreamConfiguration> conf =
> - camera_->streamConfiguration(streams);
> + camera_->streamConfiguration({ Stream::VideoRecording() });
> + Stream *stream = conf.begin()->first;
> StreamConfiguration *sconf = &conf.begin()->second;
>
> if (!configurationValid(conf)) {
> diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp
> index 856cd415f7a6aec1..2d7bfc2e227c689f 100644
> --- a/test/camera/configuration_default.cpp
> +++ b/test/camera/configuration_default.cpp
> @@ -20,14 +20,10 @@ protected:
> {
> std::map<Stream *, StreamConfiguration> conf;
>
> - /*
> - * Test that asking for default configuration for a valid
> - * array of streams returns something valid.
> - */
> - std::set<Stream *> streams = { *camera_->streams().begin() };
> - conf = camera_->streamConfiguration(streams);
> + /* Test asking for configuration for a video stream. */
> + conf = camera_->streamConfiguration({ Stream::VideoRecording() });
> if (conf.empty()) {
> - cout << "Failed to retrieve configuration for valid streams"
> + cout << "Failed to retrieve configuration for video streams"
> << endl;
> return TestFail;
> }
> @@ -39,12 +35,11 @@ protected:
>
> /*
> * Test that asking for configuration for an empty array of
> - * streams returns an empty list of configurations.
> + * stream roles returns an empty list of configurations.
> */
> - std::set<Stream *> streams_empty = {};
> - conf = camera_->streamConfiguration(streams_empty);
> + conf = camera_->streamConfiguration({});
> if (!conf.empty()) {
> - cout << "Failed to retrieve configuration for empty streams"
> + cout << "Failed to retrieve configuration for empty usage list"
> << endl;
> return TestFail;
> }
> diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp
> index cac1da959f241bc5..1bc01e66625eedf0 100644
> --- a/test/camera/configuration_set.cpp
> +++ b/test/camera/configuration_set.cpp
> @@ -18,9 +18,8 @@ class ConfigurationSet : public CameraTest
> protected:
> int run()
> {
> - std::set<Stream *> streams = { *camera_->streams().begin() };
> std::map<Stream *, StreamConfiguration> conf =
> - camera_->streamConfiguration(streams);
> + camera_->streamConfiguration({ Stream::VideoRecording() });
> StreamConfiguration *sconf = &conf.begin()->second;
>
> if (!configurationValid(conf)) {
> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
> index f4395f2b1bfed698..ab3c6fb5bea38c36 100644
> --- a/test/camera/statemachine.cpp
> +++ b/test/camera/statemachine.cpp
> @@ -235,9 +235,7 @@ protected:
>
> int run()
> {
> - Stream *stream = *camera_->streams().begin();
> - std::set<Stream *> streams = { stream };
> - defconf_ = camera_->streamConfiguration(streams);
> + defconf_ = camera_->streamConfiguration({ Stream::VideoRecording() });
>
> if (testAvailable() != TestPass) {
> cout << "State machine in Available state failed" << endl;
> --
> 2.21.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190403/4d46c10c/attachment.sig>
More information about the libcamera-devel
mailing list