linux-capture: XSelectInput tracking improvement

Track all windows corresponding to sources and ensure that we only
disable XSelectInput events once all sources for a given window have
been removed. Previously we may have stopped listening for events if
multiple sources captured the same window and one was removed.

We also move window redirection into the helper to avoid similar issues.

plugins/linux-capture/xcompcap-helper.cpp

@@ -4,6 +4,7 @@
 #include <X11/extensions/Xcomposite.h>
 
 #include <unordered_set>
+#include <map>
 #include <pthread.h>
 
 #include <obs-module.h>
@@ -215,8 +216,73 @@ int getWindowPid(Window win)
 	return 1234; //TODO
 }
 
-static std::unordered_set<Window> changedWindows;
+static std::map<XCompcapMain *, Window> windowForSource;
+static std::unordered_set<XCompcapMain *> changedSources;
 static pthread_mutex_t changeLock = PTHREAD_MUTEX_INITIALIZER;
+
+void registerSource(XCompcapMain *source, Window win)
+{
+	PLock lock(&changeLock);
+
+	blog(LOG_DEBUG, "registerSource(source=%p, win=%ld)", source, win);
+
+	auto it = windowForSource.find(source);
+
+	if (it != windowForSource.end()) {
+		windowForSource.erase(it);
+	}
+
+	// Subscribe to Events
+	XSelectInput(disp(), win,
+		     StructureNotifyMask | ExposureMask | VisibilityChangeMask);
+	XCompositeRedirectWindow(disp(), win, CompositeRedirectAutomatic);
+	XSync(disp(), 0);
+
+	windowForSource.insert(std::make_pair(source, win));
+}
+
+void unregisterSource(XCompcapMain *source)
+{
+	PLock lock(&changeLock);
+
+	blog(LOG_DEBUG, "unregisterSource(source=%p)", source);
+	{
+		auto it = windowForSource.find(source);
+		Window win = it->second;
+
+		if (it != windowForSource.end()) {
+			windowForSource.erase(it);
+		}
+
+		// check if there are still sources listening for the same window
+		it = windowForSource.begin();
+		bool windowInUse = false;
+		while (it != windowForSource.end()) {
+			if (it->second == win) {
+				windowInUse = true;
+				break;
+			}
+			it++;
+		}
+
+		if (!windowInUse) {
+			// Last source released, stop listening for events.
+			XSelectInput(disp(), win, 0);
+			XCompositeUnredirectWindow(disp(), win,
+						   CompositeRedirectAutomatic);
+			XSync(disp(), 0);
+		}
+	}
+
+	{
+		auto it = changedSources.find(source);
+
+		if (it != changedSources.end()) {
+			changedSources.erase(it);
+		}
+	}
+}
+
 void processEvents()
 {
 	PLock lock(&changeLock);
@@ -225,36 +291,56 @@ void processEvents()
 
 	while (XEventsQueued(disp(), QueuedAfterReading) > 0) {
 		XEvent ev;
+		Window win = 0;
 
 		XNextEvent(disp(), &ev);
 
 		if (ev.type == ConfigureNotify)
-			changedWindows.insert(ev.xconfigure.event);
+			win = ev.xconfigure.event;
+
+		else if (ev.type == MapNotify)
+			win = ev.xmap.event;
 
-		if (ev.type == MapNotify)
-			changedWindows.insert(ev.xmap.event);
+		else if (ev.type == Expose)
+			win = ev.xexpose.window;
 
-		if (ev.type == Expose)
-			changedWindows.insert(ev.xexpose.window);
+		else if (ev.type == VisibilityNotify)
+			win = ev.xvisibility.window;
 
-		if (ev.type == VisibilityNotify)
-			changedWindows.insert(ev.xvisibility.window);
+		else if (ev.type == DestroyNotify)
+			win = ev.xdestroywindow.event;
 
-		if (ev.type == DestroyNotify)
-			changedWindows.insert(ev.xdestroywindow.event);
+		if (win != 0) {
+			blog(LOG_DEBUG, "processEvents(): windowChanged=%ld",
+			     win);
+
+			auto it = windowForSource.begin();
+
+			while (it != windowForSource.end()) {
+				if (it->second == win) {
+					blog(LOG_DEBUG,
+					     "processEvents(): sourceChanged=%p",
+					     it->first);
+					changedSources.insert(it->first);
+				}
+				it++;
+			}
+		}
 	}
 
 	XUnlockDisplay(disp());
 }
 
