[libcamera-devel] src/ipa/raspberrypi/raspberrypi.cpp: drop constexpr for division
diff mbox series

Message ID 20211015170232.1146000-1-fontaine.fabrice@gmail.com
State Rejected
Headers show
Series
  • [libcamera-devel] src/ipa/raspberrypi/raspberrypi.cpp: drop constexpr for division
Related show

Commit Message

Fabrice Fontaine Oct. 15, 2021, 5:02 p.m. UTC
For an unknown reason, build fails with some embedded toolchains since
commit 92b8ccc42a0f3ad323c79bde3c132d6956011239 on:

In file included from ../include/libcamera/base/log.h:10,
                 from ../src/ipa/raspberrypi/raspberrypi.cpp:18:
../src/ipa/raspberrypi/raspberrypi.cpp:64:53:   in 'constexpr' expansion of 'std::chrono::operator/<long double, std::ratio<1>, double>(std::literals::chrono_literals::operator""s(1.0e+0l), 3.0e+1)'
/home/buildroot/autobuild/instance-2/output-1/host/opt/ext-toolchain/powerpc64-buildroot-linux-gnu/include/c++/9.3.0/chrono:502:32: error: '(1.0e+0l / 3.0e+1)' is not a constant expression
  502 |  return __cd(__cd(__d).count() / __s);
../src/ipa/raspberrypi/raspberrypi.cpp:73:56:   in 'constexpr' expansion of 'std::chrono::operator/<long double, std::ratio<1>, double>(std::literals::chrono_literals::operator""s(1.0e+0l), 6.0e+1)'
/home/buildroot/autobuild/instance-2/output-1/host/opt/ext-toolchain/powerpc64-buildroot-linux-gnu/include/c++/9.3.0/chrono:502:32: error: '(1.0e+0l / 6.0e+1)' is not a constant expression

So drop constexpr for both divisions to fix this build failure.

Fixes:
 - http://autobuild.buildroot.org/results/49caebe7ef7e3d63de49e78d5d6839dd0aedf10c

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 src/ipa/raspberrypi/raspberrypi.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Laurent Pinchart Oct. 15, 2021, 5:15 p.m. UTC | #1
Hi Fabrice,

Thank you for the patch.

On Fri, Oct 15, 2021 at 07:02:32PM +0200, Fabrice Fontaine wrote:
> For an unknown reason, build fails with some embedded toolchains since
> commit 92b8ccc42a0f3ad323c79bde3c132d6956011239 on:
> 
> In file included from ../include/libcamera/base/log.h:10,
>                  from ../src/ipa/raspberrypi/raspberrypi.cpp:18:
> ../src/ipa/raspberrypi/raspberrypi.cpp:64:53:   in 'constexpr' expansion of 'std::chrono::operator/<long double, std::ratio<1>, double>(std::literals::chrono_literals::operator""s(1.0e+0l), 3.0e+1)'
> /home/buildroot/autobuild/instance-2/output-1/host/opt/ext-toolchain/powerpc64-buildroot-linux-gnu/include/c++/9.3.0/chrono:502:32: error: '(1.0e+0l / 3.0e+1)' is not a constant expression
>   502 |  return __cd(__cd(__d).count() / __s);
> ../src/ipa/raspberrypi/raspberrypi.cpp:73:56:   in 'constexpr' expansion of 'std::chrono::operator/<long double, std::ratio<1>, double>(std::literals::chrono_literals::operator""s(1.0e+0l), 6.0e+1)'
> /home/buildroot/autobuild/instance-2/output-1/host/opt/ext-toolchain/powerpc64-buildroot-linux-gnu/include/c++/9.3.0/chrono:502:32: error: '(1.0e+0l / 6.0e+1)' is not a constant expression

This seems like it could be a toolchain bug. Does it work with newer gcc
versions on powerpc64 ?

Given that RPi is an ARM64 platform, I'd rather disable compilation of
RPi support on powerpc64 instead of possibly harming the main user.

