[libcamera-devel] [PATCH v5 3/6] libcamera: CameraSensorProperties: Add table of v4l2 index and test pattern

[Tomasz Figa]

On Thu, May 27, 2021 at 3:27 PM Hirokazu Honda <hiroh at chromium.org> wrote:
>
> Hi Jacopo,
>
> On Thu, May 27, 2021 at 6:08 AM Jacopo Mondi <jacopo at 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 at 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
>> >