-bool windowWasReconfigured(Window win)
+bool sourceWasReconfigured(XCompcapMain *source)
 {
 	PLock lock(&changeLock);
 
-	auto it = changedWindows.find(win);
+	auto it = changedSources.find(source);
 
-	if (it != changedWindows.end()) {
-		changedWindows.erase(it);
+	if (it != changedSources.end()) {
+		changedSources.erase(it);
+		blog(LOG_DEBUG, "sourceWasReconfigured(source=%p)=true",
+		     source);
 		return true;
 	}
 

plugins/linux-capture/xcompcap-helper.hpp

@@ -70,6 +70,8 @@ public:
 	~ObsGsContextHolder();
 };
 
+class XCompcapMain;
+
 namespace XCompcap {
 Display *disp();
 void cleanupDisplay();
@@ -92,6 +94,8 @@ inline std::string getWindowClass(Window win)
 	return getWindowAtom(win, "WM_CLASS");
 }
 
+void registerSource(XCompcapMain *source, Window win);
+void unregisterSource(XCompcapMain *source);
 void processEvents();
-bool windowWasReconfigured(Window win);
+bool sourceWasReconfigured(XCompcapMain *source);
 }

plugins/linux-capture/xcompcap-main.cpp

@@ -189,6 +189,8 @@ XCompcapMain::~XCompcapMain()
 {
 	ObsGsContextHolder obsctx;
 
+	XCompcap::unregisterSource(this);
+
 	if (p->tex) {
 		gs_texture_destroy(p->tex);
 		p->tex = 0;
@@ -237,9 +239,6 @@ static Window getWindowFromString(std::string wstr)
 	for (Window cwin : XCompcap::getTopLevelWindows()) {
 		// match by window-id
 		if (cwin == winById) {
-			blog(LOG_INFO, "Found Window '%s' by Window-ID %s",
-			     wname.c_str(), wid.c_str());
-
 			return cwin;
 		}
 	}
@@ -251,9 +250,6 @@ static Window getWindowFromString(std::string wstr)
 
 		// match by name and class
 		if (wname == cwinname && wcls == ccls) {
-			blog(LOG_INFO, "Found Window '%s' by Name & Class",
-			     wname.c_str());
-
 			return cwin;
 		}
 	}
@@ -305,9 +301,6 @@ static void xcc_cleanup(XCompcapMain_private *p)
 	}
 
 	if (p->win) {
-		XCompositeUnredirectWindow(xdisp, p->win,
-					   CompositeRedirectAutomatic);
-		XSelectInput(xdisp, p->win, 0);
 		p->win = 0;
 	}
 
@@ -365,6 +358,7 @@ void XCompcapMain::updateSettings(obs_data_t *settings)
 {
 	PLock lock(&p->lock);
 	ObsGsContextHolder obsctx;
+	XErrorLock xlock;
 
 	blog(LOG_DEBUG, "Settings updating");
 
@@ -373,11 +367,13 @@ void XCompcapMain::updateSettings(obs_data_t *settings)
 	xcc_cleanup(p);
 
 	if (settings) {
+		/* Settings initialized or changed */
 		const char *windowName =
 			obs_data_get_string(settings, "capture_window");
 
 		p->windowName = windowName;
 		p->win = getWindowFromString(windowName);
+		XCompcap::registerSource(this, p->win);
 
 		p->cut_top = obs_data_get_int(settings, "cut_top");
 		p->cut_left = obs_data_get_int(settings, "cut_left");
@@ -391,23 +387,15 @@ void XCompcapMain::updateSettings(obs_data_t *settings)
 		p->exclude_alpha = obs_data_get_bool(settings, "exclude_alpha");
 		p->draw_opaque = false;
 	} else {
+		/* New Window found (stored in p->win), just re-initialize GL-Mapping  */
 		p->win = prevWin;
 	}
 
-	XErrorLock xlock;
-	if (p->win)
-		XCompositeRedirectWindow(xdisp, p->win,
-					 CompositeRedirectAutomatic);
 	if (xlock.gotError()) {
-		blog(LOG_ERROR, "XCompositeRedirectWindow failed: %s",
+		blog(LOG_ERROR, "registeringSource failed: %s",
 		     xlock.getErrorText().c_str());
 		return;
 	}
-
-	if (p->win)
-		XSelectInput(xdisp, p->win,
-			     StructureNotifyMask | ExposureMask |
-				     VisibilityChangeMask);
 	XSync(xdisp, 0);
 
 	XWindowAttributes attr;
@@ -592,7 +580,7 @@ void XCompcapMain::tick(float seconds)
 
 	XCompcap::processEvents();
 
-	if (p->win && XCompcap::windowWasReconfigured(p->win)) {
+	if (p->win && XCompcap::sourceWasReconfigured(this)) {
 		p->window_check_time = FIND_WINDOW_INTERVAL;
 		p->win = 0;
 	}
@@ -612,6 +600,7 @@ void XCompcapMain::tick(float seconds)
 
 		if (newWin && XGetWindowAttributes(xdisp, newWin, &attr)) {
 			p->win = newWin;
+			XCompcap::registerSource(this, p->win);
 			updateSettings(0);
 		} else {
 			return;