[libcamera-devel] [PATCH 3/3] cam: kms_sink: Scale the frame buffer to full screen if supported

Laurent Pinchart laurent.pinchart at ideasonboard.com
Sun Aug 7 21:35:13 CEST 2022


Hi Kieran,

Thank you for the patch.

On Sun, Aug 07, 2022 at 08:21:34PM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart via libcamera-devel (2022-08-07 19:01:00)
> > The KMS sink currently displays the frame buffer on the top-left corner
> > of the screen, resulting in either a black area on the bottom and right
> > sides (if the frame buffer is smaller than the display resolution) of in
> > a restricted field of view (if the frame buffer is larger than the
> > display resolution). Improve this by scaling the frame buffer to full
> > screen if supported, and aligning the crop rectangle to the frame buffer
> > center if the field of view needs to be restricted.
> > 
> > The implementation test three possible composition options, from best to
> > worst. The tests are performed when the camera is started, as testing
> > atomic commits requires access to frame buffer objects, which are not
> > available at configure time. Changing this would require either a large
> > refactoring of the cam application to provide frame buffers earlier, or
> > extending the KMS API to support testing commits with dummy buffer
> > objects. Both are candidates for later development.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/cam/kms_sink.cpp | 92 ++++++++++++++++++++++++++++++++++++++++----
> >  src/cam/kms_sink.h   |  8 ++++
> >  2 files changed, 92 insertions(+), 8 deletions(-)
> > 
> > diff --git a/src/cam/kms_sink.cpp b/src/cam/kms_sink.cpp
> > index 16435ede6b6a..2f306955cf51 100644
> > --- a/src/cam/kms_sink.cpp
> > +++ b/src/cam/kms_sink.cpp
> > @@ -284,6 +284,77 @@ int KMSSink::stop()
> >         return FrameSink::stop();
> >  }
> >  
> > +bool KMSSink::testModeSet(DRM::FrameBuffer *drmBuffer,
> > +                         const libcamera::Rectangle &src,
> > +                         const libcamera::Rectangle &dst)
> > +{
> > +       DRM::AtomicRequest drmRequest{ &dev_ };
> > +
> > +       drmRequest.addProperty(connector_, "CRTC_ID", crtc_->id());
> > +
> > +       drmRequest.addProperty(crtc_, "ACTIVE", 1);
> > +       drmRequest.addProperty(crtc_, "MODE_ID", mode_->toBlob(&dev_));
> > +
> > +       drmRequest.addProperty(plane_, "CRTC_ID", crtc_->id());
> > +       drmRequest.addProperty(plane_, "FB_ID", drmBuffer->id());
> > +       drmRequest.addProperty(plane_, "SRC_X", src.x << 16);
> > +       drmRequest.addProperty(plane_, "SRC_Y", src.y << 16);
> > +       drmRequest.addProperty(plane_, "SRC_W", src.width << 16);
> > +       drmRequest.addProperty(plane_, "SRC_H", src.height << 16);
> > +       drmRequest.addProperty(plane_, "CRTC_X", dst.x);
> > +       drmRequest.addProperty(plane_, "CRTC_Y", dst.y);
> > +       drmRequest.addProperty(plane_, "CRTC_W", dst.width);
> > +       drmRequest.addProperty(plane_, "CRTC_H", dst.height);
> > +
> > +       return !drmRequest.commit(DRM::AtomicRequest::FlagAllowModeset |
> > +                                 DRM::AtomicRequest::FlagTestOnly);
> > +}
> > +
> > +bool KMSSink::setupComposition(DRM::FrameBuffer *drmBuffer)
> > +{
> > +       /*
> > +        * Test composition options, from most to least desirable, to select the
> > +        * best one.
> > +        */
> > +       const libcamera::Rectangle framebuffer{ size_ };
> > +       const libcamera::Rectangle display{ 0, 0, mode_->hdisplay, mode_->vdisplay };
> > +
> > +       /* 1. Scale the frame buffer to full screen. */
> 
> Can we preserve aspect ratio?
> 
> That might mean two tests. One that checks if we can 'scale' and center
> to fit the screen (with black/white/whatever) boundaries on the edges of
> either horizontal or vertical.
> 
> Then one which scales and crops to fit without borders...

I can give it a try.

Preserving the aspect ratio can make the image smaller, which may not
always be desirable (I've implemented this series because I want to see
the full image on the screen to check the effect of algorithms, so
maximizing the size could be better for that use case). It could be nice
to make this configurable, but to start with, I can preserve the aspect
ratio unconditionally.

> > +       libcamera::Rectangle src = libcamera::Rectangle{ size_ };
> > +       libcamera::Rectangle dst = display;
> > +
> > +       if (testModeSet(drmBuffer, src, dst)) {
> > +               std::cout << "KMS: full-screen scaled output" << std::endl;
> > +               src_ = src;
> > +               dst_ = dst;
> > +               return true;
> > +       }
> > +
> > +       /* 2. Center the frame buffer on the display. */
> > +       src = display.size().centeredTo(framebuffer.center()).boundedTo(framebuffer);
> > +       dst = framebuffer.size().centeredTo(display.center()).boundedTo(display);
> > +
> > +       if (testModeSet(drmBuffer, src, dst)) {
> > +               std::cout << "KMS: centered output" << std::endl;
> > +               src_ = src;
> > +               dst_ = dst;
> > +               return true;
> > +       }
> > +
> > +       /* 3. Align the frame buffer on the top-left of the display. */
> > +       src = framebuffer.boundedTo(display);
> > +       dst = display.boundedTo(framebuffer);
> > +
> > +       if (testModeSet(drmBuffer, src, dst)) {
> > +               std::cout << "KMS: top-left aligned output" << std::endl;
> > +               src_ = src;
> > +               dst_ = dst;
> > +               return true;
> > +       }
> > +
> > +       return false;
> > +}
> > +
> >  bool KMSSink::processRequest(libcamera::Request *camRequest)
> >  {
> >         /*
> > @@ -307,20 +378,25 @@ bool KMSSink::processRequest(libcamera::Request *camRequest)
> >  
> >         if (!active_ && !queued_) {
> >                 /* Enable the display pipeline on the first frame. */
> > +               if (!setupComposition(drmBuffer)) {
> > +                       std::cerr << "Failed to setup composition" << std::endl;
> > +                       return true;
> > +               }
> > +
> >                 drmRequest->addProperty(connector_, "CRTC_ID", crtc_->id());
> >  
> >                 drmRequest->addProperty(crtc_, "ACTIVE", 1);
> >                 drmRequest->addProperty(crtc_, "MODE_ID", mode_->toBlob(&dev_));
> >  
> >                 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", size_.width << 16);
> > -               drmRequest->addProperty(plane_, "SRC_H", size_.height << 16);
> > -               drmRequest->addProperty(plane_, "CRTC_X", 0);
> > -               drmRequest->addProperty(plane_, "CRTC_Y", 0);
> > -               drmRequest->addProperty(plane_, "CRTC_W", size_.width);
> > -               drmRequest->addProperty(plane_, "CRTC_H", size_.height);
> > +               drmRequest->addProperty(plane_, "SRC_X", src_.x << 16);
> > +               drmRequest->addProperty(plane_, "SRC_Y", src_.y << 16);
> > +               drmRequest->addProperty(plane_, "SRC_W", src_.width << 16);
> > +               drmRequest->addProperty(plane_, "SRC_H", src_.height << 16);
> > +               drmRequest->addProperty(plane_, "CRTC_X", dst_.x);
> > +               drmRequest->addProperty(plane_, "CRTC_Y", dst_.y);
> > +               drmRequest->addProperty(plane_, "CRTC_W", dst_.width);
> > +               drmRequest->addProperty(plane_, "CRTC_H", dst_.height);
> >  
> >                 flags |= DRM::AtomicRequest::FlagAllowModeset;
> >         }
> > diff --git a/src/cam/kms_sink.h b/src/cam/kms_sink.h
> > index 8f5f08667cea..76c4e611bf85 100644
> > --- a/src/cam/kms_sink.h
> > +++ b/src/cam/kms_sink.h
> > @@ -50,6 +50,11 @@ private:
> >  
> >         int selectPipeline(const libcamera::PixelFormat &format);
> >         int configurePipeline(const libcamera::PixelFormat &format);
> > +       bool testModeSet(DRM::FrameBuffer *drmBuffer,
> > +                        const libcamera::Rectangle &src,
> > +                        const libcamera::Rectangle &dst);
> > +       bool setupComposition(DRM::FrameBuffer *drmBuffer);
> > +
> >         void requestComplete(DRM::AtomicRequest *request);
> >  
> >         DRM::Device dev_;
> > @@ -63,6 +68,9 @@ private:
> >         libcamera::Size size_;
> >         unsigned int stride_;
> >  
> > +       libcamera::Rectangle src_;
> > +       libcamera::Rectangle dst_;
> > +
> >         std::map<libcamera::FrameBuffer *, std::unique_ptr<DRM::FrameBuffer>> buffers_;
> >  
> >         std::mutex lock_;

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list