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/cam/main.cpp | 38 +++++++++++++---------------- 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 ++- src/qcam/main_window.cpp | 6 +++-- src/qcam/main_window.h | 3 ++- 10 files changed, 107 insertions(+), 89 deletions(-) (limited to 'src') diff --git a/src/cam/main.cpp b/src/cam/main.cpp index cd165bea..535c2420 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 std::unique_ptr 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 &streamOptions = options[OptStream].toArray(); @@ -113,23 +111,22 @@ 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()) { + std::unique_ptr config = camera->generateConfiguration(roles); + if (!config || !config->isValid()) { std::cerr << "Failed to get default stream configuration" << std::endl; - return -EINVAL; + 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 +139,7 @@ static int prepareCameraConfig(CameraConfiguration *config) cfg.pixelFormat = conf["pixelformat"]; } - return 0; + return config; } static void requestComplete(Request *request, const std::map &buffers) @@ -191,16 +188,15 @@ static void requestComplete(Request *request, const std::map static int capture() { - CameraConfiguration config; int ret; - ret = prepareCameraConfig(&config); - if (ret) { + std::unique_ptr config = prepareCameraConfig(); + if (!config) { std::cout << "Failed to prepare camera configuration" << std::endl; - return ret; + return -EINVAL; } - ret = camera->configure(config); + ret = camera->configure(config.get()); if (ret < 0) { std::cout << "Failed to configure camera" << std::endl; return ret; @@ -208,8 +204,8 @@ static int capture() 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); } @@ -224,7 +220,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 +240,7 @@ static int capture() } std::map map; - for (StreamConfiguration &cfg : config) { + for (StreamConfiguration &cfg : *config) { Stream *stream = cfg.stream(); map[stream] = &stream->bufferPool().buffers()[i]; } 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. */ /** diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 06ae2985..16b12313 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -98,13 +98,13 @@ int MainWindow::startCapture() int ret; config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); - ret = camera_->configure(config_); + ret = camera_->configure(config_.get()); if (ret < 0) { std::cout << "Failed to configure camera" << std::endl; 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,8 @@ void MainWindow::stopCapture() camera_->freeBuffers(); isCapturing_ = false; + + config_.reset(); } void MainWindow::requestComplete(Request *request, diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h index 143b5b08..fe565cbc 100644 --- a/src/qcam/main_window.h +++ b/src/qcam/main_window.h @@ -8,6 +8,7 @@ #define __QCAM_MAIN_WINDOW_H__ #include +#include #include @@ -45,7 +46,7 @@ private: std::shared_ptr camera_; bool isCapturing_; - CameraConfiguration config_; + std::unique_ptr config_; ViewFinder *viewfinder_; }; -- cgit v1.2.1