<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>