[libcamera-devel] Fix build with LTO enabled
Victor Westerhuis
victor at westerhu.is
Sun Jan 9 01:52:53 CET 2022
Laurent Pinchart schreef op 08.01.2022 23:19:
> 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.
>
While writing my patch, I noticed that the data in the header is not
currently referenced elsewhere. That's why after changing the variable
to a getter function I had to remove the include from two other files,
to prevent a warning about unused functions.
> 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>
>> >
More information about the libcamera-devel
mailing list