[libcamera-devel] [RFC PATCH 2/5] libcamera: V4L2Subdevice: Add getter/setter function for test pattern mode

[Jacopo Mondi]

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 ?

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