[libcamera-devel,v7,11/13] libcamera: ipu3: Set stream configuration

Message ID 20190402171309.6447-12-jacopo@jmondi.org
State Superseded
Headers show
Series
  • libcamera: ipu3: Add ImgU support
Related show

Commit Message

Jacopo Mondi April 2, 2019, 5:13 p.m. UTC
Use the cached sensor maximum resolution and the pixel format
generated by the ImgU output devices as default stream configuration.

While at it, replace the hardcoded numerical value for the number of
buffers with a named constexpr.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/pipeline/ipu3/ipu3.cpp | 34 ++++++++++++----------------
 1 file changed, 15 insertions(+), 19 deletions(-)

Comments

Laurent Pinchart April 2, 2019, 5:40 p.m. UTC | #1
Hi Jacopo,

Thank you for the patch.

On Tue, Apr 02, 2019 at 07:13:07PM +0200, Jacopo Mondi wrote:
> Use the cached sensor maximum resolution and the pixel format
> generated by the ImgU output devices as default stream configuration.
> 
> While at it, replace the hardcoded numerical value for the number of
> buffers with a named constexpr.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 34 ++++++++++++----------------
>  1 file changed, 15 insertions(+), 19 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index f6d08af906da..e4f8f90e2302 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -183,6 +183,8 @@ private:
>  		Stream stream_;
>  	};
>  
> +	static constexpr unsigned int IPU3_BUFFER_COUNT = 4;
> +
>  	IPU3CameraData *cameraData(const Camera *camera)
>  	{
>  		return static_cast<IPU3CameraData *>(
> @@ -215,26 +217,20 @@ std::map<Stream *, StreamConfiguration>
>  PipelineHandlerIPU3::streamConfiguration(Camera *camera,
>  					 std::set<Stream *> &streams)
>  {
> -	IPU3CameraData *data = cameraData(camera);
>  	std::map<Stream *, StreamConfiguration> configs;
> -	V4L2SubdeviceFormat format = {};
> -
> -	/*
> -	 * FIXME: As of now, return the image format reported by the sensor.
> -	 * In future good defaults should be provided for each stream.
> -	 */
> -	if (data->cio2_.sensor_->getFormat(0, &format)) {
> -		LOG(IPU3, Error) << "Failed to create stream configurations";
> -		return configs;
> -	}
> -
> -	StreamConfiguration config = {};
> -	config.width = format.width;
> -	config.height = format.height;
> -	config.pixelFormat = V4L2_PIX_FMT_IPU3_SGRBG10;
> -	config.bufferCount = 4;
> -
> -	configs[&data->stream_] = config;
> +	IPU3CameraData *data = cameraData(camera);
> +	StreamConfiguration *config = &configs[&data->stream_];
> +	Size *maxSize = &data->.cio2_.maxSize_;
> +
> +	config->width = maxSize->width;
> +	config->height = maxSize->height;
> +	config->pixelFormat = V4L2_PIX_FMT_NV12;
> +	config->bufferCount = IPU3_BUFFER_COUNT;
> +
> +	LOG(IPU3, Debug)
> +		<< "Stream format set to " << config->width << "x"
> +		<< config->height << "-0x" << std::hex << std::setfill('0')
> +		<< std::setw(8) << config->pixelFormat;
>  
>  	return configs;
>  }
Niklas Söderlund April 2, 2019, 6:27 p.m. UTC | #2
Hi Jacopo,

Thanks for your work.

On 2019-04-02 19:13:07 +0200, Jacopo Mondi wrote:
> Use the cached sensor maximum resolution and the pixel format
> generated by the ImgU output devices as default stream configuration.
> 
> While at it, replace the hardcoded numerical value for the number of
> buffers with a named constexpr.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 34 ++++++++++++----------------
>  1 file changed, 15 insertions(+), 19 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index f6d08af906da..e4f8f90e2302 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -183,6 +183,8 @@ private:
>  		Stream stream_;
>  	};
>  
> +	static constexpr unsigned int IPU3_BUFFER_COUNT = 4;
> +
>  	IPU3CameraData *cameraData(const Camera *camera)
>  	{
>  		return static_cast<IPU3CameraData *>(
> @@ -215,26 +217,20 @@ std::map<Stream *, StreamConfiguration>
>  PipelineHandlerIPU3::streamConfiguration(Camera *camera,
>  					 std::set<Stream *> &streams)
>  {
> -	IPU3CameraData *data = cameraData(camera);
>  	std::map<Stream *, StreamConfiguration> configs;
> -	V4L2SubdeviceFormat format = {};
> -
> -	/*
> -	 * FIXME: As of now, return the image format reported by the sensor.
> -	 * In future good defaults should be provided for each stream.
> -	 */
> -	if (data->cio2_.sensor_->getFormat(0, &format)) {
> -		LOG(IPU3, Error) << "Failed to create stream configurations";
> -		return configs;
> -	}
> -
> -	StreamConfiguration config = {};
> -	config.width = format.width;
> -	config.height = format.height;
> -	config.pixelFormat = V4L2_PIX_FMT_IPU3_SGRBG10;
> -	config.bufferCount = 4;
> -
> -	configs[&data->stream_] = config;
> +	IPU3CameraData *data = cameraData(camera);
> +	StreamConfiguration *config = &configs[&data->stream_];
> +	Size *maxSize = &data->.cio2_.maxSize_;
> +
> +	config->width = maxSize->width;
> +	config->height = maxSize->height;
> +	config->pixelFormat = V4L2_PIX_FMT_NV12;
> +	config->bufferCount = IPU3_BUFFER_COUNT;
> +
> +	LOG(IPU3, Debug)
> +		<< "Stream format set to " << config->width << "x"
> +		<< config->height << "-0x" << std::hex << std::setfill('0')
> +		<< std::setw(8) << config->pixelFormat;
>  
>  	return configs;
>  }
> -- 
> 2.21.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch

diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index f6d08af906da..e4f8f90e2302 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -183,6 +183,8 @@  private:
 		Stream stream_;
 	};
 
+	static constexpr unsigned int IPU3_BUFFER_COUNT = 4;
+
 	IPU3CameraData *cameraData(const Camera *camera)
 	{
 		return static_cast<IPU3CameraData *>(
@@ -215,26 +217,20 @@  std::map<Stream *, StreamConfiguration>
 PipelineHandlerIPU3::streamConfiguration(Camera *camera,
 					 std::set<Stream *> &streams)
 {
-	IPU3CameraData *data = cameraData(camera);
 	std::map<Stream *, StreamConfiguration> configs;
-	V4L2SubdeviceFormat format = {};
-
-	/*
-	 * FIXME: As of now, return the image format reported by the sensor.
-	 * In future good defaults should be provided for each stream.
-	 */
-	if (data->cio2_.sensor_->getFormat(0, &format)) {
-		LOG(IPU3, Error) << "Failed to create stream configurations";
-		return configs;
-	}
-
-	StreamConfiguration config = {};
-	config.width = format.width;
-	config.height = format.height;
-	config.pixelFormat = V4L2_PIX_FMT_IPU3_SGRBG10;
-	config.bufferCount = 4;
-
-	configs[&data->stream_] = config;
+	IPU3CameraData *data = cameraData(camera);
+	StreamConfiguration *config = &configs[&data->stream_];
+	Size *maxSize = &data->.cio2_.maxSize_;
+
+	config->width = maxSize->width;
+	config->height = maxSize->height;
+	config->pixelFormat = V4L2_PIX_FMT_NV12;
+	config->bufferCount = IPU3_BUFFER_COUNT;
+
+	LOG(IPU3, Debug)
+		<< "Stream format set to " << config->width << "x"
+		<< config->height << "-0x" << std::hex << std::setfill('0')
+		<< std::setw(8) << config->pixelFormat;
 
 	return configs;
 }