[libcamera-devel] [PATCH v10 7/8] libcamera: Add validateColorSpaces to CameraConfiguration class
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Dec 9 22:57:34 CET 2021
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 at 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
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list