[v3,1/5] ipa: rpi: cam_helper: Add Mira220 cam helper
diff mbox series

Message ID 20260825-mira220-v3-1-7dad83521699@ideasonboard.com
State New
Headers show
Series
  • ipa: Add support for Mira220 RGB version
Related show

Commit Message

Jacopo Mondi Aug. 25, 2026, 4:44 p.m. UTC 
From: Philippe Baetens <philippebaetens@gmail.com>

AMS-OSRAM Mira220 is a global shutter sensor with a maximum
resolution of 1600x1400.

Mira220 doesn't support embedded data metadata, and uses delayed controls
which works by counting frames.

Signed-off-by: Philippe Baetens <philippebaetens@gmail.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Acked-by: Naushir Patuck <naush@raspberrypi.com>
---
 src/ipa/rpi/cam_helper/cam_helper_mira220.cpp | 59 +++++++++++++++++++++++++++
 src/ipa/rpi/cam_helper/meson.build            |  1 +
 2 files changed, 60 insertions(+)

Comments

Jai Luthra Sept. 15, 2026, 10:53 a.m. UTC | #1 
Hi Philippe, Jacopo,

Quoting Jacopo Mondi (2026-08-25 22:14:44)
> From: Philippe Baetens <philippebaetens@gmail.com>
> 
> AMS-OSRAM Mira220 is a global shutter sensor with a maximum
> resolution of 1600x1400.
> 
> Mira220 doesn't support embedded data metadata, and uses delayed controls
> which works by counting frames.
> 
> Signed-off-by: Philippe Baetens <philippebaetens@gmail.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Acked-by: Naushir Patuck <naush@raspberrypi.com>
> ---
>  src/ipa/rpi/cam_helper/cam_helper_mira220.cpp | 59 +++++++++++++++++++++++++++
>  src/ipa/rpi/cam_helper/meson.build            |  1 +
>  2 files changed, 60 insertions(+)
> 
> diff --git a/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp b/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp
> new file mode 100644
> index 000000000000..c884b853b0c1
> --- /dev/null
> +++ b/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp
> @@ -0,0 +1,59 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2026, ams-OSRAM
> + *
> + * Camera helper for Mira220 sensor
> + */
> +
> +#include <assert.h>
> +
> +#include "cam_helper.h"
> +
> +using namespace RPiController;
> +
> +class CamHelperMira220 : public CamHelper
> +{
> +public:
> +       CamHelperMira220();
> +       uint32_t gainCode(double gain) const override;
> +       double gain(uint32_t gainCode) const override;
> +       unsigned int hideFramesModeSwitch() const override;

I think the definition of this method is missing here. I recently rebased
by Raspberry Pi 4 patches and saw the following issue, bisected to this
commit:

[3:28:00.953283153] [4524] DEBUG IPAManager ipa_manager.cpp:309 IPA module /usr/local/lib/libcamera/ipa/ipa_rpi_vc4.so signature is valid
[3:28:00.953496264] [4524] DEBUG IPAProxy raspberrypi_ipa_proxy.cpp:44 initializing raspberrypi proxy in thread: loading IPA from /usr/local/lib/libcamera/ipa/ipa_rpi_vc4.so
[3:28:00.954332986] [4524] ERROR IPAModule ipa_module.cpp:416 ipa_rpi_vc4.so: Failed to open IPA module shared object: /usr/local/lib/libcamera/ipa/ipa_rpi_vc4.so: undefined symbol: _ZNK16CamHelperMira22020hideFramesModeSwitchEv
[3:28:00.954388875] [4524] ERROR IPAManager ipa_manager.h:51 Failed to load proxy
[3:28:00.954421579] [4524] ERROR RPI pipeline_base.cpp:830 Failed to load a suitable IPA library

Thanks,
    Jai

> +
> +private:
> +       /*
> +        * Smallest difference between the frame length and integration time,
> +        * in units of lines.
> +        *
> +        * The integration diff is expressed as
> +        *    Tframe - 1928 / row_length
> +        *
> +        * row_length controls the line timings and varies according to the
> +        * number of data lanes in use and the D-PHY data rate. Use an
> +        * integration diff calculated using the value of 304, which represents
> +        * the minimum row_length for a 2 data lanes configuration running at
> +        * 1.5Gbps.
> +        */
> +       static constexpr int frameIntegrationDiff = 6;
> +};
> +
> +CamHelperMira220::CamHelperMira220()
> +       : CamHelper({}, frameIntegrationDiff)
> +{
> +}
> +
> +uint32_t CamHelperMira220::gainCode(double gain) const
> +{
> +       return static_cast<uint32_t>(2048.0 - 2048.0 / gain);
> +}
> +
> +double CamHelperMira220::gain(uint32_t gainCode) const
> +{
> +       return static_cast<double>(2048.0 / (2048 - gainCode));
> +}
> +
> +static CamHelper *create()
> +{
> +       return new CamHelperMira220();
> +}
> +
> +static RegisterCamHelper reg("mira220", &create);
> diff --git a/src/ipa/rpi/cam_helper/meson.build b/src/ipa/rpi/cam_helper/meson.build
> index 0aa8c3dc2e2f..cc71bacb2c6a 100644
> --- a/src/ipa/rpi/cam_helper/meson.build
> +++ b/src/ipa/rpi/cam_helper/meson.build
> @@ -13,6 +13,7 @@ rpi_ipa_cam_helper_sources = files([
>      'cam_helper_imx519.cpp',
>      'cam_helper_imx678.cpp',
>      'cam_helper_imx708.cpp',
> +    'cam_helper_mira220.cpp',
>      'cam_helper_ov64a40.cpp',
>      'cam_helper_ov7251.cpp',
>      'cam_helper_ov9281.cpp',
> 
> -- 
> 2.55.0
>
Kieran Bingham Sept. 15, 2026, 10:58 a.m. UTC | #2 
Quoting Jai Luthra (2026-09-15 11:53:06)
> Hi Philippe, Jacopo,
> 
> Quoting Jacopo Mondi (2026-08-25 22:14:44)
> > From: Philippe Baetens <philippebaetens@gmail.com>
> > 
> > AMS-OSRAM Mira220 is a global shutter sensor with a maximum
> > resolution of 1600x1400.
> > 
> > Mira220 doesn't support embedded data metadata, and uses delayed controls
> > which works by counting frames.
> > 
> > Signed-off-by: Philippe Baetens <philippebaetens@gmail.com>
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > Acked-by: Naushir Patuck <naush@raspberrypi.com>
> > ---
> >  src/ipa/rpi/cam_helper/cam_helper_mira220.cpp | 59 +++++++++++++++++++++++++++
> >  src/ipa/rpi/cam_helper/meson.build            |  1 +
> >  2 files changed, 60 insertions(+)
> > 
> > diff --git a/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp b/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp
> > new file mode 100644
> > index 000000000000..c884b853b0c1
> > --- /dev/null
> > +++ b/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp
> > @@ -0,0 +1,59 @@
> > +/* SPDX-License-Identifier: BSD-2-Clause */
> > +/*
> > + * Copyright (C) 2026, ams-OSRAM
> > + *
> > + * Camera helper for Mira220 sensor
> > + */
> > +
> > +#include <assert.h>
> > +
> > +#include "cam_helper.h"
> > +
> > +using namespace RPiController;
> > +
> > +class CamHelperMira220 : public CamHelper
> > +{
> > +public:
> > +       CamHelperMira220();
> > +       uint32_t gainCode(double gain) const override;
> > +       double gain(uint32_t gainCode) const override;
> > +       unsigned int hideFramesModeSwitch() const override;
> 
> I think the definition of this method is missing here. I recently rebased
> by Raspberry Pi 4 patches and saw the following issue, bisected to this
> commit:
> 
> [3:28:00.953283153] [4524] DEBUG IPAManager ipa_manager.cpp:309 IPA module /usr/local/lib/libcamera/ipa/ipa_rpi_vc4.so signature is valid
> [3:28:00.953496264] [4524] DEBUG IPAProxy raspberrypi_ipa_proxy.cpp:44 initializing raspberrypi proxy in thread: loading IPA from /usr/local/lib/libcamera/ipa/ipa_rpi_vc4.so
> [3:28:00.954332986] [4524] ERROR IPAModule ipa_module.cpp:416 ipa_rpi_vc4.so: Failed to open IPA module shared object: /usr/local/lib/libcamera/ipa/ipa_rpi_vc4.so: undefined symbol: _ZNK16CamHelperMira22020hideFramesModeSwitchEv
> [3:28:00.954388875] [4524] ERROR IPAManager ipa_manager.h:51 Failed to load proxy
> [3:28:00.954421579] [4524] ERROR RPI pipeline_base.cpp:830 Failed to load a suitable IPA library
> 
> Thanks,
>     Jai


Eeek, I think it's as simple as removing that override line right? Could
you test and send a patch please? Jacopo is on holiday so it would take
a week otherwise, and we should fix the main branch.

--
Kieran
Barnabás Pőcze Sept. 15, 2026, 11:04 a.m. UTC | #3 
2026. 09. 15. 12:58 keltezéssel, Kieran Bingham írta:
> Quoting Jai Luthra (2026-09-15 11:53:06)
>> Hi Philippe, Jacopo,
>>
>> Quoting Jacopo Mondi (2026-08-25 22:14:44)
>>> From: Philippe Baetens <philippebaetens@gmail.com>
>>>
>>> AMS-OSRAM Mira220 is a global shutter sensor with a maximum
>>> resolution of 1600x1400.
>>>
>>> Mira220 doesn't support embedded data metadata, and uses delayed controls
>>> which works by counting frames.
>>>
>>> Signed-off-by: Philippe Baetens <philippebaetens@gmail.com>
>>> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>>> Acked-by: Naushir Patuck <naush@raspberrypi.com>
>>> ---
>>>   src/ipa/rpi/cam_helper/cam_helper_mira220.cpp | 59 +++++++++++++++++++++++++++
>>>   src/ipa/rpi/cam_helper/meson.build            |  1 +
>>>   2 files changed, 60 insertions(+)
>>>
>>> diff --git a/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp b/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp
>>> new file mode 100644
>>> index 000000000000..c884b853b0c1
>>> --- /dev/null
>>> +++ b/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp
>>> @@ -0,0 +1,59 @@
>>> +/* SPDX-License-Identifier: BSD-2-Clause */
>>> +/*
>>> + * Copyright (C) 2026, ams-OSRAM
>>> + *
>>> + * Camera helper for Mira220 sensor
>>> + */
>>> +
>>> +#include <assert.h>
>>> +
>>> +#include "cam_helper.h"
>>> +
>>> +using namespace RPiController;
>>> +
>>> +class CamHelperMira220 : public CamHelper
>>> +{
>>> +public:
>>> +       CamHelperMira220();
>>> +       uint32_t gainCode(double gain) const override;
>>> +       double gain(uint32_t gainCode) const override;
>>> +       unsigned int hideFramesModeSwitch() const override;
>>
>> I think the definition of this method is missing here. I recently rebased
>> by Raspberry Pi 4 patches and saw the following issue, bisected to this
>> commit:
>>
>> [3:28:00.953283153] [4524] DEBUG IPAManager ipa_manager.cpp:309 IPA module /usr/local/lib/libcamera/ipa/ipa_rpi_vc4.so signature is valid
>> [3:28:00.953496264] [4524] DEBUG IPAProxy raspberrypi_ipa_proxy.cpp:44 initializing raspberrypi proxy in thread: loading IPA from /usr/local/lib/libcamera/ipa/ipa_rpi_vc4.so
>> [3:28:00.954332986] [4524] ERROR IPAModule ipa_module.cpp:416 ipa_rpi_vc4.so: Failed to open IPA module shared object: /usr/local/lib/libcamera/ipa/ipa_rpi_vc4.so: undefined symbol: _ZNK16CamHelperMira22020hideFramesModeSwitchEv
>> [3:28:00.954388875] [4524] ERROR IPAManager ipa_manager.h:51 Failed to load proxy
>> [3:28:00.954421579] [4524] ERROR RPI pipeline_base.cpp:830 Failed to load a suitable IPA library
>>
>> Thanks,
>>      Jai
> 
> 
> Eeek, I think it's as simple as removing that override line right? Could
> you test and send a patch please? Jacopo is on holiday so it would take
> a week otherwise, and we should fix the main branch.

If the class was defined in an anonymous namespace, the missing definition
would have been diagnosed by the compiler. One of the reasons I'm a big
proponent of putting everything into them by default if possible: more
opportunities for optimization and diagnostics.

--- gcc

../src/ipa/rpi/cam_helper/cam_helper_mira220.cpp:22:22: error: ‘virtual unsigned int {anonymous}::CamHelperMira220::hideFramesModeSwitch() const’ used but never defined [-Werror]
    22 |         unsigned int hideFramesModeSwitch() const override;
       |                      ^~~~~~~~~~~~~~~~~~~~
cc1plus: all warnings being treated as errors

--- clang

../src/ipa/rpi/cam_helper/cam_helper_mira220.cpp:22:15: error: function '(anonymous namespace)::CamHelperMira220::hideFramesModeSwitch' has internal linkage but is not defined [-Werror,-Wundefined-internal]
    22 |         unsigned int hideFramesModeSwitch() const override;
       |                      ^
../src/ipa/rpi/cam_helper/cam_helper_mira220.cpp:41:19: note: used here
    41 | CamHelperMira220::CamHelperMira220()

---


> 
> --
> Kieran

Patch
diff mbox series 

diff --git a/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp b/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp
new file mode 100644
index 000000000000..c884b853b0c1
--- /dev/null
+++ b/src/ipa/rpi/cam_helper/cam_helper_mira220.cpp
@@ -0,0 +1,59 @@ 
+/* SPDX-License-Identifier: BSD-2-Clause */
+/*
+ * Copyright (C) 2026, ams-OSRAM
+ *
+ * Camera helper for Mira220 sensor
+ */
+
+#include <assert.h>
+
+#include "cam_helper.h"
+
+using namespace RPiController;
+
+class CamHelperMira220 : public CamHelper
+{
+public:
+	CamHelperMira220();
+	uint32_t gainCode(double gain) const override;
+	double gain(uint32_t gainCode) const override;
+	unsigned int hideFramesModeSwitch() const override;
+
+private:
+	/*
+	 * Smallest difference between the frame length and integration time,
+	 * in units of lines.
+	 *
+	 * The integration diff is expressed as
+	 *    Tframe - 1928 / row_length
+	 *
+	 * row_length controls the line timings and varies according to the
+	 * number of data lanes in use and the D-PHY data rate. Use an
+	 * integration diff calculated using the value of 304, which represents
+	 * the minimum row_length for a 2 data lanes configuration running at
+	 * 1.5Gbps.
+	 */
+	static constexpr int frameIntegrationDiff = 6;
+};
+
+CamHelperMira220::CamHelperMira220()
+	: CamHelper({}, frameIntegrationDiff)
+{
+}
+
+uint32_t CamHelperMira220::gainCode(double gain) const
+{
+	return static_cast<uint32_t>(2048.0 - 2048.0 / gain);
+}
+
+double CamHelperMira220::gain(uint32_t gainCode) const
+{
+	return static_cast<double>(2048.0 / (2048 - gainCode));
+}
+
+static CamHelper *create()
+{
+	return new CamHelperMira220();
+}
+
+static RegisterCamHelper reg("mira220", &create);
diff --git a/src/ipa/rpi/cam_helper/meson.build b/src/ipa/rpi/cam_helper/meson.build
index 0aa8c3dc2e2f..cc71bacb2c6a 100644
--- a/src/ipa/rpi/cam_helper/meson.build
+++ b/src/ipa/rpi/cam_helper/meson.build
@@ -13,6 +13,7 @@  rpi_ipa_cam_helper_sources = files([
     'cam_helper_imx519.cpp',
     'cam_helper_imx678.cpp',
     'cam_helper_imx708.cpp',
+    'cam_helper_mira220.cpp',
     'cam_helper_ov64a40.cpp',
     'cam_helper_ov7251.cpp',
     'cam_helper_ov9281.cpp',