> So drop constexpr for both divisions to fix this build failure.
> 
> Fixes:
>  - http://autobuild.buildroot.org/results/49caebe7ef7e3d63de49e78d5d6839dd0aedf10c
> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> ---
>  src/ipa/raspberrypi/raspberrypi.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index fed82e22..8ffb7092 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -61,7 +61,7 @@ using utils::Duration;
>  /* Configure the sensor with these values initially. */
>  constexpr double defaultAnalogueGain = 1.0;
>  constexpr Duration defaultExposureTime = 20.0ms;
> -constexpr Duration defaultMinFrameDuration = 1.0s / 30.0;
> +Duration defaultMinFrameDuration = 1.0s / 30.0;
>  constexpr Duration defaultMaxFrameDuration = 250.0s;
>  
>  /*
> @@ -70,7 +70,7 @@ constexpr Duration defaultMaxFrameDuration = 250.0s;
>   * we rate-limit the controller Prepare() and Process() calls to lower than or
>   * equal to this rate.
>   */
> -constexpr Duration controllerMinFrameDuration = 1.0s / 60.0;
> +Duration controllerMinFrameDuration = 1.0s / 60.0;
>  
>  LOG_DEFINE_CATEGORY(IPARPI)
>
Fabrice Fontaine Oct. 15, 2021, 6:28 p.m. UTC | #2
Hi Laurent,

Le ven. 15 oct. 2021 à 19:15, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> a écrit :
>
> Hi Fabrice,
>
> Thank you for the patch.
>
> On Fri, Oct 15, 2021 at 07:02:32PM +0200, Fabrice Fontaine wrote:
> > For an unknown reason, build fails with some embedded toolchains since
> > commit 92b8ccc42a0f3ad323c79bde3c132d6956011239 on:
> >
> > In file included from ../include/libcamera/base/log.h:10,
> >                  from ../src/ipa/raspberrypi/raspberrypi.cpp:18:
> > ../src/ipa/raspberrypi/raspberrypi.cpp:64:53:   in 'constexpr' expansion of 'std::chrono::operator/<long double, std::ratio<1>, double>(std::literals::chrono_literals::operator""s(1.0e+0l), 3.0e+1)'
> > /home/buildroot/autobuild/instance-2/output-1/host/opt/ext-toolchain/powerpc64-buildroot-linux-gnu/include/c++/9.3.0/chrono:502:32: error: '(1.0e+0l / 3.0e+1)' is not a constant expression
> >   502 |  return __cd(__cd(__d).count() / __s);
> > ../src/ipa/raspberrypi/raspberrypi.cpp:73:56:   in 'constexpr' expansion of 'std::chrono::operator/<long double, std::ratio<1>, double>(std::literals::chrono_literals::operator""s(1.0e+0l), 6.0e+1)'
> > /home/buildroot/autobuild/instance-2/output-1/host/opt/ext-toolchain/powerpc64-buildroot-linux-gnu/include/c++/9.3.0/chrono:502:32: error: '(1.0e+0l / 6.0e+1)' is not a constant expression
>
> This seems like it could be a toolchain bug. Does it work with newer gcc
> versions on powerpc64 ?
It builds fine with other toolchains.
>
> Given that RPi is an ARM64 platform, I'd rather disable compilation of
> RPi support on powerpc64 instead of possibly harming the main user.
OK, thanks for your feedback, I'll update buildroot.
>
> > So drop constexpr for both divisions to fix this build failure.
> >
> > Fixes:
> >  - http://autobuild.buildroot.org/results/49caebe7ef7e3d63de49e78d5d6839dd0aedf10c
> >
> > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> > ---
> >  src/ipa/raspberrypi/raspberrypi.cpp | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> > index fed82e22..8ffb7092 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -61,7 +61,7 @@ using utils::Duration;
> >  /* Configure the sensor with these values initially. */
> >  constexpr double defaultAnalogueGain = 1.0;
> >  constexpr Duration defaultExposureTime = 20.0ms;
> > -constexpr Duration defaultMinFrameDuration = 1.0s / 30.0;
> > +Duration defaultMinFrameDuration = 1.0s / 30.0;
> >  constexpr Duration defaultMaxFrameDuration = 250.0s;
> >
> >  /*
> > @@ -70,7 +70,7 @@ constexpr Duration defaultMaxFrameDuration = 250.0s;
> >   * we rate-limit the controller Prepare() and Process() calls to lower than or
> >   * equal to this rate.
> >   */
> > -constexpr Duration controllerMinFrameDuration = 1.0s / 60.0;
> > +Duration controllerMinFrameDuration = 1.0s / 60.0;
> >
> >  LOG_DEFINE_CATEGORY(IPARPI)
> >
>
> --
> Regards,
>
> Laurent Pinchart
Best Regards,

Fabrice
Laurent Pinchart Oct. 15, 2021, 8:35 p.m. UTC | #3
Hi Fabrice,

