| Message ID | 20210913145810.66515-3-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series |
|
| Related | show |
Hi Jean-Michel, Thank you for the patch. On Mon, Sep 13, 2021 at 04:58:01PM +0200, Jean-Michel Hautbois wrote: > The BDS grid has its own limitations for the log2 width and height for > each cell of pixels. Use constexpr for those and comment it to help > understanding their usage. > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > --- > src/ipa/ipu3/ipu3.cpp | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index 90053069..c71f2a6c 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -345,6 +345,8 @@ int IPAIPU3::start() > } > > /** > + * \brief Calculate a grid for the AWB statistics > + * > * This function calculates a grid for the AWB algorithm in the IPU3 firmware. > * Its input is the BDS output size calculated in the ImgU. > * It is limited for now to the simplest method: find the lesser error > @@ -364,16 +366,24 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize) > /* Set the BDS output size in the IPAConfiguration structure */ > context_.configuration.grid.bdsOutputSize = bdsOutputSize; > > - for (uint32_t widthShift = 3; widthShift <= 7; ++widthShift) { > + /* > + * The log2 of the width and height of each cell is limited by the ImgU > + * in the interval [3, 7] according to the kernel header. s/in the interval/to the interval/ > + */ > + constexpr uint32_t kCellMin = 3; > + constexpr uint32_t kCellMax = 7; I think these should be kCellMinShift and kCellMaxShift. Should they also be moved to the top of the file with kMaxCellWidthPerSet and kMaxCellHeightPerSet ? Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + > + for (uint32_t widthShift = kCellMin; widthShift <= kCellMax; ++widthShift) { > uint32_t width = std::min(kMaxCellWidthPerSet, > bdsOutputSize.width >> widthShift); > width = width << widthShift; > - for (uint32_t heightShift = 3; heightShift <= 7; ++heightShift) { > + > + for (uint32_t heightShift = kCellMin; heightShift <= kCellMax; ++heightShift) { > int32_t height = std::min(kMaxCellHeightPerSet, > bdsOutputSize.height >> heightShift); > height = height << heightShift; > uint32_t error = std::abs(static_cast<int>(width - bdsOutputSize.width)) > - + std::abs(static_cast<int>(height - bdsOutputSize.height)); > + + std::abs(static_cast<int>(height - bdsOutputSize.height)); > > if (error > minError) > continue;
diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index 90053069..c71f2a6c 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -345,6 +345,8 @@ int IPAIPU3::start() } /** + * \brief Calculate a grid for the AWB statistics + * * This function calculates a grid for the AWB algorithm in the IPU3 firmware. * Its input is the BDS output size calculated in the ImgU. * It is limited for now to the simplest method: find the lesser error @@ -364,16 +366,24 @@ void IPAIPU3::calculateBdsGrid(const Size &bdsOutputSize) /* Set the BDS output size in the IPAConfiguration structure */ context_.configuration.grid.bdsOutputSize = bdsOutputSize; - for (uint32_t widthShift = 3; widthShift <= 7; ++widthShift) { + /* + * The log2 of the width and height of each cell is limited by the ImgU + * in the interval [3, 7] according to the kernel header. + */ + constexpr uint32_t kCellMin = 3; + constexpr uint32_t kCellMax = 7; + + for (uint32_t widthShift = kCellMin; widthShift <= kCellMax; ++widthShift) { uint32_t width = std::min(kMaxCellWidthPerSet, bdsOutputSize.width >> widthShift); width = width << widthShift; - for (uint32_t heightShift = 3; heightShift <= 7; ++heightShift) { + + for (uint32_t heightShift = kCellMin; heightShift <= kCellMax; ++heightShift) { int32_t height = std::min(kMaxCellHeightPerSet, bdsOutputSize.height >> heightShift); height = height << heightShift; uint32_t error = std::abs(static_cast<int>(width - bdsOutputSize.width)) - + std::abs(static_cast<int>(height - bdsOutputSize.height)); + + std::abs(static_cast<int>(height - bdsOutputSize.height)); if (error > minError) continue;
The BDS grid has its own limitations for the log2 width and height for each cell of pixels. Use constexpr for those and comment it to help understanding their usage. Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> --- src/ipa/ipu3/ipu3.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)