| Message ID | 20251127211932.122463-9-mzamazal@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
On 2025-11-28 02:49, Milan Zamazal wrote: > The log message about adjusting an unspecified colour space should log > the value after, not before, the adjustment. > > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> Should this include a Fixes: tag? Reviewed-by: Umang Jain <uajain@igalia.com> > --- > src/libcamera/pipeline/simple/simple.cpp | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index cbfc06fb6..2820d1254 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -1305,9 +1305,6 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() > 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 > @@ -1315,6 +1312,9 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() > * to Adjusted. > */ > cfg.colorSpace->adjust(cfg.pixelFormat); > + LOG(SimplePipeline, Debug) > + << "Unspecified color space set to " > + << cfg.colorSpace.value().toString(); > } else { > if (cfg.colorSpace->adjust(cfg.pixelFormat)) { > LOG(SimplePipeline, Debug)
2025. 11. 27. 22:19 keltezéssel, Milan Zamazal írta: > The log message about adjusting an unspecified colour space should log > the value after, not before, the adjustment. > > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > --- Fixes: 5a33bc10e9d3bc ("libcamera: software_isp: Assign colour spaces in configurations") Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > src/libcamera/pipeline/simple/simple.cpp | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index cbfc06fb6..2820d1254 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -1305,9 +1305,6 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() > 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 > @@ -1315,6 +1312,9 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() > * to Adjusted. > */ > cfg.colorSpace->adjust(cfg.pixelFormat); > + LOG(SimplePipeline, Debug) > + << "Unspecified color space set to " > + << cfg.colorSpace.value().toString(); > } else { > if (cfg.colorSpace->adjust(cfg.pixelFormat)) { > LOG(SimplePipeline, Debug)
Hi Barnabás, thank you for review. Barnabás Pőcze <barnabas.pocze@ideasonboard.com> writes: > 2025. 11. 27. 22:19 keltezéssel, Milan Zamazal írta: >> The log message about adjusting an unspecified colour space should log >> the value after, not before, the adjustment. >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com> >> --- > > Fixes: 5a33bc10e9d3bc ("libcamera: software_isp: Assign colour spaces in configurations") Thank you for adding this, posted the patch now with this, as a separate patch. > Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > > > >> src/libcamera/pipeline/simple/simple.cpp | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp >> index cbfc06fb6..2820d1254 100644 >> --- a/src/libcamera/pipeline/simple/simple.cpp >> +++ b/src/libcamera/pipeline/simple/simple.cpp >> @@ -1305,9 +1305,6 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() >> 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 >> @@ -1315,6 +1312,9 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() >> * to Adjusted. >> */ >> cfg.colorSpace->adjust(cfg.pixelFormat); >> + LOG(SimplePipeline, Debug) >> + << "Unspecified color space set to " >> + << cfg.colorSpace.value().toString(); >> } else { >> if (cfg.colorSpace->adjust(cfg.pixelFormat)) { >> LOG(SimplePipeline, Debug)
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index cbfc06fb6..2820d1254 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -1305,9 +1305,6 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() 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 @@ -1315,6 +1312,9 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() * to Adjusted. */ cfg.colorSpace->adjust(cfg.pixelFormat); + LOG(SimplePipeline, Debug) + << "Unspecified color space set to " + << cfg.colorSpace.value().toString(); } else { if (cfg.colorSpace->adjust(cfg.pixelFormat)) { LOG(SimplePipeline, Debug)
The log message about adjusting an unspecified colour space should log the value after, not before, the adjustment. Signed-off-by: Milan Zamazal <mzamazal@redhat.com> --- src/libcamera/pipeline/simple/simple.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)