[PATCH v1] libcamera: Don't copy `StreamConfiguration` when iterating

[Naushir Patuck]

Hi Barnabás,

Thank you for this fix.

On Tue, 26 Nov 2024 at 18:03, Barnabás Pőcze <pobrn at protonmail.com> wrote:
>
> A copy is made in the range-based for loop, and thus all modifications
> done in the for loop body are lost, and not actually applied to
> the object in the container.
>
> Fix that by taking a reference in the range-based for loop.
>
> Fixes: 4217c9f1aa863c ("libcamera: camera: Zero streams before validate()")
> Fixes: 613d5402673eb9 ("pipeline: raspberrypi: Fix handling of colour spaces")
> Signed-off-by: Barnabás Pőcze <pobrn at protonmail.com>

I've run this change through our regression testing and it passes.  I
was expecting a failure with this change, but it turns out we have CS
validation code in Picamera2 that avoids us hitting the below
adjustment code.

Reviewed-by: Naushir Patuck <naush at raspberrypi.com>

> ---
>  src/libcamera/camera.cpp                            | 4 ++--
>  src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 7507e9dd..4c865a46 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -1178,8 +1178,8 @@ int Camera::configure(CameraConfiguration *config)
>         if (ret < 0)
>                 return ret;
>
> -       for (auto it : *config)
> -               it.setStream(nullptr);
> +       for (auto &cfg : *config)
> +               cfg.setStream(nullptr);
>
>         if (config->validate() != CameraConfiguration::Valid) {
>                 LOG(Camera, Error)
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index 9e2d9d23..6f278b29 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -105,7 +105,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validateColorSpaces([[maybe_
>         Status status = Valid;
>         yuvColorSpace_.reset();
>
> -       for (auto cfg : config_) {
> +       for (auto &cfg : config_) {
>                 /* First fix up raw streams to have the "raw" colour space. */
>                 if (PipelineHandlerBase::isRaw(cfg.pixelFormat)) {
>                         /* If there was no value here, that doesn't count as "adjusted". */
> @@ -130,7 +130,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validateColorSpaces([[maybe_
>         rgbColorSpace_->range = ColorSpace::Range::Full;
>
>         /* Go through the streams again and force everyone to the same colour space. */
> -       for (auto cfg : config_) {
> +       for (auto &cfg : config_) {
>                 if (cfg.colorSpace == ColorSpace::Raw)
>                         continue;
>
> --
> 2.47.1
>
>