| Message ID | 20260910172120.73148-2-dmanresa@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
On Thu, Sep 10, 2026 at 07:21:19PM +0200, D. Manresa wrote: > The OV5670 has a black level of 64 at 10 bits, the sensor's default BLC > target. This was confirmed on a Dell Latitude 7275 by reading the ImgU > statistics and dark frames with different black level corrections. That's not the right way to measure the data pedestal. > Provide it in the camera sensor helper so that IPAs can use it instead of > hard-coded or tuning-file values. > > Signed-off-by: D. Manresa <dmanresa@gmail.com> > --- > src/ipa/libipa/camera_sensor_helper.cpp | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp > index 82bf255..eeb3277 100644 > --- a/src/ipa/libipa/camera_sensor_helper.cpp > +++ b/src/ipa/libipa/camera_sensor_helper.cpp > @@ -802,6 +802,8 @@ class CameraSensorHelperOv5670 : public CameraSensorHelper > public: > CameraSensorHelperOv5670() > { > + /* Default BLC target of 64 at 10bits, confirmed by measurement. */ > + blackLevel_ = 4096; > gain_ = AnalogueGainLinear{ 1, 0, 0, 128 }; > } > };
Hi Laurent, On Thu, 10 Sep 2026, Laurent Pinchart wrote: > > The OV5670 has a black level of 64 at 10 bits, the sensor's default BLC > > target. This was confirmed on a Dell Latitude 7275 by reading the ImgU > > statistics and dark frames with different black level corrections. > > That's not the right way to measure the data pedestal. You are right, and it is worse than merely imprecise: the ISP output I looked at is produced by the very correction that the second patch then configures from this value, so it could not confirm anything. What this camera module does come with is a black level characterisation in the OEM tuning data, measured by the vendor over six exposure times and five gains for the four Bayer channels: 64.1 to 64.4 at unity gain, and within 62.0 to 64.5 up to a gain of 15.9. v4 cites that instead, and the sentence you quoted is gone: https://lore.kernel.org/linux-media/20260911153918.96470-1-dmanresa@gmail.com/ If you would rather see this backed by the datasheet default, I do not have the OV5670 datasheet, so I would need someone who does to confirm the register value. Thanks, D.
On Fri, Sep 11, 2026 at 05:39:51PM +0200, D. Manresa wrote: > Hi Laurent, > > On Thu, 10 Sep 2026, Laurent Pinchart wrote: > > > The OV5670 has a black level of 64 at 10 bits, the sensor's default BLC > > > target. This was confirmed on a Dell Latitude 7275 by reading the ImgU > > > statistics and dark frames with different black level corrections. > > > > That's not the right way to measure the data pedestal. > > You are right, and it is worse than merely imprecise: the ISP output I > looked at is produced by the very correction that the second patch then > configures from this value, so it could not confirm anything. I have a very hard time not reading this as AI slop. > What this camera module does come with is a black level characterisation > in the OEM tuning data, measured by the vendor over six exposure times > and five gains for the four Bayer channels: 64.1 to 64.4 at unity gain, > and within 62.0 to 64.5 up to a gain of 15.9. v4 cites that instead, and > the sentence you quoted is gone: > > https://lore.kernel.org/linux-media/20260911153918.96470-1-dmanresa@gmail.com/ > > If you would rather see this backed by the datasheet default, I do not > have the OV5670 datasheet, so I would need someone who does to confirm > the register value. > > Thanks, > D.
On Fri, 11 Sep 2026, Laurent Pinchart wrote:
> I have a very hard time not reading this as AI slop.
I direct this work and test it on my own hardware. I use an AI assistant to
write it up, and I should have said so on this list, as I do in my kernel
patches.
v4 drops the sentence you objected to and cites the vendor characterisation
instead. If you would rather have the datasheet value, I do not have the
OV5670 datasheet.
D. Manresa <dmanresa@gmail.com>
On Fri, Sep 11, 2026 at 06:52:46PM +0200, D. Manresa wrote: > On Fri, 11 Sep 2026, Laurent Pinchart wrote: > > I have a very hard time not reading this as AI slop. > > I direct this work and test it on my own hardware. I use an AI assistant to > write it up, and I should have said so on this list, as I do in my kernel > patches. I'm drafting an AI policy for libcamera that I hope to post to the list within the next few weeks, before leaving for LPC and OSS EU in Prague. We will most likely disallow the use of generative AI for libcamera development to produce any material meant to be read by humans. This will cover source code, documentation, bug reports and e-mails. Of course you couldn't have known as the policy isn't public yet, but regardless of the context of libcamera, I personally find being sent walls of AI-generated texts fairly rude. > v4 drops the sentence you objected to and cites the vendor characterisation > instead. If you would rather have the datasheet value, I do not have the > OV5670 datasheet. > > D. Manresa <dmanresa@gmail.com>
diff --git a/src/ipa/libipa/camera_sensor_helper.cpp b/src/ipa/libipa/camera_sensor_helper.cpp index 82bf255..eeb3277 100644 --- a/src/ipa/libipa/camera_sensor_helper.cpp +++ b/src/ipa/libipa/camera_sensor_helper.cpp @@ -802,6 +802,8 @@ class CameraSensorHelperOv5670 : public CameraSensorHelper public: CameraSensorHelperOv5670() { + /* Default BLC target of 64 at 10bits, confirmed by measurement. */ + blackLevel_ = 4096; gain_ = AnalogueGainLinear{ 1, 0, 0, 128 }; } };
The OV5670 has a black level of 64 at 10 bits, the sensor's default BLC target. This was confirmed on a Dell Latitude 7275 by reading the ImgU statistics and dark frames with different black level corrections. Provide it in the camera sensor helper so that IPAs can use it instead of hard-coded or tuning-file values. Signed-off-by: D. Manresa <dmanresa@gmail.com> --- src/ipa/libipa/camera_sensor_helper.cpp | 2 ++ 1 file changed, 2 insertions(+)