[libcamera-devel] [RFC PATCH 2/5] libcamera: V4L2Subdevice: Add getter/setter function for test pattern mode
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Apr 12 21:45:35 CEST 2021
Hi Jacopo, Hiro,
On 12/04/2021 14:50, Jacopo Mondi wrote:
> Hi Hiro,
>
> On Mon, Apr 12, 2021 at 10:38:54PM +0900, Hirokazu Honda wrote:
>> Hi Jacopo,
>>
>> On Mon, Apr 12, 2021 at 10:31 PM Jacopo Mondi <jacopo at jmondi.org> wrote:
>>>
>>> Hi Hiro,
>>>
>
> [snip]
>
>>>>>> +/**
>>>>>> + * \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?
>>>
>>> Do you mean recording the V4L2 test pattern modes there ? What would we gain
>>> compared to fetching them from the kernel interface ?
>>>
>>> If you mean the Android test pattern mode then no, the sensor database
>>> is a core libcamera construct, it knows nothing about Android. One
>>> option for Android-specific values is instead the HAL configuration
>>> file.
>>>
>>
>> Hmm, so should we have our own test pattern mode definition a part of
>> which are equivalent to some useful Android test pattern modes?
>
> If you mean recording them in the HAL configuration file you can
> record the android values (or better, their numeric values). There is
> not need to have them defined as libcamera controls if we have no
> strict need to do so.
>
>> Then the definitions are converted to Android ones in HAL configuration?
>>
>
> That would happen if the Camera HAL has to convert from the libcamera
> controls to android ones. It really depends if libcamera wants a
> control for the test pattern modes.
Personally, I think test patterns are a feature of the camera, and if
available should be exposed (as a libcamera control).
Indeed the difficulty might be mapping the menu type options to specific
libcamera controls though in a generic way.
> CC-ed Laurent
>
>>>> 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.
>>>
>>> Ah do you mean that the test patter names are driver specific ? Good
>>> job V4L2! I see your point there. I won't be ashamed of having them in
>>> the HAL configuration file, or to perform a best-effort mapping the
>>> Camera HAL. Not sure what's best tbh
>>>
>>
>> Right. The available test pattern modes are not device specific, like location.
>
> Well, not really. Location has a set of values it can be assigned to.
> The test pattern modes, if I got you right are free formed text, which
> makes it very hard to translate them to a fixed set of values like the
> ones android defines.
>
>> So I think it is more natural to put the info to CameraSensorDatabase.
>>
>
> I still don't get what you would record in the sensor database, maybe
> I'm still asleep :)
Perhaps there is some mapping of custom menu items to libcamera controls
required:
> v4l2-ctl -d /dev/v4l-subdev0 --list-ctrls
> VIMC Controls
>
> test_pattern 0x00f0f000 (menu) : min=0 max=21 default=0 value=0
> v4l2-ctl -d /dev/v4l-subdev0 --list-ctrls-menus
> VIMC Controls
>
> test_pattern 0x00f0f000 (menu) : min=0 max=21 default=0 value=0
> 0: 75% Colorbar
> 1: 100% Colorbar
> 2: CSC Colorbar
> 3: Horizontal 100% Colorbar
> 4: 100% Color Squares
> 5: 100% Black
> 6: 100% White
> 7: 100% Red
> 8: 100% Green
> 9: 100% Blue
> 10: 16x16 Checkers
> 11: 2x2 Checkers
> 12: 1x1 Checkers
> 13: 2x2 Red/Green Checkers
> 14: 1x1 Red/Green Checkers
> 15: Alternating Hor Lines
> 16: Alternating Vert Lines
> 17: One Pixel Wide Cross
> 18: Two Pixels Wide Cross
> 19: Ten Pixels Wide Cross
> 20: Gray Ramp
> 21: Noise
Somehow we would have to map which of those is appropriate for a
specific Android test pattern?
But that should certainly be done in the android layer - not the
libcamera layer ...
This is not looking very easy to make generic :-(
> Could you provide an example ?
>
> Thanks
> j
>
>> -Hiro
>>
>>>>
>>>> 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
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list