fix: Resolve enable state for v4l and screen capture (#728)

* fix: Resolve the enable state for v4l and screen capture

* Use instance index instead of pointer

* Second try

* fix(QtGrabber): QScreen ownership

* Remove v4l2 compState listener

include/grabber/V4L2Grabber.h

@@ -90,8 +90,6 @@ public slots:
 
 	void stop();
 
-	void compStateChangeRequest(const hyperion::Components component, bool enable);
-
 signals:
 	void newFrame(const Image<ColorRgb> & image);
 	void readError(const char* err);

include/grabber/V4L2Wrapper.h

@@ -27,7 +27,6 @@ public slots:
 	void setDeviceVideoStandard(QString device, VideoStandard videoStandard);
 
 signals:
-	void compStateChangeRequest(const hyperion::Components component, bool enable);
 
 private slots:
 	void newFrame(const Image<ColorRgb> & image);

include/hyperion/Grabber.h

@@ -97,12 +97,6 @@ public:
 	///
 	void setEnabled(bool enable);
 
-signals:
-	///
-	/// @brief PIPE component state changes from HyperionDaemon to V4L2Grabber
-	///
-	void compStateChangeRequest(const hyperion::Components component, bool enable);
-
 protected:
 	ImageResampler _imageResampler;
 

include/hyperion/GrabberWrapper.h

@@ -17,6 +17,10 @@ class Grabber;
 class GlobalSignals;
 class QTimer;
 
+/// List of Hyperion instances that requested screen capt
+static QList<int> GRABBER_SYS_CLIENTS;
+static QList<int> GRABBER_V4L_CLIENTS;
+
 ///
 /// This class will be inherted by FramebufferWrapper and others which contains the real capture interface
 ///
@@ -33,6 +37,11 @@ public:
 	///
 	virtual bool start();
 
+	///
+	/// Starts maybe the grabber wich produces led values with the specified update rate
+	///
+	virtual void tryStart();
+
 	///
 	/// Stop grabber
 	///
@@ -94,8 +103,12 @@ signals:
 	///
 	void systemImage(const QString& name, const Image<ColorRgb>& image);
 
-protected:
+private slots:
+	/// @brief Handle a source request event from Hyperion.
+	/// Will start and stop grabber based on active listeners count
+	void handleSourceRequest(const hyperion::Components& component, const int hyperionInd, const bool listen);
 
+protected:
 	QString _grabberName;
 
 	/// The timer for generating events with the specified update rate

include/utils/GlobalSignals.h

@@ -92,4 +92,12 @@ signals:
 	///
 	void globalRegRequired(int priority);
 
+	///
+	/// @brief Tell v4l2/screen capture the listener state
+	/// @param component  The component to handle
+	/// @param hyperionInd The Hyperion instance index as identifier
+	/// @param listen  True when listening, else false
+	///
+	void requestSource(const hyperion::Components& component, const int hyperionInd, const bool listen);
+
 };

libsrc/grabber/qt/QtGrabber.cpp

@@ -33,12 +33,7 @@ QtGrabber::~QtGrabber()
 
 void QtGrabber::freeResources()
 {
-	// cleanup
+	// Qt seems to hold the ownership of the QScreen pointers
-	if (_screen != nullptr)
-	{
-		delete _screen;
-		_screen = nullptr;
-	}
 }
 
 bool QtGrabber::setupDisplay()

libsrc/grabber/v4l2/V4L2Grabber.cpp

@@ -58,10 +58,6 @@ V4L2Grabber::V4L2Grabber(const QString & device
 	setPixelDecimation(pixelDecimation);
 	getV4Ldevices();
 
-	// connect componentStateChange only for build-in grabber
-	if (HyperionIManager::HIMinstance)
-		connect(this, &Grabber::compStateChangeRequest, this, &V4L2Grabber::compStateChangeRequest);
-
 	// init
 	setDeviceVideoStandard(device, videoStandard);
 }
@@ -1175,17 +1171,3 @@ void V4L2Grabber::setDeviceVideoStandard(QString device, VideoStandard videoStan
 		if(started) start();
 	}
 }
-
-void V4L2Grabber::compStateChangeRequest(const hyperion::Components component, bool enable)
-{
-	if (component == hyperion::COMP_V4L)
-	{
-		if (_initialized != enable)
-		{
-			if (enable)
-				start();
-			else
-				stop();
-		}
-	}
-}

libsrc/grabber/v4l2/V4L2Wrapper.cpp

@@ -23,8 +23,6 @@ V4L2Wrapper::V4L2Wrapper(const QString &device,
 	// Handle the image in the captured thread using a direct connection
 	connect(&_grabber, SIGNAL(newFrame(Image<ColorRgb>)), this, SLOT(newFrame(Image<ColorRgb>)), Qt::DirectConnection);
 	connect(&_grabber, SIGNAL(readError(const char*)), this, SLOT(readError(const char*)), Qt::DirectConnection);
-	
-	connect(this, &V4L2Wrapper::compStateChangeRequest, _ggrabber, &Grabber::compStateChangeRequest);
 }
 
 bool V4L2Wrapper::start()

