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

Message ID 20210622023654.969162-1-hiroh@chromium.org
State Superseded
Headers show
Series
  • [libcamera-devel,RFC,v2,1/5] libcamera: camera_sensor: Reverse the key and value of test pattern mode map
Related show

Commit Message

Hirokazu Honda June 22, 2021, 2:36 a.m. UTC 
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@chromium.org>
---
 src/libcamera/camera_sensor.cpp            |  8 +++++--
 src/libcamera/camera_sensor_properties.cpp | 28 +++++++++++-----------
 2 files changed, 20 insertions(+), 16 deletions(-)

Comments

Jacopo Mondi June 22, 2021, 10:08 a.m. UTC | #1 
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@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@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
>
Laurent Pinchart June 22, 2021, 10:14 a.m. UTC | #2 
Hi Hiro,

Thank you for the patch.

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@chromium.org>

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

> ---
>  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;
> +
>  	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

Patch
diff mbox series 

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;
+
 	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