[libcamera-devel,04/20] libcamera: pipeline: simple: converter: Use StreamConfiguration for input configuration
diff mbox series

Message ID 20210131224702.8838-5-laurent.pinchart@ideasonboard.com
State Accepted
Delegated to: Laurent Pinchart
Headers show
Series
  • [libcamera-devel,01/20] libcamera: pipeline: simple: Manage converter with std::unique_ptr<>
Related show

Commit Message

Laurent Pinchart Jan. 31, 2021, 10:46 p.m. UTC 
Group the configuration parameters for the converter input in a
StreamConfiguration instance. This makes the configure() function
signature cleaner, and will allow passing additional parameters (such as
stride and buffer count).

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/pipeline/simple/converter.cpp | 12 +++++++-----
 src/libcamera/pipeline/simple/converter.h   |  2 +-
 src/libcamera/pipeline/simple/simple.cpp    |  7 +++++--
 3 files changed, 13 insertions(+), 8 deletions(-)

Comments

Paul Elder Feb. 17, 2021, 10:05 a.m. UTC | #1 
Hi Laurent,

On Mon, Feb 01, 2021 at 12:46:46AM +0200, Laurent Pinchart wrote:
> Group the configuration parameters for the converter input in a
> StreamConfiguration instance. This makes the configure() function
> signature cleaner, and will allow passing additional parameters (such as
> stride and buffer count).
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/libcamera/pipeline/simple/converter.cpp | 12 +++++++-----
>  src/libcamera/pipeline/simple/converter.h   |  2 +-
>  src/libcamera/pipeline/simple/simple.cpp    |  7 +++++--
>  3 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
> index 8f54caaca983..6f4e41db2e00 100644
> --- a/src/libcamera/pipeline/simple/converter.cpp
> +++ b/src/libcamera/pipeline/simple/converter.cpp
> @@ -148,15 +148,17 @@ SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat,
>  	return std::make_tuple(format.planes[0].bpl, format.planes[0].size);
>  }
>  
> -int SimpleConverter::configure(PixelFormat inputFormat, const Size &inputSize,
> +int SimpleConverter::configure(const StreamConfiguration &inputCfg,
>  			       const StreamConfiguration &outputCfg)
>  {
> -	V4L2DeviceFormat format;
>  	int ret;
>  
> -	V4L2PixelFormat videoFormat = m2m_->output()->toV4L2PixelFormat(inputFormat);
> +	V4L2PixelFormat videoFormat =
> +		m2m_->output()->toV4L2PixelFormat(inputCfg.pixelFormat);
> +
> +	V4L2DeviceFormat format;
>  	format.fourcc = videoFormat;
> -	format.size = inputSize;
> +	format.size = inputCfg.size;
>  
>  	ret = m2m_->output()->setFormat(&format);
>  	if (ret < 0) {
> @@ -165,7 +167,7 @@ int SimpleConverter::configure(PixelFormat inputFormat, const Size &inputSize,
>  		return ret;
>  	}
>  
> -	if (format.fourcc != videoFormat || format.size != inputSize) {
> +	if (format.fourcc != videoFormat || format.size != inputCfg.size) {
>  		LOG(SimplePipeline, Error)
>  			<< "Input format not supported";
>  		return -EINVAL;
> diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h
> index 07a632abd8f6..47a056e582d6 100644
> --- a/src/libcamera/pipeline/simple/converter.h
> +++ b/src/libcamera/pipeline/simple/converter.h
> @@ -38,7 +38,7 @@ public:
>  	std::tuple<unsigned int, unsigned int>
>  	strideAndFrameSize(const PixelFormat &pixelFormat, const Size &size);
>  
> -	int configure(PixelFormat inputFormat, const Size &inputSize,
> +	int configure(const StreamConfiguration &inputCfg,
>  		      const StreamConfiguration &outputCfg);
>  	int exportBuffers(unsigned int count,
>  			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 4e3127814b89..0a53fa934261 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -603,8 +603,11 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
>  	useConverter_ = config->needConversion();
>  
>  	if (useConverter_) {
> -		ret = converter_->configure(pipeConfig.pixelFormat,
> -					    pipeConfig.captureSize, cfg);
> +		StreamConfiguration inputCfg;
> +		inputCfg.pixelFormat = pipeConfig.pixelFormat;
> +		inputCfg.size = pipeConfig.captureSize;
> +
> +		ret = converter_->configure(inputCfg, cfg);
>  		if (ret < 0) {
>  			LOG(SimplePipeline, Error)
>  				<< "Unable to configure converter";
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Kieran Bingham Feb. 19, 2021, 5:14 p.m. UTC | #2 
On 31/01/2021 22:46, Laurent Pinchart wrote:
> Group the configuration parameters for the converter input in a
> StreamConfiguration instance. This makes the configure() function
> signature cleaner, and will allow passing additional parameters (such as
> stride and buffer count).
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Sounds good, converts one stream type to another ;-)

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  src/libcamera/pipeline/simple/converter.cpp | 12 +++++++-----
>  src/libcamera/pipeline/simple/converter.h   |  2 +-
>  src/libcamera/pipeline/simple/simple.cpp    |  7 +++++--
>  3 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
> index 8f54caaca983..6f4e41db2e00 100644
> --- a/src/libcamera/pipeline/simple/converter.cpp
> +++ b/src/libcamera/pipeline/simple/converter.cpp
> @@ -148,15 +148,17 @@ SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat,
>  	return std::make_tuple(format.planes[0].bpl, format.planes[0].size);
>  }
>  
> -int SimpleConverter::configure(PixelFormat inputFormat, const Size &inputSize,
> +int SimpleConverter::configure(const StreamConfiguration &inputCfg,
>  			       const StreamConfiguration &outputCfg)
>  {
> -	V4L2DeviceFormat format;
>  	int ret;
>  
> -	V4L2PixelFormat videoFormat = m2m_->output()->toV4L2PixelFormat(inputFormat);
> +	V4L2PixelFormat videoFormat =
> +		m2m_->output()->toV4L2PixelFormat(inputCfg.pixelFormat);
> +
> +	V4L2DeviceFormat format;
>  	format.fourcc = videoFormat;
> -	format.size = inputSize;
> +	format.size = inputCfg.size;
>  
>  	ret = m2m_->output()->setFormat(&format);
>  	if (ret < 0) {
> @@ -165,7 +167,7 @@ int SimpleConverter::configure(PixelFormat inputFormat, const Size &inputSize,
>  		return ret;
>  	}
>  
> -	if (format.fourcc != videoFormat || format.size != inputSize) {
> +	if (format.fourcc != videoFormat || format.size != inputCfg.size) {
>  		LOG(SimplePipeline, Error)
>  			<< "Input format not supported";
>  		return -EINVAL;
> diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h
> index 07a632abd8f6..47a056e582d6 100644
> --- a/src/libcamera/pipeline/simple/converter.h
> +++ b/src/libcamera/pipeline/simple/converter.h
> @@ -38,7 +38,7 @@ public:
>  	std::tuple<unsigned int, unsigned int>
>  	strideAndFrameSize(const PixelFormat &pixelFormat, const Size &size);
>  
> -	int configure(PixelFormat inputFormat, const Size &inputSize,
> +	int configure(const StreamConfiguration &inputCfg,
>  		      const StreamConfiguration &outputCfg);
>  	int exportBuffers(unsigned int count,
>  			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 4e3127814b89..0a53fa934261 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -603,8 +603,11 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
>  	useConverter_ = config->needConversion();
>  
>  	if (useConverter_) {
> -		ret = converter_->configure(pipeConfig.pixelFormat,
> -					    pipeConfig.captureSize, cfg);
> +		StreamConfiguration inputCfg;
> +		inputCfg.pixelFormat = pipeConfig.pixelFormat;
> +		inputCfg.size = pipeConfig.captureSize;
> +
> +		ret = converter_->configure(inputCfg, cfg);
>  		if (ret < 0) {
>  			LOG(SimplePipeline, Error)
>  				<< "Unable to configure converter";
>

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
index 8f54caaca983..6f4e41db2e00 100644
--- a/src/libcamera/pipeline/simple/converter.cpp
+++ b/src/libcamera/pipeline/simple/converter.cpp
@@ -148,15 +148,17 @@  SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat,
 	return std::make_tuple(format.planes[0].bpl, format.planes[0].size);
 }
 
-int SimpleConverter::configure(PixelFormat inputFormat, const Size &inputSize,
+int SimpleConverter::configure(const StreamConfiguration &inputCfg,
 			       const StreamConfiguration &outputCfg)
 {
-	V4L2DeviceFormat format;
 	int ret;
 
-	V4L2PixelFormat videoFormat = m2m_->output()->toV4L2PixelFormat(inputFormat);
+	V4L2PixelFormat videoFormat =
+		m2m_->output()->toV4L2PixelFormat(inputCfg.pixelFormat);
+
+	V4L2DeviceFormat format;
 	format.fourcc = videoFormat;
-	format.size = inputSize;
+	format.size = inputCfg.size;
 
 	ret = m2m_->output()->setFormat(&format);
 	if (ret < 0) {
@@ -165,7 +167,7 @@  int SimpleConverter::configure(PixelFormat inputFormat, const Size &inputSize,
 		return ret;
 	}
 
-	if (format.fourcc != videoFormat || format.size != inputSize) {
+	if (format.fourcc != videoFormat || format.size != inputCfg.size) {
 		LOG(SimplePipeline, Error)
 			<< "Input format not supported";
 		return -EINVAL;
diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h
index 07a632abd8f6..47a056e582d6 100644
--- a/src/libcamera/pipeline/simple/converter.h
+++ b/src/libcamera/pipeline/simple/converter.h
@@ -38,7 +38,7 @@  public:
 	std::tuple<unsigned int, unsigned int>
 	strideAndFrameSize(const PixelFormat &pixelFormat, const Size &size);
 
-	int configure(PixelFormat inputFormat, const Size &inputSize,
+	int configure(const StreamConfiguration &inputCfg,
 		      const StreamConfiguration &outputCfg);
 	int exportBuffers(unsigned int count,
 			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 4e3127814b89..0a53fa934261 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -603,8 +603,11 @@  int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
 	useConverter_ = config->needConversion();
 
 	if (useConverter_) {
-		ret = converter_->configure(pipeConfig.pixelFormat,
-					    pipeConfig.captureSize, cfg);
+		StreamConfiguration inputCfg;
+		inputCfg.pixelFormat = pipeConfig.pixelFormat;
+		inputCfg.size = pipeConfig.captureSize;
+
+		ret = converter_->configure(inputCfg, cfg);
 		if (ret < 0) {
 			LOG(SimplePipeline, Error)
 				<< "Unable to configure converter";