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

Lord-Grey · Lord-Grey · commit b14b655c7742 · 2025-05-20T22:30:33.000+02:00
diff --git a/CHANGELOG.md b/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.
diff --git a/include/grabber/dispmanx/DispmanxFrameGrabber.h b/include/grabber/dispmanx/DispmanxFrameGrabber.h
@@ -38,7 +38,7 @@ class DispmanxFrameGrabber : public Grabber
 	///
 	/// @return true, on success (i.e. library is present), else false
 	///
-	bool isAvailable() override;
+	bool isAvailable(bool logError = true) override;
 
 	///
 	/// @brief Opens the input device.
diff --git a/include/grabber/qt/QtGrabber.h b/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.
 	///
diff --git a/include/grabber/qt/QtWrapper.h b/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
 	///
diff --git a/include/grabber/x11/X11Grabber.h b/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();
 
 	///
diff --git a/include/grabber/x11/X11Wrapper.h b/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
diff --git a/include/grabber/xcb/XcbGrabber.h b/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();
 
diff --git a/include/grabber/xcb/XcbWrapper.h b/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;
 
diff --git a/include/hyperion/Grabber.h b/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:
 
diff --git a/include/hyperion/GrabberWrapper.h b/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:
 
 	///
diff --git a/libsrc/grabber/dispmanx/DispmanxFrameGrabber.cpp b/libsrc/grabber/dispmanx/DispmanxFrameGrabber.cpp
@@ -31,7 +31,7 @@ DispmanxFrameGrabber::DispmanxFrameGrabber()
 	_useImageResampler = true;
 }
 
-bool DispmanxFrameGrabber::isAvailable()
+bool DispmanxFrameGrabber::isAvailable(bool logError)
 {
 #ifdef BCM_FOUND
 	void* bcm_host = dlopen(std::string("" BCM_LIBRARY).c_str(), RTLD_LAZY | RTLD_GLOBAL);
diff --git a/libsrc/grabber/qt/QtGrabber.cpp b/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())
diff --git a/libsrc/grabber/qt/QtWrapper.cpp b/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);
 }
diff --git a/libsrc/grabber/x11/X11Grabber.cpp b/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";
diff --git a/libsrc/grabber/x11/X11Wrapper.cpp b/libsrc/grabber/x11/X11Wrapper.cpp
diff --git a/libsrc/grabber/xcb/XcbGrabber.cpp b/libsrc/grabber/xcb/XcbGrabber.cpp
diff --git a/libsrc/grabber/xcb/XcbWrapper.cpp b/libsrc/grabber/xcb/XcbWrapper.cpp
diff --git a/src/hyperion-qt/QtWrapper.cpp b/src/hyperion-qt/QtWrapper.cpp
diff --git a/src/hyperion-x11/X11Wrapper.cpp b/src/hyperion-x11/X11Wrapper.cpp
diff --git a/src/hyperion-xcb/XcbWrapper.cpp b/src/hyperion-xcb/XcbWrapper.cpp
