| Message ID | 20241120085753.1993436-5-stefan.klug@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Stefan On Wed, Nov 20, 2024 at 09:57:43AM +0100, Stefan Klug wrote: > One Rectangle instance is used to calculate the inputCrop and the > outputCrop of the ISP in the rkisp1 pipeline. Split that into two > distinct variables, because both values will be needed in the upcoming > patches. This patch does not contain any functional changes. > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 9d36554cec6e..6491a48ee83c 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -794,14 +794,15 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) > if (ret < 0) > return ret; > > - Rectangle rect(0, 0, format.size); > - ret = isp_->setSelection(0, V4L2_SEL_TGT_CROP, &rect); > + Rectangle inputCrop(0, 0, format.size); > + Rectangle outputCrop = inputCrop; > + ret = isp_->setSelection(0, V4L2_SEL_TGT_CROP, &inputCrop); > if (ret < 0) > return ret; > > LOG(RkISP1, Debug) > << "ISP input pad configured with " << format > - << " crop " << rect; > + << " crop " << inputCrop; > > const PixelFormat &streamFormat = config->at(0).pixelFormat; > const PixelFormatInfo &info = PixelFormatInfo::info(streamFormat); > @@ -819,18 +820,18 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) > const auto &cfg = config->at(0); > Size ispCrop = format.size.boundedToAspectRatio(cfg.size) > .alignedUpTo(2, 2); > - rect = ispCrop.centeredTo(Rectangle(format.size).center()); > + outputCrop = ispCrop.centeredTo(Rectangle(format.size).center()); If we don't enter the if() {} outputCrop will not be updated, and will be initialized with the initial value of inputCrop, which could be updated by the setSelection() call. I would move the declaration Rectangle outputCrop = inputCrop; after ret = isp_->setSelection(0, V4L2_SEL_TGT_CROP, &inputCrop); if (ret < 0) return ret; With that Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > if (ispCrop != format.size) > LOG(RkISP1, Info) << "ISP output needs to be cropped to " > - << rect; > + << outputCrop; > format.size = ispCrop; > } > > LOG(RkISP1, Debug) > << "Configuring ISP output pad with " << format > - << " crop " << rect; > + << " crop " << outputCrop; > > - ret = isp_->setSelection(2, V4L2_SEL_TGT_CROP, &rect); > + ret = isp_->setSelection(2, V4L2_SEL_TGT_CROP, &outputCrop); > if (ret < 0) > return ret; > > @@ -841,7 +842,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) > > LOG(RkISP1, Debug) > << "ISP output pad configured with " << format > - << " crop " << rect; > + << " crop " << outputCrop; > > std::map<unsigned int, IPAStream> streamConfig; > std::vector<std::reference_wrapper<StreamConfiguration>> outputCfgs; > -- > 2.43.0 >
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 9d36554cec6e..6491a48ee83c 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -794,14 +794,15 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) if (ret < 0) return ret; - Rectangle rect(0, 0, format.size); - ret = isp_->setSelection(0, V4L2_SEL_TGT_CROP, &rect); + Rectangle inputCrop(0, 0, format.size); + Rectangle outputCrop = inputCrop; + ret = isp_->setSelection(0, V4L2_SEL_TGT_CROP, &inputCrop); if (ret < 0) return ret; LOG(RkISP1, Debug) << "ISP input pad configured with " << format - << " crop " << rect; + << " crop " << inputCrop; const PixelFormat &streamFormat = config->at(0).pixelFormat; const PixelFormatInfo &info = PixelFormatInfo::info(streamFormat); @@ -819,18 +820,18 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) const auto &cfg = config->at(0); Size ispCrop = format.size.boundedToAspectRatio(cfg.size) .alignedUpTo(2, 2); - rect = ispCrop.centeredTo(Rectangle(format.size).center()); + outputCrop = ispCrop.centeredTo(Rectangle(format.size).center()); if (ispCrop != format.size) LOG(RkISP1, Info) << "ISP output needs to be cropped to " - << rect; + << outputCrop; format.size = ispCrop; } LOG(RkISP1, Debug) << "Configuring ISP output pad with " << format - << " crop " << rect; + << " crop " << outputCrop; - ret = isp_->setSelection(2, V4L2_SEL_TGT_CROP, &rect); + ret = isp_->setSelection(2, V4L2_SEL_TGT_CROP, &outputCrop); if (ret < 0) return ret; @@ -841,7 +842,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) LOG(RkISP1, Debug) << "ISP output pad configured with " << format - << " crop " << rect; + << " crop " << outputCrop; std::map<unsigned int, IPAStream> streamConfig; std::vector<std::reference_wrapper<StreamConfiguration>> outputCfgs;
One Rectangle instance is used to calculate the inputCrop and the outputCrop of the ISP in the rkisp1 pipeline. Split that into two distinct variables, because both values will be needed in the upcoming patches. This patch does not contain any functional changes. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)