libsrc/hyperion/CaptureCont.cpp

@@ -86,7 +86,7 @@ void CaptureCont::setSystemCaptureEnable(const bool& enable)
 		}
 		_systemCaptEnabled = enable;
 		_hyperion->setNewComponentState(hyperion::COMP_GRABBER, enable);
-		//emit _hyperion->compStateChangeRequest(hyperion::COMP_GRABBER, enable);
+		emit GlobalSignals::getInstance()->requestSource(hyperion::COMP_GRABBER, int(_hyperion->getInstanceIndex()), enable);
 	}
 }
 
@@ -109,7 +109,7 @@ void CaptureCont::setV4LCaptureEnable(const bool& enable)
 		}
 		_v4lCaptEnabled = enable;
 		_hyperion->setNewComponentState(hyperion::COMP_V4L, enable);
-		//emit _hyperion->compStateChangeRequest(hyperion::COMP_V4L, enable);
+		emit GlobalSignals::getInstance()->requestSource(hyperion::COMP_V4L, int(_hyperion->getInstanceIndex()), enable);
 	}
 }
 

libsrc/hyperion/ComponentRegister.cpp

@@ -37,7 +37,6 @@ void ComponentRegister::setNewComponentState(const hyperion::Components comp, co
 		_componentStates[comp] = activated;
 		// emit component has changed state
 	 	emit updatedComponentState(comp, activated);
-		emit _hyperion->compStateChangeRequest(comp, activated);
 	}
 }
 

libsrc/hyperion/GrabberWrapper.cpp

@@ -28,6 +28,9 @@ GrabberWrapper::GrabberWrapper(QString grabberName, Grabber * ggrabber, unsigned
 	(_grabberName.startsWith("V4L"))
 		? connect(this, &GrabberWrapper::systemImage, GlobalSignals::getInstance(), &GlobalSignals::setV4lImage)
 		: connect(this, &GrabberWrapper::systemImage, GlobalSignals::getInstance(), &GlobalSignals::setSystemImage);
+
+	// listen for source requests
+	connect(GlobalSignals::getInstance(), &GlobalSignals::requestSource, this, &GrabberWrapper::handleSourceRequest);
 }
 
 GrabberWrapper::~GrabberWrapper()
@@ -39,6 +42,7 @@ GrabberWrapper::~GrabberWrapper()
 bool GrabberWrapper::start()
 {
 	// Start the timer with the pre configured interval
+	Debug(_log,"Grabber start()");
 	_timer->start();
 	return _timer->isActive();
 }
@@ -46,6 +50,7 @@ bool GrabberWrapper::start()
 void GrabberWrapper::stop()
 {
 	// Stop the timer, effectivly stopping the process
+	Debug(_log,"Grabber stop()");
 	_timer->stop();
 }
 
@@ -169,3 +174,40 @@ void GrabberWrapper::handleSettingsUpdate(const settings::type& type, const QJso
 		}
 	}
 }
+
+void GrabberWrapper::handleSourceRequest(const hyperion::Components& component, const int hyperionInd, const bool listen)
+{
+	if(component == hyperion::Components::COMP_GRABBER  && !_grabberName.startsWith("V4L"))
+	{
+		if(listen && !GRABBER_SYS_CLIENTS.contains(hyperionInd))
+			GRABBER_SYS_CLIENTS.append(hyperionInd);
+		else if (!listen)
+			GRABBER_SYS_CLIENTS.removeOne(hyperionInd);
+
+		if(GRABBER_SYS_CLIENTS.empty())
+			stop();
+		else
+			start();
+	}
+	else if(component == hyperion::Components::COMP_V4L && _grabberName.startsWith("V4L"))
+	{
+		if(listen && !GRABBER_V4L_CLIENTS.contains(hyperionInd))
+			GRABBER_V4L_CLIENTS.append(hyperionInd);
+		else if (!listen)
+			GRABBER_V4L_CLIENTS.removeOne(hyperionInd);
+
+		if(GRABBER_V4L_CLIENTS.empty())
+			stop();
+		else
+			start();
+	}
+}
+
+void GrabberWrapper::tryStart()
+{
+		// verify start condition
+	if((_grabberName.startsWith("V4L") && !GRABBER_V4L_CLIENTS.empty()) || (!_grabberName.startsWith("V4L") && !GRABBER_SYS_CLIENTS.empty()))
+	{
+		start();
+	}
+}

src/hyperiond/hyperiond.cpp

