[libcamera-devel] [PATCH v2 4/6] libcamera: camera: Return a pointer from generateConfiguration()
Niklas Söderlund
niklas.soderlund at ragnatech.se
Sun May 19 22:44:38 CEST 2019
Hi Laurent,
Thanks for your work.
On 2019-05-19 18:00:45 +0300, Laurent Pinchart wrote:
> To prepare for specialising the CameraConfiguration class in pipeline
> handlers, return a pointer to a camera configuration instead of a
> reference from Camera::generateConfiguration(). The camera configuration
> always needs to be allocated from the pipeline handler, and its
> ownership is passed to the application.
>
> For symmetry, change Camera::configure() to take a CameraConfiguration
> pointer instead of a reference. This aligns with our coding practice of
> passing parameters that are modified by the callee by pointer.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> include/libcamera/camera.h | 4 +--
> src/cam/main.cpp | 40 ++++++++++-----------
> src/libcamera/camera.cpp | 31 +++++++++--------
> src/libcamera/include/pipeline_handler.h | 6 ++--
> src/libcamera/pipeline/ipu3/ipu3.cpp | 31 +++++++++--------
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 25 ++++++++------
> src/libcamera/pipeline/uvcvideo.cpp | 30 ++++++++--------
> src/libcamera/pipeline/vimc.cpp | 30 ++++++++--------
> src/libcamera/pipeline_handler.cpp | 3 +-
> src/qcam/main_window.cpp | 5 ++-
> src/qcam/main_window.h | 2 +-
> test/camera/capture.cpp | 38 +++++++++++++++-----
> test/camera/configuration_default.cpp | 14 +++++---
> test/camera/configuration_set.cpp | 44 +++++++++++++++++-------
> test/camera/statemachine.cpp | 22 ++++++++++--
> 15 files changed, 202 insertions(+), 123 deletions(-)
>
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 284e5276a055..144133c5de9f 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -77,8 +77,8 @@ public:
> int release();
>
> const std::set<Stream *> &streams() const;
> - CameraConfiguration generateConfiguration(const StreamRoles &roles);
> - int configure(CameraConfiguration &config);
> + CameraConfiguration *generateConfiguration(const StreamRoles &roles);
> + int configure(CameraConfiguration *config);
>
> int allocateBuffers();
> int freeBuffers();
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index cd165bea34cd..7550ae4f3428 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -85,15 +85,13 @@ static int parseOptions(int argc, char *argv[])
> return 0;
> }
>
> -static int prepareCameraConfig(CameraConfiguration *config)
> +static CameraConfiguration *prepareCameraConfig()
> {
> StreamRoles roles;
>
> /* If no configuration is provided assume a single video stream. */
> - if (!options.isSet(OptStream)) {
> - *config = camera->generateConfiguration({ StreamRole::VideoRecording });
> - return 0;
> - }
> + if (!options.isSet(OptStream))
> + return camera->generateConfiguration({ StreamRole::VideoRecording });
>
> const std::vector<OptionValue> &streamOptions =
> options[OptStream].toArray();
> @@ -113,23 +111,23 @@ static int prepareCameraConfig(CameraConfiguration *config)
> } else {
> std::cerr << "Unknown stream role "
> << conf["role"].toString() << std::endl;
> - return -EINVAL;
> + return nullptr;
> }
> }
>
> - *config = camera->generateConfiguration(roles);
> -
> - if (!config->isValid()) {
> + CameraConfiguration *config = camera->generateConfiguration(roles);
> + if (!config || !config->isValid()) {
> std::cerr << "Failed to get default stream configuration"
> << std::endl;
> - return -EINVAL;
> + delete config;
> + return nullptr;
> }
>
> /* Apply configuration explicitly requested. */
> unsigned int i = 0;
> for (auto const &value : streamOptions) {
> KeyValueParser::Options conf = value.toKeyValues();
> - StreamConfiguration &cfg = (*config)[i++];
> + StreamConfiguration &cfg = config->at(i++);
>
> if (conf.isSet("width"))
> cfg.size.width = conf["width"];
> @@ -142,7 +140,7 @@ static int prepareCameraConfig(CameraConfiguration *config)
> cfg.pixelFormat = conf["pixelformat"];
> }
>
> - return 0;
> + return config;
> }
>
> static void requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers)
> @@ -191,25 +189,25 @@ static void requestComplete(Request *request, const std::map<Stream *, Buffer *>
>
> static int capture()
> {
> - CameraConfiguration config;
> int ret;
>
> - ret = prepareCameraConfig(&config);
> - if (ret) {
> + CameraConfiguration *config = prepareCameraConfig();
> + if (!config) {
> std::cout << "Failed to prepare camera configuration" << std::endl;
> - return ret;
> + return -EINVAL;
> }
>
> ret = camera->configure(config);
> if (ret < 0) {
> std::cout << "Failed to configure camera" << std::endl;
> + delete config;
> return ret;
> }
>
> streamInfo.clear();
>
> - for (unsigned int index = 0; index < config.size(); ++index) {
> - StreamConfiguration &cfg = config[index];
> + for (unsigned int index = 0; index < config->size(); ++index) {
> + StreamConfiguration &cfg = config->at(index);
> streamInfo[cfg.stream()] = "stream" + std::to_string(index);
> }
>
> @@ -217,6 +215,7 @@ static int capture()
> if (ret) {
> std::cerr << "Failed to allocate buffers"
> << std::endl;
> + delete config;
> return ret;
> }
>
> @@ -224,7 +223,7 @@ static int capture()
>
> /* Identify the stream with the least number of buffers. */
> unsigned int nbuffers = UINT_MAX;
> - for (StreamConfiguration &cfg : config) {
> + for (StreamConfiguration &cfg : *config) {
> Stream *stream = cfg.stream();
> nbuffers = std::min(nbuffers, stream->bufferPool().count());
> }
> @@ -244,7 +243,7 @@ static int capture()
> }
>
> std::map<Stream *, Buffer *> map;
> - for (StreamConfiguration &cfg : config) {
> + for (StreamConfiguration &cfg : *config) {
> Stream *stream = cfg.stream();
> map[stream] = &stream->bufferPool().buffers()[i];
> }
> @@ -280,6 +279,7 @@ static int capture()
> std::cout << "Failed to stop capture" << std::endl;
> out:
> camera->freeBuffers();
> + delete config;
>
> return ret;
> }
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 5848330f639a..0e80691ed862 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -542,21 +542,24 @@ const std::set<Stream *> &Camera::streams() const
> * specifies a list of stream roles and the camera returns a configuration
> * containing suitable streams and their suggested default configurations.
> *
> - * \return A valid CameraConfiguration if the requested roles can be satisfied,
> - * or a invalid one otherwise
> + * \return A CameraConfiguration if the requested roles can be satisfied, or a
> + * null pointer otherwise. The ownership of the returned configuration is
> + * passed to the caller.
> */
> -CameraConfiguration
> -Camera::generateConfiguration(const StreamRoles &roles)
> +CameraConfiguration *Camera::generateConfiguration(const StreamRoles &roles)
> {
> - if (disconnected_ || !roles.size() || roles.size() > streams_.size())
> - return CameraConfiguration();
> + if (disconnected_ || roles.size() > streams_.size())
> + return nullptr;
>
> - CameraConfiguration config = pipe_->generateConfiguration(this, roles);
> + CameraConfiguration *config = pipe_->generateConfiguration(this, roles);
>
> std::ostringstream msg("streams configuration:", std::ios_base::ate);
>
> - for (unsigned int index = 0; index < config.size(); ++index)
> - msg << " (" << index << ") " << config[index].toString();
> + if (config->empty())
> + msg << " empty";
> +
> + for (unsigned int index = 0; index < config->size(); ++index)
> + msg << " (" << index << ") " << config->at(index).toString();
>
> LOG(Camera, Debug) << msg.str();
>
> @@ -590,7 +593,7 @@ Camera::generateConfiguration(const StreamRoles &roles)
> * \retval -EACCES The camera is not in a state where it can be configured
> * \retval -EINVAL The configuration is not valid
> */
> -int Camera::configure(CameraConfiguration &config)
> +int Camera::configure(CameraConfiguration *config)
> {
> int ret;
>
> @@ -600,7 +603,7 @@ int Camera::configure(CameraConfiguration &config)
> if (!stateBetween(CameraAcquired, CameraConfigured))
> return -EACCES;
>
> - if (!config.isValid()) {
> + if (!config->isValid()) {
> LOG(Camera, Error)
> << "Can't configure camera with invalid configuration";
> return -EINVAL;
> @@ -608,8 +611,8 @@ int Camera::configure(CameraConfiguration &config)
>
> std::ostringstream msg("configuring streams:", std::ios_base::ate);
>
> - for (unsigned int index = 0; index < config.size(); ++index) {
> - StreamConfiguration &cfg = config[index];
> + for (unsigned int index = 0; index < config->size(); ++index) {
> + StreamConfiguration &cfg = config->at(index);
> cfg.setStream(nullptr);
> msg << " (" << index << ") " << cfg.toString();
> }
> @@ -621,7 +624,7 @@ int Camera::configure(CameraConfiguration &config)
> return ret;
>
> activeStreams_.clear();
> - for (const StreamConfiguration &cfg : config) {
> + for (const StreamConfiguration &cfg : *config) {
> Stream *stream = cfg.stream();
> if (!stream)
> LOG(Camera, Fatal)
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index a025905ab68f..7da6df1ab2a0 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -60,9 +60,9 @@ public:
> bool lock();
> void unlock();
>
> - virtual CameraConfiguration
> - generateConfiguration(Camera *camera, const StreamRoles &roles) = 0;
> - virtual int configure(Camera *camera, CameraConfiguration &config) = 0;
> + virtual CameraConfiguration *generateConfiguration(Camera *camera,
> + const StreamRoles &roles) = 0;
> + virtual int configure(Camera *camera, CameraConfiguration *config) = 0;
>
> virtual int allocateBuffers(Camera *camera,
> const std::set<Stream *> &streams) = 0;
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index ed0ef69de1d1..3acf82ff4665 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -150,9 +150,9 @@ class PipelineHandlerIPU3 : public PipelineHandler
> public:
> PipelineHandlerIPU3(CameraManager *manager);
>
> - CameraConfiguration
> - generateConfiguration(Camera *camera, const StreamRoles &roles) override;
> - int configure(Camera *camera, CameraConfiguration &config) override;
> + CameraConfiguration *generateConfiguration(Camera *camera,
> + const StreamRoles &roles) override;
> + int configure(Camera *camera, CameraConfiguration *config) override;
>
> int allocateBuffers(Camera *camera,
> const std::set<Stream *> &streams) override;
> @@ -207,12 +207,11 @@ PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager)
> {
> }
>
> -CameraConfiguration
> -PipelineHandlerIPU3::generateConfiguration(Camera *camera,
> - const StreamRoles &roles)
> +CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
> + const StreamRoles &roles)
> {
> IPU3CameraData *data = cameraData(camera);
> - CameraConfiguration config = {};
> + CameraConfiguration *config = new CameraConfiguration();
> std::set<IPU3Stream *> streams = {
> &data->outStream_,
> &data->vfStream_,
> @@ -290,21 +289,23 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera,
> break;
> }
>
> - if (!stream)
> - return CameraConfiguration{};
> + if (!stream) {
> + delete config;
> + return nullptr;
> + }
>
> streams.erase(stream);
>
> cfg.pixelFormat = V4L2_PIX_FMT_NV12;
> cfg.bufferCount = IPU3_BUFFER_COUNT;
>
> - config.addConfiguration(cfg);
> + config->addConfiguration(cfg);
> }
>
> return config;
> }
>
> -int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config)
> +int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *config)
> {
> IPU3CameraData *data = cameraData(camera);
> IPU3Stream *outStream = &data->outStream_;
> @@ -316,7 +317,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config)
>
> outStream->active_ = false;
> vfStream->active_ = false;
> - for (StreamConfiguration &cfg : config) {
> + for (StreamConfiguration &cfg : *config) {
> /*
> * Pick a stream for the configuration entry.
> * \todo: This is a naive temporary implementation that will be
> @@ -382,7 +383,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config)
> return ret;
>
> /* Apply the format to the configured streams output devices. */
> - for (StreamConfiguration &cfg : config) {
> + for (StreamConfiguration &cfg : *config) {
> IPU3Stream *stream = static_cast<IPU3Stream *>(cfg.stream());
> ret = imgu->configureOutput(stream->device_, cfg);
> if (ret)
> @@ -395,13 +396,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config)
> * be at least one active stream in the configuration request).
> */
> if (!outStream->active_) {
> - ret = imgu->configureOutput(outStream->device_, config[0]);
> + ret = imgu->configureOutput(outStream->device_, config->at(0));
> if (ret)
> return ret;
> }
>
> if (!vfStream->active_) {
> - ret = imgu->configureOutput(vfStream->device_, config[0]);
> + ret = imgu->configureOutput(vfStream->device_, config->at(0));
> if (ret)
> return ret;
> }
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index ec6980b0943a..01ba50f09db1 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -34,9 +34,9 @@ public:
> PipelineHandlerRkISP1(CameraManager *manager);
> ~PipelineHandlerRkISP1();
>
> - CameraConfiguration generateConfiguration(Camera *camera,
> + CameraConfiguration *generateConfiguration(Camera *camera,
> const StreamRoles &roles) override;
> - int configure(Camera *camera, CameraConfiguration &config) override;
> + int configure(Camera *camera, CameraConfiguration *config) override;
>
> int allocateBuffers(Camera *camera,
> const std::set<Stream *> &streams) override;
> @@ -105,26 +105,29 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1()
> * Pipeline Operations
> */
>
> -CameraConfiguration PipelineHandlerRkISP1::generateConfiguration(Camera *camera,
> +CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera,
> const StreamRoles &roles)
> {
> RkISP1CameraData *data = cameraData(camera);
> - CameraConfiguration config;
> - StreamConfiguration cfg{};
> + CameraConfiguration *config = new CameraConfiguration();
>
> - cfg.pixelFormat = V4L2_PIX_FMT_NV12;
> - cfg.size = data->sensor_->resolution();
> - cfg.bufferCount = RKISP1_BUFFER_COUNT;
> + if (!roles.empty()) {
I would invert this check and reduce the indentation level,
if (roles.empty())
return config;
Same for other generateConfiguration() bellow.
With this fixed,
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> + StreamConfiguration cfg{};
>
> - config.addConfiguration(cfg);
> + cfg.pixelFormat = V4L2_PIX_FMT_NV12;
> + cfg.size = data->sensor_->resolution();
> + cfg.bufferCount = RKISP1_BUFFER_COUNT;
> +
> + config->addConfiguration(cfg);
> + }
>
> return config;
> }
>
> -int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration &config)
> +int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *config)
> {
> RkISP1CameraData *data = cameraData(camera);
> - StreamConfiguration &cfg = config[0];
> + StreamConfiguration &cfg = config->at(0);
> CameraSensor *sensor = data->sensor_;
> int ret;
>
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 5dcc868b2fc9..91b4065c250b 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -25,9 +25,9 @@ class PipelineHandlerUVC : public PipelineHandler
> public:
> PipelineHandlerUVC(CameraManager *manager);
>
> - CameraConfiguration
> - generateConfiguration(Camera *camera, const StreamRoles &roles) override;
> - int configure(Camera *camera, CameraConfiguration &config) override;
> + CameraConfiguration *generateConfiguration(Camera *camera,
> + const StreamRoles &roles) override;
> + int configure(Camera *camera, CameraConfiguration *config) override;
>
> int allocateBuffers(Camera *camera,
> const std::set<Stream *> &streams) override;
> @@ -73,26 +73,28 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)
> {
> }
>
> -CameraConfiguration
> -PipelineHandlerUVC::generateConfiguration(Camera *camera,
> - const StreamRoles &roles)
> +CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera,
> + const StreamRoles &roles)
> {
> - CameraConfiguration config;
> - StreamConfiguration cfg;
> + CameraConfiguration *config = new CameraConfiguration();
>
> - cfg.pixelFormat = V4L2_PIX_FMT_YUYV;
> - cfg.size = { 640, 480 };
> - cfg.bufferCount = 4;
> + if (!roles.empty()) {
> + StreamConfiguration cfg;
>
> - config.addConfiguration(cfg);
> + cfg.pixelFormat = V4L2_PIX_FMT_YUYV;
> + cfg.size = { 640, 480 };
> + cfg.bufferCount = 4;
> +
> + config->addConfiguration(cfg);
> + }
>
> return config;
> }
>
> -int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration &config)
> +int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)
> {
> UVCCameraData *data = cameraData(camera);
> - StreamConfiguration &cfg = config[0];
> + StreamConfiguration &cfg = config->at(0);
> int ret;
>
> V4L2DeviceFormat format = {};
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index af6b6f21e3c5..510e6c082f13 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -25,9 +25,9 @@ class PipelineHandlerVimc : public PipelineHandler
> public:
> PipelineHandlerVimc(CameraManager *manager);
>
> - CameraConfiguration
> - generateConfiguration(Camera *camera, const StreamRoles &roles) override;
> - int configure(Camera *camera, CameraConfiguration &config) override;
> + CameraConfiguration *generateConfiguration(Camera *camera,
> + const StreamRoles &roles) override;
> + int configure(Camera *camera, CameraConfiguration *config) override;
>
> int allocateBuffers(Camera *camera,
> const std::set<Stream *> &streams) override;
> @@ -73,26 +73,28 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager)
> {
> }
>
> -CameraConfiguration
> -PipelineHandlerVimc::generateConfiguration(Camera *camera,
> - const StreamRoles &roles)
> +CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera,
> + const StreamRoles &roles)
> {
> - CameraConfiguration config;
> - StreamConfiguration cfg;
> + CameraConfiguration *config = new CameraConfiguration();
>
> - cfg.pixelFormat = V4L2_PIX_FMT_RGB24;
> - cfg.size = { 640, 480 };
> - cfg.bufferCount = 4;
> + if (!roles.empty()) {
> + StreamConfiguration cfg;
>
> - config.addConfiguration(cfg);
> + cfg.pixelFormat = V4L2_PIX_FMT_RGB24;
> + cfg.size = { 640, 480 };
> + cfg.bufferCount = 4;
> +
> + config->addConfiguration(cfg);
> + }
>
> return config;
> }
>
> -int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration &config)
> +int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)
> {
> VimcCameraData *data = cameraData(camera);
> - StreamConfiguration &cfg = config[0];
> + StreamConfiguration &cfg = config->at(0);
> int ret;
>
> V4L2DeviceFormat format = {};
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 4185e3557dcb..de46e98880a2 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -233,7 +233,8 @@ void PipelineHandler::unlock()
> * the group of streams parameters.
> *
> * \return A valid CameraConfiguration if the requested roles can be satisfied,
> - * or a invalid configuration otherwise
> + * or a null pointer otherwise. The ownership of the returned configuration is
> + * passed to the caller.
> */
>
> /**
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 06ae2985f80d..3ae5aae9cc76 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -104,7 +104,7 @@ int MainWindow::startCapture()
> return ret;
> }
>
> - const StreamConfiguration &cfg = config_[0];
> + const StreamConfiguration &cfg = config_->at(0);
> Stream *stream = cfg.stream();
> ret = viewfinder_->setFormat(cfg.pixelFormat, cfg.size.width,
> cfg.size.height);
> @@ -180,6 +180,9 @@ void MainWindow::stopCapture()
>
> camera_->freeBuffers();
> isCapturing_ = false;
> +
> + delete config_;
> + config_ = nullptr;
> }
>
> void MainWindow::requestComplete(Request *request,
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index 143b5b08a5a0..866866e93d23 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -45,7 +45,7 @@ private:
>
> std::shared_ptr<Camera> camera_;
> bool isCapturing_;
> - CameraConfiguration config_;
> + CameraConfiguration *config_;
>
> ViewFinder *viewfinder_;
> };
> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> index bfd11eefedcf..137aa649a505 100644
> --- a/test/camera/capture.cpp
> +++ b/test/camera/capture.cpp
> @@ -40,13 +40,25 @@ protected:
> camera_->queueRequest(request);
> }
>
> - int run()
> + int init() override
> {
> - CameraConfiguration config =
> - camera_->generateConfiguration({ StreamRole::VideoRecording });
> - StreamConfiguration *cfg = &config[0];
> + CameraTest::init();
>
> - if (!config.isValid()) {
> + config_ = camera_->generateConfiguration({ StreamRole::VideoRecording });
> + if (!config_) {
> + cout << "Failed to generate default configuration" << endl;
> + CameraTest::cleanup();
> + return TestFail;
> + }
> +
> + return TestPass;
> + }
> +
> + int run() override
> + {
> + StreamConfiguration &cfg = config_->at(0);
> +
> + if (!config_->isValid()) {
> cout << "Failed to read default configuration" << endl;
> return TestFail;
> }
> @@ -56,7 +68,7 @@ protected:
> return TestFail;
> }
>
> - if (camera_->configure(config)) {
> + if (camera_->configure(config_)) {
> cout << "Failed to set default configuration" << endl;
> return TestFail;
> }
> @@ -66,7 +78,7 @@ protected:
> return TestFail;
> }
>
> - Stream *stream = cfg->stream();
> + Stream *stream = cfg.stream();
> BufferPool &pool = stream->bufferPool();
> std::vector<Request *> requests;
> for (Buffer &buffer : pool.buffers()) {
> @@ -110,10 +122,10 @@ protected:
> while (timer.isRunning())
> dispatcher->processEvents();
>
> - if (completeRequestsCount_ <= cfg->bufferCount * 2) {
> + if (completeRequestsCount_ <= cfg.bufferCount * 2) {
> cout << "Failed to capture enough frames (got "
> << completeRequestsCount_ << " expected at least "
> - << cfg->bufferCount * 2 << ")" << endl;
> + << cfg.bufferCount * 2 << ")" << endl;
> return TestFail;
> }
>
> @@ -134,6 +146,14 @@ protected:
>
> return TestPass;
> }
> +
> + void cleanup() override
> + {
> + delete config_;
> + CameraTest::cleanup();
> + }
> +
> + CameraConfiguration *config_;
> };
>
> } /* namespace */
> diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp
> index 8c4a03db498a..d5cefc1127c9 100644
> --- a/test/camera/configuration_default.cpp
> +++ b/test/camera/configuration_default.cpp
> @@ -18,26 +18,32 @@ class ConfigurationDefault : public CameraTest
> protected:
> int run()
> {
> - CameraConfiguration config;
> + CameraConfiguration *config;
>
> /* Test asking for configuration for a video stream. */
> config = camera_->generateConfiguration({ StreamRole::VideoRecording });
> - if (!config.isValid()) {
> + if (!config || !config->isValid()) {
> cout << "Default configuration invalid" << endl;
> + delete config;
> return TestFail;
> }
>
> + delete config;
> +
> /*
> * Test that asking for configuration for an empty array of
> - * stream roles returns an empty list of configurations.
> + * stream roles returns an empty camera configuration.
> */
> config = camera_->generateConfiguration({});
> - if (config.isValid()) {
> + if (!config || config->isValid()) {
> cout << "Failed to retrieve configuration for empty roles list"
> << endl;
> + delete config;
> return TestFail;
> }
>
> + delete config;
> +
> return TestPass;
> }
> };
> diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp
> index 25b5db67103a..23c611a93355 100644
> --- a/test/camera/configuration_set.cpp
> +++ b/test/camera/configuration_set.cpp
> @@ -16,13 +16,25 @@ namespace {
> class ConfigurationSet : public CameraTest
> {
> protected:
> - int run()
> + int init() override
> {
> - CameraConfiguration config =
> - camera_->generateConfiguration({ StreamRole::VideoRecording });
> - StreamConfiguration *cfg = &config[0];
> + CameraTest::init();
>
> - if (!config.isValid()) {
> + config_ = camera_->generateConfiguration({ StreamRole::VideoRecording });
> + if (!config_) {
> + cout << "Failed to generate default configuration" << endl;
> + CameraTest::cleanup();
> + return TestFail;
> + }
> +
> + return TestPass;
> + }
> +
> + int run() override
> + {
> + StreamConfiguration &cfg = config_->at(0);
> +
> + if (!config_->isValid()) {
> cout << "Failed to read default configuration" << endl;
> return TestFail;
> }
> @@ -33,7 +45,7 @@ protected:
> }
>
> /* Test that setting the default configuration works. */
> - if (camera_->configure(config)) {
> + if (camera_->configure(config_)) {
> cout << "Failed to set default configuration" << endl;
> return TestFail;
> }
> @@ -48,7 +60,7 @@ protected:
> return TestFail;
> }
>
> - if (!camera_->configure(config)) {
> + if (!camera_->configure(config_)) {
> cout << "Setting configuration on a camera not acquired succeeded when it should have failed"
> << endl;
> return TestFail;
> @@ -64,9 +76,9 @@ protected:
> * the default configuration of the VIMC camera is known to
> * work.
> */
> - cfg->size.width *= 2;
> - cfg->size.height *= 2;
> - if (camera_->configure(config)) {
> + cfg.size.width *= 2;
> + cfg.size.height *= 2;
> + if (camera_->configure(config_)) {
> cout << "Failed to set modified configuration" << endl;
> return TestFail;
> }
> @@ -74,14 +86,22 @@ protected:
> /*
> * Test that setting an invalid configuration fails.
> */
> - cfg->size = { 0, 0 };
> - if (!camera_->configure(config)) {
> + cfg.size = { 0, 0 };
> + if (!camera_->configure(config_)) {
> cout << "Invalid configuration incorrectly accepted" << endl;
> return TestFail;
> }
>
> return TestPass;
> }
> +
> + void cleanup() override
> + {
> + delete config_;
> + CameraTest::cleanup();
> + }
> +
> + CameraConfiguration *config_;
> };
>
> } /* namespace */
> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
> index 7a74cd85a37a..a662e869fadf 100644
> --- a/test/camera/statemachine.cpp
> +++ b/test/camera/statemachine.cpp
> @@ -233,10 +233,22 @@ protected:
> return TestPass;
> }
>
> - int run()
> + int init() override
> {
> + CameraTest::init();
> +
> defconf_ = camera_->generateConfiguration({ StreamRole::VideoRecording });
> + if (!defconf_) {
> + cout << "Failed to generate default configuration" << endl;
> + CameraTest::cleanup();
> + return TestFail;
> + }
>
> + return TestPass;
> + }
> +
> + int run() override
> + {
> if (testAvailable() != TestPass) {
> cout << "State machine in Available state failed" << endl;
> return TestFail;
> @@ -265,7 +277,13 @@ protected:
> return TestPass;
> }
>
> - CameraConfiguration defconf_;
> + void cleanup() override
> + {
> + delete defconf_;
> + CameraTest::cleanup();
> + }
> +
> + CameraConfiguration *defconf_;
> };
>
> } /* namespace */
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list