diff --git a/src/zoom-loudness-meter-source.cpp b/src/zoom-loudness-meter-source.cpp index b2c6675..39e18a2 100644 --- a/src/zoom-loudness-meter-source.cpp +++ b/src/zoom-loudness-meter-source.cpp @@ -42,17 +42,22 @@ static constexpr uint32_t kMeterLoudArgb = 0xFFE04B4Bu; static constexpr uint32_t kMeterQuietArgb = 0xFFE0A03Cu; static constexpr uint32_t kMeterIdleArgb = 0xFF3A424Eu; -// Shared with the Tiles wall only by FILE PATH, not by handle -- and that is -// not the free dedupe it sounds like. gs_effect_create_from_file() allocates -// a brand-new gs_effect_t on every call; libobs does NOT cache or dedupe -// effects compiled from the same file. This second tiles_effect_load() call -// (the Tiles wall makes its own, separate one) compiles a second, independent -// copy of corevideo-tiles.effect. Harmless as written -- two handles, each -// destroyed exactly once by its own owner (this file's unload vs. the Tiles -// source's) -- but do NOT "deduplicate" the two loads into one shared -// gs_effect_t* on the strength of a caching story that isn't true: sharing a -// handle between two owners that each call gs_effect_destroy() on it once is -// a double-free at unload. +// Shared with the Tiles wall by FILE PATH, and libobs really does dedupe that +// for us -- verified in libobs source, because this comment has now been +// wrong in both directions and the guesswork should stop here: +// +// graphics.c, gs_effect_create_from_file(): calls find_cached_effect(file) +// first and returns that effect if one matches, so the second load compiles +// nothing. gs_effect_create() sets effect->cached = true for any effect +// created with a filename. +// effect.c, gs_effect_destroy(): `if (!effect->cached) actually_destroy()` +// -- destroying a cached effect is a no-op until obs_free_graphics(). +// +// So s_meter_effect.effect and the Tiles wall's s_tiles_effect.effect are the +// SAME pointer, each tiles_effect_destroy() is a no-op, and the effect is +// reclaimed at graphics shutdown. No double compile, no double-free, no leak. +// Loading our own handle rather than reaching into zoom-supersource.cpp is a +// coupling choice, not a memory-safety one. static TilesEffect s_meter_effect; static bool s_meter_pass_failed_logged = false; diff --git a/src/zoom-tiles-effect.h b/src/zoom-tiles-effect.h index eebe5a4..f0d2585 100644 --- a/src/zoom-tiles-effect.h +++ b/src/zoom-tiles-effect.h @@ -94,12 +94,21 @@ struct TilesEffect { bool tiles_effect_load(TilesEffect &out); // Drops our reference to the effect and resets the struct. Safe to call on -// an unloaded/failed TilesEffect. Note: gs_effect_create_from_file() allocates -// a fresh gs_effect_t on every call -- libobs does NOT cache or dedupe -// effects compiled from the same file (an earlier version of this comment -// claimed otherwise). gs_effect_destroy() here is a real, immediate release -// of THIS handle. Every caller that loads its own TilesEffect from this file -// (the Tiles wall and the Loudness Meter each compile an independent copy) -// owns exactly one handle and must destroy it exactly once; two owners must -// never share one gs_effect_t*, or the second destroy is a double-free. +// an unloaded/failed TilesEffect. +// +// libobs CACHES effects created from a file, and both halves of that matter +// here -- verified in libobs source, because this comment has been wrong in +// both directions before: +// +// graphics.c, gs_effect_create_from_file(): find_cached_effect(file) is +// consulted first, so loading the same path twice returns the SAME +// gs_effect_t*. gs_effect_create() marks any file-created effect cached. +// effect.c, gs_effect_destroy(): `if (!effect->cached)` guards the real +// free, so destroying a cached effect is a no-op; obs_free_graphics() +// reclaims it at shutdown. +// +// Consequence for callers: several owners may each hold a TilesEffect loaded +// from this same file (the Tiles wall and the Loudness Meter both do). They +// share one underlying pointer, each destroy is a no-op, and that is safe -- +// there is no double-free to avoid and no leak to chase. void tiles_effect_destroy(TilesEffect &fx);