Refactor Suggestion

[meawoppl]

This weekend I undertook an experimental refactor of the picamera2 API, and I wanted to field a suggestion. Right now there are a group of methods, specifically:

• switch_mode()
• capture_file()
• switch_mode_and_capture_file()
• capture_request()
• switch_mode_capture_request_and_stop()
• capture_buffer()
• switch_mode_and_capture_buffer()
• capture_metadata()
• capture_array()
• switch_mode_and_capture_array()
• capture_image()
• switch_mode_and_capture_image()

That have a set of shared behaviors that can be pretty easily factored out:

• They all accept a wait flag. If wait=True, it returns the requested resources, if wait=False it returns a None
• They all accept a signal_function that gets called on completion of the request (as far as I can tell, this is only used internally).

It is very simple to refactor all of these methods to the following structure:

• All methods return an instance of concurrent.futures.Future
• Callers that desire the wait=True behavior call future.result() on the returned future, which blocks until completion and returns the result in question.
• Callers that want the signal_function associated features, would call future.add_done_callback(func)

There are a few other benefits to this as well:

• The internal function queue and result collection machinery gets much simpler (about 200 lines less code in my assessment)
• Calls with wait=True no longer have a way to hang. Presently, this can happen if something in process_requests() errors out.
• Future objects give some elegant way to bubble up exceptions/cancellations.
• It gives the caller more ability to chain complex methods or do work while a frame-grab/configuration-change future matures
• The returned Future objects can be polled, grouped, or canceled on a mode change/shutdown for example

Another option is that all of these calls could return the current structures, unless wait=False in which case it returns the future instead. Open to suggestions, the refactor is honestly pretty easy

[meawoppl]

Here is the rogugh (unfinished) version if you are interested in taking a peek:
https://github.com/meawoppl/picamera2/pull/1/files

[davidplowman]

Hi, I think this sounds interesting. A few questions, and perhaps things I haven't understood...

1. I'm a little uncomfortable with the "basic" functions returning a future and requiring the user to understand that and call get_result (or whatever). I'm thinking particularly of some of our quite large number of relatively "unsophisticated" users who we don't hear from much on these forums.
2. Could we have a scheme where wait=True (the default), makes it wait and return the actual result, wait=False makes it return the final future object?
3. If a signal_function is passed, could we hide the setting of the done_callback? There's a general feeling among some users that they'd like less "boilerplate" stuff, so I'd like not to make that worse! The examples to look at here are the Qt applications (do a git grep wait=False in the examples folder.
4. Would we be able to lift the restriction of only one operation at once? It seems like futures would let us do that, and it might be quite cool I think. (Obviously dispatch_functions would need to grab the lock, it's perhaps a bit naughty that it doesn't.)

Thanks!

[meawoppl]

There is a lot here that I talk about a bit in this PR. Generally speaking, if we keep moving stuff in that direction, we can make the user facing functions thread/concurrent safe as they will either block or return Future() instances that unblock/complete in the order they are called.

I think if the above gets in, there is only 1-2 more small PR's to get it there :)

[meawoppl]

One very simple answer is having all the above functions have an async=False (removing wait and signal_function kwargs) default argument which returns the expected result, and if overridden to async=True it returns the future, which can be either:

• awaited/polled/etc via the Future interface or concurrent.futures methods
• Add a callback ala signal_function
• Ignored completely (if you aren't sensitive to timing/completion/ordering)

[davidplowman]

A few of other things:

• It seems like there's been some refactoring going on besides the use of futures. Is that something that could be factored in to a separate commit? It makes for a nicer "git history" if we can manage that.
• I'm aware of the PR that replaces lambdas with partials (RFC: Switch Lambda's to partial() #193) - I'd be happy to see that one move forward. (I note that that change is already present here.)
• Am I right in thinking the ability for a function to return False and remain in the list has gone? I think I'd prefer to keep that if we can, I imagine using it wait for particular things to happen (e.g. exposure values) without having to poll in the application code.

Thanks again!

[meawoppl]

Coming back to this. I see that futures have shown up in the internals, which is definitely progress in the right direction. I made a draft PR taking out the "return False to rerun" behavior, as it is not used. What are the use cases here? Some sort of request that takes a long time in a reentrant way?

I think exception based flow control would make more sense there vs. requiring a dozen or so other functions to return a slightly odd call signature. It also would implicitly support bubbling, which is nice given a lot of these calls are nested or chained. More specifically:

• You define something like class NeedsRerun(Exception)
• Calls that need a rerun throw that exception anywhere in the stack.
• You catch that specifically and requeue the function
• Other exceptions you catch/bubble as is current.

FWIW I am not attached to PR's but I throw them together in order to illustrate how I think a thing could get better, so take em or leave em, or let me know if you just want a subset of the changes etc.

[meawoppl]

I should note that we are explicitly interested in this so we can get a few things from picamera2 that would really help us:

• 1:1 request to serially captured frames (right now this is hard as you can't "queue" the internal event loop)
• More/faster insight on when settings changes are made, and the ability to do that in a clean/async way while doing other work

This is a common use-case for sky/science observing: #343

[davidplowman]

Hi, we've been doing quite a lot of work in this area. It's obviously dependent on what happens down in libcamera but we've made some proposals and implemented a prototype for the Raspberry Pi platform. Of course, the final implementation may be along the lines that we have proposed, or it could be something completely different, we can't make any assumptions yet!

We also have a version of Picamera2 that uses our prototype demo version of libcamera. It allows you to do things like send a set of controls and wait for them to take effect, something like this:

id = picam2.set_controls(controls)
request = picam2.capture_request(sync_id=id)

and also to pass in a list of "control lists" and wait for each to be applied in turn, a bit like this:

control_list = [{"ExposureTime": 10000, "AnalogueGain": 1}, {ExposureTime": 100000, "AnalogueGain": 1}]
picam2.capture_sequence(control_list, callback_function)

So I guess the message for the moment is to watch this space!