[{"id":24444,"web_url":"https://patchwork.libcamera.org/comment/24444/","msgid":"<165999668035.2190824.2981595654220825250@Monstersaurus>","date":"2022-08-08T22:11:20","subject":"Re: [libcamera-devel] [PATCH v1.1 3/3] cam: kms_sink: Scale the\n\tframe buffer to full screen if supported","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2022-08-08 22:48:51)\n> The KMS sink currently displays the frame buffer on the top-left corner\n> of the screen, resulting in either a black area on the bottom and right\n> sides (if the frame buffer is smaller than the display resolution) of in\n> a restricted field of view (if the frame buffer is larger than the\n> display resolution). Improve this by scaling the frame buffer to full\n> screen if supported, and aligning the crop rectangle to the frame buffer\n> center if the field of view needs to be restricted.\n> \n> The implementation test fiyr possible composition options, from best to\n\n... tests for possible ... ?\n\n> worst. The tests are performed when the camera is started, as testing\n> atomic commits requires access to frame buffer objects, which are not\n> available at configure time. Changing this would require either a large\n> refactoring of the cam application to provide frame buffers earlier, or\n> extending the KMS API to support testing commits with dummy buffer\n> objects. Both are candidates for later development.\n> \n> Signed-off-by: Laurent Pinchart \n> Reviewed-by: Eric Curtin \n> ---\n> Changes since v1:\n> \n> - Maintain the aspect ratio if possible\n> ---\n> src/cam/kms_sink.cpp | 109 +++++++++++++++++++++++++++++++++++++++----\n> src/cam/kms_sink.h | 8 ++++\n> 2 files changed, 109 insertions(+), 8 deletions(-)\n> \n> diff --git a/src/cam/kms_sink.cpp b/src/cam/kms_sink.cpp\n> index 16435ede6b6a..17e2fa69bff7 100644\n> --- a/src/cam/kms_sink.cpp\n> +++ b/src/cam/kms_sink.cpp\n> @@ -284,6 +284,94 @@ int KMSSink::stop()\n> return FrameSink::stop();\n> }\n> \n> +bool KMSSink::testModeSet(DRM::FrameBuffer *drmBuffer,\n> + const libcamera::Rectangle &src,\n> + const libcamera::Rectangle &dst)\n> +{\n> + DRM::AtomicRequest drmRequest{ &dev_ };\n> +\n> + drmRequest.addProperty(connector_, \"CRTC_ID\", crtc_->id());\n> +\n> + drmRequest.addProperty(crtc_, \"ACTIVE\", 1);\n> + drmRequest.addProperty(crtc_, \"MODE_ID\", mode_->toBlob(&dev_));\n> +\n> + drmRequest.addProperty(plane_, \"CRTC_ID\", crtc_->id());\n> + drmRequest.addProperty(plane_, \"FB_ID\", drmBuffer->id());\n> + drmRequest.addProperty(plane_, \"SRC_X\", src.x << 16);\n> + drmRequest.addProperty(plane_, \"SRC_Y\", src.y << 16);\n> + drmRequest.addProperty(plane_, \"SRC_W\", src.width << 16);\n> + drmRequest.addProperty(plane_, \"SRC_H\", src.height << 16);\n> + drmRequest.addProperty(plane_, \"CRTC_X\", dst.x);\n> + drmRequest.addProperty(plane_, \"CRTC_Y\", dst.y);\n> + drmRequest.addProperty(plane_, \"CRTC_W\", dst.width);\n> + drmRequest.addProperty(plane_, \"CRTC_H\", dst.height);\n> +\n> + return !drmRequest.commit(DRM::AtomicRequest::FlagAllowModeset |\n> + DRM::AtomicRequest::FlagTestOnly);\n> +}\n> +\n> +bool KMSSink::setupComposition(DRM::FrameBuffer *drmBuffer)\n> +{\n> + /*\n> + * Test composition options, from most to least desirable, to select the\n> + * best one.\n> + */\n> + const libcamera::Rectangle framebuffer{ size_ };\n> + const libcamera::Rectangle display{ 0, 0, mode_->hdisplay, mode_->vdisplay };\n> +\n> + /* 1. Scale the frame buffer to full screen, preserving aspect ratio. */\n> + libcamera::Rectangle src = framebuffer;\n> + libcamera::Rectangle dst = display.size().boundedToAspectRatio(framebuffer.size())\n> + .centeredTo(display.center());\n> +\n\nThat sounds much better to me!\n\n\nReviewed-by: Kieran Bingham \n\n> + if (testModeSet(drmBuffer, src, dst)) {\n> + std::cout << \"KMS: full-screen scaled output, square pixels\"\n> + << std::endl;\n> + src_ = src;\n> + dst_ = dst;\n> + return true;\n> + }\n> +\n> + /*\n> + * 2. Scale the frame buffer to full screen, without preserving aspect\n> + * ratio.\n> + */\n> + src = framebuffer;\n> + dst = display;\n> +\n> + if (testModeSet(drmBuffer, src, dst)) {\n> + std::cout << \"KMS: full-screen scaled output, non-square pixels\"\n> + << std::endl;\n> + src_ = src;\n> + dst_ = dst;\n> + return true;\n> + }\n> +\n> + /* 3. Center the frame buffer on the display. */\n> + src = display.size().centeredTo(framebuffer.center()).boundedTo(framebuffer);\n> + dst = framebuffer.size().centeredTo(display.center()).boundedTo(display);\n> +\n> + if (testModeSet(drmBuffer, src, dst)) {\n> + std::cout << \"KMS: centered output\" << std::endl;\n> + src_ = src;\n> + dst_ = dst;\n> + return true;\n> + }\n> +\n> + /* 4. Align the frame buffer on the top-left of the display. */\n> + src = framebuffer.boundedTo(display);\n> + dst = display.boundedTo(framebuffer);\n> +\n> + if (testModeSet(drmBuffer, src, dst)) {\n> + std::cout << \"KMS: top-left aligned output\" << std::endl;\n> + src_ = src;\n> + dst_ = dst;\n> + return true;\n> + }\n> +\n> + return false;\n> +}\n> +\n> bool KMSSink::processRequest(libcamera::Request *camRequest)\n> {\n> /*\n> @@ -307,20 +395,25 @@ bool KMSSink::processRequest(libcamera::Request *camRequest)\n> \n> if (!active_ && !queued_) {\n> /* Enable the display pipeline on the first frame. */\n> + if (!setupComposition(drmBuffer)) {\n> + std::cerr << \"Failed to setup composition\" << std::endl;\n> + return true;\n> + }\n> +\n> drmRequest->addProperty(connector_, \"CRTC_ID\", crtc_->id());\n> \n> drmRequest->addProperty(crtc_, \"ACTIVE\", 1);\n> drmRequest->addProperty(crtc_, \"MODE_ID\", mode_->toBlob(&dev_));\n> \n> drmRequest->addProperty(plane_, \"CRTC_ID\", crtc_->id());\n> - drmRequest->addProperty(plane_, \"SRC_X\", 0 << 16);\n> - drmRequest->addProperty(plane_, \"SRC_Y\", 0 << 16);\n> - drmRequest->addProperty(plane_, \"SRC_W\", size_.width << 16);\n> - drmRequest->addProperty(plane_, \"SRC_H\", size_.height << 16);\n> - drmRequest->addProperty(plane_, \"CRTC_X\", 0);\n> - drmRequest->addProperty(plane_, \"CRTC_Y\", 0);\n> - drmRequest->addProperty(plane_, \"CRTC_W\", size_.width);\n> - drmRequest->addProperty(plane_, \"CRTC_H\", size_.height);\n> + drmRequest->addProperty(plane_, \"SRC_X\", src_.x << 16);\n> + drmRequest->addProperty(plane_, \"SRC_Y\", src_.y << 16);\n> + drmRequest->addProperty(plane_, \"SRC_W\", src_.width << 16);\n> + drmRequest->addProperty(plane_, \"SRC_H\", src_.height << 16);\n> + drmRequest->addProperty(plane_, \"CRTC_X\", dst_.x);\n> + drmRequest->addProperty(plane_, \"CRTC_Y\", dst_.y);\n> + drmRequest->addProperty(plane_, \"CRTC_W\", dst_.width);\n> + drmRequest->addProperty(plane_, \"CRTC_H\", dst_.height);\n> \n> flags |= DRM::AtomicRequest::FlagAllowModeset;\n> }\n> diff --git a/src/cam/kms_sink.h b/src/cam/kms_sink.h\n> index 8f5f08667cea..76c4e611bf85 100644\n> --- a/src/cam/kms_sink.h\n> +++ b/src/cam/kms_sink.h\n> @@ -50,6 +50,11 @@ private:\n> \n> int selectPipeline(const libcamera::PixelFormat &format);\n> int configurePipeline(const libcamera::PixelFormat &format);\n> + bool testModeSet(DRM::FrameBuffer *drmBuffer,\n> + const libcamera::Rectangle &src,\n> + const libcamera::Rectangle &dst);\n> + bool setupComposition(DRM::FrameBuffer *drmBuffer);\n> +\n> void requestComplete(DRM::AtomicRequest *request);\n> \n> DRM::Device dev_;\n> @@ -63,6 +68,9 @@ private:\n> libcamera::Size size_;\n> unsigned int stride_;\n> \n> + libcamera::Rectangle src_;\n> + libcamera::Rectangle dst_;\n> +\n> std::map> buffers_;\n> \n> std::mutex lock_;\n> -- \n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E0DB3C3272\n\tfor ;\n\tMon, 8 Aug 2022 22:11:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3017E6332B;\n\tTue, 9 Aug 2022 00:11:25 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4316C63312\n\tfor ;\n\tTue, 9 Aug 2022 00:11:23 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C6ACD481;\n\tTue, 9 Aug 2022 00:11:22 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659996685;\n\tbh=U+GCMFCMa+02FZLwwK2lrPxpZSmpk+rETmgfZlBgxKo=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=ESpvA/UGy5U1fx0G57kT8GmZVdg1BTe4aY0tRlBZ/AUkhpYfE+qFJaA1uPIhbLaa2\n\t02DEl1C7MZn95UC7iQhudESJuTcsEdq9yt55tiF3Cio0mIGGleNmRxYiNdXUmDWfr/\n\tS1MqG4M4pXOfJX7OI2bLOaj3NrjZ3ih2vVOFBKOoXvQmEVmqvMQUjYhx68Bbyh9qN2\n\tXU9qCVioa2GamEr3ApRA5VsutylbepSVgcGNZOkQGNehD89fVQaD/eXBprcyM9LAfU\n\tBjhdLEt6F4Pko8SWn9blk3aXQHijv3M77a6mar9nCUf6Z7Scl7QbMwG4Z8yyIx8Kio\n\tVLKcmhsbbw32w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659996682;\n\tbh=U+GCMFCMa+02FZLwwK2lrPxpZSmpk+rETmgfZlBgxKo=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=pc9TV6Ckn43Fdt5XQJvoK+SLWVEACfF0XokkyUT6JDoxkAWv/DmY4+CF+k3Fz30JJ\n\t9cBLezNSfCQj6ru77Ux2xhUAskLB74+xt1djggu1YxTrx8i6PIpfhp0e/WV0XSlQM4\n\tm3BmQPttJ+RqNQfyFDLcYcqPj1trYybBvXV5FQaQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"pc9TV6Ck\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220808214851.30804-1-laurent.pinchart@ideasonboard.com>","References":"<20220807180100.396-4-laurent.pinchart@ideasonboard.com>\n\t<20220808214851.30804-1-laurent.pinchart@ideasonboard.com>","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 08 Aug 2022 23:11:20 +0100","Message-ID":"<165999668035.2190824.2981595654220825250@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v1.1 3/3] cam: kms_sink: Scale the\n\tframe buffer to full screen if supported","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]