[libcamera-devel] Initial settings for camera exposure and analogue gain

[Naushir Patuck]

Hi Laurent,

On Tue, 30 Jun 2020 at 00:39, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Naush,
>
> On Mon, Jun 29, 2020 at 11:54:12AM +0100, Naushir Patuck wrote:
> > On Sun, 28 Jun 2020 at 13:32, Laurent Pinchart wrote:
> > > On Mon, Jun 22, 2020 at 11:18:13AM +0100, Naushir Patuck wrote:
> > >> On Mon, 22 Jun 2020 at 05:27, Laurent Pinchart wrote:
> > >>> On Thu, Jun 11, 2020 at 10:08:59AM +0100, David Plowman wrote:
> > >>>> On Thu, 11 Jun 2020 at 08:59, Jacopo Mondi <jacopo at jmondi.org> wrote:
> > >>>>> On Thu, Jun 11, 2020 at 08:41:27AM +0100, Naushir Patuck wrote:
> > >>>>>> On Wed, 10 Jun 2020 at 17:06, Jacopo Mondi <jacopo at jmondi.org> wrote:
> > >>>>>>> On Wed, Jun 10, 2020 at 02:22:59PM +0100, David Plowman wrote:
> > >>>>>>>> On Wed, 10 Jun 2020 at 13:23, Jacopo Mondi <jacopo at jmondi.org> wrote:
> > >>>>>>>>> On Tue, Jun 09, 2020 at 03:00:49PM +0100, David Plowman wrote:
> > >>>>>>>>>> Hi
> > >>>>>>>>>>
> > >>>>>>>>>> I'd like to discuss the question of being able to set a sensor's initial
> > >>>>>>>>>> exposure time and analogue gain before the sensor is started by calling the
> > >>>>>>>>>> Camera::start method. Currently I believe this is not possible; they can
> > >>>>>>>>>> only be adjusted in the ControlList passed with a Request.
> > >>>>>>>>>
> > >>>>>>>>> For application-supplied controls I think you're right. IPAs can
> > >>>>>>>>> pre-configure sensors I guess. If I'm not mistaken your IPA protocol
> > >>>>>>>>> has a RPI_IPA_ACTION_SET_SENSOR_CONFIG event, which is be used to
> > >>>>>>>>> pre-configure sensor with known delay values, but that works at
> > >>>>>>>>> start-up time only...
> > >>>>>>>>>
> > >>>>>>>>>> This would be a helpful feature, for example when switching from preview to
> > >>>>>>>>>> capture you might want to use different exposure/gain combinations. Or for
> > >>>>>>>>>> capturing multiple exposures - you could straight away get the frame you
> > >>>>>>>>>> want without waiting a couple of frames for the camera to start streaming
> > >>>>>>>>>> with the wrong values and then change.
> > >>>
> > >>> I fully agree it would be a useful feature. It also addresses the issue
> > >>> of setting expensive parameters that require, internally, a stop-restart
> > >>> sequence. With an API to set those parameters prior to starting the
> > >>> stream there would be no restart sequence cost (setting them in requests
> > >>> would still be expensive, but that's not something we can address).
> > >>
> > >> I've been working on allowing a ControlList to be passed into
> > >> pipeline_handler::start(), but it is proving to be somewhat useless.
> > >> Take an example of a user applicaion wanting to set a fixed exposure
> > >> time on the sensor before starting. The control list with the
> > >> requested exposure time wants to then be passed into the IPA from
> > >> pipeline_handler::start(), where we convert from exposure times to
> > >> exposure lines (for example).  The IPA then needs to pass the exposure
> > >> lines back to the pipeline_handler to program into the sensor driver.
> > >> However, because of the threading model, pipeline_handler::start()
> > >> will then go on and start streaming the device drivers before it has a
> > >> chance to send the relevant v4l2 ctrls to the device drivers.
> > >>
> > >> This could be fixed in two ways:
> > >>
> > >> 1) if ipa::process_event() could be called as a blocking method, and
> > >> allow the IPA to return values back to the caller.  This will ensure
> > >> the v4l2 ctrls needed before the device stream on is available to the
> > >> pipeline handler.
> > >>
> > >> 2) Move the ControlList into pipeline_handler::configure().  This is
> > >> more inefficient, but it does get around the threading problem.
> > >> However, we still need a way of allowing the ipa::configure() method
> > >> to return arbitrary parameters back to the caller.  This is something
> > >> we talked about before - we could use this for things like the
> > >> pipeline delays for staggered_write as well.
> > >
> > > I think we need this anyway, for staggered write delays as you
> > > mentioned, and I'm sure there will be other data to pass through
> > > configure() in the future in both directions. I would rather start with
> > > this, and possibly address additional performance issues on top, by
> > > passing parameters to start() (are with any other solution we will agree
> > > on). I'll have a look at this.
> >
> > Thank you, that would clean up a few things in the pipeline handler.
>
> I see yo'uve started reviewing the series, thanks for that. I'll merge
> it once it gets fully reviewed.
>
> > >>>>>>>>>> As regards an API, we were thinking in terms of adding a "ControlList const
> > >>>>>>>>>> &controls" parameter to the Camera::start method, which would forward the
> > >>>>>>>>>> controls to the pipeline handler prior to the camera actually being
> > >>>>>>>>>> started. (We're happy to work on the feature once there's sufficient
> > >>>>>>>>>> consensus.)
> > >>>>>>>>>
> > >>>>>>>>> We started planning support for the same feature, with the idea to
> > >>>>>>>>> support Android's session parameters
> > >>>>>>>>> https://source.android.com/devices/camera/session-parameters
> > >>>>>>>>>
> > >>>>>>>>> When we (briefly) discussed this, we were considering providing a
> > >>>>>>>>> ControlList at Camera:;configure() time, probably adding it to the
> > >>>>>>>>> CameraConfiguration parameter. Thinking out loud, this would allow,
> > >>>>>>>>> in example, to return a set of pre-configured control sets from
> > >>>>>>>>> Camera::generateConfiguration() depending on the requested stream role
> > >>>>>>>>> and yet-to-be-designed capture profile.
> > >>>>>>>>>
> > >>>>>>>>> Why would you prefer to do at start() time ? That's certainly possible
> > >>>>>>>>> if there's good reasons to do so...
> > >>>>>>>>
> > >>>>>>>> As you say, having a ControlList in the configure() method would allow
> > >>>>>>>> control of these parameters whenever the camera mode has to be
> > >>>>>>>> changed, but not when it is started. I think multi-exposure/image-fusion is
> > >>>>>>>> a use-case where this would be helpful.
> > >>>>>>>>
> > >>>>>>>> For example, suppose we've been running preview and have decided
> > >>>>>>>> that we're going to use 2 exposures to capture an HDR image. The
> > >>>>>>>> sequence of events in an application might then be:
> > >>>>>>>>
> > >>>>>>>> 1. Stop the preview and teardown the preview configuration
> > >>>>>>>> 2. Configure for still capture mode.
> > >>>>>>>> 3. Write the short exposure and start the camera.
> > >>>>>>>> 4. Grab the first frame that comes back and immediately stop the camera.
> > >>>>>>>> 5. Write the long exposure and start the camera.
> > >>>>>>>> 6. Grab the first frame again, and immediately stop the camera.
> > >>>>>>>> 7. Now images can be fused and HDR processing performed...
> > >>>>>>>>
> > >>>>>>>> This would require a ControlList in the start() method (or equivalent).
> > >>>>>>>> I appreciate there are other ways to do this, but it seems a particularly
> > >>>>>>>> straightforward template that applications might wish to adopt.
> > >>>>>>>
> > >>>>>>> Surely not my strongest expertise, but if you asked me I would have
> > >>>>>>>
> > >>>>>>> 1. Stop the preview and teardown the preview configuration
> > >>>>>>> 2. Configure the camera for still capture with an initial short exposure time
> > >>>>>>> 3. Start the camera
> > >>>>>>> 4. Queue a request with the new long exposure value
> > >>>>>>> 5. Grab the first frame that comes back which will have the initially
> > >>>>>>> programmed short exposure time
> > >>>>>>> 6. Continue capturing frames until the desired long exposure time is
> > >>>>>>> not locked
> > >>>>>>> 7. Use the two images for HDR/fusion whatever.
> > >>>>>>>
> > >>>>>>> I know I'm probably missing something, so consider this exercise
> > >>>>>>> useful for my own education :)
> > >>>>>>
> > >>>>>> The above steps could indeed be used for HDR captures.  It's a toss-up
> > >>>>>> as to which is more efficient - starting/stopping the camera over
> > >>>>>> multiple exposures or letting it run on but having to wait (generally
> > >>>>>> 2 frames) before the sensor will consume the next exposure values.
> > >>>
> > >>> I think I agree with Jacopo, for this specific example, I believe
> > >>> waiting would be more efficient, as stop and especially start operations
> > >>> are very often expensive (either because if the hardware requirements,
> > >>> or at least because of suboptimal implementations in the kernel).
> > >>>
> > >>> I'd even go one step further, if you start the camera and queue two
> > >>> requests with two different exposure values, you should get the frames
> > >>> you need with a delay, but in consecutive requests. We've thought for
> > >>> quite some time about the ability to queue requests before starting the
> > >>> camera, and that could allow pipeline handlers to configure parameters
> > >>> at start time by looking at the first queued request (and possibly even
> > >>> subsequent ones).
> > >>>
> > >>> In general, do you think it would be useful to be able to prequeue
> > >>> requests as a way to provide initial parameters ? We may not even need
> > >>> to support this explicitly, pipeline handlers could possibly delay the
> > >>> real start internally until the first request is queued. I think we need
> > >>> to take all these ideas into account and decide what would be the best
> > >>> API.
> > >>
> > >> Allowing us to defer stream-on until a Request has been passed in was
> > >> actually a workaround I considered for the problems described above.
> > >> However, I thought it would not be acceptable :)
> > >>
> > >> If we agreed that this is acceptable, I think it will be a good way of
> > >> passing in parameters before the sensor starts streaming (without
> > >> extending the API).  Pre-queueing more than one Request does not
> > >> provide any further advantages, but does no harm.
> > >>
> > >> Let me know if you are ok with this, and I can work on an
> > >> implementation for review.
> > >
> > > Before starting working on an implementation, do you think we should
> > > delay the real start internally until the first request is queued, or
> > > should we allow Camera::queueRequest() to be called before
> > > Camera::start() ? The latter would have the advantage of being
> > > controllable by the application, as well as possibly centralizing part
> > > of the logic in the libcamera core (depending on the implementation),
> > > making this feature available in a uniform way across all pipeline
> > > handlers.
> >
> > Agreed, allowing Camera::queueRequest() to be called before
> > Camera::start() would allow the application to set the controls needed
> > before steraming on the sensor.  Does libcamera already allow this?
> > If so, presumably it's only a change in qcam that would be needed.
>
> No, it's not allowed it. We have to think about the implications (one of
> them being what should happen if the application changes its mind and
> wants to cancel the requests being queued before calling start()). We
> may need a flush() call, or allowing stop() to be called before start(),
> or something along those lines. flush() could also have additional use
> cases (such as flushing the queue of requests as quickly as possible in
> order to change parameters faster), but may be difficult to implement in
> pipeline handlers. Any opinion on the high-level design ?
>

This is a tricky one.  A flush mechanism could be problematic in that
buffers belonging to Requests could be sent to the driver queue.  In
this case, there is no way a flush() could be used to pull those
buffers out of the v4l2 queue is there?  We probably could use the
same mechanism that exists for stop() to handle this I suppose - or
even have the application call stop().

How do you feel about having Requests that do not pass in buffers,
only controls?  This might allow us to achieve what we need here?

Regards,
Naush


> --
> Regards,
>
> Laurent Pinchart