<div dir="ltr"><div dir="ltr">Hi Laurent,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jun 7, 2021 at 8:37 AM Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Hiro,<br>
<br>
Thank you for the patch.<br>
<br>
On Fri, May 28, 2021 at 12:05:28PM +0900, Hirokazu Honda wrote:<br>
> The V4L2 specification defines the sensor test pattern modes<br>
> through a menu control, where a numerical index is associated to<br>
> a string that describes the test pattern. The index-to-pattern<br>
> mapping is driver specific and requires a corresponding representation<br>
> in the library.<br>
> <br>
> Add to the static list of CameraSensorProperties a map of indexes to<br>
> libcamera::controls::TestPatternModes values to be able to map the<br>
> indexes returned by the driver to the corresponding test pattern mode.<br>
> <br>
> Signed-off-by: Hirokazu Honda <<a href="mailto:hiroh@chromium.org" target="_blank">hiroh@chromium.org</a>><br>
> ---<br>
> .../internal/camera_sensor_properties.h | 2 ++<br>
> src/libcamera/camera_sensor_properties.cpp | 32 +++++++++++++++++++<br>
> 2 files changed, 34 insertions(+)<br>
> <br>
> diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h<br>
> index f5e242cb..88ec7261 100644<br>
> --- a/include/libcamera/internal/camera_sensor_properties.h<br>
> +++ b/include/libcamera/internal/camera_sensor_properties.h<br>
> @@ -7,6 +7,7 @@<br>
> #ifndef __LIBCAMERA_SENSOR_CAMERA_SENSOR_PROPERTIES_H__<br>
> #define __LIBCAMERA_SENSOR_CAMERA_SENSOR_PROPERTIES_H__<br>
> <br>
> +#include <map><br>
> #include <string><br>
> <br>
> #include <libcamera/geometry.h><br>
> @@ -17,6 +18,7 @@ struct CameraSensorProperties {<br>
> static const CameraSensorProperties *get(const std::string &sensor);<br>
> <br>
> Size unitCellSize;<br>
> + std::map<uint8_t, uint8_t> testPatternModeMap;<br>
<br>
I'd use int32_t for both, given that's the type of both the TestPattern<br>
libcamera control and the V4L2 menu controls. The maps are small so it<br>
won't consume much memory, and I wouldn't be surprised if uint8_t was<br>
aligned to 32 bits anyway.<br>
<br>
> };<br>
> <br>
> } /* namespace libcamera */<br>
> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp<br>
> index 2a6e97f7..841564ff 100644<br>
> --- a/src/libcamera/camera_sensor_properties.cpp<br>
> +++ b/src/libcamera/camera_sensor_properties.cpp<br>
> @@ -9,6 +9,8 @@<br>
> <br>
> #include <map><br>
> <br>
> +#include <libcamera/control_ids.h><br>
> +<br>
> #include "libcamera/internal/log.h"<br>
> <br>
> /**<br>
> @@ -34,6 +36,11 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties)<br>
> *<br>
> * \var CameraSensorProperties::unitCellSize<br>
> * \brief The physical size of a pixel, including pixel edges, in nanometers.<br>
> + *<br>
> + * \var CameraSensorProperties::testPatternModeMap<br>
<br>
I'd name the field testPatternModes (or even testPatterns if you want to<br>
shorten it), up to you.<br>
<br>
> + * \brief Map that associates the indexes of the sensor test pattern modes as<br>
> + * returned by V4L2_CID_TEST_PATTERN with the corresponding TestPattern<br>
> + * control value<br>
> */<br>
> <br>
> /**<br>
> @@ -47,15 +54,40 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen<br>
> static const std::map<std::string, const CameraSensorProperties> sensorProps = {<br>
> { "imx219", {<br>
> .unitCellSize = { 1120, 1120 },<br>
> + .testPatternModeMap = {<br>
> + { 0, controls::draft::TestPatternModeOff },<br>
> + { 1, controls::draft::TestPatternModeColorBars },<br>
> + { 2, controls::draft::TestPatternModeSolidColor },<br>
> + { 3, controls::draft::TestPatternModeColorBarsFadeToGray },<br>
> + { 4, controls::draft::TestPatternModePn9 },<br>
> + },<br>
> } },<br>
> { "ov5670", {<br>
> .unitCellSize = { 1120, 1120 },<br>
> + .testPatternModeMap = {<br>
> + { 0, controls::draft::TestPatternModeOff },<br>
> + { 1, controls::draft::TestPatternModeColorBars },<br>
> + },<br>
> } },<br>
> { "ov13858", {<br>
> .unitCellSize = { 1120, 1120 },<br>
> + .testPatternModeMap = {<br>
> + { 0, controls::draft::TestPatternModeOff },<br>
> + { 1, controls::draft::TestPatternModeColorBars },<br>
> + { 2, controls::draft::TestPatternModeColorBarsFadeToGray },<br>
<br>
Does the OV13858 support the fade-to-grey pattern as described in patch<br>
1/6 ? I'm looking at the OV13850 datasheet (I haven't found a leaked<br>
version of the OV13858) and the fade-to-grey pattern doesn't have the<br>
quantized right half of the colour bars.<br>
<br></blockquote><div>Thanks. I dropped the fade-to-gray. In fact, our xml doesn't list it.</div><div><br></div><div>Regards,</div><div>-Hiro</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> + },<br>
> } },<br>
> { "ov5693", {<br>
> .unitCellSize = { 1400, 1400 },<br>
> + .testPatternModeMap = {<br>
> + { 0, controls::draft::TestPatternModeOff },<br>
> + { 2, controls::draft::TestPatternModeColorBars },<br>
> + /*<br>
> + * No correspondence test pattern mode for<br>
<br>
s/correspondence/corresponding/<br>
<br>
> + * 1: "Random data" and 3: "Colour Bars with<br>
> + * Rolling Bar".<br>
> + */<br>
> + },<br>
> } },<br>
> };<br>
> <br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>