[libcamera-devel] [PATCH v5 00/19] ipu3: rework pipe confiuguration
Jacopo Mondi
jacopo at jmondi.org
Fri Jul 31 17:33:01 CEST 2020
I have addressed minor comments and added a few more details in comment and
documentation here and there.
I have simplified a bit the ImgU pipe configuration by removing not-required
casts, by making the code a bit less dense, and with a small rename.
The diff between v4 and v5 is so minimal I reported the full diff below.
I tested results against the IPU3 pipe configuration tool by instrumenting
a version of it to run on a set of known values and compare the results between
its calculation and libcamera:
https://jmondi.org/cgit/intel-ipu3-pipe-config/commit/?id=78bc59f8bfe8c14908f612008a62edc621660cf3
All patches reviewed but I'm not pushing yet as I would like Kieran's ack, as
he's working on JPEG and moving the ground under his feet while developing seems
not nice.
diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp
index a3ea3eabe318..cf26980c75f1 100644
--- a/src/libcamera/pipeline/ipu3/imgu.cpp
+++ b/src/libcamera/pipeline/ipu3/imgu.cpp
@@ -25,6 +25,14 @@ LOG_DECLARE_CATEGORY(IPU3)
namespace {
+/*
+ * The procedure to calculate the ImgU pipe configuration has been ported
+ * from the pipe_config.py python script, available at:
+ * https://github.com/intel/intel-ipu3-pipecfg
+ * at revision: 61e83f2f7606 ("Add more information into README")
+ */
+
static constexpr unsigned int FILTER_H = 4;
static constexpr unsigned int IF_ALIGN_W = 2;
@@ -103,10 +111,8 @@ float findScaleFactor(float sf, const std::vector<float> &range,
bool isSameRatio(const Size &in, const Size &out)
{
- float inRatio = static_cast<float>(in.width) /
- static_cast<float>(in.height);
- float outRatio = static_cast<float>(out.width) /
- static_cast<float>(out.height);
+ float inRatio = static_cast<float>(in.width) / in.height;
+ float outRatio = static_cast<float>(out.width) / out.height;
if (std::abs(inRatio - outRatio) > 0.1)
return false;
@@ -123,16 +129,18 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc
float bdsHeight;
if (!isSameRatio(pipe->input, gdc)) {
- bool found = false;
- float estIFHeight = static_cast<float>(iif.width * gdc.height) /
+ float estIFHeight = (iif.width * gdc.height) /
static_cast<float>(gdc.width);
estIFHeight = utils::clamp<float>(estIFHeight, minIFHeight, iif.height);
+ bool found = false;
+
ifHeight = utils::alignUp(estIFHeight, IF_ALIGN_H);
- while (ifHeight >= minIFHeight &&
- static_cast<float>(ifHeight) / bdsSF >= minBDSHeight) {
- bdsHeight = static_cast<float>(ifHeight) / bdsSF;
+ while (ifHeight >= minIFHeight && ifHeight / bdsSF >= minBDSHeight) {
+
+ bdsHeight = ifHeight / bdsSF;
if (std::fmod(bdsHeight, 1.0) == 0) {
unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight);
+
if (!(bdsIntHeight % BDS_ALIGN_H)) {
found = true;
break;
@@ -143,11 +151,12 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc
}
ifHeight = utils::alignUp(estIFHeight, IF_ALIGN_H);
- while (ifHeight <= iif.height &&
- static_cast<float>(ifHeight) / bdsSF >= minBDSHeight) {
- bdsHeight = static_cast<float>(ifHeight) / bdsSF;
+ while (ifHeight <= iif.height && ifHeight / bdsSF >= minBDSHeight) {
+
+ bdsHeight = ifHeight / bdsSF;
if (std::fmod(bdsHeight, 1.0) == 0) {
unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight);
+
if (!(bdsIntHeight % BDS_ALIGN_H)) {
found = true;
break;
@@ -159,14 +168,15 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc
if (found) {
unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight);
+
pipeConfigs.push_back({ bdsSF, { iif.width, ifHeight },
{ bdsWidth, bdsIntHeight }, gdc });
return;
}
} else {
ifHeight = utils::alignUp(iif.height, IF_ALIGN_H);
- while (ifHeight > minIFHeight &&
- static_cast<float>(ifHeight) / bdsSF >= minBDSHeight) {
+ while (ifHeight > minIFHeight && ifHeight / bdsSF >= minBDSHeight) {
+
bdsHeight = ifHeight / bdsSF;
if (std::fmod(ifHeight, 1.0) == 0 && std::fmod(bdsHeight, 1.0) == 0) {
unsigned int bdsIntHeight = static_cast<unsigned int>(bdsHeight);
@@ -183,8 +193,7 @@ void calculateBDSHeight(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc
}
}
-void calculateBDS(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc,
- float bdsSF)
+void calculateBDS(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc, float bdsSF)
{
unsigned int minBDSWidth = gdc.width + FILTER_H * 2;
@@ -218,15 +227,14 @@ void calculateBDS(ImgUDevice::Pipe *pipe, const Size &iif, const Size &gdc,
Size calculateGDC(ImgUDevice::Pipe *pipe)
{
const Size &in = pipe->input;
- const Size &main = pipe->output;
+ const Size &main = pipe->main;
const Size &vf = pipe->viewfinder;
Size gdc;
if (!vf.isNull()) {
gdc.width = main.width;
- float ratio = static_cast<float>(main.width * vf.height) /
- static_cast<float>(vf.width);
+ float ratio = (main.width * vf.height) / static_cast<float>(vf.width);
gdc.height = std::max(static_cast<float>(main.height), ratio);
return gdc;
@@ -237,14 +245,13 @@ Size calculateGDC(ImgUDevice::Pipe *pipe)
return gdc;
}
- float totalSF = static_cast<float>(in.width) /
- static_cast<float>(main.width);
+ float totalSF = static_cast<float>(in.width) / main.width;
float bdsSF = totalSF > 2 ? 2 : 1;
float yuvSF = totalSF / bdsSF;
float sf = findScaleFactor(yuvSF, gdcScalingFactors);
- gdc.width = static_cast<float>(main.width) * sf;
- gdc.height = static_cast<float>(main.height) * sf;
+ gdc.width = main.width * sf;
+ gdc.height = main.height * sf;
return gdc;
}
@@ -259,6 +266,7 @@ FOV calcFOV(const Size &in, const ImgUDevice::PipeConfig &pipe)
float ifCropH = static_cast<float>(in.height - pipe.iif.height);
float gdcCropW = static_cast<float>(pipe.bds.width - pipe.gdc.width) * pipe.bds_sf;
float gdcCropH = static_cast<float>(pipe.bds.height - pipe.gdc.height) * pipe.bds_sf;
+
fov.w = (inW - (ifCropW + gdcCropW)) / inW;
fov.h = (inH - (ifCropH + gdcCropH)) / inH;
@@ -304,11 +312,11 @@ FOV calcFOV(const Size &in, const ImgUDevice::PipeConfig &pipe)
* \var Pipe::input
* \brief The input image size
*
- * \var Pipe::output
+ * \var Pipe::main
* \brief The requested main output size
*
* \var Pipe::viewfinder
- * \brief The requested viewfinder size
+ * \brief The requested viewfinder output size
*/
/**
@@ -378,7 +386,7 @@ ImgUDevice::PipeConfig ImgUDevice::calculatePipeConfig(Pipe *pipe)
LOG(IPU3, Debug) << "Calculating pipe configuration for: ";
LOG(IPU3, Debug) << "input: " << pipe->input.toString();
- LOG(IPU3, Debug) << "main: " << pipe->output.toString();
+ LOG(IPU3, Debug) << "main: " << pipe->main.toString();
LOG(IPU3, Debug) << "vf: " << pipe->viewfinder.toString();
const Size &in = pipe->input;
@@ -387,9 +395,9 @@ ImgUDevice::PipeConfig ImgUDevice::calculatePipeConfig(Pipe *pipe)
unsigned int ifWidth = utils::alignUp(in.width, IF_ALIGN_W);
unsigned int ifHeight = in.height;
unsigned int minIfWidth = in.width - IF_CROP_MAX_W;
- float bdsSF = static_cast<float>(in.width) /
- static_cast<float>(gdc.width);
+ float bdsSF = static_cast<float>(in.width) / gdc.width;
float sf = findScaleFactor(bdsSF, bdsScalingFactors, true);
+
while (ifWidth >= minIfWidth) {
Size iif{ ifWidth, ifHeight };
calculateBDS(pipe, iif, gdc, sf);
diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h
index ff8dab7d645c..c73ac5a5a37c 100644
--- a/src/libcamera/pipeline/ipu3/imgu.h
+++ b/src/libcamera/pipeline/ipu3/imgu.h
@@ -37,7 +37,7 @@ public:
struct Pipe {
Size input;
- Size output;
+ Size main;
Size viewfinder;
};
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 06afc7d79dac..161a88da8497 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -225,11 +225,12 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
* margins from the CIO2 output frame size.
*
* The ImgU outputs needs to be strictly smaller than
- * the CIO2 output frame and rounded down to 64
- * pixels in width and 32 pixels in height. This comes
- * from inspecting the pipe configuration script results
- * and the available suggested configurations in the
- * ChromeOS BSP .xml camera tuning files.
+ * the CIO2 output frame and rounded down to 64 pixels
+ * in width and 32 pixels in height. This assumption
+ * comes from inspecting the pipe configuration script
+ * results and the available suggested configurations in
+ * the ChromeOS BSP .xml camera tuning files and shall
+ * be validated.
*
* \todo Clarify what are the hardware constraints
* that require this alignements, if any. It might
@@ -267,9 +268,9 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
cfg->setStream(const_cast<Stream *>(&data_->outStream_));
mainOutputAvailable = false;
- pipe.output = cfg->size;
+ pipe.main = cfg->size;
if (yuvCount == 1)
- pipe.viewfinder = pipe.output;
+ pipe.viewfinder = pipe.main;
LOG(IPU3, Debug) << "Assigned " << cfg->toString()
<< " to the main output";
@@ -333,8 +334,8 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera,
* to the ImgU maximum output size) and aligned down to
* the required frame margin.
*
- * The alignment constraints should be clarified
- * as per the todo item in validate().
+ * \todo Clarify the alignment constraints as exaplained
+ * in validate()
*/
size = sensorResolution.boundedTo(IMGU_OUTPUT_MAX_SIZE);
size.width = utils::alignDown(size.width - 1,
Thanks
j
Jacopo Mondi (19):
libcamera: ipu3: Rename mbusCodesToInfo
libcamera: utils: Add alignUp and alignDown functions
libcamera: geometry: Add isNull() function to Rectangle class
libcamera: ipu3: Remove streams from generateConfiguration
libcamera: ipu3: Make sure the config is valid
libcamera: ipu3: cio2: Report format and sizes
libcamera: ipu3: Do not overwrite StreamConfiguration
libcamera: ipu3: Report StreamFormats
libcamera: ipu3: Remove initialization of Size
libcamera: ipu3: Validate the stream combination
libcamera: camera: Zero streams before validate()
libcamera: ipu3: cio2: Add a const sensor() method
libcamera: ipu3: Adjust and assign streams in validate()
libcamera: ipu3: Remove streams from IPU3CameraConfiguration
libcamera: ipu3: Remove camera_ from IPU3CameraConfiguration
libcamera: ipu3: imgu: Calculate ImgU pipe configuration
libcamera: ipu3: Validate the pipe configuration
libcamera: ipu3: Configure ImgU with the computed parameters
libcamera: ipu3: imgu: Rename configureInput()
include/libcamera/geometry.h | 1 +
include/libcamera/internal/utils.h | 10 +
src/libcamera/camera.cpp | 4 +-
src/libcamera/geometry.cpp | 6 +
src/libcamera/pipeline/ipu3/cio2.cpp | 54 +++-
src/libcamera/pipeline/ipu3/cio2.h | 6 +
src/libcamera/pipeline/ipu3/imgu. | 0
src/libcamera/pipeline/ipu3/imgu.cpp | 386 +++++++++++++++++++++++-
src/libcamera/pipeline/ipu3/imgu.h | 22 +-
src/libcamera/pipeline/ipu3/ipu3.cpp | 424 +++++++++++++--------------
src/libcamera/utils.cpp | 16 +
test/geometry.cpp | 14 +
test/utils.cpp | 11 +
13 files changed, 714 insertions(+), 240 deletions(-)
create mode 100644 src/libcamera/pipeline/ipu3/imgu.
--
2.27.0
More information about the libcamera-devel
mailing list