| Message ID | 20251121163016.94159-1-mzamazal@redhat.com |
|---|---|
| State | Accepted |
| Commit | 5a33bc10e9d3bc3a7cb1ead66c7ec0a413083d91 |
| Headers | show |
| Series |
|
| Related | show |
Quoting Milan Zamazal (2025-11-21 16:30:16) > StreamConfiguration's should have colorSpace set. This is not the case > in the simple pipeline. Let's set it there. This also fixes a crash in > `cam' due to accessing an unset colorSpace. > > We set the colour spaces according to the pixel format. This is not > completely correct because pixel formats and colour spaces are > different, although not completely independent, things. But for the > lack of a better practical option to determine the colour space, we use > this. > > Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/294 > Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/libcamera/pipeline/simple/simple.cpp | 40 ++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 118b4186c..c0d938cf0 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -25,6 +25,7 @@ > #include <libcamera/base/log.h> > > #include <libcamera/camera.h> > +#include <libcamera/color_space.h> > #include <libcamera/control_ids.h> > #include <libcamera/request.h> > #include <libcamera/stream.h> > @@ -36,6 +37,7 @@ > #include "libcamera/internal/converter.h" > #include "libcamera/internal/delayed_controls.h" > #include "libcamera/internal/device_enumerator.h" > +#include "libcamera/internal/formats.h" > #include "libcamera/internal/global_configuration.h" > #include "libcamera/internal/media_device.h" > #include "libcamera/internal/pipeline_handler.h" > @@ -1227,6 +1229,44 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() > status = Adjusted; > } > > + /* > + * Best effort to fix the color space. If the color space is not set, > + * set it according to the pixel format, which may not be correct (pixel > + * formats and color spaces are different things, although somewhat > + * related) but we don't have a better option at the moment. Then in any > + * case, perform the standard pixel format based color space adjustment. > + */ > + if (!cfg.colorSpace) { > + const PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat); > + switch (info.colourEncoding) { > + case PixelFormatInfo::ColourEncodingRGB: > + cfg.colorSpace = ColorSpace::Srgb; > + break; > + case PixelFormatInfo::ColourEncodingYUV: > + cfg.colorSpace = ColorSpace::Sycc; > + break; > + default: > + cfg.colorSpace = ColorSpace::Raw; > + } > + LOG(SimplePipeline, Debug) > + << "Unspecified color space set to " > + << cfg.colorSpace.value().toString(); > + /* > + * Adjust the assigned color space to make sure everything is OK. > + * Since this is assigning an unspecified color space rather than > + * adjusting a requested one, changes here shouldn't set the status > + * to Adjusted. > + */ > + cfg.colorSpace->adjust(pixelFormat); > + } else { > + if (cfg.colorSpace->adjust(pixelFormat)) { > + LOG(SimplePipeline, Debug) > + << "Color space adjusted to " > + << cfg.colorSpace.value().toString(); > + status = Adjusted; > + } > + } > + > if (!pipeConfig_->outputSizes.contains(cfg.size)) { > Size adjustedSize = pipeConfig_->captureSize; > /* > -- > 2.51.1 >
On 2025-11-21 22:00, Milan Zamazal wrote: > StreamConfiguration's should have colorSpace set. This is not the case > in the simple pipeline. Let's set it there. This also fixes a crash in > `cam' due to accessing an unset colorSpace. > > We set the colour spaces according to the pixel format. This is not > completely correct because pixel formats and colour spaces are > different, although not completely independent, things. But for the > lack of a better practical option to determine the colour space, we use > this. > > Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/294 > Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com> > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/libcamera/pipeline/simple/simple.cpp | 40 ++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 118b4186c..c0d938cf0 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -25,6 +25,7 @@ > #include <libcamera/base/log.h> > > #include <libcamera/camera.h> > +#include <libcamera/color_space.h> > #include <libcamera/control_ids.h> > #include <libcamera/request.h> > #include <libcamera/stream.h> > @@ -36,6 +37,7 @@ > #include "libcamera/internal/converter.h" > #include "libcamera/internal/delayed_controls.h" > #include "libcamera/internal/device_enumerator.h" > +#include "libcamera/internal/formats.h" > #include "libcamera/internal/global_configuration.h" > #include "libcamera/internal/media_device.h" > #include "libcamera/internal/pipeline_handler.h" > @@ -1227,6 +1229,44 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() > status = Adjusted; > } > > + /* > + * Best effort to fix the color space. If the color space is not set, > + * set it according to the pixel format, which may not be correct (pixel > + * formats and color spaces are different things, although somewhat > + * related) but we don't have a better option at the moment. Then in any > + * case, perform the standard pixel format based color space adjustment. > + */ > + if (!cfg.colorSpace) { > + const PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat); > + switch (info.colourEncoding) { > + case PixelFormatInfo::ColourEncodingRGB: > + cfg.colorSpace = ColorSpace::Srgb; > + break; > + case PixelFormatInfo::ColourEncodingYUV: > + cfg.colorSpace = ColorSpace::Sycc; > + break; > + default: > + cfg.colorSpace = ColorSpace::Raw; > + } > + LOG(SimplePipeline, Debug) > + << "Unspecified color space set to " > + << cfg.colorSpace.value().toString(); This should be logged after adjusting the colorspace. > + /* > + * Adjust the assigned color space to make sure everything is OK. > + * Since this is assigning an unspecified color space rather than > + * adjusting a requested one, changes here shouldn't set the status > + * to Adjusted. Why ? As far as I understand, any alteration or adjustment will need to report Adjusted as status. How is this different from other CameraConfiguration members for e.g. orientation ? Last I had seen, orientation (defaults to Rotate0) reports the native sensor orientation, if user hasn't supplied it explicitly. That also return Adjusted as status. > + */ > + cfg.colorSpace->adjust(pixelFormat); > + } else { > + if (cfg.colorSpace->adjust(pixelFormat)) { I think calling validateColorSpaces() out of this loop would be a better fit, given the above argument. > + LOG(SimplePipeline, Debug) > + << "Color space adjusted to " > + << cfg.colorSpace.value().toString(); > + status = Adjusted; > + } > + } > + > if (!pipeConfig_->outputSizes.contains(cfg.size)) { > Size adjustedSize = pipeConfig_->captureSize; > /*
Quoting Umang Jain (2025-11-24 06:30:33) > On 2025-11-21 22:00, Milan Zamazal wrote: > > StreamConfiguration's should have colorSpace set. This is not the case > > in the simple pipeline. Let's set it there. This also fixes a crash in > > `cam' due to accessing an unset colorSpace. > > > > We set the colour spaces according to the pixel format. This is not > > completely correct because pixel formats and colour spaces are > > different, although not completely independent, things. But for the > > lack of a better practical option to determine the colour space, we use > > this. > > > > Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/294 > > Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com> > > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > > --- > > src/libcamera/pipeline/simple/simple.cpp | 40 ++++++++++++++++++++++++ > > 1 file changed, 40 insertions(+) > > > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > > index 118b4186c..c0d938cf0 100644 > > --- a/src/libcamera/pipeline/simple/simple.cpp > > +++ b/src/libcamera/pipeline/simple/simple.cpp > > @@ -25,6 +25,7 @@ > > #include <libcamera/base/log.h> > > > > #include <libcamera/camera.h> > > +#include <libcamera/color_space.h> > > #include <libcamera/control_ids.h> > > #include <libcamera/request.h> > > #include <libcamera/stream.h> > > @@ -36,6 +37,7 @@ > > #include "libcamera/internal/converter.h" > > #include "libcamera/internal/delayed_controls.h" > > #include "libcamera/internal/device_enumerator.h" > > +#include "libcamera/internal/formats.h" > > #include "libcamera/internal/global_configuration.h" > > #include "libcamera/internal/media_device.h" > > #include "libcamera/internal/pipeline_handler.h" > > @@ -1227,6 +1229,44 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() > > status = Adjusted; > > } > > > > + /* > > + * Best effort to fix the color space. If the color space is not set, > > + * set it according to the pixel format, which may not be correct (pixel > > + * formats and color spaces are different things, although somewhat > > + * related) but we don't have a better option at the moment. Then in any > > + * case, perform the standard pixel format based color space adjustment. > > + */ > > + if (!cfg.colorSpace) { > > + const PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat); > > + switch (info.colourEncoding) { > > + case PixelFormatInfo::ColourEncodingRGB: > > + cfg.colorSpace = ColorSpace::Srgb; > > + break; > > + case PixelFormatInfo::ColourEncodingYUV: > > + cfg.colorSpace = ColorSpace::Sycc; > > + break; > > + default: > > + cfg.colorSpace = ColorSpace::Raw; > > + } > > + LOG(SimplePipeline, Debug) > > + << "Unspecified color space set to " > > + << cfg.colorSpace.value().toString(); > > This should be logged after adjusting the colorspace. Ohh I missed that, but this patch is already merged. Please post a fix on top. > > + /* > > + * Adjust the assigned color space to make sure everything is OK. > > + * Since this is assigning an unspecified color space rather than > > + * adjusting a requested one, changes here shouldn't set the status > > + * to Adjusted. > > Why ? As far as I understand, any alteration or adjustment will need to > report Adjusted as status. > > How is this different from other CameraConfiguration members for e.g. > orientation ? Last I had seen, orientation (defaults to Rotate0) reports > the native sensor orientation, if user hasn't supplied it explicitly. > That also return Adjusted as status. As I understand the Adjusted status - it's "You asked for something but we couldn't deliver it - please check to see what you are going to get instead". If the user /didn't/ ask for a specific colour space, then we're not adjusting what they want - we're just telling them what they'll get ? > > + */ > > + cfg.colorSpace->adjust(pixelFormat); > > + } else { > > + if (cfg.colorSpace->adjust(pixelFormat)) { > > I think calling validateColorSpaces() out of this loop would be a better > fit, given the above argument. > > > + LOG(SimplePipeline, Debug) > > + << "Color space adjusted to " > > + << cfg.colorSpace.value().toString(); > > + status = Adjusted; > > + } > > + } > > + > > if (!pipeConfig_->outputSizes.contains(cfg.size)) { > > Size adjustedSize = pipeConfig_->captureSize; > > /*
On 2025-11-24 14:23, Kieran Bingham wrote: > Quoting Umang Jain (2025-11-24 06:30:33) >> On 2025-11-21 22:00, Milan Zamazal wrote: >> > StreamConfiguration's should have colorSpace set. This is not the case >> > in the simple pipeline. Let's set it there. This also fixes a crash in >> > `cam' due to accessing an unset colorSpace. >> > >> > We set the colour spaces according to the pixel format. This is not >> > completely correct because pixel formats and colour spaces are >> > different, although not completely independent, things. But for the >> > lack of a better practical option to determine the colour space, we use >> > this. >> > >> > Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/294 >> > Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com> >> > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> >> > --- >> > src/libcamera/pipeline/simple/simple.cpp | 40 ++++++++++++++++++++++++ >> > 1 file changed, 40 insertions(+) >> > >> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp >> > index 118b4186c..c0d938cf0 100644 >> > --- a/src/libcamera/pipeline/simple/simple.cpp >> > +++ b/src/libcamera/pipeline/simple/simple.cpp >> > @@ -25,6 +25,7 @@ >> > #include <libcamera/base/log.h> >> > >> > #include <libcamera/camera.h> >> > +#include <libcamera/color_space.h> >> > #include <libcamera/control_ids.h> >> > #include <libcamera/request.h> >> > #include <libcamera/stream.h> >> > @@ -36,6 +37,7 @@ >> > #include "libcamera/internal/converter.h" >> > #include "libcamera/internal/delayed_controls.h" >> > #include "libcamera/internal/device_enumerator.h" >> > +#include "libcamera/internal/formats.h" >> > #include "libcamera/internal/global_configuration.h" >> > #include "libcamera/internal/media_device.h" >> > #include "libcamera/internal/pipeline_handler.h" >> > @@ -1227,6 +1229,44 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() >> > status = Adjusted; >> > } >> > >> > + /* >> > + * Best effort to fix the color space. If the color space is not set, >> > + * set it according to the pixel format, which may not be correct (pixel >> > + * formats and color spaces are different things, although somewhat >> > + * related) but we don't have a better option at the moment. Then in any >> > + * case, perform the standard pixel format based color space adjustment. >> > + */ >> > + if (!cfg.colorSpace) { >> > + const PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat); >> > + switch (info.colourEncoding) { >> > + case PixelFormatInfo::ColourEncodingRGB: >> > + cfg.colorSpace = ColorSpace::Srgb; >> > + break; >> > + case PixelFormatInfo::ColourEncodingYUV: >> > + cfg.colorSpace = ColorSpace::Sycc; >> > + break; >> > + default: >> > + cfg.colorSpace = ColorSpace::Raw; >> > + } >> > + LOG(SimplePipeline, Debug) >> > + << "Unspecified color space set to " >> > + << cfg.colorSpace.value().toString(); >> >> This should be logged after adjusting the colorspace. > > Ohh I missed that, but this patch is already merged. Please post a fix > on top. > >> > + /* >> > + * Adjust the assigned color space to make sure everything is OK. >> > + * Since this is assigning an unspecified color space rather than >> > + * adjusting a requested one, changes here shouldn't set the status >> > + * to Adjusted. >> >> Why ? As far as I understand, any alteration or adjustment will need to >> report Adjusted as status. >> >> How is this different from other CameraConfiguration members for e.g. >> orientation ? Last I had seen, orientation (defaults to Rotate0) reports >> the native sensor orientation, if user hasn't supplied it explicitly. >> That also return Adjusted as status. > > As I understand the Adjusted status - it's "You asked for something but > we couldn't deliver it - please check to see what you are going to get > instead". > > If the user /didn't/ ask for a specific colour space, then we're not > adjusting what they want - we're just telling them what they'll get ? I understand your rationale in theory, but currently we *do* return Adjusted for these cases as well. I think we need more CameraConfiguration::Status return codes to address and sweep across codebase to identify such cases and fix them. One case in previously gave is orientation one, another one I can easily spot around bufferCount. In simple pipeline handler validate(): const unsigned int bufferCount = cfg.bufferCount; if (!bufferCount) cfg.bufferCount = kNumBuffersDefault; else if (bufferCount > kNumBuffersMax) cfg.bufferCount = kNumBuffersMax; if (cfg.bufferCount != bufferCount) { LOG(SimplePipeline, Debug) << "Adjusting bufferCount from " << bufferCount << " to " << cfg.bufferCount; status = Adjusted; } If user is not aware of right no. of bufferCount (defaults to 0), kNumBuffersDefault is set in validate, with status = Adjusted. Again, how is this different from colorspace? > > > >> > + */ >> > + cfg.colorSpace->adjust(pixelFormat); >> > + } else { >> > + if (cfg.colorSpace->adjust(pixelFormat)) { >> >> I think calling validateColorSpaces() out of this loop would be a better >> fit, given the above argument. >> >> > + LOG(SimplePipeline, Debug) >> > + << "Color space adjusted to " >> > + << cfg.colorSpace.value().toString(); >> > + status = Adjusted; >> > + } >> > + } >> > + >> > if (!pipeConfig_->outputSizes.contains(cfg.size)) { >> > Size adjustedSize = pipeConfig_->captureSize; >> > /*
Quoting Umang Jain (2025-11-24 09:13:43) > On 2025-11-24 14:23, Kieran Bingham wrote: > > Quoting Umang Jain (2025-11-24 06:30:33) > >> On 2025-11-21 22:00, Milan Zamazal wrote: > >> > StreamConfiguration's should have colorSpace set. This is not the case > >> > in the simple pipeline. Let's set it there. This also fixes a crash in > >> > `cam' due to accessing an unset colorSpace. > >> > > >> > We set the colour spaces according to the pixel format. This is not > >> > completely correct because pixel formats and colour spaces are > >> > different, although not completely independent, things. But for the > >> > lack of a better practical option to determine the colour space, we use > >> > this. > >> > > >> > Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/294 > >> > Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com> > >> > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > >> > --- > >> > src/libcamera/pipeline/simple/simple.cpp | 40 ++++++++++++++++++++++++ > >> > 1 file changed, 40 insertions(+) > >> > > >> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > >> > index 118b4186c..c0d938cf0 100644 > >> > --- a/src/libcamera/pipeline/simple/simple.cpp > >> > +++ b/src/libcamera/pipeline/simple/simple.cpp > >> > @@ -25,6 +25,7 @@ > >> > #include <libcamera/base/log.h> > >> > > >> > #include <libcamera/camera.h> > >> > +#include <libcamera/color_space.h> > >> > #include <libcamera/control_ids.h> > >> > #include <libcamera/request.h> > >> > #include <libcamera/stream.h> > >> > @@ -36,6 +37,7 @@ > >> > #include "libcamera/internal/converter.h" > >> > #include "libcamera/internal/delayed_controls.h" > >> > #include "libcamera/internal/device_enumerator.h" > >> > +#include "libcamera/internal/formats.h" > >> > #include "libcamera/internal/global_configuration.h" > >> > #include "libcamera/internal/media_device.h" > >> > #include "libcamera/internal/pipeline_handler.h" > >> > @@ -1227,6 +1229,44 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() > >> > status = Adjusted; > >> > } > >> > > >> > + /* > >> > + * Best effort to fix the color space. If the color space is not set, > >> > + * set it according to the pixel format, which may not be correct (pixel > >> > + * formats and color spaces are different things, although somewhat > >> > + * related) but we don't have a better option at the moment. Then in any > >> > + * case, perform the standard pixel format based color space adjustment. > >> > + */ > >> > + if (!cfg.colorSpace) { > >> > + const PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat); > >> > + switch (info.colourEncoding) { > >> > + case PixelFormatInfo::ColourEncodingRGB: > >> > + cfg.colorSpace = ColorSpace::Srgb; > >> > + break; > >> > + case PixelFormatInfo::ColourEncodingYUV: > >> > + cfg.colorSpace = ColorSpace::Sycc; > >> > + break; > >> > + default: > >> > + cfg.colorSpace = ColorSpace::Raw; > >> > + } > >> > + LOG(SimplePipeline, Debug) > >> > + << "Unspecified color space set to " > >> > + << cfg.colorSpace.value().toString(); > >> > >> This should be logged after adjusting the colorspace. > > > > Ohh I missed that, but this patch is already merged. Please post a fix > > on top. > > > >> > + /* > >> > + * Adjust the assigned color space to make sure everything is OK. > >> > + * Since this is assigning an unspecified color space rather than > >> > + * adjusting a requested one, changes here shouldn't set the status > >> > + * to Adjusted. > >> > >> Why ? As far as I understand, any alteration or adjustment will need to > >> report Adjusted as status. > >> > >> How is this different from other CameraConfiguration members for e.g. > >> orientation ? Last I had seen, orientation (defaults to Rotate0) reports > >> the native sensor orientation, if user hasn't supplied it explicitly. > >> That also return Adjusted as status. > > > > As I understand the Adjusted status - it's "You asked for something but > > we couldn't deliver it - please check to see what you are going to get > > instead". > > > > If the user /didn't/ ask for a specific colour space, then we're not > > adjusting what they want - we're just telling them what they'll get ? > > I understand your rationale in theory, but currently we *do* return > Adjusted for these cases as well. I think we need more > CameraConfiguration::Status return codes to address and sweep across > codebase to identify such cases and fix them. One case in previously > gave is orientation one, another one I can easily spot around > bufferCount. In simple pipeline handler validate(): > > const unsigned int bufferCount = cfg.bufferCount; > if (!bufferCount) > cfg.bufferCount = kNumBuffersDefault; > else if (bufferCount > kNumBuffersMax) > cfg.bufferCount = kNumBuffersMax; > > if (cfg.bufferCount != bufferCount) { > LOG(SimplePipeline, Debug) > << "Adjusting bufferCount from " << bufferCount > << " to " << cfg.bufferCount; > status = Adjusted; > } > > If user is not aware of right no. of bufferCount (defaults to 0), > kNumBuffersDefault is set in validate, with status = Adjusted. Again, > how is this different from colorspace? colorSpace is a std::optional ? bufferCount is not ... and neither is orientation. There is /no way/ to *not* specify a bufferCount nor an orientation in the CameraConfiguration, while there is a way to *not* specify the colorSpace. If someone didn't specify a colorspace, and we fill it in - that wasn't adjusted - it was filled in. If someone didn't specify the orientation or bufferCount they are intialised with defaults, and we have no way currently to know if the application asked for that orientation and bufferCount or if they were just the default values. -- Kieran > > > > > > > > >> > + */ > >> > + cfg.colorSpace->adjust(pixelFormat); > >> > + } else { > >> > + if (cfg.colorSpace->adjust(pixelFormat)) { > >> > >> I think calling validateColorSpaces() out of this loop would be a better > >> fit, given the above argument. > >> > >> > + LOG(SimplePipeline, Debug) > >> > + << "Color space adjusted to " > >> > + << cfg.colorSpace.value().toString(); > >> > + status = Adjusted; > >> > + } > >> > + } > >> > + > >> > if (!pipeConfig_->outputSizes.contains(cfg.size)) { > >> > Size adjustedSize = pipeConfig_->captureSize; > >> > /*
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 118b4186c..c0d938cf0 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -25,6 +25,7 @@ #include <libcamera/base/log.h> #include <libcamera/camera.h> +#include <libcamera/color_space.h> #include <libcamera/control_ids.h> #include <libcamera/request.h> #include <libcamera/stream.h> @@ -36,6 +37,7 @@ #include "libcamera/internal/converter.h" #include "libcamera/internal/delayed_controls.h" #include "libcamera/internal/device_enumerator.h" +#include "libcamera/internal/formats.h" #include "libcamera/internal/global_configuration.h" #include "libcamera/internal/media_device.h" #include "libcamera/internal/pipeline_handler.h" @@ -1227,6 +1229,44 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() status = Adjusted; } + /* + * Best effort to fix the color space. If the color space is not set, + * set it according to the pixel format, which may not be correct (pixel + * formats and color spaces are different things, although somewhat + * related) but we don't have a better option at the moment. Then in any + * case, perform the standard pixel format based color space adjustment. + */ + if (!cfg.colorSpace) { + const PixelFormatInfo &info = PixelFormatInfo::info(pixelFormat); + switch (info.colourEncoding) { + case PixelFormatInfo::ColourEncodingRGB: + cfg.colorSpace = ColorSpace::Srgb; + break; + case PixelFormatInfo::ColourEncodingYUV: + cfg.colorSpace = ColorSpace::Sycc; + break; + default: + cfg.colorSpace = ColorSpace::Raw; + } + LOG(SimplePipeline, Debug) + << "Unspecified color space set to " + << cfg.colorSpace.value().toString(); + /* + * Adjust the assigned color space to make sure everything is OK. + * Since this is assigning an unspecified color space rather than + * adjusting a requested one, changes here shouldn't set the status + * to Adjusted. + */ + cfg.colorSpace->adjust(pixelFormat); + } else { + if (cfg.colorSpace->adjust(pixelFormat)) { + LOG(SimplePipeline, Debug) + << "Color space adjusted to " + << cfg.colorSpace.value().toString(); + status = Adjusted; + } + } + if (!pipeConfig_->outputSizes.contains(cfg.size)) { Size adjustedSize = pipeConfig_->captureSize; /*