pipeline: rpi: Clear immediate controls queue on stop

28202 diff mbox series

Immediate controls wait in a queue keyed by request sequence until
delayContext catches up. The queue was not cleared on stop while
request sequence numbers restart from zero, so stale entries blocked
every immediate control of the next session until it outlasted the
old one.

Fixes: b0dbd5100b1b ("pipeline: rpi: Make control lists in requests properly atomic")
Signed-off-by: Danius Kalvaitis <danius@kurokesu.com>
---
Reproduced on a Pi 5 with an IMX477, where ColourGains sent through a
request after a restart took as many frames to apply as the previous
session had run.

David, this overlaps with your ControlList queues in
https://github.com/raspberrypi/libcamera/pull/351, which probably need
clearing on stop too.

 src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 6 ++++++
 src/libcamera/pipeline/rpi/common/pipeline_base.h   | 1 +
 2 files changed, 7 insertions(+)


base-commit: aebe4861c087ce9a0cb417151d5316a1fdb2e792

From: Randy Mackay <rmackay999@gmail.com>

Hi Danius,

Thanks for the patch. I hit the same problem on a Raspberry Pi CM4 with
an IMX477 (vc4 pipeline, Raspberry Pi OS libcamera 0.7.2+rpt20260817) in
AP_CameraGimbal, ArduPilot's camera and gimbal software
(https://github.com/ArduPilot/AP_CameraGimbal). It stops the camera,
reconfigures it for a full-resolution still, and restarts it for video.
After the restart, every immediate control (Brightness, Contrast,
Saturation, AwbMode, ColourGains) was held back for as long as the
previous session had run: 20 s of video before a still delayed a
Brightness change by about 26 s.

With this patch applied to that libcamera source, the same Brightness
change applied immediately after the restart, as did an AwbMode change,
in two runs each. Without the patch the delay reproduced both times.

We've put a workaround in place until this patch goes in but I look
forward to the day we can remove that workaround.

Tested-by: Randy Mackay <rmackay999@gmail.com>

Hi Danius,


On Tue, 8 Sept 2026 at 12:01, Danius Kalvaitis <danius@kurokesu.com> wrote:
>
> Immediate controls wait in a queue keyed by request sequence until
> delayContext catches up. The queue was not cleared on stop while
> request sequence numbers restart from zero, so stale entries blocked
> every immediate control of the next session until it outlasted the
> old one.
>
> Fixes: b0dbd5100b1b ("pipeline: rpi: Make control lists in requests properly atomic")
> Signed-off-by: Danius Kalvaitis <danius@kurokesu.com>

This looks reasonable to me, but I would like @David Plowman to confirm as well.

Reviewed-by: Naushir Patuck <naush@raspberrypi.com>


> ---
> Reproduced on a Pi 5 with an IMX477, where ColourGains sent through a
> request after a restart took as many frames to apply as the previous
> session had run.
>
> David, this overlaps with your ControlList queues in
> https://github.com/raspberrypi/libcamera/pull/351, which probably need
> clearing on stop too.
>
>  src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 6 ++++++
>  src/libcamera/pipeline/rpi/common/pipeline_base.h   | 1 +
>  2 files changed, 7 insertions(+)
>
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index 71f3f26a..83a684bd 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -734,6 +734,7 @@ void PipelineHandlerBase::stopDevice(Camera *camera)
>         data->frontendDevice()->setFrameStartEnabled(false);
>
>         data->clearIncompleteRequests();
> +       data->clearImmediateControls();
>
>         /* Stop the IPA. */
>         data->ipa_->stop();
> @@ -1426,6 +1427,11 @@ void CameraData::clearIncompleteRequests()
>         }
>  }
>
> +void CameraData::clearImmediateControls()
> +{
> +       immediateControls_ = {};
> +}
> +
>  void CameraData::handleStreamBuffer(FrameBuffer *buffer, RPi::Stream *stream)
>  {
>         /*
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> index ca12a487..7b872555 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> @@ -89,6 +89,7 @@ public:
>         void frameStarted(uint32_t sequence);
>
>         void clearIncompleteRequests();
> +       void clearImmediateControls();
>         void handleStreamBuffer(FrameBuffer *buffer, Stream *stream);
>         void handleState();
>
>
> base-commit: aebe4861c087ce9a0cb417151d5316a1fdb2e792
> --
> 2.47.3
>

Hi

Thanks for sending this patch.

On Wed, 7 Oct 2026 at 08:46, Naushir Patuck <naush@raspberrypi.com> wrote:
>
> Hi Danius,
>
>
> On Tue, 8 Sept 2026 at 12:01, Danius Kalvaitis <danius@kurokesu.com> wrote:
> >
> > Immediate controls wait in a queue keyed by request sequence until
> > delayContext catches up. The queue was not cleared on stop while
> > request sequence numbers restart from zero, so stale entries blocked
> > every immediate control of the next session until it outlasted the
> > old one.
> >
> > Fixes: b0dbd5100b1b ("pipeline: rpi: Make control lists in requests properly atomic")
> > Signed-off-by: Danius Kalvaitis <danius@kurokesu.com>
>
> This looks reasonable to me, but I would like @David Plowman to confirm as well.
>
> Reviewed-by: Naushir Patuck <naush@raspberrypi.com>

Yes I think this seems like the right thing to do. There can certainly
be issues around knowing whether a control was "seen" before you stop
the camera, but leaving a nasty surprise lurking in the immediate
controls list for when the camera restarts definitely feels wrong!

Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

Thanks
David

>
>
> > ---
> > Reproduced on a Pi 5 with an IMX477, where ColourGains sent through a
> > request after a restart took as many frames to apply as the previous
> > session had run.
> >
> > David, this overlaps with your ControlList queues in
> > https://github.com/raspberrypi/libcamera/pull/351, which probably need
> > clearing on stop too.
> >
> >  src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 6 ++++++
> >  src/libcamera/pipeline/rpi/common/pipeline_base.h   | 1 +
> >  2 files changed, 7 insertions(+)
> >
> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > index 71f3f26a..83a684bd 100644
> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> > @@ -734,6 +734,7 @@ void PipelineHandlerBase::stopDevice(Camera *camera)
> >         data->frontendDevice()->setFrameStartEnabled(false);
> >
> >         data->clearIncompleteRequests();
> > +       data->clearImmediateControls();
> >
> >         /* Stop the IPA. */
> >         data->ipa_->stop();
> > @@ -1426,6 +1427,11 @@ void CameraData::clearIncompleteRequests()
> >         }
> >  }
> >
> > +void CameraData::clearImmediateControls()
> > +{
> > +       immediateControls_ = {};
> > +}
> > +
> >  void CameraData::handleStreamBuffer(FrameBuffer *buffer, RPi::Stream *stream)
> >  {
> >         /*
> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> > index ca12a487..7b872555 100644
> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h
> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h
> > @@ -89,6 +89,7 @@ public:
> >         void frameStarted(uint32_t sequence);
> >
> >         void clearIncompleteRequests();
> > +       void clearImmediateControls();
> >         void handleStreamBuffer(FrameBuffer *buffer, Stream *stream);
> >         void handleState();
> >
> >
> > base-commit: aebe4861c087ce9a0cb417151d5316a1fdb2e792
> > --
> > 2.47.3
> >