[v3,08/19] ipa: libipa: agc: Remove unused accessors
diff mbox series

Message ID 20251114-exposure-limits-v3-8-b7c07feba026@ideasonboard.com
State New
Headers show
Series
  • libipa: agc: Calculate exposure limits
Related show

Commit Message

Jacopo Mondi Nov. 14, 2025, 2:17 p.m. UTC 
Remove unused accessors. They're not only unused but it is not even
clear what the purpose of accessing the fields is.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 src/ipa/libipa/exposure_mode_helper.cpp | 25 -------------------------
 src/ipa/libipa/exposure_mode_helper.h   |  5 -----
 2 files changed, 30 deletions(-)

Comments

Barnabás Pőcze Nov. 17, 2025, 12:45 p.m. UTC | #1 
2025. 11. 14. 15:17 keltezéssel, Jacopo Mondi írta:
> Remove unused accessors. They're not only unused but it is not even
> clear what the purpose of accessing the fields is.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>


>   src/ipa/libipa/exposure_mode_helper.cpp | 25 -------------------------
>   src/ipa/libipa/exposure_mode_helper.h   |  5 -----
>   2 files changed, 30 deletions(-)
> 
> diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp
> index 29e316d9d09181cd65cb08f26babb1a680bee62a..962ce1b1b7503e0f86f3857b484b249cb6383fde 100644
> --- a/src/ipa/libipa/exposure_mode_helper.cpp
> +++ b/src/ipa/libipa/exposure_mode_helper.cpp
> @@ -266,31 +266,6 @@ ExposureModeHelper::splitExposure(utils::Duration exposure) const
>   		 exposure / (exposureTime * gain * quantGain) };
>   }
>   
> -/**
> - * \fn ExposureModeHelper::minExposureTime()
> - * \brief Retrieve the configured minimum exposure time limit set through
> - * setLimits()
> - * \return The minExposureTime_ value
> - */
> -
> -/**
> - * \fn ExposureModeHelper::maxExposureTime()
> - * \brief Retrieve the configured maximum exposure time set through setLimits()
> - * \return The maxExposureTime_ value
> - */
> -
> -/**
> - * \fn ExposureModeHelper::minGain()
> - * \brief Retrieve the configured minimum gain set through setLimits()
> - * \return The minGain_ value
> - */
> -
> -/**
> - * \fn ExposureModeHelper::maxGain()
> - * \brief Retrieve the configured maximum gain set through setLimits()
> - * \return The maxGain_ value
> - */
> -
>   } /* namespace ipa */
>   
>   } /* namespace libcamera */
> diff --git a/src/ipa/libipa/exposure_mode_helper.h b/src/ipa/libipa/exposure_mode_helper.h
> index 968192ddc5af768ae0de58aca6c7230c7b3bd507..f8b7a4aa4800b59459f8fc80f502b83647547f51 100644
> --- a/src/ipa/libipa/exposure_mode_helper.h
> +++ b/src/ipa/libipa/exposure_mode_helper.h
> @@ -33,11 +33,6 @@ public:
>   	std::tuple<utils::Duration, double, double, double>
>   	splitExposure(utils::Duration exposure) const;
>   
> -	utils::Duration minExposureTime() const { return minExposureTime_; }
> -	utils::Duration maxExposureTime() const { return maxExposureTime_; }
> -	double minGain() const { return minGain_; }
> -	double maxGain() const { return maxGain_; }
> -
>   private:
>   	utils::Duration clampExposureTime(utils::Duration exposureTime,
>   					  double *quantizationGain = nullptr) const;
>
Laurent Pinchart Nov. 19, 2025, 4:32 a.m. UTC | #2 
On Fri, Nov 14, 2025 at 03:17:03PM +0100, Jacopo Mondi wrote:
> Remove unused accessors. They're not only unused but it is not even
> clear what the purpose of accessing the fields is.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/ipa/libipa/exposure_mode_helper.cpp | 25 -------------------------
>  src/ipa/libipa/exposure_mode_helper.h   |  5 -----
>  2 files changed, 30 deletions(-)
> 
> diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp
> index 29e316d9d09181cd65cb08f26babb1a680bee62a..962ce1b1b7503e0f86f3857b484b249cb6383fde 100644
> --- a/src/ipa/libipa/exposure_mode_helper.cpp
> +++ b/src/ipa/libipa/exposure_mode_helper.cpp
> @@ -266,31 +266,6 @@ ExposureModeHelper::splitExposure(utils::Duration exposure) const
>  		 exposure / (exposureTime * gain * quantGain) };
>  }
>  
> -/**
> - * \fn ExposureModeHelper::minExposureTime()
> - * \brief Retrieve the configured minimum exposure time limit set through
> - * setLimits()
> - * \return The minExposureTime_ value
> - */
> -
> -/**
> - * \fn ExposureModeHelper::maxExposureTime()
> - * \brief Retrieve the configured maximum exposure time set through setLimits()
> - * \return The maxExposureTime_ value
> - */
> -
> -/**
> - * \fn ExposureModeHelper::minGain()
> - * \brief Retrieve the configured minimum gain set through setLimits()
> - * \return The minGain_ value
> - */
> -
> -/**
> - * \fn ExposureModeHelper::maxGain()
> - * \brief Retrieve the configured maximum gain set through setLimits()
> - * \return The maxGain_ value
> - */
> -
>  } /* namespace ipa */
>  
>  } /* namespace libcamera */
> diff --git a/src/ipa/libipa/exposure_mode_helper.h b/src/ipa/libipa/exposure_mode_helper.h
> index 968192ddc5af768ae0de58aca6c7230c7b3bd507..f8b7a4aa4800b59459f8fc80f502b83647547f51 100644
> --- a/src/ipa/libipa/exposure_mode_helper.h
> +++ b/src/ipa/libipa/exposure_mode_helper.h
> @@ -33,11 +33,6 @@ public:
>  	std::tuple<utils::Duration, double, double, double>
>  	splitExposure(utils::Duration exposure) const;
>  
> -	utils::Duration minExposureTime() const { return minExposureTime_; }
> -	utils::Duration maxExposureTime() const { return maxExposureTime_; }
> -	double minGain() const { return minGain_; }
> -	double maxGain() const { return maxGain_; }
> -
>  private:
>  	utils::Duration clampExposureTime(utils::Duration exposureTime,
>  					  double *quantizationGain = nullptr) const;
Paul Elder Feb. 6, 2026, 3:30 p.m. UTC | #3 
Quoting Jacopo Mondi (2025-11-14 23:17:03)
> Remove unused accessors. They're not only unused but it is not even
> clear what the purpose of accessing the fields is.

