[libcamera-devel] [PATCH 2/2] libcamera: pipeline: ipu3: Fix incorrect bdsHeight calculation
Jean-Michel Hautbois
jeanmichel.hautbois at ideasonboard.com
Mon May 31 20:28:13 CEST 2021
Hi Laurent,
On 31/05/2021 04:07, Laurent Pinchart wrote:
> When compiling with optimization, gcc 9 and newer throw an unitialized
> variable warning:
>
> ../../src/libcamera/pipeline/ipu3/imgu.cpp: In function ‘void libcamera::{anonymous}::calculateBDSHeight(libcamera::ImgUDevice::Pipe*, const libcamera::Size&, const libcamera::Size&, unsigned int, float)’:
> ../../src/libcamera/pipeline/ipu3/imgu.cpp:172:17: error: ‘bdsHeight’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 172 | unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight);
>
> Neither clang not gcc versions older than 9 complain. This seems to be
> a false positive.
>
> However, there's an obvious error in the code. The second while () loop
> in the first part of calculateBDSHeight() modifies the bdsHeight
> variable set by the first loop even if the second loop doesn't find a
> suitable height. This can result in an incorrect bdsHeight value. Fix
> this, which also gets rid of the compiler warning.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Tested-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> ---
> src/libcamera/pipeline/ipu3/imgu.cpp | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
> index 3e517ac67962..4eb3f7b730a9 100644
> --- a/src/libcamera/pipeline/ipu3/imgu.cpp
> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp
> @@ -138,12 +138,13 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc
> while (ifHeight >= minIFHeight && ifHeight <= iif.height &&
> ifHeight / bdsSF >= minBDSHeight) {
>
> - bdsHeight = ifHeight / bdsSF;
> - if (std::fmod(bdsHeight, 1.0) == 0) {
> - unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight);
> + float height = ifHeight / bdsSF;
> + if (std::fmod(height, 1.0) == 0) {
> + unsigned int bdsIntHeight = static_cast<unsigned int>(height);
>
> if (!(bdsIntHeight % BDS_ALIGN_H)) {
> foundIfHeight = ifHeight;
> + bdsHeight = height;
> break;
> }
> }
> @@ -155,12 +156,13 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc
> while (ifHeight >= minIFHeight && ifHeight <= iif.height &&
> ifHeight / bdsSF >= minBDSHeight) {
>
> - bdsHeight = ifHeight / bdsSF;
> - if (std::fmod(bdsHeight, 1.0) == 0) {
> - unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight);
> + float height = ifHeight / bdsSF;
> + if (std::fmod(height, 1.0) == 0) {
> + unsigned int bdsIntHeight = static_cast<unsigned int>(height);
>
> if (!(bdsIntHeight % BDS_ALIGN_H)) {
> foundIfHeight = ifHeight;
> + bdsHeight = height;
> break;
> }
> }
>
More information about the libcamera-devel
mailing list