[libcamera-devel,v2,06/14] libcamera: controls: Add AeEnable

Message ID 20190829232653.13214-7-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • libcamera: ipa: Add basic IPA support
Related show

Commit Message

Niklas Söderlund Aug. 29, 2019, 11:26 p.m. UTC
Add a control to turn Auto Exposure on or off.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 include/libcamera/control_ids.h |  1 +
 src/libcamera/controls.cpp      | 15 +++++++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

Comments

Laurent Pinchart Sept. 4, 2019, 6:12 p.m. UTC | #1
Hi Niklas,

Thank you for the patch.

On Fri, Aug 30, 2019 at 01:26:45AM +0200, Niklas Söderlund wrote:
> Add a control to turn Auto Exposure on or off.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  include/libcamera/control_ids.h |  1 +
>  src/libcamera/controls.cpp      | 15 +++++++++++++--
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libcamera/control_ids.h b/include/libcamera/control_ids.h
> index 75b6a2d5cafeca72..8cd44e571f705ac5 100644
> --- a/include/libcamera/control_ids.h
> +++ b/include/libcamera/control_ids.h
> @@ -13,6 +13,7 @@
>  namespace libcamera {
>  
>  enum ControlId {
> +	AeEnable,
>  	AwbEnable,
>  	Brightness,
>  	Contrast,
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 727fdbd9450d2f40..35861e27401d5241 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -186,6 +186,13 @@ std::string ControlValue::toString() const
>   * \brief Numerical control ID
>   */
>  
> +/**
> + * \var AeEnable
> + * ControlType: Bool
> + *
> + * Enables or disables the AE. See also \a libcamera::ControlId::ManualExposure.

s/Enables or disables/Enable or disable/
s/AE/auto-exposure algorithm/ or s/the AE/auto-expose/

You should also use \sa instead of See also.

 * \sa ControlId::ManualExposure

> + */
> +
>  /**
>   * \var AwbEnable
>   * ControlType: Bool
> @@ -218,14 +225,18 @@ std::string ControlValue::toString() const
>   * \var ManualExposure
>   * ControlType: Integer
>   *
> - * Specify a fixed exposure time in milli-seconds
> + * Specify a fixed exposure time in milli-seconds.
> + *
> + * This control is only considered if AeEnable is not enabled.

What happens if AE is enabled ? Will the fixed exposure time be recorded
internally and applied when AE gets disabled ? Same for the gain below.

>   */
>  
>  /**
>   * \var ManualGain
>   * ControlType: Integer
>   *
> - * Specify a fixed gain parameter
> + * Specify a fixed gain parameter.
> + *
> + * This control is only considered if AeEnable is not enabled.

Should AE be renamed to auto exposure and gain ? Or should we have an
AGCEnable control ?

>   */
>  
>  /**

Patch

diff --git a/include/libcamera/control_ids.h b/include/libcamera/control_ids.h
index 75b6a2d5cafeca72..8cd44e571f705ac5 100644
--- a/include/libcamera/control_ids.h
+++ b/include/libcamera/control_ids.h
@@ -13,6 +13,7 @@ 
 namespace libcamera {
 
 enum ControlId {
+	AeEnable,
 	AwbEnable,
 	Brightness,
 	Contrast,
diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
index 727fdbd9450d2f40..35861e27401d5241 100644
--- a/src/libcamera/controls.cpp
+++ b/src/libcamera/controls.cpp
@@ -186,6 +186,13 @@  std::string ControlValue::toString() const
  * \brief Numerical control ID
  */
 
+/**
+ * \var AeEnable
+ * ControlType: Bool
+ *
+ * Enables or disables the AE. See also \a libcamera::ControlId::ManualExposure.
+ */
+
 /**
  * \var AwbEnable
  * ControlType: Bool
@@ -218,14 +225,18 @@  std::string ControlValue::toString() const
  * \var ManualExposure
  * ControlType: Integer
  *
- * Specify a fixed exposure time in milli-seconds
+ * Specify a fixed exposure time in milli-seconds.
+ *
+ * This control is only considered if AeEnable is not enabled.
  */
 
 /**
  * \var ManualGain
  * ControlType: Integer
  *
- * Specify a fixed gain parameter
+ * Specify a fixed gain parameter.
+ *
+ * This control is only considered if AeEnable is not enabled.
  */
 
 /**