[libcamera-devel] [PATCH 08/14] py: move conversion funcs to libcamera.utils

[Tomi Valkeinen]

On 17/05/2022 11:55, Laurent Pinchart wrote:
> Hi Tomi,
> 
> On Tue, May 17, 2022 at 11:38:03AM +0300, Tomi Valkeinen wrote:
>> On 17/05/2022 11:23, Laurent Pinchart wrote:
>>> On Mon, May 16, 2022 at 05:10:16PM +0300, Tomi Valkeinen wrote:
>>>> cam_qt.py has a bunch of utility functions for pixel format conversions.
>>>> Create a new libcamera submodule for these.
>>>
>>> I'm not sure I like this much. Do we really want to create a format
>>> conversion library that will we'll eventually have to maintain with a
>>> stable ABI ?
>>
>> That's a good question. I've been thinking a lot about what exactly
>> should be bindings module contain.
>>
>> Just the bare-bone bindings? Then it's probably not very pythonic and
>> not easy to use.
>>
>> Or try to keep to the bare bones feature-wise (i.e. no conversions
>> features like this), but add convenience features to make it look more
>> pythonic? Then the API deviates from the C++ API.
>>
>> Or bare-bone bindings, but a separate, pure python library on top? What
>> do we call the libraries, then =). And if everyone uses the pure python
>> lib, what is the point of a separate bare-bone library?
> 
> I'd like the bare-bone bindings to be as close as possible to the C++
> API, with differences being clearly documented and globally consistent
> (e.g. all toString() functions could be replaced by __str__(), camelCase
> translated to snake_case, ...). That's more or less what we have today,
> and it doesn't seem problematic to me.
> 
> I expect we'll have convenience helpers that offer a higher-level API,
> implemented in Python, and I expect both the low-level and the
> high-level APIs to be used by different applications. Naming is a good
> question though.

Just thinking out loud, I think we have some classes of "levels":

1) Really bare bones, reflects C++ API if at all possible

2) Minor pythonic helpers (like the geometry constructors taking tuples, 
or unpacking). I think these are usually add-on functions/properties to 
existing classes.

3) Helper classes like MappedFrameBuffer

4) Higher level module which possibly fully hides the libcamera module below

The last one is clearly outside the scope here. 2) is currently in the 
native bindings. I think it could be moved to the __init__.py of the 
libcamera module, although I'm not sure if that's always easily 
possible. 3) Would be in a submodule.

This would give the user the following options:

import libcamera._libcamera (to get the bare-bones)
import libcamera (to get the bare-bones + helper methods)

I don't think anyone would use libcamera._libcamera, and that's also 
implied by the naming.

Then again, if no one uses libcamera._libcamera directly, having 2) in 
the native bindings is not really an issue.

>> I'm currently thinking that this all should be under one libcamera
>> module. "libcamera._libcamera" is the bare-bones native bindings (as it
>> already is), and libcamera module exposes a more pythonic version with
>> some helper classes (like the MappedFrameBuffer class we already have).
>> We can then have other submodules, which are not needed for libcamera
>> use, but provide extra features, like libcamera.utils. Perhaps
>> MappedFrameBuffer should also be moved there.
> 
> I like the idea of moving extra features that are not provided by the
> C++ API and that are not higher-level wrappers to a separate module.
> MappedFrameBuffer should go there indeed (until libcamera gets a C++
> Image class).
> 
>> I think the main libcamera and the libcamera.utils can be considered
>> separate modules, and I don't see a problem with having stable libcamera
>> API, but libcamera.utils is not stable. But to make it more obvious, we
>> could as well have, say, libcamera.experimental module.
> 
> What's the point in an API that can't be used by anyone without a high
> risk of breakage ? :-)

Well, things should be easy with Python. If the user has a raw bayer 
camera, and the user asks how to see the image, and we reply that you 
need to implement your own debayering function...

I think somehow we should offer code snippets or helpers that make using 
libcamera easy, even if we don't really support those helpers. Of 
course, preferably these kind of features should be found in other 
libraries that specialize in that feature-set.

>> That said, conversion functionality is something I'd expect to find from
>> some other Python module. But if I recall right, I couldn't find a
>> working bayer conversion.
> 
> What bothers me with a library that implement format conversion is that
> it will very quickly become a large project of its own, so I don't think
> it should be part of libcamera.

It shouldn't be part of the main libcamera module. But I don't see a 
problem with offering experimental/example code as a submodule. It can 
be a separate installable.

  Tomi