| Message ID | 20211004065856.3512141-1-hiroh@chromium.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Hiro, Thank you for the patch. On Mon, Oct 04, 2021 at 03:58:56PM +0900, Hirokazu Honda wrote: > The key and value of the test pattern mode are originally the index of > v4l2 control and the corresponding test pattern mode control value. > This key and value are useful in the initialization for reporting > available test pattern modes. However, the map of the reversed key and > value is much more useful in applying a requested test pattern mode. > Reverses the key and value of the map as the initialization is one > time but the test pattern mode request will be multiple times. > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/camera_sensor.cpp | 15 ++++++- > src/libcamera/camera_sensor_properties.cpp | 52 +++++++++++----------- > 2 files changed, 39 insertions(+), 28 deletions(-) > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > index 87668509..e5844c04 100644 > --- a/src/libcamera/camera_sensor.cpp > +++ b/src/libcamera/camera_sensor.cpp > @@ -320,11 +320,22 @@ void CameraSensor::initTestPatternModes( > return; > } > > + /* > + * Create a map that associates the V4L2 control index to the test Tabs instead of space for indentation. > + * pattern mode by reversing the testPatternModes map provided by the > + * camera sensor properties. This makes it easier to verify if the > + * control index is supported in the below for loop that creates the > + * list of supported test patterns. > + */ > + std::map<int32_t, int32_t> indexToTestPatternMode; > + for (const auto& it : testPatternModes) s/& it/ &it/ I'll fix those two small issues and push the patch. > + indexToTestPatternMode[it.second] = it.first; On a side note, I wonder if we should add a function to utils.h for this. > + > for (const ControlValue &value : v4l2TestPattern->second.values()) { > const int32_t index = value.get<int32_t>(); > > - const auto it = testPatternModes.find(index); > - if (it == testPatternModes.end()) { > + const auto it = indexToTestPatternMode.find(index); > + if (it == indexToTestPatternMode.end()) { > LOG(CameraSensor, Debug) > << "Test pattern mode " << index << " ignored"; > continue; > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > index 39bb282d..48305ac4 100644 > --- a/src/libcamera/camera_sensor_properties.cpp > +++ b/src/libcamera/camera_sensor_properties.cpp > @@ -38,9 +38,9 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties) > * \brief The physical size of a pixel, including pixel edges, in nanometers. > * > * \var CameraSensorProperties::testPatternModes > - * \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 > + * \brief Map that associates the TestPattern control value with the indexes of > + * the corresponding sensor test pattern modes as returned by > + * V4L2_CID_TEST_PATTERN. > */ > > /** > @@ -55,11 +55,11 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { "hi846", { > .unitCellSize = { 1120, 1120 }, > .testPatternModes = { > - { 0, controls::draft::TestPatternModeOff }, > - { 1, controls::draft::TestPatternModeSolidColor }, > - { 2, controls::draft::TestPatternModeColorBars }, > - { 3, controls::draft::TestPatternModeColorBarsFadeToGray }, > - { 4, controls::draft::TestPatternModePn9 }, > + { controls::draft::TestPatternModeOff, 0 }, > + { controls::draft::TestPatternModeSolidColor, 1 }, > + { controls::draft::TestPatternModeColorBars, 2 }, > + { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, > + { controls::draft::TestPatternModePn9, 4 }, > /* > * No corresponding test pattern mode for: > * 5: "Gradient Horizontal" > @@ -73,21 +73,21 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { "imx219", { > .unitCellSize = { 1120, 1120 }, > .testPatternModes = { > - { 0, controls::draft::TestPatternModeOff }, > - { 1, controls::draft::TestPatternModeColorBars }, > - { 2, controls::draft::TestPatternModeSolidColor }, > - { 3, controls::draft::TestPatternModeColorBarsFadeToGray }, > - { 4, controls::draft::TestPatternModePn9 }, > + { controls::draft::TestPatternModeOff, 0 }, > + { controls::draft::TestPatternModeColorBars, 1 }, > + { controls::draft::TestPatternModeSolidColor, 2 }, > + { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, > + { controls::draft::TestPatternModePn9, 4 }, > }, > } }, > { "imx258", { > .unitCellSize = { 1120, 1120 }, > .testPatternModes = { > - { 0, controls::draft::TestPatternModeOff }, > - { 1, controls::draft::TestPatternModeSolidColor }, > - { 2, controls::draft::TestPatternModeColorBars }, > - { 3, controls::draft::TestPatternModeColorBarsFadeToGray }, > - { 4, controls::draft::TestPatternModePn9 }, > + { controls::draft::TestPatternModeOff, 0 }, > + { controls::draft::TestPatternModeSolidColor, 1 }, > + { controls::draft::TestPatternModeColorBars, 2 }, > + { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, > + { controls::draft::TestPatternModePn9, 4 }, > }, > } }, > { "ov5647", { > @@ -97,15 +97,15 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { "ov5670", { > .unitCellSize = { 1120, 1120 }, > .testPatternModes = { > - { 0, controls::draft::TestPatternModeOff }, > - { 1, controls::draft::TestPatternModeColorBars }, > + { controls::draft::TestPatternModeOff, 0 }, > + { controls::draft::TestPatternModeColorBars, 1 }, > }, > } }, > { "ov5693", { > .unitCellSize = { 1400, 1400 }, > .testPatternModes = { > - { 0, controls::draft::TestPatternModeOff }, > - { 2, controls::draft::TestPatternModeColorBars }, > + { controls::draft::TestPatternModeOff, 0 }, > + { controls::draft::TestPatternModeColorBars, 2 }, > /* > * No corresponding test pattern mode for > * 1: "Random data" and 3: "Colour Bars with > @@ -116,8 +116,8 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { "ov8865", { > .unitCellSize = { 1400, 1400 }, > .testPatternModes = { > - { 0, controls::draft::TestPatternModeOff }, > - { 2, controls::draft::TestPatternModeColorBars }, > + { controls::draft::TestPatternModeOff, 0 }, > + { controls::draft::TestPatternModeColorBars, 2 }, > /* > * No corresponding test pattern mode for: > * 1: "Random data" > @@ -130,8 +130,8 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > { "ov13858", { > .unitCellSize = { 1120, 1120 }, > .testPatternModes = { > - { 0, controls::draft::TestPatternModeOff }, > - { 1, controls::draft::TestPatternModeColorBars }, > + { controls::draft::TestPatternModeOff, 0 }, > + { controls::draft::TestPatternModeColorBars, 1 }, > }, > } }, > };
Hi Laurent, On Tue, Oct 5, 2021 at 6:48 AM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Hiro, > > Thank you for the patch. > > On Mon, Oct 04, 2021 at 03:58:56PM +0900, Hirokazu Honda wrote: > > The key and value of the test pattern mode are originally the index of > > v4l2 control and the corresponding test pattern mode control value. > > This key and value are useful in the initialization for reporting > > available test pattern modes. However, the map of the reversed key and > > value is much more useful in applying a requested test pattern mode. > > Reverses the key and value of the map as the initialization is one > > time but the test pattern mode request will be multiple times. > > > > Signed-off-by: Hirokazu Honda <hiroh@chromium.org> > > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/libcamera/camera_sensor.cpp | 15 ++++++- > > src/libcamera/camera_sensor_properties.cpp | 52 +++++++++++----------- > > 2 files changed, 39 insertions(+), 28 deletions(-) > > > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp > > index 87668509..e5844c04 100644 > > --- a/src/libcamera/camera_sensor.cpp > > +++ b/src/libcamera/camera_sensor.cpp > > @@ -320,11 +320,22 @@ void CameraSensor::initTestPatternModes( > > return; > > } > > > > + /* > > + * Create a map that associates the V4L2 control index to the test > > Tabs instead of space for indentation. > > > + * pattern mode by reversing the testPatternModes map provided by the > > + * camera sensor properties. This makes it easier to verify if the > > + * control index is supported in the below for loop that creates the > > + * list of supported test patterns. > > + */ > > + std::map<int32_t, int32_t> indexToTestPatternMode; > > + for (const auto& it : testPatternModes) > > s/& it/ &it/ > > I'll fix those two small issues and push the patch. > Thanks for pushing. > > + indexToTestPatternMode[it.second] = it.first; > > On a side note, I wonder if we should add a function to utils.h for > this. > I am not sure this is such a common pattern that it is worth adding to utils.h. Let's add once we have much code like this in the codebase. -Hiro > > + > > for (const ControlValue &value : v4l2TestPattern->second.values()) { > > const int32_t index = value.get<int32_t>(); > > > > - const auto it = testPatternModes.find(index); > > - if (it == testPatternModes.end()) { > > + const auto it = indexToTestPatternMode.find(index); > > + if (it == indexToTestPatternMode.end()) { > > LOG(CameraSensor, Debug) > > << "Test pattern mode " << index << " ignored"; > > continue; > > diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp > > index 39bb282d..48305ac4 100644 > > --- a/src/libcamera/camera_sensor_properties.cpp > > +++ b/src/libcamera/camera_sensor_properties.cpp > > @@ -38,9 +38,9 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties) > > * \brief The physical size of a pixel, including pixel edges, in nanometers. > > * > > * \var CameraSensorProperties::testPatternModes > > - * \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 > > + * \brief Map that associates the TestPattern control value with the indexes of > > + * the corresponding sensor test pattern modes as returned by > > + * V4L2_CID_TEST_PATTERN. > > */ > > > > /** > > @@ -55,11 +55,11 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > > { "hi846", { > > .unitCellSize = { 1120, 1120 }, > > .testPatternModes = { > > - { 0, controls::draft::TestPatternModeOff }, > > - { 1, controls::draft::TestPatternModeSolidColor }, > > - { 2, controls::draft::TestPatternModeColorBars }, > > - { 3, controls::draft::TestPatternModeColorBarsFadeToGray }, > > - { 4, controls::draft::TestPatternModePn9 }, > > + { controls::draft::TestPatternModeOff, 0 }, > > + { controls::draft::TestPatternModeSolidColor, 1 }, > > + { controls::draft::TestPatternModeColorBars, 2 }, > > + { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, > > + { controls::draft::TestPatternModePn9, 4 }, > > /* > > * No corresponding test pattern mode for: > > * 5: "Gradient Horizontal" > > @@ -73,21 +73,21 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > > { "imx219", { > > .unitCellSize = { 1120, 1120 }, > > .testPatternModes = { > > - { 0, controls::draft::TestPatternModeOff }, > > - { 1, controls::draft::TestPatternModeColorBars }, > > - { 2, controls::draft::TestPatternModeSolidColor }, > > - { 3, controls::draft::TestPatternModeColorBarsFadeToGray }, > > - { 4, controls::draft::TestPatternModePn9 }, > > + { controls::draft::TestPatternModeOff, 0 }, > > + { controls::draft::TestPatternModeColorBars, 1 }, > > + { controls::draft::TestPatternModeSolidColor, 2 }, > > + { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, > > + { controls::draft::TestPatternModePn9, 4 }, > > }, > > } }, > > { "imx258", { > > .unitCellSize = { 1120, 1120 }, > > .testPatternModes = { > > - { 0, controls::draft::TestPatternModeOff }, > > - { 1, controls::draft::TestPatternModeSolidColor }, > > - { 2, controls::draft::TestPatternModeColorBars }, > > - { 3, controls::draft::TestPatternModeColorBarsFadeToGray }, > > - { 4, controls::draft::TestPatternModePn9 }, > > + { controls::draft::TestPatternModeOff, 0 }, > > + { controls::draft::TestPatternModeSolidColor, 1 }, > > + { controls::draft::TestPatternModeColorBars, 2 }, > > + { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, > > + { controls::draft::TestPatternModePn9, 4 }, > > }, > > } }, > > { "ov5647", { > > @@ -97,15 +97,15 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > > { "ov5670", { > > .unitCellSize = { 1120, 1120 }, > > .testPatternModes = { > > - { 0, controls::draft::TestPatternModeOff }, > > - { 1, controls::draft::TestPatternModeColorBars }, > > + { controls::draft::TestPatternModeOff, 0 }, > > + { controls::draft::TestPatternModeColorBars, 1 }, > > }, > > } }, > > { "ov5693", { > > .unitCellSize = { 1400, 1400 }, > > .testPatternModes = { > > - { 0, controls::draft::TestPatternModeOff }, > > - { 2, controls::draft::TestPatternModeColorBars }, > > + { controls::draft::TestPatternModeOff, 0 }, > > + { controls::draft::TestPatternModeColorBars, 2 }, > > /* > > * No corresponding test pattern mode for > > * 1: "Random data" and 3: "Colour Bars with > > @@ -116,8 +116,8 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > > { "ov8865", { > > .unitCellSize = { 1400, 1400 }, > > .testPatternModes = { > > - { 0, controls::draft::TestPatternModeOff }, > > - { 2, controls::draft::TestPatternModeColorBars }, > > + { controls::draft::TestPatternModeOff, 0 }, > > + { controls::draft::TestPatternModeColorBars, 2 }, > > /* > > * No corresponding test pattern mode for: > > * 1: "Random data" > > @@ -130,8 +130,8 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen > > { "ov13858", { > > .unitCellSize = { 1120, 1120 }, > > .testPatternModes = { > > - { 0, controls::draft::TestPatternModeOff }, > > - { 1, controls::draft::TestPatternModeColorBars }, > > + { controls::draft::TestPatternModeOff, 0 }, > > + { controls::draft::TestPatternModeColorBars, 1 }, > > }, > > } }, > > }; > > -- > Regards, > > Laurent Pinchart
diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index 87668509..e5844c04 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -320,11 +320,22 @@ void CameraSensor::initTestPatternModes( return; } + /* + * Create a map that associates the V4L2 control index to the test + * pattern mode by reversing the testPatternModes map provided by the + * camera sensor properties. This makes it easier to verify if the + * control index is supported in the below for loop that creates the + * list of supported test patterns. + */ + std::map<int32_t, int32_t> indexToTestPatternMode; + for (const auto& it : testPatternModes) + indexToTestPatternMode[it.second] = it.first; + for (const ControlValue &value : v4l2TestPattern->second.values()) { const int32_t index = value.get<int32_t>(); - const auto it = testPatternModes.find(index); - if (it == testPatternModes.end()) { + const auto it = indexToTestPatternMode.find(index); + if (it == indexToTestPatternMode.end()) { LOG(CameraSensor, Debug) << "Test pattern mode " << index << " ignored"; continue; diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp index 39bb282d..48305ac4 100644 --- a/src/libcamera/camera_sensor_properties.cpp +++ b/src/libcamera/camera_sensor_properties.cpp @@ -38,9 +38,9 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties) * \brief The physical size of a pixel, including pixel edges, in nanometers. * * \var CameraSensorProperties::testPatternModes - * \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 + * \brief Map that associates the TestPattern control value with the indexes of + * the corresponding sensor test pattern modes as returned by + * V4L2_CID_TEST_PATTERN. */ /** @@ -55,11 +55,11 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen { "hi846", { .unitCellSize = { 1120, 1120 }, .testPatternModes = { - { 0, controls::draft::TestPatternModeOff }, - { 1, controls::draft::TestPatternModeSolidColor }, - { 2, controls::draft::TestPatternModeColorBars }, - { 3, controls::draft::TestPatternModeColorBarsFadeToGray }, - { 4, controls::draft::TestPatternModePn9 }, + { controls::draft::TestPatternModeOff, 0 }, + { controls::draft::TestPatternModeSolidColor, 1 }, + { controls::draft::TestPatternModeColorBars, 2 }, + { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, + { controls::draft::TestPatternModePn9, 4 }, /* * No corresponding test pattern mode for: * 5: "Gradient Horizontal" @@ -73,21 +73,21 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen { "imx219", { .unitCellSize = { 1120, 1120 }, .testPatternModes = { - { 0, controls::draft::TestPatternModeOff }, - { 1, controls::draft::TestPatternModeColorBars }, - { 2, controls::draft::TestPatternModeSolidColor }, - { 3, controls::draft::TestPatternModeColorBarsFadeToGray }, - { 4, controls::draft::TestPatternModePn9 }, + { controls::draft::TestPatternModeOff, 0 }, + { controls::draft::TestPatternModeColorBars, 1 }, + { controls::draft::TestPatternModeSolidColor, 2 }, + { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, + { controls::draft::TestPatternModePn9, 4 }, }, } }, { "imx258", { .unitCellSize = { 1120, 1120 }, .testPatternModes = { - { 0, controls::draft::TestPatternModeOff }, - { 1, controls::draft::TestPatternModeSolidColor }, - { 2, controls::draft::TestPatternModeColorBars }, - { 3, controls::draft::TestPatternModeColorBarsFadeToGray }, - { 4, controls::draft::TestPatternModePn9 }, + { controls::draft::TestPatternModeOff, 0 }, + { controls::draft::TestPatternModeSolidColor, 1 }, + { controls::draft::TestPatternModeColorBars, 2 }, + { controls::draft::TestPatternModeColorBarsFadeToGray, 3 }, + { controls::draft::TestPatternModePn9, 4 }, }, } }, { "ov5647", { @@ -97,15 +97,15 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen { "ov5670", { .unitCellSize = { 1120, 1120 }, .testPatternModes = { - { 0, controls::draft::TestPatternModeOff }, - { 1, controls::draft::TestPatternModeColorBars }, + { controls::draft::TestPatternModeOff, 0 }, + { controls::draft::TestPatternModeColorBars, 1 }, }, } }, { "ov5693", { .unitCellSize = { 1400, 1400 }, .testPatternModes = { - { 0, controls::draft::TestPatternModeOff }, - { 2, controls::draft::TestPatternModeColorBars }, + { controls::draft::TestPatternModeOff, 0 }, + { controls::draft::TestPatternModeColorBars, 2 }, /* * No corresponding test pattern mode for * 1: "Random data" and 3: "Colour Bars with @@ -116,8 +116,8 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen { "ov8865", { .unitCellSize = { 1400, 1400 }, .testPatternModes = { - { 0, controls::draft::TestPatternModeOff }, - { 2, controls::draft::TestPatternModeColorBars }, + { controls::draft::TestPatternModeOff, 0 }, + { controls::draft::TestPatternModeColorBars, 2 }, /* * No corresponding test pattern mode for: * 1: "Random data" @@ -130,8 +130,8 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen { "ov13858", { .unitCellSize = { 1120, 1120 }, .testPatternModes = { - { 0, controls::draft::TestPatternModeOff }, - { 1, controls::draft::TestPatternModeColorBars }, + { controls::draft::TestPatternModeOff, 0 }, + { controls::draft::TestPatternModeColorBars, 1 }, }, } }, };