feat: expose subtitle state change event

[Balamurugan-100]

• Host apps could not observe subtitle enable, disable, or language changes, making it difficult to persist the user's caption preference.
• Expose subtitle state changes to host apps while reporting only actual state changes and keeping persistence responsibility with the host.
• Keep the change backward compatible with a default listener implementation.

[claude]

Claude finished @Balamurugan-100's task in 2m 41s —— View job

---

Todo

• Gather context (PR diff, repo files)
• Review changed files for correctness, regressions, lifecycle, thread-safety, API compatibility
• Post review summary

Summary

Reviewed all 5 changed files: TextTrackManager.kt, TPStreamsPlayer.kt, TPStreamsPlayerView.kt, and the sample app's PlayerActivity.kt/MainActivity.kt.

The feature is implemented cleanly:

• TextTrackManager tracks previousSubtitleLanguage and only invokes the callback when the language actually changes (TextTrackManager.kt:67-71), satisfying the "report only actual state changes" requirement.
• TPStreamsPlayer.Listener.onSubtitleStateChanged has a default no-op body (TPStreamsPlayer.kt:91), consistent with the existing pattern used for onNetworkDiagnosticsStarted, so existing implementers remain source/binary compatible.
• TPStreamsPlayerView's listener-wrapping logic (TPStreamsPlayerView.kt:356-361, ViewPlayerListener.onSubtitleStateChanged at :631-633) correctly preserves and forwards to the host's original listener — verified this doesn't clobber a listener set directly on TPStreamsPlayer before attaching to the view, as done in the sample PlayerActivity.kt.
• Dedup is also enforced one layer up in Captions.kt:39-41, so no redundant setTextTrackByLanguage calls from the UI bottom sheet either.
• No new threading concerns: setTextTrackByLanguage and the onTracksChanged callback that drives auto-caption selection all run on the player's application thread, consistent with existing code.

Non-blocking observations:

1. app/src/main/java/com/tpstreams/player/MainActivity.kt:138 — the new hardcoded demo access token "3d9838f3-db51-4fc3-8472-075ab5e40b64 " has trailing whitespace. Sample-app only, but if this is a real token used for the DRM demo tab, the trailing spaces will likely break the request. Worth double-checking/trimming.
   Fix this →

2. TextTrackManager.kt:22 — previousSubtitleLanguage only updates when setTextTrackByLanguage is called explicitly. If a manifest/track-selector ever pre-selects a text track by other means (e.g. a "default"/"forced" flag) before any explicit call, onSubtitleStateChanged won't fire to reflect that initial state, and a host relying solely on this event (rather than also calling getActiveTextTrack() on attach) could start out of sync. Given today's DefaultTrackSelector config (setDisabledTextTrackSelectionFlags only changed via this manager), this is a narrow edge case, not a regression — just worth noting for host apps building persistence on top of this event alone.

Regression Assessment

No regressions identified in playback, DRM, downloads, seeking, track selection, adaptive switching, or player lifecycle. The change is additive (new listener method + internal callback wiring) and existing call paths (Captions.kt, resolution/track selection) are untouched aside from the new callback hook.

Final Verdict

⚠️ APPROVED WITH SUGGESTIONS
No blocking correctness, regression, lifecycle, or security issues found. The two notes above are minor/non-blocking (one demo-app-only, one a documentation/edge-case consideration).

[claude]

Claude finished @Balamurugan-100's task in 1m 1s —— View job

---

Todo

• Gather context (PR diff, repo files)
• Review changed files for correctness, regressions, lifecycle, thread-safety, API compatibility
• Post review summary

Summary

Reviewed all 5 changed files (TextTrackManager.kt, TPStreamsPlayer.kt, TPStreamsPlayerView.kt, PlayerActivity.kt, MainActivity.kt).

The overall design is sound: TPStreamsPlayer.Listener.onSubtitleStateChanged has a no-op default body (source/binary compatible), and TPStreamsPlayerView's listener-wrapping correctly preserves a listener set directly on TPStreamsPlayer before the view attaches (TPStreamsPlayerView.kt:356-364), which the sample PlayerActivity.kt now relies on.

However, there is a functional bug that breaks the core "report actual state changes" requirement for the most common trigger path (auto-enabled default captions):

TextTrackManager.kt:90-94 — enableDefaultCaptions() pre-sets previousSubtitleLanguage, suppressing the callback it's supposed to trigger.

