[libcamera-devel,v2,4/7] libcamera: Define a PixelFormat type for application-facing formats
diff mbox series

Message ID 20191028110208.15751-5-laurent.pinchart@ideasonboard.com
State Accepted
Commit 656d8757347f1f9f5df93da863f66fa589cee063
Headers show
Series
  • libcamera: Introduce a PixelFormat type
Related show

Commit Message

Laurent Pinchart Oct. 28, 2019, 11:02 a.m. UTC 
From: Jacopo Mondi <jacopo@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.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/meson.build    |  1 +
 include/libcamera/pixelformats.h | 18 ++++++++++++++++++
 src/libcamera/meson.build        |  1 +
 src/libcamera/pixelformats.cpp   | 26 ++++++++++++++++++++++++++
 4 files changed, 46 insertions(+)
 create mode 100644 include/libcamera/pixelformats.h
 create mode 100644 src/libcamera/pixelformats.cpp

Comments

Kieran Bingham Oct. 28, 2019, 11:05 a.m. UTC | #1 
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@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@ideasonboard.com>


> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/libcamera/meson.build    |  1 +
>  include/libcamera/pixelformats.h | 18 ++++++++++++++++++
>  src/libcamera/meson.build        |  1 +
>  src/libcamera/pixelformats.cpp   | 26 ++++++++++++++++++++++++++
>  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 */
>
Kieran Bingham Oct. 28, 2019, 11:56 a.m. UTC | #2 
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@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@ideasonboard.com>
> 
> 
>>
>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart@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.

--
Kieran



>>  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 */
>>
>
Laurent Pinchart Oct. 28, 2019, 3:02 p.m. UTC | #3 
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@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@ideasonboard.com>
> > 
> >> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@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 */

Patch
diff mbox series 

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 */