Message ID | 20211209092906.37303-1-tomi.valkeinen@ideasonboard.com |
---|---|
Headers | show |
Series |
|
Related | show |
Hi Tomi, everyone Thanks for submitting this set, very pleased to see this moving again! I've actually been using these bindings extensively over the last several weeks, so we could add: Tested-by: David Plowman <david.plowman@raspberrypi.com> to all of them. A couple of points: * For my Picamera2 use cases (which are not remarkable) I had to add support for array controls, controls in the camera start() method, and an implementation of the transforms. I can submit those changes on top of this patch set at a later date, though they probably lack the requisite loveliness at the moment. * About the name "pycamera". Could we use "pylibcamera" instead? I worry that "pycamera" would cause more confusion with "picamera", which is of course our legacy stack Python library that Picamera2 is replacing. Thanks! David On Thu, 9 Dec 2021 at 09:29, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote: > > Hi, > > Third RFC of the python bindings. I believe it's over a year since the > last version, and I've been tinkering with the bindings every now and > then. > > This version includes cam.py, which mimics 'cam' tool. The idea is not > to try to replace 'cam', but to test the bindings in a real use case, > rather than artificial tests. > > There are a few clear items that should be done to make this more > usable: > > - Fix the ControlValue handling, as arrays are not handled at all. > Possibly other types are not handled correctly. > - Add ControlList parameter to Camera.start() > - Add Transform > > Tomi > > Tomi Valkeinen (5): > HACK: Camera public destructor > libcamera: Request: expose Camera from Request > gitignore: add .cache > Add Python bindings > py: Add cam.py > > .gitignore | 2 + > include/libcamera/camera.h | 2 +- > include/libcamera/request.h | 2 + > meson.build | 1 + > meson_options.txt | 5 + > src/libcamera/request.cpp | 5 + > src/meson.build | 1 + > src/py/meson.build | 1 + > src/py/pycamera/__init__.py | 10 + > src/py/pycamera/meson.build | 43 ++++ > src/py/pycamera/pymain.cpp | 424 ++++++++++++++++++++++++++++++++ > src/py/test/cam.py | 464 ++++++++++++++++++++++++++++++++++++ > src/py/test/cam_kms.py | 185 ++++++++++++++ > src/py/test/cam_null.py | 46 ++++ > src/py/test/cam_qt.py | 355 +++++++++++++++++++++++++++ > src/py/test/cam_qtgl.py | 385 ++++++++++++++++++++++++++++++ > src/py/test/gl_helpers.py | 67 ++++++ > subprojects/pybind11.wrap | 12 + > 18 files changed, 2009 insertions(+), 1 deletion(-) > create mode 100644 src/py/meson.build > create mode 100644 src/py/pycamera/__init__.py > create mode 100644 src/py/pycamera/meson.build > create mode 100644 src/py/pycamera/pymain.cpp > create mode 100755 src/py/test/cam.py > create mode 100644 src/py/test/cam_kms.py > create mode 100644 src/py/test/cam_null.py > create mode 100644 src/py/test/cam_qt.py > create mode 100644 src/py/test/cam_qtgl.py > create mode 100644 src/py/test/gl_helpers.py > create mode 100644 subprojects/pybind11.wrap > > -- > 2.25.1 >
Hello, On Thu, Dec 09, 2021 at 10:32:17AM +0000, David Plowman wrote: > Hi Tomi, everyone > > Thanks for submitting this set, very pleased to see this moving again! > > I've actually been using these bindings extensively over the last > several weeks, so we could add: > > Tested-by: David Plowman <david.plowman@raspberrypi.com> > > to all of them. > > A couple of points: > > * For my Picamera2 use cases (which are not remarkable) I had to add > support for array controls, controls in the camera start() method, and > an implementation of the transforms. I can submit those changes on top > of this patch set at a later date, though they probably lack the > requisite loveliness at the moment. > > * About the name "pycamera". Could we use "pylibcamera" instead? I > worry that "pycamera" would cause more confusion with "picamera", > which is of course our legacy stack Python library that Picamera2 is > replacing. One always loves a naming discussion. Do we need the "py" prefix ? I would assume that a Python module can safely be considered to be python-related, even with a "py" prefix :-) We could then name the module "libcamera", "camera" or "cam" (the last one may be a bit too short). > On Thu, 9 Dec 2021 at 09:29, Tomi Valkeinen wrote: > > > > Hi, > > > > Third RFC of the python bindings. I believe it's over a year since the > > last version, and I've been tinkering with the bindings every now and > > then. > > > > This version includes cam.py, which mimics 'cam' tool. The idea is not > > to try to replace 'cam', but to test the bindings in a real use case, > > rather than artificial tests. > > > > There are a few clear items that should be done to make this more > > usable: > > > > - Fix the ControlValue handling, as arrays are not handled at all. > > Possibly other types are not handled correctly. > > - Add ControlList parameter to Camera.start() > > - Add Transform > > > > Tomi > > > > Tomi Valkeinen (5): > > HACK: Camera public destructor > > libcamera: Request: expose Camera from Request > > gitignore: add .cache > > Add Python bindings > > py: Add cam.py > > > > .gitignore | 2 + > > include/libcamera/camera.h | 2 +- > > include/libcamera/request.h | 2 + > > meson.build | 1 + > > meson_options.txt | 5 + > > src/libcamera/request.cpp | 5 + > > src/meson.build | 1 + > > src/py/meson.build | 1 + > > src/py/pycamera/__init__.py | 10 + > > src/py/pycamera/meson.build | 43 ++++ > > src/py/pycamera/pymain.cpp | 424 ++++++++++++++++++++++++++++++++ > > src/py/test/cam.py | 464 ++++++++++++++++++++++++++++++++++++ > > src/py/test/cam_kms.py | 185 ++++++++++++++ > > src/py/test/cam_null.py | 46 ++++ > > src/py/test/cam_qt.py | 355 +++++++++++++++++++++++++++ > > src/py/test/cam_qtgl.py | 385 ++++++++++++++++++++++++++++++ > > src/py/test/gl_helpers.py | 67 ++++++ > > subprojects/pybind11.wrap | 12 + > > 18 files changed, 2009 insertions(+), 1 deletion(-) > > create mode 100644 src/py/meson.build > > create mode 100644 src/py/pycamera/__init__.py > > create mode 100644 src/py/pycamera/meson.build > > create mode 100644 src/py/pycamera/pymain.cpp > > create mode 100755 src/py/test/cam.py > > create mode 100644 src/py/test/cam_kms.py > > create mode 100644 src/py/test/cam_null.py > > create mode 100644 src/py/test/cam_qt.py > > create mode 100644 src/py/test/cam_qtgl.py > > create mode 100644 src/py/test/gl_helpers.py > > create mode 100644 subprojects/pybind11.wrap
On 09/12/2021 18:44, Laurent Pinchart wrote: > Hello, > > On Thu, Dec 09, 2021 at 10:32:17AM +0000, David Plowman wrote: >> Hi Tomi, everyone >> >> Thanks for submitting this set, very pleased to see this moving again! >> >> I've actually been using these bindings extensively over the last >> several weeks, so we could add: >> >> Tested-by: David Plowman <david.plowman@raspberrypi.com> >> >> to all of them. >> >> A couple of points: >> >> * For my Picamera2 use cases (which are not remarkable) I had to add >> support for array controls, controls in the camera start() method, and >> an implementation of the transforms. I can submit those changes on top >> of this patch set at a later date, though they probably lack the >> requisite loveliness at the moment. >> >> * About the name "pycamera". Could we use "pylibcamera" instead? I >> worry that "pycamera" would cause more confusion with "picamera", >> which is of course our legacy stack Python library that Picamera2 is >> replacing. > > One always loves a naming discussion. > > Do we need the "py" prefix ? I would assume that a Python module can > safely be considered to be python-related, even with a "py" prefix :-) > We could then name the module "libcamera", "camera" or "cam" (the last > one may be a bit too short). The binding generator currently builds "pycamera.so". I'm not sure if creating a libcamera.so for both the libcamera library and the python bindings will create problems. The bindings .so won't be in any common lib directory, but I can imagine having interesting issues due to this. I can give it a try. Tomi
Hi Tomi, On Fri, Dec 10, 2021 at 02:16:40PM +0200, Tomi Valkeinen wrote: > On 09/12/2021 18:44, Laurent Pinchart wrote: > > On Thu, Dec 09, 2021 at 10:32:17AM +0000, David Plowman wrote: > >> Hi Tomi, everyone > >> > >> Thanks for submitting this set, very pleased to see this moving again! > >> > >> I've actually been using these bindings extensively over the last > >> several weeks, so we could add: > >> > >> Tested-by: David Plowman <david.plowman@raspberrypi.com> > >> > >> to all of them. > >> > >> A couple of points: > >> > >> * For my Picamera2 use cases (which are not remarkable) I had to add > >> support for array controls, controls in the camera start() method, and > >> an implementation of the transforms. I can submit those changes on top > >> of this patch set at a later date, though they probably lack the > >> requisite loveliness at the moment. > >> > >> * About the name "pycamera". Could we use "pylibcamera" instead? I > >> worry that "pycamera" would cause more confusion with "picamera", > >> which is of course our legacy stack Python library that Picamera2 is > >> replacing. > > > > One always loves a naming discussion. > > > > Do we need the "py" prefix ? I would assume that a Python module can > > safely be considered to be python-related, even with a "py" prefix :-) > > We could then name the module "libcamera", "camera" or "cam" (the last > > one may be a bit too short). > > The binding generator currently builds "pycamera.so". I'm not sure if > creating a libcamera.so for both the libcamera library and the python > bindings will create problems. The bindings .so won't be in any common > lib directory, but I can imagine having interesting issues due to this. "pycamera.so" sounds good to me, but as we have a __init__.py, maybe we could store it in a "camera" directory, and import * from pycamera in __init__.py (or something similar) ? PEP8 also recommends "When an extension module written in C or C++ has an accompanying Python module that provides a higher level (e.g. more object oriented) interface, the C/C++ module has a leading underscore (e.g. _socket)." (https://www.python.org/dev/peps/pep-0008/#package-and-module-names) > I can give it a try.