[libcamera-devel,03/20] libcamera: pipeline: simple: converter: Group query functions together
diff mbox series

Message ID 20210131224702.8838-4-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
The SimpleConverter class has different sets of functions, related to
static queries, device configuration and runtime operation. Group the
query functions together. While at it, swap the arguments to the
strideAndFrameSize() function to match the order in which pixel format
and size are usually specified.

No functional change is included.

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

Comments

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

On Mon, Feb 01, 2021 at 12:46:45AM +0200, Laurent Pinchart wrote:
> The SimpleConverter class has different sets of functions, related to
> static queries, device configuration and runtime operation. Group the
> query functions together. While at it, swap the arguments to the
> strideAndFrameSize() function to match the order in which pixel format
> and size are usually specified.
> 
> No functional change is included.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

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

> ---
>  src/libcamera/pipeline/simple/converter.cpp | 30 ++++++++++-----------
>  src/libcamera/pipeline/simple/converter.h   |  5 ++--
>  src/libcamera/pipeline/simple/simple.cpp    |  2 +-
>  3 files changed, 18 insertions(+), 19 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
> index 87d15c781ed8..8f54caaca983 100644
> --- a/src/libcamera/pipeline/simple/converter.cpp
> +++ b/src/libcamera/pipeline/simple/converter.cpp
> @@ -133,6 +133,21 @@ SizeRange SimpleConverter::sizes(const Size &input)
>  	return sizes;
>  }
>  
> +std::tuple<unsigned int, unsigned int>
> +SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat,
> +				    const Size &size)
> +{
> +	V4L2DeviceFormat format;
> +	format.fourcc = m2m_->capture()->toV4L2PixelFormat(pixelFormat);
> +	format.size = size;
> +
> +	int ret = m2m_->capture()->tryFormat(&format);
> +	if (ret < 0)
> +		return std::make_tuple(0, 0);
> +
> +	return std::make_tuple(format.planes[0].bpl, format.planes[0].size);
> +}
> +
>  int SimpleConverter::configure(PixelFormat inputFormat, const Size &inputSize,
>  			       const StreamConfiguration &outputCfg)
>  {
> @@ -254,19 +269,4 @@ void SimpleConverter::outputBufferReady(FrameBuffer *buffer)
>  	}
>  }
>  
> -std::tuple<unsigned int, unsigned int>
> -SimpleConverter::strideAndFrameSize(const Size &size,
> -				    const PixelFormat &pixelFormat)
> -{
> -	V4L2DeviceFormat format;
> -	format.fourcc = m2m_->capture()->toV4L2PixelFormat(pixelFormat);
> -	format.size = size;
> -
> -	int ret = m2m_->capture()->tryFormat(&format);
> -	if (ret < 0)
> -		return std::make_tuple(0, 0);
> -
> -	return std::make_tuple(format.planes[0].bpl, format.planes[0].size);
> -}
> -
>  } /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h
> index 06d66f8caba7..07a632abd8f6 100644
> --- a/src/libcamera/pipeline/simple/converter.h
> +++ b/src/libcamera/pipeline/simple/converter.h
> @@ -35,6 +35,8 @@ public:
>  
>  	std::vector<PixelFormat> formats(PixelFormat input);
>  	SizeRange sizes(const Size &input);
> +	std::tuple<unsigned int, unsigned int>
> +	strideAndFrameSize(const PixelFormat &pixelFormat, const Size &size);
>  
>  	int configure(PixelFormat inputFormat, const Size &inputSize,
>  		      const StreamConfiguration &outputCfg);
> @@ -46,9 +48,6 @@ public:
>  
>  	int queueBuffers(FrameBuffer *input, FrameBuffer *output);
>  
> -	std::tuple<unsigned int, unsigned int>
> -	strideAndFrameSize(const Size &size, const PixelFormat &pixelFormat);
> -
>  	Signal<FrameBuffer *, FrameBuffer *> bufferReady;
>  
>  private:
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index a97b8442d9a4..4e3127814b89 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -494,7 +494,7 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()
>  	SimpleConverter *converter = pipe->converter();
>  
>  	std::tie(cfg.stride, cfg.frameSize) =
> -		converter->strideAndFrameSize(cfg.size, cfg.pixelFormat);
> +		converter->strideAndFrameSize(cfg.pixelFormat, cfg.size);
>  	if (cfg.stride == 0)
>  		return Invalid;
>  
> -- 
> 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:13 p.m. UTC | #2
On 31/01/2021 22:46, Laurent Pinchart wrote:
> The SimpleConverter class has different sets of functions, related to
> static queries, device configuration and runtime operation. Group the
> query functions together. While at it, swap the arguments to the
> strideAndFrameSize() function to match the order in which pixel format
> and size are usually specified.
> 
> No functional change is included.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/pipeline/simple/converter.cpp | 30 ++++++++++-----------
>  src/libcamera/pipeline/simple/converter.h   |  5 ++--
>  src/libcamera/pipeline/simple/simple.cpp    |  2 +-
>  3 files changed, 18 insertions(+), 19 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
> index 87d15c781ed8..8f54caaca983 100644
> --- a/src/libcamera/pipeline/simple/converter.cpp
> +++ b/src/libcamera/pipeline/simple/converter.cpp
> @@ -133,6 +133,21 @@ SizeRange SimpleConverter::sizes(const Size &input)
>  	return sizes;
>  }
>  
> +std::tuple<unsigned int, unsigned int>
> +SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat,
> +				    const Size &size)
> +{
> +	V4L2DeviceFormat format;
> +	format.fourcc = m2m_->capture()->toV4L2PixelFormat(pixelFormat);
> +	format.size = size;
> +
> +	int ret = m2m_->capture()->tryFormat(&format);
> +	if (ret < 0)
> +		return std::make_tuple(0, 0);
> +
> +	return std::make_tuple(format.planes[0].bpl, format.planes[0].size);
> +}
> +
>  int SimpleConverter::configure(PixelFormat inputFormat, const Size &inputSize,
>  			       const StreamConfiguration &outputCfg)
>  {
> @@ -254,19 +269,4 @@ void SimpleConverter::outputBufferReady(FrameBuffer *buffer)
>  	}
>  }
>  
> -std::tuple<unsigned int, unsigned int>
> -SimpleConverter::strideAndFrameSize(const Size &size,
> -				    const PixelFormat &pixelFormat)
> -{
> -	V4L2DeviceFormat format;
> -	format.fourcc = m2m_->capture()->toV4L2PixelFormat(pixelFormat);
> -	format.size = size;
> -
> -	int ret = m2m_->capture()->tryFormat(&format);
> -	if (ret < 0)
> -		return std::make_tuple(0, 0);
> -
> -	return std::make_tuple(format.planes[0].bpl, format.planes[0].size);
> -}
> -
>  } /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h
> index 06d66f8caba7..07a632abd8f6 100644
> --- a/src/libcamera/pipeline/simple/converter.h
> +++ b/src/libcamera/pipeline/simple/converter.h
> @@ -35,6 +35,8 @@ public:
>  
>  	std::vector<PixelFormat> formats(PixelFormat input);
>  	SizeRange sizes(const Size &input);

