[libcamera-devel,2/2] ipa: ipu3: Tidy-up includes
diff mbox series

Message ID 20210716143215.67454-3-jacopo@jmondi.org
State Superseded
Headers show
Series
  • libcamera: Initialize controls in the IPA
Related show

Commit Message

Jacopo Mondi July 16, 2021, 2:32 p.m. UTC 
Tidy-up a bit the inclusions directive in the IPU3 IPA module.

In detail:
- ipu3.cpp is missing inclusions for:
  std::abs from <cmath>
  std::map from <map>
  std::min/max from <algorithm>
  std::numeric_limits from <limits>
  std::unique_ptr from <memory>
  std::vector from <vector>

  and does not require <sys/mman.h>

- ipu3_agc has two not used inclusions in the header file and one the cpp file
  and is missing <chrono> for std::literals::chrono_literals

- ipu3_awb is missing <algorithm> for std::sort and does not use
  <numeric> or <unordered_map>

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/ipa/ipu3/ipu3.cpp     | 7 ++++++-
 src/ipa/ipu3/ipu3_agc.cpp | 2 +-
 src/ipa/ipu3/ipu3_agc.h   | 3 ---
 src/ipa/ipu3/ipu3_awb.cpp | 3 +--
 4 files changed, 8 insertions(+), 7 deletions(-)

Comments

Umang Jain July 22, 2021, 6:37 a.m. UTC | #1 
Hi Jacopo,

On 7/16/21 8:02 PM, Jacopo Mondi wrote:
> Tidy-up a bit the inclusions directive in the IPU3 IPA module.
>
> In detail:
> - ipu3.cpp is missing inclusions for:
>    std::abs from <cmath>
>    std::map from <map>
>    std::min/max from <algorithm>
>    std::numeric_limits from <limits>
>    std::unique_ptr from <memory>
>    std::vector from <vector>
>
>    and does not require <sys/mman.h>
>
> - ipu3_agc has two not used inclusions in the header file and one the cpp file
>    and is missing <chrono> for std::literals::chrono_literals

The <chrono> comes from <libcamera/base/utils.h> included in ipu3_agc.h, 
since we use `Duration` wrapper in class IPU3Agc. Should be again 
including the header in ipu3_agc.cpp for chrono_literals?


>
> - ipu3_awb is missing <algorithm> for std::sort and does not use
>    <numeric> or <unordered_map>
>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Rest good looks so,

Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>

> ---
>   src/ipa/ipu3/ipu3.cpp     | 7 ++++++-
>   src/ipa/ipu3/ipu3_agc.cpp | 2 +-
>   src/ipa/ipu3/ipu3_agc.h   | 3 ---
>   src/ipa/ipu3/ipu3_awb.cpp | 3 +--
>   4 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index d3c69bc07bd0..dc22acd4fd08 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -5,10 +5,15 @@
>    * ipu3.cpp - IPU3 Image Processing Algorithms
>    */
>   
> +#include <algorithm>
>   #include <array>
> +#include <cmath>
> +#include <limits>
> +#include <map>
> +#include <memory>
>   #include <stdint.h>
> -#include <sys/mman.h>
>   #include <utility>
> +#include <vector>
>   
>   #include <linux/intel-ipu3.h>
>   #include <linux/v4l2-controls.h>
> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/ipu3_agc.cpp
> index 6253ab94cff1..408eb849b428 100644
> --- a/src/ipa/ipu3/ipu3_agc.cpp
> +++ b/src/ipa/ipu3/ipu3_agc.cpp
> @@ -8,8 +8,8 @@
>   #include "ipu3_agc.h"
>   
>   #include <algorithm>
> +#include <chrono>
>   #include <cmath>
> -#include <numeric>
>   
>   #include <libcamera/base/log.h>
>   
> diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/ipu3_agc.h
> index 3deca3ae6933..9f3d4257d945 100644
> --- a/src/ipa/ipu3/ipu3_agc.h
> +++ b/src/ipa/ipu3/ipu3_agc.h
> @@ -7,9 +7,6 @@
>   #ifndef __LIBCAMERA_IPU3_AGC_H__
>   #define __LIBCAMERA_IPU3_AGC_H__
>   
> -#include <array>
> -#include <unordered_map>
> -
>   #include <linux/intel-ipu3.h>
>   
>   #include <libcamera/base/utils.h>
> diff --git a/src/ipa/ipu3/ipu3_awb.cpp b/src/ipa/ipu3/ipu3_awb.cpp
> index 9b409c8ffad9..4bb321b377a2 100644
> --- a/src/ipa/ipu3/ipu3_awb.cpp
> +++ b/src/ipa/ipu3/ipu3_awb.cpp
> @@ -6,9 +6,8 @@
>    */
>   #include "ipu3_awb.h"
>   
> +#include <algorithm>
>   #include <cmath>
> -#include <numeric>
> -#include <unordered_map>
>   
>   #include <libcamera/base/log.h>
>
Laurent Pinchart July 25, 2021, 1:29 a.m. UTC | #2 
Hi Jacopo,

