[libcamera-devel] [PATCH v6 5/6] libcamera: pipeline: extend pipelines to support stream configuration

Niklas Söderlund niklas.soderlund at ragnatech.se
Fri Feb 1 08:20:42 CET 2019


Hi Laurent,

On 2019-02-01 02:04:17 +0200, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Thu, Jan 31, 2019 at 07:18:52PM +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, log that a default configuration have been
> 
> s/have been/has been/
> 
> > requested and log that a new configuration has been set. Future 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>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Thanks for all your hard work in reviewing this series which I just 
pushed to master.

> 
> > ---
> >  src/libcamera/include/pipeline_handler.h |  7 +++++
> >  src/libcamera/pipeline/ipu3/ipu3.cpp     | 36 ++++++++++++++++++++++++
> >  src/libcamera/pipeline/uvcvideo.cpp      | 32 +++++++++++++++++++++
> >  src/libcamera/pipeline/vimc.cpp          | 35 +++++++++++++++++++++++
> >  src/libcamera/pipeline_handler.cpp       | 34 ++++++++++++++++++++++
> >  5 files changed, 144 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;
> > +	virtual int configureStreams(Camera *camera,
> > +				     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..7823bbb55d9bde16 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) override;
> > +	int configureStreams(Camera *camera,
> > +			     std::map<Stream *, StreamConfiguration> &config) 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 = 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 = cameraData(camera);
> > +
> > +	StreamConfiguration *cfg = &config[&data->stream_];
> > +
> > +	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 d2d3a1edf6a9f53a..821e4c2189caabd0 100644
> > --- a/src/libcamera/pipeline/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo.cpp
> > @@ -24,6 +24,12 @@ public:
> >  	PipelineHandlerUVC(CameraManager *manager);
> >  	~PipelineHandlerUVC();
> >  
> > +	std::map<Stream *, StreamConfiguration>
> > +	streamConfiguration(Camera *camera,
> > +			    std::vector<Stream *> &streams) override;
> > +	int configureStreams(Camera *camera,
> > +			     std::map<Stream *, StreamConfiguration> &config) override;
> > +
> >  	bool match(DeviceEnumerator *enumerator);
> >  
> >  private:
> > @@ -46,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..6ed069edec550a61 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) override;
> > +	int configureStreams(Camera *camera,
> > +			     std::map<Stream *, StreamConfiguration> &config) override;
> > +
> >  	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

-- 
Regards,
Niklas Söderlund


More information about the libcamera-devel mailing list