I might have put a new line here, because the return value and function
are indented the same, so it might be better to separate it from the
previous one... but it's white space so it doesn't matter much.

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

> +	std::tuple<unsigned int, unsigned int>
> +	strideAndFrameSize(const PixelFormat &pixelFormat, const Size &size);
>  
>  	int configure(PixelFormat inputFormat, const Size &inputSize,
>  		      const StreamConfiguration &outputCfg);
> @@ -46,9 +48,6 @@ public:
>  
>  	int queueBuffers(FrameBuffer *input, FrameBuffer *output);
>  
> -	std::tuple<unsigned int, unsigned int>
> -	strideAndFrameSize(const Size &size, const PixelFormat &pixelFormat);
> -
>  	Signal<FrameBuffer *, FrameBuffer *> bufferReady;
>  
>  private:
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index a97b8442d9a4..4e3127814b89 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -494,7 +494,7 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()
>  	SimpleConverter *converter = pipe->converter();
>  
>  	std::tie(cfg.stride, cfg.frameSize) =
> -		converter->strideAndFrameSize(cfg.size, cfg.pixelFormat);
> +		converter->strideAndFrameSize(cfg.pixelFormat, cfg.size);
>  	if (cfg.stride == 0)
>  		return Invalid;
>  
>

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
index 87d15c781ed8..8f54caaca983 100644
--- a/src/libcamera/pipeline/simple/converter.cpp
+++ b/src/libcamera/pipeline/simple/converter.cpp
@@ -133,6 +133,21 @@  SizeRange SimpleConverter::sizes(const Size &input)
 	return sizes;
 }
 
