[Widevine][android] Implementing KID status callback

[CastagnaIT]

Description

allow to keep track of the status of the session KIDs
at each license update will trigger OnKeyStatusChange callback
this in the future it may also be useful for better managing the widevine key rotation feature
and should allow to implement a real solution for issue #1973

this will rework part of relative key status change callbacks also on non-android (CDM) interface
each adapter now has its own status "management"

on android wv interface allowed to implement the missing HasLicenseKey/HasKeyId methods

this PR depends from:
xbmc/libandroidjni#55
xbmc/xbmc#27923

Motivation and context

never implemented from early versions of this add-on a long standing todo
non-android widevine interface (CDM) already has this feature implemented in similar way

How has this been tested?

Screenshots (if appropriate):

Types of change

• Bug fix (non-breaking change which fixes an issue)
• Clean up (non-breaking change which removes non-working, unmaintained functionality)
• Improvement (non-breaking change which improves existing functionality)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that will cause existing functionality to change)
• Cosmetic change (non-breaking change that doesn't touch code)
• None of the above (please explain below)

Checklist:

• I have read the Contributing document
• My code follows the Code Guidelines of this project
• My change requires a change to the Wiki documentation
• I have updated the documentation accordingly

[kodiai]

📝 Kodiai Draft Review Summary

Draft — This PR is still in draft. Feedback is exploratory; findings use softer language.

What Changed

Implements KID (Key ID) status change callbacks for Android Widevine to track session key statuses through OnKeyStatusChange events, mirroring functionality already present in non-Android CDM implementations.

Reviewed: core logic, docs

Strengths

• ✅ Preserves existing exception handling pattern consistently across both listener implementations (lines 102-108, 114-120)
• ✅ Uses RAII with std::unique_ptr for listener lifecycle management, preventing manual memory management errors

Observations

Impact

[MAJOR] src/decrypters/widevineandroid/WVCdmAdapter.cpp (240, 248): Null-terminated string assumption on binary data
Consider: sessionId is a std::vector<char> containing binary data without guaranteed null termination. Using sessionId.data() in %s format specifier at line 240 causes undefined behavior when the logging function expects null-terminated strings. Similarly, the loop at line 248 uses signed int i with keyInformation.size() which returns size_t, creating a type mismatch that could cause issues with large collections.

[MAJOR] src/decrypters/widevineandroid/WVCdmAdapter.cpp (109, 122): Exception check performed after listener assignment
Consider: The exception check at line 102 occurs after creating the listener but before setOnEventListener call at line 109. However, you might want to check for exceptions after the setOnEventListener call as well, since JNI calls can fail. The same pattern appears at lines 114-122 where setOnKeyStatusChangeListener is called at line 122 without a subsequent exception check.

[MEDIUM] depends/android/libandroidjni/libandroidjni.txt (1): Non-reproducible dependency reference
Consider: Using refs/heads/mediadrm_keys instead of a specific commit hash makes builds non-reproducible and potentially unstable. Once the PR xbmc/libandroidjni#55 is merged, you might want to reference a specific tagged release or commit SHA.

Suggestions

• Optional: Consider moving CJNIMediaDrmKeyStatus::PopulateStaticFields() to an earlier initialization phase rather than constructor, as suggested by the inline comment at line 121
• Future consideration: The OnKeyStatusChange implementation currently only logs key statuses. You might want to implement the actual KID tracking mechanism and HasLicenseKey/HasKeyId functionality mentioned in the PR description

Verdict

🔴 Address before merging -- 2 blocking issue(s) found

Review Details

• Files reviewed: 4
• Lines changed: +102 -3
• Profile: balanced (auto, lines changed: 105)
• Author: core (adapted tone)
• Findings: 0 critical, 2 major, 1 medium, 0 minor (includes 3 from summary observations)
• Review completed: 2026-02-28T08:47:34.719Z
• Keyword parsing:
  • focus hints: [WIDEVINE], [ANDROID]
  • breaking change in body

[github-actions]

Review

Must Fix | Exception check regression (WVCdmAdapter.cpp:109)
The existing exception check (lines 102–108) was moved to check make_unique instead of setOnEventListener. If setOnEventListener throws a JNI exception it now goes unchecked. Same issue applies to setOnKeyStatusChangeListener at line 122 — no ExceptionCheck after the call.

---

Critical | Undefined behavior: non-null-terminated vector<char> passed to %s (WVCdmAdapter.cpp:240)

LOG::LogF(LOGDEBUG, "MediaDrm OnKeyStatusChange event: SID %s", sessionId.data());

std::vector<char>::data() is NOT null-terminated. Passing to a %s format specifier is UB and will read past the buffer. Convert to a hex string or use STRING::ToHexadecimal as done for KIDs on line 254.

---

Must Fix | Mutable branch URL in dependency (libandroidjni.txt:1)
refs/heads/mediadrm_keys points to a live branch HEAD — the tarball content can change at any time while the sha256 hash will become stale, breaking builds or silently pulling in different code. Must pin to a specific commit hash (like the previous entry).

---

Medium | Wrong log level for normal key status events (WVCdmAdapter.cpp:253)
Key status changes are expected/normal DRM lifecycle events, yet they're logged at LOGWARNING. Should be LOGDEBUG.

---

Minor | Signed/unsigned comparison in loop (WVCdmAdapter.cpp:248)
int i compared against keyInformation.size() (unsigned/jsize type). Use size_t or a range-for loop.

[CastagnaIT]

@kodiai i updated the PR, please review

[kodiai]

kodiai response

Good progress! The exception checking issues from the previous review have been fixed (WVCdmAdapter.cpp:102-109, 115-122). However, there are still some items to address before this can be approved:

Issues:

(1) [major] src/decrypters/widevineandroid/WVCencSingleSampleDecrypter.cpp (213): The OnKeyStatusChangeNotify implementation is incomplete — there's a TODO placeholder and the received key status information isn't being stored. The non-Android version (src/decrypters/widevine/WVCencSingleSampleDecrypter.cpp:396-415) stores this in a m_keys member variable and updates it when status changes arrive.

(2) [major] src/decrypters/widevineandroid/WVCencSingleSampleDecrypter.h: Missing HasKeyId() method and m_keys storage. The non-Android version has both:

• bool HasKeyId(const std::vector<uint8_t>& keyid) method (declared at line 48 in widevine/WVCencSingleSampleDecrypter.h)
• std::vector<DRM::KeyInfo> m_keys; member (line 101 in widevine/WVCencSingleSampleDecrypter.h)

Without these, the Android implementation can't track KID statuses or support HasLicenseKey functionality mentioned in the PR description.

(3) [major] src/decrypters/widevineandroid/WVDecrypter.h: Missing HasLicenseKey() method. The non-Android version (src/decrypters/widevine/WVDecrypter.cpp:124-134) implements this to support DRM session reuse logic (see src/decrypters/DrmEngine.cpp:388).

Next step: Implement the missing key tracking infrastructure (m_keys storage, OnKeyStatusChangeNotify update logic, HasKeyId method) and the HasLicenseKey wrapper in WVDecrypterA to match the non-Android implementation pattern.

[github-actions]

Code Review

Critical - Two bugs found in the CDM v2 key status path (missing emplace_back + SendOnKeyStatusChange inside the loop).

Must Fix - HasLicenseKey error path returns false instead of nullopt.

Medium - Semantic mismatch: kReleased maps to EXPIRED.

Minor - Constructor parameter name inconsistency in WVCdmAdapter.h.

[CastagnaIT]

@kodiai updated pr with fixes, please review, and ignore dependecy tarball link is not updated yet

[kodiai]

kodiai response

Decision: APPROVE

Issues: none