[v2,1/2] pipeline: virtual: Provide and validate colorspace

23703 diff mbox series

Virtual pipeline handler should provide colorSpace in
generateConfiguration() and validate the colorspace in validate().
It is mandatory for a pipeline handler to set the colorspace if it
is unset in the stream configuration, during validate().

Signed-off-by: Umang Jain <uajain@igalia.com>
---
 src/libcamera/pipeline/virtual/virtual.cpp | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Quoting Umang Jain (2025-07-01 12:29:21)
> Virtual pipeline handler should provide colorSpace in
> generateConfiguration() and validate the colorspace in validate().
> It is mandatory for a pipeline handler to set the colorspace if it
> is unset in the stream configuration, during validate().
> 
> Signed-off-by: Umang Jain <uajain@igalia.com>
> ---
>  src/libcamera/pipeline/virtual/virtual.cpp | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp
> index 049ebcba..841846b4 100644
> --- a/src/libcamera/pipeline/virtual/virtual.cpp
> +++ b/src/libcamera/pipeline/virtual/virtual.cpp
> @@ -180,6 +180,17 @@ CameraConfiguration::Status VirtualCameraConfiguration::validate()
>                         adjusted = true;
>                 }
>  
> +               if (!cfg.colorSpace) {
> +                       cfg.colorSpace = ColorSpace::Sycc;
> +                       status = Adjusted;
> +                       adjusted = true;
> +               }

It looks like virtual only uses NV12 so I think this is fine.


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

> +
> +               if (validateColorSpaces() == Adjusted) {
> +                       status = Adjusted;
> +                       adjusted = true;
> +               }
> +
>                 if (adjusted)
>                         LOG(Virtual, Info)
>                                 << "Stream configuration adjusted to " << cfg.toString();
> @@ -244,6 +255,7 @@ PipelineHandlerVirtual::generateConfiguration(Camera *camera,
>                 cfg.pixelFormat = pixelFormat;
>                 cfg.size = data->config_.maxResolutionSize;
>                 cfg.bufferCount = VirtualCameraConfiguration::kBufferCount;
> +               cfg.colorSpace = ColorSpace::Sycc;
>  
>                 config->addConfiguration(cfg);
>         }
> -- 
> 2.50.0
>

Quoting Kieran Bingham (2025-07-10 08:45:28)
> Quoting Umang Jain (2025-07-01 12:29:21)
> > Virtual pipeline handler should provide colorSpace in
> > generateConfiguration() and validate the colorspace in validate().
> > It is mandatory for a pipeline handler to set the colorspace if it
> > is unset in the stream configuration, during validate().
> > 
> > Signed-off-by: Umang Jain <uajain@igalia.com>

From Robert, For patchwork:

Tested-by: Robert Mader <robert.mader@collabora.com>

> > ---
> >  src/libcamera/pipeline/virtual/virtual.cpp | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp
> > index 049ebcba..841846b4 100644
> > --- a/src/libcamera/pipeline/virtual/virtual.cpp
> > +++ b/src/libcamera/pipeline/virtual/virtual.cpp
> > @@ -180,6 +180,17 @@ CameraConfiguration::Status VirtualCameraConfiguration::validate()
> >                         adjusted = true;
> >                 }
> >  
> > +               if (!cfg.colorSpace) {
> > +                       cfg.colorSpace = ColorSpace::Sycc;
> > +                       status = Adjusted;
> > +                       adjusted = true;
> > +               }
> 
> It looks like virtual only uses NV12 so I think this is fine.
> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > +
> > +               if (validateColorSpaces() == Adjusted) {
> > +                       status = Adjusted;
> > +                       adjusted = true;
> > +               }
> > +
> >                 if (adjusted)
> >                         LOG(Virtual, Info)
> >                                 << "Stream configuration adjusted to " << cfg.toString();
> > @@ -244,6 +255,7 @@ PipelineHandlerVirtual::generateConfiguration(Camera *camera,
> >                 cfg.pixelFormat = pixelFormat;
> >                 cfg.size = data->config_.maxResolutionSize;
> >                 cfg.bufferCount = VirtualCameraConfiguration::kBufferCount;
> > +               cfg.colorSpace = ColorSpace::Sycc;
> >  
> >                 config->addConfiguration(cfg);
> >         }
> > -- 
> > 2.50.0
> >