| Message ID | 20210318103941.18837-7-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Delegated to: | Jacopo Mondi |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thanks for the patch, On 18/03/2021 11:39, Jacopo Mondi wrote: > Add pipe calculation debug with a new associated log category. > > This helps compare the pipe calculation with the one performed by the > python script. That is useful indeed ! > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> Tested-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > --- > src/libcamera/pipeline/ipu3/imgu.cpp | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp > index ccc85864fc39..b6a1fe34494e 100644 > --- a/src/libcamera/pipeline/ipu3/imgu.cpp > +++ b/src/libcamera/pipeline/ipu3/imgu.cpp > @@ -23,6 +23,7 @@ > namespace libcamera { > > LOG_DECLARE_CATEGORY(IPU3) > +LOG_DEFINE_CATEGORY(IMGUPIPE) > > namespace { > > @@ -128,6 +129,8 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc > unsigned int ifHeight; > float bdsHeight; > > + LOG(IMGUPIPE, Debug) << "BDS sf: " << bdsSF << " BDS width: " << bdsWidth; > + > if (!isSameRatio(pipe->input, gdc)) { > unsigned int foundIfHeights[2] = { 0, 0 }; > float estIFHeight = (iif.width * gdc.height) / > @@ -135,6 +138,9 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc > estIFHeight = std::clamp<float>(estIFHeight, minIFHeight, iif.height); > > ifHeight = utils::alignUp(estIFHeight, IF_ALIGN_H); > + LOG(IMGUPIPE, Debug) << "Estimate IF Height: " << estIFHeight > + << " IF Height: " << ifHeight; > + > while (ifHeight >= minIFHeight && ifHeight <= iif.height && > ifHeight / bdsSF >= minBDSHeight) { > > @@ -176,6 +182,11 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc > if (foundIfHeights[0] || foundIfHeights[1]) { > unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight); > > + LOG(IMGUPIPE, Debug) > + << "IF: " << Size(iif.width, ifHeight).toString() > + << "BDS: " << Size(bdsWidth, bdsIntHeight).toString() > + << "GDC: " << gdc.toString(); > + > pipeConfigs.push_back({ bdsSF, { iif.width, ifHeight }, > { bdsWidth, bdsIntHeight }, gdc }); > return; > @@ -190,6 +201,12 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc > > if (!(ifHeight % IF_ALIGN_H) && > !(bdsIntHeight % BDS_ALIGN_H)) { > + > + LOG(IMGUPIPE, Debug) > + << "IF: " << Size(iif.width, ifHeight).toString() > + << "BDS: " << Size(bdsWidth, bdsIntHeight).toString() > + << "GDC: " << gdc.toString(); > + > pipeConfigs.push_back({ bdsSF, { iif.width, ifHeight }, > { bdsWidth, bdsIntHeight }, gdc }); > } > @@ -269,6 +286,8 @@ Size calculateGDC(ImgUDevice::Pipe *pipe) > gdc.width = main.width * sf; > gdc.height = main.height * sf; > > + LOG(IMGUPIPE, Debug) << "GDC: " << gdc.toString(); > + > return gdc; > } > > @@ -286,6 +305,11 @@ FOV calcFOV(const Size &in, const ImgUDevice::PipeConfig &pipe) > fov.w = (inW - (ifCropW + gdcCropW)) / inW; > fov.h = (inH - (ifCropH + gdcCropH)) / inH; > > + LOG(IMGUPIPE, Debug) > + << "IF (" << pipe.iif.toString() << ") - BDS (" > + << pipe.bds.toString() << ") - GDC (" << pipe.gdc.toString() > + << ") -> FOV: " << fov.w << "x" << fov.h; > + > return fov; > } > >
Hi Jacopo, Thank you for the patch. On Thu, Mar 18, 2021 at 11:39:40AM +0100, Jacopo Mondi wrote: > Add pipe calculation debug with a new associated log category. > > This helps compare the pipe calculation with the one performed by the > python script. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/pipeline/ipu3/imgu.cpp | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp > index ccc85864fc39..b6a1fe34494e 100644 > --- a/src/libcamera/pipeline/ipu3/imgu.cpp > +++ b/src/libcamera/pipeline/ipu3/imgu.cpp > @@ -23,6 +23,7 @@ > namespace libcamera { > > LOG_DECLARE_CATEGORY(IPU3) > +LOG_DEFINE_CATEGORY(IMGUPIPE) ImgUPipe > > namespace { > > @@ -128,6 +129,8 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc > unsigned int ifHeight; > float bdsHeight; > > + LOG(IMGUPIPE, Debug) << "BDS sf: " << bdsSF << " BDS width: " << bdsWidth; s/ BDS width/, BDS width/ > + > if (!isSameRatio(pipe->input, gdc)) { > unsigned int foundIfHeights[2] = { 0, 0 }; > float estIFHeight = (iif.width * gdc.height) / > @@ -135,6 +138,9 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc > estIFHeight = std::clamp<float>(estIFHeight, minIFHeight, iif.height); > > ifHeight = utils::alignUp(estIFHeight, IF_ALIGN_H); > + LOG(IMGUPIPE, Debug) << "Estimate IF Height: " << estIFHeight s/Estimate/Estimated/ > + << " IF Height: " << ifHeight; s/ IF/, IF/ and similarly below (the code is certainly readable when split across multiple lines, but the debug message will be printed in the log on a single line, so commas help readability). > + > while (ifHeight >= minIFHeight && ifHeight <= iif.height && > ifHeight / bdsSF >= minBDSHeight) { > > @@ -176,6 +182,11 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc > if (foundIfHeights[0] || foundIfHeights[1]) { > unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight); > > + LOG(IMGUPIPE, Debug) > + << "IF: " << Size(iif.width, ifHeight).toString() > + << "BDS: " << Size(bdsWidth, bdsIntHeight).toString() > + << "GDC: " << gdc.toString(); I'd create two variables to store the IF and BDS sizes, as they're used on the next line. > + > pipeConfigs.push_back({ bdsSF, { iif.width, ifHeight }, > { bdsWidth, bdsIntHeight }, gdc }); > return; > @@ -190,6 +201,12 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc > > if (!(ifHeight % IF_ALIGN_H) && > !(bdsIntHeight % BDS_ALIGN_H)) { > + > + LOG(IMGUPIPE, Debug) > + << "IF: " << Size(iif.width, ifHeight).toString() > + << "BDS: " << Size(bdsWidth, bdsIntHeight).toString() > + << "GDC: " << gdc.toString(); Same here. > + > pipeConfigs.push_back({ bdsSF, { iif.width, ifHeight }, > { bdsWidth, bdsIntHeight }, gdc }); > } > @@ -269,6 +286,8 @@ Size calculateGDC(ImgUDevice::Pipe *pipe) > gdc.width = main.width * sf; > gdc.height = main.height * sf; > > + LOG(IMGUPIPE, Debug) << "GDC: " << gdc.toString(); > + > return gdc; > } > > @@ -286,6 +305,11 @@ FOV calcFOV(const Size &in, const ImgUDevice::PipeConfig &pipe) > fov.w = (inW - (ifCropW + gdcCropW)) / inW; > fov.h = (inH - (ifCropH + gdcCropH)) / inH; > > + LOG(IMGUPIPE, Debug) > + << "IF (" << pipe.iif.toString() << ") - BDS (" > + << pipe.bds.toString() << ") - GDC (" << pipe.gdc.toString() > + << ") -> FOV: " << fov.w << "x" << fov.h; > + > return fov; On a side note, we should create a SizeF class, but that's out of scope for this series. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > } >
diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp index ccc85864fc39..b6a1fe34494e 100644 --- a/src/libcamera/pipeline/ipu3/imgu.cpp +++ b/src/libcamera/pipeline/ipu3/imgu.cpp @@ -23,6 +23,7 @@ namespace libcamera { LOG_DECLARE_CATEGORY(IPU3) +LOG_DEFINE_CATEGORY(IMGUPIPE) namespace { @@ -128,6 +129,8 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc unsigned int ifHeight; float bdsHeight; + LOG(IMGUPIPE, Debug) << "BDS sf: " << bdsSF << " BDS width: " << bdsWidth; + if (!isSameRatio(pipe->input, gdc)) { unsigned int foundIfHeights[2] = { 0, 0 }; float estIFHeight = (iif.width * gdc.height) / @@ -135,6 +138,9 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc estIFHeight = std::clamp<float>(estIFHeight, minIFHeight, iif.height); ifHeight = utils::alignUp(estIFHeight, IF_ALIGN_H); + LOG(IMGUPIPE, Debug) << "Estimate IF Height: " << estIFHeight + << " IF Height: " << ifHeight; + while (ifHeight >= minIFHeight && ifHeight <= iif.height && ifHeight / bdsSF >= minBDSHeight) { @@ -176,6 +182,11 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc if (foundIfHeights[0] || foundIfHeights[1]) { unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight); + LOG(IMGUPIPE, Debug) + << "IF: " << Size(iif.width, ifHeight).toString() + << "BDS: " << Size(bdsWidth, bdsIntHeight).toString() + << "GDC: " << gdc.toString(); + pipeConfigs.push_back({ bdsSF, { iif.width, ifHeight }, { bdsWidth, bdsIntHeight }, gdc }); return; @@ -190,6 +201,12 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc if (!(ifHeight % IF_ALIGN_H) && !(bdsIntHeight % BDS_ALIGN_H)) { + + LOG(IMGUPIPE, Debug) + << "IF: " << Size(iif.width, ifHeight).toString() + << "BDS: " << Size(bdsWidth, bdsIntHeight).toString() + << "GDC: " << gdc.toString(); + pipeConfigs.push_back({ bdsSF, { iif.width, ifHeight }, { bdsWidth, bdsIntHeight }, gdc }); } @@ -269,6 +286,8 @@ Size calculateGDC(ImgUDevice::Pipe *pipe) gdc.width = main.width * sf; gdc.height = main.height * sf; + LOG(IMGUPIPE, Debug) << "GDC: " << gdc.toString(); + return gdc; } @@ -286,6 +305,11 @@ FOV calcFOV(const Size &in, const ImgUDevice::PipeConfig &pipe) fov.w = (inW - (ifCropW + gdcCropW)) / inW; fov.h = (inH - (ifCropH + gdcCropH)) / inH; + LOG(IMGUPIPE, Debug) + << "IF (" << pipe.iif.toString() << ") - BDS (" + << pipe.bds.toString() << ") - GDC (" << pipe.gdc.toString() + << ") -> FOV: " << fov.w << "x" << fov.h; + return fov; }
Add pipe calculation debug with a new associated log category. This helps compare the pipe calculation with the one performed by the python script. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/pipeline/ipu3/imgu.cpp | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)