[libcamera-devel,v2,8/8] cam: Add support for viewfinder through DRM/KMS
diff mbox series

Message ID 20210730010306.19956-9-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: Add DRM/KMS viewfinder display to cam
Related show

Commit Message

Laurent Pinchart July 30, 2021, 1:03 a.m. UTC 
Use the KMSSink class to display the viewfinder stream, if any, through
DRM/KMS. The output connector is selected through the new -D/--display
argument.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
Changes since v1:

- Validate display option in CamApp::init()
---
 src/cam/camera_session.cpp | 28 ++++++++++++++++++++++++++++
 src/cam/main.cpp           |  6 ++++++
 src/cam/main.h             |  1 +
 3 files changed, 35 insertions(+)

Comments

Paul Elder Aug. 4, 2021, 7:10 a.m. UTC | #1 
Hi Laurent,

On Fri, Jul 30, 2021 at 04:03:06AM +0300, Laurent Pinchart wrote:
> Use the KMSSink class to display the viewfinder stream, if any, through
> DRM/KMS. The output connector is selected through the new -D/--display
> argument.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> Changes since v1:
> 
> - Validate display option in CamApp::init()
> ---
>  src/cam/camera_session.cpp | 28 ++++++++++++++++++++++++++++
>  src/cam/main.cpp           |  6 ++++++
>  src/cam/main.h             |  1 +
>  3 files changed, 35 insertions(+)
> 
> diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp
> index 4f1f9ec8eb10..8d8c1b8ba469 100644
> --- a/src/cam/camera_session.cpp
> +++ b/src/cam/camera_session.cpp
> @@ -16,6 +16,9 @@
>  #include "camera_session.h"
>  #include "event_loop.h"
>  #include "file_sink.h"
> +#ifdef HAVE_KMS
> +#include "kms_sink.h"
> +#endif
>  #include "main.h"
>  #include "stream_options.h"
>  
> @@ -66,6 +69,26 @@ CameraSession::CameraSession(CameraManager *cm,
>  
>  	bool strictFormats = options_.isSet(OptStrictFormats);
>  
> +	if (options_.isSet(OptDisplay)) {

I was wondering if this needs a guard too, but I guess this will just
never be satisfied, so maybe this is cleaner.

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> +		if (options_.isSet(OptFile)) {
> +			std::cerr << "--display and --file options are mutually exclusive"
> +				  << std::endl;
> +			return;
> +		}
> +
> +		if (roles.size() != 1) {
> +			std::cerr << "Display doesn't support multiple streams"
> +				  << std::endl;
> +			return;
> +		}
> +
> +		if (roles[0] != StreamRole::Viewfinder) {
> +			std::cerr << "Display requires a viewfinder stream"
> +				  << std::endl;
> +			return;
> +		}
> +	}
> +
>  	switch (config->validate()) {
>  	case CameraConfiguration::Valid:
>  		break;
> @@ -161,6 +184,11 @@ int CameraSession::start()
>  
>  	camera_->requestCompleted.connect(this, &CameraSession::requestComplete);
>  
> +#ifdef HAVE_KMS
> +	if (options_.isSet(OptDisplay))
> +		sink_ = std::make_unique<KMSSink>(options_[OptDisplay].toString());
> +#endif
> +
>  	if (options_.isSet(OptFile)) {
>  		if (!options_[OptFile].toString().empty())
>  			sink_ = std::make_unique<FileSink>(options_[OptFile]);
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index 34cbc3229563..c7f664b903e0 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -132,6 +132,12 @@ int CamApp::parseOptions(int argc, char *argv[])
>  			 "Capture until interrupted by user or until <count> frames captured",
>  			 "capture", ArgumentOptional, "count", false,
>  			 OptCamera);
> +#ifdef HAVE_KMS
> +	parser.addOption(OptDisplay, OptionString,
> +			 "Display viewfinder through DRM/KMS on specified connector",
> +			 "display", ArgumentOptional, "connector", false,
> +			 OptCamera);
> +#endif
>  	parser.addOption(OptFile, OptionString,
>  			 "Write captured frames to disk\n"
>  			 "If the file name ends with a '/', it sets the directory in which\n"
> diff --git a/src/cam/main.h b/src/cam/main.h
> index d22451f59817..1c2fab763698 100644
> --- a/src/cam/main.h
> +++ b/src/cam/main.h
> @@ -10,6 +10,7 @@
>  enum {
>  	OptCamera = 'c',
>  	OptCapture = 'C',
> +	OptDisplay = 'D',
>  	OptFile = 'F',
>  	OptHelp = 'h',
>  	OptInfo = 'I',
> -- 
> Regards,
> 
> Laurent Pinchart
>
Laurent Pinchart Aug. 4, 2021, 8:59 a.m. UTC | #2 
Hi Paul,

On Wed, Aug 04, 2021 at 04:10:57PM +0900, paul.elder@ideasonboard.com wrote:
> On Fri, Jul 30, 2021 at 04:03:06AM +0300, Laurent Pinchart wrote:
> > Use the KMSSink class to display the viewfinder stream, if any, through
> > DRM/KMS. The output connector is selected through the new -D/--display
> > argument.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > Changes since v1:
> > 
> > - Validate display option in CamApp::init()
> > ---
> >  src/cam/camera_session.cpp | 28 ++++++++++++++++++++++++++++
> >  src/cam/main.cpp           |  6 ++++++
> >  src/cam/main.h             |  1 +
> >  3 files changed, 35 insertions(+)
> > 
> > diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp
> > index 4f1f9ec8eb10..8d8c1b8ba469 100644
> > --- a/src/cam/camera_session.cpp
> > +++ b/src/cam/camera_session.cpp
> > @@ -16,6 +16,9 @@
> >  #include "camera_session.h"
> >  #include "event_loop.h"
> >  #include "file_sink.h"
> > +#ifdef HAVE_KMS
> > +#include "kms_sink.h"
> > +#endif
> >  #include "main.h"
> >  #include "stream_options.h"
> >  
> > @@ -66,6 +69,26 @@ CameraSession::CameraSession(CameraManager *cm,
> >  
> >  	bool strictFormats = options_.isSet(OptStrictFormats);
> >  
> > +	if (options_.isSet(OptDisplay)) {
> 
> I was wondering if this needs a guard too, but I guess this will just
> never be satisfied, so maybe this is cleaner.

Good point. A guard would avoid dead code, so I think that's valuable.
I'll add one.

> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> > +		if (options_.isSet(OptFile)) {
> > +			std::cerr << "--display and --file options are mutually exclusive"
> > +				  << std::endl;
> > +			return;
> > +		}
> > +
> > +		if (roles.size() != 1) {
> > +			std::cerr << "Display doesn't support multiple streams"
> > +				  << std::endl;
> > +			return;
> > +		}
> > +
> > +		if (roles[0] != StreamRole::Viewfinder) {
> > +			std::cerr << "Display requires a viewfinder stream"
> > +				  << std::endl;
> > +			return;
> > +		}
> > +	}
> > +
> >  	switch (config->validate()) {
> >  	case CameraConfiguration::Valid:
> >  		break;
> > @@ -161,6 +184,11 @@ int CameraSession::start()
> >  
> >  	camera_->requestCompleted.connect(this, &CameraSession::requestComplete);
> >  
> > +#ifdef HAVE_KMS
> > +	if (options_.isSet(OptDisplay))
> > +		sink_ = std::make_unique<KMSSink>(options_[OptDisplay].toString());
> > +#endif
> > +
> >  	if (options_.isSet(OptFile)) {
> >  		if (!options_[OptFile].toString().empty())
> >  			sink_ = std::make_unique<FileSink>(options_[OptFile]);
> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> > index 34cbc3229563..c7f664b903e0 100644
> > --- a/src/cam/main.cpp
> > +++ b/src/cam/main.cpp
> > @@ -132,6 +132,12 @@ int CamApp::parseOptions(int argc, char *argv[])
> >  			 "Capture until interrupted by user or until <count> frames captured",
> >  			 "capture", ArgumentOptional, "count", false,
> >  			 OptCamera);
> > +#ifdef HAVE_KMS
> > +	parser.addOption(OptDisplay, OptionString,
> > +			 "Display viewfinder through DRM/KMS on specified connector",
> > +			 "display", ArgumentOptional, "connector", false,
> > +			 OptCamera);
> > +#endif
> >  	parser.addOption(OptFile, OptionString,
> >  			 "Write captured frames to disk\n"
> >  			 "If the file name ends with a '/', it sets the directory in which\n"
> > diff --git a/src/cam/main.h b/src/cam/main.h
> > index d22451f59817..1c2fab763698 100644
> > --- a/src/cam/main.h
> > +++ b/src/cam/main.h
> > @@ -10,6 +10,7 @@
> >  enum {
> >  	OptCamera = 'c',
> >  	OptCapture = 'C',
> > +	OptDisplay = 'D',
> >  	OptFile = 'F',
> >  	OptHelp = 'h',
> >  	OptInfo = 'I',

Patch
diff mbox series 

diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp
index 4f1f9ec8eb10..8d8c1b8ba469 100644
--- a/src/cam/camera_session.cpp
+++ b/src/cam/camera_session.cpp
@@ -16,6 +16,9 @@ 
 #include "camera_session.h"
 #include "event_loop.h"
 #include "file_sink.h"
+#ifdef HAVE_KMS
+#include "kms_sink.h"
+#endif
 #include "main.h"
 #include "stream_options.h"
 
@@ -66,6 +69,26 @@  CameraSession::CameraSession(CameraManager *cm,
 
 	bool strictFormats = options_.isSet(OptStrictFormats);
 
+	if (options_.isSet(OptDisplay)) {
+		if (options_.isSet(OptFile)) {
+			std::cerr << "--display and --file options are mutually exclusive"
+				  << std::endl;
+			return;
+		}
+
+		if (roles.size() != 1) {
+			std::cerr << "Display doesn't support multiple streams"
+				  << std::endl;
+			return;
+		}
+
+		if (roles[0] != StreamRole::Viewfinder) {
+			std::cerr << "Display requires a viewfinder stream"
+				  << std::endl;
+			return;
+		}
+	}
+
 	switch (config->validate()) {
 	case CameraConfiguration::Valid:
 		break;
@@ -161,6 +184,11 @@  int CameraSession::start()
 
 	camera_->requestCompleted.connect(this, &CameraSession::requestComplete);
 
+#ifdef HAVE_KMS
+	if (options_.isSet(OptDisplay))
+		sink_ = std::make_unique<KMSSink>(options_[OptDisplay].toString());
+#endif
+
 	if (options_.isSet(OptFile)) {
 		if (!options_[OptFile].toString().empty())
 			sink_ = std::make_unique<FileSink>(options_[OptFile]);
diff --git a/src/cam/main.cpp b/src/cam/main.cpp
index 34cbc3229563..c7f664b903e0 100644
--- a/src/cam/main.cpp
+++ b/src/cam/main.cpp
@@ -132,6 +132,12 @@  int CamApp::parseOptions(int argc, char *argv[])
 			 "Capture until interrupted by user or until <count> frames captured",
 			 "capture", ArgumentOptional, "count", false,
 			 OptCamera);
+#ifdef HAVE_KMS
+	parser.addOption(OptDisplay, OptionString,
+			 "Display viewfinder through DRM/KMS on specified connector",
+			 "display", ArgumentOptional, "connector", false,
+			 OptCamera);
+#endif
 	parser.addOption(OptFile, OptionString,
 			 "Write captured frames to disk\n"
 			 "If the file name ends with a '/', it sets the directory in which\n"
diff --git a/src/cam/main.h b/src/cam/main.h
index d22451f59817..1c2fab763698 100644
--- a/src/cam/main.h
+++ b/src/cam/main.h
@@ -10,6 +10,7 @@ 
 enum {
 	OptCamera = 'c',
 	OptCapture = 'C',
+	OptDisplay = 'D',
 	OptFile = 'F',
 	OptHelp = 'h',
 	OptInfo = 'I',