This looks like something I wrote... :)

It was probably meant to be used exactly how you did earlier in this series,
except the helper functions are obviously more useful than using these manually
and in the exact same way.

> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/ipa/libipa/exposure_mode_helper.cpp | 25 -------------------------
>  src/ipa/libipa/exposure_mode_helper.h   |  5 -----
>  2 files changed, 30 deletions(-)
> 
> diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp
> index 29e316d9d09181cd65cb08f26babb1a680bee62a..962ce1b1b7503e0f86f3857b484b249cb6383fde 100644
> --- a/src/ipa/libipa/exposure_mode_helper.cpp
> +++ b/src/ipa/libipa/exposure_mode_helper.cpp
> @@ -266,31 +266,6 @@ ExposureModeHelper::splitExposure(utils::Duration exposure) const
>                  exposure / (exposureTime * gain * quantGain) };
>  }
>  
> -/**
> - * \fn ExposureModeHelper::minExposureTime()
> - * \brief Retrieve the configured minimum exposure time limit set through
> - * setLimits()
> - * \return The minExposureTime_ value
> - */
> -
> -/**
> - * \fn ExposureModeHelper::maxExposureTime()
> - * \brief Retrieve the configured maximum exposure time set through setLimits()
> - * \return The maxExposureTime_ value
> - */
> -
> -/**
> - * \fn ExposureModeHelper::minGain()
> - * \brief Retrieve the configured minimum gain set through setLimits()
> - * \return The minGain_ value
> - */
> -
> -/**
> - * \fn ExposureModeHelper::maxGain()
> - * \brief Retrieve the configured maximum gain set through setLimits()
> - * \return The maxGain_ value
> - */
> -
>  } /* namespace ipa */
>  
>  } /* namespace libcamera */
> diff --git a/src/ipa/libipa/exposure_mode_helper.h b/src/ipa/libipa/exposure_mode_helper.h
> index 968192ddc5af768ae0de58aca6c7230c7b3bd507..f8b7a4aa4800b59459f8fc80f502b83647547f51 100644
> --- a/src/ipa/libipa/exposure_mode_helper.h
> +++ b/src/ipa/libipa/exposure_mode_helper.h
> @@ -33,11 +33,6 @@ public:
>         std::tuple<utils::Duration, double, double, double>
>         splitExposure(utils::Duration exposure) const;
>  
> -       utils::Duration minExposureTime() const { return minExposureTime_; }
> -       utils::Duration maxExposureTime() const { return maxExposureTime_; }
> -       double minGain() const { return minGain_; }
> -       double maxGain() const { return maxGain_; }
> -
>  private:
>         utils::Duration clampExposureTime(utils::Duration exposureTime,
>                                           double *quantizationGain = nullptr) const;
> 
> -- 
> 2.51.1
>

