[libcamera-devel] [PATCH 04/10] ipa: libcamera: add metadata for the ov8858 sensor
Nicholas Roth
nicholas at rothemail.net
Thu Oct 27 19:25:20 CEST 2022
> please pardon me for being pedantic, but I would:
> libcamera: Add support for ov8858 sensor
>
> instead of:
> ipa: libcamera: add metadata for the ov8858 sensor
Happy to make the change.
> The sensor doesn't seem to be supported mainline :(...
> Let's start from the first point: where does this driver lives ? What
> kernel are you using ?
I'm using Manjaro's kernel 5.19.8-1-MANJARO-ARM (from uname -r).
It looks like my package manager is picking up the Manjaro kernel from
https://gitlab.manjaro.org/manjaro-arm/packages/core/linux-pinephonepro/-/blob/5.19-megi/config,
which sets "CONFIG_VIDEO_OV8858=m" in /config otherwise has no 8858-related
changes. This ultimately pulls the driver from
https://github.com/megous/linux/blob/orange-pi-5.19/drivers/media/i2c/ov8858.c
.
Looks like this has been in and out of Torvalds' peripheral view, e.g.:
https://lkml.iu.edu/hypermail/linux/kernel/1711.1/06245.html
If you think that it would be valuable to try and mainline this, I'd be
interested to do so once I can get libcamera working on my hardware, though
I'll likely need some guidance in the process, since I'll be learning a lot.
> Nice this matches the CCS defined linear gain model.
> However the sensor allows to select two formats for the analogue gain
> the "real gain" format and "sensor gain" format. The selection is made by
> register 0x3503[2] and it would be nice to validate that the driver
> uses the format that you expect.
Frankly, I'm not sure what you mean by this, but I'll try to find out from
the driver above and verify.
> Knowing what driver you're using is relevant, in example, as the
> OV5688 sensor computes exposure in 1/16 of line length. This is not
> what libcamera expects, and sensor drivers are expected to express the
> V4L2_CID_EXPOSURE control in line units.
> From Documentation/sensor_driver_requirements.rst
> While V4L2 doesn't specify a unit for the ``EXPOSURE`` control, libcamera
> requires it to be expressed as a number of image lines. Camera sensor
drivers
> that do not comply with this requirement will need to be adapted or will
produce
> incorrect results.
Let me read the driver and get back to you.
On Thu, Oct 27, 2022 at 11:41 AM Jacopo Mondi <jacopo at jmondi.org> wrote:
> Hi Nicholas,
>
> please pardon me for being pedantic, but I would:
> libcamera: Add support for ov8858 sensor
>
> instead of:
> ipa: libcamera: add metadata for the ov8858 sensor
>
> "metadata" is an overloaded already term and usually refers to the
> properties associated to a captured frame (the term comes from the
> Android lingo, but we use it too).
>
> On Thu, Oct 27, 2022 at 12:55:09AM -0500, Nicholas Roth via
> libcamera-devel wrote:
> > From: Nicholas Roth <nicholas at rothemail.net>
> >
> > Currently, libcamera does not have information for the ov8858 sensor
> > used in the PinePhone Pro, a phone designed to run Linux.
> >
> > This commit adds metadata, especially that sensor gain is reported and
> > set in 1/16 discrete increments.
> >
> > For more information, see "5.8 manual exposure compensation/ manual
> > gain compensation" in [0].
> >
> > [0] http://www.ahdsensor.com/uploadfile/202008/55322e75316871.pdf
>
> The sensor doesn't seem to be supported mainline :(
> libcamera has a policy that requires supported components to be
> upstream or at least on their way to upstream (ie posted to the
> linux-media mailing list). The policy is there because different
> downstream driver implementations might behave differently one from
> the other, making it impossible for libcamera to support the part
> fully. The policy is a strict requirement for ISPs, I guess we're a
> bit more elastic when it comes to sensor. Howver knowing what driver
> you are using, where it lives, and if there's any plan to upstream it
> would be great.
>
> Let's start from the first point: where does this driver lives ? What
> kernel are you using ?
>
> Knowing what driver you're using is relevant, in example, as the
> OV5688 sensor computes exposure in 1/16 of line length. This is not
> what libcamera expects, and sensor drivers are expected to express the
> V4L2_CID_EXPOSURE control in line units.
>
> From Documentation/sensor_driver_requirements.rst
>
> While V4L2 doesn't specify a unit for the ``EXPOSURE`` control, libcamera
> requires it to be expressed as a number of image lines. Camera sensor
> drivers
> that do not comply with this requirement will need to be adapted or will
> produce
> incorrect results.
>
> >
> > Signed-off-by: Nicholas Roth <nicholas at rothemail.net>
> > ---
> > src/ipa/libipa/camera_sensor_helper.cpp | 11 +++++++++++
> > src/libcamera/camera_sensor_properties.cpp | 14 ++++++++++++++
> > 2 files changed, 25 insertions(+)
> >
> > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp
> b/src/ipa/libipa/camera_sensor_helper.cpp
> > index 35056bec..f2040cbd 100644
> > --- a/src/ipa/libipa/camera_sensor_helper.cpp
> > +++ b/src/ipa/libipa/camera_sensor_helper.cpp
> > @@ -476,6 +476,17 @@ public:
> > };
> > REGISTER_CAMERA_SENSOR_HELPER("ov5693", CameraSensorHelperOv5693)
> >
> > +class CameraSensorHelperOv8858 : public CameraSensorHelper
> > +{
> > +public:
> > + CameraSensorHelperOv8858()
> > + {
> > + gainType_ = AnalogueGainLinear;
> > + gainConstants_.linear = { 1, 0, 0, 16 };
> > + }
> > +};
>
> Nice this matches the CCS defined linear gain model.
> However the sensor allows to select two formats for the analogue gain
> the "real gain" format and "sensor gain" format. The selection is made by
> register 0x3503[2] and it would be nice to validate that the driver
> uses the format that you expect.
>
> > +REGISTER_CAMERA_SENSOR_HELPER("m00_f_ov8858", CameraSensorHelperOv8858)
>
> Ah! that's a weird name for the sensor entity :)
>
> > +
> > class CameraSensorHelperOv8865 : public CameraSensorHelper
> > {
> > public:
> > diff --git a/src/libcamera/camera_sensor_properties.cpp
> b/src/libcamera/camera_sensor_properties.cpp
> > index e5f27f06..d0757c15 100644
> > --- a/src/libcamera/camera_sensor_properties.cpp
> > +++ b/src/libcamera/camera_sensor_properties.cpp
> > @@ -146,6 +146,20 @@ const CameraSensorProperties
> *CameraSensorProperties::get(const std::string &sen
> > */
> > },
> > } },
> > + { "m00_f_ov8858", {
> > + .unitCellSize = { 1200, 1200 },
>
> I read 1.12 um x 1.12 um
>
> > + .testPatternModes = {
> > + { controls::draft::TestPatternModeOff, 0 },
> > + {
> controls::draft::TestPatternModeColorBars, 1 },
> > + /*
> > + * No best corresponding test pattern for:
> > + * 1: "Vertical Color Bar Type 1",
> > + * 2: "Vertical Color Bar Type 2",
> > + * 3: "Vertical Color Bar Type 3",
> > + * 4: "Vertical Color Bar Type 4"
> > + */
> > + },
> > + } },
> > { "ov8865", {
> > .unitCellSize = { 1400, 1400 },
> > .testPatternModes = {
> > --
> > 2.34.1
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20221027/2452c1a5/attachment.htm>
More information about the libcamera-devel
mailing list