libcamera: Replace usage of lroundf() with std::lround()
diff mbox series

Message ID 20240926002335.27509-1-laurent.pinchart@ideasonboard.com
State Accepted
Commit fa2dfd55cc9e86589e505f9edc82a94b57405677
Headers show
Series
  • libcamera: Replace usage of lroundf() with std::lround()
Related show

Commit Message

Laurent Pinchart Sept. 26, 2024, 12:23 a.m. UTC 
As explained in the coding style document, usage of std::lround() is
preferred over lroundf() as it picks the correct function based on
the argument type. Replace calls to lroundf() with std::lround() through
libcamera.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/guides/pipeline-handler.rst    | 4 ++--
 src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 8 ++++----
 src/libcamera/pipeline/vimc/vimc.cpp         | 4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)


base-commit: f2088eb91fd6477b152233b9031cb115ca1ae824

Comments

Kieran Bingham Sept. 26, 2024, 8:13 a.m. UTC | #1 
Quoting Laurent Pinchart (2024-09-26 01:23:35)
> As explained in the coding style document, usage of std::lround() is
> preferred over lroundf() as it picks the correct function based on
> the argument type. Replace calls to lroundf() with std::lround() through
> libcamera.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Sounds fine to me.


> ---
>  Documentation/guides/pipeline-handler.rst    | 4 ++--
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 8 ++++----
>  src/libcamera/pipeline/vimc/vimc.cpp         | 4 ++--
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst
> index 26aea43341d4..69e832a5587e 100644
> --- a/Documentation/guides/pipeline-handler.rst
> +++ b/Documentation/guides/pipeline-handler.rst
> @@ -1350,7 +1350,7 @@ before being set.
>                          continue;
>                   }
>  
> -                 int32_t value = lroundf(it.second.get<float>() * 128 + offset);
> +                 int32_t value = std::lround(it.second.get<float>() * 128 + offset);
>                   controls.set(cid, std::clamp(value, 0, 255));
>            }
>  
> @@ -1414,7 +1414,7 @@ value translation operations:
>  
>  .. code-block:: cpp
>  
> -   #include <math.h>
> +   #include <cmath>

It looks like we have quite a few math.h usages throughout elsewhere. Should they
all get changed too ?

Perhaps they'll get changed in time.


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

>  
>  Frame completion and event handling
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 6b32fa18799b..7fa01bb71744 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -6,9 +6,9 @@
>   */
>  
>  #include <algorithm>
> +#include <cmath>
>  #include <fstream>
>  #include <map>
> -#include <math.h>
>  #include <memory>
>  #include <set>
>  #include <string>
> @@ -320,14 +320,14 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
>         case V4L2_CID_BRIGHTNESS: {
>                 float scale = std::max(max - def, def - min);
>                 float fvalue = value.get<float>() * scale + def;
> -               controls->set(cid, static_cast<int32_t>(lroundf(fvalue)));
> +               controls->set(cid, static_cast<int32_t>(std::lround(fvalue)));
>                 break;
>         }
>  
>         case V4L2_CID_SATURATION: {
>                 float scale = def - min;
>                 float fvalue = value.get<float>() * scale + min;
> -               controls->set(cid, static_cast<int32_t>(lroundf(fvalue)));
> +               controls->set(cid, static_cast<int32_t>(std::lround(fvalue)));
>                 break;
>         }
>  
> @@ -354,7 +354,7 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
>                 }
>  
>                 float fvalue = (value.get<float>() - p) / m;
> -               controls->set(cid, static_cast<int32_t>(lroundf(fvalue)));
> +               controls->set(cid, static_cast<int32_t>(std::lround(fvalue)));
>                 break;
>         }
>  
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index 325174b90087..2165bae890cb 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -6,9 +6,9 @@
>   */
>  
>  #include <algorithm>
> +#include <cmath>
>  #include <iomanip>
>  #include <map>
> -#include <math.h>
>  #include <tuple>
>  
>  #include <linux/media-bus-format.h>
> @@ -420,7 +420,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
>                         continue;
>                 }
>  
> -               int32_t value = lroundf(it.second.get<float>() * 128 + offset);
> +               int32_t value = std::lround(it.second.get<float>() * 128 + offset);
>                 controls.set(cid, std::clamp(value, 0, 255));
>         }
>  
> 
> base-commit: f2088eb91fd6477b152233b9031cb115ca1ae824
> -- 
> Regards,
> 
> Laurent Pinchart
>
Laurent Pinchart Sept. 26, 2024, 9:27 a.m. UTC | #2 
On Thu, Sep 26, 2024 at 09:13:37AM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2024-09-26 01:23:35)
> > As explained in the coding style document, usage of std::lround() is
> > preferred over lroundf() as it picks the correct function based on
> > the argument type. Replace calls to lroundf() with std::lround() through
> > libcamera.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Sounds fine to me.
> 
> > ---
> >  Documentation/guides/pipeline-handler.rst    | 4 ++--
> >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 8 ++++----
> >  src/libcamera/pipeline/vimc/vimc.cpp         | 4 ++--
> >  3 files changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst
> > index 26aea43341d4..69e832a5587e 100644
> > --- a/Documentation/guides/pipeline-handler.rst
> > +++ b/Documentation/guides/pipeline-handler.rst
> > @@ -1350,7 +1350,7 @@ before being set.
> >                          continue;
> >                   }
> >  
> > -                 int32_t value = lroundf(it.second.get<float>() * 128 + offset);
> > +                 int32_t value = std::lround(it.second.get<float>() * 128 + offset);
> >                   controls.set(cid, std::clamp(value, 0, 255));
> >            }
> >  
> > @@ -1414,7 +1414,7 @@ value translation operations:
> >  
> >  .. code-block:: cpp
> >  
> > -   #include <math.h>
> > +   #include <cmath>
> 
> It looks like we have quite a few math.h usages throughout elsewhere. Should they
> all get changed too ?

I'm on it.

> Perhaps they'll get changed in time.
> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> >  
> >  Frame completion and event handling
> >  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > index 6b32fa18799b..7fa01bb71744 100644
> > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > @@ -6,9 +6,9 @@
> >   */
> >  
> >  #include <algorithm>
> > +#include <cmath>
> >  #include <fstream>
> >  #include <map>
> > -#include <math.h>
> >  #include <memory>
> >  #include <set>
> >  #include <string>
> > @@ -320,14 +320,14 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
> >         case V4L2_CID_BRIGHTNESS: {
> >                 float scale = std::max(max - def, def - min);
> >                 float fvalue = value.get<float>() * scale + def;
> > -               controls->set(cid, static_cast<int32_t>(lroundf(fvalue)));
> > +               controls->set(cid, static_cast<int32_t>(std::lround(fvalue)));
> >                 break;
> >         }
> >  
> >         case V4L2_CID_SATURATION: {
> >                 float scale = def - min;
> >                 float fvalue = value.get<float>() * scale + min;
> > -               controls->set(cid, static_cast<int32_t>(lroundf(fvalue)));
> > +               controls->set(cid, static_cast<int32_t>(std::lround(fvalue)));
> >                 break;
> >         }
> >  
> > @@ -354,7 +354,7 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
> >                 }
> >  
> >                 float fvalue = (value.get<float>() - p) / m;
> > -               controls->set(cid, static_cast<int32_t>(lroundf(fvalue)));
> > +               controls->set(cid, static_cast<int32_t>(std::lround(fvalue)));
> >                 break;
> >         }
> >  
> > diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> > index 325174b90087..2165bae890cb 100644
> > --- a/src/libcamera/pipeline/vimc/vimc.cpp
> > +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> > @@ -6,9 +6,9 @@
> >   */
> >  
> >  #include <algorithm>
> > +#include <cmath>
> >  #include <iomanip>
> >  #include <map>
> > -#include <math.h>
> >  #include <tuple>
> >  
> >  #include <linux/media-bus-format.h>
> > @@ -420,7 +420,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
> >                         continue;
> >                 }
> >  
> > -               int32_t value = lroundf(it.second.get<float>() * 128 + offset);
> > +               int32_t value = std::lround(it.second.get<float>() * 128 + offset);
> >                 controls.set(cid, std::clamp(value, 0, 255));
> >         }
> >  
> > 
> > base-commit: f2088eb91fd6477b152233b9031cb115ca1ae824
> > -- 
> > Regards,
> > 
> > Laurent Pinchart
> >
Jacopo Mondi Sept. 26, 2024, 2 p.m. UTC | #3 
Hi LAurent

On Thu, Sep 26, 2024 at 03:23:35AM GMT, Laurent Pinchart wrote:
> As explained in the coding style document, usage of std::lround() is
> preferred over lroundf() as it picks the correct function based on
> the argument type. Replace calls to lroundf() with std::lround() through
> libcamera.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