@@ -327,22 +327,52 @@ void HyperionDaemon::handleSettingsUpdate(const settings::type& settingsType, co
 
 			// stop all capture interfaces
 			#ifdef ENABLE_FB
-			if(_fbGrabber != nullptr)  _fbGrabber->stop();
+			if(_fbGrabber != nullptr)
+			{ 
+				_fbGrabber->stop();
+				delete _fbGrabber;
+				_fbGrabber = nullptr;
+			}
 			#endif
 			#ifdef ENABLE_DISPMANX
-			if(_dispmanx != nullptr)   _dispmanx->stop();
+			if(_dispmanx != nullptr)
+			{
+				_dispmanx->stop();
+				delete _dispmanx;
+				_dispmanx = nullptr;
+			}
 			#endif
 			#ifdef ENABLE_AMLOGIC
-			if(_amlGrabber != nullptr) _amlGrabber->stop();
+			if(_amlGrabber != nullptr)
+			{
+				_amlGrabber->stop();
+				delete _amlGrabber;
+				_amlGrabber = nullptr;
+			}
 			#endif
 			#ifdef ENABLE_OSX
-			if(_osxGrabber != nullptr) _osxGrabber->stop();
+			if(_osxGrabber != nullptr)
+			{
+				_osxGrabber->stop();
+				delete _osxGrabber;
+				_osxGrabber = nullptr;
+			}
 			#endif
 			#ifdef ENABLE_X11
-			if(_x11Grabber != nullptr) _x11Grabber->stop();
+			if(_x11Grabber != nullptr)
+			{
+				 _x11Grabber->stop();
+				 delete _x11Grabber;
+				 _x11Grabber = nullptr;
+			}
 			#endif
 			#ifdef ENABLE_QT
-			if(_qtGrabber != nullptr) _qtGrabber->stop();
+			if(_qtGrabber != nullptr)
+			{
+				_qtGrabber->stop();
+				delete _qtGrabber;
+				_qtGrabber = nullptr;
+			}
 			#endif
 
 			// create/start capture interface
@@ -351,7 +381,7 @@ void HyperionDaemon::handleSettingsUpdate(const settings::type& settingsType, co
 				if(_fbGrabber == nullptr)
 					createGrabberFramebuffer(grabberConfig);
 				#ifdef ENABLE_FB
-				_fbGrabber->start();
+				_fbGrabber->tryStart();
 				#endif
 			}
 			else if(type == "dispmanx")
@@ -359,7 +389,7 @@ void HyperionDaemon::handleSettingsUpdate(const settings::type& settingsType, co
 				if(_dispmanx == nullptr)
 					createGrabberDispmanx();
 				#ifdef ENABLE_DISPMANX
-				_dispmanx->start();
+				_dispmanx->tryStart();
 				#endif
 			}
 			else if(type == "amlogic")
@@ -367,7 +397,7 @@ void HyperionDaemon::handleSettingsUpdate(const settings::type& settingsType, co
 				if(_amlGrabber == nullptr)
 					createGrabberAmlogic();
 				#ifdef ENABLE_AMLOGIC
-				_amlGrabber->start();
+				_amlGrabber->tryStart();
 				#endif
 			}
 			else if(type == "osx")
@@ -375,7 +405,7 @@ void HyperionDaemon::handleSettingsUpdate(const settings::type& settingsType, co
 				if(_osxGrabber == nullptr)
 					createGrabberOsx(grabberConfig);
 				#ifdef ENABLE_OSX
-				_osxGrabber->start();
+				_osxGrabber->tryStart();
 				#endif
 			}
 			else if(type == "x11")
@@ -383,7 +413,7 @@ void HyperionDaemon::handleSettingsUpdate(const settings::type& settingsType, co
 				if(_x11Grabber == nullptr)
 					createGrabberX11(grabberConfig);
 				#ifdef ENABLE_X11
-				_x11Grabber->start();
+				_x11Grabber->tryStart();
 				#endif
 			}
 			else if(type == "qt")
@@ -391,7 +421,7 @@ void HyperionDaemon::handleSettingsUpdate(const settings::type& settingsType, co
 				if(_qtGrabber == nullptr)
 					createGrabberQt(grabberConfig);
 				#ifdef ENABLE_QT
-				_qtGrabber->start();
+				_qtGrabber->tryStart();
 				#endif
 			}
 			else
@@ -436,7 +466,6 @@ void HyperionDaemon::handleSettingsUpdate(const settings::type& settingsType, co
 			// connect to HyperionDaemon signal
 			connect(this, &HyperionDaemon::videoMode, _v4l2Grabber, &V4L2Wrapper::setVideoMode);
 			connect(this, &HyperionDaemon::settingsChanged, _v4l2Grabber, &V4L2Wrapper::handleSettingsUpdate);
-			connect(this, &HyperionDaemon::compStateChangeRequest, _v4l2Grabber, &V4L2Wrapper::compStateChangeRequest);
 #else
 		Error(_log, "The v4l2 grabber can not be instantiated, because it has been left out from the build");
 #endif