feat(player): add image watermark overlay support

[syed-tp]

• Add ImageWatermarkConfig data model with URL, position, dimensions, and opacity validation
• Introduce BaseWatermarkOverlayView to share percentage coordinate and video frame layout calculations
• Implement ImageWatermarkOverlayView with async URLSession fetching and in-memory NSCache
• Rename WatermarkOverlayView to TextWatermarkOverlayView for naming symmetry
• Add auto-fade support when controls toggle via onControlsVisibilityChanged in SwiftUI and toggleControlsVisibility in UIKit
• Wire imageWatermarks configuration to TPStreamPlayerConfiguration, TPStreamPlayerView, and TPStreamPlayerViewController
• Update SwiftUI and Storyboard example apps with image watermark demonstration

[claude]

Claude finished @syed-tp's task in 2m 17s —— View job

---

Claude finished reviewing this PR

Todo list

• Gather context (diff, CLAUDE.md, changed files)
• Review new watermark model & config (WatermarkConfig.swift)
• Review BaseWatermarkOverlayView / ImageWatermarkOverlayView / TextWatermarkOverlayView
• Review integration points (TPStreamPlayerConfiguration, TPStreamPlayerView, TPStreamPlayerViewController, PlayerControlsView)
• Review example app changes
• Post final review

---

Overall this is a clean, well-factored addition — pulling the shared percentage/frame math into BaseWatermarkOverlayView is a nice touch, and the controls-visibility wiring between UIKit and SwiftUI paths is consistent (textWatermarkOverlayView/imageWatermarkOverlayView stay in sync with controlsView.isHidden in TPStreamPlayerViewController, and showControls/areControlsVisible default to the same value in the SwiftUI path). The WatermarkOverlayView → TextWatermarkOverlayView rename is safe since the original type was internal, not public, so it's not a breaking API change. TPStreamPlayerConfiguration.imageWatermarks defaults to [] and the builder method is additive, so TPStreamPlayerConfiguration stays backward compatible.

A couple of issues worth addressing:

1. precondition in ImageWatermarkConfig.init will crash the host app, and can be bypassed anyway

Source/Views/WatermarkConfig.swift (~line 75-80):

precondition(!imageUrl.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty, "imageUrl must not be empty")
precondition(width > 0, "width must be greater than 0")
...

This is a new pattern for the codebase — nowhere else in Source/ uses precondition, and the sibling WatermarkConfig (text) does no validation at all. precondition traps and crashes the host app, not the SDK, on bad config (e.g. an empty/blank imageUrl, or x/y outside 0...100). That's a harsh failure mode for a value coming from a CMS/API response.

It's also not actually enforced as an invariant: every property (imageUrl, width, height, x, y, opacity) is a mutable public var, so var config = ImageWatermarkConfig(imageUrl: "ok"); config.opacity = 5.0 bypasses validation entirely post-init. Meanwhile ImageWatermarkOverlayView's layoutIn(area:) already defensively clamps x/y to 0...100 at the point of use — which suggests clamping (rather than crashing) is the intended failure mode here too. Consider dropping the preconditions and clamping/defaulting invalid values the same way x/y already are, consistent with TPStreamPlayerError-style graceful handling used elsewhere in the SDK rather than introducing a new crash-on-bad-input path.

Fix this →

2. Minor: unbounded static NSCache

Source/Views/UIKit/ImageWatermarkOverlayView.swift:61 — ImageWatermarkItemView.imageCache has no countLimit/totalCostLimit, so decoded watermark images accumulate for the process lifetime across every distinct imageUrl ever configured. Likely fine given watermarks are small logos, but worth a bound if players can be configured with many different per-course/per-video watermark URLs over a long session.

Nits

• print("[ImageWatermark] ...") for load failures (lines 96, 103, 109) matches existing SDK conventions (e.g. TPStreamsDownloadManager.swift, TPAVPlayer.swift), so not a blocker, just flagging since the review checklist calls out avoiding raw error strings — could route through TPStreamPlayerError/a consistent logger if the team wants a broader cleanup, but that's larger than this PR's scope.

No lifecycle/memory-leak issues found — dataTask is stored and cancelled in ImageWatermarkItemView.deinit, all closures correctly use [weak self], and no new KVO/time observers were introduced.

[syed-tp]

@claude review

1. precondition replaced with defensive clamping: Removed all precondition checks in ImageWatermarkConfig.init. x and y are now clamped to 0...100, opacity to 0.0...1.0, and dimensions to max(..., 0) — consistent with the rest of the SDK's graceful handling without risking host app crashes on unexpected API inputs.
2. Cache layer removed: Removed the custom NSCache completely from ImageWatermarkItemView. Images are now fetched directly via standard URLSession data tasks with early guards on empty/blank URL strings.

Post the verdict (approved/ changes requested)

[claude]

Claude finished @syed-tp's task in 39s —— View job

---

Follow-up review — verdict: ✅ Approved

Todo list

• Gather context (diff since previous review)
• Verify precondition → clamping fix in ImageWatermarkConfig
• Verify NSCache removal in ImageWatermarkOverlayView
• Check for any regressions elsewhere
• Post verdict

