From 75de3bff9e165e512be5f53e19f31c7388dc08b1 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Mon, 29 Apr 2019 20:58:34 +0300 Subject: libcamera: camera: Return a pointer from generateConfiguration() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- src/libcamera/camera.cpp | 42 ++++++++++++++++++++------------ src/libcamera/include/pipeline_handler.h | 6 ++--- src/libcamera/pipeline/ipu3/ipu3.cpp | 31 +++++++++++------------ src/libcamera/pipeline/rkisp1/rkisp1.cpp | 19 +++++++++------ src/libcamera/pipeline/uvcvideo.cpp | 24 +++++++++--------- src/libcamera/pipeline/vimc.cpp | 24 +++++++++--------- src/libcamera/pipeline_handler.cpp | 3 ++- 7 files changed, 84 insertions(+), 65 deletions(-) (limited to 'src/libcamera') diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 5848330f..e7931ce0 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -540,27 +540,37 @@ const std::set &Camera::streams() const * * Generate a camera configuration for a set of desired stream roles. The caller * specifies a list of stream roles and the camera returns a configuration - * containing suitable streams and their suggested default configurations. + * containing suitable streams and their suggested default configurations. An + * empty list of roles is valid, and will generate an empty configuration that + * can be filled by the caller. * - * \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) +std::unique_ptr 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); + if (!config) { + LOG(Camera, Debug) + << "Pipeline handler failed to generate configuration"; + return nullptr; + } 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(); - return config; + return std::unique_ptr(config); } /** @@ -590,7 +600,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 +610,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 +618,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 +631,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 a025905a..7da6df1a 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 &streams) = 0; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index ed0ef69d..3acf82ff 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 &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 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(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 ec6980b0..c8d217ab 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 &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(); + + if (roles.empty()) + return config; + StreamConfiguration cfg{}; cfg.pixelFormat = V4L2_PIX_FMT_NV12; cfg.size = data->sensor_->resolution(); cfg.bufferCount = RKISP1_BUFFER_COUNT; - config.addConfiguration(cfg); + 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 5dcc868b..120d8d3a 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 &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(); + if (roles.empty()) + return config; + + StreamConfiguration cfg{}; cfg.pixelFormat = V4L2_PIX_FMT_YUYV; cfg.size = { 640, 480 }; cfg.bufferCount = 4; - config.addConfiguration(cfg); + 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 af6b6f21..f6aa3268 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 &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(); + if (roles.empty()) + return config; + + StreamConfiguration cfg{}; cfg.pixelFormat = V4L2_PIX_FMT_RGB24; cfg.size = { 640, 480 }; cfg.bufferCount = 4; - config.addConfiguration(cfg); + 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 4185e355..de46e988 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. */ /** -- cgit v1.2.1