[libcamera-devel,1/4] ipa: raspberrypi: fix bin_x calculation
diff mbox series

Message ID 20201007110743.55561-2-tomi.valkeinen@iki.fi
State Superseded
Headers show
Series
  • ipa: raspberrypi: fix uninit variables
Related show

Commit Message

Tomi Valkeinen Oct. 7, 2020, 11:07 a.m. UTC
I presume this code is supposed to set bin_x and bin_y, and not bin_y
two times. This caused use of uninitialized variable later when bin_x
was used.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@iki.fi>
---
 src/ipa/raspberrypi/raspberrypi.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Kieran Bingham Oct. 7, 2020, 12:32 p.m. UTC | #1
Hi Tomi,

On 07/10/2020 12:07, Tomi Valkeinen wrote:
> I presume this code is supposed to set bin_x and bin_y, and not bin_y
> two times. This caused use of uninitialized variable later when bin_x
> was used.
> 

Ayeee - indeed this looks accurate to me.

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


> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@iki.fi>
> ---
>  src/ipa/raspberrypi/raspberrypi.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index b0c7d1c..48a72dd 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -178,7 +178,7 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)
>  	 *
>  	 * \todo Get the pipeline handle to provide the full data
>  	 */
> -	mode_.bin_y = std::min(2, static_cast<int>(mode_.scale_x));
> +	mode_.bin_x = std::min(2, static_cast<int>(mode_.scale_x));
>  	mode_.bin_y = std::min(2, static_cast<int>(mode_.scale_y));
>  
>  	/* The noise factor is the square root of the total binning factor. */
>
David Plowman Oct. 7, 2020, 1:05 p.m. UTC | #2
Hi Tomi

On Wed, 7 Oct 2020 at 13:32, Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Hi Tomi,
>
> On 07/10/2020 12:07, Tomi Valkeinen wrote:
> > I presume this code is supposed to set bin_x and bin_y, and not bin_y
> > two times. This caused use of uninitialized variable later when bin_x
> > was used.
> >
>
> Ayeee - indeed this looks accurate to me.
>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>

Indeed!

Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

Thanks very much for this patch!

Best regards
David

>
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@iki.fi>
> > ---
> >  src/ipa/raspberrypi/raspberrypi.cpp | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> > index b0c7d1c..48a72dd 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -178,7 +178,7 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)
> >        *
> >        * \todo Get the pipeline handle to provide the full data
> >        */
> > -     mode_.bin_y = std::min(2, static_cast<int>(mode_.scale_x));
> > +     mode_.bin_x = std::min(2, static_cast<int>(mode_.scale_x));
> >       mode_.bin_y = std::min(2, static_cast<int>(mode_.scale_y));
> >
> >       /* The noise factor is the square root of the total binning factor. */
> >
>
> --
> Regards
> --
> Kieran

Patch
diff mbox series

diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index b0c7d1c..48a72dd 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -178,7 +178,7 @@  void IPARPi::setMode(const CameraSensorInfo &sensorInfo)
 	 *
 	 * \todo Get the pipeline handle to provide the full data
 	 */
-	mode_.bin_y = std::min(2, static_cast<int>(mode_.scale_x));
+	mode_.bin_x = std::min(2, static_cast<int>(mode_.scale_x));
 	mode_.bin_y = std::min(2, static_cast<int>(mode_.scale_y));
 
 	/* The noise factor is the square root of the total binning factor. */