Both points from the previous review are addressed cleanly:

1. precondition → clamping (Source/Views/WatermarkConfig.swift:67-81): ImageWatermarkConfig.init now clamps width/height to max(_, 0), x/y to 0...100, and opacity to 0.0...1.0 instead of trapping. This matches the existing clamping already done at use-site in layoutIn(area:) (ImageWatermarkOverlayView.swift:119-120), so bad CMS/API input degrades gracefully instead of crashing the host app. No lingering precondition calls anywhere in Source/.

2. NSCache removed (Source/Views/UIKit/ImageWatermarkOverlayView.swift): ImageWatermarkItemView now fetches directly via URLSession.shared.dataTask, with an early-return guard on empty/blank imageUrl before constructing the request. dataTask is still stored and cancelled in deinit, and the completion closure still uses [weak self], so lifecycle/retain-cycle behavior is unchanged. No leftover cache references anywhere in the codebase.

No new issues introduced by this round of changes. The only outstanding item is the previously-flagged non-blocking nit: print("[ImageWatermark] ...") for load failures (lines 93, 100, 106) still uses raw strings rather than a structured logger/TPStreamPlayerError, consistent with existing SDK conventions elsewhere (e.g. TPAVPlayer.swift) — fine to leave as-is for this PR's scope.

Verdict: Approved.

[syed-tp]

@claude review

[claude]

Claude finished @syed-tp's task in 2m 45s —— View job

---

Follow-up review of latest commit (d199871) — verdict: changes requested

Todo list

• Gather context (diff since previous "Approved" review)
• Re-check previously flagged items (precondition/clamping, NSCache removal)
• Review full current diff for new issues
• Post final review

The two items from the earlier rounds are still resolved correctly (ImageWatermarkConfig.init clamps instead of using precondition, and the NSCache layer is gone). Since the last "Approved" verdict, a new commit (d199871, "consolidate text and image watermarks into a single unified configuration array") landed, which introduces one issue worth addressing before merge:

1. Public API break: TPStreamPlayerConfiguration.watermarks type changed from [WatermarkConfig] to [BaseWatermarkConfig]

Source/TPStreamPlayerConfiguration.swift:27,117:

public var watermarks: [BaseWatermarkConfig] = []
...
public func setWatermarks(_ watermarks: [BaseWatermarkConfig]) -> Self {

WatermarkConfig is still around as a typealias for TextWatermarkConfig (Source/Views/WatermarkConfig.swift:41), so writing to .watermarks or calling setWatermarks(...) with a [WatermarkConfig]/[TextWatermarkConfig] value still compiles fine (Swift's implicit array-to-protocol-array conversion covers that direction). However, any existing host app that reads config.watermarks back out with the old concrete type — e.g. let wms: [WatermarkConfig] = config.watermarks, or code that pattern-matches/iterates assuming WatermarkConfig — will now fail to compile, since going from [BaseWatermarkConfig] to a concrete [WatermarkConfig] requires an explicit compactMap { $0 as? WatermarkConfig }. This is a source-breaking change to a public property/method on TPStreamPlayerConfiguration, which the review guidelines call out specifically ("No breaking changes to public/open types without a major version bump", "TPStreamPlayerConfiguration changes must stay backward-compatible"), and there's no version bump accompanying this PR (podspec is still 1.2.41).

Since imageWatermarks was already wired in as its own separate array in the prior commits (per the PR description: "Wire imageWatermarks configuration to TPStreamPlayerConfiguration..."), consider whether this consolidation commit is necessary at all — keeping watermarks: [WatermarkConfig] and imageWatermarks: [ImageWatermarkConfig] as two separate, independently-typed arrays (as the PR description originally stated) avoids the protocol-array break entirely while still supporting both watermark kinds. If the unified-array API is intentional and desired going forward, it should at least be called out as a breaking change (and the "typealias for 100% backward compatibility" comment on WatermarkConfig at WatermarkConfig.swift:40 should be corrected/qualified, since compatibility only holds in the write direction).

Fix this →

Verified as correct

• ImageWatermarkOverlayView / ImageWatermarkItemView (Source/Views/UIKit/ImageWatermarkOverlayView.swift): direct URLSession fetch, dataTask cancelled in deinit, [weak self] in the completion closure, no cache.
• BaseWatermarkOverlayView shared layout math is used consistently by both TextWatermarkOverlayView and ImageWatermarkOverlayView.
• Controls-visibility wiring stays consistent between UIKit (toggleControlsVisibility ↔ imageWatermarkOverlayView.setControlsVisible) and SwiftUI (showControls/areControlsVisible both default to false, kept in sync via onControlsVisibilityChanged).
• No leftover references to the old WatermarkOverlayView/WatermarkOverlayViewRepresentable names after the rename.

Nit (carried over, still non-blocking)

• print("[ImageWatermark] ...") for load failures still uses raw strings rather than TPStreamPlayerError/a structured logger — consistent with existing SDK conventions elsewhere, fine to leave for this PR's scope.