| Message ID | 20190521192740.28112-5-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On Tue, May 21, 2019 at 10:27:38PM +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@ideasonboard.com> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > --- > Changes since v2: > > - Return an std::unique_ptr<> from Camera::generateConfiguration() > - Document that Camera::generateConfiguration() can take an empty list > of roles > --- > include/libcamera/camera.h | 4 +-- > src/cam/main.cpp | 38 +++++++++++------------- > src/libcamera/camera.cpp | 37 +++++++++++++---------- > 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 +- > test/camera/capture.cpp | 32 ++++++++++++++------ > test/camera/configuration_default.cpp | 8 ++--- > test/camera/configuration_set.cpp | 38 ++++++++++++++++-------- > test/camera/statemachine.cpp | 30 +++++++++++++------ > 15 files changed, 178 insertions(+), 125 deletions(-) > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > index 284e5276a055..a3a7289a7aa7 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); > + std::unique_ptr<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..535c2420893e 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<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,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<CameraConfiguration> 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<Stream *, Buffer *> &buffers) > @@ -191,16 +188,15 @@ static void requestComplete(Request *request, const std::map<Stream *, Buffer *> > > static int capture() > { > - CameraConfiguration config; > int ret; > > - ret = prepareCameraConfig(&config); > - if (ret) { > + std::unique_ptr<CameraConfiguration> 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<Stream *, Buffer *> 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 5848330f639a..0e5fd7f57271 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -540,27 +540,32 @@ const std::set<Stream *> &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<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(); > > - return config; > + return std::unique_ptr<CameraConfiguration>(config); > } > > /** > @@ -590,7 +595,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 +605,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 +613,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 +626,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..c8d217abdca8 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; > + 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 5dcc868b2fc9..120d8d3a1522 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(); > > + 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 af6b6f21e3c5..f6aa32683e5e 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(); > > + 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 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..16b123132dd9 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 143b5b08a5a0..fe565cbcb460 100644 > --- a/src/qcam/main_window.h > +++ b/src/qcam/main_window.h > @@ -8,6 +8,7 @@ > #define __QCAM_MAIN_WINDOW_H__ > > #include <map> > +#include <memory> > > #include <QMainWindow> > > @@ -45,7 +46,7 @@ private: > > std::shared_ptr<Camera> camera_; > bool isCapturing_; > - CameraConfiguration config_; > + std::unique_ptr<CameraConfiguration> config_; > > ViewFinder *viewfinder_; > }; > diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp > index bfd11eefedcf..bb7d380cdc1a 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_.get())) { > 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,8 @@ protected: > > return TestPass; > } > + > + std::unique_ptr<CameraConfiguration> config_; > }; > > } /* namespace */ > diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp > index 8c4a03db498a..8a767d2321e0 100644 > --- a/test/camera/configuration_default.cpp > +++ b/test/camera/configuration_default.cpp > @@ -18,21 +18,21 @@ class ConfigurationDefault : public CameraTest > protected: > int run() > { > - CameraConfiguration config; > + std::unique_ptr<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; > return TestFail; > } > > /* > * 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; > return TestFail; > diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp > index 25b5db67103a..ece987c7752a 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_.get())) { > cout << "Failed to set default configuration" << endl; > return TestFail; > } > @@ -48,7 +60,7 @@ protected: > return TestFail; > } > > - if (!camera_->configure(config)) { > + if (!camera_->configure(config_.get())) { > 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_.get())) { > cout << "Failed to set modified configuration" << endl; > return TestFail; > } > @@ -74,14 +86,16 @@ protected: > /* > * Test that setting an invalid configuration fails. > */ > - cfg->size = { 0, 0 }; > - if (!camera_->configure(config)) { > + cfg.size = { 0, 0 }; > + if (!camera_->configure(config_.get())) { > cout << "Invalid configuration incorrectly accepted" << endl; > return TestFail; > } > > return TestPass; > } > + > + std::unique_ptr<CameraConfiguration> config_; > }; > > } /* namespace */ > diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp > index 7a74cd85a37a..d489f197e402 100644 > --- a/test/camera/statemachine.cpp > +++ b/test/camera/statemachine.cpp > @@ -19,7 +19,7 @@ protected: > int testAvailable() > { > /* Test operations which should fail. */ > - if (camera_->configure(defconf_) != -EACCES) > + if (camera_->configure(defconf_.get()) != -EACCES) > return TestFail; > > if (camera_->allocateBuffers() != -EACCES) > @@ -84,7 +84,7 @@ protected: > if (camera_->acquire()) > return TestFail; > > - if (camera_->configure(defconf_)) > + if (camera_->configure(defconf_.get())) > return TestFail; > > return TestPass; > @@ -113,7 +113,7 @@ protected: > return TestFail; > > /* Test operations which should pass. */ > - if (camera_->configure(defconf_)) > + if (camera_->configure(defconf_.get())) > return TestFail; > > /* Test valid state transitions, end in Prepared state. */ > @@ -123,7 +123,7 @@ protected: > if (camera_->acquire()) > return TestFail; > > - if (camera_->configure(defconf_)) > + if (camera_->configure(defconf_.get())) > return TestFail; > > if (camera_->allocateBuffers()) > @@ -141,7 +141,7 @@ protected: > if (camera_->release() != -EBUSY) > return TestFail; > > - if (camera_->configure(defconf_) != -EACCES) > + if (camera_->configure(defconf_.get()) != -EACCES) > return TestFail; > > if (camera_->allocateBuffers() != -EACCES) > @@ -172,7 +172,7 @@ protected: > if (camera_->acquire()) > return TestFail; > > - if (camera_->configure(defconf_)) > + if (camera_->configure(defconf_.get())) > return TestFail; > > if (camera_->allocateBuffers()) > @@ -193,7 +193,7 @@ protected: > if (camera_->release() != -EBUSY) > return TestFail; > > - if (camera_->configure(defconf_) != -EACCES) > + if (camera_->configure(defconf_.get()) != -EACCES) > return TestFail; > > if (camera_->allocateBuffers() != -EACCES) > @@ -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,7 @@ protected: > return TestPass; > } > > - CameraConfiguration defconf_; > + std::unique_ptr<CameraConfiguration> defconf_; > }; > > } /* namespace */ > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 284e5276a055..a3a7289a7aa7 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); + std::unique_ptr<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..535c2420893e 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<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,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<CameraConfiguration> 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<Stream *, Buffer *> &buffers) @@ -191,16 +188,15 @@ static void requestComplete(Request *request, const std::map<Stream *, Buffer *> static int capture() { - CameraConfiguration config; int ret; - ret = prepareCameraConfig(&config); - if (ret) { + std::unique_ptr<CameraConfiguration> 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<Stream *, Buffer *> 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 5848330f639a..0e5fd7f57271 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -540,27 +540,32 @@ const std::set<Stream *> &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<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(); - return config; + return std::unique_ptr<CameraConfiguration>(config); } /** @@ -590,7 +595,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 +605,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 +613,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 +626,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..c8d217abdca8 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; + 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 5dcc868b2fc9..120d8d3a1522 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(); + 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 af6b6f21e3c5..f6aa32683e5e 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(); + 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 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..16b123132dd9 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 143b5b08a5a0..fe565cbcb460 100644 --- a/src/qcam/main_window.h +++ b/src/qcam/main_window.h @@ -8,6 +8,7 @@ #define __QCAM_MAIN_WINDOW_H__ #include <map> +#include <memory> #include <QMainWindow> @@ -45,7 +46,7 @@ private: std::shared_ptr<Camera> camera_; bool isCapturing_; - CameraConfiguration config_; + std::unique_ptr<CameraConfiguration> config_; ViewFinder *viewfinder_; }; diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index bfd11eefedcf..bb7d380cdc1a 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_.get())) { 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,8 @@ protected: return TestPass; } + + std::unique_ptr<CameraConfiguration> config_; }; } /* namespace */ diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp index 8c4a03db498a..8a767d2321e0 100644 --- a/test/camera/configuration_default.cpp +++ b/test/camera/configuration_default.cpp @@ -18,21 +18,21 @@ class ConfigurationDefault : public CameraTest protected: int run() { - CameraConfiguration config; + std::unique_ptr<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; return TestFail; } /* * 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; return TestFail; diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp index 25b5db67103a..ece987c7752a 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_.get())) { cout << "Failed to set default configuration" << endl; return TestFail; } @@ -48,7 +60,7 @@ protected: return TestFail; } - if (!camera_->configure(config)) { + if (!camera_->configure(config_.get())) { 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_.get())) { cout << "Failed to set modified configuration" << endl; return TestFail; } @@ -74,14 +86,16 @@ protected: /* * Test that setting an invalid configuration fails. */ - cfg->size = { 0, 0 }; - if (!camera_->configure(config)) { + cfg.size = { 0, 0 }; + if (!camera_->configure(config_.get())) { cout << "Invalid configuration incorrectly accepted" << endl; return TestFail; } return TestPass; } + + std::unique_ptr<CameraConfiguration> config_; }; } /* namespace */ diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp index 7a74cd85a37a..d489f197e402 100644 --- a/test/camera/statemachine.cpp +++ b/test/camera/statemachine.cpp @@ -19,7 +19,7 @@ protected: int testAvailable() { /* Test operations which should fail. */ - if (camera_->configure(defconf_) != -EACCES) + if (camera_->configure(defconf_.get()) != -EACCES) return TestFail; if (camera_->allocateBuffers() != -EACCES) @@ -84,7 +84,7 @@ protected: if (camera_->acquire()) return TestFail; - if (camera_->configure(defconf_)) + if (camera_->configure(defconf_.get())) return TestFail; return TestPass; @@ -113,7 +113,7 @@ protected: return TestFail; /* Test operations which should pass. */ - if (camera_->configure(defconf_)) + if (camera_->configure(defconf_.get())) return TestFail; /* Test valid state transitions, end in Prepared state. */ @@ -123,7 +123,7 @@ protected: if (camera_->acquire()) return TestFail; - if (camera_->configure(defconf_)) + if (camera_->configure(defconf_.get())) return TestFail; if (camera_->allocateBuffers()) @@ -141,7 +141,7 @@ protected: if (camera_->release() != -EBUSY) return TestFail; - if (camera_->configure(defconf_) != -EACCES) + if (camera_->configure(defconf_.get()) != -EACCES) return TestFail; if (camera_->allocateBuffers() != -EACCES) @@ -172,7 +172,7 @@ protected: if (camera_->acquire()) return TestFail; - if (camera_->configure(defconf_)) + if (camera_->configure(defconf_.get())) return TestFail; if (camera_->allocateBuffers()) @@ -193,7 +193,7 @@ protected: if (camera_->release() != -EBUSY) return TestFail; - if (camera_->configure(defconf_) != -EACCES) + if (camera_->configure(defconf_.get()) != -EACCES) return TestFail; if (camera_->allocateBuffers() != -EACCES) @@ -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,7 @@ protected: return TestPass; } - CameraConfiguration defconf_; + std::unique_ptr<CameraConfiguration> defconf_; }; } /* namespace */