Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions doc/releases/migration-guide-4.1.rst
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,16 @@ Stepper
Regulator
=========

Video
=====

* The :file:`include/zephyr/drivers/video-controls.h` got updated to have video controls IDs (CIDs)
matching the definitions in the Linux kernel file ``include/uapi/linux/v4l2-controls.h``.
In most cases, removing the category prefix is enough: ``VIDEO_CID_CAMERA_GAIN`` becomes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
In most cases, removing the category prefix is enough: ``VIDEO_CID_CAMERA_GAIN`` becomes
In most cases, removing the category prefix is enough, e.g. ``VIDEO_CID_CAMERA_GAIN`` becomes

``VIDEO_CID_GAIN``.
The new ``video-controls.h`` source now contains description of each control ID to help
disambiguating.

Bluetooth
*********

Expand Down
3 changes: 3 additions & 0 deletions doc/releases/release-notes-4.1.rst
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,9 @@ Drivers and Sensors

* Video

* Changed :file:`include/zephyr/drivers/video-controls.h` to have control IDs (CIDs) matching
those present in the Linux kernel.

* Watchdog

* Wi-Fi
Expand Down
16 changes: 8 additions & 8 deletions drivers/video/ov2640.c
Original file line number Diff line number Diff line change
Expand Up @@ -940,28 +940,28 @@ static int ov2640_set_ctrl(const struct device *dev,
case VIDEO_CID_VFLIP:
ret |= ov2640_set_vertical_flip(dev, (int)value);
break;
case VIDEO_CID_CAMERA_EXPOSURE:
case VIDEO_CID_EXPOSURE:
ret |= ov2640_set_exposure_ctrl(dev, (int)value);
break;
case VIDEO_CID_CAMERA_GAIN:
case VIDEO_CID_GAIN:
ret |= ov2640_set_gain_ctrl(dev, (int)value);
break;
case VIDEO_CID_CAMERA_BRIGHTNESS:
case VIDEO_CID_BRIGHTNESS:
ret |= ov2640_set_brightness(dev, (int)value);
break;
case VIDEO_CID_CAMERA_SATURATION:
case VIDEO_CID_SATURATION:
ret |= ov2640_set_saturation(dev, (int)value);
break;
case VIDEO_CID_CAMERA_WHITE_BAL:
case VIDEO_CID_WHITE_BALANCE_TEMPERATURE:
ret |= ov2640_set_white_bal(dev, (int)value);
break;
case VIDEO_CID_CAMERA_CONTRAST:
case VIDEO_CID_CONTRAST:
ret |= ov2640_set_contrast(dev, (int)value);
break;
case VIDEO_CID_CAMERA_TEST_PATTERN:
case VIDEO_CID_TEST_PATTERN:
ret |= ov2640_set_colorbar(dev, (int)value);
break;
case VIDEO_CID_CAMERA_QUALITY:
case VIDEO_CID_JPEG_COMPRESSION_QUALITY:
ret |= ov2640_set_quality(dev, (int)value);
break;
default:
Expand Down
12 changes: 6 additions & 6 deletions drivers/video/ov5640.c
Original file line number Diff line number Diff line change
Expand Up @@ -844,17 +844,17 @@ static int ov5640_set_ctrl_power_line_freq(const struct device *dev, int value)
static int ov5640_set_ctrl(const struct device *dev, unsigned int cid, void *value)
{
switch (cid) {
case VIDEO_CID_CAMERA_TEST_PATTERN:
case VIDEO_CID_TEST_PATTERN:
return ov5640_set_ctrl_test_pattern(dev, (int)value);
case VIDEO_CID_CAMERA_HUE:
case VIDEO_CID_HUE:
return ov5640_set_ctrl_hue(dev, (int)value);
case VIDEO_CID_CAMERA_SATURATION:
case VIDEO_CID_SATURATION:
return ov5640_set_ctrl_saturation(dev, (int)(value));
case VIDEO_CID_CAMERA_BRIGHTNESS:
case VIDEO_CID_BRIGHTNESS:
return ov5640_set_ctrl_brightness(dev, (int)(value));
case VIDEO_CID_CAMERA_CONTRAST:
case VIDEO_CID_CONTRAST:
return ov5640_set_ctrl_contrast(dev, (int)value);
case VIDEO_CID_CAMERA_GAIN:
case VIDEO_CID_GAIN:
return ov5640_set_ctrl_gain(dev, (int)(value));
case VIDEO_CID_HFLIP:
return ov5640_set_ctrl_hflip(dev, (int)(value));
Expand Down
165 changes: 120 additions & 45 deletions include/zephyr/drivers/video-controls.h
Original file line number Diff line number Diff line change
@@ -1,85 +1,162 @@
/**
* @file
*
* @brief Public APIs for Video.
*/

/*
* Copyright (c) 2019 Linaro Limited.
* Copyright (c) 2024 tinyVision.ai Inc.
*
* SPDX-License-Identifier: Apache-2.0
*/
#ifndef ZEPHYR_INCLUDE_VIDEO_CONTROLS_H_
#define ZEPHYR_INCLUDE_VIDEO_CONTROLS_H_

/**
* @file
*
* @brief Public APIs for Video.
*/

/**
* @brief Video controls
* @defgroup video_controls Video Controls
* @ingroup io_interfaces
*
* The Video control IDs (CIDs) are introduced with the same name as
* Linux V4L2 subsystem and under the same class. This facilitates
* inter-operability and debugging devices end-to-end across Linux and
* Zephyr.
*
* This list is maintained compatible to the Linux kernel definitions in
* @c linux/include/uapi/linux/v4l2-controls.h
*
* @{
*/

#include <zephyr/device.h>
#include <stddef.h>
#include <zephyr/kernel.h>

#include <zephyr/types.h>

#ifdef __cplusplus
extern "C" {
#endif

/**
* @name Control classes
* @name Base class control IDs
* @{
*/
#define VIDEO_CTRL_CLASS_GENERIC 0x00000000 /**< Generic class controls */
#define VIDEO_CTRL_CLASS_CAMERA 0x00010000 /**< Camera class controls */
#define VIDEO_CTRL_CLASS_MPEG 0x00020000 /**< MPEG-compression controls */
#define VIDEO_CTRL_CLASS_JPEG 0x00030000 /**< JPEG-compression controls */
#define VIDEO_CTRL_CLASS_VENDOR 0xFFFF0000 /**< Vendor-specific class controls */
#define VIDEO_CID_BASE 0x00980900

/** Amount of perceived light of the image, the luma (Y') value. */
#define VIDEO_CID_BRIGHTNESS (VIDEO_CID_BASE + 0)

/** Amount of difference between the bright colors and dark colors. */
#define VIDEO_CID_CONTRAST (VIDEO_CID_BASE + 1)

/** Colorfulness of the image while preserving its brightness */
#define VIDEO_CID_SATURATION (VIDEO_CID_BASE + 2)

/** Shift in the tint of every colors, clockwise in a RGB color wheel */
#define VIDEO_CID_HUE (VIDEO_CID_BASE + 3)

/** Amount of time an image sensor is exposed to light, affecting the brightness */
#define VIDEO_CID_EXPOSURE (VIDEO_CID_BASE + 17)
Comment on lines +42 to +55
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the effort to make sure all public API is documented but FWIW I find these oddly worded for some reason. I guess that's what happens when the names are already mostly self-explanatory and one doesn't want to literally paraphrase them in the comment. Not blocking though :)

Copy link
Contributor

@ngphibang ngphibang Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I have same thoughts too and sometimes, the explanation is not totally correct as it is difficult to explain in short words, so it is better to not explain, developers who actually use these CID need to understand them, I think :-). But it's not a blocking point.

Copy link
Contributor Author

@josuah josuah Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading them again, these do not help understanding what the control do.

Better not risk defining them slightly differently than Linux ones.
If there need to be any doc effort, this can go in Linux.

Frequent issue of newbies: they reinvent the wheel.

New PR to address these small changes. Thank you for the feedback . :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think it's a good idea but I am not sure if it's OK in Zephyr to refer to documentation of a different project.


/** Amount of amplification performed to each pixel electrical signal, affecting the brightness */
#define VIDEO_CID_GAIN (VIDEO_CID_BASE + 19)

/** Flip the image horizontally: the left side becomes the right side */
#define VIDEO_CID_HFLIP (VIDEO_CID_BASE + 20)

/** Flip the image vertically: the top side becomes the bottom side */
#define VIDEO_CID_VFLIP (VIDEO_CID_BASE + 21)

/** Frequency of the power line to compensate for, avoiding flicker due to artificial lighting */
#define VIDEO_CID_POWER_LINE_FREQUENCY (VIDEO_CID_BASE + 24)
enum video_power_line_frequency {
VIDEO_CID_POWER_LINE_FREQUENCY_DISABLED = 0,
VIDEO_CID_POWER_LINE_FREQUENCY_50HZ = 1,
VIDEO_CID_POWER_LINE_FREQUENCY_60HZ = 2,
VIDEO_CID_POWER_LINE_FREQUENCY_AUTO = 3,
};

/** Balance of colors in direction of blue (cold) or red (warm) */
#define VIDEO_CID_WHITE_BALANCE_TEMPERATURE (VIDEO_CID_BASE + 26)

/**
* @}
*/

/**
* @name Stateful codec controls IDs
* @{
*/
#define VIDEO_CID_CODEC_CLASS_BASE 0x00990900

/**
* @}
*/

/**
* @name Generic class control IDs
* @name Camera class controls IDs
* @{
*/
/** Mirror the picture horizontally */
#define VIDEO_CID_HFLIP (VIDEO_CTRL_CLASS_GENERIC + 0)
/** Mirror the picture vertically */
#define VIDEO_CID_VFLIP (VIDEO_CTRL_CLASS_GENERIC + 1)
/** Power line frequency (enum) filter to avoid flicker */
#define VIDEO_CID_POWER_LINE_FREQUENCY (VIDEO_CTRL_CLASS_GENERIC + 2)
#define VIDEO_CID_CAMERA_CLASS_BASE 0x009a0900

/** Amount of optical zoom applied through to the camera optics */
#define VIDEO_CID_ZOOM_ABSOLUTE (VIDEO_CID_CAMERA_CLASS_BASE + 13)

/**
* @}
*/

/**
* @name Camera Flash class control IDs
* @{
*/
#define VIDEO_CID_FLASH_CLASS_BASE 0x009c0900

/**
* @}
*/

/**
* @name JPEG class control IDs
* @{
*/
#define VIDEO_CID_JPEG_CLASS_BASE 0x009d0900

/** Quality (Q) factor of the JPEG algorithm, also increasing the data size */
#define VIDEO_CID_JPEG_COMPRESSION_QUALITY (VIDEO_CID_JPEG_CLASS_BASE + 3)

/**
* @}
*/

/**
* @name Image Source class control IDs
* @{
*/
#define VIDEO_CID_IMAGE_SOURCE_CLASS_BASE 0x009e0900

/**
* @}
*/

/**
* @name Image Processing class control IDs
* @{
*/
#define VIDEO_CID_IMAGE_PROC_CLASS_BASE 0x009f0900

/** Pixel rate (pixels/second) in the device's pixel array. This control is read-only. */
#define VIDEO_CID_PIXEL_RATE (VIDEO_CTRL_CLASS_GENERIC + 3)
#define VIDEO_CID_PIXEL_RATE (VIDEO_CID_IMAGE_PROC_CLASS_BASE + 2)

enum video_power_line_frequency {
VIDEO_CID_POWER_LINE_FREQUENCY_DISABLED = 0,
VIDEO_CID_POWER_LINE_FREQUENCY_50HZ = 1,
VIDEO_CID_POWER_LINE_FREQUENCY_60HZ = 2,
VIDEO_CID_POWER_LINE_FREQUENCY_AUTO = 3,
};
/** Selection of the type of test pattern to represent */
#define VIDEO_CID_TEST_PATTERN (VIDEO_CID_IMAGE_PROC_CLASS_BASE + 3)

/**
* @}
*/

/**
* @name Camera class control IDs
* @name Vendor-specific class control IDs
* @{
*/
#define VIDEO_CID_CAMERA_EXPOSURE (VIDEO_CTRL_CLASS_CAMERA + 0)
#define VIDEO_CID_CAMERA_GAIN (VIDEO_CTRL_CLASS_CAMERA + 1)
#define VIDEO_CID_CAMERA_ZOOM (VIDEO_CTRL_CLASS_CAMERA + 2)
#define VIDEO_CID_CAMERA_BRIGHTNESS (VIDEO_CTRL_CLASS_CAMERA + 3)
#define VIDEO_CID_CAMERA_SATURATION (VIDEO_CTRL_CLASS_CAMERA + 4)
#define VIDEO_CID_CAMERA_WHITE_BAL (VIDEO_CTRL_CLASS_CAMERA + 5)
#define VIDEO_CID_CAMERA_CONTRAST (VIDEO_CTRL_CLASS_CAMERA + 6)
#define VIDEO_CID_CAMERA_TEST_PATTERN (VIDEO_CTRL_CLASS_CAMERA + 7)
#define VIDEO_CID_CAMERA_QUALITY (VIDEO_CTRL_CLASS_CAMERA + 8)
#define VIDEO_CID_CAMERA_HUE (VIDEO_CTRL_CLASS_CAMERA + 9)
#define VIDEO_CID_PRIVATE_BASE 0x08000000

/**
* @}
*/
Expand All @@ -88,8 +165,6 @@ enum video_power_line_frequency {
}
#endif

/* Controls */

/**
* @}
*/
Expand Down
2 changes: 1 addition & 1 deletion samples/drivers/video/capture/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ int main(void)
}

#ifdef CONFIG_TEST
video_set_ctrl(video_dev, VIDEO_CID_CAMERA_TEST_PATTERN, (void *)1);
video_set_ctrl(video_dev, VIDEO_CID_TEST_PATTERN, (void *)1);
#endif

#if DT_HAS_CHOSEN(zephyr_display)
Expand Down
Loading