+std::tuple<unsigned int, unsigned int>
+SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat,
+				    const Size &size)
+{
+	V4L2DeviceFormat format;
+	format.fourcc = m2m_->capture()->toV4L2PixelFormat(pixelFormat);
+	format.size = size;
+
+	int ret = m2m_->capture()->tryFormat(&format);
+	if (ret < 0)
+		return std::make_tuple(0, 0);
+
+	return std::make_tuple(format.planes[0].bpl, format.planes[0].size);
+}
+
 int SimpleConverter::configure(PixelFormat inputFormat, const Size &inputSize,
 			       const StreamConfiguration &outputCfg)
 {
@@ -254,19 +269,4 @@  void SimpleConverter::outputBufferReady(FrameBuffer *buffer)
 	}
 }
 
-std::tuple<unsigned int, unsigned int>
-SimpleConverter::strideAndFrameSize(const Size &size,
-				    const PixelFormat &pixelFormat)
-{
-	V4L2DeviceFormat format;
-	format.fourcc = m2m_->capture()->toV4L2PixelFormat(pixelFormat);
-	format.size = size;
-
-	int ret = m2m_->capture()->tryFormat(&format);
-	if (ret < 0)
-		return std::make_tuple(0, 0);
-
-	return std::make_tuple(format.planes[0].bpl, format.planes[0].size);
-}
-
 } /* namespace libcamera */
diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h
index 06d66f8caba7..07a632abd8f6 100644
--- a/src/libcamera/pipeline/simple/converter.h
+++ b/src/libcamera/pipeline/simple/converter.h
@@ -35,6 +35,8 @@  public:
 
 	std::vector<PixelFormat> formats(PixelFormat input);
 	SizeRange sizes(const Size &input);
+	std::tuple<unsigned int, unsigned int>
+	strideAndFrameSize(const PixelFormat &pixelFormat, const Size &size);
 
 	int configure(PixelFormat inputFormat, const Size &inputSize,
 		      const StreamConfiguration &outputCfg);
@@ -46,9 +48,6 @@  public:
 
 	int queueBuffers(FrameBuffer *input, FrameBuffer *output);
 
-	std::tuple<unsigned int, unsigned int>
-	strideAndFrameSize(const Size &size, const PixelFormat &pixelFormat);
-
 	Signal<FrameBuffer *, FrameBuffer *> bufferReady;
 
 private:
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index a97b8442d9a4..4e3127814b89 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -494,7 +494,7 @@  CameraConfiguration::Status SimpleCameraConfiguration::validate()
 	SimpleConverter *converter = pipe->converter();
 
 	std::tie(cfg.stride, cfg.frameSize) =
-		converter->strideAndFrameSize(cfg.size, cfg.pixelFormat);
+		converter->strideAndFrameSize(cfg.pixelFormat, cfg.size);
 	if (cfg.stride == 0)
 		return Invalid;