[libcamera-devel] [PATCH 10/14] py: implement PixelFormat class
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue May 17 10:32:08 CEST 2022
Hi Tomi,
Thank you for the patch.
On Mon, May 16, 2022 at 05:10:18PM +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 | 4 +--
> src/py/cam/cam_kms.py | 9 +-----
> src/py/cam/cam_qt.py | 2 +-
> src/py/cam/cam_qtgl.py | 17 +-----------
> src/py/cam/gl_helpers.py | 8 ------
> src/py/libcamera/pymain.cpp | 51 ++++++++++++++++++++--------------
> src/py/libcamera/utils/conv.py | 2 ++
> 7 files changed, 37 insertions(+), 56 deletions(-)
>
> diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py
> index c7da97d7..63c67126 100755
> --- a/src/py/cam/cam.py
> +++ b/src/py/cam/cam.py
> @@ -80,7 +80,7 @@ def do_cmd_info(ctx):
>
> formats = stream_config.formats
> for fmt in formats.pixel_formats:
> - print('\t * Pixelformat:', fmt, formats.range(fmt))
> + print('\t * Pixelformat:', fmt.name, formats.range(fmt))
>
> for size in formats.sizes(fmt):
> print('\t -', size)
> @@ -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.from_name(stream_opts['pixelformat'])
>
> stat = camconfig.validate()
>
> diff --git a/src/py/cam/cam_kms.py b/src/py/cam/cam_kms.py
> index ae6be277..7d11564b 100644
> --- a/src/py/cam/cam_kms.py
> +++ b/src/py/cam/cam_kms.py
> @@ -5,13 +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,
> -}
> -
>
> class KMSRenderer:
> def __init__(self, state):
> @@ -118,7 +111,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 dc5219eb..64f49f33 100644
> --- a/src/py/cam/cam_qt.py
> +++ b/src/py/cam/cam_qt.py
> @@ -139,7 +139,7 @@ class MainWindow(QtWidgets.QWidget):
> w, h = cfg.size
> pitch = cfg.stride
>
> - if cfg.pixel_format == 'MJPEG':
> + if cfg.pixel_format.name == 'MJPEG':
> 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..cc2ddee5 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,27 @@ 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_property_readonly("fourcc", &PixelFormat::fourcc)
> + .def_property_readonly("modifier", &PixelFormat::modifier)
> + .def_property_readonly("name", &PixelFormat::toString)
> + .def(py::self == py::self)
> + .def("__str__", [](const PixelFormat &self) {
> + return "<libcamera.PixelFormat '" + self.toString() + "'>";
> + })
I'd just return self.toString() here. We could also implement a __repr__
that prints "libcamera.PixelFormat('" + self.toString() + "')" if we add
a constructor that takes a string as __repr__ is supposed to return a
canonical string representation that can be run to recreate the object.
> + .def_static("from_name", [](const std::string &name) {
Could we name this "from_string" to avoid diverging from the C++ API ?
> + return PixelFormat::fromString(name);
> + })
> + .def_static("from_fourcc", [](const std::string &fourcc) {
> + if (fourcc.size() != 4)
> + throw std::invalid_argument("Invalid fourcc length");
> +
> + uint32_t v = fourcc[0] | (fourcc[1] << 8) |
> + (fourcc[2] << 16) | (fourcc[3] << 24);
> +
> + return PixelFormat(v);
> + });
This isn't used in your series, so I'd drop it.
> }
> diff --git a/src/py/libcamera/utils/conv.py b/src/py/libcamera/utils/conv.py
> index 2e483003..30f6733e 100644
> --- a/src/py/libcamera/utils/conv.py
> +++ b/src/py/libcamera/utils/conv.py
> @@ -76,6 +76,8 @@ def to_rgb(fmt, size, data):
> w = size[0]
> h = size[1]
>
> + fmt = fmt.name
> +
> if fmt == 'YUYV':
> # YUV422
> yuyv = data.reshape((h, w // 2 * 4))
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list