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

15097 diff mbox series

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

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 include/libcamera/camera.h | 10 +++++
 src/libcamera/camera.cpp   | 84 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+)

Hi David,

Thank you for the patch.

On Thu, Dec 09, 2021 at 10:12:44AM +0000, David Plowman wrote:
> This method forces raw streams to have the "raw" color space, and also

s/method/function/

> optionally makes all output streams to share the same color space as
> some platforms may require this.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>  include/libcamera/camera.h | 10 +++++
>  src/libcamera/camera.cpp   | 84 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 94 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..87ef8d72 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,88 @@ 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 ColorSpaceFlag::None

Doxygen warns here, you need

 * \var CameraConfiguration::ColorSpaceFlag::None

Same for the next one.

> + * \brief No extra validation of color spaces is required
> + * \var 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 shareOutputColorSpaces is set, all output streams are forced to share

s/shareOutputColorSpaces/the StreamsShareColorSpace flag/

> + * the same color space (this may be a constraint on some platforms).
> + *
> + * It is optional for a pipeline handler to use this method.

s/method/function/

> + *
> + * \return A CameraConfiguration::Status value that describes the validation
> + * status.
> + * \retval CameraConfiguration::Invalid The configuration is invalid and can't
> + * be adjusted. This may only occur in extreme cases such as when the
> + * configuration is empty.

You can drop this one, it can never happen.

> + * \retval CameraConfigutation::Adjusted The configuration has been adjusted
> + * and is now valid. Parameters may have changed for any stream, and stream
> + * configurations may have been removed. The caller shall check the
> + * configuration carefully.

No stream configuration removal :-) You can adapt the message to make it
specific to this function, likely with s/configuration/color spaces/

> + * \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) && cfg.colorSpace != ColorSpace::Raw) {
> +			cfg.colorSpace = ColorSpace::Raw;
> +			status = Adjusted;
> +		} else if (cfg.colorSpace && (index == -1 || cfg.size > config_[i].size))
> +			index = i;

		} 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