<div dir="auto"><div><span class="gmail_chip gmail_plusreply" dir="auto"><a href="mailto:david.plowman@raspberrypi.com" style="color:#15c;text-decoration:underline">+David Plowman</a></span><span> </span><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 26 Nov 2024, 6:09 pm Laurent Pinchart, <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">CC'ing Naush<br>
<br>
On Tue, Nov 26, 2024 at 08:08:59PM +0200, Laurent Pinchart wrote:<br>
> Hi Barnabás,<br>
> <br>
> Thank you for the patch.<br>
> <br>
> On Tue, Nov 26, 2024 at 06:03:05PM +0000, Barnabás Pőcze wrote:<br>
> > A copy is made in the range-based for loop, and thus all modifications<br>
> > done in the for loop body are lost, and not actually applied to<br>
> > the object in the container.<br>
> > <br>
> > Fix that by taking a reference in the range-based for loop.<br>
> > <br>
> > Fixes: 4217c9f1aa863c ("libcamera: camera: Zero streams before validate()")<br>
> > Fixes: 613d5402673eb9 ("pipeline: raspberrypi: Fix handling of colour spaces")<br>
> > Signed-off-by: Barnabás Pőcze <<a href="mailto:pobrn@protonmail.com" target="_blank" rel="noreferrer">pobrn@protonmail.com</a>><br>
> <br>
> Reviewed-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank" rel="noreferrer">laurent.pinchart@ideasonboard.com</a>><br>
> <br>
> Naush, it looks like you may have been missing some tests related to<br>
> colour space handling. Could you double-check that this change doesn't<br>
> break anything ? It could be that fixing a bug would uncover another<br>
> one.<br>
> <br>
> > ---<br>
> >  src/libcamera/camera.cpp                            | 4 ++--<br>
> >  src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 4 ++--<br>
> >  2 files changed, 4 insertions(+), 4 deletions(-)<br>
> > <br>
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp<br>
> > index 7507e9dd..4c865a46 100644<br>
> > --- a/src/libcamera/camera.cpp<br>
> > +++ b/src/libcamera/camera.cpp<br>
> > @@ -1178,8 +1178,8 @@ int Camera::configure(CameraConfiguration *config)<br>
> >     if (ret < 0)<br>
> >             return ret;<br>
> >  <br>
> > -   for (auto it : *config)<br>
> > -           it.setStream(nullptr);<br>
> > +   for (auto &cfg : *config)<br>
> > +           cfg.setStream(nullptr);<br>
> >  <br>
> >     if (config->validate() != CameraConfiguration::Valid) {<br>
> >             LOG(Camera, Error)<br>
> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp<br>
> > index 9e2d9d23..6f278b29 100644<br>
> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp<br>
> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp<br>
> > @@ -105,7 +105,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validateColorSpaces([[maybe_<br>
> >     Status status = Valid;<br>
> >     yuvColorSpace_.reset();<br>
> >  <br>
> > -   for (auto cfg : config_) {<br>
> > +   for (auto &cfg : config_) {<br>
> >             /* First fix up raw streams to have the "raw" colour space. */<br>
> >             if (PipelineHandlerBase::isRaw(cfg.pixelFormat)) {<br>
> >                     /* If there was no value here, that doesn't count as "adjusted". */<br>
> > @@ -130,7 +130,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validateColorSpaces([[maybe_<br>
> >     rgbColorSpace_->range = ColorSpace::Range::Full;<br>
> >  <br>
> >     /* Go through the streams again and force everyone to the same colour space. */<br>
> > -   for (auto cfg : config_) {<br>
> > +   for (auto &cfg : config_) {<br>
> >             if (cfg.colorSpace == ColorSpace::Raw)<br>
> >                     continue;<br>
> >  <br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div></div>