[RFC,v2,0/1] Add queueControls mechanism
mbox series

Message ID 20260312160009.18654-1-david.plowman@raspberrypi.com
Headers show
Series
  • Add queueControls mechanism
Related show

Message

David Plowman March 12, 2026, 3:10 p.m. UTC
Hi everyone

Here's a version of Barnabas's previous patch (hence "v2") that added
a separate applyControls function, to bypass the request queue. If you
recall, I was a fan of this one.

I've re-worked it only slightly, replacing the single pending
ControlList with a queue, which is the functionality that was
important to us. To make that explicit, I've renamed "applyControls"
to "queueControls" everywhere.

I've also tried, in passing, to answer some of the hanging questions:

* Like requests, I've not allowed controls to be queued before the
  camera is running. Don't feel strongly either way, really.

* I've allowed queueing empty ControlLists. Doing this allows you to
  get "another frame like the last one".

* I clear out all pending controls when the camera stops. If an
  application needs to be sure they have been applied, it should wait
  (which kind of leads us on to PFC, but not a subject for this
  patch!). Or if it doesn't want to wait, it should re-apply any
  in-question values when it re-starts the camera.

* I'm using the fallback only when the PH explicitly doesn't support
  queueControlsDevice. Really, if it's supported it's got to work
  every time, even if that only means putting it in a queue in the
  PH. Having some ControlLists go into the PH here, and then to have
  it fail and put others into the pending queue, just feels entirely
  confusing!

* In the fallback case I'm copying the ControlList so that we can
  recover if the request fails to queue. Firstly, I don't think this
  is too horrible, and anyway, it's a fallback. If anyone cares about
  this they should implement queueControlsDevice.

TBH, I'm a bit unconvinced by the whole fallback thing. We (RPi)
always queue up lots of requests as soon as we can, and to my
understanding they all go through to our PH pretty much straight away,
which kind of nullifies the whole point. But maybe other PHs behave
differently? Always happy to be corrected on any misconceptions!

Thanks
David

David Plowman (1):
  libcamera: Add independent queue for ControlLists

 include/libcamera/camera.h                    |  1 +
 include/libcamera/internal/camera.h           |  1 +
 include/libcamera/internal/pipeline_handler.h |  7 ++
 src/libcamera/camera.cpp                      | 61 +++++++++++++++
 src/libcamera/pipeline_handler.cpp            | 76 +++++++++++++++++++
 5 files changed, 146 insertions(+)

Comments

Stefan Klug March 20, 2026, 4:42 p.m. UTC | #1
Hi David,

Thank you for the patch.

Quoting David Plowman (2026-03-12 16:10:37)
> Hi everyone
> 
> Here's a version of Barnabas's previous patch (hence "v2") that added
> a separate applyControls function, to bypass the request queue. If you
> recall, I was a fan of this one.
> 
> I've re-worked it only slightly, replacing the single pending
> ControlList with a queue, which is the functionality that was
> important to us. To make that explicit, I've renamed "applyControls"
> to "queueControls" everywhere.

I'm a bit hesitant here. The thing that nags me is that this introduces
a second queuing mechanism. I see that it would help the RPi use case
(we should get a name for that) and I feel bad that we're still not
there, yet.

applyControls() was meant as a mechanism to apply controls as fast as
possible (basically modifying the current state of the camera) without
going through any queue. So multiple calls to this function would all be
collated into the next possible point where we can apply controls.
Replacing that with a queing mechanism contradicts that target.

I still believe that the use case you are targeting can well be
fulfilled with the request based model but requires us to split requests
from buffers (which is on several todo lists since far too long, but I'm
sure it will happen).

So introducing another queuing mechanism would likely bring us in a
situation where the behavior is even harder to describe.

> 
> I've also tried, in passing, to answer some of the hanging questions:
> 
> * Like requests, I've not allowed controls to be queued before the
>   camera is running. Don't feel strongly either way, really.
> 
> * I've allowed queueing empty ControlLists. Doing this allows you to
>   get "another frame like the last one".

As said above, that defeats the "set the exposure value on every change
event of a slider UI control and the last one wins" use case.

Best regards,
Stefan

> 
> * I clear out all pending controls when the camera stops. If an
>   application needs to be sure they have been applied, it should wait
>   (which kind of leads us on to PFC, but not a subject for this
>   patch!). Or if it doesn't want to wait, it should re-apply any
>   in-question values when it re-starts the camera.
> 
> * I'm using the fallback only when the PH explicitly doesn't support
>   queueControlsDevice. Really, if it's supported it's got to work
>   every time, even if that only means putting it in a queue in the
>   PH. Having some ControlLists go into the PH here, and then to have
>   it fail and put others into the pending queue, just feels entirely
>   confusing!
> 
> * In the fallback case I'm copying the ControlList so that we can
>   recover if the request fails to queue. Firstly, I don't think this
>   is too horrible, and anyway, it's a fallback. If anyone cares about
>   this they should implement queueControlsDevice.
> 
> TBH, I'm a bit unconvinced by the whole fallback thing. We (RPi)
> always queue up lots of requests as soon as we can, and to my
> understanding they all go through to our PH pretty much straight away,
> which kind of nullifies the whole point. But maybe other PHs behave
> differently? Always happy to be corrected on any misconceptions!
> 
> Thanks
> David
> 
> David Plowman (1):
>   libcamera: Add independent queue for ControlLists
> 
>  include/libcamera/camera.h                    |  1 +
>  include/libcamera/internal/camera.h           |  1 +
>  include/libcamera/internal/pipeline_handler.h |  7 ++
>  src/libcamera/camera.cpp                      | 61 +++++++++++++++
>  src/libcamera/pipeline_handler.cpp            | 76 +++++++++++++++++++
>  5 files changed, 146 insertions(+)
> 
> -- 
> 2.47.3
>
David Plowman March 23, 2026, 1:59 p.m. UTC | #2
Hi Stefan

Thanks for the comments. I'm always very happy when people want to
discuss this!!

On Fri, 20 Mar 2026 at 16:42, Stefan Klug <stefan.klug@ideasonboard.com> wrote:
>
> Hi David,
>
> Thank you for the patch.
>
> Quoting David Plowman (2026-03-12 16:10:37)
> > Hi everyone
> >
> > Here's a version of Barnabas's previous patch (hence "v2") that added
> > a separate applyControls function, to bypass the request queue. If you
> > recall, I was a fan of this one.
> >
> > I've re-worked it only slightly, replacing the single pending
> > ControlList with a queue, which is the functionality that was
> > important to us. To make that explicit, I've renamed "applyControls"
> > to "queueControls" everywhere.
>
> I'm a bit hesitant here. The thing that nags me is that this introduces
> a second queuing mechanism. I see that it would help the RPi use case
> (we should get a name for that) and I feel bad that we're still not
> there, yet.
>
> applyControls() was meant as a mechanism to apply controls as fast as
> possible (basically modifying the current state of the camera) without
> going through any queue. So multiple calls to this function would all be
> collated into the next possible point where we can apply controls.
> Replacing that with a queing mechanism contradicts that target.

Yes, I can appreciate that there may be a use-case where you want to
apply controls immediately and aren't interested in getting sequences
of controlled frames.

Rather than just trying to implement one or the other, I'd be fine
with making both alternatives available - so long as we all get to
bypass the request queue!

>
> I still believe that the use case you are targeting can well be
> fulfilled with the request based model but requires us to split requests
> from buffers (which is on several todo lists since far too long, but I'm
> sure it will happen).
>
> So introducing another queuing mechanism would likely bring us in a
> situation where the behavior is even harder to describe.

The whole business of splitting buffers (or controls) from requests is
of course important here. I always struggle with the terminology.

If you take buffers out of requests into their own queue, a request
queue is really just a control list queue. Or if you take the control
lists out of the requests, then the request queue is basically a
buffer queue (which is what I had). So it's not terribly clear to me
what a "request" ends up being, but I don't think I mind too much what
things are called so long as we have the functionality we want.

But whatever we do, I think we end up with more than one queue, so
defining how they interact is unavoidable. The Kamaros view was at
least interesting in that respect - it had a "command" (mostly like a
control list) queue (actually more than one!), and then separate
buffer queues. Not saying that's the thing to do, but it's one data
point.

I think I nominated this as a topic for Nice, so really hopeful that
folks are up for a discussion and that we can plan some concrete first
steps in this area.

Thanks!
David

>
> >
> > I've also tried, in passing, to answer some of the hanging questions:
> >
> > * Like requests, I've not allowed controls to be queued before the
> >   camera is running. Don't feel strongly either way, really.
> >
> > * I've allowed queueing empty ControlLists. Doing this allows you to
> >   get "another frame like the last one".
>
> As said above, that defeats the "set the exposure value on every change
> event of a slider UI control and the last one wins" use case.
>
> Best regards,
> Stefan
>
> >
> > * I clear out all pending controls when the camera stops. If an
> >   application needs to be sure they have been applied, it should wait
> >   (which kind of leads us on to PFC, but not a subject for this
> >   patch!). Or if it doesn't want to wait, it should re-apply any
> >   in-question values when it re-starts the camera.
> >
> > * I'm using the fallback only when the PH explicitly doesn't support
> >   queueControlsDevice. Really, if it's supported it's got to work
> >   every time, even if that only means putting it in a queue in the
> >   PH. Having some ControlLists go into the PH here, and then to have
> >   it fail and put others into the pending queue, just feels entirely
> >   confusing!
> >
> > * In the fallback case I'm copying the ControlList so that we can
> >   recover if the request fails to queue. Firstly, I don't think this
> >   is too horrible, and anyway, it's a fallback. If anyone cares about
> >   this they should implement queueControlsDevice.
> >
> > TBH, I'm a bit unconvinced by the whole fallback thing. We (RPi)
> > always queue up lots of requests as soon as we can, and to my
> > understanding they all go through to our PH pretty much straight away,
> > which kind of nullifies the whole point. But maybe other PHs behave
> > differently? Always happy to be corrected on any misconceptions!
> >
> > Thanks
> > David
> >
> > David Plowman (1):
> >   libcamera: Add independent queue for ControlLists
> >
> >  include/libcamera/camera.h                    |  1 +
> >  include/libcamera/internal/camera.h           |  1 +
> >  include/libcamera/internal/pipeline_handler.h |  7 ++
> >  src/libcamera/camera.cpp                      | 61 +++++++++++++++
> >  src/libcamera/pipeline_handler.cpp            | 76 +++++++++++++++++++
> >  5 files changed, 146 insertions(+)
> >
> > --
> > 2.47.3
> >