Revive the generated-initials logo bug by caching it, not by fetching it at render time #652

Open
opened 2026-07-26 20:53:25 +02:00 by timothy · 0 comments
Owner

Split out of #510, which switched the generated-initials nameplate bug OFF at every watermark attachment point. This issue is the "do it properly" path, deliberately deferred there on scope rather than rejected on merit.

Background

A channel with a ChannelLogo watermark and no logo artwork used to render a generated nameplate — the channel name drawn by ChannelLogoGenerator, served from GET /iptv/logos/gen?text=…. It only worked when the watermark arrived via a deco; the three precedence levels (playout item / channel / global) had always dropped it.

#510 confirmed by live-E2E that it genuinely rendered (a real transcoded frame showed the nameplate compositing through the graphics engine — /iptv/logos/gen sits on ArtworkController, which has no auth filter, so the container-internal self-fetch succeeded), then unified all four sites onto "no usable logo ⇒ no bug".

Why it was switched off rather than generalised

Two reasons, both about how it was served, not whether the feature is wanted:

  1. It is an HTTP fetch inside stream startup, per element init. That is exactly what graphics.channel-logo-caching (#525) eliminated for channel logos, and what #511 had to bound beforehand.
  2. ChannelLogoGenerator.GenerateChannelLogoUrl() hardcodes http://localhost:{StreamingPort} — issue #1, which was closed as a topology problem without removing the hardcode. It happens to work for a container-internal self-fetch and is wrong anywhere else.

What to build

Make the generated image an ordinary cached local logo, so it needs no HTTP at render time and #1 becomes irrelevant to this path:

  • On resolve (or on channel save), render the nameplate PNG in-process via IChannelLogoGenerator and write it through IImageCache.SaveArtworkToCache, exactly as #525 does for a downloaded external URL — content-hash name, byte-identical to an uploaded logo.
  • WatermarkSelector.ChannelLogoWatermarkOptions then resolves a local path and the existing existence check applies unchanged.
  • Apply the shared RemoteImageDecodeBudget/validator only if the generated image can be attacker-influenced in size; it is drawn from the channel name at a fixed 200×100, so probably not — confirm rather than assume.
  • Decide and document the invalidation rule: the nameplate embeds the channel name, so a rename must produce a new cache entry (a content-hash name gives this for free) and ideally drop the stale one.
  • Decide whether it is opt-in. Enabling it unconditionally would newly show nameplates on every logoless channel with a ChannelLogo watermark — currently 0 channels on prod, but that is a real behavior change for anyone who has been relying on "no logo ⇒ no bug". A per-channel or per-watermark toggle is the conservative option.

Notes

  • Prod today: 0 Deco rows, 0 DecoWatermark rows, all 43 channels have logo artwork — so nothing renders differently either before or after #510, and there is no urgency here.
  • The relevant decision record is ffmpeg.watermark-resolution-unified; it names this issue as the right way to revive the fallback and explicitly asks that it not be done by reinstating a render-time URL.
  • docs/channels.md → Watermarks documents the current "no usable logo ⇒ no bug" behavior and would need updating.

Done-when

  • Generated nameplate is produced in-process and stored in the image cache (no render-time HTTP fetch on this path)
  • WatermarkSelector resolves it as a local cached path through the existing shared resolver, with no new per-caller policy
  • Rename/invalidation behavior decided, implemented and documented
  • Opt-in vs always-on decided and recorded in a decision record
  • Tests cover: nameplate cached and resolved, rename produces a new entry, and the parity invariant from #510 still holds across all four attachment points
  • docs/channels.md updated
  • Live-E2E: a real transcoded frame shows the nameplate, served from a local cached file (assert no outbound fetch for it)
  • Adversarial review passed
Split out of **#510**, which switched the generated-initials nameplate bug OFF at every watermark attachment point. This issue is the "do it properly" path, deliberately deferred there on scope rather than rejected on merit. ## Background A channel with a `ChannelLogo` watermark and **no logo artwork** used to render a generated nameplate — the channel name drawn by `ChannelLogoGenerator`, served from `GET /iptv/logos/gen?text=…`. It only worked when the watermark arrived via a **deco**; the three precedence levels (playout item / channel / global) had always dropped it. #510 confirmed by live-E2E that it genuinely rendered (a real transcoded frame showed the nameplate compositing through the graphics engine — `/iptv/logos/gen` sits on `ArtworkController`, which has no auth filter, so the container-internal self-fetch succeeded), then unified all four sites onto "no usable logo ⇒ no bug". ## Why it was switched off rather than generalised Two reasons, both about *how* it was served, not whether the feature is wanted: 1. It is an **HTTP fetch inside stream startup**, per element init. That is exactly what `graphics.channel-logo-caching` (#525) eliminated for channel logos, and what #511 had to bound beforehand. 2. `ChannelLogoGenerator.GenerateChannelLogoUrl()` hardcodes `http://localhost:{StreamingPort}` — issue **#1**, which was closed as a topology problem without removing the hardcode. It happens to work for a container-internal self-fetch and is wrong anywhere else. ## What to build Make the generated image an ordinary cached local logo, so it needs no HTTP at render time and `#1` becomes irrelevant to this path: - On resolve (or on channel save), render the nameplate PNG **in-process** via `IChannelLogoGenerator` and write it through `IImageCache.SaveArtworkToCache`, exactly as #525 does for a downloaded external URL — content-hash name, byte-identical to an uploaded logo. - `WatermarkSelector.ChannelLogoWatermarkOptions` then resolves a **local** path and the existing existence check applies unchanged. - Apply the shared `RemoteImageDecodeBudget`/validator only if the generated image can be attacker-influenced in size; it is drawn from the channel name at a fixed 200×100, so probably not — confirm rather than assume. - Decide and document the invalidation rule: the nameplate embeds the channel **name**, so a rename must produce a new cache entry (a content-hash name gives this for free) and ideally drop the stale one. - Decide whether it is **opt-in**. Enabling it unconditionally would newly show nameplates on every logoless channel with a `ChannelLogo` watermark — currently 0 channels on prod, but that is a real behavior change for anyone who has been relying on "no logo ⇒ no bug". A per-channel or per-watermark toggle is the conservative option. ## Notes - Prod today: 0 `Deco` rows, 0 `DecoWatermark` rows, all 43 channels have logo artwork — so nothing renders differently either before or after #510, and there is no urgency here. - The relevant decision record is `ffmpeg.watermark-resolution-unified`; it names this issue as the right way to revive the fallback and explicitly asks that it not be done by reinstating a render-time URL. - `docs/channels.md` → Watermarks documents the current "no usable logo ⇒ no bug" behavior and would need updating. ## Done-when - [ ] Generated nameplate is produced in-process and stored in the image cache (no render-time HTTP fetch on this path) - [ ] `WatermarkSelector` resolves it as a local cached path through the existing shared resolver, with no new per-caller policy - [ ] Rename/invalidation behavior decided, implemented and documented - [ ] Opt-in vs always-on decided and recorded in a decision record - [ ] Tests cover: nameplate cached and resolved, rename produces a new entry, and the parity invariant from #510 still holds across all four attachment points - [ ] `docs/channels.md` updated - [ ] Live-E2E: a real transcoded frame shows the nameplate, served from a local cached file (assert no outbound fetch for it) - [ ] Adversarial review passed
timothy added the enhancementpriority: low labels 2026-07-26 20:53:32 +02:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: timothy/ersatztv#652