[libcamera-devel] [PATCH v3 3/6] libcamera: Refactor the camera configuration storage and API
Jacopo Mondi
jacopo at jmondi.org
Tue May 21 22:00:42 CEST 2019
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 at jmondi.org>
Thanks
j
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at 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 at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190521/0aea5d13/attachment-0001.sig>
More information about the libcamera-devel
mailing list