| Message ID | 20251017-exposure-limits-v1-0-6288cd86e719@ideasonboard.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, thank you for the series - I quickly tested it on the PinePhone Pro and can confirm that it fixes way too low frame rates with the back-camera that are reliably reproducible on master. On 10/17/25 11:00, Jacopo Mondi wrote: > By definition, the exposure time that can be programmed on a sensor is > limited by the frame duration and whenever the frame duration is > updated, the exposure time limits should be updated as well. > > This is not the case in the RkISP1 IPA, where the exposure limits > are computed at configure() time and never updated. > > This has two implications: > 1) The AGC algorithms will always operate with an exposure time bound to > the frame duration programmed at startup time > 2) The Camera::controls() limits are not updated to reflect the new > constraints > > This series addresses the above issues with 2 patches. > > The first one for the IPA simply updates the exposure limts whenever the > VBLANK changes in response to a FrameDurationLimits change. > > The second one for the IPA/pipeline introduces a new operation in the > IPA interface which the pipeline handler calls to have the limits of the > controls registered by the IPA updated. > > The timing of these two operations is critical. > > The IPA limits should be updated whenever a new VBLANK is computed. The > actual blanking value on the sensor will be changed with a few > frames of latency by DelayedControls, but for the IPA this is not > relevant as all the computations for the 'next' frames will be delayed > by the same latency and should use the new limits. > > The Camera::controls() limits should instead be updated when the Request > with the updated FrameDurationLimits completes. For this reason the new > 'updateControlsLimits()' IPA operation is called before completing a > Request on the pipeline handler side. > > Finally, the last patch, not meant for inclusion, validates that the > Camera::controls() limits are updated correctly. The IPA limits update > has been visually validated with camshark by tuning the > FrameDurationLimits control and verifying the AGC algorithm limits are > updated accordingly. > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > --- > Jacopo Mondi (3): > ipa: rkisp1: Update exposure limits on vblank change > libcamera: rkisp1: Update camera::controls() on limit changes > DNI: cam: Test Camera::controls() update with capture script > > change-frame-duration.yaml | 4 +++ > include/libcamera/ipa/rkisp1.mojom | 2 ++ > src/apps/cam/camera_session.cpp | 21 ++++++++++++ > src/ipa/rkisp1/algorithms/agc.cpp | 3 ++ > src/ipa/rkisp1/ipa_context.h | 5 ++- > src/ipa/rkisp1/rkisp1.cpp | 57 ++++++++++++++++++++++++++++++-- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++ > 7 files changed, 91 insertions(+), 4 deletions(-) > --- > base-commit: b9fa6e0e61d3ea605fe4b1201ede5745cd5800e5 > change-id: 20251017-exposure-limits-5c48252fe548 > > Best regards,
Hi Robert, On Tue, Oct 21, 2025 at 02:12:54PM +0200, Robert Mader wrote: > Hi Jacopo, thank you for the series - I quickly tested it on the PinePhone > Pro and can confirm that it fixes way too low frame rates with the > back-camera that are reliably reproducible on master. Ah that's great. Thanks for testing. Does it mean I can add your tag to at least 1/3 ? Thanks j > > On 10/17/25 11:00, Jacopo Mondi wrote: > > By definition, the exposure time that can be programmed on a sensor is > > limited by the frame duration and whenever the frame duration is > > updated, the exposure time limits should be updated as well. > > > > This is not the case in the RkISP1 IPA, where the exposure limits > > are computed at configure() time and never updated. > > > > This has two implications: > > 1) The AGC algorithms will always operate with an exposure time bound to > > the frame duration programmed at startup time > > 2) The Camera::controls() limits are not updated to reflect the new > > constraints > > > > This series addresses the above issues with 2 patches. > > > > The first one for the IPA simply updates the exposure limts whenever the > > VBLANK changes in response to a FrameDurationLimits change. > > > > The second one for the IPA/pipeline introduces a new operation in the > > IPA interface which the pipeline handler calls to have the limits of the > > controls registered by the IPA updated. > > > > The timing of these two operations is critical. > > > > The IPA limits should be updated whenever a new VBLANK is computed. The > > actual blanking value on the sensor will be changed with a few > > frames of latency by DelayedControls, but for the IPA this is not > > relevant as all the computations for the 'next' frames will be delayed > > by the same latency and should use the new limits. > > > > The Camera::controls() limits should instead be updated when the Request > > with the updated FrameDurationLimits completes. For this reason the new > > 'updateControlsLimits()' IPA operation is called before completing a > > Request on the pipeline handler side. > > > > Finally, the last patch, not meant for inclusion, validates that the > > Camera::controls() limits are updated correctly. The IPA limits update > > has been visually validated with camshark by tuning the > > FrameDurationLimits control and verifying the AGC algorithm limits are > > updated accordingly. > > > > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > --- > > Jacopo Mondi (3): > > ipa: rkisp1: Update exposure limits on vblank change > > libcamera: rkisp1: Update camera::controls() on limit changes > > DNI: cam: Test Camera::controls() update with capture script > > > > change-frame-duration.yaml | 4 +++ > > include/libcamera/ipa/rkisp1.mojom | 2 ++ > > src/apps/cam/camera_session.cpp | 21 ++++++++++++ > > src/ipa/rkisp1/algorithms/agc.cpp | 3 ++ > > src/ipa/rkisp1/ipa_context.h | 5 ++- > > src/ipa/rkisp1/rkisp1.cpp | 57 ++++++++++++++++++++++++++++++-- > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++ > > 7 files changed, 91 insertions(+), 4 deletions(-) > > --- > > base-commit: b9fa6e0e61d3ea605fe4b1201ede5745cd5800e5 > > change-id: 20251017-exposure-limits-5c48252fe548 > > > > Best regards, > > -- > Robert Mader > Consultant Software Developer > > Collabora Ltd. > Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK > Registered in England & Wales, no. 5513718 >
By definition, the exposure time that can be programmed on a sensor is limited by the frame duration and whenever the frame duration is updated, the exposure time limits should be updated as well. This is not the case in the RkISP1 IPA, where the exposure limits are computed at configure() time and never updated. This has two implications: 1) The AGC algorithms will always operate with an exposure time bound to the frame duration programmed at startup time 2) The Camera::controls() limits are not updated to reflect the new constraints This series addresses the above issues with 2 patches. The first one for the IPA simply updates the exposure limts whenever the VBLANK changes in response to a FrameDurationLimits change. The second one for the IPA/pipeline introduces a new operation in the IPA interface which the pipeline handler calls to have the limits of the controls registered by the IPA updated. The timing of these two operations is critical. The IPA limits should be updated whenever a new VBLANK is computed. The actual blanking value on the sensor will be changed with a few frames of latency by DelayedControls, but for the IPA this is not relevant as all the computations for the 'next' frames will be delayed by the same latency and should use the new limits. The Camera::controls() limits should instead be updated when the Request with the updated FrameDurationLimits completes. For this reason the new 'updateControlsLimits()' IPA operation is called before completing a Request on the pipeline handler side. Finally, the last patch, not meant for inclusion, validates that the Camera::controls() limits are updated correctly. The IPA limits update has been visually validated with camshark by tuning the FrameDurationLimits control and verifying the AGC algorithm limits are updated accordingly. Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> --- Jacopo Mondi (3): ipa: rkisp1: Update exposure limits on vblank change libcamera: rkisp1: Update camera::controls() on limit changes DNI: cam: Test Camera::controls() update with capture script change-frame-duration.yaml | 4 +++ include/libcamera/ipa/rkisp1.mojom | 2 ++ src/apps/cam/camera_session.cpp | 21 ++++++++++++ src/ipa/rkisp1/algorithms/agc.cpp | 3 ++ src/ipa/rkisp1/ipa_context.h | 5 ++- src/ipa/rkisp1/rkisp1.cpp | 57 ++++++++++++++++++++++++++++++-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++ 7 files changed, 91 insertions(+), 4 deletions(-) --- base-commit: b9fa6e0e61d3ea605fe4b1201ede5745cd5800e5 change-id: 20251017-exposure-limits-5c48252fe548 Best regards,