Skip to content

Add taskbar media widget#4642

Open
umk0 wants to merge 1 commit into
ramensoftware:mainfrom
umk0:add-taskbar-media-widget
Open

Add taskbar media widget#4642
umk0 wants to merge 1 commit into
ramensoftware:mainfrom
umk0:add-taskbar-media-widget

Conversation

@umk0

@umk0 umk0 commented Jul 1, 2026

Copy link
Copy Markdown

Adds a compact Windows taskbar media widget that reads metadata and controls playback through Global System Media Transport Controls.

Features:

  • Track title, artist, album art, progress and playback controls
  • Hover controls with seeking
  • Idle launchers for configured music apps
  • Direct2D rendering with DWM backdrop support

Checked locally with Windhawk clang syntax and object compilation.

@m417z

m417z commented Jul 2, 2026

Copy link
Copy Markdown
Member

Submission review

Note: This review was done by Claude, and then refined manually. Due to the amount of submissions, doing a fully manual review for each pull request is no longer feasible. Thank you for understanding.

Please address the following issues. The items in the collapsed sections are optional, so it's your call whether to address them.

The main point to address is the overlaps with existing mods, mainly Taskbar Fluent Media Player which is actively maintained. Please explain which features your mod adds that Taskbar Fluent Media Player is missing, and let's discuss whether it'd be better to add them to Taskbar Fluent Media Player instead.

See also:
https://gist.github.com/m417z/334ee4371449592faa4575aa2a1bbfc4


1. Substantial overlap with existing mods. A Fluent-styled taskbar media widget (track info + artwork + playback controls) already exists more than once:

The maintainer's strong preference is to extend an existing mod (add an option, or PR the original author's repo) rather than merge a near-duplicate. This mod's differentiators (hover seek preview, idle app launchers, artwork-derived tinting) are real but not clearly enough to justify a fourth media widget on its own. Please state in the PR how it meaningfully differs from the above and why it can't be an option/contribution to one of them; if the delta is thin, consider consolidating.

2. This should be a "mods as tools" mod, not injected into explorer.exe. The mod installs zero function hooks — it only creates a top-most overlay window and uses EnumWindows, timers, GDI+/Direct2D and WinRT, all of which work from any process. That is the textbook "Mods as tools: running mods in a dedicated process" case. Two concrete problems with running inside Explorer:

  • Any crash in the ~3000 lines of D2D/WinRT/GDI+ code takes down the shell. As a tool mod it would only crash its own helper process.
  • With "launch folder windows in a separate process" enabled, every secondary explorer.exe runs this mod too, and each spins WidgetThread forever in the FindCurrentProcessTaskbarWnd() then Sleep(500) loop because those processes have no Shell_TrayWnd.

Dynamic Island for Windows is direct precedent — the same kind of taskbar media overlay implemented as a tool mod (@include windhawk.exe, WhTool_* callbacks). Recommended change: @include windhawk.exe, rename Wh_ModInit/Wh_ModSettingsChanged/Wh_ModUninit to WhTool_*, paste the verbatim launcher snippet from the wiki (see the bottom of explorer-folder-hover-menu.wh.cpp), and drop the GetCurrentProcessId() filter in FindTaskbarWindowProc so it locates Explorer's taskbar cross-process.

3. The window class is never unregistered — breaks on reload / risks a crash. WidgetThread calls RegisterClassW(&wndClass) but nothing ever calls UnregisterClass. When the mod DLL unloads, the class registration lingers with lpfnWndProc still pointing into the now-unmapped mod image. On the next load (toggle / update / settings-driven reload), RegisterClassW fails with ERROR_CLASS_ALREADY_EXISTS (the return value is ignored), and CreateWindowExW then reuses the stale class whose WndProc points at freed/relocated code, causing a crash. Register on load, and unregister in Wh_ModUninit using the same hInstance you registered with:

void Wh_ModUninit() {
    ...
    if (g_widgetThread.joinable()) g_widgetThread.join();
    UnregisterClassW(L"WindhawkTaskbarMediaWidget", GetModuleHandle(nullptr));
    ...
}

4. Global std::thread calls std::terminate() at process shutdown. g_widgetThread is a global std::thread joined only in Wh_ModUninit. But Wh_ModUninit does not run when the host process itself terminates (Explorer restart, sign-out, reboot) — the CRT just runs global destructors during DLL_PROCESS_DETACH. At that point the thread was never joined, so it's still joinable(), and ~thread() calls std::terminate(), aborting the host during shutdown. Prefer a pointer with a trivial destructor (intentionally leaked at shutdown), keeping the real join in Wh_ModUninit:

std::thread* g_widgetThread = nullptr;
// init:   g_widgetThread = new std::thread(WidgetThread);
// uninit: if (g_widgetThread->joinable()) g_widgetThread->join();
//         delete g_widgetThread; g_widgetThread = nullptr;