Thank you for the patch.

On Fri, Jul 16, 2021 at 04:32:15PM +0200, Jacopo Mondi wrote:
> Tidy-up a bit the inclusions directive in the IPU3 IPA module.
> 
> In detail:
> - ipu3.cpp is missing inclusions for:
>   std::abs from <cmath>
>   std::map from <map>
>   std::min/max from <algorithm>
>   std::numeric_limits from <limits>
>   std::unique_ptr from <memory>
>   std::vector from <vector>
> 
>   and does not require <sys/mman.h>
> 
> - ipu3_agc has two not used inclusions in the header file and one the cpp file
>   and is missing <chrono> for std::literals::chrono_literals
> 
> - ipu3_awb is missing <algorithm> for std::sort and does not use
>   <numeric> or <unordered_map>
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/ipa/ipu3/ipu3.cpp     | 7 ++++++-
>  src/ipa/ipu3/ipu3_agc.cpp | 2 +-
>  src/ipa/ipu3/ipu3_agc.h   | 3 ---
>  src/ipa/ipu3/ipu3_awb.cpp | 3 +--
>  4 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index d3c69bc07bd0..dc22acd4fd08 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -5,10 +5,15 @@
>   * ipu3.cpp - IPU3 Image Processing Algorithms
>   */
>  
> +#include <algorithm>
>  #include <array>
> +#include <cmath>
> +#include <limits>
> +#include <map>
> +#include <memory>
>  #include <stdint.h>
> -#include <sys/mman.h>
>  #include <utility>
> +#include <vector>
>  
>  #include <linux/intel-ipu3.h>
>  #include <linux/v4l2-controls.h>
> diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/ipu3_agc.cpp
> index 6253ab94cff1..408eb849b428 100644
> --- a/src/ipa/ipu3/ipu3_agc.cpp
> +++ b/src/ipa/ipu3/ipu3_agc.cpp
> @@ -8,8 +8,8 @@
>  #include "ipu3_agc.h"
>  
>  #include <algorithm>
> +#include <chrono>
>  #include <cmath>
> -#include <numeric>
>  
>  #include <libcamera/base/log.h>
>  
> diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/ipu3_agc.h
> index 3deca3ae6933..9f3d4257d945 100644
> --- a/src/ipa/ipu3/ipu3_agc.h
> +++ b/src/ipa/ipu3/ipu3_agc.h
> @@ -7,9 +7,6 @@
>  #ifndef __LIBCAMERA_IPU3_AGC_H__
>  #define __LIBCAMERA_IPU3_AGC_H__
>  
> -#include <array>
> -#include <unordered_map>
> -
>  #include <linux/intel-ipu3.h>
>  
>  #include <libcamera/base/utils.h>
> diff --git a/src/ipa/ipu3/ipu3_awb.cpp b/src/ipa/ipu3/ipu3_awb.cpp
> index 9b409c8ffad9..4bb321b377a2 100644
> --- a/src/ipa/ipu3/ipu3_awb.cpp
> +++ b/src/ipa/ipu3/ipu3_awb.cpp
> @@ -6,9 +6,8 @@
>   */
>  #include "ipu3_awb.h"
>  
> +#include <algorithm>
>  #include <cmath>
> -#include <numeric>
> -#include <unordered_map>
>  
>  #include <libcamera/base/log.h>
>
Jacopo Mondi July 26, 2021, 8:20 a.m. UTC | #3 
Hello Umang,

On Thu, Jul 22, 2021 at 12:07:05PM +0530, Umang Jain wrote:
> Hi Jacopo,
>
> On 7/16/21 8:02 PM, Jacopo Mondi wrote:
> > Tidy-up a bit the inclusions directive in the IPU3 IPA module.
> >
> > In detail:
> > - ipu3.cpp is missing inclusions for:
> >    std::abs from <cmath>
> >    std::map from <map>
> >    std::min/max from <algorithm>
> >    std::numeric_limits from <limits>
> >    std::unique_ptr from <memory>
> >    std::vector from <vector>
> >
> >    and does not require <sys/mman.h>
> >
> > - ipu3_agc has two not used inclusions in the header file and one the cpp file
> >    and is missing <chrono> for std::literals::chrono_literals
>
> The <chrono> comes from <libcamera/base/utils.h> included in ipu3_agc.h,
> since we use `Duration` wrapper in class IPU3Agc. Should be again including
> the header in ipu3_agc.cpp for chrono_literals?
>

I think it's always good not to resort on indirect inclusions. It
guarantees the file is valid regardless of what's included in the one
it relies on, and there's no penalty to pay in terms of additional
file size thanks for headers' inclusion guards.

>
> >
> > - ipu3_awb is missing <algorithm> for std::sort and does not use
> >    <numeric> or <unordered_map>
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
>
> Rest good looks so,
>
> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
>

Thanks
   j

> > ---
> >   src/ipa/ipu3/ipu3.cpp     | 7 ++++++-
> >   src/ipa/ipu3/ipu3_agc.cpp | 2 +-
> >   src/ipa/ipu3/ipu3_agc.h   | 3 ---
> >   src/ipa/ipu3/ipu3_awb.cpp | 3 +--
> >   4 files changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> > index d3c69bc07bd0..dc22acd4fd08 100644
> > --- a/src/ipa/ipu3/ipu3.cpp
> > +++ b/src/ipa/ipu3/ipu3.cpp
> > @@ -5,10 +5,15 @@
> >    * ipu3.cpp - IPU3 Image Processing Algorithms
> >    */
> > +#include <algorithm>
> >   #include <array>
> > +#include <cmath>
> > +#include <limits>
> > +#include <map>
> > +#include <memory>
> >   #include <stdint.h>
> > -#include <sys/mman.h>
> >   #include <utility>
> > +#include <vector>
> >   #include <linux/intel-ipu3.h>
> >   #include <linux/v4l2-controls.h>
> > diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/ipu3_agc.cpp
> > index 6253ab94cff1..408eb849b428 100644
> > --- a/src/ipa/ipu3/ipu3_agc.cpp
> > +++ b/src/ipa/ipu3/ipu3_agc.cpp
> > @@ -8,8 +8,8 @@
> >   #include "ipu3_agc.h"
> >   #include <algorithm>
> > +#include <chrono>
> >   #include <cmath>
> > -#include <numeric>
> >   #include <libcamera/base/log.h>
> > diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/ipu3_agc.h
> > index 3deca3ae6933..9f3d4257d945 100644
> > --- a/src/ipa/ipu3/ipu3_agc.h
> > +++ b/src/ipa/ipu3/ipu3_agc.h
> > @@ -7,9 +7,6 @@
> >   #ifndef __LIBCAMERA_IPU3_AGC_H__
> >   #define __LIBCAMERA_IPU3_AGC_H__
> > -#include <array>
> > -#include <unordered_map>
> > -
> >   #include <linux/intel-ipu3.h>
> >   #include <libcamera/base/utils.h>
> > diff --git a/src/ipa/ipu3/ipu3_awb.cpp b/src/ipa/ipu3/ipu3_awb.cpp
> > index 9b409c8ffad9..4bb321b377a2 100644
> > --- a/src/ipa/ipu3/ipu3_awb.cpp
> > +++ b/src/ipa/ipu3/ipu3_awb.cpp
> > @@ -6,9 +6,8 @@
> >    */
> >   #include "ipu3_awb.h"
> > +#include <algorithm>
> >   #include <cmath>
> > -#include <numeric>
> > -#include <unordered_map>
> >   #include <libcamera/base/log.h>

Patch
diff mbox series 

diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index d3c69bc07bd0..dc22acd4fd08 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -5,10 +5,15 @@ 
  * ipu3.cpp - IPU3 Image Processing Algorithms
  */
 
+#include <algorithm>
 #include <array>
+#include <cmath>
+#include <limits>
+#include <map>
+#include <memory>
 #include <stdint.h>
-#include <sys/mman.h>
 #include <utility>
+#include <vector>
 
 #include <linux/intel-ipu3.h>
 #include <linux/v4l2-controls.h>
diff --git a/src/ipa/ipu3/ipu3_agc.cpp b/src/ipa/ipu3/ipu3_agc.cpp
index 6253ab94cff1..408eb849b428 100644
--- a/src/ipa/ipu3/ipu3_agc.cpp
+++ b/src/ipa/ipu3/ipu3_agc.cpp
@@ -8,8 +8,8 @@ 
 #include "ipu3_agc.h"
 
 #include <algorithm>
+#include <chrono>
 #include <cmath>
-#include <numeric>
 
 #include <libcamera/base/log.h>
 
diff --git a/src/ipa/ipu3/ipu3_agc.h b/src/ipa/ipu3/ipu3_agc.h
index 3deca3ae6933..9f3d4257d945 100644
--- a/src/ipa/ipu3/ipu3_agc.h
+++ b/src/ipa/ipu3/ipu3_agc.h
@@ -7,9 +7,6 @@ 
 #ifndef __LIBCAMERA_IPU3_AGC_H__
 #define __LIBCAMERA_IPU3_AGC_H__
 
-#include <array>
-#include <unordered_map>
-
 #include <linux/intel-ipu3.h>
 
 #include <libcamera/base/utils.h>
diff --git a/src/ipa/ipu3/ipu3_awb.cpp b/src/ipa/ipu3/ipu3_awb.cpp
index 9b409c8ffad9..4bb321b377a2 100644
--- a/src/ipa/ipu3/ipu3_awb.cpp
+++ b/src/ipa/ipu3/ipu3_awb.cpp
@@ -6,9 +6,8 @@ 
  */
 #include "ipu3_awb.h"
 
+#include <algorithm>
 #include <cmath>
-#include <numeric>
-#include <unordered_map>
 
 #include <libcamera/base/log.h>