[libcamera-devel] Fix build with LTO enabled
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Jan 8 23:19:24 CET 2022
Hello,
On Sat, Jan 08, 2022 at 09:47:36PM +0000, Kieran Bingham wrote:
> Quoting victor at westerhu.is (2022-01-07 20:51:05)
> > From 4f419bd0310462616a107c89510a4864a3b8db31 Mon Sep 17 00:00:00 2001
> > From: Victor Westerhuis <victor at westerhu.is>
> > Date: Fri, 7 Jan 2022 17:10:53 +0100
> > Subject: [PATCH] Fix build with LTO enabled
> >
> > libcamera::RPi::Controls in raspberrypi.h depends on
> > libcamera::controls::controls in control_ids.cpp, instantiated
> > from control_ids.cpp.in.
> >
> > The order of initialization is not defined between these two
> > namespace scope objects. This patch changes RPi::Controls to a
> > function-level static, initialized on first use and therefore
> > safe to use.
> >
> > This leads to warnings about getControls not being used in
> > src/ipa/raspberrypi/raspberrypi.cpp and
> > src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> > (through src/libcamera/pipeline/raspberrypi/rpi_stream.h).
> > This patch therefore drops the include without ill effects.
> >
> > Signed-off-by: Victor Westerhuis <victor at westerhu.is>
> > ---
> > This patch fixes https://bugs.libcamera.org/show_bug.cgi?id=83
> >
> > Perhaps this data should not be in a header at all?
>
> Looking at this patch, it certainly looks like this should be moved to
> the .cpp file, and doesn't need to be in the .h file.
The reason why the ControlInfoMap is defined in the header file is that
it's used by the IPA module and the pipeline handler. If you moved it to
a .cpp file on one side, it wouldn't be accessible to the other side.
I think the information should be store din the IPA module, and
communicated to the pipeline handler at init time.
> Naush, what do you think?
>
> Maybe this set of controls should be part of the class too. Would that
> help guarantee the order of construction?
>
> > Please CC me when responding, since I'm not subscribed to this mailing
> > list.
> >
> > include/libcamera/ipa/raspberrypi.h | 42 ++++++++++---------
> > src/ipa/raspberrypi/raspberrypi.cpp | 1 -
> > .../pipeline/raspberrypi/raspberrypi.cpp | 2 +-
> > .../pipeline/raspberrypi/rpi_stream.h | 1 -
> > 4 files changed, 24 insertions(+), 22 deletions(-)
> >
> > diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
> > index 7f705e49..545df355 100644
> > --- a/include/libcamera/ipa/raspberrypi.h
> > +++ b/include/libcamera/ipa/raspberrypi.h
> > @@ -27,26 +27,30 @@ namespace RPi {
> > * and the pipeline handler may be reverted so that it aborts when an
> > * unsupported control is encountered.
> > */
> > -static const ControlInfoMap Controls({
> > - { &controls::AeEnable, ControlInfo(false, true) },
> > - { &controls::ExposureTime, ControlInfo(0, 999999) },
> > - { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) },
> > - { &controls::AeMeteringMode, ControlInfo(controls::AeMeteringModeValues) },
> > - { &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) },
> > - { &controls::AeExposureMode, ControlInfo(controls::AeExposureModeValues) },
> > - { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) },
> > - { &controls::AwbEnable, ControlInfo(false, true) },
> > - { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
> > - { &controls::AwbMode, ControlInfo(controls::AwbModeValues) },
> > - { &controls::Brightness, ControlInfo(-1.0f, 1.0f) },
> > - { &controls::Contrast, ControlInfo(0.0f, 32.0f) },
> > - { &controls::Saturation, ControlInfo(0.0f, 32.0f) },
> > - { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
> > - { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },
> > - { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },
> > - { &controls::FrameDurationLimits, ControlInfo(INT64_C(1000), INT64_C(1000000000)) },
> > - { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) }
> > +static const ControlInfoMap &getControls()
> > +{
> > + static const ControlInfoMap controls({
> > + { &controls::AeEnable, ControlInfo(false, true) },
> > + { &controls::ExposureTime, ControlInfo(0, 999999) },
> > + { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) },
> > + { &controls::AeMeteringMode, ControlInfo(controls::AeMeteringModeValues) },
> > + { &controls::AeConstraintMode, ControlInfo(controls::AeConstraintModeValues) },
> > + { &controls::AeExposureMode, ControlInfo(controls::AeExposureModeValues) },
> > + { &controls::ExposureValue, ControlInfo(0.0f, 16.0f) },
> > + { &controls::AwbEnable, ControlInfo(false, true) },
> > + { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },
> > + { &controls::AwbMode, ControlInfo(controls::AwbModeValues) },
> > + { &controls::Brightness, ControlInfo(-1.0f, 1.0f) },
> > + { &controls::Contrast, ControlInfo(0.0f, 32.0f) },
> > + { &controls::Saturation, ControlInfo(0.0f, 32.0f) },
> > + { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
> > + { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },
> > + { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },
> > + { &controls::FrameDurationLimits, ControlInfo(INT64_C(1000), INT64_C(1000000000)) },
> > + { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) }
> > }, controls::controls);
> > + return controls;
> > +}
> >
> > } /* namespace RPi */
> >
> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> > index 0ed41385..b2717dfc 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -24,7 +24,6 @@
> > #include <libcamera/framebuffer.h>
> > #include <libcamera/ipa/ipa_interface.h>
> > #include <libcamera/ipa/ipa_module_info.h>
> > -#include <libcamera/ipa/raspberrypi.h>
> > #include <libcamera/ipa/raspberrypi_ipa_interface.h>
> > #include <libcamera/request.h>
> >
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index 168bbcef..a48f1130 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -1244,7 +1244,7 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me
> > data->sensorMetadata_ = sensorConfig.sensorMetadata;
> >
> > /* Register the controls that the Raspberry Pi IPA can handle. */
> > - data->controlInfo_ = RPi::Controls;
> > + data->controlInfo_ = RPi::getControls();
> > /* Initialize the camera properties. */
> > data->properties_ = data->sensor_->properties();
> >
> > diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> > index d6f49d34..b0fc1119 100644
> > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> > @@ -12,7 +12,6 @@
> > #include <unordered_map>
> > #include <vector>
> >
> > -#include <libcamera/ipa/raspberrypi.h>
> > #include <libcamera/ipa/raspberrypi_ipa_interface.h>
> > #include <libcamera/stream.h>
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list