[libcamera-devel] [PATCH v4 3/6] libcamera: CameraSensorProperties: Adds table of v4l2 index and test pattern

[Jacopo Mondi]

Hi Hiro

On Thu, May 06, 2021 at 04:54:46PM +0900, Hirokazu Honda wrote:
> In V4L2 API, a driver returns an index (also with a name) to
> represent a test pattern, but it is a driver specific what test
> pattern is represented by the index. Therefore, this adds a
> mapping table from the index and a test pattern into a static
> configuration of a sensor.

A bit convoluted... What about something along the lines of:

The V4L2 specification defines the sensor test pattern modes
through a menu control, where a numerical index is associated to
a string that describes the test pattern. The index-to-pattern
mapping is driver specific and requires a corresponding representation
in the library.

Add to the static list of CameraSensorProperties a map of indexes to
libcamera::controls::TestPatternModes values to be able to map the
indexes returned by the driver to the corresponding test pattern mode.


>
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> ---
>  .../internal/camera_sensor_properties.h       |  2 ++
>  src/libcamera/camera_sensor_properties.cpp    | 20 +++++++++++++++++++
>  2 files changed, 22 insertions(+)
>
> diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h
> index f5e242cb..f52890b4 100644
> --- a/include/libcamera/internal/camera_sensor_properties.h
> +++ b/include/libcamera/internal/camera_sensor_properties.h
> @@ -7,6 +7,7 @@
>  #ifndef __LIBCAMERA_SENSOR_CAMERA_SENSOR_PROPERTIES_H__
>  #define __LIBCAMERA_SENSOR_CAMERA_SENSOR_PROPERTIES_H__
>
> +#include <map>
>  #include <string>
>
>  #include <libcamera/geometry.h>
> @@ -17,6 +18,7 @@ struct CameraSensorProperties {
>  	static const CameraSensorProperties *get(const std::string &sensor);
>
>  	Size unitCellSize;
> +	std::map<int32_t, int32_t> testPatternModeMap;

I would expect that an uint8_t to uint8_t should do

>  };
>
>  } /* namespace libcamera */
> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp
> index 6ded31dc..89eaf9f8 100644
> --- a/src/libcamera/camera_sensor_properties.cpp
> +++ b/src/libcamera/camera_sensor_properties.cpp
> @@ -9,6 +9,8 @@
>
>  #include <map>
>
> +#include <libcamera/control_ids.h>
> +
>  #include "libcamera/internal/log.h"
>
>  /**
> @@ -44,15 +46,33 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties)
>   */
>  const CameraSensorProperties *CameraSensorProperties::get(const std::string &sensor)
>  {
> +

Unrelated blank line

>  	static const std::map<std::string, const CameraSensorProperties> sensorProps = {
>  		{ "imx219", {
>  			.unitCellSize = { 1120, 1120 },
> +			.testPatternModeMap = {
> +				{ 0, controls::draft::TestPatternModeOff },
> +				{ 1, controls::draft::TestPatternModeColorBars },
> +				{ 2, controls::draft::TestPatternModeSolidColor },
> +				{ 3, controls::draft::TestPatternModeColorBarsFadeToGray },
> +				{ 4, controls::draft::TestPatternModePn9 },
> +			},

This is from the imx219 driver

#define IMX219_TEST_PATTERN_DISABLE	0
#define IMX219_TEST_PATTERN_SOLID_COLOR	1
#define IMX219_TEST_PATTERN_COLOR_BARS	2
#define IMX219_TEST_PATTERN_GREY_COLOR	3
#define IMX219_TEST_PATTERN_PN9		4

Should you map 1 to controls::draft::TestPatternModeSolidColor and 2
to controls::draft::TestPatternModeColorBars ?

>  		} },
>  		{ "ov5670", {
>  			.unitCellSize = { 1120, 1120 },
> +			.testPatternModeMap = {
> +				{ 0, controls::draft::TestPatternModeOff },
> +				{ 1, controls::draft::TestPatternModeColorBars },
> +			},

The driver only defines:
#define OV5670_TEST_PATTERN_ENABLE	BIT(3)

does this map to ColorBars ?

> +

Unrelated blank line

>  		} },
>  		{ "ov13858", {
>  			.unitCellSize = { 1120, 1120 },
> +			.testPatternModeMap =  {
> +				{ 0, controls::draft::TestPatternModeOff },
> +				{ 1, controls::draft::TestPatternModeColorBars },
> +				{ 2, controls::draft::TestPatternModeColorBarsFadeToGray },

The driver only reports 1 mode:
#define OV13858_TEST_PATTERN_ENABLE	BIT(7)

Where does the second come from ?

Thanks
   j

> +			},
>  		} },
>  	};
>
> --
> 2.31.1.607.g51e8a6a459-goog
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel