[libcamera-devel] [PATCH v3 2/6] libcamera: Use stream roles directly instead of StreamUsage

Jacopo Mondi jacopo at jmondi.org
Tue May 21 21:50:50 CEST 2019


Hi Laurent,
   just one minor thing

On Tue, May 21, 2019 at 10:27:36PM +0300, Laurent Pinchart wrote:

[snip]

> @@ -275,15 +273,13 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera,
>  			stream = &data->vfStream_;
>
>  			/*
> -			 * Align the requested viewfinder size to the
> -			 * maximum available sensor resolution and to the
> -			 * IPU3 alignment constraints.
> +			 * Align the default viewfinder size to the maximum
> +			 * available sensor resolution and to the IPU3
> +			 * alignment constraints.
>  			 */
>  			const Size &res = data->cio2_.sensor_->resolution();
> -			unsigned int width = std::min(usage.size().width,
> -						      res.width);
> -			unsigned int height = std::min(usage.size().height,
> -						       res.height);
> +			unsigned int width = std::min(1280U, res.width);
> +			unsigned int height = std::min(720U, res.height);

From my testing it's safe to use IPU3 at 2560x1920, just tested on
Soraka though

I think you've lost my form v2
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

>  			cfg.size = { width & ~7, height & ~3 };
>
>  			break;
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 4d02f9604ad9..4bd8c5101a96 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -35,7 +35,7 @@ public:
>  	~PipelineHandlerRkISP1();
>
>  	CameraConfiguration generateConfiguration(Camera *camera,
> -		const std::vector<StreamUsage> &usages) override;
> +		const StreamRoles &roles) override;
>  	int configure(Camera *camera,
>  		const CameraConfiguration &config) override;
>
> @@ -107,7 +107,7 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1()
>   */
>
>  CameraConfiguration PipelineHandlerRkISP1::generateConfiguration(Camera *camera,
> -	const std::vector<StreamUsage> &usages)
> +	const StreamRoles &roles)
>  {
>  	RkISP1CameraData *data = cameraData(camera);
>  	CameraConfiguration config;
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 118b97457d2a..d2e1f7d4e5b2 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -26,8 +26,7 @@ public:
>  	PipelineHandlerUVC(CameraManager *manager);
>
>  	CameraConfiguration
> -	generateConfiguration(Camera *camera,
> -			      const std::vector<StreamUsage> &usages) override;
> +	generateConfiguration(Camera *camera, const StreamRoles &roles) override;
>  	int configure(Camera *camera,
>  		      const CameraConfiguration &config) override;
>
> @@ -77,7 +76,7 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)
>
>  CameraConfiguration
>  PipelineHandlerUVC::generateConfiguration(Camera *camera,
> -					  const std::vector<StreamUsage> &usages)
> +					  const StreamRoles &roles)
>  {
>  	UVCCameraData *data = cameraData(camera);
>  	CameraConfiguration config;
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index 74959581a7ef..17e2491e5c27 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -26,8 +26,7 @@ public:
>  	PipelineHandlerVimc(CameraManager *manager);
>
>  	CameraConfiguration
> -	generateConfiguration(Camera *camera,
> -			      const std::vector<StreamUsage> &usages) override;
> +	generateConfiguration(Camera *camera, const StreamRoles &roles) override;
>  	int configure(Camera *camera,
>  		      const CameraConfiguration &config) override;
>
> @@ -77,7 +76,7 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager)
>
>  CameraConfiguration
>  PipelineHandlerVimc::generateConfiguration(Camera *camera,
> -					   const std::vector<StreamUsage> &usages)
> +					   const StreamRoles &roles)
>  {
>  	VimcCameraData *data = cameraData(camera);
>  	CameraConfiguration config;
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index b9ac64328f1d..81c11149c9fe 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -221,18 +221,18 @@ void PipelineHandler::unlock()
>   * \fn PipelineHandler::generateConfiguration()
>   * \brief Generate a camera configuration for a specified camera
>   * \param[in] camera The camera to generate a default configuration for
> - * \param[in] usages A list of stream usages
> + * \param[in] roles A list of stream roles
>   *
> - * Generate a default configuration for the \a camera for a specified group of
> - * use-cases. The caller shall populate the \a usages array with the use-cases
> - * it wishes to fetch the default configuration for. The returned configuration
> + * Generate a default configuration for the \a camera for a specified list of
> + * stream roles. The caller shall populate the \a roles with the use-cases it
> + * wishes to fetch the default configuration for. The returned configuration
>   * can then be examined by the caller to learn about the selected streams and
>   * their default parameters.
>   *
>   * The intended companion to this is \a configure() which can be used to change
>   * the group of streams parameters.
>   *
> - * \return A valid CameraConfiguration if the requested usages can be satisfied,
> + * \return A valid CameraConfiguration if the requested roles can be satisfied,
>   * or a invalid configuration otherwise
>   */
>
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> index af259510b19c..fe4c4ecf4150 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -75,61 +75,31 @@ std::string StreamConfiguration::toString() const
>  }
>
>  /**
> - * \class StreamUsage
> - * \brief Stream usage information
> - *
> - * The StreamUsage class describes how an application intends to use a stream.
> - * Usages are specified by applications and passed to cameras, that then select
> - * the most appropriate streams and their default configurations.
> - */
> -
> -/**
> - * \enum StreamUsage::Role
> + * \enum StreamRole
>   * \brief Identify the role a stream is intended to play
> - * \var StreamUsage::StillCapture
> + *
> + * The StreamRole describes how an application intends to use a stream. Roles
> + * are specified by applications and passed to cameras, that then select the
> + * most appropriate streams and their default configurations.
> + *
> + * \var StillCapture
>   * The stream is intended to capture high-resolution, high-quality still images
>   * with low frame rate. The captured frames may be exposed with flash.
> - * \var StreamUsage::VideoRecording
> + * \var VideoRecording
>   * The stream is intended to capture video for the purpose of recording or
>   * streaming. The video stream may produce a high frame rate and may be
>   * enhanced with video stabilization.
> - * \var StreamUsage::Viewfinder
> + * \var Viewfinder
>   * The stream is intended to capture video for the purpose of display on the
> - * local screen. The StreamUsage includes the desired resolution. Trade-offs
> - * between quality and usage of system resources are acceptable.
> + * local screen. Trade-offs between quality and usage of system resources are
> + * acceptable.
>   */
>
>  /**
> - * \fn StreamUsage::role()
> - * \brief Retrieve the stream role
> - * \return The stream role
> + * \typedef StreamRoles
> + * \brief A vector of StreamRole
>   */
>
> -/**
> - * \fn StreamUsage::size()
> - * \brief Retrieve desired size
> - * \return The desired size
> - */
> -
> -/**
> - * \brief Create a stream usage
> - * \param[in] role Stream role
> - */
> -StreamUsage::StreamUsage(Role role)
> -	: role_(role)
> -{
> -}
> -
> -/**
> - * \brief Create a stream usage with a desired size
> - * \param[in] role Stream role
> - * \param[in] size The desired size
> - */
> -StreamUsage::StreamUsage(Role role, const Size &size)
> -	: role_(role), size_(size)
> -{
> -}
> -
>  /**
>   * \class Stream
>   * \brief Video stream for a camera
> @@ -148,39 +118,6 @@ StreamUsage::StreamUsage(Role role, const Size &size)
>   * optimal stream for the task.
>   */
>
> -/**
> - * \class Stream::StillCapture
> - * \brief Describe a still capture usage
> - */
> -Stream::StillCapture::StillCapture()
> -	: StreamUsage(Role::StillCapture)
> -{
> -}
> -
> -/**
> - * \class Stream::VideoRecording
> - * \brief Describe a video recording usage
> - */
> -Stream::VideoRecording::VideoRecording()
> -	: StreamUsage(Role::VideoRecording)
> -{
> -}
> -
> -/**
> - * \class Stream::Viewfinder
> - * \brief Describe a viewfinder usage
> - */
> -
> -/**
> - * \brief Create a viewfinder usage with a desired dimension
> - * \param[in] width The desired viewfinder width
> - * \param[in] height The desired viewfinder height
> - */
> -Stream::Viewfinder::Viewfinder(int width, int height)
> -	: StreamUsage(Role::Viewfinder, Size(width, height))
> -{
> -}
> -
>  /**
>   * \brief Construct a stream with default parameters
>   */
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index c91b82727ec6..a984aaca764f 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -97,7 +97,7 @@ int MainWindow::startCapture()
>  {
>  	int ret;
>
> -	config_ = camera_->generateConfiguration({ Stream::VideoRecording() });
> +	config_ = camera_->generateConfiguration({ StreamRole::VideoRecording });
>  	Stream *stream = config_.front();
>  	ret = camera_->configure(config_);
>  	if (ret < 0) {
> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> index bc3a4d6cb9f2..e7e6438203b9 100644
> --- a/test/camera/capture.cpp
> +++ b/test/camera/capture.cpp
> @@ -43,7 +43,7 @@ protected:
>  	int run()
>  	{
>  		CameraConfiguration config =
> -			camera_->generateConfiguration({ Stream::VideoRecording() });
> +			camera_->generateConfiguration({ StreamRole::VideoRecording });
>  		Stream *stream = config.front();
>  		StreamConfiguration *cfg = &config[stream];
>
> diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp
> index 340b5f58f04c..8c4a03db498a 100644
> --- a/test/camera/configuration_default.cpp
> +++ b/test/camera/configuration_default.cpp
> @@ -21,7 +21,7 @@ protected:
>  		CameraConfiguration config;
>
>  		/* Test asking for configuration for a video stream. */
> -		config = camera_->generateConfiguration({ Stream::VideoRecording() });
> +		config = camera_->generateConfiguration({ StreamRole::VideoRecording });
>  		if (!config.isValid()) {
>  			cout << "Default configuration invalid" << endl;
>  			return TestFail;
> @@ -29,11 +29,11 @@ protected:
>
>  		/*
>  		 * Test that asking for configuration for an empty array of
> -		 * stream usages returns an empty list of configurations.
> +		 * stream roles returns an empty list of configurations.
>  		 */
>  		config = camera_->generateConfiguration({});
>  		if (config.isValid()) {
> -			cout << "Failed to retrieve configuration for empty usage list"
> +			cout << "Failed to retrieve configuration for empty roles list"
>  			     << endl;
>  			return TestFail;
>  		}
> diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp
> index 24d5ca6690b7..76d8bc3e40a4 100644
> --- a/test/camera/configuration_set.cpp
> +++ b/test/camera/configuration_set.cpp
> @@ -19,7 +19,7 @@ protected:
>  	int run()
>  	{
>  		CameraConfiguration config =
> -			camera_->generateConfiguration({ Stream::VideoRecording() });
> +			camera_->generateConfiguration({ StreamRole::VideoRecording });
>  		StreamConfiguration *cfg = &config[config.front()];
>
>  		if (!config.isValid()) {
> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
> index bd2e61ff2939..7a74cd85a37a 100644
> --- a/test/camera/statemachine.cpp
> +++ b/test/camera/statemachine.cpp
> @@ -235,7 +235,7 @@ protected:
>
>  	int run()
>  	{
> -		defconf_ = camera_->generateConfiguration({ Stream::VideoRecording() });
> +		defconf_ = camera_->generateConfiguration({ StreamRole::VideoRecording });
>
>  		if (testAvailable() != TestPass) {
>  			cout << "State machine in Available state failed" << 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/081153b8/attachment.sig>


More information about the libcamera-devel mailing list