Patch
diff mbox series 

diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp
index 29e316d9d09181cd65cb08f26babb1a680bee62a..962ce1b1b7503e0f86f3857b484b249cb6383fde 100644
--- a/src/ipa/libipa/exposure_mode_helper.cpp
+++ b/src/ipa/libipa/exposure_mode_helper.cpp
@@ -266,31 +266,6 @@  ExposureModeHelper::splitExposure(utils::Duration exposure) const
 		 exposure / (exposureTime * gain * quantGain) };
 }
 
-/**
- * \fn ExposureModeHelper::minExposureTime()
- * \brief Retrieve the configured minimum exposure time limit set through
- * setLimits()
- * \return The minExposureTime_ value
- */
-
-/**
- * \fn ExposureModeHelper::maxExposureTime()
- * \brief Retrieve the configured maximum exposure time set through setLimits()
- * \return The maxExposureTime_ value
- */
-
-/**
- * \fn ExposureModeHelper::minGain()
- * \brief Retrieve the configured minimum gain set through setLimits()
- * \return The minGain_ value
- */
-
-/**
- * \fn ExposureModeHelper::maxGain()
- * \brief Retrieve the configured maximum gain set through setLimits()
- * \return The maxGain_ value
- */
-
 } /* namespace ipa */
 
 } /* namespace libcamera */
diff --git a/src/ipa/libipa/exposure_mode_helper.h b/src/ipa/libipa/exposure_mode_helper.h
index 968192ddc5af768ae0de58aca6c7230c7b3bd507..f8b7a4aa4800b59459f8fc80f502b83647547f51 100644
--- a/src/ipa/libipa/exposure_mode_helper.h
+++ b/src/ipa/libipa/exposure_mode_helper.h
@@ -33,11 +33,6 @@  public:
 	std::tuple<utils::Duration, double, double, double>
 	splitExposure(utils::Duration exposure) const;
 
-	utils::Duration minExposureTime() const { return minExposureTime_; }
-	utils::Duration maxExposureTime() const { return maxExposureTime_; }
-	double minGain() const { return minGain_; }
-	double maxGain() const { return maxGain_; }
-
 private:
 	utils::Duration clampExposureTime(utils::Duration exposureTime,
 					  double *quantizationGain = nullptr) const;