[libcamera-devel] [PATCH v5 5/6] libcamera: pipeline: extend pipelines to support stream configuration
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Jan 31 01:02:55 CET 2019
Hi Niklas,
Thank you for the patch.
On Wed, Jan 30, 2019 at 12:56:14PM +0100, Niklas Söderlund wrote:
> All streams which are to be used for capture need to be configured at
> the same time. This allows the pipeline handler to take any dependencies
> between the different streams and their configuration into account when
> setting up the hardware.
>
> Extend the pipeline API and all pipeline implementations with two new
> functions, one to read a default configuration and one to set a new
> configuration. Both functions operate on a group of streams which the
> pipeline handler should consider when performing the operations.
>
> In the current implemented pipelines this is rather easy as they only
> have one stream each per camera. Furthermore as there is yet no way for
> the pipeline handlers to interact with the hardware all they do is
> return a null format and logs that a default configuration have been
s/and logs/, log/
> requested and log that a new configuration have been requested. Future
s/have been requested/has been set/
> work based on more components are needed to make the pipelines return a
> good default format and actually interact with the hardware.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> src/libcamera/include/pipeline_handler.h | 7 +++++
> src/libcamera/pipeline/ipu3/ipu3.cpp | 36 ++++++++++++++++++++++++
> src/libcamera/pipeline/uvcvideo.cpp | 35 +++++++++++++++++++++++
> src/libcamera/pipeline/vimc.cpp | 35 +++++++++++++++++++++++
> src/libcamera/pipeline_handler.cpp | 34 ++++++++++++++++++++++
> 5 files changed, 147 insertions(+)
>
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index 080997e22e545e01..b4321f0fa0f765be 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -18,6 +18,8 @@ class Camera;
> class CameraManager;
> class DeviceEnumerator;
> class MediaDevice;
> +class Stream;
> +class StreamConfiguration;
>
> class CameraData
> {
> @@ -38,6 +40,11 @@ public:
> PipelineHandler(CameraManager *manager);
> virtual ~PipelineHandler();
>
> + virtual std::map<Stream *, StreamConfiguration>
> + streamConfiguration(Camera *camera, std::vector<Stream *> &streams) = 0;
As this function doesn't change the state of the PipelineHandler, you
can make it
virtual std::map<Stream *, StreamConfiguration>
streamConfiguration(Camera *camera, std::vector<Stream *> &streams) const = 0;
> + virtual int configureStreams(Camera *camera,
> + std::map<Stream *, StreamConfiguration> &config) = 0;
And I would pass a const reference to config:
virtual int configureStreams(Camera *camera,
const std::map<Stream *, StreamConfiguration> &config) = 0;
> +
> virtual bool match(DeviceEnumerator *enumerator) = 0;
>
> protected:
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 52844da78419943d..39553b7c19823a52 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -28,6 +28,12 @@ public:
> PipelineHandlerIPU3(CameraManager *manager);
> ~PipelineHandlerIPU3();
>
> + std::map<Stream *, StreamConfiguration>
> + streamConfiguration(Camera *camera,
> + std::vector<Stream *> &streams);
> + int configureStreams(Camera *camera,
> + std::map<Stream *, StreamConfiguration> &config);
> +
A good practice is to mark overloaded functions with override.
> bool match(DeviceEnumerator *enumerator);
>
> private:
> @@ -68,6 +74,36 @@ PipelineHandlerIPU3::~PipelineHandlerIPU3()
> imgu_->release();
> }
>
> +std::map<Stream *, StreamConfiguration>
> +PipelineHandlerIPU3::streamConfiguration(Camera *camera,
> + std::vector<Stream *> &streams)
> +{
> + IPU3CameraData *data = dynamic_cast<IPU3CameraData *>(cameraData(camera));
We don't yse dynamic_cast in libcamera (see the Google C++ style guide).
The cameraData() method defined by PipelineHandlerIPU3 returns a
IPU3CameraData pointer already, so you can just do
IPU3CameraData *data = cameraData(camera));
> + std::map<Stream *, StreamConfiguration> configs;
> +
> + StreamConfiguration config{};
> +
> + LOG(IPU3, Info) << "TODO: Return a good default format";
> +
> + configs[&data->stream_] = config;
> +
> + return configs;
> +}
> +
> +int PipelineHandlerIPU3::configureStreams(Camera *camera,
> + std::map<Stream *, StreamConfiguration> &config)
> +{
> + IPU3CameraData *data = dynamic_cast<IPU3CameraData *>(cameraData(camera));
> +
> + StreamConfiguration *cfg = &config[&data->stream_];
This will throw an exception if the stream isn't part of the config. It
may be acceptable in a stub implementation, but will need to be fixed
very soon.
All these comments apply to the other pipeline handlers.
> +
> + LOG(IPU3, Info) << "TODO: Configure the camera for resolution " <<
> + cfg->width << "x" << cfg->height;
> +
> + return 0;
> +}
> +
> bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)
> {
> DeviceMatch cio2_dm("ipu3-cio2");
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 8ea7ac74d2a5395d..4647ed95e929940a 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -9,18 +9,27 @@
> #include <libcamera/stream.h>
>
> #include "device_enumerator.h"
> +#include "log.h"
> #include "media_device.h"
> #include "pipeline_handler.h"
> #include "v4l2_device.h"
>
> namespace libcamera {
>
> +LOG_DEFINE_CATEGORY(UVC)
> +
> class PipelineHandlerUVC : public PipelineHandler
> {
> public:
> PipelineHandlerUVC(CameraManager *manager);
> ~PipelineHandlerUVC();
>
> + std::map<Stream *, StreamConfiguration>
> + streamConfiguration(Camera *camera,
> + std::vector<Stream *> &streams);
> + int configureStreams(Camera *camera,
> + std::map<Stream *, StreamConfiguration> &config);
> +
> bool match(DeviceEnumerator *enumerator);
>
> private:
> @@ -43,6 +52,32 @@ PipelineHandlerUVC::~PipelineHandlerUVC()
> media_->release();
> }
>
> +std::map<Stream *, StreamConfiguration>
> +PipelineHandlerUVC::streamConfiguration(Camera *camera,
> + std::vector<Stream *> &streams)
> +{
> + std::map<Stream *, StreamConfiguration> configs;
> +
> + StreamConfiguration config{};
> +
> + LOG(UVC, Info) << "TODO: Return a good default format";
> +
> + configs[&stream_] = config;
> +
> + return configs;
> +}
> +
> +int PipelineHandlerUVC::configureStreams(Camera *camera,
> + std::map<Stream *, StreamConfiguration> &config)
> +{
> + StreamConfiguration *cfg = &config[&stream_];
> +
> + LOG(UVC, Info) << "TODO: Configure the camera for resolution " <<
> + cfg->width << "x" << cfg->height;
> +
> + return 0;
> +}
> +
> bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
> {
> DeviceMatch dm("uvcvideo");
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index 9e1cf11a20c7a7e3..835f8f512c4ac53a 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -9,17 +9,26 @@
> #include <libcamera/stream.h>
>
> #include "device_enumerator.h"
> +#include "log.h"
> #include "media_device.h"
> #include "pipeline_handler.h"
>
> namespace libcamera {
>
> +LOG_DEFINE_CATEGORY(VIMC)
> +
> class PipeHandlerVimc : public PipelineHandler
> {
> public:
> PipeHandlerVimc(CameraManager *manager);
> ~PipeHandlerVimc();
>
> + std::map<Stream *, StreamConfiguration>
> + streamConfiguration(Camera *camera,
> + std::vector<Stream *> &streams);
> + int configureStreams(Camera *camera,
> + std::map<Stream *, StreamConfiguration> &config);
> +
> bool match(DeviceEnumerator *enumerator);
>
> private:
> @@ -38,6 +47,32 @@ PipeHandlerVimc::~PipeHandlerVimc()
> media_->release();
> }
>
> +std::map<Stream *, StreamConfiguration>
> +PipeHandlerVimc::streamConfiguration(Camera *camera,
> + std::vector<Stream *> &streams)
> +{
> + std::map<Stream *, StreamConfiguration> configs;
> +
> + StreamConfiguration config{};
> +
> + LOG(VIMC, Info) << "TODO: Return a good default format";
> +
> + configs[&stream_] = config;
> +
> + return configs;
> +}
> +
> +int PipeHandlerVimc::configureStreams(Camera *camera,
> + std::map<Stream *, StreamConfiguration> &config)
> +{
> + StreamConfiguration *cfg = &config[&stream_];
> +
> + LOG(VIMC, Info) << "TODO: Configure the camera for resolution " <<
> + cfg->width << "x" << cfg->height;
> +
> + return 0;
> +}
> +
> bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)
> {
> DeviceMatch dm("vimc");
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 1c8640799f714686..3a560e10c442717f 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -75,6 +75,40 @@ PipelineHandler::~PipelineHandler()
> {
> };
>
> +
> +/**
> + * \fn PipelineHandler::streamConfiguration()
> + * \brief Retrieve a group of stream configurations for a specified camera
> + * \param[in] camera The camera to fetch default configuration from
> + * \param[in] streams An array of streams to fetch information about
> + *
> + * Retrieve the species camera's default configuration for a specified group of
> + * streams. The caller shall populate the \a streams array with the streams it
> + * wish to fetch the configuration from. The map of streams and configuration
> + * returned can then be examined by the caller to learn about the defualt
> + * parameters for the specified streams.
> + *
> + * The intended companion to this is \a configureStreams() which can be used to
> + * change the group of streams parameters.
> + *
> + * \return A map of successfully retrieved streams and configurations or an
> + * empty map on error.
> + */
> +
> +/**
> + * \fn PipelineHandler::configureStreams()
> + * \brief Configure a group of streams for capture
> + * \param[in] camera The camera to configure
> + * \param[in] config A map of stream configurations to apply
> + *
> + * Configure the specified group of streams for \a camera according to the
> + * configuration specified in \a configs. The intended caller of this interface
> + * is the Camera class which will receive configuration to apply from the
> + * application.
> + *
> + * \return 0 on success or a negative error code on error.
> + */
> +
> /**
> * \fn PipelineHandler::match(DeviceEnumerator *enumerator)
> * \brief Match media devices and create camera instances
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list