[libcamera-devel] [PATCH v3 8/8] libcamera: Switch to CameraConfiguration
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Apr 6 19:08:24 CEST 2019
Hi Niklas,
Thank you for the patch.
On Sat, Apr 06, 2019 at 01:58:42AM +0200, Niklas Söderlund wrote:
> Implement the camera configuration thru out the library, tests, cam and
> qcam tools.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> include/libcamera/camera.h | 4 ++--
> src/cam/main.cpp | 8 ++++----
> src/libcamera/camera.cpp | 25 ++++++++++++------------
> src/libcamera/include/pipeline_handler.h | 6 +++---
> src/libcamera/pipeline/ipu3/ipu3.cpp | 10 +++++-----
> src/libcamera/pipeline/uvcvideo.cpp | 12 ++++++------
> src/libcamera/pipeline/vimc.cpp | 12 ++++++------
> src/libcamera/pipeline_handler.cpp | 6 +++---
> src/qcam/main_window.cpp | 2 +-
> src/qcam/main_window.h | 2 +-
> test/camera/camera_test.cpp | 18 -----------------
> test/camera/camera_test.h | 2 --
> test/camera/capture.cpp | 8 ++++----
> test/camera/configuration_default.cpp | 12 +++---------
> test/camera/configuration_set.cpp | 6 +++---
> test/camera/statemachine.cpp | 2 +-
> 16 files changed, 54 insertions(+), 81 deletions(-)
>
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 8455049151d5c5a2..06c7d2e6ae63fa70 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -72,9 +72,9 @@ public:
> int release();
>
> const std::set<Stream *> &streams() const;
> - std::map<Stream *, StreamConfiguration>
> + CameraConfiguration
> streamConfiguration(const std::vector<StreamUsage> &usage);
> - int configureStreams(std::map<Stream *, StreamConfiguration> &config);
> + int configureStreams(const CameraConfiguration &config);
>
> int allocateBuffers();
> int freeBuffers();
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index d45ffd372d932d76..99ce564afd695b07 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -78,10 +78,10 @@ static int parseOptions(int argc, char *argv[])
> return 0;
> }
>
> -static int prepareCameraConfig(std::map<Stream *, StreamConfiguration> *config)
> +static int prepareCameraConfig(CameraConfiguration *config)
> {
> *config = camera->streamConfiguration({ Stream::VideoRecording() });
> - Stream *stream = config->begin()->first;
> + Stream *stream = config->front();
>
> if (options.isSet(OptFormat)) {
> KeyValueParser::Options format = options[OptFormat];
> @@ -135,7 +135,7 @@ static void requestComplete(Request *request, const std::map<Stream *, Buffer *>
>
> static int capture()
> {
> - std::map<Stream *, StreamConfiguration> config;
> + CameraConfiguration config;
> std::vector<Request *> requests;
> int ret;
>
> @@ -151,7 +151,7 @@ static int capture()
> return ret;
> }
>
> - Stream *stream = config.begin()->first;
> + Stream *stream = config.front();
>
> ret = camera->allocateBuffers();
> if (ret) {
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 98145edea1ac9c91..e357bca11214969a 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -525,21 +525,21 @@ const std::set<Stream *> &Camera::streams() const
> * list of stream usages and the camera returns a map of suitable streams and
> * their suggested default configurations.
> *
> - * \return A map of streams to configurations if the requested usages can be
> - * satisfied, or an empty map otherwise
> + * \return A valid CameraConfiguration if the requested usages can be satisfied,
> + * or a invalid one otherwise
> */
> -std::map<Stream *, StreamConfiguration>
> +CameraConfiguration
> Camera::streamConfiguration(const std::vector<StreamUsage> &usages)
> {
> if (disconnected_ || !usages.size() || usages.size() > streams_.size())
> - return std::map<Stream *, StreamConfiguration>{};
> + return CameraConfiguration();
>
> return pipe_->streamConfiguration(this, usages);
> }
>
> /**
> * \brief Configure the camera's streams prior to capture
> - * \param[in] config A map of stream IDs and configurations to setup
> + * \param[in] config The camera configurations to setup
> *
> * Prior to starting capture, the camera must be configured to select a
> * group of streams to be involved in the capture and their configuration.
> @@ -561,7 +561,7 @@ Camera::streamConfiguration(const std::vector<StreamUsage> &usages)
> * \retval -EACCES The camera is not in a state where it can be configured
> * \retval -EINVAL The configuration is not valid
> */
> -int Camera::configureStreams(std::map<Stream *, StreamConfiguration> &config)
> +int Camera::configureStreams(const CameraConfiguration &config)
> {
> int ret;
>
> @@ -571,14 +571,14 @@ int Camera::configureStreams(std::map<Stream *, StreamConfiguration> &config)
> if (!stateBetween(CameraAcquired, CameraConfigured))
> return -EACCES;
>
> - if (!config.size()) {
> + if (!config.valid()) {
> LOG(Camera, Error)
> - << "Can't configure streams without a configuration";
> + << "Can't configure camera with invalid configuration";
> return -EINVAL;
> }
>
> - for (auto const &iter : config) {
> - if (streams_.find(iter.first) == streams_.end())
> + for (Stream *stream : config) {
> + if (streams_.find(stream) == streams_.end())
> return -EINVAL;
> }
>
> @@ -587,9 +587,8 @@ int Camera::configureStreams(std::map<Stream *, StreamConfiguration> &config)
> return ret;
>
> activeStreams_.clear();
> - for (auto const &iter : config) {
> - Stream *stream = iter.first;
> - const StreamConfiguration &cfg = iter.second;
> + for (Stream *stream : config) {
> + const StreamConfiguration &cfg = config[stream];
>
> stream->configuration_ = cfg;
> activeStreams_.insert(stream);
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index 8a1706fad5377bf4..b6cbd3bae51b08dc 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -18,6 +18,7 @@ namespace libcamera {
> class Buffer;
> class BufferPool;
> class Camera;
> +class CameraConfiguration;
> class CameraManager;
> class DeviceEnumerator;
> class MediaDevice;
> @@ -52,10 +53,9 @@ public:
>
> virtual bool match(DeviceEnumerator *enumerator) = 0;
>
> - virtual std::map<Stream *, StreamConfiguration>
> + virtual CameraConfiguration
> streamConfiguration(Camera *camera, const std::vector<StreamUsage> &usages) = 0;
> - virtual int configureStreams(Camera *camera,
> - std::map<Stream *, StreamConfiguration> &config) = 0;
> + virtual int configureStreams(Camera *camera, const CameraConfiguration &config) = 0;
>
> virtual int allocateBuffers(Camera *camera, Stream *stream) = 0;
> virtual int freeBuffers(Camera *camera, Stream *stream) = 0;
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 5d01504e24471bcf..ca09da753b908448 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -139,11 +139,11 @@ public:
> PipelineHandlerIPU3(CameraManager *manager);
> ~PipelineHandlerIPU3();
>
> - std::map<Stream *, StreamConfiguration>
> + CameraConfiguration
> streamConfiguration(Camera *camera,
> const std::vector<StreamUsage> &usages) override;
> int configureStreams(Camera *camera,
> - std::map<Stream *, StreamConfiguration> &config) override;
> + const CameraConfiguration &config) override;
>
> int allocateBuffers(Camera *camera, Stream *stream) override;
> int freeBuffers(Camera *camera, Stream *stream) override;
> @@ -204,11 +204,11 @@ PipelineHandlerIPU3::~PipelineHandlerIPU3()
> imguMediaDev_->release();
> }
>
> -std::map<Stream *, StreamConfiguration>
> +CameraConfiguration
> PipelineHandlerIPU3::streamConfiguration(Camera *camera,
> const std::vector<StreamUsage> &usages)
> {
> - std::map<Stream *, StreamConfiguration> configs;
> + CameraConfiguration configs;
> IPU3CameraData *data = cameraData(camera);
> StreamConfiguration *config = &configs[&data->stream_];
>
> @@ -234,7 +234,7 @@ PipelineHandlerIPU3::streamConfiguration(Camera *camera,
> }
>
> int PipelineHandlerIPU3::configureStreams(Camera *camera,
> - std::map<Stream *, StreamConfiguration> &config)
> + const CameraConfiguration &config)
> {
> IPU3CameraData *data = cameraData(camera);
> const StreamConfiguration &cfg = config[&data->stream_];
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index dfff7116bc2aa5d3..cd472cfadd86b7ba 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -26,11 +26,11 @@ public:
> PipelineHandlerUVC(CameraManager *manager);
> ~PipelineHandlerUVC();
>
> - std::map<Stream *, StreamConfiguration>
> + CameraConfiguration
> streamConfiguration(Camera *camera,
> const std::vector<StreamUsage> &usages) override;
> int configureStreams(Camera *camera,
> - std::map<Stream *, StreamConfiguration> &config) override;
> + const CameraConfiguration &config) override;
>
> int allocateBuffers(Camera *camera, Stream *stream) override;
> int freeBuffers(Camera *camera, Stream *stream) override;
> @@ -82,12 +82,12 @@ PipelineHandlerUVC::~PipelineHandlerUVC()
> media_->release();
> }
>
> -std::map<Stream *, StreamConfiguration>
> +CameraConfiguration
> PipelineHandlerUVC::streamConfiguration(Camera *camera,
> const std::vector<StreamUsage> &usages)
> {
> UVCCameraData *data = cameraData(camera);
> - std::map<Stream *, StreamConfiguration> configs;
> + CameraConfiguration configs;
> StreamConfiguration config{};
>
> config.width = 640;
> @@ -101,10 +101,10 @@ PipelineHandlerUVC::streamConfiguration(Camera *camera,
> }
>
> int PipelineHandlerUVC::configureStreams(Camera *camera,
> - std::map<Stream *, StreamConfiguration> &config)
> + const CameraConfiguration &config)
> {
> UVCCameraData *data = cameraData(camera);
> - StreamConfiguration *cfg = &config[&data->stream_];
> + const StreamConfiguration *cfg = &config[&data->stream_];
> int ret;
>
> LOG(UVC, Debug) << "Configure the camera for resolution "
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index 4da4ca018d4b942a..c8bbe2a19847ba1e 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -26,11 +26,11 @@ public:
> PipelineHandlerVimc(CameraManager *manager);
> ~PipelineHandlerVimc();
>
> - std::map<Stream *, StreamConfiguration>
> + CameraConfiguration
> streamConfiguration(Camera *camera,
> const std::vector<StreamUsage> &usages) override;
> int configureStreams(Camera *camera,
> - std::map<Stream *, StreamConfiguration> &config) override;
> + const CameraConfiguration &config) override;
>
> int allocateBuffers(Camera *camera, Stream *stream) override;
> int freeBuffers(Camera *camera, Stream *stream) override;
> @@ -82,12 +82,12 @@ PipelineHandlerVimc::~PipelineHandlerVimc()
> media_->release();
> }
>
> -std::map<Stream *, StreamConfiguration>
> +CameraConfiguration
> PipelineHandlerVimc::streamConfiguration(Camera *camera,
> const std::vector<StreamUsage> &usages)
> {
> VimcCameraData *data = cameraData(camera);
> - std::map<Stream *, StreamConfiguration> configs;
> + CameraConfiguration configs;
> StreamConfiguration config{};
>
> config.width = 640;
> @@ -101,10 +101,10 @@ PipelineHandlerVimc::streamConfiguration(Camera *camera,
> }
>
> int PipelineHandlerVimc::configureStreams(Camera *camera,
> - std::map<Stream *, StreamConfiguration> &config)
> + const CameraConfiguration &config)
> {
> VimcCameraData *data = cameraData(camera);
> - StreamConfiguration *cfg = &config[&data->stream_];
> + const StreamConfiguration *cfg = &config[&data->stream_];
> int ret;
>
> LOG(VIMC, Debug) << "Configure the camera for resolution "
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 0574f9904367605c..09d86b11e2a39d9b 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -163,15 +163,15 @@ PipelineHandler::~PipelineHandler()
> * The intended companion to this is \a configureStreams() which can be used to
> * change the group of streams parameters.
> *
> - * \return A map of successfully retrieved streams and configurations or an
> - * empty map on error.
> + * \return A valid CameraConfiguration if the requested usages can be satisfied,
> + * or a invalid one otherwise
s/invalid one/invalid configuration/
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> */
>
> /**
> * \fn PipelineHandler::configureStreams()
> * \brief Configure a group of streams for capture
> * \param[in] camera The camera to configure
> - * \param[in] config A map of stream configurations to apply
> + * \param[in] config The camera configurations to setup
> *
> * Configure the specified group of streams for \a camera according to the
> * configuration specified in \a configs. The intended caller of this interface
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index faa3bc5739dd8453..4bc0440370410525 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -98,7 +98,7 @@ int MainWindow::startCapture()
> int ret;
>
> config_ = camera_->streamConfiguration({ Stream::VideoRecording() });
> - Stream *stream = config_.begin()->first;
> + Stream *stream = config_.front();
> ret = camera_->configureStreams(config_);
> if (ret < 0) {
> std::cout << "Failed to configure camera" << std::endl;
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index 5e27a8fd6b4eb8f3..9b31da2bf4b75a03 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -46,7 +46,7 @@ private:
>
> std::shared_ptr<Camera> camera_;
> bool isCapturing_;
> - std::map<Stream *, StreamConfiguration> config_;
> + CameraConfiguration config_;
>
> ViewFinder *viewfinder_;
> };
> diff --git a/test/camera/camera_test.cpp b/test/camera/camera_test.cpp
> index 1609c4b02842186a..24ff5fe0c64d331e 100644
> --- a/test/camera/camera_test.cpp
> +++ b/test/camera/camera_test.cpp
> @@ -45,21 +45,3 @@ void CameraTest::cleanup()
>
> cm_->stop();
> };
> -
> -bool CameraTest::configurationValid(const std::map<Stream *, StreamConfiguration> &config) const
> -{
> - /* Test that the configuration is not empty. */
> - if (config.empty())
> - return false;
> -
> - /* Test that configuration is valid. */
> - for (auto const &it : config) {
> - const StreamConfiguration &conf = it.second;
> -
> - if (conf.width == 0 || conf.height == 0 ||
> - conf.pixelFormat == 0 || conf.bufferCount == 0)
> - return false;
> - }
> -
> - return true;
> -}
> diff --git a/test/camera/camera_test.h b/test/camera/camera_test.h
> index 5801fad3281e1653..ffc8a485bfaff836 100644
> --- a/test/camera/camera_test.h
> +++ b/test/camera/camera_test.h
> @@ -23,8 +23,6 @@ protected:
> int init();
> void cleanup();
>
> - bool configurationValid(const std::map<Stream *, StreamConfiguration> &config) const;
> -
> std::shared_ptr<Camera> camera_;
>
> private:
> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> index b8dbdb62f9a50a33..95b2202738d2b73b 100644
> --- a/test/camera/capture.cpp
> +++ b/test/camera/capture.cpp
> @@ -42,12 +42,12 @@ protected:
>
> int run()
> {
> - std::map<Stream *, StreamConfiguration> conf =
> + CameraConfiguration conf =
> camera_->streamConfiguration({ Stream::VideoRecording() });
> - Stream *stream = conf.begin()->first;
> - StreamConfiguration *sconf = &conf.begin()->second;
> + Stream *stream = conf.front();
> + StreamConfiguration *sconf = &conf[stream];
>
> - if (!configurationValid(conf)) {
> + if (!conf.valid()) {
> cout << "Failed to read default configuration" << endl;
> return TestFail;
> }
> diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp
> index 09861716973d752c..a0295a3ed82663d0 100644
> --- a/test/camera/configuration_default.cpp
> +++ b/test/camera/configuration_default.cpp
> @@ -18,17 +18,11 @@ class ConfigurationDefault : public CameraTest
> protected:
> int run()
> {
> - std::map<Stream *, StreamConfiguration> conf;
> + CameraConfiguration conf;
>
> /* Test asking for configuration for a video stream. */
> conf = camera_->streamConfiguration({ Stream::VideoRecording() });
> - if (conf.empty()) {
> - cout << "Failed to retrieve configuration for video streams"
> - << endl;
> - return TestFail;
> - }
> -
> - if (!configurationValid(conf)) {
> + if (!conf.valid()) {
> cout << "Default configuration invalid" << endl;
> return TestFail;
> }
> @@ -38,7 +32,7 @@ protected:
> * stream usages returns an empty list of configurations.
> */
> conf = camera_->streamConfiguration({});
> - if (!conf.empty()) {
> + if (conf.valid()) {
> 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 1bc01e66625eedf0..48d1b2ee98d76f5c 100644
> --- a/test/camera/configuration_set.cpp
> +++ b/test/camera/configuration_set.cpp
> @@ -18,11 +18,11 @@ class ConfigurationSet : public CameraTest
> protected:
> int run()
> {
> - std::map<Stream *, StreamConfiguration> conf =
> + CameraConfiguration conf =
> camera_->streamConfiguration({ Stream::VideoRecording() });
> - StreamConfiguration *sconf = &conf.begin()->second;
> + StreamConfiguration *sconf = &conf[conf.front()];
>
> - if (!configurationValid(conf)) {
> + if (!conf.valid()) {
> cout << "Failed to read default configuration" << endl;
> return TestFail;
> }
> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
> index ab3c6fb5bea38c36..8ae93bee7ca178d4 100644
> --- a/test/camera/statemachine.cpp
> +++ b/test/camera/statemachine.cpp
> @@ -265,7 +265,7 @@ protected:
> return TestPass;
> }
>
> - std::map<Stream *, StreamConfiguration> defconf_;
> + CameraConfiguration defconf_;
> };
>
> } /* namespace */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list