[libcamera-devel] [RFC PATCH v2 1/5] libcamera: camera_sensor: Reverse the key and value of test pattern mode map

[Jacopo Mondi]

Hi Hiro,

On Tue, Jun 22, 2021 at 11:36:50AM +0900, Hirokazu Honda wrote:
> The key and value of the test pattern mode are originally the index of
> v4l2 control and the corresponding test pattern mode control value.
> This key and value are useful in the initialization for reporting
> available test pattern modes. However, the map of the reversed key and
> value is much more useful in applying a requested test pattern mode.
> Reverses the key and value of the map as the initialization is one
> time but the test pattern mode request will be multiple times.
>
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> ---
>  src/libcamera/camera_sensor.cpp            |  8 +++++--
>  src/libcamera/camera_sensor_properties.cpp | 28 +++++++++++-----------
>  2 files changed, 20 insertions(+), 16 deletions(-)
>
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index 3e135353..70bbd97a 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -319,11 +319,15 @@ void CameraSensor::initTestPatternModes(
>  		return;
>  	}
>
> +	std::map<int32_t, int32_t> indexToTestPatternMode;
> +	for (const auto& it : testPatternModes)
> +		indexToTestPatternMode[it.second] = it.first;
> +

At the expense of this additional vector, it is probably more
efficient to create it here instead of walking the testPatternModes
map looking for a value matching the index in all the iterations of
the below for loop like I would have done.

However, is this worth a comment ?

        /*
         * Create a map that associates the V4L2 control index to the
         * test pattern mode by reversing the testPatternModes map
         * provided by the camera sensor properties. This makes it
         * easier to verify if the control index is supported in the below
         * for loop that creates the list of supported test patterns.
         */

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

>  	for (const ControlValue &value : v4l2TestPattern->second.values()) {
>  		const int32_t index = value.get<int32_t>();
>
> -		const auto it = testPatternModes.find(index);
> -		if (it == testPatternModes.end()) {
> +		const auto it = indexToTestPatternMode.find(index);
> +		if (it == indexToTestPatternMode.end()) {
>  			LOG(CameraSensor, Debug)
>  				<< "Test pattern mode " << index << " ignored";
>  			continue;
> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp
> index f660743a..d0b2ae0e 100644
> --- a/src/libcamera/camera_sensor_properties.cpp
> +++ b/src/libcamera/camera_sensor_properties.cpp
> @@ -38,9 +38,9 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties)
>   * \brief The physical size of a pixel, including pixel edges, in nanometers.
>   *
>   * \var CameraSensorProperties::testPatternModes
> - * \brief Map that associates the indexes of the sensor test pattern modes as
> - * returned by V4L2_CID_TEST_PATTERN with the corresponding TestPattern
> - * control value
> + * \brief Map that associates the TestPattern control value with the indexes of
> + * the corresponding sensor test pattern modes as returned by
> + * V4L2_CID_TEST_PATTERN.
>   */
>
>  /**
> @@ -55,11 +55,11 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  		{ "imx219", {
>  			.unitCellSize = { 1120, 1120 },
>  			.testPatternModes = {
> -				{ 0, controls::draft::TestPatternModeOff },
> -				{ 1, controls::draft::TestPatternModeColorBars },
> -				{ 2, controls::draft::TestPatternModeSolidColor },
> -				{ 3, controls::draft::TestPatternModeColorBarsFadeToGray },
> -				{ 4, controls::draft::TestPatternModePn9 },
> +				{ controls::draft::TestPatternModeOff, 0 },
> +				{ controls::draft::TestPatternModeColorBars, 1 },
> +				{ controls::draft::TestPatternModeSolidColor, 2 },
> +				{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },
> +				{ controls::draft::TestPatternModePn9, 4 },
>  			},
>  		} },
>  		{ "imx258", {
> @@ -70,22 +70,22 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
>  		{ "ov5670", {
>  			.unitCellSize = { 1120, 1120 },
>  			.testPatternModes = {
> -				{ 0, controls::draft::TestPatternModeOff },
> -				{ 1, controls::draft::TestPatternModeColorBars },
> +				{ controls::draft::TestPatternModeOff, 0 },
> +				{ controls::draft::TestPatternModeColorBars, 1 },
>  			},
>  		} },
>  		{ "ov13858", {
>  			.unitCellSize = { 1120, 1120 },
>  			.testPatternModes =  {
> -				{ 0, controls::draft::TestPatternModeOff },
> -				{ 1, controls::draft::TestPatternModeColorBars },
> +				{ controls::draft::TestPatternModeOff, 0 },
> +				{ controls::draft::TestPatternModeColorBars, 1 },
>  			},
>  		} },
>  		{ "ov5693", {
>  			.unitCellSize = { 1400, 1400 },
>  			.testPatternModes = {
> -				{ 0, controls::draft::TestPatternModeOff },
> -				{ 2, controls::draft::TestPatternModeColorBars },
> +				{ controls::draft::TestPatternModeOff, 0 },
> +				{ controls::draft::TestPatternModeColorBars, 2 },
>  				/*
>  				 * No corresponding test pattern mode for
>  				 * 1: "Random data" and 3: "Colour Bars with
> --
> 2.32.0.288.g62a8d224e6-goog
>