[libcamera-devel,v12,7/8] libcamera: Add validateColorSpaces to CameraConfiguration class

15120 diff mbox series

This function forces raw streams to have the "raw" color space, and
also optionally makes all non-raw output streams to share the same
color space as some platforms may require this.

When sharing color spaces we take the shared value to be the one from
the largest of these streams. This choice is ultimately arbitrary, but
can be appropriate if smaller output streams are used for image
analysis rather than human consumption, when the precise colours may
be less important.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/camera.h | 10 +++++
 src/libcamera/camera.cpp   | 83 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+)

Hi David,

On Fri, Dec 10, 2021 at 02:44:23PM +0000, David Plowman wrote:
> This function forces raw streams to have the "raw" color space, and
> also optionally makes all non-raw output streams to share the same
> color space as some platforms may require this.
>
> When sharing color spaces we take the shared value to be the one from
> the largest of these streams. This choice is ultimately arbitrary, but
> can be appropriate if smaller output streams are used for image
> analysis rather than human consumption, when the precise colours may
> be less important.
>
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

I would have said usage of Flag<> is really an overkill, but I see
you've been suggested so, so no big deal

Thanks
   j

> ---
>  include/libcamera/camera.h | 10 +++++
>  src/libcamera/camera.cpp   | 83 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 93 insertions(+)
>
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index a7759ccb..5bb06584 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -13,6 +13,7 @@
>  #include <string>
>
>  #include <libcamera/base/class.h>
> +#include <libcamera/base/flags.h>
>  #include <libcamera/base/object.h>
>  #include <libcamera/base/signal.h>
>
> @@ -69,6 +70,15 @@ public:
>  protected:
>  	CameraConfiguration();
>
> +	enum class ColorSpaceFlag {
> +		None,
> +		StreamsShareColorSpace,
> +	};
> +
> +	using ColorSpaceFlags = Flags<ColorSpaceFlag>;
> +
> +	Status validateColorSpaces(ColorSpaceFlags flags = ColorSpaceFlag::None);
> +
>  	std::vector<StreamConfiguration> config_;
>  };
>
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 400a7cf0..86d84ac0 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -14,12 +14,14 @@
>  #include <libcamera/base/log.h>
>  #include <libcamera/base/thread.h>
>
> +#include <libcamera/color_space.h>
>  #include <libcamera/framebuffer_allocator.h>
>  #include <libcamera/request.h>
>  #include <libcamera/stream.h>
>
>  #include "libcamera/internal/camera.h"
>  #include "libcamera/internal/camera_controls.h"
> +#include "libcamera/internal/formats.h"
>  #include "libcamera/internal/pipeline_handler.h"
>
>  /**
> @@ -314,6 +316,87 @@ std::size_t CameraConfiguration::size() const
>  	return config_.size();
>  }
>
> +namespace {
> +
> +bool isRaw(const PixelFormat &pixFmt)
> +{
> +	const PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);
> +	return info.isValid() &&
> +	       info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;
> +}
> +
> +} /* namespace */
> +
> +/**
> + * \enum CameraConfiguration::ColorSpaceFlag
> + * \brief Specify the behaviour of validateColorSpaces
> + * \var CameraConfiguration::ColorSpaceFlag::None
> + * \brief No extra validation of color spaces is required
> + * \var CameraConfiguration::ColorSpaceFlag::StreamsShareColorSpace
> + * \brief Non-raw output streams must share the same color space
> + */
> +
> +/**
> + * \typedef CameraConfiguration::ColorSpaceFlags
> + * \brief A bitwise combination of ColorSpaceFlag values
> + */
> +
> +/**
> + * \brief Check the color spaces requested for each stream
> + * \param[in] flags Flags to control the behaviour of this function
> + *
> + * This function performs certain consistency checks on the color spaces of
> + * the streams and may adjust them so that:
> + *
> + * - Any raw streams have the Raw color space
> + * - If the StreamsShareColorSpace flag is set, all output streams are forced
> + * to share the same color space (this may be a constraint on some platforms).
> + *
> + * It is optional for a pipeline handler to use this function.
> + *
> + * \return A CameraConfiguration::Status value that describes the validation
> + * status.
> + * \retval CameraConfigutation::Adjusted The configuration has been adjusted
> + * and is now valid. The color space of some or all of the streams may bave
> + * benn changed. The caller shall check the color spaces carefully.
> + * \retval CameraConfiguration::Valid The configuration was already valid and
> + * hasn't been adjusted.
> + */
> +CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceFlags flags)
> +{
> +	Status status = Valid;
> +
> +	/*
> +	 * Set all raw streams to the Raw color space, and make a note of the largest
> +	 * non-raw stream with a defined color space (if there is one).
> +	 */
> +	int index = -1;
> +	for (auto [i, cfg] : utils::enumerate(config_)) {
> +		if (isRaw(cfg.pixelFormat)) {
> +			if (cfg.colorSpace != ColorSpace::Raw) {
> +				cfg.colorSpace = ColorSpace::Raw;
> +				status = Adjusted;
> +			}
> +		} else if (cfg.colorSpace && (index == -1 || cfg.size > config_[i].size)) {
> +			index = i;
> +		}
> +	}
> +
> +	if (index < 0 || !(flags & ColorSpaceFlag::StreamsShareColorSpace))
> +		return status;
> +
> +	/* Make all output color spaces the same, if requested. */
> +	for (auto &cfg : config_) {
> +		if (!isRaw(cfg.pixelFormat) &&
> +		    cfg.colorSpace != config_[index].colorSpace) {
> +			cfg.colorSpace = config_[index].colorSpace;
> +			status = Adjusted;
> +		}
> +	}
> +
> +	return status;
> +}
> +
>  /**
>   * \var CameraConfiguration::transform
>   * \brief User-specified transform to be applied to the image
> --
> 2.30.2
>