[libcamera-devel] [PATCH v2] cam: kms_sink: Remove limitation that camera and display must match
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Feb 7 17:11:50 CET 2022
Hi Eric,
Quoting Eric Curtin (2022-02-07 15:00:59)
> There is a limitation that requires input and output to be pixel
> for pixel identical in terms of height and width. Remove this
> limitation to enable more hardware that doesn't match exactly in
> terms of pixels. Centralize the image. This works for the case
> where camera output has more pixels than the display and
> vice-versa. In the case where there are too many pixels for the
> display, we take the most central part of the image cropping out
> the border.
Thankyou, I'm very pleased to see this patch, As I'm sure you're aware
from our discussions on IRC.
I haven't actually been able to test direct render with the DRM part
here, so I think this will be really helpful. I can't test this right
now though, but I hope to this week, so just some quick comments while I
skim through.
> Signed-off-by: Eric Curtin <ecurtin at redhat.com>
> ---
>
> Changes in v2:
> - Tested and support drawing from negative pixel range
> kernel parameter (video=960x540 at 60) was useful here
>
> src/cam/kms_sink.cpp | 32 ++++++++++++++------------------
> src/cam/kms_sink.h | 2 ++
> 2 files changed, 16 insertions(+), 18 deletions(-)
>
> diff --git a/src/cam/kms_sink.cpp b/src/cam/kms_sink.cpp
> index 973cd370..8eb51454 100644
> --- a/src/cam/kms_sink.cpp
> +++ b/src/cam/kms_sink.cpp
> @@ -113,24 +113,20 @@ int KMSSink::configure(const libcamera::CameraConfiguration &config)
> const libcamera::StreamConfiguration &cfg = config.at(0);
>
> const std::vector<DRM::Mode> &modes = connector_->modes();
> - const auto iter = std::find_if(modes.begin(), modes.end(),
> - [&](const DRM::Mode &mode) {
> - return mode.hdisplay == cfg.size.width &&
> - mode.vdisplay == cfg.size.height;
> - });
Hrm, I would maybe expect to see some sort of 'best mode' search here to
find the closest mode that fits the incoming image. I'm sure that might
be a pattern that exists already somewhere.
But I expect being able to draw on any mode, is going to make this work
on more devices than only drawing on an exact mode.
> - if (iter == modes.end()) {
> - std::cerr
> - << "No mode matching " << cfg.size.toString()
> - << std::endl;
> - return -EINVAL;
> - }
>
> int ret = configurePipeline(cfg.pixelFormat);
> if (ret < 0)
> return ret;
>
> - mode_ = &*iter;
> + mode_ = &modes[0];
> size_ = cfg.size;
> +
> + // We need to cast for the case where the camera output has more
> + // pixels than the display, in this case we start drawing from a
> + // negative pixel point to crop out the content to display just
> + // the middle part.
Our code style uses
/*
*
*/
For comment blocks. I guess our checkstyle doesn't actually pick up on
that though.
> + x_ = (mode_->hdisplay - static_cast<int>(size_.width)) / 2;
> + y_ = (mode_->vdisplay - static_cast<int>(size_.height)) / 2;
We have a Rectangle class in include/libcamera/geometry.h which can
handle centering, and might be useful here.
> stride_ = cfg.stride;
>
> return 0;
> @@ -297,12 +293,12 @@ bool KMSSink::processRequest(libcamera::Request *camRequest)
> drmRequest->addProperty(plane_, "CRTC_ID", crtc_->id());
> drmRequest->addProperty(plane_, "SRC_X", 0 << 16);
> drmRequest->addProperty(plane_, "SRC_Y", 0 << 16);
> - drmRequest->addProperty(plane_, "SRC_W", mode_->hdisplay << 16);
> - drmRequest->addProperty(plane_, "SRC_H", mode_->vdisplay << 16);
> - drmRequest->addProperty(plane_, "CRTC_X", 0);
> - drmRequest->addProperty(plane_, "CRTC_Y", 0);
> - drmRequest->addProperty(plane_, "CRTC_W", mode_->hdisplay);
> - drmRequest->addProperty(plane_, "CRTC_H", mode_->vdisplay);
> + drmRequest->addProperty(plane_, "SRC_W", size_.width << 16);
> + drmRequest->addProperty(plane_, "SRC_H", size_.height << 16);
> + drmRequest->addProperty(plane_, "CRTC_X", x_);
> + drmRequest->addProperty(plane_, "CRTC_Y", y_);
> + drmRequest->addProperty(plane_, "CRTC_W", size_.width);
> + drmRequest->addProperty(plane_, "CRTC_H", size_.height);
>
> flags |= DRM::AtomicRequest::FlagAllowModeset;
> }
> diff --git a/src/cam/kms_sink.h b/src/cam/kms_sink.h
> index 4a0a872c..2c16182c 100644
> --- a/src/cam/kms_sink.h
> +++ b/src/cam/kms_sink.h
> @@ -61,6 +61,8 @@ private:
> libcamera::PixelFormat format_;
> libcamera::Size size_;
> unsigned int stride_;
> + int x_; // Where to start drawing camera output
> + int y_; // Where to start drawing camera output
/* */ comments here too please.
> std::map<libcamera::FrameBuffer *, std::unique_ptr<DRM::FrameBuffer>> buffers_;
>
> --
> 2.34.1
>
More information about the libcamera-devel
mailing list