[libcamera-devel] pipeline: raspberrypi: Fix error in validateColorSpaces
diff mbox series

Message ID 20230213091934.2356-1-david.plowman@raspberrypi.com
State Accepted
Commit 02f0492c55f293b6a00ee702cee51b5f1b1c2e50
Headers show
Series
  • [libcamera-devel] pipeline: raspberrypi: Fix error in validateColorSpaces
Related show

Commit Message

David Plowman Feb. 13, 2023, 9:19 a.m. UTC
This fixes a small error in the validateColorSpaces method where the
colour space was not being set to Raw if it had an "unset" (null
option) value.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Umang Jain Feb. 13, 2023, 9:40 a.m. UTC | #1
Hi David,

Thank you for the patch

On 2/13/23 2:49 PM, David Plowman via libcamera-devel wrote:
> This fixes a small error in the validateColorSpaces method where the
> colour space was not being set to Raw if it had an "unset" (null
> option) value.
>
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>

LGTM,

Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>

> ---
>   src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index c0dd9551..84120954 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -449,10 +449,9 @@ CameraConfiguration::Status RPiCameraConfiguration::validateColorSpaces([[maybe_
>   		/* First fix up raw streams to have the "raw" colour space. */
>   		if (isRaw(cfg.pixelFormat)) {
>   			/* If there was no value here, that doesn't count as "adjusted". */
> -			if (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw) {
> +			if (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)
>   				status = Adjusted;
> -				cfg.colorSpace = ColorSpace::Raw;
> -			}
> +			cfg.colorSpace = ColorSpace::Raw;
>   			continue;
>   		}
>
Laurent Pinchart Feb. 13, 2023, 10:12 a.m. UTC | #2
Hi David,

Thank you for the patch.

On Mon, Feb 13, 2023 at 09:19:34AM +0000, David Plowman via libcamera-devel wrote:
> This fixes a small error in the validateColorSpaces method where the
> colour space was not being set to Raw if it had an "unset" (null
> option) value.

Should this have a Fixes: tag ?

> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index c0dd9551..84120954 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -449,10 +449,9 @@ CameraConfiguration::Status RPiCameraConfiguration::validateColorSpaces([[maybe_
>  		/* First fix up raw streams to have the "raw" colour space. */
>  		if (isRaw(cfg.pixelFormat)) {
>  			/* If there was no value here, that doesn't count as "adjusted". */
> -			if (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw) {
> +			if (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)
>  				status = Adjusted;
> -				cfg.colorSpace = ColorSpace::Raw;
> -			}
> +			cfg.colorSpace = ColorSpace::Raw;
>  			continue;
>  		}
>
David Plowman Feb. 13, 2023, 10:13 a.m. UTC | #3
Hi Laurent

Ah yes, forgot about "fixes" tags. Let me try again...!

Thanks
David

On Mon, 13 Feb 2023 at 10:12, Laurent Pinchart <
laurent.pinchart@ideasonboard.com> wrote:

> Hi David,
>
> Thank you for the patch.
>
> On Mon, Feb 13, 2023 at 09:19:34AM +0000, David Plowman via
> libcamera-devel wrote:
> > This fixes a small error in the validateColorSpaces method where the
> > colour space was not being set to Raw if it had an "unset" (null
> > option) value.
>
> Should this have a Fixes: tag ?
>
> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > ---
> >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index c0dd9551..84120954 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -449,10 +449,9 @@ CameraConfiguration::Status
> RPiCameraConfiguration::validateColorSpaces([[maybe_
> >               /* First fix up raw streams to have the "raw" colour
> space. */
> >               if (isRaw(cfg.pixelFormat)) {
> >                       /* If there was no value here, that doesn't count
> as "adjusted". */
> > -                     if (cfg.colorSpace && cfg.colorSpace !=
> ColorSpace::Raw) {
> > +                     if (cfg.colorSpace && cfg.colorSpace !=
> ColorSpace::Raw)
> >                               status = Adjusted;
> > -                             cfg.colorSpace = ColorSpace::Raw;
> > -                     }
> > +                     cfg.colorSpace = ColorSpace::Raw;
> >                       continue;
> >               }
> >
>
> --
> Regards,
>
> Laurent Pinchart
>

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index c0dd9551..84120954 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -449,10 +449,9 @@  CameraConfiguration::Status RPiCameraConfiguration::validateColorSpaces([[maybe_
 		/* First fix up raw streams to have the "raw" colour space. */
 		if (isRaw(cfg.pixelFormat)) {
 			/* If there was no value here, that doesn't count as "adjusted". */
-			if (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw) {
+			if (cfg.colorSpace && cfg.colorSpace != ColorSpace::Raw)
 				status = Adjusted;
-				cfg.colorSpace = ColorSpace::Raw;
-			}
+			cfg.colorSpace = ColorSpace::Raw;
 			continue;
 		}