fun enableDefaultCaptions() {
    val defaultTrack = getAvailableTextTracks().firstOrNull() ?: return
    previousSubtitleLanguage = defaultTrack.first   // set BEFORE calling setTextTrackByLanguage
    setTextTrackByLanguage(defaultTrack.first)
}

setTextTrackByLanguage (line 68) only fires onSubtitleStateChanged when language != previousSubtitleLanguage. Because enableDefaultCaptions() already assigned previousSubtitleLanguage = defaultTrack.first on line 92, the guard on line 68 evaluates to false inside the very next call, so the callback never fires when captions are auto-enabled.

This path is not a rare edge case — it's the primary flow wired up in TPStreamsPlayer.kt:424-426:

if (showDefaultCaptions && isPrepared && textTracks.isNotEmpty()) {
    enableDefaultCaptions()
}

Any host app that opts into showDefaultCaptions = true (a public, existing constructor param) and wants to persist the user's caption preference via this new event will silently miss the initial "subtitles turned ON to language X" notification — exactly the use case described in the PR body. Explicit user-initiated calls to setTextTrackByLanguage (e.g. from the captions bottom sheet) are unaffected and work correctly.

Fix: remove the premature assignment on line 92 and let setTextTrackByLanguage own previousSubtitleLanguage exclusively:

fun enableDefaultCaptions() {
    val defaultTrack = getAvailableTextTracks().firstOrNull() ?: return
    setTextTrackByLanguage(defaultTrack.first)
}

Fix this →

Non-blocking observation:

• app/src/main/java/com/tpstreams/player/MainActivity.kt:138 — sample-app-only hardcoded demo token; not a product issue.

Regression Assessment

No regressions in playback, DRM, downloads, seeking, general track selection, or player lifecycle — the change is additive. The bug above is isolated to the new default-captions notification path and does not affect existing subtitle selection behavior.

Final Verdict

❌ CHANGES REQUESTED

The enableDefaultCaptions() ordering bug directly undermines the PR's stated goal — host apps enabling showDefaultCaptions won't be notified when captions are auto-enabled, which is likely the most common real-world trigger for this event. This should be fixed before merge.

[claude]

Claude finished @Balamurugan-100's task in 46s —— View job

---

Todo

• Gather context (PR diff, repo files)
• Check whether previously reported enableDefaultCaptions() bug was fixed
• Review changed files for correctness, regressions, lifecycle, thread-safety, API compatibility
• Post review summary

Summary

Re-reviewed after the latest push. Both issues raised in the prior review round have been addressed:

1. Fixed — enableDefaultCaptions() no longer suppresses the callback. TextTrackManager.kt:90-93 now reads:

   fun enableDefaultCaptions() {
       val defaultTrack = getAvailableTextTracks().firstOrNull() ?: return
       setTextTrackByLanguage(defaultTrack.first)
   }

   The premature previousSubtitleLanguage = defaultTrack.first assignment was removed, so setTextTrackByLanguage's language != previousSubtitleLanguage guard (TextTrackManager.kt:68) now correctly evaluates to true and fires onSubtitleStateChanged on the auto-enable path driven by showDefaultCaptions (TPStreamsPlayer.kt:424-425). This was the blocking bug — the primary real-world trigger for this event now works.

2. Fixed — demo token whitespace. MainActivity.kt:138 now sets "3d9838f3-db51-4fc3-8472-075ab5e40b64" with no trailing spaces.

Design/Implementation Check

• TPStreamsPlayer.Listener.onSubtitleStateChanged retains its no-op default body (TPStreamsPlayer.kt:92) — source/binary compatible with existing implementers.
• TextTrackManager dedupes on actual language change only (TextTrackManager.kt:67-71).
• TPStreamsPlayerView's listener-wrapping (TPStreamsPlayerView.kt:356-364, ViewPlayerListener.onSubtitleStateChanged at :630-632) correctly preserves and forwards to a listener set directly on TPStreamsPlayer before the view attaches — verified against the sample PlayerActivity.kt:32-40, which now sets the listener before assigning binding.playerView.player.
• All callback invocations occur on the player's application thread (same call path as before), so no new thread-safety concerns.
• No unrelated behavior changed; diff remains additive.

Regression Assessment

No regressions identified in playback, DRM, downloads, seeking, track selection, adaptive switching, or player lifecycle. The default-captions notification path — the bug found in the previous round — is now verified correct by code inspection.

Final Verdict

✅ APPROVED

No blocking correctness, regression, lifecycle, API compatibility, or security issues remain. Both previously flagged issues are resolved.