fix(vlc): converge VLC video placement below the ticker strip (v0.87.2) (#200)

v0.87.1's single-shot wmctrl correction mis-landed on the live karaoke
window (y=160 instead of 80): VLC nudges/resizes its own window while a 4K
file loads, so the one 0.4s-later measurement read a stale position and
the lone correction was wrong. Replace it with a short closed loop —
request, settle, measure, adjust the next request by the observed error —
that converges to (0, margin) within a 2px tolerance, and soft-warns
(never falsely claims success) if it can't settle. Validated end-to-end on
NomadPC: VLC video settles at 1920x1000+0+80, matching mpv.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: beveradb

docs/ARCHITECTURE.md

@@ -298,7 +298,7 @@ The overlay system uses a three-component architecture: (1) the KJ Controller we
 
 **Layers:** `desktop/rotation_source.py` (pure stdlib) parses `/tmp/rotation_cache.json` into structured data; `desktop/overlay_painters.py` (pure pycairo, no GTK — headless-testable) holds one painter per overlay type; `desktop/overlay_engine.py` (the only `gi`/GTK module) owns the window + render loop and a gi-free `--render-png` mode for headless/on-device visual checks. Communication with the Flask backend is via `data/overlays.json` polled by mtime every ~1 second. Six overlay types are supported: `rotation_list` (the between-songs home screen — heading, stats, singer list with status badges/paid hearts, page cycling), `ticker` (scrolling bar; `source='rotation'` composes the "up next" text directly from the rotation cache; loops seamlessly — a configurable `loop_separator` glyph, default `♪`, is appended to form a repeating unit that tiles back-to-back so there's no blank gap after the last singer), `static_text`, `image`, `countdown`, and `qr_code`. Each overlay has an independent `show_over_video` flag — when false it is hidden during karaoke playback (e.g. the rotation list) and shown when playback stops. The `karaoke_playing` state is set by the play/control routes and a `MpvManager.on_karaoke_end` callback.
 
-**Partial redraw + reserved top strip (4K frame-drop fix, 2026-07):** the render loop invalidates only each **animated overlay's own bounding box** (`queue_draw_area(*painter.bbox())`), not the whole window. Previously every ticker frame called `queue_draw()`, forcing the compositor to re-blend the entire screen — including the 4K video region — 30×/s, which cost measurable frame drops on the N97 iGPU. Complementing this, the karaoke video is rendered **below a reserved top strip** (`video_top_margin_px`, default 80): `mpv_manager` launches mpv borderless at `--geometry=<W>x<H-margin>+0+<margin>` (was `--fs`; margin 0 restores fullscreen for rollback), so a top-strip ticker's damage rect never overlaps the video window and the compositor stops re-blending video pixels for the ticker entirely. This is a **persisted cross-process contract, not runtime IPC**: kj-controller writes `video_top_margin_px` into `overlays.json` (`OverlayManager.set_video_top_margin`, at startup) and sizes the video from the same `config.py` value; the overlay engine reads that strip height back out (`load_config` injects it into each ticker's config as `_strip_h`) and a `position:'top'` ticker sizes its bar to **fill** the strip — so there's no wallpaper gap between the ticker and the video, and the ticker's damage rect is exactly the reserved strip. Both sides independently read one persisted setting; `video_top_margin_px` is the single source of truth for both the video geometry and the ticker height, so they can't drift. VLC honours the same strip (device-validated 2026-07): it launches windowed and — because it maps its video window only once a song plays and ignores its own geometry CLI flags — `VlcKaraokePlayer._position_window` places the window **per-play** with `wmctrl`, matched by the unambiguous `VLC media player` title (the filler VLC is audio-only with no window; VLC leaves `_NET_WM_PID` unset, so the title is the only key). It corrects xfwm4's fixed frame-extent offset by measuring where the window lands after the first move and re-requesting `2×target − actual`. `margin_px <= 0` restores fullscreen for both engines (clean rollback). mpv remains the default and the only engine that can *hardware-decode* 4K on this box — VLC 3.0's VAAPI decoder never engages, so VLC always software-decodes (fine at 1080p, glitchy at 4K).
+**Partial redraw + reserved top strip (4K frame-drop fix, 2026-07):** the render loop invalidates only each **animated overlay's own bounding box** (`queue_draw_area(*painter.bbox())`), not the whole window. Previously every ticker frame called `queue_draw()`, forcing the compositor to re-blend the entire screen — including the 4K video region — 30×/s, which cost measurable frame drops on the N97 iGPU. Complementing this, the karaoke video is rendered **below a reserved top strip** (`video_top_margin_px`, default 80): `mpv_manager` launches mpv borderless at `--geometry=<W>x<H-margin>+0+<margin>` (was `--fs`; margin 0 restores fullscreen for rollback), so a top-strip ticker's damage rect never overlaps the video window and the compositor stops re-blending video pixels for the ticker entirely. This is a **persisted cross-process contract, not runtime IPC**: kj-controller writes `video_top_margin_px` into `overlays.json` (`OverlayManager.set_video_top_margin`, at startup) and sizes the video from the same `config.py` value; the overlay engine reads that strip height back out (`load_config` injects it into each ticker's config as `_strip_h`) and a `position:'top'` ticker sizes its bar to **fill** the strip — so there's no wallpaper gap between the ticker and the video, and the ticker's damage rect is exactly the reserved strip. Both sides independently read one persisted setting; `video_top_margin_px` is the single source of truth for both the video geometry and the ticker height, so they can't drift. VLC honours the same strip (device-validated 2026-07): it launches windowed and — because it maps its video window only once a song plays and ignores its own geometry CLI flags — `VlcKaraokePlayer._position_window` places the window **per-play** with `wmctrl`, matched by the unambiguous `VLC media player` title (the filler VLC is audio-only with no window; VLC leaves `_NET_WM_PID` unset, so the title is the only key). It drives a short **closed loop** (request → settle → measure → adjust the next request by the observed error) until the window lands within 2px of target — needed because xfwm4 offsets wmctrl moves by a fixed frame-extent amount *and* VLC nudges/resizes its own window while a 4K file loads, so a single measure-and-correct can act on a stale position. `margin_px <= 0` restores fullscreen for both engines (clean rollback). mpv remains the default and the only engine that can *hardware-decode* 4K on this box — VLC 3.0's VAAPI decoder never engages, so VLC always software-decodes (fine at 1080p, glitchy at 4K).
 
 ### Singer Rotation System
 

docs/CHANGELOG.md

@@ -2,17 +2,21 @@
 
 Device configuration changes. For Pi details, see [archive/NOMADPI-DETAILS.md](archive/NOMADPI-DETAILS.md). For mini PC setup, see [MINIPC-SETUP.md](MINIPC-SETUP.md).
 
-## 2026-07-17 - Fix: VLC video no longer covered by the rotation ticker (v0.87.1)
+## 2026-07-17 - Fix: VLC video no longer covered by the rotation ticker (v0.87.2)
 
 The VLC karaoke renderer now honours the reserved top ticker strip
 (`video_top_margin_px`, default 80) just like mpv — previously VLC launched
 fullscreen and the ticker composited over the top of its video, while mpv
 rendered below the strip. VLC ignores its own geometry CLI flags and maps its
 video window only when a song starts, so `VlcKaraokePlayer._position_window`
 now places the window **per-play** with `wmctrl` (matched by the `VLC media
-player` window title), correcting xfwm4's fixed frame-extent offset by
-re-requesting `2×target − actual`. Placement validated on NomadPC by
-screenshot: VLC video sits at `1920×1000+0+80`, matching mpv.
+player` window title). Because xfwm4 offsets wmctrl moves by a fixed
+frame-extent amount *and* VLC nudges its own window while a 4K file loads, it
+drives a short closed loop (request → settle → measure → adjust by the observed
+error) until the window lands within 2px of target. Validated end-to-end on
+NomadPC: VLC video settles at `1920×1000+0+80`, matching mpv. (v0.87.1 shipped
+the per-play placement but a single-shot correction mis-landed on the live 4K
+window at `y=160`; v0.87.2 replaced it with the convergence loop.)
 
 Investigation context (no code change): confirmed **VLC 3.0.20 hardware-decodes
 nothing via VAAPI** on NomadPC — its decoder module declines every codec, so

kj-controller/pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "kj-controller"
-version = "0.87.1"
+version = "0.87.2"
 description = "Web-based karaoke show management with mpv + VLC playback"
 requires-python = ">=3.11"
 

kj-controller/tests/unit/test_vlc_karaoke_player.py

@@ -447,20 +447,42 @@ def test_play_skips_positioning_without_margin(mock_config, mocker, tmp_path):
 def test_position_window_self_corrects_wm_offset(mock_config, mocker):
     filler = FillerVLC(mock_config, enabled=False)
     p = VlcKaraokePlayer(mock_config, filler, enabled=True)
-    # Window found immediately; after the first move it lands offset at (4, 152)
-    # (xfwm4's fixed frame-extent skew).
+    # Poll finds the window; the first move to (0,80) lands offset at (4,152)
+    # (xfwm4's fixed frame-extent skew); the corrected request then lands on
+    # target so the loop stops.
     mocker.patch.object(p, '_find_window',
-                        side_effect=[('0xwin', 0, 0), ('0xwin', 4, 152)])
+                        side_effect=[('0xwin', 0, 0),      # poll: found
+                                     ('0xwin', 4, 152),    # after req (0,80)
+                                     ('0xwin', 0, 80)])    # after req (-4,8): done
     wmctrl = mocker.patch.object(p, '_wmctrl',
                                  return_value=mocker.Mock(returncode=0))
     mocker.patch('vlc.subprocess.run')  # xprop decoration removal
     mocker.patch('vlc.time.sleep')
+    logs = mocker.patch('vlc.log_message')
     p._position_window(80, 1920, 1080)
     moves = [c.args[0] for c in wmctrl.call_args_list]
-    # First move requests the raw target; the correction requests 2*target-actual
-    # so the window lands exactly at (0, 80).
+    # First request is the raw target; the next adjusts by the observed error
+    # (req += target - actual) so the window lands exactly at (0, 80).
     assert ['-i', '-r', '0xwin', '-e', '0,0,80,1920,1000'] in moves
     assert ['-i', '-r', '0xwin', '-e', '0,-4,8,1920,1000'] in moves
+    msgs = ' '.join(str(c.args[0]) for c in logs.call_args_list)
+    assert 'Positioned karaoke VLC' in msgs
+
+
+def test_position_window_soft_warns_when_never_settles(mock_config, mocker):
+    # If the window never reaches target (keeps drifting), the loop gives up
+    # after its max iterations with a soft warning — never claims success.
+    filler = FillerVLC(mock_config, enabled=False)
+    p = VlcKaraokePlayer(mock_config, filler, enabled=True)
+    mocker.patch.object(p, '_find_window', return_value=('0xwin', 500, 500))
+    mocker.patch.object(p, '_wmctrl', return_value=mocker.Mock(returncode=0))
+    mocker.patch('vlc.subprocess.run')
+    mocker.patch('vlc.time.sleep')
+    logs = mocker.patch('vlc.log_message')
+    p._position_window(80, 1920, 1080)
+    msgs = ' '.join(str(c.args[0]) for c in logs.call_args_list)
+    assert 'did not settle' in msgs
+    assert 'Positioned karaoke VLC' not in msgs
 
 
 def test_position_window_bails_on_wmctrl_nonzero_exit(mock_config, mocker):

kj-controller/vlc.py

@@ -249,22 +249,41 @@ def _position_window(self, margin_px, screen_w, screen_h):
                 ['-i', '-r', winid, '-b',
                  'remove,maximized_vert,maximized_horz']):
             return
-        if not self._wmctrl_ok(
-                ['-i', '-r', winid, '-e',
-                 f'0,0,{margin_px},{screen_w},{video_h}']):
-            return
-        time.sleep(0.4)
-        cur = self._find_window()
-        if cur:
+
+        # Converge on (0, margin_px). Two things move the window off a single
+        # request: xfwm4 offsets wmctrl moves by a fixed frame-extent amount, and
+        # VLC nudges/resizes its own window while a (4K) file loads. So we drive a
+        # closed loop — request, settle, measure, and adjust the *next* request by
+        # the observed error (req += target - actual) — until the window lands on
+        # target within a 2px tolerance (covers the WM border). A stale read one
+        # round is simply corrected the next.
+        req_x, req_y = 0, margin_px
+        placed = False
+        last = None
+        for _ in range(6):
+            if not self._wmctrl_ok(
+                    ['-i', '-r', winid, '-e',
+                     f'0,{req_x},{req_y},{screen_w},{video_h}']):
+                return
+            time.sleep(0.5)
+            cur = self._find_window()
+            if not cur:
+                break
             _, ax, ay = cur
-            # Correct the fixed WM offset: request 2*target - actual so the
-            # window lands exactly at (0, margin_px). target_x is 0. Best-effort
-            # refinement — the primary placement above already succeeded.
-            self._wmctrl_ok(
-                ['-i', '-r', winid, '-e',
-                 f'0,{-ax},{2 * margin_px - ay},{screen_w},{video_h}'])
-        log_message(
-            f"Positioned karaoke VLC below {margin_px}px ticker strip.", self.config)
+            last = (ax, ay)
+            if abs(ax) <= 2 and abs(ay - margin_px) <= 2:
+                placed = True
+                break
+            req_x += -ax
+            req_y += margin_px - ay
+        if placed:
+            log_message(
+                f"Positioned karaoke VLC below {margin_px}px ticker strip.",
+                self.config)
+        else:
+            log_message(
+                f"Karaoke VLC placement did not settle (last={last}) — video may "
+                f"be slightly off the {margin_px}px strip.", self.config)
 
     # ── Lifecycle ──────────────────────────────────────────────────────