[libcamera-devel,05/20] libcamera: pipeline: simple: converter: Configure input stride
diff mbox series

Message ID 20210131224702.8838-6-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 
Use the stride of the video capture device to configure the converter
input. This ensures that no stride mismatch occurs inadvertently along
the pipeline.

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

Comments

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

On Mon, Feb 01, 2021 at 12:46:47AM +0200, Laurent Pinchart wrote:
> Use the stride of the video capture device to configure the converter
> input. This ensures that no stride mismatch occurs inadvertently along
> the pipeline.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

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

> ---
>  src/libcamera/pipeline/simple/converter.cpp | 5 ++++-
>  src/libcamera/pipeline/simple/simple.cpp    | 1 +
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
> index 6f4e41db2e00..550b2bcfb001 100644
> --- a/src/libcamera/pipeline/simple/converter.cpp
> +++ b/src/libcamera/pipeline/simple/converter.cpp
> @@ -159,6 +159,8 @@ int SimpleConverter::configure(const StreamConfiguration &inputCfg,
>  	V4L2DeviceFormat format;
>  	format.fourcc = videoFormat;
>  	format.size = inputCfg.size;
> +	format.planesCount = 1;
> +	format.planes[0].bpl = inputCfg.stride;
>  
>  	ret = m2m_->output()->setFormat(&format);
>  	if (ret < 0) {
> @@ -167,7 +169,8 @@ int SimpleConverter::configure(const StreamConfiguration &inputCfg,
>  		return ret;
>  	}
>  
> -	if (format.fourcc != videoFormat || format.size != inputCfg.size) {
> +	if (format.fourcc != videoFormat || format.size != inputCfg.size ||
> +	    format.planes[0].bpl != inputCfg.stride) {
>  		LOG(SimplePipeline, Error)
>  			<< "Input format not supported";
>  		return -EINVAL;
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 0a53fa934261..1ed67bcec490 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -606,6 +606,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
>  		StreamConfiguration inputCfg;
>  		inputCfg.pixelFormat = pipeConfig.pixelFormat;
>  		inputCfg.size = pipeConfig.captureSize;
> +		inputCfg.stride = captureFormat.planes[0].bpl;
>  
>  		ret = converter_->configure(inputCfg, cfg);
>  		if (ret < 0) {
> -- 
> 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:19 p.m. UTC | #2 
On 31/01/2021 22:46, Laurent Pinchart wrote:
> Use the stride of the video capture device to configure the converter
> input. This ensures that no stride mismatch occurs inadvertently along
> the pipeline.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/pipeline/simple/converter.cpp | 5 ++++-
>  src/libcamera/pipeline/simple/simple.cpp    | 1 +
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
> index 6f4e41db2e00..550b2bcfb001 100644
> --- a/src/libcamera/pipeline/simple/converter.cpp
> +++ b/src/libcamera/pipeline/simple/converter.cpp
> @@ -159,6 +159,8 @@ int SimpleConverter::configure(const StreamConfiguration &inputCfg,
>  	V4L2DeviceFormat format;
>  	format.fourcc = videoFormat;
>  	format.size = inputCfg.size;
> +	format.planesCount = 1;
> +	format.planes[0].bpl = inputCfg.stride;

Are we lacking multiple plane description in the StreamConfiguration or
such ?

I guess it doesn't matter if this only supports a single planar format
anyway, given the hardcoded planesCount.

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

>  
>  	ret = m2m_->output()->setFormat(&format);
>  	if (ret < 0) {
> @@ -167,7 +169,8 @@ int SimpleConverter::configure(const StreamConfiguration &inputCfg,
>  		return ret;
>  	}
>  
> -	if (format.fourcc != videoFormat || format.size != inputCfg.size) {
> +	if (format.fourcc != videoFormat || format.size != inputCfg.size ||
> +	    format.planes[0].bpl != inputCfg.stride) {
>  		LOG(SimplePipeline, Error)
>  			<< "Input format not supported";
>  		return -EINVAL;
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 0a53fa934261..1ed67bcec490 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -606,6 +606,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
>  		StreamConfiguration inputCfg;
>  		inputCfg.pixelFormat = pipeConfig.pixelFormat;
>  		inputCfg.size = pipeConfig.captureSize;
> +		inputCfg.stride = captureFormat.planes[0].bpl;
>  
>  		ret = converter_->configure(inputCfg, cfg);
>  		if (ret < 0) {
>
Laurent Pinchart Feb. 21, 2021, 1:06 p.m. UTC | #3 
Hi Kieran,

On Fri, Feb 19, 2021 at 05:19:14PM +0000, Kieran Bingham wrote:
> On 31/01/2021 22:46, Laurent Pinchart wrote:
> > Use the stride of the video capture device to configure the converter
> > input. This ensures that no stride mismatch occurs inadvertently along
> > the pipeline.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/libcamera/pipeline/simple/converter.cpp | 5 ++++-
> >  src/libcamera/pipeline/simple/simple.cpp    | 1 +
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
> > index 6f4e41db2e00..550b2bcfb001 100644
> > --- a/src/libcamera/pipeline/simple/converter.cpp
> > +++ b/src/libcamera/pipeline/simple/converter.cpp
> > @@ -159,6 +159,8 @@ int SimpleConverter::configure(const StreamConfiguration &inputCfg,
> >  	V4L2DeviceFormat format;
> >  	format.fourcc = videoFormat;
> >  	format.size = inputCfg.size;
> > +	format.planesCount = 1;
> > +	format.planes[0].bpl = inputCfg.stride;
> 
> Are we lacking multiple plane description in the StreamConfiguration or
> such ?

Yes, we're not doing great when it comes to multi-planar formats. This
is something we need to fix.

> I guess it doesn't matter if this only supports a single planar format
> anyway, given the hardcoded planesCount.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> >  
> >  	ret = m2m_->output()->setFormat(&format);
> >  	if (ret < 0) {
> > @@ -167,7 +169,8 @@ int SimpleConverter::configure(const StreamConfiguration &inputCfg,
> >  		return ret;
> >  	}
> >  
> > -	if (format.fourcc != videoFormat || format.size != inputCfg.size) {
> > +	if (format.fourcc != videoFormat || format.size != inputCfg.size ||
> > +	    format.planes[0].bpl != inputCfg.stride) {
> >  		LOG(SimplePipeline, Error)
> >  			<< "Input format not supported";
> >  		return -EINVAL;
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index 0a53fa934261..1ed67bcec490 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -606,6 +606,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
> >  		StreamConfiguration inputCfg;
> >  		inputCfg.pixelFormat = pipeConfig.pixelFormat;
> >  		inputCfg.size = pipeConfig.captureSize;
> > +		inputCfg.stride = captureFormat.planes[0].bpl;
> >  
> >  		ret = converter_->configure(inputCfg, cfg);
> >  		if (ret < 0) {
> >

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
index 6f4e41db2e00..550b2bcfb001 100644
--- a/src/libcamera/pipeline/simple/converter.cpp
+++ b/src/libcamera/pipeline/simple/converter.cpp
@@ -159,6 +159,8 @@  int SimpleConverter::configure(const StreamConfiguration &inputCfg,
 	V4L2DeviceFormat format;
 	format.fourcc = videoFormat;
 	format.size = inputCfg.size;
+	format.planesCount = 1;
+	format.planes[0].bpl = inputCfg.stride;
 
 	ret = m2m_->output()->setFormat(&format);
 	if (ret < 0) {
@@ -167,7 +169,8 @@  int SimpleConverter::configure(const StreamConfiguration &inputCfg,
 		return ret;
 	}
 
-	if (format.fourcc != videoFormat || format.size != inputCfg.size) {
+	if (format.fourcc != videoFormat || format.size != inputCfg.size ||
+	    format.planes[0].bpl != inputCfg.stride) {
 		LOG(SimplePipeline, Error)
 			<< "Input format not supported";
 		return -EINVAL;
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 0a53fa934261..1ed67bcec490 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -606,6 +606,7 @@  int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
 		StreamConfiguration inputCfg;
 		inputCfg.pixelFormat = pipeConfig.pixelFormat;
 		inputCfg.size = pipeConfig.captureSize;
+		inputCfg.stride = captureFormat.planes[0].bpl;
 
 		ret = converter_->configure(inputCfg, cfg);
 		if (ret < 0) {