EffectEngine: Refactoring, stabalizing and fixes

CHANGELOG.md

@@ -13,20 +13,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### ✨ Added
 
 - V4L2/ImageResampler: add support for pixelformats YUV422P and NV21
+- New Jugger Effect
 ---
 
 ### 🔧 Changed
 
 - **Fixes:**
-- Windows DDA Grabber - Prevent image updates when mouse is moved. Provide a Warning on incomptible setting. (#2002)
+  - Windows DDA Grabber - Prevent image updates when mouse is moved. Provide a Warning on incompatible setting. (#2002)
+  - EffectModule - Reference Counting (Use-After-Free) bugs (#2010) - _Thanks to @wr-web_
+  - EffectEngine - Follow-up Python C-API null-pointer and stability fixes (#2011)
+  - EffectFileHandler - Path traversal vulnerability when saving user-defined effects (#2011)
+  - Effect scripts: Minor stability and style fixes in `pacman.py`, `traces.py`, `trails.py`(#2011)
 ---
-
-### 🗑️ Removed
-
----
-
 ### Technical
 
+- EffectEngine: Refactor Python C-extension module to reduce nesting depth and cognitive complexity.
+- EffectModule - Refactor and stablising
+- EffectFileHandler: Refactor effect file management
+- EffectEngine: Added dedicated `hyperion.effect` debug logging category
+- Empty image consistency applied. 0×0 is now the canonical empty image; 1×1 is no longer treated as empty
+
 ## [2.2.1](https://github.com/hyperion-project/hyperion.ng/releases/tag/2.2.1) - 2026-04-06
 
 ### ✨ Added

assets/webconfig/i18n/en.json

@@ -832,6 +832,8 @@
   "edt_eff_url": "Image adress",
   "edt_eff_initial_blink": "Flash for attention",
   "edt_eff_interval": "Interval",
+  "edt_eff_juggler_header": "Juggler",
+  "edt_eff_juggler_header_desc": "Colored balls are juggled accross the screen",
   "edt_eff_knightrider_header": "Knight Rider",
   "edt_eff_knightrider_header_desc": "K.I.T.T is back! The front-scanner of the well-known car, this time not just in red.",
   "edt_eff_ledlist": "LED List",

effects/collision.py

@@ -6,9 +6,14 @@
 explodeRadius = int(hyperion.args.get('explodeRadius', 8))
 
 # Ensure that the range for pixel indices stays within bounds
-maxPixelIndex = hyperion.ledCount - 1
-if trailLength > maxPixelIndex or explodeRadius > maxPixelIndex:
-    exit(f"Error: Color length ({trailLength}) and detonation range ({explodeRadius}) must be less than number of LEDs configured ({hyperion.ledCount})")
+# trailLength and explodeRadius must each be less than hyperion.ledCount
+minLedCount = max(trailLength, explodeRadius) + 1
+if hyperion.ledCount < minLedCount:
+    raise SystemExit(
+        f"Collision requires at least {minLedCount} LEDs (configured: {hyperion.ledCount}). "
+        f"'trailLength' must be < ledCount (currently {trailLength}, max allowed {hyperion.ledCount - 1}), "
+        f"'explodeRadius' must be < ledCount (currently {explodeRadius}, max allowed {hyperion.ledCount - 1})."
+    )
 
 # Create additional variables
 increment = None

effects/juggler.json

@@ -0,0 +1,8 @@
+{
+    "name" : "Juggler",
+    "script" : "juggler.py",
+    "args" :
+    {
+        "brightness" : 100
+    }
+}

effects/juggler.py

@@ -0,0 +1,78 @@
+import hyperion, time, math, colorsys
+
+# Parameter
+brightness = float(hyperion.args.get('brightness', 100)) / 100.0
+
+# Image setup
+hyperion.imageMinSize(64, 64)
+iW = hyperion.imageWidth()
+iH = hyperion.imageHeight()
+
+sleepTime = max(hyperion.lowestUpdateInterval(), 1.0 / 60.0)
+
+# Buffer
+buf = [[[0, 0, 0] for _ in range(iH)] for _ in range(iW)]
+
+# Constants
+numDots = 8
+dotR = max(2, min(iW, iH) // 20)
+dotRSq = dotR * dotR
+
+# Precompute the circular dot mask
+dotMask = []
+for ox in range(-dotR, dotR + 1):
+	for oy in range(-dotR, dotR + 1):
+		distSq = ox * ox + oy * oy
+		if distSq <= dotRSq:
+			dist = math.sqrt(distSq)
+			intensity = max(0, 255 - int(dist / dotR * 200))
+			dotMask.append((ox, oy, intensity))
+
+# Main loop
+startTime = time.time()
+while not hyperion.abort():
+	tMs = (time.time() - startTime) * 1000.0
+
+	# Fade buffer
+	scale = max(0.0, 1.0 - 25 / 256.0)
+	for x in range(iW):
+		for y in range(iH):
+			buf[x][y][0] = int(buf[x][y][0] * scale)
+			buf[x][y][1] = int(buf[x][y][1] * scale)
+			buf[x][y][2] = int(buf[x][y][2] * scale)
+
+	# Draw moving dots
+	for i in range(numDots):
+		angleX = (tMs / 60000.0) * (i + 5) * 2.0 * math.pi + i * 1.3
+		angleY = (tMs / 60000.0) * (i + 3) * 2.0 * math.pi + i * 0.9 + math.pi
+		dx = int((math.sin(angleX) + 1.0) * 0.5 * (iW - 1))
+		dy = int((math.sin(angleY) + 1.0) * 0.5 * (iH - 1))
+
+		r, g, b = colorsys.hsv_to_rgb(((i * 32) & 255) / 255.0, 200 / 255.0, 255 / 255.0)
+		r = int(r * 255)
+		g = int(g * 255)
+		b = int(b * 255)
+
+		for ox, oy, intensity in dotMask:
+			x = dx + ox
+			y = dy + oy
+			if 0 <= x < iW and 0 <= y < iH:
+				pr = r * intensity // 255
+				pg = g * intensity // 255
+				pb = b * intensity // 255
+				buf[x][y][0] = max(buf[x][y][0], pr)
+				buf[x][y][1] = max(buf[x][y][1], pg)
+				buf[x][y][2] = max(buf[x][y][2], pb)
+
+	# Push buffer to Hyperion image
+	for x in range(iW):
+		for y in range(iH):
+			r = int(buf[x][y][0] * brightness)
+			g = int(buf[x][y][1] * brightness)
+			b = int(buf[x][y][2] * brightness)
+			hyperion.imageSetPixel(x, y, r, g, b)
+
+	hyperion.imageShow()
+	hyperion.imageStackClear()
+
+	time.sleep(sleepTime)

effects/pacman.py

@@ -28,6 +28,12 @@
 posSlowGuy = posBlueGuy + diffGuys
 posRedGuy = posSlowGuy + diffGuys
 
+# minimum LEDs needed: randint(10, ledCount) requires ledCount >= 10,
+# and posRedGuy + 1 LEDs are needed to show all characters
+minLedCount = max(10, posRedGuy + 1)
+if hyperion.ledCount < minLedCount:
+	raise SystemExit(f"Pac-Man requires at least {minLedCount} LEDs (configured: {hyperion.ledCount}). Increase LED count or reduce 'margin-pos'.")
+
 # initialize the led data
 ledDataEscape = bytearray()
 for i in range(hyperion.ledCount):
@@ -56,19 +62,19 @@
 # increment = 3, because LED-Color is defined by 3 Bytes
 increment = 3
 
-def shiftLED(ledData, increment, limit, lightPos=None):
+def shift_led(led_data, increment, limit, light_pos=None):
 	state = 0
 	while state < limit and not hyperion.abort():
-		ledData = ledData[increment:] + ledData[:increment]
+		led_data = led_data[increment:] + led_data[:increment]
 
-		if (lightPos):
-			tmp = ledData[lightPos]
-			ledData[lightPos] = light
+		if (light_pos):
+			tmp = led_data[light_pos]
+			led_data[light_pos] = light
 
-		hyperion.setColor(ledData)
+		hyperion.setColor(led_data)
 
-		if (lightPos):
-			ledData[lightPos] = tmp
+		if (light_pos):
+			led_data[light_pos] = tmp
 
 		time.sleep(sleepTime)
 		state += 1
@@ -78,17 +84,17 @@ def shiftLED(ledData, increment, limit, lightPos=None):
 
 	# escape mode
 	ledData = ledDataEscape
-	shiftLED(ledData, increment, hyperion.ledCount)
+	shift_led(ledData, increment, hyperion.ledCount)
 
 	random = randint(10,hyperion.ledCount)
 
 	# escape mode + power pellet
 	s = slice(3*random, 3*random+3)
-	shiftLED(ledData, increment, hyperion.ledCount - random, s)
+	shift_led(ledData, increment, hyperion.ledCount - random, s)
 
 	# chase mode
 	shift   = 3*(hyperion.ledCount - random)
 	ledData = ledDataChase[shift:]+ledDataChase[:shift]
-	shiftLED(ledData, -increment, 2*hyperion.ledCount-random)
+	shift_led(ledData, -increment, 2*hyperion.ledCount-random)
 	time.sleep(sleepTime)
 

effects/schema/juggler.schema.json

@@ -0,0 +1,18 @@
+{
+	"type": "object",
+	"script": "juggler.py",
+	"title": "edt_eff_juggler_header",
+	"required": true,
+	"properties": {
+		"brightness": {
+			"type": "number",
+			"title": "edt_eff_brightness",
+			"default": 100,
+			"minimum": 1,
+			"maximum": 100,
+			"append": "edt_append_percent",
+			"propertyOrder": 1
+		}
+	},
+	"additionalProperties": false
+}

effects/snake.py

@@ -29,7 +29,8 @@ def lerp(a, b, t):
     return (a[0]*(1-t)+b[0]*t, a[1]*(1-t)+b[1]*t, a[2]*(1-t)+b[2]*t)
 
 for i in range(0, snakeLeds):
-	hsv = lerp(color_hsv, backgroundColor_hsv, 1.0/(snakeLeds-1)*i)
+	t = 0.0 if snakeLeds == 1 else (1.0 / (snakeLeds - 1) * i)
+	hsv = lerp(color_hsv, backgroundColor_hsv, t)
 	rgb = colorsys.hsv_to_rgb(hsv[0], hsv[1], hsv[2])
 	ledData += bytearray((int(rgb[0]*255), int(rgb[1]*255), int(rgb[2]*255)))
 

effects/traces.py

@@ -1,5 +1,9 @@
 import hyperion, time, random, math
 
+# Traces requires at least 2 LEDs so runners can advance positions and ledData is non-empty
+if hyperion.ledCount < 2:
+	raise SystemExit(f"Traces requires at least 2 LEDs (configured: {hyperion.ledCount}). Increase LED count.")
+
 # Initialize the led data
 ledData = bytearray()
 for i in range(hyperion.ledCount):
@@ -27,7 +31,6 @@
 while not hyperion.abort():
 	for r in runners:
 		if r["c"] == 0:
-			#ledData[r["pos"]*3+r["i"]] = 0
 			r["c"] = r["step"]
 			r["pos"] = (r["pos"]+1)%hyperion.ledCount
 			ledData[r["pos"]*3+r["i"]] = int(r["lvl"]*(0.2+0.8*random.random()))

effects/trails.py

@@ -5,16 +5,15 @@
 
 min_len = int(hyperion.args.get('min_len', 3))
 max_len = int(hyperion.args.get('max_len', 3))
-#iHeight = int(hyperion.args.get('iHeight', 8))
 trails = int(hyperion.args.get('int', 8))
 sleepTime = float(hyperion.args.get('speed', 1)) / 1000.0
 color = list(hyperion.args.get('color', (255,255,255)))
 randomise = bool(hyperion.args.get('random', False))
 iWidth = hyperion.imageWidth()
-iHeight = hyperion.imageHeight()
-
-# Limit update rate
-sleepTime = max(hyperion.lowestUpdateInterval(), sleepTime)
+iHeight = hyperion.imageHeight()
+
+# Limit update rate
+sleepTime = max(hyperion.lowestUpdateInterval(), sleepTime)
 
 class trail:
 	def __init__(self):
@@ -28,14 +27,14 @@ def start(self, x, y, step, color, _len, _h):
 		self.data = []
 		brigtness = color[2]
 		step_brigtness = color[2] / _len
-		for i in range(0, _len):
+		for _ in range(0, _len):
 			rgb = colorsys.hsv_to_rgb(color[0], color[1], brigtness)
 			self.data.insert(0, (int(255*rgb[0]), int(255*rgb[1]), int(255*rgb[2])))
 			brigtness -= step_brigtness
 
 		self.data.extend([(0,0,0)]*(_h-y))
 		if len(self.data) < _h:
-			for i in range (_h-len(self.data)):
+			for _ in range (_h-len(self.data)):
 				self.data.insert(0, (0,0,0))
 
 	def getdata(self):

include/effectengine/Effect.h

@@ -12,6 +12,7 @@
 #include <utils/Image.h>
 
 #include <atomic>
+#include <QScopedPointer>
 
 class Hyperion;
 class Logger;
@@ -42,7 +43,7 @@ class Effect : public QThread
 	/// @brief Set manual interruption to true,
 	///        Note: DO NOT USE QThread::interruption!
 	///
-	void requestInterruption() { _interupt = true; }
+	void requestInterruption() { _interupt = true; } // NOSONAR - QThread::requestInterruption is not used to avoid confusion with the effect's own interruption mechanism
 
 	///
 	/// @brief Check an interruption was requested.
@@ -51,7 +52,7 @@ class Effect : public QThread
 	///
 	/// @return    The flag state
 	///
-	bool isInterruptionRequested();
+	bool isInterruptionRequested() const; // NOSONAR - QThread::requestInterruption is not used to avoid confusion with the effect's own interruption mechanism
 
 	///
 	/// @brief Get the remaining timeout, or indication it is endless
@@ -110,7 +111,7 @@ public slots:
 
 	QSize           _imageSize;
 	QImage          _image;
-	QPainter*       _painter;
+	QScopedPointer<QPainter> _painter;
 	QVector<QImage> _imageStack;
 
 	double	_lowestUpdateIntervalInSeconds;

include/effectengine/EffectDefinition.h

@@ -4,9 +4,15 @@
 #include <QString>
 #include <QJsonObject>
 
+#include <QLoggingCategory>
+
+Q_DECLARE_LOGGING_CATEGORY(effect);
+
 struct EffectDefinition
 {
-	QString name, script, file;
+	QString name;
+	QString script;
+	QString file;
 	QJsonObject args;
 	unsigned smoothCfg;
 };

include/effectengine/EffectFileHandler.h

@@ -1,5 +1,12 @@
 #pragma once
 
+#include <QDir>
+#include <QFileInfo>
+#include <QJsonArray>
+#include <QMap>
+#include <QSharedPointer>
+#include <QWeakPointer>
+
 // util
 #include <utils/Logger.h>
 #include <effectengine/EffectDefinition.h>
@@ -82,6 +89,36 @@ public slots:
 	///
 	bool loadEffectSchema(const QString& path, const QString& effectSchemaFile, EffectSchema& effectSchema);
 
+	///
+	/// @brief Load all effect definitions from a directory, called by updateEffects()
+	///
+	void loadEffectsFromDirectory(const QString& path, const QDir& directory, const QStringList& disableList, QMap<QString, EffectDefinition>& availableEffects);
+
+	///
+	/// @brief Load all effect schemas from a directory, called by updateEffects()
+	///
+	void loadSchemasFromDirectory(const QString& path, QDir& directory);
+
+	///
+	/// @brief Resolve the file path for a new/updated effect and write it, called by saveEffect()
+	///
+	QString resolveEffectFilePath(const QJsonObject& message, const QString& effectName, const QJsonArray& effectArray, QJsonObject& effectJson);
+
+	///
+	/// @brief Save an embedded image for a gif effect, called by saveEffect()
+	///
+	QString saveEffectImage(const QJsonObject& message, const QJsonArray& effectArray, QJsonObject& effectJson);
+
+	///
+	/// @brief Remove the unused imageSource key (url/file) from the effect args
+	///
+	void cleanImageSource(const QJsonObject& message, QJsonObject& effectJson) const;
+
+	///
+	/// @brief Remove effect configuration and associated image files
+	///
+	QString removeEffectFiles(const EffectDefinition& effect, const QFileInfo& effectConfigurationFile);
+
 private:
  	QJsonObject _effectConfig;
 	QSharedPointer<Logger> _log;