| Message ID | 20241219005958.16856-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Thu, Dec 19, 2024 at 12:59:58AM +0000, Kieran Bingham wrote: > The OV5675 uses different delays for gain and exposure than are configured > in the default sensorDelays utilised by the CameraSensorLegacy. > > Empirical testing using a Lenovo X13s shows that the exposure delay is > only a single frame, and the current setting of 2 frame delay produces > exceedingly frequent oscillations in the image exposure. Have you been able to test the other delays ? If not I'd like at least a comment here that indicates this needs to be checked. Any volunteer to write a measurement tool ? > Update the OV5675 sensor delays table accordingly. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/libcamera/sensor/camera_sensor_properties.cpp | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp > index bd1fc86977ce..813878386a35 100644 > --- a/src/libcamera/sensor/camera_sensor_properties.cpp > +++ b/src/libcamera/sensor/camera_sensor_properties.cpp > @@ -360,7 +360,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { controls::draft::TestPatternModeOff, 0 }, > { controls::draft::TestPatternModeColorBars, 1 }, > }, > - .sensorDelays = { }, > + .sensorDelays = { > + .exposureDelay = 1, > + .gainDelay = 1, > + .vblankDelay = 2, > + .hblankDelay = 2 > + }, > } }, > { "ov5693", { > .unitCellSize = { 1400, 1400 },
Hi Kieran, On Thu, 19 Dec 2024 at 01:00, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > > The OV5675 uses different delays for gain and exposure than are configured > in the default sensorDelays utilised by the CameraSensorLegacy. > > Empirical testing using a Lenovo X13s shows that the exposure delay is > only a single frame, and the current setting of 2 frame delay produces > exceedingly frequent oscillations in the image exposure. > > Update the OV5675 sensor delays table accordingly. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/libcamera/sensor/camera_sensor_properties.cpp | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp > index bd1fc86977ce..813878386a35 100644 > --- a/src/libcamera/sensor/camera_sensor_properties.cpp > +++ b/src/libcamera/sensor/camera_sensor_properties.cpp > @@ -360,7 +360,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { controls::draft::TestPatternModeOff, 0 }, > { controls::draft::TestPatternModeColorBars, 1 }, > }, > - .sensorDelays = { }, > + .sensorDelays = { > + .exposureDelay = 1, From my experience, an exposure delay of 1 is very unlikely with a rolling shutter sensor because of the integration overlap between successive frames. Of course, I could be wrong since I've not encounter this sensor before. Perhaps it's worth looking at other delays to see if the general interaction between vblank/exposure/gain is causing the oscillations? Regards, Naush > + .gainDelay = 1, > + .vblankDelay = 2, > + .hblankDelay = 2 > + }, > } }, > { "ov5693", { > .unitCellSize = { 1400, 1400 }, > -- > 2.47.1 >
Hi, On 19-Dec-24 10:33 AM, Naushir Patuck wrote: > Hi Kieran, > > On Thu, 19 Dec 2024 at 01:00, Kieran Bingham > <kieran.bingham@ideasonboard.com> wrote: >> >> The OV5675 uses different delays for gain and exposure than are configured >> in the default sensorDelays utilised by the CameraSensorLegacy. >> >> Empirical testing using a Lenovo X13s shows that the exposure delay is >> only a single frame, and the current setting of 2 frame delay produces >> exceedingly frequent oscillations in the image exposure. >> >> Update the OV5675 sensor delays table accordingly. >> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> --- >> src/libcamera/sensor/camera_sensor_properties.cpp | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp >> index bd1fc86977ce..813878386a35 100644 >> --- a/src/libcamera/sensor/camera_sensor_properties.cpp >> +++ b/src/libcamera/sensor/camera_sensor_properties.cpp >> @@ -360,7 +360,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen >> { controls::draft::TestPatternModeOff, 0 }, >> { controls::draft::TestPatternModeColorBars, 1 }, >> }, >> - .sensorDelays = { }, >> + .sensorDelays = { >> + .exposureDelay = 1, > > From my experience, an exposure delay of 1 is very unlikely with a > rolling shutter sensor because of the integration overlap between > successive frames. Of course, I could be wrong since I've not > encounter this sensor before. Perhaps it's worth looking at other > delays to see if the general interaction between vblank/exposure/gain > is causing the oscillations? I wonder if this is caused by the simple pipeline handler applying new controls coming from the software ISP IPA immediately rather then waiting for the start of the next frame ? This issue is fixed by: https://patchwork.libcamera.org/patch/22397/ Kieran can you test if that maybe fixes the oscillation without needing to change the delays ? p.s. regardless of it helping, that patch is ready for merging. Regards, Hans
diff --git a/src/libcamera/sensor/camera_sensor_properties.cpp b/src/libcamera/sensor/camera_sensor_properties.cpp index bd1fc86977ce..813878386a35 100644 --- a/src/libcamera/sensor/camera_sensor_properties.cpp +++ b/src/libcamera/sensor/camera_sensor_properties.cpp @@ -360,7 +360,12 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen { controls::draft::TestPatternModeOff, 0 }, { controls::draft::TestPatternModeColorBars, 1 }, }, - .sensorDelays = { }, + .sensorDelays = { + .exposureDelay = 1, + .gainDelay = 1, + .vblankDelay = 2, + .hblankDelay = 2 + }, } }, { "ov5693", { .unitCellSize = { 1400, 1400 },
The OV5675 uses different delays for gain and exposure than are configured in the default sensorDelays utilised by the CameraSensorLegacy. Empirical testing using a Lenovo X13s shows that the exposure delay is only a single frame, and the current setting of 2 frame delay produces exceedingly frequent oscillations in the image exposure. Update the OV5675 sensor delays table accordingly. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- src/libcamera/sensor/camera_sensor_properties.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)