On Fri, Oct 15, 2021 at 08:28:22PM +0200, Fabrice Fontaine wrote:
> Le ven. 15 oct. 2021 à 19:15, Laurent Pinchart a écrit :
> > On Fri, Oct 15, 2021 at 07:02:32PM +0200, Fabrice Fontaine wrote:
> > > For an unknown reason, build fails with some embedded toolchains since
> > > commit 92b8ccc42a0f3ad323c79bde3c132d6956011239 on:
> > >
> > > In file included from ../include/libcamera/base/log.h:10,
> > >                  from ../src/ipa/raspberrypi/raspberrypi.cpp:18:
> > > ../src/ipa/raspberrypi/raspberrypi.cpp:64:53:   in 'constexpr' expansion of 'std::chrono::operator/<long double, std::ratio<1>, double>(std::literals::chrono_literals::operator""s(1.0e+0l), 3.0e+1)'
> > > /home/buildroot/autobuild/instance-2/output-1/host/opt/ext-toolchain/powerpc64-buildroot-linux-gnu/include/c++/9.3.0/chrono:502:32: error: '(1.0e+0l / 3.0e+1)' is not a constant expression
> > >   502 |  return __cd(__cd(__d).count() / __s);
> > > ../src/ipa/raspberrypi/raspberrypi.cpp:73:56:   in 'constexpr' expansion of 'std::chrono::operator/<long double, std::ratio<1>, double>(std::literals::chrono_literals::operator""s(1.0e+0l), 6.0e+1)'
> > > /home/buildroot/autobuild/instance-2/output-1/host/opt/ext-toolchain/powerpc64-buildroot-linux-gnu/include/c++/9.3.0/chrono:502:32: error: '(1.0e+0l / 6.0e+1)' is not a constant expression
> >
> > This seems like it could be a toolchain bug. Does it work with newer gcc
> > versions on powerpc64 ?
>
> It builds fine with other toolchains.

Does that include newer powerpc64 toolchains ? I'd like to know if we
should report a bug to gcc of if it's fixed already.

> > Given that RPi is an ARM64 platform, I'd rather disable compilation of
> > RPi support on powerpc64 instead of possibly harming the main user.
>
> OK, thanks for your feedback, I'll update buildroot.

Thank you.

> > > So drop constexpr for both divisions to fix this build failure.
> > >
> > > Fixes:
> > >  - http://autobuild.buildroot.org/results/49caebe7ef7e3d63de49e78d5d6839dd0aedf10c
> > >
> > > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> > > ---
> > >  src/ipa/raspberrypi/raspberrypi.cpp | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> > > index fed82e22..8ffb7092 100644
> > > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > > @@ -61,7 +61,7 @@ using utils::Duration;
> > >  /* Configure the sensor with these values initially. */
> > >  constexpr double defaultAnalogueGain = 1.0;
> > >  constexpr Duration defaultExposureTime = 20.0ms;
> > > -constexpr Duration defaultMinFrameDuration = 1.0s / 30.0;
> > > +Duration defaultMinFrameDuration = 1.0s / 30.0;
> > >  constexpr Duration defaultMaxFrameDuration = 250.0s;
> > >
> > >  /*
> > > @@ -70,7 +70,7 @@ constexpr Duration defaultMaxFrameDuration = 250.0s;
> > >   * we rate-limit the controller Prepare() and Process() calls to lower than or
> > >   * equal to this rate.
> > >   */
> > > -constexpr Duration controllerMinFrameDuration = 1.0s / 60.0;
> > > +Duration controllerMinFrameDuration = 1.0s / 60.0;
> > >
> > >  LOG_DEFINE_CATEGORY(IPARPI)
> > >
Fabrice Fontaine Oct. 16, 2021, 8:36 p.m. UTC | #4
Le ven. 15 oct. 2021 à 22:35, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> a écrit :
>
> Hi Fabrice,
>
> On Fri, Oct 15, 2021 at 08:28:22PM +0200, Fabrice Fontaine wrote:
> > Le ven. 15 oct. 2021 à 19:15, Laurent Pinchart a écrit :
> > > On Fri, Oct 15, 2021 at 07:02:32PM +0200, Fabrice Fontaine wrote:
> > > > For an unknown reason, build fails with some embedded toolchains since
> > > > commit 92b8ccc42a0f3ad323c79bde3c132d6956011239 on:
> > > >
> > > > In file included from ../include/libcamera/base/log.h:10,
> > > >                  from ../src/ipa/raspberrypi/raspberrypi.cpp:18:
> > > > ../src/ipa/raspberrypi/raspberrypi.cpp:64:53:   in 'constexpr' expansion of 'std::chrono::operator/<long double, std::ratio<1>, double>(std::literals::chrono_literals::operator""s(1.0e+0l), 3.0e+1)'
> > > > /home/buildroot/autobuild/instance-2/output-1/host/opt/ext-toolchain/powerpc64-buildroot-linux-gnu/include/c++/9.3.0/chrono:502:32: error: '(1.0e+0l / 3.0e+1)' is not a constant expression
> > > >   502 |  return __cd(__cd(__d).count() / __s);
> > > > ../src/ipa/raspberrypi/raspberrypi.cpp:73:56:   in 'constexpr' expansion of 'std::chrono::operator/<long double, std::ratio<1>, double>(std::literals::chrono_literals::operator""s(1.0e+0l), 6.0e+1)'
> > > > /home/buildroot/autobuild/instance-2/output-1/host/opt/ext-toolchain/powerpc64-buildroot-linux-gnu/include/c++/9.3.0/chrono:502:32: error: '(1.0e+0l / 6.0e+1)' is not a constant expression
> > >
> > > This seems like it could be a toolchain bug. Does it work with newer gcc
> > > versions on powerpc64 ?
> >
> > It builds fine with other toolchains.
>
> Does that include newer powerpc64 toolchains ? I'd like to know if we
> should report a bug to gcc of if it's fixed already.
I have the same build failure with a glibc toolchain and gcc 10.3.0
(qemu_ppc64_e5500_defconfig).

