| Message ID | 20210528030531.189492-3-hiroh@chromium.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series |
|
| Related | show |
Hi Hiro, Thank you for the patch. On Fri, May 28, 2021 at 12:05:28PM +0900, Hirokazu Honda wrote: > 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@chromium.org> > --- > .../internal/camera_sensor_properties.h | 2 ++ > src/libcamera/camera_sensor_properties.cpp | 32 +++++++++++++++++++ > 2 files changed, 34 insertions(+) > > diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h > index f5e242cb..88ec7261 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<uint8_t, uint8_t> testPatternModeMap; I'd use int32_t for both, given that's the type of both the TestPattern libcamera control and the V4L2 menu controls. The maps are small so it won't consume much memory, and I wouldn't be surprised if uint8_t was aligned to 32 bits anyway. > }; > > } /* namespace libcamera */ > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > index 2a6e97f7..841564ff 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" > > /** > @@ -34,6 +36,11 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties) > * > * \var CameraSensorProperties::unitCellSize > * \brief The physical size of a pixel, including pixel edges, in nanometers. > + * > + * \var CameraSensorProperties::testPatternModeMap I'd name the field testPatternModes (or even testPatterns if you want to shorten it), up to you. > + * \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 > */ > > /** > @@ -47,15 +54,40 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > 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 }, > + }, > } }, > { "ov5670", { > .unitCellSize = { 1120, 1120 }, > + .testPatternModeMap = { > + { 0, controls::draft::TestPatternModeOff }, > + { 1, controls::draft::TestPatternModeColorBars }, > + }, > } }, > { "ov13858", { > .unitCellSize = { 1120, 1120 }, > + .testPatternModeMap = { > + { 0, controls::draft::TestPatternModeOff }, > + { 1, controls::draft::TestPatternModeColorBars }, > + { 2, controls::draft::TestPatternModeColorBarsFadeToGray }, Does the OV13858 support the fade-to-grey pattern as described in patch 1/6 ? I'm looking at the OV13850 datasheet (I haven't found a leaked version of the OV13858) and the fade-to-grey pattern doesn't have the quantized right half of the colour bars. > + }, > } }, > { "ov5693", { > .unitCellSize = { 1400, 1400 }, > + .testPatternModeMap = { > + { 0, controls::draft::TestPatternModeOff }, > + { 2, controls::draft::TestPatternModeColorBars }, > + /* > + * No correspondence test pattern mode for s/correspondence/corresponding/ > + * 1: "Random data" and 3: "Colour Bars with > + * Rolling Bar". > + */ > + }, > } }, > }; >
Hi Laurent, On Mon, Jun 7, 2021 at 8:37 AM Laurent Pinchart < laurent.pinchart@ideasonboard.com> wrote: > Hi Hiro, > > Thank you for the patch. > > On Fri, May 28, 2021 at 12:05:28PM +0900, Hirokazu Honda wrote: > > 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@chromium.org> > > --- > > .../internal/camera_sensor_properties.h | 2 ++ > > src/libcamera/camera_sensor_properties.cpp | 32 +++++++++++++++++++ > > 2 files changed, 34 insertions(+) > > > > diff --git a/include/libcamera/internal/camera_sensor_properties.h > b/include/libcamera/internal/camera_sensor_properties.h > > index f5e242cb..88ec7261 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<uint8_t, uint8_t> testPatternModeMap; > > I'd use int32_t for both, given that's the type of both the TestPattern > libcamera control and the V4L2 menu controls. The maps are small so it > won't consume much memory, and I wouldn't be surprised if uint8_t was > aligned to 32 bits anyway. > > > }; > > > > } /* namespace libcamera */ > > diff --git a/src/libcamera/camera_sensor_properties.cpp > b/src/libcamera/camera_sensor_properties.cpp > > index 2a6e97f7..841564ff 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" > > > > /** > > @@ -34,6 +36,11 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties) > > * > > * \var CameraSensorProperties::unitCellSize > > * \brief The physical size of a pixel, including pixel edges, in > nanometers. > > + * > > + * \var CameraSensorProperties::testPatternModeMap > > I'd name the field testPatternModes (or even testPatterns if you want to > shorten it), up to you. > > > + * \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 > > */ > > > > /** > > @@ -47,15 +54,40 @@ const CameraSensorProperties > *CameraSensorProperties::get(const std::string &sen > > 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 }, > > + }, > > } }, > > { "ov5670", { > > .unitCellSize = { 1120, 1120 }, > > + .testPatternModeMap = { > > + { 0, controls::draft::TestPatternModeOff }, > > + { 1, > controls::draft::TestPatternModeColorBars }, > > + }, > > } }, > > { "ov13858", { > > .unitCellSize = { 1120, 1120 }, > > + .testPatternModeMap = { > > + { 0, controls::draft::TestPatternModeOff }, > > + { 1, > controls::draft::TestPatternModeColorBars }, > > + { 2, > controls::draft::TestPatternModeColorBarsFadeToGray }, > > Does the OV13858 support the fade-to-grey pattern as described in patch > 1/6 ? I'm looking at the OV13850 datasheet (I haven't found a leaked > version of the OV13858) and the fade-to-grey pattern doesn't have the > quantized right half of the colour bars. > > Thanks. I dropped the fade-to-gray. In fact, our xml doesn't list it. Regards, -Hiro > > + }, > > } }, > > { "ov5693", { > > .unitCellSize = { 1400, 1400 }, > > + .testPatternModeMap = { > > + { 0, controls::draft::TestPatternModeOff }, > > + { 2, > controls::draft::TestPatternModeColorBars }, > > + /* > > + * No correspondence test pattern mode for > > s/correspondence/corresponding/ > > > + * 1: "Random data" and 3: "Colour Bars > with > > + * Rolling Bar". > > + */ > > + }, > > } }, > > }; > > > > -- > Regards, > > Laurent Pinchart >
diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h index f5e242cb..88ec7261 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<uint8_t, uint8_t> testPatternModeMap; }; } /* namespace libcamera */ diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp index 2a6e97f7..841564ff 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" /** @@ -34,6 +36,11 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties) * * \var CameraSensorProperties::unitCellSize * \brief The physical size of a pixel, including pixel edges, in nanometers. + * + * \var CameraSensorProperties::testPatternModeMap + * \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 */ /** @@ -47,15 +54,40 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen 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 }, + }, } }, { "ov5670", { .unitCellSize = { 1120, 1120 }, + .testPatternModeMap = { + { 0, controls::draft::TestPatternModeOff }, + { 1, controls::draft::TestPatternModeColorBars }, + }, } }, { "ov13858", { .unitCellSize = { 1120, 1120 }, + .testPatternModeMap = { + { 0, controls::draft::TestPatternModeOff }, + { 1, controls::draft::TestPatternModeColorBars }, + { 2, controls::draft::TestPatternModeColorBarsFadeToGray }, + }, } }, { "ov5693", { .unitCellSize = { 1400, 1400 }, + .testPatternModeMap = { + { 0, controls::draft::TestPatternModeOff }, + { 2, controls::draft::TestPatternModeColorBars }, + /* + * No correspondence test pattern mode for + * 1: "Random data" and 3: "Colour Bars with + * Rolling Bar". + */ + }, } }, };
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@chromium.org> --- .../internal/camera_sensor_properties.h | 2 ++ src/libcamera/camera_sensor_properties.cpp | 32 +++++++++++++++++++ 2 files changed, 34 insertions(+)