[libcamera-devel,3/4] ipa: raspberrypi: Add camera helper for Sony IMX296 sensor

15169 diff mbox series

From: Naushir Patuck <naush@raspberrypi.com>

The Sony IMX296 sensor has an exponential gain model, and adds a fixed
14.26µs offset to the exposure time expressed in line units.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/ipa/raspberrypi/cam_helper_imx296.cpp | 126 ++++++++++++++++++++++
 src/ipa/raspberrypi/meson.build           |   1 +
 2 files changed, 127 insertions(+)
 create mode 100644 src/ipa/raspberrypi/cam_helper_imx296.cpp

Hi Laurent,

Thank you for your work.

On Sun, 19 Dec 2021 at 23:27, Laurent Pinchart <
laurent.pinchart@ideasonboard.com> wrote:

> From: Naushir Patuck <naush@raspberrypi.com>
>
> The Sony IMX296 sensor has an exponential gain model, and adds a fixed
> 14.26µs offset to the exposure time expressed in line units.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/ipa/raspberrypi/cam_helper_imx296.cpp | 126 ++++++++++++++++++++++
>  src/ipa/raspberrypi/meson.build           |   1 +
>  2 files changed, 127 insertions(+)
>  create mode 100644 src/ipa/raspberrypi/cam_helper_imx296.cpp
>
> diff --git a/src/ipa/raspberrypi/cam_helper_imx296.cpp
> b/src/ipa/raspberrypi/cam_helper_imx296.cpp
> new file mode 100644
> index 000000000000..9c34582861f1
> --- /dev/null
> +++ b/src/ipa/raspberrypi/cam_helper_imx296.cpp
> @@ -0,0 +1,126 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2020, Raspberry Pi (Trading) Limited
> + *
> + * cam_helper_imx296.cpp - camera helper for imx296 sensor
> + */
> +
> +#include <assert.h>
> +#include <cmath>
> +#include <stddef.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +/*
> + * We have observed the imx296 embedded data stream randomly return junk
> + * reister values.  Do not rely on embedded data until this has been
> resolved.
> + */
> +#define ENABLE_EMBEDDED_DATA 0
>

I'm afraid this may not be completely true :-)

This file was created by doing a search/replace from the imx219 cam helper.
Perhaps it might be better to completely remove embedded data support
until we have a chance to properly evaluate this functionality on the
imx296?

Either way,

Signed-off-by: Naushir Patuck <naush <naushir@gmail.com>@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush <naushir@gmail.com>@raspberrypi.com>



> +
> +#include "cam_helper.hpp"
> +#if ENABLE_EMBEDDED_DATA
> +#include "md_parser.hpp"
> +#endif
> +
> +//#include "imx296_gain_table.hpp"
> +
> +using namespace RPiController;
> +using libcamera::utils::Duration;
> +using namespace std::literals::chrono_literals;
> +
> +/*
> + * We care about one gain register and a pair of exposure registers.
> Their I2C
> + * addresses from the Sony IMX296 datasheet:
> + */
> +constexpr uint32_t gainReg = 0x157;
> +constexpr uint32_t expHiReg = 0x15a;
> +constexpr uint32_t expLoReg = 0x15b;
> +constexpr std::initializer_list<uint32_t> registerList [[maybe_unused]] =
> { expHiReg, expLoReg, gainReg };
> +
> +class CamHelperImx296 : public CamHelper
> +{
> +public:
> +       CamHelperImx296();
> +       uint32_t GainCode(double gain) const override;
> +       double Gain(uint32_t gain_code) const override;
> +       uint32_t ExposureLines(Duration exposure) const override;
> +       Duration Exposure(uint32_t exposure_lines) const override;
> +       unsigned int MistrustFramesModeSwitch() const override;
> +       bool SensorEmbeddedDataPresent() const override;
> +
> +private:
> +       static constexpr uint32_t maxGainCode = 239;
> +       static constexpr Duration timePerLine = 550.0 / 37.125e6 * 1.0s;
> +
> +       /*
> +        * Smallest difference between the frame length and integration
> time,
> +        * in units of lines.
> +        */
> +       static constexpr int frameIntegrationDiff = 4;
> +
> +       void PopulateMetadata(const MdParser::RegisterMap &registers,
> +                             Metadata &metadata) const override;
> +};
> +
> +CamHelperImx296::CamHelperImx296()
> +#if ENABLE_EMBEDDED_DATA
> +       : CamHelper(std::make_unique<MdParserSmia>(registerList),
> frameIntegrationDiff)
> +#else
> +       : CamHelper(nullptr, frameIntegrationDiff)
> +#endif
> +{
> +}
> +
> +uint32_t CamHelperImx296::GainCode(double gain) const
> +{
> +       uint32_t code = 20 * std::log10(gain) * 10;
> +       return std::min(code, maxGainCode);
> +}
> +
> +double CamHelperImx296::Gain(uint32_t gain_code) const
> +{
> +       return std::pow(10.0, gain_code / 200.0);
> +}
> +
> +uint32_t CamHelperImx296::ExposureLines(Duration exposure) const
> +{
> +       return (exposure - 14.26us) / timePerLine;
> +}
> +
> +Duration CamHelperImx296::Exposure(uint32_t exposure_lines) const
> +{
> +       return exposure_lines * timePerLine + 14.26us;
> +}
> +
> +unsigned int CamHelperImx296::MistrustFramesModeSwitch() const
> +{
> +       /*
> +        * For reasons unknown, we do occasionally get a bogus metadata
> frame
> +        * at a mode switch (though not at start-up). Possibly warrants
> some
> +        * investigation, though not a big deal.
> +        */
> +       return 1;
>

This may also be something to evaluate (assuming metadata gets enabled).


> +}
> +
> +bool CamHelperImx296::SensorEmbeddedDataPresent() const
> +{
> +       return ENABLE_EMBEDDED_DATA;
> +}
> +
> +void CamHelperImx296::PopulateMetadata(const MdParser::RegisterMap
> &registers,
> +                                      Metadata &metadata) const
> +{
> +       DeviceStatus deviceStatus;
> +
> +       deviceStatus.shutter_speed = Exposure(registers.at(expHiReg) *
> 256 + registers.at(expLoReg));
> +       deviceStatus.analogue_gain = Gain(registers.at(gainReg));
> +
> +       metadata.Set("device.status", deviceStatus);
> +}
> +
> +static CamHelper *Create()
> +{
> +       return new CamHelperImx296();
> +}
> +
> +static RegisterCamHelper reg("imx296", &Create);
> diff --git a/src/ipa/raspberrypi/meson.build
> b/src/ipa/raspberrypi/meson.build
> index 176055f42248..32897e07dad9 100644
> --- a/src/ipa/raspberrypi/meson.build
> +++ b/src/ipa/raspberrypi/meson.build
> @@ -21,6 +21,7 @@ rpi_ipa_sources = files([
>      'cam_helper_ov5647.cpp',
>      'cam_helper_imx219.cpp',
>      'cam_helper_imx290.cpp',
> +    'cam_helper_imx296.cpp',
>      'cam_helper_imx477.cpp',
>      'cam_helper_imx519.cpp',
>      'cam_helper_ov9281.cpp',
> --
> Regards,
>
> Laurent Pinchart
>
>

Hi Naush,

On Mon, Dec 20, 2021 at 07:41:37AM +0000, Naushir Patuck wrote:
> On Sun, 19 Dec 2021 at 23:27, Laurent Pinchart wrote:
> 
> > From: Naushir Patuck <naush@raspberrypi.com>
> >
> > The Sony IMX296 sensor has an exponential gain model, and adds a fixed
> > 14.26µs offset to the exposure time expressed in line units.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/ipa/raspberrypi/cam_helper_imx296.cpp | 126 ++++++++++++++++++++++
> >  src/ipa/raspberrypi/meson.build           |   1 +
> >  2 files changed, 127 insertions(+)
> >  create mode 100644 src/ipa/raspberrypi/cam_helper_imx296.cpp
> >
> > diff --git a/src/ipa/raspberrypi/cam_helper_imx296.cpp
> > b/src/ipa/raspberrypi/cam_helper_imx296.cpp
> > new file mode 100644
> > index 000000000000..9c34582861f1
> > --- /dev/null
> > +++ b/src/ipa/raspberrypi/cam_helper_imx296.cpp
> > @@ -0,0 +1,126 @@
> > +/* SPDX-License-Identifier: BSD-2-Clause */
> > +/*
> > + * Copyright (C) 2020, Raspberry Pi (Trading) Limited
> > + *
> > + * cam_helper_imx296.cpp - camera helper for imx296 sensor
> > + */
> > +
> > +#include <assert.h>
> > +#include <cmath>
> > +#include <stddef.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +
> > +/*
> > + * We have observed the imx296 embedded data stream randomly return junk
> > + * reister values.  Do not rely on embedded data until this has been resolved.
> > + */
> > +#define ENABLE_EMBEDDED_DATA 0
> 
> I'm afraid this may not be completely true :-)
> 
> This file was created by doing a search/replace from the imx219 cam helper.
> Perhaps it might be better to completely remove embedded data support
> until we have a chance to properly evaluate this functionality on the
> imx296?

Sounds good to me. I'll drop it.

> Either way,
> 
> Signed-off-by: Naushir Patuck <naush <naushir@gmail.com>@raspberrypi.com>
> Reviewed-by: Naushir Patuck <naush <naushir@gmail.com>@raspberrypi.com>
> 
> > +
> > +#include "cam_helper.hpp"
> > +#if ENABLE_EMBEDDED_DATA
> > +#include "md_parser.hpp"
> > +#endif
> > +
> > +//#include "imx296_gain_table.hpp"
> > +
> > +using namespace RPiController;
> > +using libcamera::utils::Duration;
> > +using namespace std::literals::chrono_literals;
> > +
> > +/*
> > + * We care about one gain register and a pair of exposure registers. Their I2C
> > + * addresses from the Sony IMX296 datasheet:
> > + */
> > +constexpr uint32_t gainReg = 0x157;
> > +constexpr uint32_t expHiReg = 0x15a;
> > +constexpr uint32_t expLoReg = 0x15b;
> > +constexpr std::initializer_list<uint32_t> registerList [[maybe_unused]] =
> > { expHiReg, expLoReg, gainReg };
> > +
> > +class CamHelperImx296 : public CamHelper
> > +{
> > +public:
> > +       CamHelperImx296();
> > +       uint32_t GainCode(double gain) const override;
> > +       double Gain(uint32_t gain_code) const override;
> > +       uint32_t ExposureLines(Duration exposure) const override;
> > +       Duration Exposure(uint32_t exposure_lines) const override;
> > +       unsigned int MistrustFramesModeSwitch() const override;
> > +       bool SensorEmbeddedDataPresent() const override;
> > +
> > +private:
> > +       static constexpr uint32_t maxGainCode = 239;
> > +       static constexpr Duration timePerLine = 550.0 / 37.125e6 * 1.0s;
> > +
> > +       /*
> > +        * Smallest difference between the frame length and integration time,
> > +        * in units of lines.
> > +        */
> > +       static constexpr int frameIntegrationDiff = 4;
> > +
> > +       void PopulateMetadata(const MdParser::RegisterMap &registers,
> > +                             Metadata &metadata) const override;
> > +};
> > +
> > +CamHelperImx296::CamHelperImx296()
> > +#if ENABLE_EMBEDDED_DATA
> > +       : CamHelper(std::make_unique<MdParserSmia>(registerList),
> > frameIntegrationDiff)
> > +#else
> > +       : CamHelper(nullptr, frameIntegrationDiff)
> > +#endif
> > +{
> > +}
> > +
> > +uint32_t CamHelperImx296::GainCode(double gain) const
> > +{
> > +       uint32_t code = 20 * std::log10(gain) * 10;
> > +       return std::min(code, maxGainCode);
> > +}
> > +
> > +double CamHelperImx296::Gain(uint32_t gain_code) const
> > +{
> > +       return std::pow(10.0, gain_code / 200.0);
> > +}
> > +
> > +uint32_t CamHelperImx296::ExposureLines(Duration exposure) const
> > +{
> > +       return (exposure - 14.26us) / timePerLine;
> > +}
> > +
> > +Duration CamHelperImx296::Exposure(uint32_t exposure_lines) const
> > +{
> > +       return exposure_lines * timePerLine + 14.26us;
> > +}
> > +
> > +unsigned int CamHelperImx296::MistrustFramesModeSwitch() const
> > +{
> > +       /*
> > +        * For reasons unknown, we do occasionally get a bogus metadata frame
> > +        * at a mode switch (though not at start-up). Possibly warrants some
> > +        * investigation, though not a big deal.
> > +        */
> > +       return 1;
> 
> This may also be something to evaluate (assuming metadata gets enabled).

I'll drop it too.

> > +}
> > +
> > +bool CamHelperImx296::SensorEmbeddedDataPresent() const
> > +{
> > +       return ENABLE_EMBEDDED_DATA;
> > +}
> > +
> > +void CamHelperImx296::PopulateMetadata(const MdParser::RegisterMap &registers,
> > +                                      Metadata &metadata) const
> > +{
> > +       DeviceStatus deviceStatus;
> > +
> > +       deviceStatus.shutter_speed = Exposure(registers.at(expHiReg) * 256 + registers.at(expLoReg));
> > +       deviceStatus.analogue_gain = Gain(registers.at(gainReg));
> > +
> > +       metadata.Set("device.status", deviceStatus);
> > +}
> > +
> > +static CamHelper *Create()
> > +{
> > +       return new CamHelperImx296();
> > +}
> > +
> > +static RegisterCamHelper reg("imx296", &Create);
> > diff --git a/src/ipa/raspberrypi/meson.build
> > b/src/ipa/raspberrypi/meson.build
> > index 176055f42248..32897e07dad9 100644
> > --- a/src/ipa/raspberrypi/meson.build
> > +++ b/src/ipa/raspberrypi/meson.build
> > @@ -21,6 +21,7 @@ rpi_ipa_sources = files([
> >      'cam_helper_ov5647.cpp',
> >      'cam_helper_imx219.cpp',
> >      'cam_helper_imx290.cpp',
> > +    'cam_helper_imx296.cpp',
> >      'cam_helper_imx477.cpp',
> >      'cam_helper_imx519.cpp',
> >      'cam_helper_ov9281.cpp',