| Message ID | 20211126104045.4756-7-david.plowman@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi David On Fri, Nov 26, 2021 at 10:40:44AM +0000, David Plowman wrote: > 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 | 2 ++ > src/libcamera/camera.cpp | 38 ++++++++++++++++++++++++++++++++++++++ Helpers of this type doesn't really have a place in the current codebase. Ideally this could go in a CameraConfiguration::Private which we don't have at the moment ? I don't have better places to suggest I'm afraid, also the PipelineHandler base class should be considered. Otherwise it's fine here.. > 2 files changed, 40 insertions(+) > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > index a7759ccb..32a7f812 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 400a7cf0..dd06f600 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -20,6 +20,7 @@ > > #include "libcamera/internal/camera.h" > #include "libcamera/internal/camera_controls.h" > +#include "libcamera/internal/formats.h" > #include "libcamera/internal/pipeline_handler.h" You should include color_space.h > > /** > @@ -314,6 +315,43 @@ 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; > +} > + > +CameraConfiguration::Status CameraConfiguration::validateColorSpaces(bool shareOuputColorSpaces) > +{ > + 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; This might actually be an adjustment if (cfg.colorSpace != ColorSpace::Raw) { cfg.colorSpace = ColorSpace::Raw; status = Adjusted; } > + else if (cfg.colorSpace && (index == -1 || cfg.size > config_[i].size)) > + index = i; > + } > + > + /* Make all output color spaces the same, if requested. */ > + if (index >= 0 && shareOuputColorSpaces) { I would if (index < 0 || !shareOuputColorSpaces) return status; to reduce one indentation level and return earlier. up to you. > + for (auto &cfg : config_) { > + if (!isRaw(cfg.pixelFormat) && Same here if (isRaw(cfg.pixelFormat) continue; up to you > + 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 >
Hi again On Fri, Nov 26, 2021 at 10:40:44AM +0000, David Plowman wrote: > 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 | 2 ++ > src/libcamera/camera.cpp | 38 ++++++++++++++++++++++++++++++++++++++ > 2 files changed, 40 insertions(+) > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h > index a7759ccb..32a7f812 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 400a7cf0..dd06f600 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -20,6 +20,7 @@ > > #include "libcamera/internal/camera.h" > #include "libcamera/internal/camera_controls.h" > +#include "libcamera/internal/formats.h" > #include "libcamera/internal/pipeline_handler.h" > > /** > @@ -314,6 +315,43 @@ 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; > +} > + Documentation is missing include/libcamera/camera.h:72: warning: Member validateColorSpaces(bool sharedColorSpace) (function) of class libcamera::CameraConfiguration is not documented. > +CameraConfiguration::Status CameraConfiguration::validateColorSpaces(bool shareOuputColorSpaces) > +{ > + 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; > + else if (cfg.colorSpace && (index == -1 || cfg.size > config_[i].size)) > + index = i; > + } > + > + /* Make all output color spaces the same, if requested. */ > + if (index >= 0 && shareOuputColorSpaces) { > + 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..32a7f812 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 400a7cf0..dd06f600 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -20,6 +20,7 @@ #include "libcamera/internal/camera.h" #include "libcamera/internal/camera_controls.h" +#include "libcamera/internal/formats.h" #include "libcamera/internal/pipeline_handler.h" /** @@ -314,6 +315,43 @@ 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; +} + +CameraConfiguration::Status CameraConfiguration::validateColorSpaces(bool shareOuputColorSpaces) +{ + 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; + else if (cfg.colorSpace && (index == -1 || cfg.size > config_[i].size)) + index = i; + } + + /* Make all output color spaces the same, if requested. */ + if (index >= 0 && shareOuputColorSpaces) { + 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
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 | 2 ++ src/libcamera/camera.cpp | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+)