[libcamera-devel] [PATCH v4 3/6] libcamera: CameraSensorProperties: Adds table of v4l2 index and test pattern
Hirokazu Honda
hiroh at chromium.org
Wed May 19 07:31:23 CEST 2021
Hi Jacopo, thanks for reviewing.
On Tue, May 18, 2021 at 9:21 PM Jacopo Mondi <jacopo at jmondi.org> wrote:
> Hi Hiro
>
> On Thu, May 06, 2021 at 04:54:46PM +0900, Hirokazu Honda wrote:
> > In V4L2 API, a driver returns an index (also with a name) to
> > represent a test pattern, but it is a driver specific what test
> > pattern is represented by the index. Therefore, this adds a
> > mapping table from the index and a test pattern into a static
> > configuration of a sensor.
>
> A bit convoluted... What about something along the lines of:
>
> 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 at chromium.org>
> > ---
> > .../internal/camera_sensor_properties.h | 2 ++
> > src/libcamera/camera_sensor_properties.cpp | 20 +++++++++++++++++++
> > 2 files changed, 22 insertions(+)
> >
> > diff --git a/include/libcamera/internal/camera_sensor_properties.h
> b/include/libcamera/internal/camera_sensor_properties.h
> > index f5e242cb..f52890b4 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<int32_t, int32_t> testPatternModeMap;
>
> I would expect that an uint8_t to uint8_t should do
>
>
It should be enough although I do so because control value and index value
are int32_t.
I don't have a strong opinion. Changed to uint8_t.
> > };
> >
> > } /* namespace libcamera */
> > diff --git a/src/libcamera/camera_sensor_properties.cpp
> b/src/libcamera/camera_sensor_properties.cpp
> > index 6ded31dc..89eaf9f8 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"
> >
> > /**
> > @@ -44,15 +46,33 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties)
> > */
> > const CameraSensorProperties *CameraSensorProperties::get(const
> std::string &sensor)
> > {
> > +
>
> Unrelated blank line
>
> > 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 },
> > + },
>
> This is from the imx219 driver
>
> #define IMX219_TEST_PATTERN_DISABLE 0
> #define IMX219_TEST_PATTERN_SOLID_COLOR 1
> #define IMX219_TEST_PATTERN_COLOR_BARS 2
> #define IMX219_TEST_PATTERN_GREY_COLOR 3
> #define IMX219_TEST_PATTERN_PN9 4
>
> Should you map 1 to controls::draft::TestPatternModeSolidColor and 2
> to controls::draft::TestPatternModeColorBars ?
>
>
Those values are for the imx219 register.
Interestingly, imx_219_test_pattern_menu[] and imx_219_test_pattern_val[]
are in different order from the values.
I think my map is correct. This is very confusing. IMO, the different order
among them doesn't make sense, which should be fixed.
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/kernel/next/drivers/media/i2c/imx219.c;l=395;drc=c90b4d70b1746f5a46fb7bad988731e604a44d4e
> > } },
> > { "ov5670", {
> > .unitCellSize = { 1120, 1120 },
> > + .testPatternModeMap = {
> > + { 0, controls::draft::TestPatternModeOff },
> > + { 1,
> controls::draft::TestPatternModeColorBars },
> > + },
>
> The driver only defines:
> #define OV5670_TEST_PATTERN_ENABLE BIT(3)
>
> does this map to ColorBars ?
>
>
The value is a value to notify the ov5670 sensor tha a test pattern mode is
enabled.
The array for th test pattern menu is
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/kernel/next/drivers/media/i2c/ov5670.c;l=1718;drc=eba08021e15076afc21b506e71e2f4e523f27f8c
.
I think my map is correct.
> > +
>
> Unrelated blank line
>
> > } },
> > { "ov13858", {
> > .unitCellSize = { 1120, 1120 },
> > + .testPatternModeMap = {
> > + { 0, controls::draft::TestPatternModeOff },
> > + { 1,
> controls::draft::TestPatternModeColorBars },
> > + { 2,
> controls::draft::TestPatternModeColorBarsFadeToGray },
>
> The driver only reports 1 mode:
> #define OV13858_TEST_PATTERN_ENABLE BIT(7)
>
> Where does the second come from ?
>
>
Same. The value is a value to notify the ov13858 sensor tha a test pattern
mode is enabled.
The array for the test pattern menu is
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/kernel/next/drivers/media/i2c/ov13858.c;l=929;drc=74c3ddd9887f60824891d2574a1689e8c13bf191
I think my map is correct.
Thanks,
-Hiro
> Thanks
> j
>
> > + },
> > } },
> > };
> >
> > --
> > 2.31.1.607.g51e8a6a459-goog
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210519/20fee93b/attachment-0001.htm>
More information about the libcamera-devel
mailing list