ipa: rkisp1: blc: Drop [[maybe_unused]] attribute

20540 diff mbox series

The context parameter of the BlackLevelCorrection::init() function is
used. Drop the [[maybe_unused]] attribute.

Fixes: 50c28e135100 ("ipa: rkisp1: blc: Query black levels from camera sensor helper")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
I'm starting to wonder if we should omit parameter names for parameters
that are never used, and use [[maybe_unused]] for parameters that are
conditionally unused, as recommended by
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-unused.
---
 src/ipa/rkisp1/algorithms/blc.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)


base-commit: 196abb8d1d6e0fe9d190315e72a85eb12d16a554

Hi Laurent,

Thank you for the patch.

On 03/07/24 11:01 pm, Laurent Pinchart wrote:
> The context parameter of the BlackLevelCorrection::init() function is
> used. Drop the [[maybe_unused]] attribute.
>
> Fixes: 50c28e135100 ("ipa: rkisp1: blc: Query black levels from camera sensor helper")
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
> I'm starting to wonder if we should omit parameter names for parameters
> that are never used, and use [[maybe_unused]] for parameters that are
> conditionally unused, as recommended by
> https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-unused.

ack
> ---
>   src/ipa/rkisp1/algorithms/blc.cpp | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp
> index 4b00034cc434..464a0e066dd4 100644
> --- a/src/ipa/rkisp1/algorithms/blc.cpp
> +++ b/src/ipa/rkisp1/algorithms/blc.cpp
> @@ -52,8 +52,7 @@ BlackLevelCorrection::BlackLevelCorrection()
>   /**
>    * \copydoc libcamera::ipa::Algorithm::init
>    */
> -int BlackLevelCorrection::init([[maybe_unused]] IPAContext &context,
> -			       const YamlObject &tuningData)
> +int BlackLevelCorrection::init(IPAContext &context, const YamlObject &tuningData)
>   {
>   	std::optional<int16_t> levelRed = tuningData["R"].get<int16_t>();
>   	std::optional<int16_t> levelGreenR = tuningData["Gr"].get<int16_t>();
>
> base-commit: 196abb8d1d6e0fe9d190315e72a85eb12d16a554

Hi Laurent,

Thank you for the patch.

On Wed, Jul 03, 2024 at 08:31:20PM +0300, Laurent Pinchart wrote:
> The context parameter of the BlackLevelCorrection::init() function is
> used. Drop the [[maybe_unused]] attribute.
> 
> Fixes: 50c28e135100 ("ipa: rkisp1: blc: Query black levels from camera sensor helper")
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Sorry, I should have checked that. Thanks for the fix.

Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> 

> ---
> I'm starting to wonder if we should omit parameter names for parameters
> that are never used, and use [[maybe_unused]] for parameters that are
> conditionally unused, as recommended by
> https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-unused.

That would make it more obvious when we reintroduce a param and would
remove most [[maybe_unused]]. Sounds great.

Best regards,
Stefan

> ---
>  src/ipa/rkisp1/algorithms/blc.cpp | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp
> index 4b00034cc434..464a0e066dd4 100644
> --- a/src/ipa/rkisp1/algorithms/blc.cpp
> +++ b/src/ipa/rkisp1/algorithms/blc.cpp
> @@ -52,8 +52,7 @@ BlackLevelCorrection::BlackLevelCorrection()
>  /**
>   * \copydoc libcamera::ipa::Algorithm::init
>   */
> -int BlackLevelCorrection::init([[maybe_unused]] IPAContext &context,
> -			       const YamlObject &tuningData)
> +int BlackLevelCorrection::init(IPAContext &context, const YamlObject &tuningData)
>  {
>  	std::optional<int16_t> levelRed = tuningData["R"].get<int16_t>();
>  	std::optional<int16_t> levelGreenR = tuningData["Gr"].get<int16_t>();
> 
> base-commit: 196abb8d1d6e0fe9d190315e72a85eb12d16a554
> -- 
> Regards,
> 
> Laurent Pinchart
>

On Wed, Jul 03, 2024 at 08:31:20PM +0300, Laurent Pinchart wrote:
> The context parameter of the BlackLevelCorrection::init() function is
> used. Drop the [[maybe_unused]] attribute.
> 
> Fixes: 50c28e135100 ("ipa: rkisp1: blc: Query black levels from camera sensor helper")
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

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

> ---
> I'm starting to wonder if we should omit parameter names for parameters
> that are never used, and use [[maybe_unused]] for parameters that are
> conditionally unused, as recommended by
> https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-unused.
> ---
>  src/ipa/rkisp1/algorithms/blc.cpp | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp
> index 4b00034cc434..464a0e066dd4 100644
> --- a/src/ipa/rkisp1/algorithms/blc.cpp
> +++ b/src/ipa/rkisp1/algorithms/blc.cpp
> @@ -52,8 +52,7 @@ BlackLevelCorrection::BlackLevelCorrection()
>  /**
>   * \copydoc libcamera::ipa::Algorithm::init
>   */
> -int BlackLevelCorrection::init([[maybe_unused]] IPAContext &context,
> -			       const YamlObject &tuningData)
> +int BlackLevelCorrection::init(IPAContext &context, const YamlObject &tuningData)
>  {
>  	std::optional<int16_t> levelRed = tuningData["R"].get<int16_t>();
>  	std::optional<int16_t> levelGreenR = tuningData["Gr"].get<int16_t>();
> 
> base-commit: 196abb8d1d6e0fe9d190315e72a85eb12d16a554