[libcamera-devel] [PATCH v4 02/31] libcamera: formats: Define FormatEnum type

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wed, Mar 20, 2019 at 05:30:26PM +0100, Jacopo Mondi wrote:
> Add an internal format.h and format.cpp files to collect libcamera image
> format related types, helpers and structures. Define and document the
> FormatEnum type, used to enumerate pixel image formats and associated
> image resolutions.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/formats.cpp              | 27 ++++++++++++++++++++++++++
>  src/libcamera/include/formats.h        | 22 +++++++++++++++++++++
>  src/libcamera/include/v4l2_subdevice.h |  4 ++--
>  src/libcamera/meson.build              |  1 +
>  src/libcamera/v4l2_subdevice.cpp       |  5 ++---
>  5 files changed, 54 insertions(+), 5 deletions(-)
>  create mode 100644 src/libcamera/formats.cpp
>  create mode 100644 src/libcamera/include/formats.h
> 
> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> new file mode 100644
> index 000000000000..a6251fe91cec
> --- /dev/null
> +++ b/src/libcamera/formats.cpp
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * formats.cpp - Libcamera image formats
> + */
> +
> +#include "formats.h"
> +
> +/**
> + * \file formats.h
> + * \brief Types and helper methods to handle libcamera image formats
> + */
> +
> +namespace libcamera {
> +
> +/**
> + * \typedef FormatEnum
> + * \brief Type definition for the map of image formats and sizes
> + *
> + * Type definition used to enumerate the supported pixel formats and image frame
> + * sizes. The type associates in a map a pixel code, used to represent memory or

The terms used in V4L2 are either pixel format or media bus code, not
pixel code. I would write "The type associates in a map a pixel format
(for memory formats) or media bus code (for bus formats), ...".

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> + * wire image formats, to a vector of image resolutions represented by SizeRange
> + * items.
> + */
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/include/formats.h b/src/libcamera/include/formats.h
> new file mode 100644
> index 000000000000..5fcfb11318e7
> --- /dev/null
> +++ b/src/libcamera/include/formats.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * formats.h - Libcamera image formats
> + */
> +
> +#ifndef __LIBCAMERA_FORMATS_H__
> +#define __LIBCAMERA_FORMATS_H__
> +
> +#include <map>
> +#include <vector>
> +
> +#include "geometry.h"
> +
> +namespace libcamera {
> +
> +typedef std::map<unsigned int, std::vector<SizeRange>> FormatEnum;
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_FORMATS_H__ */
> diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h
> index 700e66bcddac..3ecf08514898 100644
> --- a/src/libcamera/include/v4l2_subdevice.h
> +++ b/src/libcamera/include/v4l2_subdevice.h
> @@ -11,6 +11,7 @@
>  #include <string>
>  #include <vector>
>  
> +#include "formats.h"
>  #include "geometry.h"
>  #include "log.h"
>  #include "media_object.h"
> @@ -42,8 +43,7 @@ public:
>  	int setCrop(unsigned int pad, Rectangle *rect);
>  	int setCompose(unsigned int pad, Rectangle *rect);
>  
> -	const std::map<unsigned int, std::vector<SizeRange>>
> -						formats(unsigned int pad);
> +	FormatEnum formats(unsigned int pad);
>  
>  	int getFormat(unsigned int pad, V4L2SubdeviceFormat *format);
>  	int setFormat(unsigned int pad, V4L2SubdeviceFormat *format);
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 8384cd0af451..4433abfceca3 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -6,6 +6,7 @@ libcamera_sources = files([
>      'event_dispatcher.cpp',
>      'event_dispatcher_poll.cpp',
>      'event_notifier.cpp',
> +    'formats.cpp',
>      'geometry.cpp',
>      'log.cpp',
>      'media_device.cpp',
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index 11d22da25728..7084f28c0be8 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -210,10 +210,9 @@ int V4L2Subdevice::setCompose(unsigned int pad, Rectangle *rect)
>   * \return A map of image formats associated with a list of image sizes, or
>   * an empty map on error or if the pad does not exist
>   */
> -const std::map<unsigned int, std::vector<SizeRange>>
> -V4L2Subdevice::formats(unsigned int pad)
> +FormatEnum V4L2Subdevice::formats(unsigned int pad)
>  {
> -	std::map<unsigned int, std::vector<SizeRange>> formatMap = {};
> +	FormatEnum formatMap = {};
>  	struct v4l2_subdev_mbus_code_enum mbusEnum = {};
>  	int ret;
>  

-- 
Regards,

Laurent Pinchart