[libcamera-devel] [PATCH v2 4/7] libcamera: Define a PixelFormat type for application-facing formats

[Laurent Pinchart]

Hi Kieran,

On Mon, Oct 28, 2019 at 11:56:31AM +0000, Kieran Bingham wrote:
> coming back to this one for a minor comment:
> 
> On 28/10/2019 11:05, Kieran Bingham wrote:
> > Hi Jacopo, Laurent,
> > 
> > I like having this wrapped to an explicit type.
> > 
> > 
> > On 28/10/2019 11:02, Laurent Pinchart wrote:
> >> From: Jacopo Mondi <jacopo at jmondi.org>
> >>
> >> Define a PixelFormat type as a simple typedef to an uin32_t. The usage
> >> of a dedicated type creates a cleaner and more self-described aPI.
> > 
> > Trivials:
> > 
> >   s/uin32_t/uint32_t/
> >   s/aPI/API/
> > 
> > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > 
> >> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> >> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >> ---
> >>  include/libcamera/meson.build    |  1 +
> >>  include/libcamera/pixelformats.h | 18 ++++++++++++++++++
> >>  src/libcamera/meson.build        |  1 +
> >>  src/libcamera/pixelformats.cpp   | 26 ++++++++++++++++++++++++++
> 
> The new type added is "PixelFormat", not PixelFormats, and we haven't
> pluralised other types, (i.e. objects, requests, streams, signals etc.)
> 
> If you've specifically chosen to pluralise this for an extended reason,
> please add it to the commit log.

We have controls.cpp though, not control.cpp :-) I don't care much about
this, so I'll drop the S.

> >>  4 files changed, 46 insertions(+)
> >>  create mode 100644 include/libcamera/pixelformats.h
> >>  create mode 100644 src/libcamera/pixelformats.cpp
> >>
> >> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> >> index ed8ff917e35a..99abf0609940 100644
> >> --- a/include/libcamera/meson.build
> >> +++ b/include/libcamera/meson.build
> >> @@ -9,6 +9,7 @@ libcamera_api = files([
> >>      'geometry.h',
> >>      'logging.h',
> >>      'object.h',
> >> +    'pixelformats.h',
> >>      'request.h',
> >>      'signal.h',
> >>      'stream.h',
> >> diff --git a/include/libcamera/pixelformats.h b/include/libcamera/pixelformats.h
> >> new file mode 100644
> >> index 000000000000..6e25b8d8b76e
> >> --- /dev/null
> >> +++ b/include/libcamera/pixelformats.h
> >> @@ -0,0 +1,18 @@
> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> >> +/*
> >> + * Copyright (C) 2019, Google Inc.
> >> + *
> >> + * pixelformats.h - libcamera pixel formats
> >> + */
> >> +#ifndef __LIBCAMERA_PIXEL_FORMATS_H__
> >> +#define __LIBCAMERA_PIXEL_FORMATS_H__
> >> +
> >> +#include <stdint.h>
> >> +
> >> +namespace libcamera {
> >> +
> >> +using PixelFormat = uint32_t;
> >> +
> >> +} /* namespace libcamera */
> >> +
> >> +#endif /* __LIBCAMERA_PIXEL_FORMATS_H__ */
> >> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> >> index d329820b9582..f201f408ef07 100644
> >> --- a/src/libcamera/meson.build
> >> +++ b/src/libcamera/meson.build
> >> @@ -25,6 +25,7 @@ libcamera_sources = files([
> >>      'message.cpp',
> >>      'object.cpp',
> >>      'pipeline_handler.cpp',
> >> +    'pixelformats.cpp',
> >>      'process.cpp',
> >>      'request.cpp',
> >>      'signal.cpp',
> >> diff --git a/src/libcamera/pixelformats.cpp b/src/libcamera/pixelformats.cpp
> >> new file mode 100644
> >> index 000000000000..9377fb5e0749
> >> --- /dev/null
> >> +++ b/src/libcamera/pixelformats.cpp
> >> @@ -0,0 +1,26 @@
> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> >> +/*
> >> + * Copyright (C) 2019, Google Inc.
> >> + *
> >> + * pixelformats.cpp - libcamera pixel formats
> >> + */
> >> +
> >> +#include <libcamera/pixelformats.h>
> >> +
> >> +/**
> >> + * \file pixelformats.h
> >> + * \brief libcamera pixel formats
> >> + */
> >> +
> >> +namespace libcamera {
> >> +
> >> +/**
> >> + * \typedef PixelFormat
> >> + * \brief libcamera image pixel format
> >> + *
> >> + * The PixelFormat type describes the format of images in the public libcamera
> >> + * API. It stores a FourCC value in a 32-bit unsigned integer. The values are
> >> + * defined in the Linux kernel V4L2 API (see linux/videodev2.h).
> >> + */
> >> +
> >> +} /* namespace libcamera */

-- 
Regards,

Laurent Pinchart