| Message ID | 20260115111630.1892890-2-barnabas.pocze@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Quoting Barnabás Pőcze (2026-01-15 11:16:29) > `utils::Duration` derives from `std::chrono::duration<...>`, but multiplying > it yields an `std::chrono::duration<...>`, not `Duration`. chrono duration > types only have `operator<<` in C++20 or later, so this usage should not > compile. It only did so because the `operator<<` for `Duration` was in > the `libcamera` namespace and `Duration` has an implicit constructor from > any chrono duration type. > > This will cease to work when that operator is moved into the `utils` namespace > for ADL purposes. So fix it by making the cast to `Duration` explicit. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> This looks reasonable to me - but it's in ipa/rpi so it needs an ack from RPi. But from me: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/ipa/rpi/common/ipa_base.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp > index 14aba4500..f95a0f838 100644 > --- a/src/ipa/rpi/common/ipa_base.cpp > +++ b/src/ipa/rpi/common/ipa_base.cpp > @@ -602,7 +602,7 @@ void IpaBase::setMode(const IPACameraSensorInfo &sensorInfo) > mode_.minLineLength = adjustedLineLength; > } else { > LOG(IPARPI, Error) > - << "Sensor minimum line length of " << pixelTime * mode_.width > + << "Sensor minimum line length of " << Duration(pixelTime * mode_.width) > << " (" << 1us / pixelTime << " MPix/s)" > << " is below the minimum allowable ISP limit of " > << adjustedLineLength > -- > 2.52.0 >
Hi Barnabás, Thank you for the patch. On Thu, Jan 15, 2026 at 12:16:29PM +0100, Barnabás Pőcze wrote: > `utils::Duration` derives from `std::chrono::duration<...>`, but multiplying > it yields an `std::chrono::duration<...>`, not `Duration`. chrono duration > types only have `operator<<` in C++20 or later, so this usage should not > compile. It only did so because the `operator<<` for `Duration` was in > the `libcamera` namespace and `Duration` has an implicit constructor from > any chrono duration type. > > This will cease to work when that operator is moved into the `utils` namespace > for ADL purposes. So fix it by making the cast to `Duration` explicit. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/ipa/rpi/common/ipa_base.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp > index 14aba4500..f95a0f838 100644 > --- a/src/ipa/rpi/common/ipa_base.cpp > +++ b/src/ipa/rpi/common/ipa_base.cpp > @@ -602,7 +602,7 @@ void IpaBase::setMode(const IPACameraSensorInfo &sensorInfo) > mode_.minLineLength = adjustedLineLength; > } else { > LOG(IPARPI, Error) > - << "Sensor minimum line length of " << pixelTime * mode_.width > + << "Sensor minimum line length of " << Duration(pixelTime * mode_.width) << "Sensor minimum line length of " << Duration(pixelTime * mode_.width) Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > << " (" << 1us / pixelTime << " MPix/s)" > << " is below the minimum allowable ISP limit of " > << adjustedLineLength
Naush, David, Any objections here - I'd like to get these ones merged as this series fixes ADL/log string handling. -- Kieran Quoting Barnabás Pőcze (2026-01-15 11:16:29) > `utils::Duration` derives from `std::chrono::duration<...>`, but multiplying > it yields an `std::chrono::duration<...>`, not `Duration`. chrono duration > types only have `operator<<` in C++20 or later, so this usage should not > compile. It only did so because the `operator<<` for `Duration` was in > the `libcamera` namespace and `Duration` has an implicit constructor from > any chrono duration type. > > This will cease to work when that operator is moved into the `utils` namespace > for ADL purposes. So fix it by making the cast to `Duration` explicit. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/ipa/rpi/common/ipa_base.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp > index 14aba4500..f95a0f838 100644 > --- a/src/ipa/rpi/common/ipa_base.cpp > +++ b/src/ipa/rpi/common/ipa_base.cpp > @@ -602,7 +602,7 @@ void IpaBase::setMode(const IPACameraSensorInfo &sensorInfo) > mode_.minLineLength = adjustedLineLength; > } else { > LOG(IPARPI, Error) > - << "Sensor minimum line length of " << pixelTime * mode_.width > + << "Sensor minimum line length of " << Duration(pixelTime * mode_.width) > << " (" << 1us / pixelTime << " MPix/s)" > << " is below the minimum allowable ISP limit of " > << adjustedLineLength > -- > 2.52.0 >
No objections from me Reviewed-by: Naushir Patuck <naush@raspberrypi.com> On Wed, 28 Jan 2026, 4:11 pm Kieran Bingham, < kieran.bingham@ideasonboard.com> wrote: > Naush, David, > > Any objections here - I'd like to get these ones merged as this series > fixes ADL/log string handling. > > -- > Kieran > > > Quoting Barnabás Pőcze (2026-01-15 11:16:29) > > `utils::Duration` derives from `std::chrono::duration<...>`, but > multiplying > > it yields an `std::chrono::duration<...>`, not `Duration`. chrono > duration > > types only have `operator<<` in C++20 or later, so this usage should not > > compile. It only did so because the `operator<<` for `Duration` was in > > the `libcamera` namespace and `Duration` has an implicit constructor from > > any chrono duration type. > > > > This will cease to work when that operator is moved into the `utils` > namespace > > for ADL purposes. So fix it by making the cast to `Duration` explicit. > > > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > --- > > src/ipa/rpi/common/ipa_base.cpp | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/ipa/rpi/common/ipa_base.cpp > b/src/ipa/rpi/common/ipa_base.cpp > > index 14aba4500..f95a0f838 100644 > > --- a/src/ipa/rpi/common/ipa_base.cpp > > +++ b/src/ipa/rpi/common/ipa_base.cpp > > @@ -602,7 +602,7 @@ void IpaBase::setMode(const IPACameraSensorInfo > &sensorInfo) > > mode_.minLineLength = adjustedLineLength; > > } else { > > LOG(IPARPI, Error) > > - << "Sensor minimum line length of " << > pixelTime * mode_.width > > + << "Sensor minimum line length of " << > Duration(pixelTime * mode_.width) > > << " (" << 1us / pixelTime << " MPix/s)" > > << " is below the minimum allowable ISP > limit of " > > << adjustedLineLength > > -- > > 2.52.0 > > >
diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp index 14aba4500..f95a0f838 100644 --- a/src/ipa/rpi/common/ipa_base.cpp +++ b/src/ipa/rpi/common/ipa_base.cpp @@ -602,7 +602,7 @@ void IpaBase::setMode(const IPACameraSensorInfo &sensorInfo) mode_.minLineLength = adjustedLineLength; } else { LOG(IPARPI, Error) - << "Sensor minimum line length of " << pixelTime * mode_.width + << "Sensor minimum line length of " << Duration(pixelTime * mode_.width) << " (" << 1us / pixelTime << " MPix/s)" << " is below the minimum allowable ISP limit of " << adjustedLineLength
`utils::Duration` derives from `std::chrono::duration<...>`, but multiplying it yields an `std::chrono::duration<...>`, not `Duration`. chrono duration types only have `operator<<` in C++20 or later, so this usage should not compile. It only did so because the `operator<<` for `Duration` was in the `libcamera` namespace and `Duration` has an implicit constructor from any chrono duration type. This will cease to work when that operator is moved into the `utils` namespace for ADL purposes. So fix it by making the cast to `Duration` explicit. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/ipa/rpi/common/ipa_base.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)