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

[Kieran Bingham]

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.

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

-- 
Regards
--
Kieran