Message ID | 20211210144424.14747-8-david.plowman@raspberrypi.com |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
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 >
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