[{"id":18536,"web_url":"https://patchwork.libcamera.org/comment/18536/","msgid":"<20210804071057.GG2167@pyrite.rasen.tech>","date":"2021-08-04T07:10:57","subject":"Re: [libcamera-devel] [PATCH v2 8/8] cam: Add support for\n\tviewfinder through DRM/KMS","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Laurent,\n\nOn Fri, Jul 30, 2021 at 04:03:06AM +0300, Laurent Pinchart wrote:\n> Use the KMSSink class to display the viewfinder stream, if any, through\n> DRM/KMS. The output connector is selected through the new -D/--display\n> argument.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> Changes since v1:\n> \n> - Validate display option in CamApp::init()\n> ---\n> src/cam/camera_session.cpp | 28 ++++++++++++++++++++++++++++\n> src/cam/main.cpp | 6 ++++++\n> src/cam/main.h | 1 +\n> 3 files changed, 35 insertions(+)\n> \n> diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp\n> index 4f1f9ec8eb10..8d8c1b8ba469 100644\n> --- a/src/cam/camera_session.cpp\n> +++ b/src/cam/camera_session.cpp\n> @@ -16,6 +16,9 @@\n> #include \"camera_session.h\"\n> #include \"event_loop.h\"\n> #include \"file_sink.h\"\n> +#ifdef HAVE_KMS\n> +#include \"kms_sink.h\"\n> +#endif\n> #include \"main.h\"\n> #include \"stream_options.h\"\n> \n> @@ -66,6 +69,26 @@ CameraSession::CameraSession(CameraManager *cm,\n> \n> \tbool strictFormats = options_.isSet(OptStrictFormats);\n> \n> +\tif (options_.isSet(OptDisplay)) {\n\nI was wondering if this needs a guard too, but I guess this will just\nnever be satisfied, so maybe this is cleaner.\n\nReviewed-by: Paul Elder \n\n> +\t\tif (options_.isSet(OptFile)) {\n> +\t\t\tstd::cerr << \"--display and --file options are mutually exclusive\"\n> +\t\t\t\t << std::endl;\n> +\t\t\treturn;\n> +\t\t}\n> +\n> +\t\tif (roles.size() != 1) {\n> +\t\t\tstd::cerr << \"Display doesn't support multiple streams\"\n> +\t\t\t\t << std::endl;\n> +\t\t\treturn;\n> +\t\t}\n> +\n> +\t\tif (roles[0] != StreamRole::Viewfinder) {\n> +\t\t\tstd::cerr << \"Display requires a viewfinder stream\"\n> +\t\t\t\t << std::endl;\n> +\t\t\treturn;\n> +\t\t}\n> +\t}\n> +\n> \tswitch (config->validate()) {\n> \tcase CameraConfiguration::Valid:\n> \t\tbreak;\n> @@ -161,6 +184,11 @@ int CameraSession::start()\n> \n> \tcamera_->requestCompleted.connect(this, &CameraSession::requestComplete);\n> \n> +#ifdef HAVE_KMS\n> +\tif (options_.isSet(OptDisplay))\n> +\t\tsink_ = std::make_unique(options_[OptDisplay].toString());\n> +#endif\n> +\n> \tif (options_.isSet(OptFile)) {\n> \t\tif (!options_[OptFile].toString().empty())\n> \t\t\tsink_ = std::make_unique(options_[OptFile]);\n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index 34cbc3229563..c7f664b903e0 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -132,6 +132,12 @@ int CamApp::parseOptions(int argc, char *argv[])\n> \t\t\t \"Capture until interrupted by user or until frames captured\",\n> \t\t\t \"capture\", ArgumentOptional, \"count\", false,\n> \t\t\t OptCamera);\n> +#ifdef HAVE_KMS\n> +\tparser.addOption(OptDisplay, OptionString,\n> +\t\t\t \"Display viewfinder through DRM/KMS on specified connector\",\n> +\t\t\t \"display\", ArgumentOptional, \"connector\", false,\n> +\t\t\t OptCamera);\n> +#endif\n> \tparser.addOption(OptFile, OptionString,\n> \t\t\t \"Write captured frames to disk\\n\"\n> \t\t\t \"If the file name ends with a '/', it sets the directory in which\\n\"\n> diff --git a/src/cam/main.h b/src/cam/main.h\n> index d22451f59817..1c2fab763698 100644\n> --- a/src/cam/main.h\n> +++ b/src/cam/main.h\n> @@ -10,6 +10,7 @@\n> enum {\n> \tOptCamera = 'c',\n> \tOptCapture = 'C',\n> +\tOptDisplay = 'D',\n> \tOptFile = 'F',\n> \tOptHelp = 'h',\n> \tOptInfo = 'I',\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 375AEC3232\n\tfor ;\n\tWed, 4 Aug 2021 07:11:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AA80A6881B;\n\tWed, 4 Aug 2021 09:11:06 +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 722D36880F\n\tfor ;\n\tWed, 4 Aug 2021 09:11:05 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E4FB224F;\n\tWed, 4 Aug 2021 09:11:03 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"FDosSW8S\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628061065;\n\tbh=lRFFOPjLUtbebSzg476I9sbbyWVGdL1sqaSOYbvPXUM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FDosSW8Subct5IwjY+mOq8MDHcAmi++xhcg9ml+/qjIWrtttbi9/VVspa54XKhrOj\n\tB0DL503rH6HHQehp3oEV7N6ykc4Sx6STiiB+IQk0hKIyvFm2JimQpL34qvpdPq31Qo\n\tiz5QK83JqgkjcCllnn2JxIh0QMDg9v29l2oS8QLY=","Date":"Wed, 4 Aug 2021 16:10:57 +0900","From":"paul.elder@ideasonboard.com","To":"Laurent Pinchart ","Message-ID":"<20210804071057.GG2167@pyrite.rasen.tech>","References":"<20210730010306.19956-1-laurent.pinchart@ideasonboard.com>\n\t<20210730010306.19956-9-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210730010306.19956-9-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 8/8] cam: Add support for\n\tviewfinder through DRM/KMS","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18545,"web_url":"https://patchwork.libcamera.org/comment/18545/","msgid":"","date":"2021-08-04T08:59:29","subject":"Re: [libcamera-devel] [PATCH v2 8/8] cam: Add support for\n\tviewfinder through DRM/KMS","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nOn Wed, Aug 04, 2021 at 04:10:57PM +0900, paul.elder@ideasonboard.com wrote:\n> On Fri, Jul 30, 2021 at 04:03:06AM +0300, Laurent Pinchart wrote:\n> > Use the KMSSink class to display the viewfinder stream, if any, through\n> > DRM/KMS. The output connector is selected through the new -D/--display\n> > argument.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > Changes since v1:\n> > \n> > - Validate display option in CamApp::init()\n> > ---\n> > src/cam/camera_session.cpp | 28 ++++++++++++++++++++++++++++\n> > src/cam/main.cpp | 6 ++++++\n> > src/cam/main.h | 1 +\n> > 3 files changed, 35 insertions(+)\n> > \n> > diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp\n> > index 4f1f9ec8eb10..8d8c1b8ba469 100644\n> > --- a/src/cam/camera_session.cpp\n> > +++ b/src/cam/camera_session.cpp\n> > @@ -16,6 +16,9 @@\n> > #include \"camera_session.h\"\n> > #include \"event_loop.h\"\n> > #include \"file_sink.h\"\n> > +#ifdef HAVE_KMS\n> > +#include \"kms_sink.h\"\n> > +#endif\n> > #include \"main.h\"\n> > #include \"stream_options.h\"\n> > \n> > @@ -66,6 +69,26 @@ CameraSession::CameraSession(CameraManager *cm,\n> > \n> > \tbool strictFormats = options_.isSet(OptStrictFormats);\n> > \n> > +\tif (options_.isSet(OptDisplay)) {\n> \n> I was wondering if this needs a guard too, but I guess this will just\n> never be satisfied, so maybe this is cleaner.\n\nGood point. A guard would avoid dead code, so I think that's valuable.\nI'll add one.\n\n> Reviewed-by: Paul Elder \n> \n> > +\t\tif (options_.isSet(OptFile)) {\n> > +\t\t\tstd::cerr << \"--display and --file options are mutually exclusive\"\n> > +\t\t\t\t << std::endl;\n> > +\t\t\treturn;\n> > +\t\t}\n> > +\n> > +\t\tif (roles.size() != 1) {\n> > +\t\t\tstd::cerr << \"Display doesn't support multiple streams\"\n> > +\t\t\t\t << std::endl;\n> > +\t\t\treturn;\n> > +\t\t}\n> > +\n> > +\t\tif (roles[0] != StreamRole::Viewfinder) {\n> > +\t\t\tstd::cerr << \"Display requires a viewfinder stream\"\n> > +\t\t\t\t << std::endl;\n> > +\t\t\treturn;\n> > +\t\t}\n> > +\t}\n> > +\n> > \tswitch (config->validate()) {\n> > \tcase CameraConfiguration::Valid:\n> > \t\tbreak;\n> > @@ -161,6 +184,11 @@ int CameraSession::start()\n> > \n> > \tcamera_->requestCompleted.connect(this, &CameraSession::requestComplete);\n> > \n> > +#ifdef HAVE_KMS\n> > +\tif (options_.isSet(OptDisplay))\n> > +\t\tsink_ = std::make_unique(options_[OptDisplay].toString());\n> > +#endif\n> > +\n> > \tif (options_.isSet(OptFile)) {\n> > \t\tif (!options_[OptFile].toString().empty())\n> > \t\t\tsink_ = std::make_unique(options_[OptFile]);\n> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > index 34cbc3229563..c7f664b903e0 100644\n> > --- a/src/cam/main.cpp\n> > +++ b/src/cam/main.cpp\n> > @@ -132,6 +132,12 @@ int CamApp::parseOptions(int argc, char *argv[])\n> > \t\t\t \"Capture until interrupted by user or until frames captured\",\n> > \t\t\t \"capture\", ArgumentOptional, \"count\", false,\n> > \t\t\t OptCamera);\n> > +#ifdef HAVE_KMS\n> > +\tparser.addOption(OptDisplay, OptionString,\n> > +\t\t\t \"Display viewfinder through DRM/KMS on specified connector\",\n> > +\t\t\t \"display\", ArgumentOptional, \"connector\", false,\n> > +\t\t\t OptCamera);\n> > +#endif\n> > \tparser.addOption(OptFile, OptionString,\n> > \t\t\t \"Write captured frames to disk\\n\"\n> > \t\t\t \"If the file name ends with a '/', it sets the directory in which\\n\"\n> > diff --git a/src/cam/main.h b/src/cam/main.h\n> > index d22451f59817..1c2fab763698 100644\n> > --- a/src/cam/main.h\n> > +++ b/src/cam/main.h\n> > @@ -10,6 +10,7 @@\n> > enum {\n> > \tOptCamera = 'c',\n> > \tOptCapture = 'C',\n> > +\tOptDisplay = 'D',\n> > \tOptFile = 'F',\n> > \tOptHelp = 'h',\n> > \tOptInfo = 'I',","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 B66CFC3232\n\tfor ;\n\tWed, 4 Aug 2021 08:59:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 73AC96880F;\n\tWed, 4 Aug 2021 10:59:42 +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 C6C4A6026C\n\tfor ;\n\tWed, 4 Aug 2021 10:59:41 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 62E5324F;\n\tWed, 4 Aug 2021 10:59:41 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"J3r6uMoR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628067581;\n\tbh=1ioQcCY3Z7TLWJpQWY/BT6xiVnopv1T8neFtQsjAQ48=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=J3r6uMoR9hxvISnae+vyWnWI+3f2aqppwvRLm/Ev4JNL4I97M9yKjM5dHLEmKGjjM\n\tP+c8lEn5T7OglLKVk2t3AEVf4eV9v+oxKNJ6FSbKDB5NGm+2MKjYvYeC+eVwD5kGkJ\n\tedBxLnjKUrd4t6CMk89YAQHX7SUIV3wfyrAJfs9M=","Date":"Wed, 4 Aug 2021 11:59:29 +0300","From":"Laurent Pinchart ","To":"paul.elder@ideasonboard.com","Message-ID":"","References":"<20210730010306.19956-1-laurent.pinchart@ideasonboard.com>\n\t<20210730010306.19956-9-laurent.pinchart@ideasonboard.com>\n\t<20210804071057.GG2167@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210804071057.GG2167@pyrite.rasen.tech>","Subject":"Re: [libcamera-devel] [PATCH v2 8/8] cam: Add support for\n\tviewfinder through DRM/KMS","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]