[libcamera-devel] [PATCH v9 1/3] libcamera: camera_sensor: Reference test pattern modes by enum type
Kieran Bingham
kieran.bingham at ideasonboard.com
Sat Dec 4 21:35:58 CET 2021
Hi Hiro,
Quoting Hirokazu Honda (2021-12-03 22:15:14)
> Hi Jacopo,
>
> On Thu, Dec 2, 2021 at 12:04 AM Jacopo Mondi <jacopo at jmondi.org> wrote:
> >
> > Hi Hiro,
> >
> > while running CTS with this series applied I have noticed that once
> > a test pattern is set in one of the test, the rest of the session is
> > run with a test mode applied. I noticed as the displayed images are
> > actually the sensor's test patterns. Running the Camera app after CTS
> > has completed displays test pattern modes.
> >
> > It's pretty obvious that the test pattern mode is reset to Off during
> > CameraSensor::init() which is called only at pipeline handler
> > initialization time, so if the library is not tore down the test
> > pattern mode is never reset.
> >
> > Could you check if the same happens on your side by running CTS please ?
> >
>
> Thanks. It is reproducible for me.
> I confirmed the issue is resolved if I call setTestPatternMode(off) in
> IPU3 pipeline handler configure().
> Do you think it is a correct fix?
Yes, I would think that configuring the testPatternMode as off directly
during configure() is appropriate and correct.
A test pattern should only then be applied if a request comes in with a
test pattern mode explicitly set.
--
Kieran
>
> Thanks,
> -Hiro
>
>
>
> > On Mon, Nov 29, 2021 at 05:34:22PM +0900, Hirokazu Honda wrote:
> > > The CameraSensor stores TestPatternModes as an int32_t. This prevents
> > > the compiler from verifying the usage against the defined enum types.
> > >
> > > Fix references to the TestPatternMode to store the value as the
> > > TestPatternModeEnum type which is defined by the control generator.
> > >
> > > Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> > > Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> > > Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > > include/libcamera/internal/camera_sensor.h | 10 +++++++---
> > > include/libcamera/internal/camera_sensor_properties.h | 3 ++-
> > > src/libcamera/camera_sensor.cpp | 4 ++--
> > > src/libcamera/pipeline/ipu3/ipu3.cpp | 7 ++++---
> > > 4 files changed, 15 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
> > > index 2facbf3c..310571ca 100644
> > > --- a/include/libcamera/internal/camera_sensor.h
> > > +++ b/include/libcamera/internal/camera_sensor.h
> > > @@ -14,8 +14,10 @@
> > > #include <libcamera/base/class.h>
> > > #include <libcamera/base/log.h>
> > >
> > > +#include <libcamera/control_ids.h>
> > > #include <libcamera/controls.h>
> > > #include <libcamera/geometry.h>
> > > +
> > > #include <libcamera/ipa/core_ipa_interface.h>
> > >
> > > #include "libcamera/internal/formats.h"
> > > @@ -40,7 +42,8 @@ public:
> > > const std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; }
> > > const std::vector<Size> sizes(unsigned int mbusCode) const;
> > > Size resolution() const;
> > > - const std::vector<int32_t> &testPatternModes() const
> > > + const std::vector<controls::draft::TestPatternModeEnum>
> > > + &testPatternModes() const
> > > {
> > > return testPatternModes_;
> > > }
> > > @@ -71,7 +74,8 @@ private:
> > > void initVimcDefaultProperties();
> > > void initStaticProperties();
> > > void initTestPatternModes(
> > > - const std::map<int32_t, int32_t> &testPatternModeMap);
> > > + const std::map<controls::draft::TestPatternModeEnum, int32_t>
> > > + &testPatternModeMap);
> > > int initProperties();
> > >
> > > const MediaEntity *entity_;
> > > @@ -84,7 +88,7 @@ private:
> > > V4L2Subdevice::Formats formats_;
> > > std::vector<unsigned int> mbusCodes_;
> > > std::vector<Size> sizes_;
> > > - std::vector<int32_t> testPatternModes_;
> > > + std::vector<controls::draft::TestPatternModeEnum> testPatternModes_;
> > >
> > > Size pixelArraySize_;
> > > Rectangle activeArea_;
> > > diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h
> > > index af381a12..1ee3cb99 100644
> > > --- a/include/libcamera/internal/camera_sensor_properties.h
> > > +++ b/include/libcamera/internal/camera_sensor_properties.h
> > > @@ -10,6 +10,7 @@
> > > #include <map>
> > > #include <string>
> > >
> > > +#include <libcamera/control_ids.h>
> > > #include <libcamera/geometry.h>
> > >
> > > namespace libcamera {
> > > @@ -18,7 +19,7 @@ struct CameraSensorProperties {
> > > static const CameraSensorProperties *get(const std::string &sensor);
> > >
> > > Size unitCellSize;
> > > - std::map<int32_t, int32_t> testPatternModes;
> > > + std::map<controls::draft::TestPatternModeEnum, int32_t> testPatternModes;
> > > };
> > >
> > > } /* namespace libcamera */
> > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> > > index 9fdb8c09..f0aa9f24 100644
> > > --- a/src/libcamera/camera_sensor.cpp
> > > +++ b/src/libcamera/camera_sensor.cpp
> > > @@ -311,7 +311,7 @@ void CameraSensor::initStaticProperties()
> > > }
> > >
> > > void CameraSensor::initTestPatternModes(
> > > - const std::map<int32_t, int32_t> &testPatternModes)
> > > + const std::map<controls::draft::TestPatternModeEnum, int32_t> &testPatternModes)
> > > {
> > > const auto &v4l2TestPattern = controls().find(V4L2_CID_TEST_PATTERN);
> > > if (v4l2TestPattern == controls().end()) {
> > > @@ -327,7 +327,7 @@ void CameraSensor::initTestPatternModes(
> > > * control index is supported in the below for loop that creates the
> > > * list of supported test patterns.
> > > */
> > > - std::map<int32_t, int32_t> indexToTestPatternMode;
> > > + std::map<int32_t, controls::draft::TestPatternModeEnum> indexToTestPatternMode;
> > > for (const auto &it : testPatternModes)
> > > indexToTestPatternMode[it.second] = it.first;
> > >
> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > index c65afdb2..25490dcf 100644
> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > @@ -982,13 +982,14 @@ int PipelineHandlerIPU3::updateControls(IPU3CameraData *data)
> > > return ret;
> > >
> > > ControlInfoMap::Map controls = IPU3Controls;
> > > - const std::vector<int32_t> &testPatternModes = sensor->testPatternModes();
> > > + const std::vector<controls::draft::TestPatternModeEnum>
> > > + &testPatternModes = sensor->testPatternModes();
> > > if (!testPatternModes.empty()) {
> > > std::vector<ControlValue> values;
> > > values.reserve(testPatternModes.size());
> > >
> > > - for (int32_t pattern : testPatternModes)
> > > - values.emplace_back(pattern);
> > > + for (auto pattern : testPatternModes)
> > > + values.emplace_back(static_cast<int32_t>(pattern));
> > >
> > > controls[&controls::draft::TestPatternMode] = ControlInfo(values);
> > > }
> > > --
> > > 2.34.0.rc2.393.gf8c9666880-goog
> > >
More information about the libcamera-devel
mailing list