[7/7] libcamera: mali-c55: Record stride in rawConfig
diff mbox series

Message ID 20240613155949.1041061-8-dan.scally@ideasonboard.com
State Superseded
Headers show
Series
  • Miscellaneous Mali-C55 Pipeline Fixes
Related show

Commit Message

Daniel Scally June 13, 2024, 3:59 p.m. UTC
The DNGWriter class needs to know the stride of a RAW image for some
of its uses. Add stride to rawConfig in the mali-c55 pipeline handler
so that it's available to DNGWriter.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
 src/libcamera/pipeline/mali-c55/mali-c55.cpp | 3 +++
 1 file changed, 3 insertions(+)

Comments

Kieran Bingham June 13, 2024, 7:38 p.m. UTC | #1
Quoting Daniel Scally (2024-06-13 16:59:49)
> The DNGWriter class needs to know the stride of a RAW image for some
> of its uses. Add stride to rawConfig in the mali-c55 pipeline handler
> so that it's available to DNGWriter.
> 
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
>  src/libcamera/pipeline/mali-c55/mali-c55.cpp | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> index 9cd243a3..a9ea2e04 100644
> --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> @@ -382,6 +382,9 @@ CameraConfiguration::Status MaliC55CameraConfiguration::validate()
>  
>                 maxSize = rawSize;
>  
> +               const PixelFormatInfo &info = PixelFormatInfo::info(rawConfig->pixelFormat);
> +               rawConfig->stride = info.stride(rawConfig->size.width, 0, 64);
> +

Does the frameSize also get set? I would anticipate the two being
together.

We should add such tests to lc-compliance to make sure the
configurations are fully populated after validation.

But I  think this one stands...


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

>                 rawConfig->setStream(const_cast<Stream *>(&data_->frStream_));
>                 frPipeAvailable = false;
>         }
> -- 
> 2.30.2
>
Laurent Pinchart June 17, 2024, 5:15 a.m. UTC | #2
On Thu, Jun 13, 2024 at 08:38:15PM +0100, Kieran Bingham wrote:
> Quoting Daniel Scally (2024-06-13 16:59:49)
> > The DNGWriter class needs to know the stride of a RAW image for some
> > of its uses. Add stride to rawConfig in the mali-c55 pipeline handler
> > so that it's available to DNGWriter.

As mentioned by Kieran in the review of a previous patch, this may be
true, but it's hardly the reason.

> > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> > ---
> >  src/libcamera/pipeline/mali-c55/mali-c55.cpp | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> > index 9cd243a3..a9ea2e04 100644
> > --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> > +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
> > @@ -382,6 +382,9 @@ CameraConfiguration::Status MaliC55CameraConfiguration::validate()
> >  
> >                 maxSize = rawSize;
> >  
> > +               const PixelFormatInfo &info = PixelFormatInfo::info(rawConfig->pixelFormat);
> > +               rawConfig->stride = info.stride(rawConfig->size.width, 0, 64);
> > +
> 
> Does the frameSize also get set? I would anticipate the two being
> together.

This patch should set both.

> We should add such tests to lc-compliance to make sure the
> configurations are fully populated after validation.
> 
> But I  think this one stands...
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> >                 rawConfig->setStream(const_cast<Stream *>(&data_->frStream_));
> >                 frPipeAvailable = false;
> >         }

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
index 9cd243a3..a9ea2e04 100644
--- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp
+++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp
@@ -382,6 +382,9 @@  CameraConfiguration::Status MaliC55CameraConfiguration::validate()
 
 		maxSize = rawSize;
 
+		const PixelFormatInfo &info = PixelFormatInfo::info(rawConfig->pixelFormat);
+		rawConfig->stride = info.stride(rawConfig->size.width, 0, 64);
+
 		rawConfig->setStream(const_cast<Stream *>(&data_->frStream_));
 		frPipeAvailable = false;
 	}