libcamera: software_isp: Remove type casts in statistics computation
diff mbox series

Message ID 20250718144444.58573-1-mzamazal@redhat.com
State Accepted
Commit cc137b1c6d5dbfc5365f2eae4e53dab9ca134224
Headers show
Series
  • libcamera: software_isp: Remove type casts in statistics computation
Related show

Commit Message

Milan Zamazal July 18, 2025, 2:44 p.m. UTC 
Type casting from unsigned int to int performed in stats computation is
unnecessary, window_.width is unsigned and the array index is always
non-negative.  Let's simply use unsigned int in all the
SwStatsCpu:stats* methods.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 src/libcamera/software_isp/swstats_cpu.cpp | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Umang Jain July 21, 2025, 4:30 a.m. UTC | #1 
On Fri, Jul 18, 2025 at 04:44:44PM +0200, Milan Zamazal wrote:
> Type casting from unsigned int to int performed in stats computation is
> unnecessary, window_.width is unsigned and the array index is always
> non-negative.  Let's simply use unsigned int in all the
> SwStatsCpu:stats* methods.
> 
> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>

Reviewed-by: Umang Jain <uajain@igalia.com>

> ---
>  src/libcamera/software_isp/swstats_cpu.cpp | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp
> index c520c806e..4b77b3600 100644
> --- a/src/libcamera/software_isp/swstats_cpu.cpp
> +++ b/src/libcamera/software_isp/swstats_cpu.cpp
> @@ -175,7 +175,7 @@ void SwStatsCpu::statsBGGR8Line0(const uint8_t *src[])
>  		std::swap(src0, src1);
>  
>  	/* x += 4 sample every other 2x2 block */
> -	for (int x = 0; x < (int)window_.width; x += 4) {
> +	for (unsigned int x = 0; x < window_.width; x += 4) {
>  		b = src0[x];
>  		g = src0[x + 1];
>  		g2 = src1[x];
> @@ -200,7 +200,7 @@ void SwStatsCpu::statsBGGR10Line0(const uint8_t *src[])
>  		std::swap(src0, src1);
>  
>  	/* x += 4 sample every other 2x2 block */
> -	for (int x = 0; x < (int)window_.width; x += 4) {
> +	for (unsigned int x = 0; x < window_.width; x += 4) {
>  		b = src0[x];
>  		g = src0[x + 1];
>  		g2 = src1[x];
> @@ -226,7 +226,7 @@ void SwStatsCpu::statsBGGR12Line0(const uint8_t *src[])
>  		std::swap(src0, src1);
>  
>  	/* x += 4 sample every other 2x2 block */
> -	for (int x = 0; x < (int)window_.width; x += 4) {
> +	for (unsigned int x = 0; x < window_.width; x += 4) {
>  		b = src0[x];
>  		g = src0[x + 1];
>  		g2 = src1[x];
> @@ -245,7 +245,7 @@ void SwStatsCpu::statsBGGR10PLine0(const uint8_t *src[])
>  {
>  	const uint8_t *src0 = src[1] + window_.x * 5 / 4;
>  	const uint8_t *src1 = src[2] + window_.x * 5 / 4;
> -	const int widthInBytes = window_.width * 5 / 4;
> +	const unsigned int widthInBytes = window_.width * 5 / 4;
>  
>  	if (swapLines_)
>  		std::swap(src0, src1);
> @@ -253,7 +253,7 @@ void SwStatsCpu::statsBGGR10PLine0(const uint8_t *src[])
>  	SWSTATS_START_LINE_STATS(uint8_t)
>  
>  	/* x += 5 sample every other 2x2 block */
> -	for (int x = 0; x < widthInBytes; x += 5) {
> +	for (unsigned int x = 0; x < widthInBytes; x += 5) {
>  		/* BGGR */
>  		b = src0[x];
>  		g = src0[x + 1];
> @@ -271,7 +271,7 @@ void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])
>  {
>  	const uint8_t *src0 = src[1] + window_.x * 5 / 4;
>  	const uint8_t *src1 = src[2] + window_.x * 5 / 4;
> -	const int widthInBytes = window_.width * 5 / 4;
> +	const unsigned int widthInBytes = window_.width * 5 / 4;
>  
>  	if (swapLines_)
>  		std::swap(src0, src1);
> @@ -279,7 +279,7 @@ void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])
>  	SWSTATS_START_LINE_STATS(uint8_t)
>  
>  	/* x += 5 sample every other 2x2 block */
> -	for (int x = 0; x < widthInBytes; x += 5) {
> +	for (unsigned int x = 0; x < widthInBytes; x += 5) {
>  		/* GBRG */
>  		g = src0[x];
>  		b = src0[x + 1];
> -- 
> 2.50.1
>
Kieran Bingham July 21, 2025, 11:26 a.m. UTC | #2 
Quoting Umang Jain (2025-07-21 05:30:21)
> On Fri, Jul 18, 2025 at 04:44:44PM +0200, Milan Zamazal wrote:
> > Type casting from unsigned int to int performed in stats computation is
> > unnecessary, window_.width is unsigned and the array index is always
> > non-negative.  Let's simply use unsigned int in all the
> > SwStatsCpu:stats* methods.
> > 
> > Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> 
> Reviewed-by: Umang Jain <uajain@igalia.com>
> 

https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1473924 is
green, so merging.


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


> > ---
> >  src/libcamera/software_isp/swstats_cpu.cpp | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> > 
> > diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp
> > index c520c806e..4b77b3600 100644
> > --- a/src/libcamera/software_isp/swstats_cpu.cpp
> > +++ b/src/libcamera/software_isp/swstats_cpu.cpp
> > @@ -175,7 +175,7 @@ void SwStatsCpu::statsBGGR8Line0(const uint8_t *src[])
> >               std::swap(src0, src1);
> >  
> >       /* x += 4 sample every other 2x2 block */
> > -     for (int x = 0; x < (int)window_.width; x += 4) {
> > +     for (unsigned int x = 0; x < window_.width; x += 4) {
> >               b = src0[x];
> >               g = src0[x + 1];
> >               g2 = src1[x];
> > @@ -200,7 +200,7 @@ void SwStatsCpu::statsBGGR10Line0(const uint8_t *src[])
> >               std::swap(src0, src1);
> >  
> >       /* x += 4 sample every other 2x2 block */
> > -     for (int x = 0; x < (int)window_.width; x += 4) {
> > +     for (unsigned int x = 0; x < window_.width; x += 4) {
> >               b = src0[x];
> >               g = src0[x + 1];
> >               g2 = src1[x];
> > @@ -226,7 +226,7 @@ void SwStatsCpu::statsBGGR12Line0(const uint8_t *src[])
> >               std::swap(src0, src1);
> >  
> >       /* x += 4 sample every other 2x2 block */
> > -     for (int x = 0; x < (int)window_.width; x += 4) {
> > +     for (unsigned int x = 0; x < window_.width; x += 4) {
> >               b = src0[x];
> >               g = src0[x + 1];
> >               g2 = src1[x];
> > @@ -245,7 +245,7 @@ void SwStatsCpu::statsBGGR10PLine0(const uint8_t *src[])
> >  {
> >       const uint8_t *src0 = src[1] + window_.x * 5 / 4;
> >       const uint8_t *src1 = src[2] + window_.x * 5 / 4;
> > -     const int widthInBytes = window_.width * 5 / 4;
> > +     const unsigned int widthInBytes = window_.width * 5 / 4;
> >  
> >       if (swapLines_)
> >               std::swap(src0, src1);
> > @@ -253,7 +253,7 @@ void SwStatsCpu::statsBGGR10PLine0(const uint8_t *src[])
> >       SWSTATS_START_LINE_STATS(uint8_t)
> >  
> >       /* x += 5 sample every other 2x2 block */
> > -     for (int x = 0; x < widthInBytes; x += 5) {
> > +     for (unsigned int x = 0; x < widthInBytes; x += 5) {
> >               /* BGGR */
> >               b = src0[x];
> >               g = src0[x + 1];
> > @@ -271,7 +271,7 @@ void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])
> >  {
> >       const uint8_t *src0 = src[1] + window_.x * 5 / 4;
> >       const uint8_t *src1 = src[2] + window_.x * 5 / 4;
> > -     const int widthInBytes = window_.width * 5 / 4;
> > +     const unsigned int widthInBytes = window_.width * 5 / 4;
> >  
> >       if (swapLines_)
> >               std::swap(src0, src1);
> > @@ -279,7 +279,7 @@ void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])
> >       SWSTATS_START_LINE_STATS(uint8_t)
> >  
> >       /* x += 5 sample every other 2x2 block */
> > -     for (int x = 0; x < widthInBytes; x += 5) {
> > +     for (unsigned int x = 0; x < widthInBytes; x += 5) {
> >               /* GBRG */
> >               g = src0[x];
> >               b = src0[x + 1];
> > -- 
> > 2.50.1
> >

Patch
diff mbox series 

diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp
index c520c806e..4b77b3600 100644
--- a/src/libcamera/software_isp/swstats_cpu.cpp
+++ b/src/libcamera/software_isp/swstats_cpu.cpp
@@ -175,7 +175,7 @@  void SwStatsCpu::statsBGGR8Line0(const uint8_t *src[])
 		std::swap(src0, src1);
 
 	/* x += 4 sample every other 2x2 block */
-	for (int x = 0; x < (int)window_.width; x += 4) {
+	for (unsigned int x = 0; x < window_.width; x += 4) {
 		b = src0[x];
 		g = src0[x + 1];
 		g2 = src1[x];
@@ -200,7 +200,7 @@  void SwStatsCpu::statsBGGR10Line0(const uint8_t *src[])
 		std::swap(src0, src1);
 
 	/* x += 4 sample every other 2x2 block */
-	for (int x = 0; x < (int)window_.width; x += 4) {
+	for (unsigned int x = 0; x < window_.width; x += 4) {
 		b = src0[x];
 		g = src0[x + 1];
 		g2 = src1[x];
@@ -226,7 +226,7 @@  void SwStatsCpu::statsBGGR12Line0(const uint8_t *src[])
 		std::swap(src0, src1);
 
 	/* x += 4 sample every other 2x2 block */
-	for (int x = 0; x < (int)window_.width; x += 4) {
+	for (unsigned int x = 0; x < window_.width; x += 4) {
 		b = src0[x];
 		g = src0[x + 1];
 		g2 = src1[x];
@@ -245,7 +245,7 @@  void SwStatsCpu::statsBGGR10PLine0(const uint8_t *src[])
 {
 	const uint8_t *src0 = src[1] + window_.x * 5 / 4;
 	const uint8_t *src1 = src[2] + window_.x * 5 / 4;
-	const int widthInBytes = window_.width * 5 / 4;
+	const unsigned int widthInBytes = window_.width * 5 / 4;
 
 	if (swapLines_)
 		std::swap(src0, src1);
@@ -253,7 +253,7 @@  void SwStatsCpu::statsBGGR10PLine0(const uint8_t *src[])
 	SWSTATS_START_LINE_STATS(uint8_t)
 
 	/* x += 5 sample every other 2x2 block */
-	for (int x = 0; x < widthInBytes; x += 5) {
+	for (unsigned int x = 0; x < widthInBytes; x += 5) {
 		/* BGGR */
 		b = src0[x];
 		g = src0[x + 1];
@@ -271,7 +271,7 @@  void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])
 {
 	const uint8_t *src0 = src[1] + window_.x * 5 / 4;
 	const uint8_t *src1 = src[2] + window_.x * 5 / 4;
-	const int widthInBytes = window_.width * 5 / 4;
+	const unsigned int widthInBytes = window_.width * 5 / 4;
 
 	if (swapLines_)
 		std::swap(src0, src1);
@@ -279,7 +279,7 @@  void SwStatsCpu::statsGBRG10PLine0(const uint8_t *src[])
 	SWSTATS_START_LINE_STATS(uint8_t)
 
 	/* x += 5 sample every other 2x2 block */
-	for (int x = 0; x < widthInBytes; x += 5) {
+	for (unsigned int x = 0; x < widthInBytes; x += 5) {
 		/* GBRG */
 		g = src0[x];
 		b = src0[x + 1];