| Message ID | 20210519075941.1337388-3-hiroh@chromium.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Hiro, On Wed, May 19, 2021 at 04:59:38PM +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 | 22 +++++++++++++++++++ > 2 files changed, 24 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; > }; > > } /* namespace libcamera */ > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > index 6ded31dc..3bf8e500 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,10 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties) > * > * \var CameraSensorProperties::unitCellSize > * \brief The physical size of a pixel, including pixel edges, in nanometers. > + * > + * \var CameraSensorProperties::testPatternModeMap > + * \brief The table from a v4l2 menu index for V4L2_CID_TEST_PATTERN to the > + * control value of libcamera test pattern mode. How about: 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,12 +53,28 @@ 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 }, > + }, > } }, This patch breaks compilation as it requires the .testPatternModeMap field to be defined for the ov5693 sensor too. I've now realized at the time that sensor was not in the database :) > }; > > -- > 2.31.1.751.gd2f1c929bd-goog >
Hi Jacopo, On Thu, May 27, 2021 at 6:08 AM Jacopo Mondi <jacopo@jmondi.org> wrote: > Hi Hiro, > > On Wed, May 19, 2021 at 04:59:38PM +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 | 22 +++++++++++++++++++ > > 2 files changed, 24 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; > > }; > > > > } /* namespace libcamera */ > > diff --git a/src/libcamera/camera_sensor_properties.cpp > b/src/libcamera/camera_sensor_properties.cpp > > index 6ded31dc..3bf8e500 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,10 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties) > > * > > * \var CameraSensorProperties::unitCellSize > > * \brief The physical size of a pixel, including pixel edges, in > nanometers. > > + * > > + * \var CameraSensorProperties::testPatternModeMap > > + * \brief The table from a v4l2 menu index for V4L2_CID_TEST_PATTERN to > the > > + * control value of libcamera test pattern mode. > > How about: > > 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,12 +53,28 @@ 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 }, > > + }, > > } }, > > This patch breaks compilation as it requires the .testPatternModeMap > field to be defined for the ov5693 sensor too. I've now realized at > the time that sensor was not in the database :) > > ov5693 is mainly used in MS surface tablets according to the commit message of the patch adding this. Probably because of that, the sensor driver for it is not upstream to linux kernel and it is impossible to construct this table for ov5693. I couldn't even test pattern modes supported by it. Jean, do you know anything about this? -Hiro > > }; > > > > -- > > 2.31.1.751.gd2f1c929bd-goog > > >
On Thu, May 27, 2021 at 3:27 PM Hirokazu Honda <hiroh@chromium.org> wrote: > > Hi Jacopo, > > On Thu, May 27, 2021 at 6:08 AM Jacopo Mondi <jacopo@jmondi.org> wrote: >> >> Hi Hiro, >> >> On Wed, May 19, 2021 at 04:59:38PM +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 | 22 +++++++++++++++++++ >> > 2 files changed, 24 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; >> > }; >> > >> > } /* namespace libcamera */ >> > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp >> > index 6ded31dc..3bf8e500 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,10 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties) >> > * >> > * \var CameraSensorProperties::unitCellSize >> > * \brief The physical size of a pixel, including pixel edges, in nanometers. >> > + * >> > + * \var CameraSensorProperties::testPatternModeMap >> > + * \brief The table from a v4l2 menu index for V4L2_CID_TEST_PATTERN to the >> > + * control value of libcamera test pattern mode. >> >> How about: >> >> 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,12 +53,28 @@ 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 }, >> > + }, >> > } }, >> >> This patch breaks compilation as it requires the .testPatternModeMap >> field to be defined for the ov5693 sensor too. I've now realized at >> the time that sensor was not in the database :) >> > > ov5693 is mainly used in MS surface tablets according to the commit message of the patch adding this. > Probably because of that, the sensor driver for it is not upstream to linux kernel and it is impossible to construct this table for ov5693. > I couldn't even test pattern modes supported by it. Hmm, was the patch applied accidentally? I indeed don't see the driver upstream. > > Jean, do you know anything about this? > > -Hiro >> >> > }; >> > >> > -- >> > 2.31.1.751.gd2f1c929bd-goog >> >
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 6ded31dc..3bf8e500 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,10 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties) * * \var CameraSensorProperties::unitCellSize * \brief The physical size of a pixel, including pixel edges, in nanometers. + * + * \var CameraSensorProperties::testPatternModeMap + * \brief The table from a v4l2 menu index for V4L2_CID_TEST_PATTERN to the + * control value of libcamera test pattern mode. */ /** @@ -47,12 +53,28 @@ 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 }, + }, } }, };
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 | 22 +++++++++++++++++++ 2 files changed, 24 insertions(+)