[libcamera-devel] [PATCH] libcamera: pipeline: rkisp1: fix crop configuration
Niklas Söderlund
niklas.soderlund at ragnatech.se
Fri Nov 13 00:57:47 CET 2020
Hi Helen,
Thanks for your work.
On 2020-11-06 17:32:27 -0300, Helen Koike wrote:
> Crop rectangle was not being configured on the isp output pad nor in the
> resizer input pad, causing an unecessary crop in the image and an
> unecessary scaling by the resizer when streaming with a higher
> resolution then the default 800x600.
>
> Example:
> cam -c 1 -C -s width=3280,height=2464
>
> In the pipeline:
> sensor->isp->resizer->dma_engine
>
> isp output crop is set to 800x600, which limits the output format to
> 800x600, which is propagated to the resizer input format set to 800x600,
> and the resizer output format is set to the desired end resolution
> 3280x2464.
>
> Signed-off-by: Helen Koike <helen.koike at collabora.com>
> ---
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 16 +++++++++++++---
> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 7 ++++++-
> 2 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index c74a2e9b..10d44400 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -698,17 +698,27 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> if (ret < 0)
> return ret;
>
> - LOG(RkISP1, Debug) << "ISP input pad configured with " << format.toString();
> + LOG(RkISP1, Debug)
> + << "ISP input pad configured with " << format.toString()
> + << " crop " << rect.toString();
>
> /* YUYV8_2X8 is required on the ISP source path pad for YUV output. */
> format.mbus_code = MEDIA_BUS_FMT_YUYV8_2X8;
> - LOG(RkISP1, Debug) << "Configuring ISP output pad with " << format.toString();
> + LOG(RkISP1, Debug)
> + << "Configuring ISP output pad with " << format.toString()
> + << " crop (img stabilizer) " << rect.toString();
I understand the crop rectangle may be used to implement image
stabilization, but it's not yet. Is there any specific reason you
mention it here or would it make sens to s/(img stabilizer)// ?
For the patch itself,
Tested-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
I will wait for us to figure out the best wording here, but I can fix it
while applying once we do.
> +
> + ret = isp_->setSelection(2, V4L2_SEL_TGT_CROP, &rect);
> + if (ret < 0)
> + return ret;
>
> ret = isp_->setFormat(2, &format);
> if (ret < 0)
> return ret;
>
> - LOG(RkISP1, Debug) << "ISP output pad configured with " << format.toString();
> + LOG(RkISP1, Debug)
> + << "ISP output pad configured with " << format.toString()
> + << " crop (img stabilizer) " << rect.toString();
>
> for (const StreamConfiguration &cfg : *config) {
> if (cfg.stream() == &data->mainPathStream_)
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> index e98515c8..50c0747c 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> @@ -117,9 +117,14 @@ int RkISP1Path::configure(const StreamConfiguration &config,
> if (ret < 0)
> return ret;
>
> + Rectangle rect(0, 0, ispFormat.size);
> + ret = resizer_->setSelection(0, V4L2_SEL_TGT_CROP, &rect);
> + if (ret < 0)
> + return ret;
> +
> LOG(RkISP1, Debug)
> << "Configured " << name_ << " resizer input pad with "
> - << ispFormat.toString();
> + << ispFormat.toString() << " crop " << rect.toString();
>
> ispFormat.size = config.size;
>
> --
> 2.29.2
>
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list