[libcamera-devel] [PATCH v3 6/7] libcamera: Add validateColorSpaces to CameraConfiguration class

[Naushir Patuck]

Hi David,

Thank you for your work.


On Wed, 20 Oct 2021 at 12:08, David Plowman <david.plowman at raspberrypi.com>
wrote:

> This method checks that the requested color spaces are sensible and do
> not contain any undefined enum values. It also initialises the
> "actual" color space field to the same value as the one requested, in
> the expectation that the rest of the validate() method will be able to
> check this.
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  include/libcamera/camera.h |  2 ++
>  src/libcamera/camera.cpp   | 53 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 55 insertions(+)
>
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 601ee46e..fdab4410 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -69,6 +69,8 @@ public:
>  protected:
>         CameraConfiguration();
>
> +       Status validateColorSpaces(bool sharedColorSpace);
> +
>         std::vector<StreamConfiguration> config_;
>  };
>
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 71809bcd..79451ef4 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -19,6 +19,7 @@
>  #include <libcamera/stream.h>
>
>  #include "libcamera/internal/camera.h"
> +#include "libcamera/internal/formats.h"
>  #include "libcamera/internal/pipeline_handler.h"
>
>  /**
> @@ -313,6 +314,58 @@ std::size_t CameraConfiguration::size() const
>         return config_.size();
>  }
>
> +static bool isRaw(const PixelFormat &pixFmt)
> +{
> +       const PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);
> +       return info.isValid() &&
> +              info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;
> +}
>

Not as part of this series, but we ought to move this to the
PixelFomat class,
there is an identical function in our pipeline handler :-)


> +
> +CameraConfiguration::Status CameraConfiguration::validateColorSpaces(bool
> sharedColorSpace)
> +{
> +       Status status = Valid;
> +
> +       /* Find the largest non-raw stream (if any). */
> +       int index = -1;
> +       for (unsigned int i = 0; i < config_.size(); i++) {
> +               const StreamConfiguration &cfg = config_[i];
> +               if (!isRaw(cfg.pixelFormat) &&
> +                   (index < 0 ||
> +                    cfg.size.width * cfg.size.height >
> config_[i].size.width * config_[i].size.height))
>

You can replace this with "cfg.size > config_[i].size". Apart from that:

Reviewed-by: Naushir Patuck <naush at raspberrypi.com>


> +                       index = i;
> +       }
> +
> +       /*
> +        * Here we force raw streams to the correct color space and signal
> +        * an error if we encounter anything undefined. We handle the case
> +        * where all output streams are to share a color space, which we
> +        * choose to be the color space of the largest stream.
> +        */
> +       for (auto &cfg : config_) {
> +               ColorSpace initialColorSpace = cfg.requestedColorSpace;
> +
> +               if (isRaw(cfg.pixelFormat))
> +                       cfg.requestedColorSpace = ColorSpace::Raw;
> +               else if (!cfg.requestedColorSpace.isFullyDefined()) {
> +                       LOG(Camera, Error) << "Stream has undefined color
> space";
> +                       cfg.requestedColorSpace = ColorSpace::Jpeg;
> +               } else if (sharedColorSpace)
> +                       cfg.requestedColorSpace =
> config_[index].requestedColorSpace;
> +
> +               if (cfg.requestedColorSpace != initialColorSpace)
> +                       status = Adjusted;
> +
> +               /*
> +                * We also initialise the actual color space as if the
> +                * hardware can do what we want. But note that the rest
> +                * of the validate() method may change this.
> +                */
> +               cfg.actualColorSpace = cfg.requestedColorSpace;
> +       }
> +
> +       return status;
> +}
> +
>  /**
>   * \var CameraConfiguration::transform
>   * \brief User-specified transform to be applied to the image
> --
> 2.20.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20211025/7414dddf/attachment.htm>