<div dir="ltr"><div dir="ltr">Hi Laurent, thank you for the patch.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, May 31, 2021 at 11:08 AM Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">When compiling with optimization, gcc 9 and newer throw an unitialized<br>
variable warning:<br>
<br>
../../src/libcamera/pipeline/ipu3/imgu.cpp: In function ‘void libcamera::{anonymous}::calculateBDSHeight(libcamera::ImgUDevice::Pipe*, const libcamera::Size&, const libcamera::Size&, unsigned int, float)’:<br>
../../src/libcamera/pipeline/ipu3/imgu.cpp:172:17: error: ‘bdsHeight’ may be used uninitialized in this function [-Werror=maybe-uninitialized]<br>
172 | unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight);<br>
<br>
Neither clang not gcc versions older than 9 complain. This seems to be<br>
a false positive.<br>
<br>
However, there's an obvious error in the code. The second while () loop<br>
in the first part of calculateBDSHeight() modifies the bdsHeight<br>
variable set by the first loop even if the second loop doesn't find a<br>
suitable height. This can result in an incorrect bdsHeight value. Fix<br>
this, which also gets rid of the compiler warning.<br>
<br>
Signed-off-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br></blockquote><div><br></div><div>The fix looks correct to me.</div><div>Reviewed-by: Hirokazu Honda <<a href="mailto:hiroh@chromium.org">hiroh@chromium.org</a>></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
---<br>
src/libcamera/pipeline/ipu3/imgu.cpp | 14 ++++++++------<br>
1 file changed, 8 insertions(+), 6 deletions(-)<br>
<br>
diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp<br>
index 3e517ac67962..4eb3f7b730a9 100644<br>
--- a/src/libcamera/pipeline/ipu3/imgu.cpp<br>
+++ b/src/libcamera/pipeline/ipu3/imgu.cpp<br>
@@ -138,12 +138,13 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc<br>
while (ifHeight >= minIFHeight && ifHeight <= iif.height &&<br>
ifHeight / bdsSF >= minBDSHeight) {<br>
<br>
- bdsHeight = ifHeight / bdsSF;<br>
- if (std::fmod(bdsHeight, 1.0) == 0) {<br>
- unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight);<br>
+ float height = ifHeight / bdsSF;<br>
+ if (std::fmod(height, 1.0) == 0) {<br>
+ unsigned int bdsIntHeight = static_cast<unsigned int>(height);<br>
<br>
if (!(bdsIntHeight % BDS_ALIGN_H)) {<br>
foundIfHeight = ifHeight;<br>
+ bdsHeight = height;<br>
break;<br>
}<br>
}<br>
@@ -155,12 +156,13 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc<br>
while (ifHeight >= minIFHeight && ifHeight <= iif.height &&<br>
ifHeight / bdsSF >= minBDSHeight) {<br>
<br>
- bdsHeight = ifHeight / bdsSF;<br>
- if (std::fmod(bdsHeight, 1.0) == 0) {<br>
- unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight);<br>
+ float height = ifHeight / bdsSF;<br>
+ if (std::fmod(height, 1.0) == 0) {<br>
+ unsigned int bdsIntHeight = static_cast<unsigned int>(height);<br>
<br>
if (!(bdsIntHeight % BDS_ALIGN_H)) {<br>
foundIfHeight = ifHeight;<br>
+ bdsHeight = height;<br>
break;<br>
}<br>
}<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
<br>
</blockquote></div></div>