| Message ID | 20261019184526.252977-1-barnabas.pocze@ideasonboard.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Barnabas On Mon, Oct 19, 2026 at 08:45:07PM +0200, Barnabás Pőcze wrote: > The maximum available exposure depends on the frame duration, but this > is not considered by the current agc implementation, it fixes the maximum > exposure time based on whatever configuration was used to configure the > algorithm. This means that in certain situations the full exposure range > might not be used, even in dark scenes. > > This a third attempt at adapting https://patchwork.libcamera.org/project/libcamera/list/?series=5590 > on the current master branch. But now that the agc handling is largely unified, > fewer changes are needed to make this work on every platform. > > So effectively only patch 7 is taken directly, but only the exposure margin > introduction part of it. > > This was mainly tested on rkisp1+imx219, softisp+ov2740, rppx1+imx708. > > Unfortunately, back to RFC this time, mainly due to https://gitlab.freedesktop.org/camera/libcamera/-/work_items/362 > It turns out to be a bit problematic. So this version adds significant changes: > > * last exposure stage constitutes a hard-limit for both exposure time and gain, > this matches the behaviour of the raspberry pi ipa (see AgcChannel::computeTargetExposure() ) > ( https://gitlab.freedesktop.org/camera/libcamera/-/work_items/362 ) I think that's fine a desirable. We had several attempts on the list to introduce artificial limits for exposure and gain stages from configuration file https://patchwork.libcamera.org/patch/28144/ https://patchwork.libcamera.org/patch/26665/ if I'm not mistaken using agc stages might support the above use cases ? > > * empty stage list is no longer supported, instead two new exposure modes are introduced: > minimize and maximize, which implement the very simple behaviour of either minimizing > or maximizing the exposure time, respectively; these modes are always available, > even without any specific tuning, and this effectively preserves the current behaviour So if no gain stages are specified, which mode is used ? Maximize or Minimize ? > > * the default max frame duration is now determined based on the largest > exposure time among all stages (if any), otherwise it is capped to > the arbitrary limit of 100ms ~ 10fps Honestly, now that we support stages, and one can easily use them to limit the exposure time, I would lift any artificial limitation, we'll never find a default that satisfies everyone, so it's maybe better to let the algorithm adjust and suggest to use stages to limit it ? > > changes in v2: > * report `FrameDurationLimits` in metadata > * add exposure margin to more sensor helpers > > v2: https://patchwork.libcamera.org/cover/28367/ > v1: https://patchwork.libcamera.org/cover/28111/ > > Barnabás Pőcze (18): > ipa: libipa: exposure_mode_helper: Remove defaulted destructor > ipa: libipa: exposure_mode_helper: Keep stage params together > ipa: libipa: exposure_mode_helper: Remove limit getters > ipa: libipa: exposure_mode_helper: Move `stageGain` variable > ipa: libipa: exposure_mode_helper: Handle no stages > ipa: libipa: exposure_mode_helper: Consider last stage as hard-limit > ipa: libipa: exposure_mode_helper: Add separate maximize/minimize > modes > libcamera: controls: Add two new exposure modes > ipa: libipa: agc: Keep frame duration limits ordered > ipa: libipa: agc: Retrieve `FrameDurationLimits` earlier > ipa: libipa: agc: Report `FrameDurationLimits` > ipa: libipa: agc: Calculate vblank and frame duration sooner > ipa: libipa: agc: Take parameters from active state for calculation > libcamera: pipeline: Set vblank on more platforms > ipa: libipa: agc: Rework frame duration limit calculation > ipa: libipa: agc: Move control info map setup > ipa: libipa: exposure_mode_helper: Add getter for max exp time > ipa: libipa: agc: Take exposure margin into account > > Jacopo Mondi (1): > ipa: camera_sensor_helper: Introduce exposure margin > > src/ipa/ipu3/algorithms/agc.cpp | 2 +- > src/ipa/ipu3/ipu3.cpp | 3 +- > src/ipa/libipa/agc.cpp | 311 +++++++++++-------- > src/ipa/libipa/agc.h | 18 +- > src/ipa/libipa/agc_mean_luminance.cpp | 37 ++- > src/ipa/libipa/camera_sensor_helper.cpp | 68 ++++ > src/ipa/libipa/camera_sensor_helper.h | 2 + > src/ipa/libipa/exposure_mode_helper.cpp | 231 +++++++++----- > src/ipa/libipa/exposure_mode_helper.h | 50 ++- > src/ipa/mali-c55/algorithms/agc.cpp | 2 +- > src/ipa/mali-c55/mali-c55.cpp | 3 +- > src/ipa/rkisp1/algorithms/agc.cpp | 2 +- > src/ipa/rkisp1/rkisp1.cpp | 4 +- > src/ipa/softisp/algorithms/agc.cpp | 2 +- > src/ipa/softisp/softisp.cpp | 3 +- > src/libcamera/control_ids_core.yaml | 6 + > src/libcamera/pipeline/ipu3/ipu3.cpp | 1 + > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 1 + > src/libcamera/pipeline/simple/simple.cpp | 1 + > 19 files changed, 500 insertions(+), 247 deletions(-) > > -- > 2.56.0
2026. 10. 20. 14:49 keltezéssel, Jacopo Mondi írta: > Hi Barnabas > > On Mon, Oct 19, 2026 at 08:45:07PM +0200, Barnabás Pőcze wrote: >> The maximum available exposure depends on the frame duration, but this >> is not considered by the current agc implementation, it fixes the maximum >> exposure time based on whatever configuration was used to configure the >> algorithm. This means that in certain situations the full exposure range >> might not be used, even in dark scenes. >> >> This a third attempt at adapting https://patchwork.libcamera.org/project/libcamera/list/?series=5590 >> on the current master branch. But now that the agc handling is largely unified, >> fewer changes are needed to make this work on every platform. >> >> So effectively only patch 7 is taken directly, but only the exposure margin >> introduction part of it. >> >> This was mainly tested on rkisp1+imx219, softisp+ov2740, rppx1+imx708. >> >> Unfortunately, back to RFC this time, mainly due to https://gitlab.freedesktop.org/camera/libcamera/-/work_items/362 >> It turns out to be a bit problematic. So this version adds significant changes: >> >> * last exposure stage constitutes a hard-limit for both exposure time and gain, >> this matches the behaviour of the raspberry pi ipa (see AgcChannel::computeTargetExposure() ) >> ( https://gitlab.freedesktop.org/camera/libcamera/-/work_items/362 ) > > I think that's fine a desirable. > > We had several attempts on the list to introduce artificial limits for > exposure and gain stages from configuration file > https://patchwork.libcamera.org/patch/28144/ I believe this one is not entirely related because it is about awb gains. > https://patchwork.libcamera.org/patch/26665/ This is probably covered by the AgcMeanLuminance tuning, yes. > > if I'm not mistaken using agc stages might support the above use > cases ? > >> >> * empty stage list is no longer supported, instead two new exposure modes are introduced: >> minimize and maximize, which implement the very simple behaviour of either minimizing >> or maximizing the exposure time, respectively; these modes are always available, >> even without any specific tuning, and this effectively preserves the current behaviour > > So if no gain stages are specified, which mode is used ? Maximize or > Minimize ? Maximize - which matches the current behaviour. > >> >> * the default max frame duration is now determined based on the largest >> exposure time among all stages (if any), otherwise it is capped to >> the arbitrary limit of 100ms ~ 10fps > > Honestly, now that we support stages, and one can easily use them to > limit the exposure time, I would lift any artificial limitation, we'll > never find a default that satisfies everyone, so it's maybe better to > let the algorithm adjust and suggest to use stages to limit it ? I would still want to have a limit just to ensure that things are somewhat reasonable even without any tuning, just for first impressions' sake. > >> >> changes in v2: >> * report `FrameDurationLimits` in metadata >> * add exposure margin to more sensor helpers >> >> v2: https://patchwork.libcamera.org/cover/28367/ >> v1: https://patchwork.libcamera.org/cover/28111/ >> >> Barnabás Pőcze (18): >> ipa: libipa: exposure_mode_helper: Remove defaulted destructor >> ipa: libipa: exposure_mode_helper: Keep stage params together >> ipa: libipa: exposure_mode_helper: Remove limit getters >> ipa: libipa: exposure_mode_helper: Move `stageGain` variable >> ipa: libipa: exposure_mode_helper: Handle no stages >> ipa: libipa: exposure_mode_helper: Consider last stage as hard-limit >> ipa: libipa: exposure_mode_helper: Add separate maximize/minimize >> modes >> libcamera: controls: Add two new exposure modes >> ipa: libipa: agc: Keep frame duration limits ordered >> ipa: libipa: agc: Retrieve `FrameDurationLimits` earlier >> ipa: libipa: agc: Report `FrameDurationLimits` >> ipa: libipa: agc: Calculate vblank and frame duration sooner >> ipa: libipa: agc: Take parameters from active state for calculation >> libcamera: pipeline: Set vblank on more platforms >> ipa: libipa: agc: Rework frame duration limit calculation >> ipa: libipa: agc: Move control info map setup >> ipa: libipa: exposure_mode_helper: Add getter for max exp time >> ipa: libipa: agc: Take exposure margin into account >> >> Jacopo Mondi (1): >> ipa: camera_sensor_helper: Introduce exposure margin >> >> src/ipa/ipu3/algorithms/agc.cpp | 2 +- >> src/ipa/ipu3/ipu3.cpp | 3 +- >> src/ipa/libipa/agc.cpp | 311 +++++++++++-------- >> src/ipa/libipa/agc.h | 18 +- >> src/ipa/libipa/agc_mean_luminance.cpp | 37 ++- >> src/ipa/libipa/camera_sensor_helper.cpp | 68 ++++ >> src/ipa/libipa/camera_sensor_helper.h | 2 + >> src/ipa/libipa/exposure_mode_helper.cpp | 231 +++++++++----- >> src/ipa/libipa/exposure_mode_helper.h | 50 ++- >> src/ipa/mali-c55/algorithms/agc.cpp | 2 +- >> src/ipa/mali-c55/mali-c55.cpp | 3 +- >> src/ipa/rkisp1/algorithms/agc.cpp | 2 +- >> src/ipa/rkisp1/rkisp1.cpp | 4 +- >> src/ipa/softisp/algorithms/agc.cpp | 2 +- >> src/ipa/softisp/softisp.cpp | 3 +- >> src/libcamera/control_ids_core.yaml | 6 + >> src/libcamera/pipeline/ipu3/ipu3.cpp | 1 + >> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 1 + >> src/libcamera/pipeline/simple/simple.cpp | 1 + >> 19 files changed, 500 insertions(+), 247 deletions(-) >> >> -- >> 2.56.0
Hi Barnabas On Tue, Oct 20, 2026 at 02:59:08PM +0200, Barnabás Pőcze wrote: > 2026. 10. 20. 14:49 keltezéssel, Jacopo Mondi írta: > > Hi Barnabas > > > > On Mon, Oct 19, 2026 at 08:45:07PM +0200, Barnabás Pőcze wrote: > > > The maximum available exposure depends on the frame duration, but this > > > is not considered by the current agc implementation, it fixes the maximum > > > exposure time based on whatever configuration was used to configure the > > > algorithm. This means that in certain situations the full exposure range > > > might not be used, even in dark scenes. > > > > > > This a third attempt at adapting https://patchwork.libcamera.org/project/libcamera/list/?series=5590 > > > on the current master branch. But now that the agc handling is largely unified, > > > fewer changes are needed to make this work on every platform. > > > > > > So effectively only patch 7 is taken directly, but only the exposure margin > > > introduction part of it. > > > > > > This was mainly tested on rkisp1+imx219, softisp+ov2740, rppx1+imx708. > > > > > > Unfortunately, back to RFC this time, mainly due to https://gitlab.freedesktop.org/camera/libcamera/-/work_items/362 > > > It turns out to be a bit problematic. So this version adds significant changes: > > > > > > * last exposure stage constitutes a hard-limit for both exposure time and gain, > > > this matches the behaviour of the raspberry pi ipa (see AgcChannel::computeTargetExposure() ) > > > ( https://gitlab.freedesktop.org/camera/libcamera/-/work_items/362 ) > > > > I think that's fine a desirable. > > > > We had several attempts on the list to introduce artificial limits for > > exposure and gain stages from configuration file > > https://patchwork.libcamera.org/patch/28144/ > > I believe this one is not entirely related because it is about awb gains. > ah yes, sorry, I defintely mis-read it > > > https://patchwork.libcamera.org/patch/26665/ > > This is probably covered by the AgcMeanLuminance tuning, yes. > ack > > > > > if I'm not mistaken using agc stages might support the above use > > cases ? > > > > > > > > * empty stage list is no longer supported, instead two new exposure modes are introduced: > > > minimize and maximize, which implement the very simple behaviour of either minimizing > > > or maximizing the exposure time, respectively; these modes are always available, > > > even without any specific tuning, and this effectively preserves the current behaviour > > > > So if no gain stages are specified, which mode is used ? Maximize or > > Minimize ? > > Maximize - which matches the current behaviour. ok. can the user select between Maximize and Minimize ? > > > > > > > > > > * the default max frame duration is now determined based on the largest > > > exposure time among all stages (if any), otherwise it is capped to > > > the arbitrary limit of 100ms ~ 10fps > > > > Honestly, now that we support stages, and one can easily use them to > > limit the exposure time, I would lift any artificial limitation, we'll > > never find a default that satisfies everyone, so it's maybe better to > > let the algorithm adjust and suggest to use stages to limit it ? > > I would still want to have a limit just to ensure that things are somewhat > reasonable even without any tuning, just for first impressions' sake. Understandable. However, 10 fps is "good enough" to be considered a "bad behaving" camera. In other words, if my camera runs at 1fps it is clear something has to be changed, if it runs at 10 fps I might just assume that's how that camera performs. That's why I was considering letting the algorithm to push frame duration as much as it likes. It will prompt users into investigating how to limit that. Also, should we instead of introducing a limit in the algorithm add stages to all our tuning file with a min 10 fps instead for users to get an example ? > > > > > > > > > > changes in v2: > > > * report `FrameDurationLimits` in metadata > > > * add exposure margin to more sensor helpers > > > > > > v2: https://patchwork.libcamera.org/cover/28367/ > > > v1: https://patchwork.libcamera.org/cover/28111/ > > > > > > Barnabás Pőcze (18): > > > ipa: libipa: exposure_mode_helper: Remove defaulted destructor > > > ipa: libipa: exposure_mode_helper: Keep stage params together > > > ipa: libipa: exposure_mode_helper: Remove limit getters > > > ipa: libipa: exposure_mode_helper: Move `stageGain` variable > > > ipa: libipa: exposure_mode_helper: Handle no stages > > > ipa: libipa: exposure_mode_helper: Consider last stage as hard-limit > > > ipa: libipa: exposure_mode_helper: Add separate maximize/minimize > > > modes > > > libcamera: controls: Add two new exposure modes > > > ipa: libipa: agc: Keep frame duration limits ordered > > > ipa: libipa: agc: Retrieve `FrameDurationLimits` earlier > > > ipa: libipa: agc: Report `FrameDurationLimits` > > > ipa: libipa: agc: Calculate vblank and frame duration sooner > > > ipa: libipa: agc: Take parameters from active state for calculation > > > libcamera: pipeline: Set vblank on more platforms > > > ipa: libipa: agc: Rework frame duration limit calculation > > > ipa: libipa: agc: Move control info map setup > > > ipa: libipa: exposure_mode_helper: Add getter for max exp time > > > ipa: libipa: agc: Take exposure margin into account > > > > > > Jacopo Mondi (1): > > > ipa: camera_sensor_helper: Introduce exposure margin > > > > > > src/ipa/ipu3/algorithms/agc.cpp | 2 +- > > > src/ipa/ipu3/ipu3.cpp | 3 +- > > > src/ipa/libipa/agc.cpp | 311 +++++++++++-------- > > > src/ipa/libipa/agc.h | 18 +- > > > src/ipa/libipa/agc_mean_luminance.cpp | 37 ++- > > > src/ipa/libipa/camera_sensor_helper.cpp | 68 ++++ > > > src/ipa/libipa/camera_sensor_helper.h | 2 + > > > src/ipa/libipa/exposure_mode_helper.cpp | 231 +++++++++----- > > > src/ipa/libipa/exposure_mode_helper.h | 50 ++- > > > src/ipa/mali-c55/algorithms/agc.cpp | 2 +- > > > src/ipa/mali-c55/mali-c55.cpp | 3 +- > > > src/ipa/rkisp1/algorithms/agc.cpp | 2 +- > > > src/ipa/rkisp1/rkisp1.cpp | 4 +- > > > src/ipa/softisp/algorithms/agc.cpp | 2 +- > > > src/ipa/softisp/softisp.cpp | 3 +- > > > src/libcamera/control_ids_core.yaml | 6 + > > > src/libcamera/pipeline/ipu3/ipu3.cpp | 1 + > > > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 1 + > > > src/libcamera/pipeline/simple/simple.cpp | 1 + > > > 19 files changed, 500 insertions(+), 247 deletions(-) > > > > > > -- > > > 2.56.0 >
2026. 10. 20. 15:26 keltezéssel, Jacopo Mondi írta: > Hi Barnabas > > On Tue, Oct 20, 2026 at 02:59:08PM +0200, Barnabás Pőcze wrote: >> 2026. 10. 20. 14:49 keltezéssel, Jacopo Mondi írta: >>> Hi Barnabas >>> >>> On Mon, Oct 19, 2026 at 08:45:07PM +0200, Barnabás Pőcze wrote: >>>> The maximum available exposure depends on the frame duration, but this >>>> is not considered by the current agc implementation, it fixes the maximum >>>> exposure time based on whatever configuration was used to configure the >>>> algorithm. This means that in certain situations the full exposure range >>>> might not be used, even in dark scenes. >>>> >>>> This a third attempt at adapting https://patchwork.libcamera.org/project/libcamera/list/?series=5590 >>>> on the current master branch. But now that the agc handling is largely unified, >>>> fewer changes are needed to make this work on every platform. >>>> >>>> So effectively only patch 7 is taken directly, but only the exposure margin >>>> introduction part of it. >>>> >>>> This was mainly tested on rkisp1+imx219, softisp+ov2740, rppx1+imx708. >>>> >>>> Unfortunately, back to RFC this time, mainly due to https://gitlab.freedesktop.org/camera/libcamera/-/work_items/362 >>>> It turns out to be a bit problematic. So this version adds significant changes: >>>> >>>> * last exposure stage constitutes a hard-limit for both exposure time and gain, >>>> this matches the behaviour of the raspberry pi ipa (see AgcChannel::computeTargetExposure() ) >>>> ( https://gitlab.freedesktop.org/camera/libcamera/-/work_items/362 ) >>> >>> I think that's fine a desirable. >>> >>> We had several attempts on the list to introduce artificial limits for >>> exposure and gain stages from configuration file >>> https://patchwork.libcamera.org/patch/28144/ >> >> I believe this one is not entirely related because it is about awb gains. >> > > ah yes, sorry, I defintely mis-read it > >> >>> https://patchwork.libcamera.org/patch/26665/ >> >> This is probably covered by the AgcMeanLuminance tuning, yes. >> > > ack > >> >>> >>> if I'm not mistaken using agc stages might support the above use >>> cases ? >>> >>>> >>>> * empty stage list is no longer supported, instead two new exposure modes are introduced: >>>> minimize and maximize, which implement the very simple behaviour of either minimizing >>>> or maximizing the exposure time, respectively; these modes are always available, >>>> even without any specific tuning, and this effectively preserves the current behaviour >>> >>> So if no gain stages are specified, which mode is used ? Maximize or >>> Minimize ? >> >> Maximize - which matches the current behaviour. > > ok. can the user select between Maximize and Minimize ? In the tuning file? No. With the `AeExposureMode` control? Yes. > >> >> >>> >>>> >>>> * the default max frame duration is now determined based on the largest >>>> exposure time among all stages (if any), otherwise it is capped to >>>> the arbitrary limit of 100ms ~ 10fps >>> >>> Honestly, now that we support stages, and one can easily use them to >>> limit the exposure time, I would lift any artificial limitation, we'll >>> never find a default that satisfies everyone, so it's maybe better to >>> let the algorithm adjust and suggest to use stages to limit it ? >> >> I would still want to have a limit just to ensure that things are somewhat >> reasonable even without any tuning, just for first impressions' sake. > > Understandable. However, 10 fps is "good enough" to be considered a > "bad behaving" camera. In other words, if my camera runs at 1fps it is > clear something has to be changed, if it runs at 10 fps I might just > assume that's how that camera performs. That's why I was considering > letting the algorithm to push frame duration as much as it likes. It > will prompt users into investigating how to limit that. Or it will push them to go "oh well this sucks". :( I feel like one is more motivated to improve something that almost works than something that works quite badly. And we are definitely not on top of upstreaming / creating / maintaining tuning files, so that also makes the situation worse. > > Also, should we instead of introducing a limit in the algorithm add > stages to all our tuning file with a min 10 fps instead for users to > get an example ? > >> >> >>> >>>> >>>> changes in v2: >>>> * report `FrameDurationLimits` in metadata >>>> * add exposure margin to more sensor helpers >>>> >>>> v2: https://patchwork.libcamera.org/cover/28367/ >>>> v1: https://patchwork.libcamera.org/cover/28111/ >>>> >>>> Barnabás Pőcze (18): >>>> ipa: libipa: exposure_mode_helper: Remove defaulted destructor >>>> ipa: libipa: exposure_mode_helper: Keep stage params together >>>> ipa: libipa: exposure_mode_helper: Remove limit getters >>>> ipa: libipa: exposure_mode_helper: Move `stageGain` variable >>>> ipa: libipa: exposure_mode_helper: Handle no stages >>>> ipa: libipa: exposure_mode_helper: Consider last stage as hard-limit >>>> ipa: libipa: exposure_mode_helper: Add separate maximize/minimize >>>> modes >>>> libcamera: controls: Add two new exposure modes >>>> ipa: libipa: agc: Keep frame duration limits ordered >>>> ipa: libipa: agc: Retrieve `FrameDurationLimits` earlier >>>> ipa: libipa: agc: Report `FrameDurationLimits` >>>> ipa: libipa: agc: Calculate vblank and frame duration sooner >>>> ipa: libipa: agc: Take parameters from active state for calculation >>>> libcamera: pipeline: Set vblank on more platforms >>>> ipa: libipa: agc: Rework frame duration limit calculation >>>> ipa: libipa: agc: Move control info map setup >>>> ipa: libipa: exposure_mode_helper: Add getter for max exp time >>>> ipa: libipa: agc: Take exposure margin into account >>>> >>>> Jacopo Mondi (1): >>>> ipa: camera_sensor_helper: Introduce exposure margin >>>> >>>> src/ipa/ipu3/algorithms/agc.cpp | 2 +- >>>> src/ipa/ipu3/ipu3.cpp | 3 +- >>>> src/ipa/libipa/agc.cpp | 311 +++++++++++-------- >>>> src/ipa/libipa/agc.h | 18 +- >>>> src/ipa/libipa/agc_mean_luminance.cpp | 37 ++- >>>> src/ipa/libipa/camera_sensor_helper.cpp | 68 ++++ >>>> src/ipa/libipa/camera_sensor_helper.h | 2 + >>>> src/ipa/libipa/exposure_mode_helper.cpp | 231 +++++++++----- >>>> src/ipa/libipa/exposure_mode_helper.h | 50 ++- >>>> src/ipa/mali-c55/algorithms/agc.cpp | 2 +- >>>> src/ipa/mali-c55/mali-c55.cpp | 3 +- >>>> src/ipa/rkisp1/algorithms/agc.cpp | 2 +- >>>> src/ipa/rkisp1/rkisp1.cpp | 4 +- >>>> src/ipa/softisp/algorithms/agc.cpp | 2 +- >>>> src/ipa/softisp/softisp.cpp | 3 +- >>>> src/libcamera/control_ids_core.yaml | 6 + >>>> src/libcamera/pipeline/ipu3/ipu3.cpp | 1 + >>>> src/libcamera/pipeline/mali-c55/mali-c55.cpp | 1 + >>>> src/libcamera/pipeline/simple/simple.cpp | 1 + >>>> 19 files changed, 500 insertions(+), 247 deletions(-) >>>> >>>> -- >>>> 2.56.0 >>
Hi Barnabas On Tue, Oct 20, 2026 at 03:35:51PM +0200, Barnabás Pőcze wrote: > 2026. 10. 20. 15:26 keltezéssel, Jacopo Mondi írta: > > Hi Barnabas > > > > On Tue, Oct 20, 2026 at 02:59:08PM +0200, Barnabás Pőcze wrote: > > > 2026. 10. 20. 14:49 keltezéssel, Jacopo Mondi írta: > > > > Hi Barnabas > > > > > > > > On Mon, Oct 19, 2026 at 08:45:07PM +0200, Barnabás Pőcze wrote: > > > > > The maximum available exposure depends on the frame duration, but this > > > > > is not considered by the current agc implementation, it fixes the maximum > > > > > exposure time based on whatever configuration was used to configure the > > > > > algorithm. This means that in certain situations the full exposure range > > > > > might not be used, even in dark scenes. > > > > > > > > > > This a third attempt at adapting https://patchwork.libcamera.org/project/libcamera/list/?series=5590 > > > > > on the current master branch. But now that the agc handling is largely unified, > > > > > fewer changes are needed to make this work on every platform. > > > > > > > > > > So effectively only patch 7 is taken directly, but only the exposure margin > > > > > introduction part of it. > > > > > > > > > > This was mainly tested on rkisp1+imx219, softisp+ov2740, rppx1+imx708. > > > > > > > > > > Unfortunately, back to RFC this time, mainly due to https://gitlab.freedesktop.org/camera/libcamera/-/work_items/362 > > > > > It turns out to be a bit problematic. So this version adds significant changes: > > > > > > > > > > * last exposure stage constitutes a hard-limit for both exposure time and gain, > > > > > this matches the behaviour of the raspberry pi ipa (see AgcChannel::computeTargetExposure() ) > > > > > ( https://gitlab.freedesktop.org/camera/libcamera/-/work_items/362 ) > > > > > > > > I think that's fine a desirable. > > > > > > > > We had several attempts on the list to introduce artificial limits for > > > > exposure and gain stages from configuration file > > > > https://patchwork.libcamera.org/patch/28144/ > > > > > > I believe this one is not entirely related because it is about awb gains. > > > > > > > ah yes, sorry, I defintely mis-read it > > > > > > > > > https://patchwork.libcamera.org/patch/26665/ > > > > > > This is probably covered by the AgcMeanLuminance tuning, yes. > > > > > > > ack > > > > > > > > > > > > > if I'm not mistaken using agc stages might support the above use > > > > cases ? > > > > > > > > > > > > > > * empty stage list is no longer supported, instead two new exposure modes are introduced: > > > > > minimize and maximize, which implement the very simple behaviour of either minimizing > > > > > or maximizing the exposure time, respectively; these modes are always available, > > > > > even without any specific tuning, and this effectively preserves the current behaviour > > > > > > > > So if no gain stages are specified, which mode is used ? Maximize or > > > > Minimize ? > > > > > > Maximize - which matches the current behaviour. > > > > ok. can the user select between Maximize and Minimize ? > > In the tuning file? No. With the `AeExposureMode` control? Yes. > I don't know, it still feels like a best effort solution. If a camera application can go and control AeExposureMode, it means the user is working on a solution specific to a device/product and could very well define stages and use them For the webcam use case, I don't think browsers or applications going through pipewire in general can select AeExposureMode, so it won't help. For testing with camshark, yes, it's nice, but only for testing. > > > > > > > > > > > > > > > > > > > > > > > * the default max frame duration is now determined based on the largest > > > > > exposure time among all stages (if any), otherwise it is capped to > > > > > the arbitrary limit of 100ms ~ 10fps > > > > > > > > Honestly, now that we support stages, and one can easily use them to > > > > limit the exposure time, I would lift any artificial limitation, we'll > > > > never find a default that satisfies everyone, so it's maybe better to > > > > let the algorithm adjust and suggest to use stages to limit it ? > > > > > > I would still want to have a limit just to ensure that things are somewhat > > > reasonable even without any tuning, just for first impressions' sake. > > > > Understandable. However, 10 fps is "good enough" to be considered a > > "bad behaving" camera. In other words, if my camera runs at 1fps it is > > clear something has to be changed, if it runs at 10 fps I might just > > assume that's how that camera performs. That's why I was considering > > letting the algorithm to push frame duration as much as it likes. It > > will prompt users into investigating how to limit that. > > Or it will push them to go "oh well this sucks". :( I feel like one is > more motivated to improve something that almost works than something > that works quite badly. As a lazy person when it comes to adjusting broken technology, if it barely works it's fine with me. Then I'll complain it sucks. > > And we are definitely not on top of upstreaming / creating / maintaining tuning files, > so that also makes the situation worse. Wouldn't adding a stage with a max duration of 100msec effectively do the same ? uncalibrated.yaml should have stages as well > > > > > > Also, should we instead of introducing a limit in the algorithm add > > stages to all our tuning file with a min 10 fps instead for users to > > get an example ? > > > > > > > > > > > > > > > > > > > > > > changes in v2: > > > > > * report `FrameDurationLimits` in metadata > > > > > * add exposure margin to more sensor helpers > > > > > > > > > > v2: https://patchwork.libcamera.org/cover/28367/ > > > > > v1: https://patchwork.libcamera.org/cover/28111/ > > > > > > > > > > Barnabás Pőcze (18): > > > > > ipa: libipa: exposure_mode_helper: Remove defaulted destructor > > > > > ipa: libipa: exposure_mode_helper: Keep stage params together > > > > > ipa: libipa: exposure_mode_helper: Remove limit getters > > > > > ipa: libipa: exposure_mode_helper: Move `stageGain` variable > > > > > ipa: libipa: exposure_mode_helper: Handle no stages > > > > > ipa: libipa: exposure_mode_helper: Consider last stage as hard-limit > > > > > ipa: libipa: exposure_mode_helper: Add separate maximize/minimize > > > > > modes > > > > > libcamera: controls: Add two new exposure modes > > > > > ipa: libipa: agc: Keep frame duration limits ordered > > > > > ipa: libipa: agc: Retrieve `FrameDurationLimits` earlier > > > > > ipa: libipa: agc: Report `FrameDurationLimits` > > > > > ipa: libipa: agc: Calculate vblank and frame duration sooner > > > > > ipa: libipa: agc: Take parameters from active state for calculation > > > > > libcamera: pipeline: Set vblank on more platforms > > > > > ipa: libipa: agc: Rework frame duration limit calculation > > > > > ipa: libipa: agc: Move control info map setup > > > > > ipa: libipa: exposure_mode_helper: Add getter for max exp time > > > > > ipa: libipa: agc: Take exposure margin into account > > > > > > > > > > Jacopo Mondi (1): > > > > > ipa: camera_sensor_helper: Introduce exposure margin > > > > > > > > > > src/ipa/ipu3/algorithms/agc.cpp | 2 +- > > > > > src/ipa/ipu3/ipu3.cpp | 3 +- > > > > > src/ipa/libipa/agc.cpp | 311 +++++++++++-------- > > > > > src/ipa/libipa/agc.h | 18 +- > > > > > src/ipa/libipa/agc_mean_luminance.cpp | 37 ++- > > > > > src/ipa/libipa/camera_sensor_helper.cpp | 68 ++++ > > > > > src/ipa/libipa/camera_sensor_helper.h | 2 + > > > > > src/ipa/libipa/exposure_mode_helper.cpp | 231 +++++++++----- > > > > > src/ipa/libipa/exposure_mode_helper.h | 50 ++- > > > > > src/ipa/mali-c55/algorithms/agc.cpp | 2 +- > > > > > src/ipa/mali-c55/mali-c55.cpp | 3 +- > > > > > src/ipa/rkisp1/algorithms/agc.cpp | 2 +- > > > > > src/ipa/rkisp1/rkisp1.cpp | 4 +- > > > > > src/ipa/softisp/algorithms/agc.cpp | 2 +- > > > > > src/ipa/softisp/softisp.cpp | 3 +- > > > > > src/libcamera/control_ids_core.yaml | 6 + > > > > > src/libcamera/pipeline/ipu3/ipu3.cpp | 1 + > > > > > src/libcamera/pipeline/mali-c55/mali-c55.cpp | 1 + > > > > > src/libcamera/pipeline/simple/simple.cpp | 1 + > > > > > 19 files changed, 500 insertions(+), 247 deletions(-) > > > > > > > > > > -- > > > > > 2.56.0 > > > >
The maximum available exposure depends on the frame duration, but this is not considered by the current agc implementation, it fixes the maximum exposure time based on whatever configuration was used to configure the algorithm. This means that in certain situations the full exposure range might not be used, even in dark scenes. This a third attempt at adapting https://patchwork.libcamera.org/project/libcamera/list/?series=5590 on the current master branch. But now that the agc handling is largely unified, fewer changes are needed to make this work on every platform. So effectively only patch 7 is taken directly, but only the exposure margin introduction part of it. This was mainly tested on rkisp1+imx219, softisp+ov2740, rppx1+imx708. Unfortunately, back to RFC this time, mainly due to https://gitlab.freedesktop.org/camera/libcamera/-/work_items/362 It turns out to be a bit problematic. So this version adds significant changes: * last exposure stage constitutes a hard-limit for both exposure time and gain, this matches the behaviour of the raspberry pi ipa (see AgcChannel::computeTargetExposure() ) ( https://gitlab.freedesktop.org/camera/libcamera/-/work_items/362 ) * empty stage list is no longer supported, instead two new exposure modes are introduced: minimize and maximize, which implement the very simple behaviour of either minimizing or maximizing the exposure time, respectively; these modes are always available, even without any specific tuning, and this effectively preserves the current behaviour * the default max frame duration is now determined based on the largest exposure time among all stages (if any), otherwise it is capped to the arbitrary limit of 100ms ~ 10fps changes in v2: * report `FrameDurationLimits` in metadata * add exposure margin to more sensor helpers v2: https://patchwork.libcamera.org/cover/28367/ v1: https://patchwork.libcamera.org/cover/28111/ Barnabás Pőcze (18): ipa: libipa: exposure_mode_helper: Remove defaulted destructor ipa: libipa: exposure_mode_helper: Keep stage params together ipa: libipa: exposure_mode_helper: Remove limit getters ipa: libipa: exposure_mode_helper: Move `stageGain` variable ipa: libipa: exposure_mode_helper: Handle no stages ipa: libipa: exposure_mode_helper: Consider last stage as hard-limit ipa: libipa: exposure_mode_helper: Add separate maximize/minimize modes libcamera: controls: Add two new exposure modes ipa: libipa: agc: Keep frame duration limits ordered ipa: libipa: agc: Retrieve `FrameDurationLimits` earlier ipa: libipa: agc: Report `FrameDurationLimits` ipa: libipa: agc: Calculate vblank and frame duration sooner ipa: libipa: agc: Take parameters from active state for calculation libcamera: pipeline: Set vblank on more platforms ipa: libipa: agc: Rework frame duration limit calculation ipa: libipa: agc: Move control info map setup ipa: libipa: exposure_mode_helper: Add getter for max exp time ipa: libipa: agc: Take exposure margin into account Jacopo Mondi (1): ipa: camera_sensor_helper: Introduce exposure margin src/ipa/ipu3/algorithms/agc.cpp | 2 +- src/ipa/ipu3/ipu3.cpp | 3 +- src/ipa/libipa/agc.cpp | 311 +++++++++++-------- src/ipa/libipa/agc.h | 18 +- src/ipa/libipa/agc_mean_luminance.cpp | 37 ++- src/ipa/libipa/camera_sensor_helper.cpp | 68 ++++ src/ipa/libipa/camera_sensor_helper.h | 2 + src/ipa/libipa/exposure_mode_helper.cpp | 231 +++++++++----- src/ipa/libipa/exposure_mode_helper.h | 50 ++- src/ipa/mali-c55/algorithms/agc.cpp | 2 +- src/ipa/mali-c55/mali-c55.cpp | 3 +- src/ipa/rkisp1/algorithms/agc.cpp | 2 +- src/ipa/rkisp1/rkisp1.cpp | 4 +- src/ipa/softisp/algorithms/agc.cpp | 2 +- src/ipa/softisp/softisp.cpp | 3 +- src/libcamera/control_ids_core.yaml | 6 + src/libcamera/pipeline/ipu3/ipu3.cpp | 1 + src/libcamera/pipeline/mali-c55/mali-c55.cpp | 1 + src/libcamera/pipeline/simple/simple.cpp | 1 + 19 files changed, 500 insertions(+), 247 deletions(-) -- 2.56.0