[libcamera-devel,1/3] libcamera: ipa_manager: Fix IPA module min/max version check
diff mbox series

Message ID 20210711231547.19664-2-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: ipa_manager: Fix, cleanup, and allow forcing isolation
Related show

Commit Message

Laurent Pinchart July 11, 2021, 11:15 p.m. UTC
The IPAManager::createIPA() function has its minVersion and maxVersion
parameters inverted. This doesn't cause any issue at the moment as both
the minimum and maximum version are set to 1 by all callers, but it's
still a bug. Fix it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/internal/ipa_manager.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Kieran Bingham July 12, 2021, 7:47 a.m. UTC | #1
Hi Laurent,

On 12/07/2021 00:15, Laurent Pinchart wrote:
> The IPAManager::createIPA() function has its minVersion and maxVersion
> parameters inverted. This doesn't cause any issue at the moment as both
> the minimum and maximum version are set to 1 by all callers, but it's
> still a bug. Fix it.


Certainly seems more logical to have min,max, rather than max,min!

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/libcamera/internal/ipa_manager.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h
> index 34224e330da7..42201839901b 100644
> --- a/include/libcamera/internal/ipa_manager.h
> +++ b/include/libcamera/internal/ipa_manager.h
> @@ -31,8 +31,8 @@ public:
>  
>  	template<typename T>
>  	static std::unique_ptr<T> createIPA(PipelineHandler *pipe,
> -					    uint32_t maxVersion,
> -					    uint32_t minVersion)
> +					    uint32_t minVersion,
> +					    uint32_t maxVersion)
>  	{
>  		IPAModule *m = nullptr;
>  
>
Paul Elder July 19, 2021, 3:15 a.m. UTC | #2
Hi Laurent,

On Mon, Jul 12, 2021 at 02:15:45AM +0300, Laurent Pinchart wrote:
> The IPAManager::createIPA() function has its minVersion and maxVersion
> parameters inverted. This doesn't cause any issue at the moment as both
> the minimum and maximum version are set to 1 by all callers, but it's
> still a bug. Fix it.

Huh, idk why it was inverted in the first place :/

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

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

> ---
>  include/libcamera/internal/ipa_manager.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h
> index 34224e330da7..42201839901b 100644
> --- a/include/libcamera/internal/ipa_manager.h
> +++ b/include/libcamera/internal/ipa_manager.h
> @@ -31,8 +31,8 @@ public:
>  
>  	template<typename T>
>  	static std::unique_ptr<T> createIPA(PipelineHandler *pipe,
> -					    uint32_t maxVersion,
> -					    uint32_t minVersion)
> +					    uint32_t minVersion,
> +					    uint32_t maxVersion)
>  	{
>  		IPAModule *m = nullptr;
>  
> -- 
> Regards,
> 
> Laurent Pinchart
>

Patch
diff mbox series

diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h
index 34224e330da7..42201839901b 100644
--- a/include/libcamera/internal/ipa_manager.h
+++ b/include/libcamera/internal/ipa_manager.h
@@ -31,8 +31,8 @@  public:
 
 	template<typename T>
 	static std::unique_ptr<T> createIPA(PipelineHandler *pipe,
-					    uint32_t maxVersion,
-					    uint32_t minVersion)
+					    uint32_t minVersion,
+					    uint32_t maxVersion)
 	{
 		IPAModule *m = nullptr;