The same "global with a non-trivial destructor" caveat applies to g_thumbnailBitmap, g_idleApps (GDI+ bitmaps) and the D2D/WinRT globals — but the thread is the one that actively crashes, so it's the priority.

5. Wh_ModSettingsChanged mutates shared state from the wrong thread, causing a data race / use-after-free. Wh_ModSettingsChanged runs on an arbitrary Windhawk thread and directly calls LoadSettings() (rewrites g_settings), ScanIdleApps() (clears and repopulates g_idleApps, freeing the GDI+ bitmaps), plus ApplySystemBackdrop/RepositionWidget/RefreshWidget on g_widgetWnd. Meanwhile the widget thread is concurrently reading g_settings and iterating g_idleApps in its 33 ms paint / hit-test handlers. Freeing an IdleApp's bitmap while drawIdleApps() is drawing it is a use-after-free that can crash Explorer. Marshal the reload onto the widget thread instead of touching state cross-thread — e.g. post a private message and do the reload work inside WidgetWndProc:

// Wh_ModSettingsChanged (arbitrary thread):
if (g_widgetWnd) PostMessage(g_widgetWnd, WM_APP_RELOAD_SETTINGS, 0, 0);
// WidgetWndProc handles WM_APP_RELOAD_SETTINGS on the owning thread.

That also fixes the cross-thread SetWindowPos/DWM calls in the current path.

6. Continuous 30 fps redraw even when nothing is animating. The kTimerRepaint timer fires every 33 ms and unconditionally calls RefreshWidget() (InvalidateRect), forcing a full Direct2D repaint 30 times a second whenever the widget is visible — including a static, paused, non-hovered track. That's a constant CPU/GPU (and battery) cost for a widget that only needs high-FPS updates during hover/press animations. Have TickAnimations() report whether any value actually changed and only invalidate then, and idle the fast timer (fall back to roughly 1 fps for progress) when nothing is animating.

7. Non-English default settings. User-facing defaults should be English. The default idleAppNames is "Яндекс Музыка, Spotify, Apple Music" (and the code hard-codes the same Cyrillic fallback in LoadSettings/ShortcutAllowedByUserList). Default to English (e.g. "Spotify, Apple Music") and let users add regional apps themselves. Keeping Cyrillic matching logic is fine; it's the default value that should be English.

8. Add a screenshot/GIF to the README. This is a highly visual mod but the README has no image. Please add a screenshot or short GIF (only i.imgur.com and raw.githubusercontent.com are allowed hosts) so users can see what it looks like.

Optional improvements

Minor polish — none of this affects users, so it's your call.

  • WindhawkUtils::StringSetting (RAII) instead of the manual Wh_GetStringSetting + Wh_FreeStringSetting dance in LoadSettings() — less bookkeeping and exception-safe. Also note Wh_GetStringSetting never returns NULL (it returns L""), so the idleAppNames ? idleAppNames : L"..." fallback is dead code.
  • ScanIdleApps() re-scans the entire Start Menu (recursive, depth 6, across four known folders) at init and on every settings change. Cache the result and only re-scan when idleAppNames actually changes.
  • WhitelistedSourceForShortcut / the hard-coded app whitelist (yandex/spotify/apple/aimp/foobar/winamp/deezer/tidal/musicbee) plus the multi-layer name-to-source matching reads as over-engineered on top of the user-supplied idleAppNames. Consider simplifying to the user list alone.
  • init_apartment(apartment_type::multi_threaded) on the widget thread is never paired with uninit_apartment() before the thread exits.

Functionality notes

Non-critical observations and ideas about the feature behavior itself.

  • Event-driven media instead of 1 s polling. PollMedia re-runs GlobalSystemMediaTransportControlsSessionManager::RequestAsync().get() every second. GSMTC exposes SessionsChanged, and per-session MediaPropertiesChanged / PlaybackInfoChanged / TimelinePropertiesChanged events — subscribing would cut the periodic work and make the widget update instantly on track/state changes. Bigger change, so just an FYI.
  • Progress/seek assume startTime == 0. In drawProgress, the fill ratio is position / (endTime - startTime) where position is the absolute GetEffectivePosition(); and SeekFromPoint targets duration * ratio without adding startTime. When a session reports a non-zero StartTime (some streams/podcasts), the bar starts partly filled and seeks land off. Use (pos - startTime) / (endTime - startTime) and seek to startTime + duration * ratio.
  • Multi-monitor / Win11 layout / DPI. The widget is pinned to the left edge of the first Shell_TrayWnd in physical pixels. On a centered Win11 taskbar it lands in the far-left corner (near Start/Widgets), secondary-monitor taskbars aren't handled, and the fixed pixel sizes don't scale per-monitor DPI.
  • Blocking .get() in message handlers. PollMedia and the button/seek handlers block the widget's own message loop on the WinRT .get() calls. It only stalls the widget itself (its own thread), but during a slow poll hover/click won't respond for that interval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants