rpi: pipeline_base: print old color space in before updating
diff mbox series

Message ID 20260604090044.3536188-1-tju_cooyun@163.com
State New
Headers show
Series
  • rpi: pipeline_base: print old color space in before updating
Related show

Commit Message

tju_cooyun@163.com June 4, 2026, 9 a.m. UTC 
From: zcy <tju_cooyun@163.com>

PipelineHandlerBase::updateStreamConfig now saves the previous
stream color space before assignment when a color space change
is detected. This ensures the debug log reports the transition
as 'from <old> to <new>' instead of showing the new value twice.

Signed-off-by: zcy <tju_cooyun@163.com>
---
 src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Barnabás Pőcze June 4, 2026, 10:25 a.m. UTC | #1 
2026. 06. 04. 11:00 keltezéssel, tju_cooyun@163.com írta:
> From: zcy <tju_cooyun@163.com>
> 
> PipelineHandlerBase::updateStreamConfig now saves the previous
> stream color space before assignment when a color space change
> is detected. This ensures the debug log reports the transition
> as 'from <old> to <new>' instead of showing the new value twice.
> 
> Signed-off-by: zcy <tju_cooyun@163.com>
> ---

Looks ok to me. I think this can have

   Fixes: 71bbf10a2f79 ("libcamera: rpi: Add some helpers to PipelineHandlerBase")

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>


>   src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index 263a48387..5a5acf6a1 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -342,12 +342,12 @@ bool PipelineHandlerBase::updateStreamConfig(StreamConfiguration *stream,
>   	}
> 
>   	if (stream->colorSpace != format.colorSpace) {
> -		stream->colorSpace = format.colorSpace;
> -		adjusted = true;
>   		LOG(RPI, Debug)
>   			<< "Color space changed from "
>   			<< ColorSpace::toString(stream->colorSpace) << " to "
>   			<< ColorSpace::toString(format.colorSpace);
> +		stream->colorSpace = format.colorSpace;
> +		adjusted = true;
>   	}
> 
>   	stream->stride = format.planes[0].bpl;
> --
> 2.43.0
>
David Plowman June 4, 2026, 10:27 a.m. UTC | #2 
Hi

Thanks for the patch. Yes, I think that's clearly what was intended!

On Thu, 4 Jun 2026 at 11:25, Barnabás Pőcze
<barnabas.pocze@ideasonboard.com> wrote:
>
> 2026. 06. 04. 11:00 keltezéssel, tju_cooyun@163.com írta:
> > From: zcy <tju_cooyun@163.com>
> >
> > PipelineHandlerBase::updateStreamConfig now saves the previous
> > stream color space before assignment when a color space change
> > is detected. This ensures the debug log reports the transition
> > as 'from <old> to <new>' instead of showing the new value twice.
> >
> > Signed-off-by: zcy <tju_cooyun@163.com>
> > ---
>
> Looks ok to me. I think this can have
>
>    Fixes: 71bbf10a2f79 ("libcamera: rpi: Add some helpers to PipelineHandlerBase")
>
> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

Best regards

David

>
>
> >   src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > index 263a48387..5a5acf6a1 100644
> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > @@ -342,12 +342,12 @@ bool PipelineHandlerBase::updateStreamConfig(StreamConfiguration *stream,
> >       }
> >
> >       if (stream->colorSpace != format.colorSpace) {
> > -             stream->colorSpace = format.colorSpace;
> > -             adjusted = true;
> >               LOG(RPI, Debug)
> >                       << "Color space changed from "
> >                       << ColorSpace::toString(stream->colorSpace) << " to "
> >                       << ColorSpace::toString(format.colorSpace);
> > +             stream->colorSpace = format.colorSpace;
> > +             adjusted = true;
> >       }
> >
> >       stream->stride = format.planes[0].bpl;
> > --
> > 2.43.0
> >
>
Barnabás Pőcze June 4, 2026, 10:28 a.m. UTC | #3 
2026. 06. 04. 12:25 keltezéssel, Barnabás Pőcze írta:
> 2026. 06. 04. 11:00 keltezéssel, tju_cooyun@163.com írta:
>> From: zcy <tju_cooyun@163.com>
>>
>> PipelineHandlerBase::updateStreamConfig now saves the previous
>> stream color space before assignment when a color space change
>> is detected. This ensures the debug log reports the transition
>> as 'from <old> to <new>' instead of showing the new value twice.
>>
>> Signed-off-by: zcy <tju_cooyun@163.com>
>> ---
> 
> Looks ok to me. I think this can have
> 
>     Fixes: 71bbf10a2f79 ("libcamera: rpi: Add some helpers to PipelineHandlerBase")

Additionally, the subject prefix should probably be

   pipeline: rpi:

instead of

   rpi: pipeline_base:

with the first letter of the succeeding text capitalized, e.g:

   pipeline: rpi: Print old color space before updating

> 
> Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> 
> 
>>    src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 4 ++--
>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
>> index 263a48387..5a5acf6a1 100644
>> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
>> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
>> @@ -342,12 +342,12 @@ bool PipelineHandlerBase::updateStreamConfig(StreamConfiguration *stream,
>>    	}
>>
>>    	if (stream->colorSpace != format.colorSpace) {
>> -		stream->colorSpace = format.colorSpace;
>> -		adjusted = true;
>>    		LOG(RPI, Debug)
>>    			<< "Color space changed from "
>>    			<< ColorSpace::toString(stream->colorSpace) << " to "
>>    			<< ColorSpace::toString(format.colorSpace);
>> +		stream->colorSpace = format.colorSpace;
>> +		adjusted = true;
>>    	}
>>
>>    	stream->stride = format.planes[0].bpl;
>> --
>> 2.43.0
>>
>
Kieran Bingham June 4, 2026, 10:32 a.m. UTC | #4 
Quoting Barnabás Pőcze (2026-06-04 11:28:14)
> 2026. 06. 04. 12:25 keltezéssel, Barnabás Pőcze írta:
> > 2026. 06. 04. 11:00 keltezéssel, tju_cooyun@163.com írta:
> >> From: zcy <tju_cooyun@163.com>
> >>
> >> PipelineHandlerBase::updateStreamConfig now saves the previous
> >> stream color space before assignment when a color space change
> >> is detected. This ensures the debug log reports the transition
> >> as 'from <old> to <new>' instead of showing the new value twice.
> >>
> >> Signed-off-by: zcy <tju_cooyun@163.com>
> >> ---
> > 
> > Looks ok to me. I think this can have
> > 
> >     Fixes: 71bbf10a2f79 ("libcamera: rpi: Add some helpers to PipelineHandlerBase")
> 
> Additionally, the subject prefix should probably be
> 
>    pipeline: rpi:
> 
> instead of
> 
>    rpi: pipeline_base:
> 
> with the first letter of the succeeding text capitalized, e.g:
> 
>    pipeline: rpi: Print old color space before updating

I'll handle all that while applying.

--
Thanks

Kieran


> 
> > 
> > Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> > 
> > 
> >>    src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 4 ++--
> >>    1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> >> index 263a48387..5a5acf6a1 100644
> >> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> >> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> >> @@ -342,12 +342,12 @@ bool PipelineHandlerBase::updateStreamConfig(StreamConfiguration *stream,
> >>      }
> >>
> >>      if (stream->colorSpace != format.colorSpace) {
> >> -            stream->colorSpace = format.colorSpace;
> >> -            adjusted = true;
> >>              LOG(RPI, Debug)
> >>                      << "Color space changed from "
> >>                      << ColorSpace::toString(stream->colorSpace) << " to "
> >>                      << ColorSpace::toString(format.colorSpace);
> >> +            stream->colorSpace = format.colorSpace;
> >> +            adjusted = true;
> >>      }
> >>
> >>      stream->stride = format.planes[0].bpl;
> >> --
> >> 2.43.0
> >>
> > 
>

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
index 263a48387..5a5acf6a1 100644
--- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
+++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
@@ -342,12 +342,12 @@  bool PipelineHandlerBase::updateStreamConfig(StreamConfiguration *stream,
 	}
 
 	if (stream->colorSpace != format.colorSpace) {
-		stream->colorSpace = format.colorSpace;
-		adjusted = true;
 		LOG(RPI, Debug)
 			<< "Color space changed from "
 			<< ColorSpace::toString(stream->colorSpace) << " to "
 			<< ColorSpace::toString(format.colorSpace);
+		stream->colorSpace = format.colorSpace;
+		adjusted = true;
 	}
 
 	stream->stride = format.planes[0].bpl;