>
> > > Given that RPi is an ARM64 platform, I'd rather disable compilation of
> > > RPi support on powerpc64 instead of possibly harming the main user.
> >
> > OK, thanks for your feedback, I'll update buildroot.
>
> Thank you.
>
> > > > So drop constexpr for both divisions to fix this build failure.
> > > >
> > > > Fixes:
> > > >  - http://autobuild.buildroot.org/results/49caebe7ef7e3d63de49e78d5d6839dd0aedf10c
> > > >
> > > > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> > > > ---
> > > >  src/ipa/raspberrypi/raspberrypi.cpp | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> > > > index fed82e22..8ffb7092 100644
> > > > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > > > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > > > @@ -61,7 +61,7 @@ using utils::Duration;
> > > >  /* Configure the sensor with these values initially. */
> > > >  constexpr double defaultAnalogueGain = 1.0;
> > > >  constexpr Duration defaultExposureTime = 20.0ms;
> > > > -constexpr Duration defaultMinFrameDuration = 1.0s / 30.0;
> > > > +Duration defaultMinFrameDuration = 1.0s / 30.0;
> > > >  constexpr Duration defaultMaxFrameDuration = 250.0s;
> > > >
> > > >  /*
> > > > @@ -70,7 +70,7 @@ constexpr Duration defaultMaxFrameDuration = 250.0s;
> > > >   * we rate-limit the controller Prepare() and Process() calls to lower than or
> > > >   * equal to this rate.
> > > >   */
> > > > -constexpr Duration controllerMinFrameDuration = 1.0s / 60.0;
> > > > +Duration controllerMinFrameDuration = 1.0s / 60.0;
> > > >
> > > >  LOG_DEFINE_CATEGORY(IPARPI)
> > > >
>
> --
> Regards,
>
> Laurent Pinchart

Patch
diff mbox series

diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index fed82e22..8ffb7092 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -61,7 +61,7 @@  using utils::Duration;
 /* Configure the sensor with these values initially. */
 constexpr double defaultAnalogueGain = 1.0;
 constexpr Duration defaultExposureTime = 20.0ms;
-constexpr Duration defaultMinFrameDuration = 1.0s / 30.0;
+Duration defaultMinFrameDuration = 1.0s / 30.0;
 constexpr Duration defaultMaxFrameDuration = 250.0s;
 
 /*
@@ -70,7 +70,7 @@  constexpr Duration defaultMaxFrameDuration = 250.0s;
  * we rate-limit the controller Prepare() and Process() calls to lower than or
  * equal to this rate.
  */
-constexpr Duration controllerMinFrameDuration = 1.0s / 60.0;
+Duration controllerMinFrameDuration = 1.0s / 60.0;
 
 LOG_DEFINE_CATEGORY(IPARPI)