| Message ID | 20190521192740.28112-4-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On Tue, May 21, 2019 at 10:27:37PM +0300, Laurent Pinchart wrote: > Refactor the CameraConfiguration structure to not rely on Stream > instances. This is a step towards making the camera configuration object > more powerful with configuration validation using "try" semantics. > > The CameraConfiguration now exposes a simple vector-like API to access > the contained stream configurations. Both operator[]() and at() are > provided to access elements. The isEmpty() method is renamed to empty() > and the methods reordered to match the std::vector class. > > As applications need access to the Stream instances associated with the > configuration entries in order to associate buffers with streams when > creating requests, expose the stream selected by the pipeline handler > through a new StreamConfiguration::stream(). > I'm fine with having both (*config)[] and config->at() for the moment Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > Changes since v2: > > - Add a constructor to StreamConfiguration to initialise the private > stream_ pointer to nullptr > --- > include/libcamera/camera.h | 36 +-- > include/libcamera/stream.h | 12 + > src/cam/main.cpp | 35 +-- > src/libcamera/camera.cpp | 268 +++++++++++------------ > src/libcamera/include/pipeline_handler.h | 2 +- > src/libcamera/pipeline/ipu3/ipu3.cpp | 32 +-- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 +- > src/libcamera/pipeline/uvcvideo.cpp | 23 +- > src/libcamera/pipeline/vimc.cpp | 23 +- > src/libcamera/pipeline_handler.cpp | 4 + > src/libcamera/stream.cpp | 22 ++ > src/qcam/main_window.cpp | 4 +- > test/camera/capture.cpp | 4 +- > test/camera/configuration_set.cpp | 2 +- > 14 files changed, 256 insertions(+), 223 deletions(-) > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > index 42ba5201eabc..284e5276a055 100644 > --- a/include/libcamera/camera.h > +++ b/include/libcamera/camera.h > @@ -25,30 +25,36 @@ class Request; > class CameraConfiguration > { > public: > - using iterator = std::vector<Stream *>::iterator; > - using const_iterator = std::vector<Stream *>::const_iterator; > + using iterator = std::vector<StreamConfiguration>::iterator; > + using const_iterator = std::vector<StreamConfiguration>::const_iterator; > > CameraConfiguration(); > > + void addConfiguration(const StreamConfiguration &cfg); > + > + bool isValid() const; > + > + StreamConfiguration &at(unsigned int index); > + const StreamConfiguration &at(unsigned int index) const; > + StreamConfiguration &operator[](unsigned int index) > + { > + return at(index); > + } > + const StreamConfiguration &operator[](unsigned int index) const > + { > + return at(index); > + } > + > iterator begin(); > - iterator end(); > const_iterator begin() const; > + iterator end(); > const_iterator end() const; > > - bool isValid() const; > - bool isEmpty() const; > + bool empty() const; > std::size_t size() const; > > - Stream *front(); > - const Stream *front() const; > - > - Stream *operator[](unsigned int index) const; > - StreamConfiguration &operator[](Stream *stream); > - const StreamConfiguration &operator[](Stream *stream) const; > - > private: > - std::vector<Stream *> order_; > - std::map<Stream *, StreamConfiguration> config_; > + std::vector<StreamConfiguration> config_; > }; > > class Camera final > @@ -72,7 +78,7 @@ public: > > const std::set<Stream *> &streams() const; > CameraConfiguration generateConfiguration(const StreamRoles &roles); > - int configure(const CameraConfiguration &config); > + int configure(CameraConfiguration &config); > > int allocateBuffers(); > int freeBuffers(); > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h > index 59bdf217eb31..e38c0e7e827d 100644 > --- a/include/libcamera/stream.h > +++ b/include/libcamera/stream.h > @@ -16,14 +16,26 @@ > namespace libcamera { > > class Camera; > +class Stream; > > struct StreamConfiguration { > + StreamConfiguration() > + : stream_(nullptr) > + { > + } > + > unsigned int pixelFormat; > Size size; > > unsigned int bufferCount; > > + Stream *stream() const { return stream_; } > + void setStream(Stream *stream) { stream_ = stream; } > + > std::string toString() const; > + > +private: > + Stream *stream_; > }; > > enum StreamRole { > diff --git a/src/cam/main.cpp b/src/cam/main.cpp > index d603228c0116..cd165bea34cd 100644 > --- a/src/cam/main.cpp > +++ b/src/cam/main.cpp > @@ -89,12 +89,9 @@ static int prepareCameraConfig(CameraConfiguration *config) > { > StreamRoles roles; > > - streamInfo.clear(); > - > /* If no configuration is provided assume a single video stream. */ > if (!options.isSet(OptStream)) { > *config = camera->generateConfiguration({ StreamRole::VideoRecording }); > - streamInfo[config->front()] = "stream0"; > return 0; > } > > @@ -129,27 +126,20 @@ static int prepareCameraConfig(CameraConfiguration *config) > } > > /* Apply configuration explicitly requested. */ > - CameraConfiguration::iterator it = config->begin(); > + unsigned int i = 0; > for (auto const &value : streamOptions) { > KeyValueParser::Options conf = value.toKeyValues(); > - Stream *stream = *it; > - it++; > + StreamConfiguration &cfg = (*config)[i++]; > > if (conf.isSet("width")) > - (*config)[stream].size.width = conf["width"]; > + cfg.size.width = conf["width"]; > > if (conf.isSet("height")) > - (*config)[stream].size.height = conf["height"]; > + cfg.size.height = conf["height"]; > > /* TODO: Translate 4CC string to ID. */ > if (conf.isSet("pixelformat")) > - (*config)[stream].pixelFormat = conf["pixelformat"]; > - } > - > - unsigned int index = 0; > - for (Stream *stream : *config) { > - streamInfo[stream] = "stream" + std::to_string(index); > - index++; > + cfg.pixelFormat = conf["pixelformat"]; > } > > return 0; > @@ -216,6 +206,13 @@ static int capture() > return ret; > } > > + streamInfo.clear(); > + > + for (unsigned int index = 0; index < config.size(); ++index) { > + StreamConfiguration &cfg = config[index]; > + streamInfo[cfg.stream()] = "stream" + std::to_string(index); > + } > + > ret = camera->allocateBuffers(); > if (ret) { > std::cerr << "Failed to allocate buffers" > @@ -227,8 +224,10 @@ static int capture() > > /* Identify the stream with the least number of buffers. */ > unsigned int nbuffers = UINT_MAX; > - for (Stream *stream : config) > + for (StreamConfiguration &cfg : config) { > + Stream *stream = cfg.stream(); > nbuffers = std::min(nbuffers, stream->bufferPool().count()); > + } > > /* > * TODO: make cam tool smarter to support still capture by for > @@ -245,8 +244,10 @@ static int capture() > } > > std::map<Stream *, Buffer *> map; > - for (Stream *stream : config) > + for (StreamConfiguration &cfg : config) { > + Stream *stream = cfg.stream(); > map[stream] = &stream->bufferPool().buffers()[i]; > + } > > ret = request->setBuffers(map); > if (ret < 0) { > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index a3921f91f1c9..5848330f639a 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -46,72 +46,40 @@ LOG_DECLARE_CATEGORY(Camera) > * \class CameraConfiguration > * \brief Hold configuration for streams of the camera > > - * The CameraConfiguration holds an ordered list of streams and their associated > - * StreamConfiguration. From a data storage point of view, the class operates as > - * a map of Stream pointers to StreamConfiguration, with entries accessed with > - * operator[](Stream *). Accessing an entry for a Stream pointer not yet stored > - * in the configuration inserts a new empty entry. > - * > - * The class also suppors iterators, and from that point of view operates as a > - * vector of Stream pointers. The streams are iterated in insertion order, and > - * the operator[](int) returns the Stream pointer based on its insertion index. > - * Accessing a stream with an invalid index returns a null pointer. > + * The CameraConfiguration holds an ordered list of stream configurations. It > + * supports iterators and operates as a vector of StreamConfiguration instances. > + * The stream configurations are inserted by addConfiguration(), and the > + * operator[](int) returns a reference to the StreamConfiguration based on its > + * insertion index. Accessing a stream configuration with an invalid index > + * results in undefined behaviour. > */ > > /** > * \typedef CameraConfiguration::iterator > - * \brief Iterator for the streams in the configuration > + * \brief Iterator for the stream configurations in the camera configuration > */ > > /** > * \typedef CameraConfiguration::const_iterator > - * \brief Const iterator for the streams in the configuration > + * \brief Const iterator for the stream configuration in the camera > + * configuration > */ > > /** > * \brief Create an empty camera configuration > */ > CameraConfiguration::CameraConfiguration() > - : order_({}), config_({}) > + : config_({}) > { > } > > /** > - * \brief Retrieve an iterator to the first stream in the sequence > - * \return An iterator to the first stream > + * \brief Add a stream configuration to the camera configuration > + * \param[in] cfg The stream configuration > */ > -std::vector<Stream *>::iterator CameraConfiguration::begin() > +void CameraConfiguration::addConfiguration(const StreamConfiguration &cfg) > { > - return order_.begin(); > -} > - > -/** > - * \brief Retrieve an iterator pointing to the past-the-end stream in the > - * sequence > - * \return An iterator to the element following the last stream > - */ > -std::vector<Stream *>::iterator CameraConfiguration::end() > -{ > - return order_.end(); > -} > - > -/** > - * \brief Retrieve a const iterator to the first element of the streams > - * \return A const iterator to the first stream > - */ > -std::vector<Stream *>::const_iterator CameraConfiguration::begin() const > -{ > - return order_.begin(); > -} > - > -/** > - * \brief Retrieve a const iterator pointing to the past-the-end stream in the > - * sequence > - * \return A const iterator to the element following the last stream > - */ > -std::vector<Stream *>::const_iterator CameraConfiguration::end() const > -{ > - return order_.end(); > + config_.push_back(cfg); > } > > /** > @@ -125,12 +93,10 @@ std::vector<Stream *>::const_iterator CameraConfiguration::end() const > */ > bool CameraConfiguration::isValid() const > { > - if (isEmpty()) > + if (empty()) > return false; > > - for (auto const &it : config_) { > - const StreamConfiguration &cfg = it.second; > - > + for (const StreamConfiguration &cfg : config_) { > if (cfg.size.width == 0 || cfg.size.height == 0 || > cfg.pixelFormat == 0 || cfg.bufferCount == 0) > return false; > @@ -139,13 +105,108 @@ bool CameraConfiguration::isValid() const > return true; > } > > +/** > + * \brief Retrieve a reference to a stream configuration > + * \param[in] index Numerical index > + * > + * The \a index represents the zero based insertion order of stream > + * configuration into the camera configuration with addConfiguration(). Calling > + * this method with an invalid index results in undefined behaviour. > + * > + * \return The stream configuration > + */ > +StreamConfiguration &CameraConfiguration::at(unsigned int index) > +{ > + return config_[index]; > +} > + > +/** > + * \brief Retrieve a const reference to a stream configuration > + * \param[in] index Numerical index > + * > + * The \a index represents the zero based insertion order of stream > + * configuration into the camera configuration with addConfiguration(). Calling > + * this method with an invalid index results in undefined behaviour. > + * > + * \return The stream configuration > + */ > +const StreamConfiguration &CameraConfiguration::at(unsigned int index) const > +{ > + return config_[index]; > +} > + > +/** > + * \fn StreamConfiguration &CameraConfiguration::operator[](unsigned int) > + * \brief Retrieve a reference to a stream configuration > + * \param[in] index Numerical index > + * > + * The \a index represents the zero based insertion order of stream > + * configuration into the camera configuration with addConfiguration(). Calling > + * this method with an invalid index results in undefined behaviour. > + * > + * \return The stream configuration > + */ > + > +/** > + * \fn const StreamConfiguration &CameraConfiguration::operator[](unsigned int) const > + * \brief Retrieve a const reference to a stream configuration > + * \param[in] index Numerical index > + * > + * The \a index represents the zero based insertion order of stream > + * configuration into the camera configuration with addConfiguration(). Calling > + * this method with an invalid index results in undefined behaviour. > + * > + * \return The stream configuration > + */ > + > +/** > + * \brief Retrieve an iterator to the first stream configuration in the > + * sequence > + * \return An iterator to the first stream configuration > + */ > +CameraConfiguration::iterator CameraConfiguration::begin() > +{ > + return config_.begin(); > +} > + > +/** > + * \brief Retrieve a const iterator to the first element of the stream > + * configurations > + * \return A const iterator to the first stream configuration > + */ > +CameraConfiguration::const_iterator CameraConfiguration::begin() const > +{ > + return config_.begin(); > +} > + > +/** > + * \brief Retrieve an iterator pointing to the past-the-end stream > + * configuration in the sequence > + * \return An iterator to the element following the last stream configuration > + */ > +CameraConfiguration::iterator CameraConfiguration::end() > +{ > + return config_.end(); > +} > + > +/** > + * \brief Retrieve a const iterator pointing to the past-the-end stream > + * configuration in the sequence > + * \return A const iterator to the element following the last stream > + * configuration > + */ > +CameraConfiguration::const_iterator CameraConfiguration::end() const > +{ > + return config_.end(); > +} > + > /** > * \brief Check if the camera configuration is empty > * \return True if the configuration is empty > */ > -bool CameraConfiguration::isEmpty() const > +bool CameraConfiguration::empty() const > { > - return order_.empty(); > + return config_.empty(); > } > > /** > @@ -154,75 +215,7 @@ bool CameraConfiguration::isEmpty() const > */ > std::size_t CameraConfiguration::size() const > { > - return order_.size(); > -} > - > -/** > - * \brief Access the first stream in the configuration > - * \return The first stream in the configuration > - */ > -Stream *CameraConfiguration::front() > -{ > - return order_.front(); > -} > - > -/** > - * \brief Access the first stream in the configuration > - * \return The first const stream pointer in the configuration > - */ > -const Stream *CameraConfiguration::front() const > -{ > - return order_.front(); > -} > - > -/** > - * \brief Retrieve a stream pointer from index > - * \param[in] index Numerical index > - * > - * The \a index represents the zero based insertion order of stream and stream > - * configuration into the camera configuration. > - * > - * \return The stream pointer at index, or a nullptr if the index is out of > - * bounds > - */ > -Stream *CameraConfiguration::operator[](unsigned int index) const > -{ > - if (index >= order_.size()) > - return nullptr; > - > - return order_.at(index); > -} > - > -/** > - * \brief Retrieve a reference to a stream configuration > - * \param[in] stream Stream to retrieve configuration for > - * > - * If the camera configuration does not yet contain a configuration for > - * the requested stream, create and return an empty stream configuration. > - * > - * \return The configuration for the stream > - */ > -StreamConfiguration &CameraConfiguration::operator[](Stream *stream) > -{ > - if (config_.find(stream) == config_.end()) > - order_.push_back(stream); > - > - return config_[stream]; > -} > - > -/** > - * \brief Retrieve a const reference to a stream configuration > - * \param[in] stream Stream to retrieve configuration for > - * > - * No new stream configuration is created if called with \a stream that is not > - * already part of the camera configuration, doing so is an invalid operation > - * and results in undefined behaviour. > - * > - * \return The configuration for the stream > - */ > -const StreamConfiguration &CameraConfiguration::operator[](Stream *stream) const > -{ > - return config_.at(stream); > + return config_.size(); > } > > /** > @@ -561,13 +554,9 @@ Camera::generateConfiguration(const StreamRoles &roles) > CameraConfiguration config = pipe_->generateConfiguration(this, roles); > > std::ostringstream msg("streams configuration:", std::ios_base::ate); > - unsigned int index = 0; > > - for (Stream *stream : config) { > - const StreamConfiguration &cfg = config[stream]; > - msg << " (" << index << ") " << cfg.toString(); > - index++; > - } > + for (unsigned int index = 0; index < config.size(); ++index) > + msg << " (" << index << ") " << config[index].toString(); > > LOG(Camera, Debug) << msg.str(); > > @@ -593,12 +582,15 @@ Camera::generateConfiguration(const StreamRoles &roles) > * > * This function affects the state of the camera, see \ref camera_operation. > * > + * Upon return the StreamConfiguration entries in \a config are associated with > + * Stream instances which can be retrieved with StreamConfiguration::stream(). > + * > * \return 0 on success or a negative error code otherwise > * \retval -ENODEV The camera has been disconnected from the system > * \retval -EACCES The camera is not in a state where it can be configured > * \retval -EINVAL The configuration is not valid > */ > -int Camera::configure(const CameraConfiguration &config) > +int Camera::configure(CameraConfiguration &config) > { > int ret; > > @@ -615,16 +607,11 @@ int Camera::configure(const CameraConfiguration &config) > } > > std::ostringstream msg("configuring streams:", std::ios_base::ate); > - unsigned int index = 0; > > - for (Stream *stream : config) { > - if (streams_.find(stream) == streams_.end()) > - return -EINVAL; > - > - const StreamConfiguration &cfg = config[stream]; > - msg << std::dec << " (" << index << ") " << cfg.toString(); > - > - index++; > + for (unsigned int index = 0; index < config.size(); ++index) { > + StreamConfiguration &cfg = config[index]; > + cfg.setStream(nullptr); > + msg << " (" << index << ") " << cfg.toString(); > } > > LOG(Camera, Info) << msg.str(); > @@ -634,8 +621,11 @@ int Camera::configure(const CameraConfiguration &config) > return ret; > > activeStreams_.clear(); > - for (Stream *stream : config) { > - const StreamConfiguration &cfg = config[stream]; > + for (const StreamConfiguration &cfg : config) { > + Stream *stream = cfg.stream(); > + if (!stream) > + LOG(Camera, Fatal) > + << "Pipeline handler failed to update stream configuration"; > > stream->configuration_ = cfg; > activeStreams_.insert(stream); > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h > index 3352cb0e5bc9..a025905ab68f 100644 > --- a/src/libcamera/include/pipeline_handler.h > +++ b/src/libcamera/include/pipeline_handler.h > @@ -62,7 +62,7 @@ public: > > virtual CameraConfiguration > generateConfiguration(Camera *camera, const StreamRoles &roles) = 0; > - virtual int configure(Camera *camera, const CameraConfiguration &config) = 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 d234a8ac5289..ed0ef69de1d1 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -152,8 +152,7 @@ public: > > CameraConfiguration > generateConfiguration(Camera *camera, const StreamRoles &roles) override; > - int configure(Camera *camera, > - const CameraConfiguration &config) override; > + int configure(Camera *camera, CameraConfiguration &config) override; > > int allocateBuffers(Camera *camera, > const std::set<Stream *> &streams) override; > @@ -299,14 +298,13 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, > cfg.pixelFormat = V4L2_PIX_FMT_NV12; > cfg.bufferCount = IPU3_BUFFER_COUNT; > > - config[stream] = cfg; > + config.addConfiguration(cfg); > } > > return config; > } > > -int PipelineHandlerIPU3::configure(Camera *camera, > - const CameraConfiguration &config) > +int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config) > { > IPU3CameraData *data = cameraData(camera); > IPU3Stream *outStream = &data->outStream_; > @@ -318,9 +316,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, > > outStream->active_ = false; > vfStream->active_ = false; > - for (Stream *s : config) { > - IPU3Stream *stream = static_cast<IPU3Stream *>(s); > - const StreamConfiguration &cfg = config[stream]; > + for (StreamConfiguration &cfg : config) { > + /* > + * Pick a stream for the configuration entry. > + * \todo: This is a naive temporary implementation that will be > + * reworked when implementing camera configuration validation. > + */ > + IPU3Stream *stream = vfStream->active_ ? outStream : vfStream; > > /* > * Verify that the requested size respects the IPU3 alignment > @@ -355,6 +357,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, > sensorSize.height = cfg.size.height; > > stream->active_ = true; > + cfg.setStream(stream); > } > > /* > @@ -379,10 +382,9 @@ int PipelineHandlerIPU3::configure(Camera *camera, > return ret; > > /* Apply the format to the configured streams output devices. */ > - for (Stream *s : config) { > - IPU3Stream *stream = static_cast<IPU3Stream *>(s); > - > - ret = imgu->configureOutput(stream->device_, config[stream]); > + for (StreamConfiguration &cfg : config) { > + IPU3Stream *stream = static_cast<IPU3Stream *>(cfg.stream()); > + ret = imgu->configureOutput(stream->device_, cfg); > if (ret) > return ret; > } > @@ -393,15 +395,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, > * be at least one active stream in the configuration request). > */ > if (!outStream->active_) { > - ret = imgu->configureOutput(outStream->device_, > - config[vfStream]); > + ret = imgu->configureOutput(outStream->device_, config[0]); > if (ret) > return ret; > } > > if (!vfStream->active_) { > - ret = imgu->configureOutput(vfStream->device_, > - config[outStream]); > + ret = imgu->configureOutput(vfStream->device_, config[0]); > if (ret) > return ret; > } > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 4bd8c5101a96..ec6980b0943a 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -36,8 +36,7 @@ public: > > CameraConfiguration generateConfiguration(Camera *camera, > const StreamRoles &roles) override; > - int configure(Camera *camera, > - const CameraConfiguration &config) override; > + int configure(Camera *camera, CameraConfiguration &config) override; > > int allocateBuffers(Camera *camera, > const std::set<Stream *> &streams) override; > @@ -117,16 +116,15 @@ CameraConfiguration PipelineHandlerRkISP1::generateConfiguration(Camera *camera, > cfg.size = data->sensor_->resolution(); > cfg.bufferCount = RKISP1_BUFFER_COUNT; > > - config[&data->stream_] = cfg; > + config.addConfiguration(cfg); > > return config; > } > > -int PipelineHandlerRkISP1::configure(Camera *camera, > - const CameraConfiguration &config) > +int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration &config) > { > RkISP1CameraData *data = cameraData(camera); > - const StreamConfiguration &cfg = config[&data->stream_]; > + StreamConfiguration &cfg = config[0]; > CameraSensor *sensor = data->sensor_; > int ret; > > @@ -217,6 +215,8 @@ int PipelineHandlerRkISP1::configure(Camera *camera, > return -EINVAL; > } > > + cfg.setStream(&data->stream_); > + > return 0; > } > > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp > index d2e1f7d4e5b2..5dcc868b2fc9 100644 > --- a/src/libcamera/pipeline/uvcvideo.cpp > +++ b/src/libcamera/pipeline/uvcvideo.cpp > @@ -27,8 +27,7 @@ public: > > CameraConfiguration > generateConfiguration(Camera *camera, const StreamRoles &roles) override; > - int configure(Camera *camera, > - const CameraConfiguration &config) override; > + int configure(Camera *camera, CameraConfiguration &config) override; > > int allocateBuffers(Camera *camera, > const std::set<Stream *> &streams) override; > @@ -78,38 +77,38 @@ CameraConfiguration > PipelineHandlerUVC::generateConfiguration(Camera *camera, > const StreamRoles &roles) > { > - UVCCameraData *data = cameraData(camera); > CameraConfiguration config; > - StreamConfiguration cfg{}; > + StreamConfiguration cfg; > > cfg.pixelFormat = V4L2_PIX_FMT_YUYV; > cfg.size = { 640, 480 }; > cfg.bufferCount = 4; > > - config[&data->stream_] = cfg; > + config.addConfiguration(cfg); > > return config; > } > > -int PipelineHandlerUVC::configure(Camera *camera, > - const CameraConfiguration &config) > +int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration &config) > { > UVCCameraData *data = cameraData(camera); > - const StreamConfiguration *cfg = &config[&data->stream_]; > + StreamConfiguration &cfg = config[0]; > int ret; > > V4L2DeviceFormat format = {}; > - format.fourcc = cfg->pixelFormat; > - format.size = cfg->size; > + format.fourcc = cfg.pixelFormat; > + format.size = cfg.size; > > ret = data->video_->setFormat(&format); > if (ret) > return ret; > > - if (format.size != cfg->size || > - format.fourcc != cfg->pixelFormat) > + if (format.size != cfg.size || > + format.fourcc != cfg.pixelFormat) > return -EINVAL; > > + cfg.setStream(&data->stream_); > + > return 0; > } > > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp > index 17e2491e5c27..af6b6f21e3c5 100644 > --- a/src/libcamera/pipeline/vimc.cpp > +++ b/src/libcamera/pipeline/vimc.cpp > @@ -27,8 +27,7 @@ public: > > CameraConfiguration > generateConfiguration(Camera *camera, const StreamRoles &roles) override; > - int configure(Camera *camera, > - const CameraConfiguration &config) override; > + int configure(Camera *camera, CameraConfiguration &config) override; > > int allocateBuffers(Camera *camera, > const std::set<Stream *> &streams) override; > @@ -78,38 +77,38 @@ CameraConfiguration > PipelineHandlerVimc::generateConfiguration(Camera *camera, > const StreamRoles &roles) > { > - VimcCameraData *data = cameraData(camera); > CameraConfiguration config; > - StreamConfiguration cfg{}; > + StreamConfiguration cfg; > > cfg.pixelFormat = V4L2_PIX_FMT_RGB24; > cfg.size = { 640, 480 }; > cfg.bufferCount = 4; > > - config[&data->stream_] = cfg; > + config.addConfiguration(cfg); > > return config; > } > > -int PipelineHandlerVimc::configure(Camera *camera, > - const CameraConfiguration &config) > +int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration &config) > { > VimcCameraData *data = cameraData(camera); > - const StreamConfiguration *cfg = &config[&data->stream_]; > + StreamConfiguration &cfg = config[0]; > int ret; > > V4L2DeviceFormat format = {}; > - format.fourcc = cfg->pixelFormat; > - format.size = cfg->size; > + format.fourcc = cfg.pixelFormat; > + format.size = cfg.size; > > ret = data->video_->setFormat(&format); > if (ret) > return ret; > > - if (format.size != cfg->size || > - format.fourcc != cfg->pixelFormat) > + if (format.size != cfg.size || > + format.fourcc != cfg.pixelFormat) > return -EINVAL; > > + cfg.setStream(&data->stream_); > + > return 0; > } > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index 81c11149c9fe..4185e3557dcb 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -255,6 +255,10 @@ void PipelineHandler::unlock() > * configuration of a subset of the streams can't be satisfied, the > * whole configuration is considered invalid. > * > + * Once the configuration is validated and the camera configured, the pipeline > + * handler shall associate a Stream instance to each StreamConfiguration entry > + * in the CameraConfiguration with the StreamConfiguration::setStream() method. > + * > * \return 0 on success or a negative error code otherwise > */ > > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp > index fe4c4ecf4150..0c59a31a3a05 100644 > --- a/src/libcamera/stream.cpp > +++ b/src/libcamera/stream.cpp > @@ -58,6 +58,28 @@ namespace libcamera { > * \brief Requested number of buffers to allocate for the stream > */ > > +/** > + * \fn StreamConfiguration::stream() > + * \brief Retrieve the stream associated with the configuration > + * > + * When a camera is configured with Camera::configure() Stream instances are > + * associated with each stream configuration entry. This method retrieves the > + * associated Stream, which remains valid until the next call to > + * Camera::configure() or Camera::release(). > + * > + * \return The stream associated with the configuration > + */ > + > +/** > + * \fn StreamConfiguration::setStream() > + * \brief Associate a stream with a configuration > + * > + * This method is meant for the PipelineHandler::configure() method and shall > + * not be called by applications. > + * > + * \param[in] stream The stream > + */ > + > /** > * \brief Assemble and return a string describing the configuration > * > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp > index a984aaca764f..06ae2985f80d 100644 > --- a/src/qcam/main_window.cpp > +++ b/src/qcam/main_window.cpp > @@ -98,14 +98,14 @@ int MainWindow::startCapture() > int ret; > > config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); > - Stream *stream = config_.front(); > ret = camera_->configure(config_); > if (ret < 0) { > std::cout << "Failed to configure camera" << std::endl; > return ret; > } > > - const StreamConfiguration &cfg = config_[stream]; > + const StreamConfiguration &cfg = config_[0]; > + Stream *stream = cfg.stream(); > ret = viewfinder_->setFormat(cfg.pixelFormat, cfg.size.width, > cfg.size.height); > if (ret < 0) { > diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp > index e7e6438203b9..bfd11eefedcf 100644 > --- a/test/camera/capture.cpp > +++ b/test/camera/capture.cpp > @@ -44,8 +44,7 @@ protected: > { > CameraConfiguration config = > camera_->generateConfiguration({ StreamRole::VideoRecording }); > - Stream *stream = config.front(); > - StreamConfiguration *cfg = &config[stream]; > + StreamConfiguration *cfg = &config[0]; > > if (!config.isValid()) { > cout << "Failed to read default configuration" << endl; > @@ -67,6 +66,7 @@ protected: > return TestFail; > } > > + Stream *stream = cfg->stream(); > BufferPool &pool = stream->bufferPool(); > std::vector<Request *> requests; > for (Buffer &buffer : pool.buffers()) { > diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp > index 76d8bc3e40a4..25b5db67103a 100644 > --- a/test/camera/configuration_set.cpp > +++ b/test/camera/configuration_set.cpp > @@ -20,7 +20,7 @@ protected: > { > CameraConfiguration config = > camera_->generateConfiguration({ StreamRole::VideoRecording }); > - StreamConfiguration *cfg = &config[config.front()]; > + StreamConfiguration *cfg = &config[0]; > > if (!config.isValid()) { > cout << "Failed to read default configuration" << endl; > -- > 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 42ba5201eabc..284e5276a055 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -25,30 +25,36 @@ class Request; class CameraConfiguration { public: - using iterator = std::vector<Stream *>::iterator; - using const_iterator = std::vector<Stream *>::const_iterator; + using iterator = std::vector<StreamConfiguration>::iterator; + using const_iterator = std::vector<StreamConfiguration>::const_iterator; CameraConfiguration(); + void addConfiguration(const StreamConfiguration &cfg); + + bool isValid() const; + + StreamConfiguration &at(unsigned int index); + const StreamConfiguration &at(unsigned int index) const; + StreamConfiguration &operator[](unsigned int index) + { + return at(index); + } + const StreamConfiguration &operator[](unsigned int index) const + { + return at(index); + } + iterator begin(); - iterator end(); const_iterator begin() const; + iterator end(); const_iterator end() const; - bool isValid() const; - bool isEmpty() const; + bool empty() const; std::size_t size() const; - Stream *front(); - const Stream *front() const; - - Stream *operator[](unsigned int index) const; - StreamConfiguration &operator[](Stream *stream); - const StreamConfiguration &operator[](Stream *stream) const; - private: - std::vector<Stream *> order_; - std::map<Stream *, StreamConfiguration> config_; + std::vector<StreamConfiguration> config_; }; class Camera final @@ -72,7 +78,7 @@ public: const std::set<Stream *> &streams() const; CameraConfiguration generateConfiguration(const StreamRoles &roles); - int configure(const CameraConfiguration &config); + int configure(CameraConfiguration &config); int allocateBuffers(); int freeBuffers(); diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index 59bdf217eb31..e38c0e7e827d 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -16,14 +16,26 @@ namespace libcamera { class Camera; +class Stream; struct StreamConfiguration { + StreamConfiguration() + : stream_(nullptr) + { + } + unsigned int pixelFormat; Size size; unsigned int bufferCount; + Stream *stream() const { return stream_; } + void setStream(Stream *stream) { stream_ = stream; } + std::string toString() const; + +private: + Stream *stream_; }; enum StreamRole { diff --git a/src/cam/main.cpp b/src/cam/main.cpp index d603228c0116..cd165bea34cd 100644 --- a/src/cam/main.cpp +++ b/src/cam/main.cpp @@ -89,12 +89,9 @@ static int prepareCameraConfig(CameraConfiguration *config) { StreamRoles roles; - streamInfo.clear(); - /* If no configuration is provided assume a single video stream. */ if (!options.isSet(OptStream)) { *config = camera->generateConfiguration({ StreamRole::VideoRecording }); - streamInfo[config->front()] = "stream0"; return 0; } @@ -129,27 +126,20 @@ static int prepareCameraConfig(CameraConfiguration *config) } /* Apply configuration explicitly requested. */ - CameraConfiguration::iterator it = config->begin(); + unsigned int i = 0; for (auto const &value : streamOptions) { KeyValueParser::Options conf = value.toKeyValues(); - Stream *stream = *it; - it++; + StreamConfiguration &cfg = (*config)[i++]; if (conf.isSet("width")) - (*config)[stream].size.width = conf["width"]; + cfg.size.width = conf["width"]; if (conf.isSet("height")) - (*config)[stream].size.height = conf["height"]; + cfg.size.height = conf["height"]; /* TODO: Translate 4CC string to ID. */ if (conf.isSet("pixelformat")) - (*config)[stream].pixelFormat = conf["pixelformat"]; - } - - unsigned int index = 0; - for (Stream *stream : *config) { - streamInfo[stream] = "stream" + std::to_string(index); - index++; + cfg.pixelFormat = conf["pixelformat"]; } return 0; @@ -216,6 +206,13 @@ static int capture() return ret; } + streamInfo.clear(); + + for (unsigned int index = 0; index < config.size(); ++index) { + StreamConfiguration &cfg = config[index]; + streamInfo[cfg.stream()] = "stream" + std::to_string(index); + } + ret = camera->allocateBuffers(); if (ret) { std::cerr << "Failed to allocate buffers" @@ -227,8 +224,10 @@ static int capture() /* Identify the stream with the least number of buffers. */ unsigned int nbuffers = UINT_MAX; - for (Stream *stream : config) + for (StreamConfiguration &cfg : config) { + Stream *stream = cfg.stream(); nbuffers = std::min(nbuffers, stream->bufferPool().count()); + } /* * TODO: make cam tool smarter to support still capture by for @@ -245,8 +244,10 @@ static int capture() } std::map<Stream *, Buffer *> map; - for (Stream *stream : config) + for (StreamConfiguration &cfg : config) { + Stream *stream = cfg.stream(); map[stream] = &stream->bufferPool().buffers()[i]; + } ret = request->setBuffers(map); if (ret < 0) { diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index a3921f91f1c9..5848330f639a 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -46,72 +46,40 @@ LOG_DECLARE_CATEGORY(Camera) * \class CameraConfiguration * \brief Hold configuration for streams of the camera - * The CameraConfiguration holds an ordered list of streams and their associated - * StreamConfiguration. From a data storage point of view, the class operates as - * a map of Stream pointers to StreamConfiguration, with entries accessed with - * operator[](Stream *). Accessing an entry for a Stream pointer not yet stored - * in the configuration inserts a new empty entry. - * - * The class also suppors iterators, and from that point of view operates as a - * vector of Stream pointers. The streams are iterated in insertion order, and - * the operator[](int) returns the Stream pointer based on its insertion index. - * Accessing a stream with an invalid index returns a null pointer. + * The CameraConfiguration holds an ordered list of stream configurations. It + * supports iterators and operates as a vector of StreamConfiguration instances. + * The stream configurations are inserted by addConfiguration(), and the + * operator[](int) returns a reference to the StreamConfiguration based on its + * insertion index. Accessing a stream configuration with an invalid index + * results in undefined behaviour. */ /** * \typedef CameraConfiguration::iterator - * \brief Iterator for the streams in the configuration + * \brief Iterator for the stream configurations in the camera configuration */ /** * \typedef CameraConfiguration::const_iterator - * \brief Const iterator for the streams in the configuration + * \brief Const iterator for the stream configuration in the camera + * configuration */ /** * \brief Create an empty camera configuration */ CameraConfiguration::CameraConfiguration() - : order_({}), config_({}) + : config_({}) { } /** - * \brief Retrieve an iterator to the first stream in the sequence - * \return An iterator to the first stream + * \brief Add a stream configuration to the camera configuration + * \param[in] cfg The stream configuration */ -std::vector<Stream *>::iterator CameraConfiguration::begin() +void CameraConfiguration::addConfiguration(const StreamConfiguration &cfg) { - return order_.begin(); -} - -/** - * \brief Retrieve an iterator pointing to the past-the-end stream in the - * sequence - * \return An iterator to the element following the last stream - */ -std::vector<Stream *>::iterator CameraConfiguration::end() -{ - return order_.end(); -} - -/** - * \brief Retrieve a const iterator to the first element of the streams - * \return A const iterator to the first stream - */ -std::vector<Stream *>::const_iterator CameraConfiguration::begin() const -{ - return order_.begin(); -} - -/** - * \brief Retrieve a const iterator pointing to the past-the-end stream in the - * sequence - * \return A const iterator to the element following the last stream - */ -std::vector<Stream *>::const_iterator CameraConfiguration::end() const -{ - return order_.end(); + config_.push_back(cfg); } /** @@ -125,12 +93,10 @@ std::vector<Stream *>::const_iterator CameraConfiguration::end() const */ bool CameraConfiguration::isValid() const { - if (isEmpty()) + if (empty()) return false; - for (auto const &it : config_) { - const StreamConfiguration &cfg = it.second; - + for (const StreamConfiguration &cfg : config_) { if (cfg.size.width == 0 || cfg.size.height == 0 || cfg.pixelFormat == 0 || cfg.bufferCount == 0) return false; @@ -139,13 +105,108 @@ bool CameraConfiguration::isValid() const return true; } +/** + * \brief Retrieve a reference to a stream configuration + * \param[in] index Numerical index + * + * The \a index represents the zero based insertion order of stream + * configuration into the camera configuration with addConfiguration(). Calling + * this method with an invalid index results in undefined behaviour. + * + * \return The stream configuration + */ +StreamConfiguration &CameraConfiguration::at(unsigned int index) +{ + return config_[index]; +} + +/** + * \brief Retrieve a const reference to a stream configuration + * \param[in] index Numerical index + * + * The \a index represents the zero based insertion order of stream + * configuration into the camera configuration with addConfiguration(). Calling + * this method with an invalid index results in undefined behaviour. + * + * \return The stream configuration + */ +const StreamConfiguration &CameraConfiguration::at(unsigned int index) const +{ + return config_[index]; +} + +/** + * \fn StreamConfiguration &CameraConfiguration::operator[](unsigned int) + * \brief Retrieve a reference to a stream configuration + * \param[in] index Numerical index + * + * The \a index represents the zero based insertion order of stream + * configuration into the camera configuration with addConfiguration(). Calling + * this method with an invalid index results in undefined behaviour. + * + * \return The stream configuration + */ + +/** + * \fn const StreamConfiguration &CameraConfiguration::operator[](unsigned int) const + * \brief Retrieve a const reference to a stream configuration + * \param[in] index Numerical index + * + * The \a index represents the zero based insertion order of stream + * configuration into the camera configuration with addConfiguration(). Calling + * this method with an invalid index results in undefined behaviour. + * + * \return The stream configuration + */ + +/** + * \brief Retrieve an iterator to the first stream configuration in the + * sequence + * \return An iterator to the first stream configuration + */ +CameraConfiguration::iterator CameraConfiguration::begin() +{ + return config_.begin(); +} + +/** + * \brief Retrieve a const iterator to the first element of the stream + * configurations + * \return A const iterator to the first stream configuration + */ +CameraConfiguration::const_iterator CameraConfiguration::begin() const +{ + return config_.begin(); +} + +/** + * \brief Retrieve an iterator pointing to the past-the-end stream + * configuration in the sequence + * \return An iterator to the element following the last stream configuration + */ +CameraConfiguration::iterator CameraConfiguration::end() +{ + return config_.end(); +} + +/** + * \brief Retrieve a const iterator pointing to the past-the-end stream + * configuration in the sequence + * \return A const iterator to the element following the last stream + * configuration + */ +CameraConfiguration::const_iterator CameraConfiguration::end() const +{ + return config_.end(); +} + /** * \brief Check if the camera configuration is empty * \return True if the configuration is empty */ -bool CameraConfiguration::isEmpty() const +bool CameraConfiguration::empty() const { - return order_.empty(); + return config_.empty(); } /** @@ -154,75 +215,7 @@ bool CameraConfiguration::isEmpty() const */ std::size_t CameraConfiguration::size() const { - return order_.size(); -} - -/** - * \brief Access the first stream in the configuration - * \return The first stream in the configuration - */ -Stream *CameraConfiguration::front() -{ - return order_.front(); -} - -/** - * \brief Access the first stream in the configuration - * \return The first const stream pointer in the configuration - */ -const Stream *CameraConfiguration::front() const -{ - return order_.front(); -} - -/** - * \brief Retrieve a stream pointer from index - * \param[in] index Numerical index - * - * The \a index represents the zero based insertion order of stream and stream - * configuration into the camera configuration. - * - * \return The stream pointer at index, or a nullptr if the index is out of - * bounds - */ -Stream *CameraConfiguration::operator[](unsigned int index) const -{ - if (index >= order_.size()) - return nullptr; - - return order_.at(index); -} - -/** - * \brief Retrieve a reference to a stream configuration - * \param[in] stream Stream to retrieve configuration for - * - * If the camera configuration does not yet contain a configuration for - * the requested stream, create and return an empty stream configuration. - * - * \return The configuration for the stream - */ -StreamConfiguration &CameraConfiguration::operator[](Stream *stream) -{ - if (config_.find(stream) == config_.end()) - order_.push_back(stream); - - return config_[stream]; -} - -/** - * \brief Retrieve a const reference to a stream configuration - * \param[in] stream Stream to retrieve configuration for - * - * No new stream configuration is created if called with \a stream that is not - * already part of the camera configuration, doing so is an invalid operation - * and results in undefined behaviour. - * - * \return The configuration for the stream - */ -const StreamConfiguration &CameraConfiguration::operator[](Stream *stream) const -{ - return config_.at(stream); + return config_.size(); } /** @@ -561,13 +554,9 @@ Camera::generateConfiguration(const StreamRoles &roles) CameraConfiguration config = pipe_->generateConfiguration(this, roles); std::ostringstream msg("streams configuration:", std::ios_base::ate); - unsigned int index = 0; - for (Stream *stream : config) { - const StreamConfiguration &cfg = config[stream]; - msg << " (" << index << ") " << cfg.toString(); - index++; - } + for (unsigned int index = 0; index < config.size(); ++index) + msg << " (" << index << ") " << config[index].toString(); LOG(Camera, Debug) << msg.str(); @@ -593,12 +582,15 @@ Camera::generateConfiguration(const StreamRoles &roles) * * This function affects the state of the camera, see \ref camera_operation. * + * Upon return the StreamConfiguration entries in \a config are associated with + * Stream instances which can be retrieved with StreamConfiguration::stream(). + * * \return 0 on success or a negative error code otherwise * \retval -ENODEV The camera has been disconnected from the system * \retval -EACCES The camera is not in a state where it can be configured * \retval -EINVAL The configuration is not valid */ -int Camera::configure(const CameraConfiguration &config) +int Camera::configure(CameraConfiguration &config) { int ret; @@ -615,16 +607,11 @@ int Camera::configure(const CameraConfiguration &config) } std::ostringstream msg("configuring streams:", std::ios_base::ate); - unsigned int index = 0; - for (Stream *stream : config) { - if (streams_.find(stream) == streams_.end()) - return -EINVAL; - - const StreamConfiguration &cfg = config[stream]; - msg << std::dec << " (" << index << ") " << cfg.toString(); - - index++; + for (unsigned int index = 0; index < config.size(); ++index) { + StreamConfiguration &cfg = config[index]; + cfg.setStream(nullptr); + msg << " (" << index << ") " << cfg.toString(); } LOG(Camera, Info) << msg.str(); @@ -634,8 +621,11 @@ int Camera::configure(const CameraConfiguration &config) return ret; activeStreams_.clear(); - for (Stream *stream : config) { - const StreamConfiguration &cfg = config[stream]; + for (const StreamConfiguration &cfg : config) { + Stream *stream = cfg.stream(); + if (!stream) + LOG(Camera, Fatal) + << "Pipeline handler failed to update stream configuration"; stream->configuration_ = cfg; activeStreams_.insert(stream); diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 3352cb0e5bc9..a025905ab68f 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -62,7 +62,7 @@ public: virtual CameraConfiguration generateConfiguration(Camera *camera, const StreamRoles &roles) = 0; - virtual int configure(Camera *camera, const CameraConfiguration &config) = 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 d234a8ac5289..ed0ef69de1d1 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -152,8 +152,7 @@ public: CameraConfiguration generateConfiguration(Camera *camera, const StreamRoles &roles) override; - int configure(Camera *camera, - const CameraConfiguration &config) override; + int configure(Camera *camera, CameraConfiguration &config) override; int allocateBuffers(Camera *camera, const std::set<Stream *> &streams) override; @@ -299,14 +298,13 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, cfg.pixelFormat = V4L2_PIX_FMT_NV12; cfg.bufferCount = IPU3_BUFFER_COUNT; - config[stream] = cfg; + config.addConfiguration(cfg); } return config; } -int PipelineHandlerIPU3::configure(Camera *camera, - const CameraConfiguration &config) +int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config) { IPU3CameraData *data = cameraData(camera); IPU3Stream *outStream = &data->outStream_; @@ -318,9 +316,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, outStream->active_ = false; vfStream->active_ = false; - for (Stream *s : config) { - IPU3Stream *stream = static_cast<IPU3Stream *>(s); - const StreamConfiguration &cfg = config[stream]; + for (StreamConfiguration &cfg : config) { + /* + * Pick a stream for the configuration entry. + * \todo: This is a naive temporary implementation that will be + * reworked when implementing camera configuration validation. + */ + IPU3Stream *stream = vfStream->active_ ? outStream : vfStream; /* * Verify that the requested size respects the IPU3 alignment @@ -355,6 +357,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, sensorSize.height = cfg.size.height; stream->active_ = true; + cfg.setStream(stream); } /* @@ -379,10 +382,9 @@ int PipelineHandlerIPU3::configure(Camera *camera, return ret; /* Apply the format to the configured streams output devices. */ - for (Stream *s : config) { - IPU3Stream *stream = static_cast<IPU3Stream *>(s); - - ret = imgu->configureOutput(stream->device_, config[stream]); + for (StreamConfiguration &cfg : config) { + IPU3Stream *stream = static_cast<IPU3Stream *>(cfg.stream()); + ret = imgu->configureOutput(stream->device_, cfg); if (ret) return ret; } @@ -393,15 +395,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, * be at least one active stream in the configuration request). */ if (!outStream->active_) { - ret = imgu->configureOutput(outStream->device_, - config[vfStream]); + ret = imgu->configureOutput(outStream->device_, config[0]); if (ret) return ret; } if (!vfStream->active_) { - ret = imgu->configureOutput(vfStream->device_, - config[outStream]); + ret = imgu->configureOutput(vfStream->device_, config[0]); if (ret) return ret; } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 4bd8c5101a96..ec6980b0943a 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -36,8 +36,7 @@ public: CameraConfiguration generateConfiguration(Camera *camera, const StreamRoles &roles) override; - int configure(Camera *camera, - const CameraConfiguration &config) override; + int configure(Camera *camera, CameraConfiguration &config) override; int allocateBuffers(Camera *camera, const std::set<Stream *> &streams) override; @@ -117,16 +116,15 @@ CameraConfiguration PipelineHandlerRkISP1::generateConfiguration(Camera *camera, cfg.size = data->sensor_->resolution(); cfg.bufferCount = RKISP1_BUFFER_COUNT; - config[&data->stream_] = cfg; + config.addConfiguration(cfg); return config; } -int PipelineHandlerRkISP1::configure(Camera *camera, - const CameraConfiguration &config) +int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration &config) { RkISP1CameraData *data = cameraData(camera); - const StreamConfiguration &cfg = config[&data->stream_]; + StreamConfiguration &cfg = config[0]; CameraSensor *sensor = data->sensor_; int ret; @@ -217,6 +215,8 @@ int PipelineHandlerRkISP1::configure(Camera *camera, return -EINVAL; } + cfg.setStream(&data->stream_); + return 0; } diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index d2e1f7d4e5b2..5dcc868b2fc9 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -27,8 +27,7 @@ public: CameraConfiguration generateConfiguration(Camera *camera, const StreamRoles &roles) override; - int configure(Camera *camera, - const CameraConfiguration &config) override; + int configure(Camera *camera, CameraConfiguration &config) override; int allocateBuffers(Camera *camera, const std::set<Stream *> &streams) override; @@ -78,38 +77,38 @@ CameraConfiguration PipelineHandlerUVC::generateConfiguration(Camera *camera, const StreamRoles &roles) { - UVCCameraData *data = cameraData(camera); CameraConfiguration config; - StreamConfiguration cfg{}; + StreamConfiguration cfg; cfg.pixelFormat = V4L2_PIX_FMT_YUYV; cfg.size = { 640, 480 }; cfg.bufferCount = 4; - config[&data->stream_] = cfg; + config.addConfiguration(cfg); return config; } -int PipelineHandlerUVC::configure(Camera *camera, - const CameraConfiguration &config) +int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration &config) { UVCCameraData *data = cameraData(camera); - const StreamConfiguration *cfg = &config[&data->stream_]; + StreamConfiguration &cfg = config[0]; int ret; V4L2DeviceFormat format = {}; - format.fourcc = cfg->pixelFormat; - format.size = cfg->size; + format.fourcc = cfg.pixelFormat; + format.size = cfg.size; ret = data->video_->setFormat(&format); if (ret) return ret; - if (format.size != cfg->size || - format.fourcc != cfg->pixelFormat) + if (format.size != cfg.size || + format.fourcc != cfg.pixelFormat) return -EINVAL; + cfg.setStream(&data->stream_); + return 0; } diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 17e2491e5c27..af6b6f21e3c5 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -27,8 +27,7 @@ public: CameraConfiguration generateConfiguration(Camera *camera, const StreamRoles &roles) override; - int configure(Camera *camera, - const CameraConfiguration &config) override; + int configure(Camera *camera, CameraConfiguration &config) override; int allocateBuffers(Camera *camera, const std::set<Stream *> &streams) override; @@ -78,38 +77,38 @@ CameraConfiguration PipelineHandlerVimc::generateConfiguration(Camera *camera, const StreamRoles &roles) { - VimcCameraData *data = cameraData(camera); CameraConfiguration config; - StreamConfiguration cfg{}; + StreamConfiguration cfg; cfg.pixelFormat = V4L2_PIX_FMT_RGB24; cfg.size = { 640, 480 }; cfg.bufferCount = 4; - config[&data->stream_] = cfg; + config.addConfiguration(cfg); return config; } -int PipelineHandlerVimc::configure(Camera *camera, - const CameraConfiguration &config) +int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration &config) { VimcCameraData *data = cameraData(camera); - const StreamConfiguration *cfg = &config[&data->stream_]; + StreamConfiguration &cfg = config[0]; int ret; V4L2DeviceFormat format = {}; - format.fourcc = cfg->pixelFormat; - format.size = cfg->size; + format.fourcc = cfg.pixelFormat; + format.size = cfg.size; ret = data->video_->setFormat(&format); if (ret) return ret; - if (format.size != cfg->size || - format.fourcc != cfg->pixelFormat) + if (format.size != cfg.size || + format.fourcc != cfg.pixelFormat) return -EINVAL; + cfg.setStream(&data->stream_); + return 0; } diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 81c11149c9fe..4185e3557dcb 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -255,6 +255,10 @@ void PipelineHandler::unlock() * configuration of a subset of the streams can't be satisfied, the * whole configuration is considered invalid. * + * Once the configuration is validated and the camera configured, the pipeline + * handler shall associate a Stream instance to each StreamConfiguration entry + * in the CameraConfiguration with the StreamConfiguration::setStream() method. + * * \return 0 on success or a negative error code otherwise */ diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index fe4c4ecf4150..0c59a31a3a05 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -58,6 +58,28 @@ namespace libcamera { * \brief Requested number of buffers to allocate for the stream */ +/** + * \fn StreamConfiguration::stream() + * \brief Retrieve the stream associated with the configuration + * + * When a camera is configured with Camera::configure() Stream instances are + * associated with each stream configuration entry. This method retrieves the + * associated Stream, which remains valid until the next call to + * Camera::configure() or Camera::release(). + * + * \return The stream associated with the configuration + */ + +/** + * \fn StreamConfiguration::setStream() + * \brief Associate a stream with a configuration + * + * This method is meant for the PipelineHandler::configure() method and shall + * not be called by applications. + * + * \param[in] stream The stream + */ + /** * \brief Assemble and return a string describing the configuration * diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index a984aaca764f..06ae2985f80d 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -98,14 +98,14 @@ int MainWindow::startCapture() int ret; config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); - Stream *stream = config_.front(); ret = camera_->configure(config_); if (ret < 0) { std::cout << "Failed to configure camera" << std::endl; return ret; } - const StreamConfiguration &cfg = config_[stream]; + const StreamConfiguration &cfg = config_[0]; + Stream *stream = cfg.stream(); ret = viewfinder_->setFormat(cfg.pixelFormat, cfg.size.width, cfg.size.height); if (ret < 0) { diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index e7e6438203b9..bfd11eefedcf 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -44,8 +44,7 @@ protected: { CameraConfiguration config = camera_->generateConfiguration({ StreamRole::VideoRecording }); - Stream *stream = config.front(); - StreamConfiguration *cfg = &config[stream]; + StreamConfiguration *cfg = &config[0]; if (!config.isValid()) { cout << "Failed to read default configuration" << endl; @@ -67,6 +66,7 @@ protected: return TestFail; } + Stream *stream = cfg->stream(); BufferPool &pool = stream->bufferPool(); std::vector<Request *> requests; for (Buffer &buffer : pool.buffers()) { diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp index 76d8bc3e40a4..25b5db67103a 100644 --- a/test/camera/configuration_set.cpp +++ b/test/camera/configuration_set.cpp @@ -20,7 +20,7 @@ protected: { CameraConfiguration config = camera_->generateConfiguration({ StreamRole::VideoRecording }); - StreamConfiguration *cfg = &config[config.front()]; + StreamConfiguration *cfg = &config[0]; if (!config.isValid()) { cout << "Failed to read default configuration" << endl;