[libcamera-devel] [RFC PATCH 2/5] libcamera: V4L2Subdevice: Add getter/setter function for test pattern mode
Hirokazu Honda
hiroh at chromium.org
Mon Apr 12 15:12:07 CEST 2021
Hi Jacopo, thanks for reviewing.
On Fri, Apr 9, 2021 at 5:55 PM Jacopo Mondi <jacopo at jmondi.org> wrote:
>
> Hi Hiro,
>
> On Fri, Apr 09, 2021 at 01:32:05PM +0900, Hirokazu Honda wrote:
> > The supported test pattern modes can be obtained by calling
> > VIDIOC_QUERYMENU to a camera sensor device. This implements the
> > getter and setter functions for the test pattern mode in
> > V4L2SubDevice.
> >
> > Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> > ---
> > include/libcamera/internal/v4l2_subdevice.h | 5 +
> > src/libcamera/v4l2_subdevice.cpp | 104 ++++++++++++++++++++
> > 2 files changed, 109 insertions(+)
> >
> > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h
> > index d2b9ca55..f2f5d3f6 100644
> > --- a/include/libcamera/internal/v4l2_subdevice.h
> > +++ b/include/libcamera/internal/v4l2_subdevice.h
> > @@ -60,6 +60,9 @@ public:
> > int setFormat(unsigned int pad, V4L2SubdeviceFormat *format,
> > Whence whence = ActiveFormat);
> >
> > + std::vector<int32_t> getAvailableTestPatternModes();
> > + int setTestPatternMode(int32_t testPatternMode);
> > +
> > static std::unique_ptr<V4L2Subdevice>
> > fromEntityName(const MediaDevice *media, const std::string &entity);
> >
> > @@ -74,6 +77,8 @@ private:
> > unsigned int code);
> >
> > const MediaEntity *entity_;
> > +
> > + std::map<int32_t, uint32_t> testPatternModeMap_;
> > };
> >
> > } /* namespace libcamera */
> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > index 721ff5a9..130e9c4d 100644
> > --- a/src/libcamera/v4l2_subdevice.cpp
> > +++ b/src/libcamera/v4l2_subdevice.cpp
> > @@ -24,6 +24,7 @@
> > #include "libcamera/internal/media_object.h"
> > #include "libcamera/internal/utils.h"
> >
> > +#include "android/metadata/system/camera_metadata_tags.h"
>
> I'm afraid this is not correct. We don't want any android specific
> definition in libcamera core. libcamera run on non-android systems,
> and we don't want generic application to depend on anything Andoroid.
>
> > /**
> > * \file v4l2_subdevice.h
> > * \brief V4L2 Subdevice API
> > @@ -523,4 +524,107 @@ std::vector<SizeRange> V4L2Subdevice::enumPadSizes(unsigned int pad,
> > return sizes;
> > }
> >
> > +/**
> > + * \var V4L2Subdevice::testPatternModeMap_
> > + * \brief The map from controls::SensorTestPatternMode to an index value to be
> > + * used for V4L2_CID_TEST_PATTERN.
> > + *
> > + * The map is constructed in getAvailableTestPatternModes() and referred in
> > + * setTestPatternMode() to find a value associated with the requested test
> > + * pattern mode.
> > + */
> > +/**
> > + * \fn V4L2Subdevice::getAvailableTestPatternModes()
> > + * \brief Retrieve the available test pattern modes.
> > + *
> > + * \return The available control::SensorTestPatternMode values.
> > + */
> > +std::vector<int32_t> V4L2Subdevice::getAvailableTestPatternModes()
> > +{
> > + std::vector<int32_t> patternModes;
> > + if (!testPatternModeMap_.empty()) {
> > + for (const auto &mode : testPatternModeMap_)
> > + patternModes.push_back(mode.first);
> > + return patternModes;
> > + }
> > +
> > + v4l2_queryctrl ctrl;
> > + memset(&ctrl, 0, sizeof(ctrl));
> > + ctrl.id = V4L2_CID_TEST_PATTERN;
> > + int ret = ioctl(VIDIOC_QUERYCTRL, &ctrl);
>
> I'm not sure if you have considered the following and found any
> blocker, but:
>
> - Controls are enumerated in V4L2Device::listControls() with
> VIDIOC_QUERY_EXT_CTRL
> - Menu controls are accepted but not really handled there yet. I think
> you should modify listControls() to handle menu controls properly
> and store them as ControlInfo. ControlInfo currently support being
> constructed with a Span<> of values but as far as I can tell
> V4l2ControlInfo does not.
> - Once you have valid ControlInfo for the menu control, it should
> contain the V4L2 identifers for the menu entries
> - The pipeline handler should then populate the libcamera controls in
> Camera::controls_ by inspecting the V4L2 controls returned by the
> v4l2 subdevice as we do today in IPU3::listControls(). You should
> use the libcamera controls identifiers that you have defined in
> patch #1, not the android defined values
> - The camera HAL inspects the Camera::controls() to translate from
> libcamera defined controls to Android defined metadata
>
> Does this make sense to you ?
>
That sounds good to me.
However, do you think it makes more sense to add available test
pattern modes to CameraSensorDataBase?
The reason is, as you saw in this patch, there is no way of mapping to
V4L2_CID_TEST_PATTERN value to android one.
This is a problem in v4l2 api. An app has to know beforehand what test
pattern mode the name returned by a driver represents.
What do you think?
-Hiro
> Thanks
> j
>
> > + if (ret < 0) {
> > + LOG(V4L2, Error) << "Unable to get test pattern mode :"
> > + << strerror(-ret);
> > + return {};
> > + }
>
> > +
> > + v4l2_querymenu menu;
> > + memset(&menu, 0, sizeof(menu));
> > + menu.id = ctrl.id;
> > + for (menu.index = ctrl.minimum;
> > + static_cast<int>(menu.index) <= ctrl.maximum; menu.index++) {
> > + if (ioctl(VIDIOC_QUERYMENU, &menu) != 0) {
> > + continue;
> > + }
> > +
> > + const std::string modeName(reinterpret_cast<char *>(menu.name));
> > + std::optional<int32_t> androidTestPatternMode;
> > + /*
> > + * ov13858 and ov5670.
> > + * No corresponding value for "Vertical Color Bar Type 3" and
> > + * "Vertical Color Bar Type 4".
> > + */
> > + if (modeName == "Disabled") {
> > + androidTestPatternMode =
> > + ANDROID_SENSOR_TEST_PATTERN_MODE_OFF;
> > + } else if (modeName == "Vertical Color Bar Type 1") {
> > + androidTestPatternMode =
> > + ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS;
> > + } else if (modeName == "Vertical Color Bar Type 2") {
> > + androidTestPatternMode =
> > + ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS_FADE_TO_GRAY;
> > + }
> > +
> > + if (androidTestPatternMode) {
> > + testPatternModeMap_[*androidTestPatternMode] = menu.index;
> > + patternModes.push_back(*androidTestPatternMode);
> > + } else {
> > + LOG(V4L2, Debug) << "Skip test pattern mode: "
> > + << modeName;
> > + }
> > + }
> > +
> > + return patternModes;
> > +}
> > +
> > +/**
> > + * \fn V4L2Subdevice::getAvailableTestPatternModes()
> > + * \brief Set the test pattern mode.
> > + *
> > + * \return 0 on success or a negative error code otherwise.
> > + */
> > +int V4L2Subdevice::setTestPatternMode(int32_t testPatternMode)
> > +{
> > + auto it = testPatternModeMap_.find(testPatternMode);
> > + if (it != testPatternModeMap_.end()) {
> > + LOG(V4L2, Error) << "Unsupported test pattern mode: "
> > + << testPatternMode;
> > +
> > + return -EINVAL;
> > + }
> > +
> > + v4l2_control ctrl;
> > + memset(&ctrl, 0, sizeof(ctrl));
> > + ctrl.id = V4L2_CID_TEST_PATTERN;
> > + ctrl.value = it->second;
> > + int ret = ioctl(VIDIOC_S_CTRL, &ctrl);
> > + if (ret < 0) {
> > + LOG(V4L2, Error) << "Unable to set test pattern mode: "
> > + << strerror(-ret);
> > +
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > } /* namespace libcamera */
> > --
> > 2.31.1.295.g9ea45b61b8-goog
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
More information about the libcamera-devel
mailing list