| Message ID | 20250925192856.77881-2-mzamazal@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
2025. 09. 25. 21:28 keltezéssel, Milan Zamazal írta: > SwStatsCpu::setWindow reduces the window width by the added x-offset, to > prevent exceeding image bounds. But if the window width is smaller than > the x-offset, we get unsigned integer underflow. Fix it by setting the > window width to 0 in such a case. > > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/libcamera/software_isp/swstats_cpu.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp > index 4b77b3600..e8a1d52f2 100644 > --- a/src/libcamera/software_isp/swstats_cpu.cpp > +++ b/src/libcamera/software_isp/swstats_cpu.cpp > @@ -426,7 +426,7 @@ void SwStatsCpu::setWindow(const Rectangle &window) > window_.y &= ~(patternSize_.height - 1); > > /* width_ - xShift_ to make sure the window fits */ > - window_.width -= xShift_; > + window_.width = (window_.width > xShift_ ? window_.width - xShift_ : 0); The `window_.width == 0` check in `SwStatsCpu::startFrame()` will now cause a bit of a misleading message to be logged, but that is still better than going ahead with the wrapped around width. Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > window_.width &= ~(patternSize_.width - 1); > window_.height &= ~(patternSize_.height - 1); > }
Hi Milan, On 25-Sep-25 21:28, Milan Zamazal wrote: > SwStatsCpu::setWindow reduces the window width by the added x-offset, to > prevent exceeding image bounds. But if the window width is smaller than > the x-offset, we get unsigned integer underflow. Fix it by setting the > window width to 0 in such a case. > > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/libcamera/software_isp/swstats_cpu.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp > index 4b77b3600..e8a1d52f2 100644 > --- a/src/libcamera/software_isp/swstats_cpu.cpp > +++ b/src/libcamera/software_isp/swstats_cpu.cpp > @@ -426,7 +426,7 @@ void SwStatsCpu::setWindow(const Rectangle &window) > window_.y &= ~(patternSize_.height - 1); > > /* width_ - xShift_ to make sure the window fits */ > - window_.width -= xShift_; > + window_.width = (window_.width > xShift_ ? window_.width - xShift_ : 0); > window_.width &= ~(patternSize_.width - 1); > window_.height &= ~(patternSize_.height - 1); > } At the moment window_.width equals the debayer outputsize which is at least 2x2 (*), and xShift_ is 0 or 1, so this is not strictly necessary. Still I think this is a good thing to have with the upcoming changes to use different stat-windows: Reviewed-by: Hans de Goede <hansg@kernel.org> Regards, Hans *) see sizes() method in debayer_cpu.cpp, this sets a min-outputsize of patternSize-width x patternSize-height which is a minimum of 2x2, this is enforced by DebayerCpu::configure()
diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp index 4b77b3600..e8a1d52f2 100644 --- a/src/libcamera/software_isp/swstats_cpu.cpp +++ b/src/libcamera/software_isp/swstats_cpu.cpp @@ -426,7 +426,7 @@ void SwStatsCpu::setWindow(const Rectangle &window) window_.y &= ~(patternSize_.height - 1); /* width_ - xShift_ to make sure the window fits */ - window_.width -= xShift_; + window_.width = (window_.width > xShift_ ? window_.width - xShift_ : 0); window_.width &= ~(patternSize_.width - 1); window_.height &= ~(patternSize_.height - 1); }
SwStatsCpu::setWindow reduces the window width by the added x-offset, to prevent exceeding image bounds. But if the window width is smaller than the x-offset, we get unsigned integer underflow. Fix it by setting the window width to 0 in such a case. Signed-off-by: Milan Zamazal <mzamazal@redhat.com> --- src/libcamera/software_isp/swstats_cpu.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)