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

[Laurent Pinchart]

Hi Barnabás,

Thank you for the patch.

On Tue, Nov 26, 2024 at 06:03:05PM +0000, Barnabás Pőcze 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>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Naush, it looks like you may have been missing some tests related to
colour space handling. Could you double-check that this change doesn't
break anything ? It could be that fixing a bug would uncover another
one.

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

-- 
Regards,

Laurent Pinchart