From d1f621f0b43c30bcd920beb3c74ed6f1405aa912 Mon Sep 17 00:00:00 2001 From: "alexei.dolgolyov" Date: Fri, 15 May 2026 14:45:40 +0300 Subject: [PATCH] fix(displays): verify DDC/CI writes and trust capability string for picture mode DDC/CI writes are fire-and-forget at the protocol level: a successful send does not mean the monitor honored the value. Many monitors (LG ultrawides in particular) silently drop writes for VCP codes whose registers exist but whose feature isn't really implemented in firmware. - New _verify_after_set helper polls readback after every DDC/CI write and reports {success: false} when the monitor didn't apply the value. Wired into set_contrast, set_input_source, set_color_preset, set_picture_mode. Input source uses a longer settle window since switching can briefly disrupt the DDC/CI link. - Picture mode (VCP 0xDC) now requires the capability string to declare supported codes under cmds[0xDC]. Without that declaration we treat the feature as unsupported even when reads succeed - the LG case where reads return a stuck value and every write is silently ignored. --- media_server/services/display_service.py | 84 ++++++++++++++++++++---- 1 file changed, 70 insertions(+), 14 deletions(-) diff --git a/media_server/services/display_service.py b/media_server/services/display_service.py index 1fd01fd..0df96ba 100644 --- a/media_server/services/display_service.py +++ b/media_server/services/display_service.py @@ -267,20 +267,27 @@ def _probe_static_open(mon, mc, monitor_id: int) -> dict: except Exception as e: logger.debug("Monitor %d: color_preset unsupported: %s", monitor_id, e) - # Picture / scene mode (VCP 0xDC) — not exposed by monitorcontrol's - # high-level API, so probe via raw VCP transport. - try: - mon.vcp.get_vcp_feature(PICTURE_MODE_VCP) - static["picture_mode_supported"] = True - cmds = caps.get("cmds") or {} - declared = cmds.get(PICTURE_MODE_VCP) - codes = sorted(declared) if declared else sorted(PICTURE_MODE_LABELS.keys()) - static["available_picture_modes"] = [ - {"code": c, "label": PICTURE_MODE_LABELS.get(c, f"Mode {c}")} - for c in codes - ] - except Exception as e: - logger.debug("Monitor %d: picture_mode unsupported: %s", monitor_id, e) + # Picture / scene mode (VCP 0xDC). Trickier than color preset because + # many monitors (LG ultrawides included) respond to READS but silently + # drop every WRITE - they implement the register but not the feature. + # The capability string is the most reliable signal: a monitor that + # really implements picture mode declares its supported codes under + # cmds[0xDC]. If 0xDC isn't declared, treat the feature as unsupported + # to avoid exposing a non-functional select. + cmds = caps.get("cmds") or {} + declared = cmds.get(PICTURE_MODE_VCP) + if declared: + try: + mon.vcp.get_vcp_feature(PICTURE_MODE_VCP) + static["picture_mode_supported"] = True + static["available_picture_modes"] = [ + {"code": c, "label": PICTURE_MODE_LABELS.get(c, f"Mode {c}")} + for c in sorted(declared) + ] + except Exception as e: + logger.debug("Monitor %d: picture_mode declared but unreadable: %s", monitor_id, e) + else: + logger.debug("Monitor %d: picture_mode (VCP 0xDC) not declared in capability string", monitor_id) return static @@ -498,6 +505,31 @@ def set_power(monitor_id: int, on: bool) -> bool: return False +def _verify_after_set(getter, expected, *, retries: int = 3, delay: float = 0.1) -> bool: + """Poll a DDC/CI getter to confirm the monitor actually applied a write. + + DDC/CI writes are fire-and-forget at the protocol level: a successful + send does not mean the monitor honored the value. Many monitors silently + drop writes for codes their firmware doesn't really implement (LG's + ColorPreset / Picture Mode are common offenders). Without this check the + API would report `success: true` while the monitor sat unchanged. + + Compares both raw and `.value` forms so enum/int mismatches don't flag a + spurious failure. + """ + expected_int = getattr(expected, "value", expected) + for _ in range(retries): + time.sleep(delay) + try: + actual = getter() + except Exception: + continue + actual_int = getattr(actual, "value", actual) + if actual == expected or actual_int == expected_int: + return True + return False + + def set_contrast(monitor_id: int, value: int) -> bool: """Set contrast for a specific monitor (0-100) via DDC/CI.""" mc = _load_monitorcontrol() @@ -511,6 +543,9 @@ def set_contrast(monitor_id: int, value: int) -> bool: return False with ddc_monitors[monitor_id] as monitor: monitor.set_contrast(value) + if not _verify_after_set(monitor.get_contrast, value): + logger.warning("Monitor %d: contrast %d not applied", monitor_id, value) + return False _invalidate_cache() return True except Exception as e: @@ -536,6 +571,11 @@ def set_input_source(monitor_id: int, source: str) -> bool: return False with ddc_monitors[monitor_id] as monitor: monitor.set_input_source(target) + # Source switches can briefly disrupt the DDC/CI link; allow a + # longer settle window before declaring failure. + if not _verify_after_set(monitor.get_input_source, target, retries=5, delay=0.2): + logger.warning("Monitor %d: input source %s not applied", monitor_id, source) + return False _invalidate_cache() return True except Exception as e: @@ -561,6 +601,12 @@ def set_color_preset(monitor_id: int, preset: str) -> bool: return False with ddc_monitors[monitor_id] as monitor: monitor.set_color_preset(target) + if not _verify_after_set(monitor.get_color_preset, target): + logger.warning( + "Monitor %d: color preset %s not applied (monitor silently rejected)", + monitor_id, preset, + ) + return False _invalidate_cache() return True except Exception as e: @@ -584,6 +630,16 @@ def set_picture_mode(monitor_id: int, code: int) -> bool: return False with ddc_monitors[monitor_id] as monitor: monitor.vcp.set_vcp_feature(PICTURE_MODE_VCP, code) + # Raw VCP read returns (current, maximum) — only compare current. + def _read_picture_mode(): + current, _ = monitor.vcp.get_vcp_feature(PICTURE_MODE_VCP) + return current + if not _verify_after_set(_read_picture_mode, code): + logger.warning( + "Monitor %d: picture mode code %d not applied (monitor silently rejected)", + monitor_id, code, + ) + return False _invalidate_cache() return True except Exception as e: