[libcamera-devel] [PATCH v2 11/13] py: implement PixelFormat class
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue May 17 19:11:56 CEST 2022
Quoting Laurent Pinchart (2022-05-17 17:15:39)
> Hi Tomi,
>
> Thank you for the patch.
>
> On Tue, May 17, 2022 at 05:33:23PM +0300, Tomi Valkeinen wrote:
> > Implement PixelFormat bindings properly with a PixelFormat class. Change
> > the bindings to use the new class instead of a string.
> >
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
> > ---
> > src/py/cam/cam.py | 2 +-
> > src/py/cam/cam_kms.py | 10 +--------
> > src/py/cam/cam_qt.py | 4 +++-
> > src/py/cam/cam_qtgl.py | 17 +--------------
> > src/py/cam/gl_helpers.py | 8 -------
> > src/py/libcamera/pymain.cpp | 42 ++++++++++++++++++-------------------
> > 6 files changed, 27 insertions(+), 56 deletions(-)
> >
> > diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py
> > index c7da97d7..001fb9de 100755
> > --- a/src/py/cam/cam.py
> > +++ b/src/py/cam/cam.py
> > @@ -164,7 +164,7 @@ def configure(ctx):
> > stream_config.size = (stream_opts['width'], stream_opts['height'])
> >
> > if 'pixelformat' in stream_opts:
> > - stream_config.pixel_format = stream_opts['pixelformat']
> > + stream_config.pixel_format = libcam.PixelFormat(stream_opts['pixelformat'])
> >
> > stat = camconfig.validate()
> >
> > diff --git a/src/py/cam/cam_kms.py b/src/py/cam/cam_kms.py
> > index d8ff0284..04381da1 100644
> > --- a/src/py/cam/cam_kms.py
> > +++ b/src/py/cam/cam_kms.py
> > @@ -5,14 +5,6 @@ import pykms
> > import selectors
> > import sys
> >
> > -FMT_MAP = {
> > - 'RGB888': pykms.PixelFormat.RGB888,
> > - 'YUYV': pykms.PixelFormat.YUYV,
> > - 'ARGB8888': pykms.PixelFormat.ARGB8888,
> > - 'XRGB8888': pykms.PixelFormat.XRGB8888,
> > - 'NV12': pykms.PixelFormat.NV12,
> > -}
> > -
> >
> > class KMSRenderer:
> > def __init__(self, state):
> > @@ -120,7 +112,7 @@ class KMSRenderer:
> >
> > cfg = stream.configuration
> > fmt = cfg.pixel_format
> > - fmt = FMT_MAP[fmt]
> > + fmt = pykms.PixelFormat(fmt.fourcc)
> >
> > plane = self.resman.reserve_generic_plane(self.crtc, fmt)
> > assert(plane is not None)
> > diff --git a/src/py/cam/cam_qt.py b/src/py/cam/cam_qt.py
> > index fb485b9b..45a30aeb 100644
> > --- a/src/py/cam/cam_qt.py
> > +++ b/src/py/cam/cam_qt.py
> > @@ -87,6 +87,8 @@ def to_rgb(fmt, size, data):
> > w = size[0]
> > h = size[1]
> >
> > + fmt = str(fmt)
> > +
> > if fmt == 'YUYV':
> > # YUV422
> > yuyv = data.reshape((h, w // 2 * 4))
> > @@ -293,7 +295,7 @@ class MainWindow(QtWidgets.QWidget):
> > w, h = cfg.size
> > pitch = cfg.stride
> >
> > - if cfg.pixel_format == 'MJPEG':
> > + if str(cfg.pixel_format) == 'MJPEG':
>
> Ideally this should be
>
> if cfg.pixel_format == libcamera.formats.MJPEG
>
> but that can come later. Maybe a todo comment in the bindings to
> remember that ?
I assume this isn't currently possible, as there's no full conversion of
the types, just runtime conversion with fromString() etc?
But a todo works for me too, and the str() conversion looks like it
already does the right thing at least in this instance. so
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> > img = Image.open(BytesIO(mfb.planes[0]))
> > qim = ImageQt(img).copy()
> > pix = QtGui.QPixmap.fromImage(qim)
> > diff --git a/src/py/cam/cam_qtgl.py b/src/py/cam/cam_qtgl.py
> > index 8a95994e..261accb8 100644
> > --- a/src/py/cam/cam_qtgl.py
> > +++ b/src/py/cam/cam_qtgl.py
> > @@ -30,14 +30,6 @@ from OpenGL.GL import shaders
> >
> > from gl_helpers import *
> >
> > -# libcamera format string -> DRM fourcc
> > -FMT_MAP = {
> > - 'RGB888': 'RG24',
> > - 'XRGB8888': 'XR24',
> > - 'ARGB8888': 'AR24',
> > - 'YUYV': 'YUYV',
> > -}
> > -
> >
> > class EglState:
> > def __init__(self):
> > @@ -204,12 +196,6 @@ class MainWindow(QtWidgets.QWidget):
> > self.current[ctx['idx']] = []
> >
> > for stream in ctx['streams']:
> > - fmt = stream.configuration.pixel_format
> > - size = stream.configuration.size
> > -
> > - if fmt not in FMT_MAP:
> > - raise Exception('Unsupported pixel format: ' + str(fmt))
> > -
> > self.textures[stream] = None
> >
> > num_tiles = len(self.textures)
> > @@ -281,8 +267,7 @@ class MainWindow(QtWidgets.QWidget):
> >
> > def create_texture(self, stream, fb):
> > cfg = stream.configuration
> > - fmt = cfg.pixel_format
> > - fmt = str_to_fourcc(FMT_MAP[fmt])
> > + fmt = cfg.pixel_format.fourcc
> > w, h = cfg.size
> >
> > attribs = [
> > diff --git a/src/py/cam/gl_helpers.py b/src/py/cam/gl_helpers.py
> > index ac5e6889..53b3e9df 100644
> > --- a/src/py/cam/gl_helpers.py
> > +++ b/src/py/cam/gl_helpers.py
> > @@ -30,14 +30,6 @@ def getglEGLImageTargetTexture2DOES():
> >
> > glEGLImageTargetTexture2DOES = getglEGLImageTargetTexture2DOES()
> >
> > -# \todo This can be dropped when we have proper PixelFormat bindings
> > -def str_to_fourcc(str):
> > - assert(len(str) == 4)
> > - fourcc = 0
> > - for i, v in enumerate([ord(c) for c in str]):
> > - fourcc |= v << (i * 8)
> > - return fourcc
> > -
> >
> > def get_gl_extensions():
> > n = GLint()
> > diff --git a/src/py/libcamera/pymain.cpp b/src/py/libcamera/pymain.cpp
> > index af22205e..97b05903 100644
> > --- a/src/py/libcamera/pymain.cpp
> > +++ b/src/py/libcamera/pymain.cpp
> > @@ -8,7 +8,6 @@
> > /*
> > * \todo Add geometry classes (Point, Rectangle...)
> > * \todo Add bindings for the ControlInfo class
> > - * \todo Add bindings for the PixelFormat class
> > */
> >
> > #include <mutex>
> > @@ -173,6 +172,7 @@ PYBIND11_MODULE(_libcamera, m)
> > auto pyColorSpaceTransferFunction = py::enum_<ColorSpace::TransferFunction>(pyColorSpace, "TransferFunction");
> > auto pyColorSpaceYcbcrEncoding = py::enum_<ColorSpace::YcbcrEncoding>(pyColorSpace, "YcbcrEncoding");
> > auto pyColorSpaceRange = py::enum_<ColorSpace::Range>(pyColorSpace, "Range");
> > + auto pyPixelFormat = py::class_<PixelFormat>(m, "PixelFormat");
> >
> > /* Global functions */
> > m.def("log_set_level", &logSetLevel);
> > @@ -404,14 +404,7 @@ PYBIND11_MODULE(_libcamera, m)
> > self.size.width = std::get<0>(size);
> > self.size.height = std::get<1>(size);
> > })
> > - .def_property(
> > - "pixel_format",
> > - [](StreamConfiguration &self) {
> > - return self.pixelFormat.toString();
> > - },
> > - [](StreamConfiguration &self, std::string fmt) {
> > - self.pixelFormat = PixelFormat::fromString(fmt);
> > - })
> > + .def_readwrite("pixel_format", &StreamConfiguration::pixelFormat)
> > .def_readwrite("stride", &StreamConfiguration::stride)
> > .def_readwrite("frame_size", &StreamConfiguration::frameSize)
> > .def_readwrite("buffer_count", &StreamConfiguration::bufferCount)
> > @@ -420,22 +413,15 @@ PYBIND11_MODULE(_libcamera, m)
> > .def_readwrite("color_space", &StreamConfiguration::colorSpace);
> >
> > pyStreamFormats
> > - .def_property_readonly("pixel_formats", [](StreamFormats &self) {
> > - std::vector<std::string> fmts;
> > - for (auto &fmt : self.pixelformats())
> > - fmts.push_back(fmt.toString());
> > - return fmts;
> > - })
> > - .def("sizes", [](StreamFormats &self, const std::string &pixelFormat) {
> > - auto fmt = PixelFormat::fromString(pixelFormat);
> > + .def_property_readonly("pixel_formats", &StreamFormats::pixelformats)
> > + .def("sizes", [](StreamFormats &self, const PixelFormat &pixelFormat) {
> > std::vector<std::tuple<uint32_t, uint32_t>> fmts;
> > - for (const auto &s : self.sizes(fmt))
> > + for (const auto &s : self.sizes(pixelFormat))
> > fmts.push_back(std::make_tuple(s.width, s.height));
> > return fmts;
> > })
> > - .def("range", [](StreamFormats &self, const std::string &pixelFormat) {
> > - auto fmt = PixelFormat::fromString(pixelFormat);
> > - const auto &range = self.range(fmt);
> > + .def("range", [](StreamFormats &self, const PixelFormat &pixelFormat) {
> > + const auto &range = self.range(pixelFormat);
> > return make_tuple(std::make_tuple(range.hStep, range.vStep),
> > std::make_tuple(range.min.width, range.min.height),
> > std::make_tuple(range.max.width, range.max.height));
> > @@ -648,4 +634,18 @@ PYBIND11_MODULE(_libcamera, m)
> > pyColorSpaceRange
> > .value("Full", ColorSpace::Range::Full)
> > .value("Limited", ColorSpace::Range::Limited);
> > +
> > + pyPixelFormat
> > + .def(py::init<>())
> > + .def(py::init<uint32_t, uint64_t>())
> > + .def(py::init<>([](const std::string &str) {
> > + return PixelFormat::fromString(str);
> > + }))
> > + .def_property_readonly("fourcc", &PixelFormat::fourcc)
> > + .def_property_readonly("modifier", &PixelFormat::modifier)
> > + .def(py::self == py::self)
> > + .def("__str__", &PixelFormat::toString)
> > + .def("__repr__", [](const PixelFormat &self) {
> > + return "libcamera.PixelFormat('" + self.toString() + "')";
> > + });
> > }
>
> --
> Regards,
>
> Laurent Pinchart
More information about the libcamera-devel
mailing list