Fixed Crash when XCB,X11,Qt grabber were configured and display manager changed to Wayland

Author: Lord-Grey

CHANGELOG.md

@@ -64,6 +64,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fixed: Python 3.12 crashes (#1747)
 - osX Grabber: Use ScreenCaptureKit under macOS 15 and above
 - Removed maximum LED number constraint from Matrix layout schema which was not synced with the UI behaviour (#1804)
+- UI: Instance listings are sorted, enabled instances are high-lighted in drop-downs 
 - Fixed bespoke WebSocket implementation by using of QWebSockets (#1816, #1448, #1247, #1130)
 - Fixed mDNS Browser deadlock, plus run in own thread now
 - Fixed that LED Buffer and Layout might get out of sync.
@@ -72,7 +73,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fixed Last update of an effect event is not removed in sources overview
 - Fixed Removed stale _logger object
 - Fixed Smoothing (#1863)
-- UI: Instance listings are sorted, enabled instances are high-lighted in drop-downs 
+- Fixed Crash when XCB,X11 was configured and display manager changed to Wayland
 
 **JSON-API**
 - Refactored JSON-API to ensure consistent authorization behaviour across sessions and single requests with token authorization.

include/grabber/qt/QtGrabber.h

@@ -72,6 +72,13 @@ class QtGrabber : public Grabber
 	///
 	bool setupDisplay();
 
+	///
+	/// @brief Determine if the grabber is available.
+	///
+	/// @return true, on success (i.e. Window Manager is not Wayland), else false
+	///
+	bool isAvailable(bool logError = true) override;
+
 	///
 	/// @brief Opens the input device.
 	///

include/grabber/qt/QtWrapper.h

@@ -38,6 +38,18 @@ class QtWrapper: public GrabberWrapper
 	///
 	QtWrapper(const QJsonDocument& grabberConfig = QJsonDocument());
 
+	///
+	/// @brief Determine if the grabber is available for usage on the platform
+	///
+	/// @return true, on success, else false
+	///
+	bool isAvailable() override;
+
+	///
+	/// Starts the grabber, if available
+	///
+	bool start() override;
+
 	///
 	/// Starts the grabber which produces led values with the specified update rate
 	///

include/grabber/x11/X11Grabber.h

@@ -34,8 +34,14 @@ class X11Grabber : public Grabber , public QAbstractNativeEventFilter
 
 	~X11Grabber() override;
 
-	bool open();
+	///
+	/// @brief Determine if the grabber is available.
+	///
+	/// @return true, on success (i.e. Window Manager is not Wayland), else false
+	///
+	bool isAvailable(bool logError = true) override;
 
+	bool open();
 	bool setupDisplay();
 
 	///

include/grabber/x11/X11Wrapper.h

@@ -38,6 +38,23 @@ class X11Wrapper: public GrabberWrapper
 	///
 	~X11Wrapper() override;
 
+	///
+	/// @brief Determine if the grabber is available for usage on the platform
+	///
+	/// @return true, on success, else false
+	///
+	bool isAvailable() override;
+
+	///
+	/// Starts the grabber, if available
+	///
+	bool start() override;
+
+	///
+	/// Starts the grabber which produces led values with the specified update rate
+	///
+	bool open() override;
+
 public slots:
 	///
 	/// Performs a single frame grab and computes the led-colors

include/grabber/xcb/XcbGrabber.h

@@ -29,6 +29,13 @@ class XcbGrabber : public Grabber, public QAbstractNativeEventFilter
 
 	~XcbGrabber() override;
 
+	///
+	/// @brief Determine if the grabber is available.
+	///
+	/// @return true, on success (i.e. Window Manager is not Wayland), else false
+	///
+	bool isAvailable(bool logError = true) override;
+
 	bool open();
 	bool setupDisplay();
 

include/grabber/xcb/XcbWrapper.h

@@ -37,6 +37,23 @@ class XcbWrapper: public GrabberWrapper
 
 	~XcbWrapper() override;
 
+	///
+	/// @brief Determine if the grabber is available for usage on the platform
+	///
+	/// @return true, on success, else false
+	///
+	bool isAvailable() override;
+
+	///
+	/// Starts the grabber, if available
+	///
+	bool start() override;
+
+	///
+	/// Starts the grabber which produces led values with the specified update rate
+	///
+	bool open() override;
+
 public slots:
 	void action() override;
 

include/hyperion/Grabber.h

@@ -118,7 +118,7 @@ class Grabber : public QObject
 	///
 	/// @return true, on success (i.e. library is present), else false
 	///
-	virtual bool isAvailable() { return _isAvailable; }
+	virtual bool isAvailable(bool logError = true)  { return _isAvailable; }
 
 public slots:
 

include/hyperion/GrabberWrapper.h

@@ -168,9 +168,6 @@ private slots:
 	/// Will start and stop grabber based on active listeners count
 	void handleSourceRequest(hyperion::Components component, int hyperionInd, bool listen);
 
-	///
-
-
 protected:
 
 	///

libsrc/grabber/qt/QtGrabber.cpp

@@ -46,34 +46,34 @@ void QtGrabber::freeResources()
 	// Qt seems to hold the ownership of the QScreen pointers
 }
 
-bool QtGrabber::open()
+bool QtGrabber::isAvailable(bool logError)
 {
-	bool rc = false;
-
-#ifndef _WIN32
+	#ifndef _WIN32
 	if (getenv("WAYLAND_DISPLAY") != nullptr)
 	{
+		ErrorIf(logError, _log, "Grabber does not work under Wayland!");
 		_isWayland = true;
 	}
-	else
-#endif
-	{
-		rc = true;
-	}
-	return rc;
+	#endif
+
+	_isAvailable = !_isWayland;
+	return _isAvailable;
+}
+
+bool QtGrabber::open()
+{
+	return _isAvailable;
 }
 
 bool QtGrabber::setupDisplay()
 {
-	bool result = false;
-	if (!open())
+	if (!_isAvailable)
 	{
-		if (_isWayland)
-		{
-			Error(_log, "Grabber does not work under Wayland!");
-		}
+		return false;
 	}
-	else
+
+	bool result = false;
+	if (open())
 	{
 		// cleanup last screen
 		freeResources();
@@ -385,7 +385,7 @@ QJsonObject QtGrabber::discover(const QJsonObject& params)
 	DebugIf(verbose, _log, "params: [%s]", QString(QJsonDocument(params).toJson(QJsonDocument::Compact)).toUtf8().constData());
 
 	QJsonObject inputsDiscovered;
-	if (open())
+	if (isAvailable(false) && open())
 	{
 		QList<QScreen*> screens = QGuiApplication::screens();
 		if (!screens.isEmpty())

libsrc/grabber/qt/QtWrapper.cpp

@@ -17,7 +17,26 @@ QtWrapper::QtWrapper(const QJsonDocument& grabberConfig)
 				GrabberWrapper::DEFAULT_PIXELDECIMATION,
 				0,0,0,0)
 {
-	this->handleSettingsUpdate(settings::SYSTEMCAPTURE, grabberConfig);
+	_isAvailable = _grabber.isAvailable();
+	if (_isAvailable)
+	{
+		this->handleSettingsUpdate(settings::SYSTEMCAPTURE, grabberConfig);
+	}
+}
+
+bool QtWrapper::isAvailable()
+{
+	return _isAvailable;
+}
+
+bool QtWrapper::start()
+{
+	if (_isAvailable)
+	{
+		return GrabberWrapper::start();
+	}
+
+	return false;
 }
 
 bool QtWrapper::open()
@@ -27,5 +46,10 @@ bool QtWrapper::open()
 
 void QtWrapper::action()
 {
+	if (!_isAvailable)
+	{
+		return;
+	}
+
 	transferFrame(_grabber);
 }

libsrc/grabber/x11/X11Grabber.cpp

@@ -113,15 +113,25 @@ void X11Grabber::setupResources()
 	}
 }
 
-bool X11Grabber::open()
-{
-	bool rc = false;
 
+bool X11Grabber::isAvailable(bool logError)
+{
 	if (getenv("WAYLAND_DISPLAY") != nullptr)
 	{
+		ErrorIf(logError, _log, "Grabber does not work under Wayland!");
 		_isWayland = true;
 	}
-	else
+
+	_isAvailable = !_isWayland;
+	return _isAvailable;
+}
+
+
+bool X11Grabber::open()
+{
+	bool rc = false;
+
+	if (_isAvailable)
 	{
 		_x11Display = XOpenDisplay(nullptr);
 		if (_x11Display != nullptr)
@@ -134,25 +144,24 @@ bool X11Grabber::open()
 
 bool X11Grabber::setupDisplay()
 {
+	if (!_isAvailable)
+	{
+		return false;
+	}
+
 	bool result = false;
 
 	if ( ! open() )
 	{
-		if ( _isWayland  )
+		if (getenv("DISPLAY") != nullptr)
 		{
-			Error(_log, "Grabber does not work under Wayland!");
+			Error(_log, "Unable to open display [%s]",getenv("DISPLAY"));
 		}
 		else
 		{
-			if (getenv("DISPLAY") != nullptr)
-			{
-				Error(_log, "Unable to open display [%s]",getenv("DISPLAY"));
-			}
-			else
-			{
-				Error(_log, "DISPLAY environment variable not set");
-			}
+			Error(_log, "DISPLAY environment variable not set");
 		}
+
 	}
 	else
 	{
@@ -405,7 +414,7 @@ QJsonObject X11Grabber::discover(const QJsonObject& params)
 	DebugIf(verbose, _log, "params: [%s]", QString(QJsonDocument(params).toJson(QJsonDocument::Compact)).toUtf8().constData());
 
 	QJsonObject inputsDiscovered;
-	if ( open() )
+	if ( isAvailable(false) && open() )
 	{
 		inputsDiscovered["device"] = "x11";
 		inputsDiscovered["device_name"] = "X11";

libsrc/grabber/x11/X11Wrapper.cpp

@@ -15,7 +15,11 @@ X11Wrapper::X11Wrapper(const QJsonDocument& grabberConfig)
 				 GrabberWrapper::DEFAULT_PIXELDECIMATION,
 				 0,0,0,0)
 {
-	this->handleSettingsUpdate(settings::SYSTEMCAPTURE, grabberConfig);
+	_isAvailable = _grabber.isAvailable();
+	if (_isAvailable)
+	{
+		this->handleSettingsUpdate(settings::SYSTEMCAPTURE, grabberConfig);
+	}
 }
 
 X11Wrapper::~X11Wrapper()
@@ -26,22 +30,51 @@ X11Wrapper::~X11Wrapper()
 	}
 }
 
+
+bool X11Wrapper::isAvailable()
+{
+	return _isAvailable;
+}
+
+bool X11Wrapper::start()
+{
+	if (_isAvailable)
+	{
+		return GrabberWrapper::start();
+	}
+
+	return false;
+}
+
+
+bool X11Wrapper::open()
+{
+	bool isOpen {false};
+	if ( _isAvailable && _grabber.setupDisplay())
+	{
+		if (_grabber.updateScreenDimensions() >= 0)
+		{
+			isOpen = true;
+		}
+	}
+
+	return isOpen;
+}
+
 void X11Wrapper::action()
 {
+	if (!_isAvailable)
+	{
+		return;
+	}
+
 	if (! _init )
 	{
 		_init = true;
-		if ( ! _grabber.setupDisplay() )
+		if ( ! open() )
 		{
 			stop();
 		}
-		else
-		{
-			if (_grabber.updateScreenDimensions() < 0 )
-			{
-				stop();
-			}
-		}
 	}
 
 	if (isActive())