<div dir="ltr"><div dir="ltr">Hi David,<div><br></div><div>Thank you for your work.</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, 20 Oct 2021 at 12:08, David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">This method checks that the requested color spaces are sensible and do<br>
not contain any undefined enum values. It also initialises the<br>
"actual" color space field to the same value as the one requested, in<br>
the expectation that the rest of the validate() method will be able to<br>
check this.<br>
<br>
Signed-off-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
---<br>
include/libcamera/camera.h | 2 ++<br>
src/libcamera/camera.cpp | 53 ++++++++++++++++++++++++++++++++++++++<br>
2 files changed, 55 insertions(+)<br>
<br>
diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h<br>
index 601ee46e..fdab4410 100644<br>
--- a/include/libcamera/camera.h<br>
+++ b/include/libcamera/camera.h<br>
@@ -69,6 +69,8 @@ public:<br>
protected:<br>
CameraConfiguration();<br>
<br>
+ Status validateColorSpaces(bool sharedColorSpace);<br>
+<br>
std::vector<StreamConfiguration> config_;<br>
};<br>
<br>
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp<br>
index 71809bcd..79451ef4 100644<br>
--- a/src/libcamera/camera.cpp<br>
+++ b/src/libcamera/camera.cpp<br>
@@ -19,6 +19,7 @@<br>
#include <libcamera/stream.h><br>
<br>
#include "libcamera/internal/camera.h"<br>
+#include "libcamera/internal/formats.h"<br>
#include "libcamera/internal/pipeline_handler.h"<br>
<br>
/**<br>
@@ -313,6 +314,58 @@ std::size_t CameraConfiguration::size() const<br>
return config_.size();<br>
}<br>
<br>
+static bool isRaw(const PixelFormat &pixFmt)<br>
+{<br>
+ const PixelFormatInfo &info = PixelFormatInfo::info(pixFmt);<br>
+ return info.isValid() &&<br>
+ info.colourEncoding == PixelFormatInfo::ColourEncodingRAW;<br>
+}<br></blockquote><div><br></div><div>Not as part of this series, but we ought to move this to the PixelFomat class,</div><div>there is an identical function in our pipeline handler :-)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+CameraConfiguration::Status CameraConfiguration::validateColorSpaces(bool sharedColorSpace)<br>
+{<br>
+ Status status = Valid;<br>
+<br>
+ /* Find the largest non-raw stream (if any). */<br>
+ int index = -1;<br>
+ for (unsigned int i = 0; i < config_.size(); i++) {<br>
+ const StreamConfiguration &cfg = config_[i];<br>
+ if (!isRaw(cfg.pixelFormat) &&<br>
+ (index < 0 ||<br>
+ cfg.size.width * cfg.size.height > config_[i].size.width * config_[i].size.height))<br></blockquote><div><br></div><div>You can replace this with "cfg.size > config_[i].size". Apart from that:</div><div><br></div><div>Reviewed-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com">naush@raspberrypi.com</a>></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ index = i;<br>
+ }<br>
+<br>
+ /*<br>
+ * Here we force raw streams to the correct color space and signal<br>
+ * an error if we encounter anything undefined. We handle the case<br>
+ * where all output streams are to share a color space, which we<br>
+ * choose to be the color space of the largest stream.<br>
+ */<br>
+ for (auto &cfg : config_) {<br>
+ ColorSpace initialColorSpace = cfg.requestedColorSpace;<br>
+<br>
+ if (isRaw(cfg.pixelFormat))<br>
+ cfg.requestedColorSpace = ColorSpace::Raw;<br>
+ else if (!cfg.requestedColorSpace.isFullyDefined()) {<br>
+ LOG(Camera, Error) << "Stream has undefined color space";<br>
+ cfg.requestedColorSpace = ColorSpace::Jpeg;<br>
+ } else if (sharedColorSpace)<br>
+ cfg.requestedColorSpace = config_[index].requestedColorSpace;<br>
+<br>
+ if (cfg.requestedColorSpace != initialColorSpace)<br>
+ status = Adjusted;<br>
+<br>
+ /*<br>
+ * We also initialise the actual color space as if the<br>
+ * hardware can do what we want. But note that the rest<br>
+ * of the validate() method may change this.<br>
+ */<br>
+ cfg.actualColorSpace = cfg.requestedColorSpace;<br>
+ }<br>
+<br>
+ return status;<br>
+}<br>
+<br>
/**<br>
* \var CameraConfiguration::transform<br>
* \brief User-specified transform to be applied to the image<br>
-- <br>
2.20.1<br>
<br>
</blockquote></div></div>