> ---
>  Documentation/guides/pipeline-handler.rst    | 4 ++--
>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 8 ++++----
>  src/libcamera/pipeline/vimc/vimc.cpp         | 4 ++--
>  3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst
> index 26aea43341d4..69e832a5587e 100644
> --- a/Documentation/guides/pipeline-handler.rst
> +++ b/Documentation/guides/pipeline-handler.rst
> @@ -1350,7 +1350,7 @@ before being set.
>                          continue;
>                   }
>
> -                 int32_t value = lroundf(it.second.get<float>() * 128 + offset);
> +                 int32_t value = std::lround(it.second.get<float>() * 128 + offset);
>                   controls.set(cid, std::clamp(value, 0, 255));
>            }
>
> @@ -1414,7 +1414,7 @@ value translation operations:
>
>  .. code-block:: cpp
>
> -   #include <math.h>
> +   #include <cmath>
>
>  Frame completion and event handling
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> index 6b32fa18799b..7fa01bb71744 100644
> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> @@ -6,9 +6,9 @@
>   */
>
>  #include <algorithm>
> +#include <cmath>
>  #include <fstream>
>  #include <map>
> -#include <math.h>
>  #include <memory>
>  #include <set>
>  #include <string>
> @@ -320,14 +320,14 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
>  	case V4L2_CID_BRIGHTNESS: {
>  		float scale = std::max(max - def, def - min);
>  		float fvalue = value.get<float>() * scale + def;
> -		controls->set(cid, static_cast<int32_t>(lroundf(fvalue)));
> +		controls->set(cid, static_cast<int32_t>(std::lround(fvalue)));
>  		break;
>  	}
>
>  	case V4L2_CID_SATURATION: {
>  		float scale = def - min;
>  		float fvalue = value.get<float>() * scale + min;
> -		controls->set(cid, static_cast<int32_t>(lroundf(fvalue)));
> +		controls->set(cid, static_cast<int32_t>(std::lround(fvalue)));
>  		break;
>  	}
>
> @@ -354,7 +354,7 @@ int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
>  		}
>
>  		float fvalue = (value.get<float>() - p) / m;
> -		controls->set(cid, static_cast<int32_t>(lroundf(fvalue)));
> +		controls->set(cid, static_cast<int32_t>(std::lround(fvalue)));
>  		break;
>  	}
>
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index 325174b90087..2165bae890cb 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -6,9 +6,9 @@
>   */
>
>  #include <algorithm>
> +#include <cmath>
>  #include <iomanip>
>  #include <map>
> -#include <math.h>
>  #include <tuple>
>
>  #include <linux/media-bus-format.h>
> @@ -420,7 +420,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
>  			continue;
>  		}
>
> -		int32_t value = lroundf(it.second.get<float>() * 128 + offset);
> +		int32_t value = std::lround(it.second.get<float>() * 128 + offset);
>  		controls.set(cid, std::clamp(value, 0, 255));
>  	}
>
>
> base-commit: f2088eb91fd6477b152233b9031cb115ca1ae824
> --
> Regards,
>
> Laurent Pinchart
>

Patch
diff mbox series 

diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst
index 26aea43341d4..69e832a5587e 100644
--- a/Documentation/guides/pipeline-handler.rst
+++ b/Documentation/guides/pipeline-handler.rst
@@ -1350,7 +1350,7 @@  before being set.
                         continue;
                  }
 
-                 int32_t value = lroundf(it.second.get<float>() * 128 + offset);
+                 int32_t value = std::lround(it.second.get<float>() * 128 + offset);
                  controls.set(cid, std::clamp(value, 0, 255));
           }
 
@@ -1414,7 +1414,7 @@  value translation operations:
 
 .. code-block:: cpp
 
-   #include <math.h>
+   #include <cmath>
 
 Frame completion and event handling
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
index 6b32fa18799b..7fa01bb71744 100644
--- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
@@ -6,9 +6,9 @@ 
  */
 
 #include <algorithm>
+#include <cmath>
 #include <fstream>
 #include <map>
-#include <math.h>
 #include <memory>
 #include <set>
 #include <string>
@@ -320,14 +320,14 @@  int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
 	case V4L2_CID_BRIGHTNESS: {
 		float scale = std::max(max - def, def - min);
 		float fvalue = value.get<float>() * scale + def;
-		controls->set(cid, static_cast<int32_t>(lroundf(fvalue)));
+		controls->set(cid, static_cast<int32_t>(std::lround(fvalue)));
 		break;
 	}
 
 	case V4L2_CID_SATURATION: {
 		float scale = def - min;
 		float fvalue = value.get<float>() * scale + min;
-		controls->set(cid, static_cast<int32_t>(lroundf(fvalue)));
+		controls->set(cid, static_cast<int32_t>(std::lround(fvalue)));
 		break;
 	}
 
@@ -354,7 +354,7 @@  int PipelineHandlerUVC::processControl(ControlList *controls, unsigned int id,
 		}
 
 		float fvalue = (value.get<float>() - p) / m;
-		controls->set(cid, static_cast<int32_t>(lroundf(fvalue)));
+		controls->set(cid, static_cast<int32_t>(std::lround(fvalue)));
 		break;
 	}
 
diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
index 325174b90087..2165bae890cb 100644
--- a/src/libcamera/pipeline/vimc/vimc.cpp
+++ b/src/libcamera/pipeline/vimc/vimc.cpp
@@ -6,9 +6,9 @@ 
  */
 
 #include <algorithm>
+#include <cmath>
 #include <iomanip>
 #include <map>
-#include <math.h>
 #include <tuple>
 
 #include <linux/media-bus-format.h>
@@ -420,7 +420,7 @@  int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
 			continue;
 		}
 
-		int32_t value = lroundf(it.second.get<float>() * 128 + offset);
+		int32_t value = std::lround(it.second.get<float>() * 128 + offset);
 		controls.set(cid, std::clamp(value, 0, 255));
 	}