Fix Android crashes caused by attempts to use unsupported DRMs

[CastagnaIT]

Description

The crash is caused from the construction of CJNIMediaDrm with unsupported UUID/Keysystem

inputstream.adaptive/src/decrypters/widevineandroid/WVCdmAdapter.cpp

Lines 91 to 92 in f731fc1

	CJNIUUID uuid(mostSigBits, leastSigBits);
	m_cdmAdapter = std::make_shared<CJNIMediaDrm>(uuid);

this is because ISA assume that on android all UUIDs/keysystems are supported by default,
where instead is not so on all android devices

to solve the problem now DRMs are constructed all in one time
in order to check what KeySystems are supported from each DRM
and this require isCryptoSchemeSupported method support from JNI

depends from JNI PR: xbmc/libandroidjni#59

Motivation and context

i was testing playready videos on last NVIDIA Shield firmware and noticed that it causes Kodi to crash to launcher

by inspecting logcat looks like nvidia has removed PlayReady support from last firmware or they has broken the playready drm interface showing a bug regarding ISA

How has this been tested?

try play playready video on NVIDIA Shield with last firmware and instead of crashing, the playback is now stopped with a "DRM not supported" message

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

Consider: Refactored DRM initialization on Android to check crypto scheme support before instantiation, preventing crashes from unsupported DRMs. Reviewed: core logic, docs

Strengths

• ✅ Good defensive approach at src/decrypters/widevineandroid/WVCdmAdapter.cpp:373 - using isCryptoSchemeSupported static check before instantiation prevents the crash
• ✅ Clean refactoring at src/decrypters/DrmEngine.cpp:131-196 - centralizing DRM initialization and filtering unsupported systems

Observations

Impact

[CRITICAL] src/decrypters/DrmEngine.cpp (188-194): Shared DRM instance reused across multiple key systems
On Android, GetDecrypters() creates only one CWVDecrypterA instance that supports multiple key systems (Widevine/PlayReady/Wiseplay). This single instance gets added to m_drms multiple times with different key systems. When OpenDRMSystem is called (line 444), it overwrites m_WVCdmAdapter (WVDecrypter.cpp:69), potentially breaking sessions for other active key systems that share the same DRM instance.

[MAJOR] src/decrypters/DrmEngine.cpp (441-444): Race condition on shared DRM instance
IsInitialised() check followed by OpenDRMSystem() is not atomic. If the shared DRM instance is being used by multiple sessions with different key systems, one thread could pass the check while another is in the process of calling OpenDRMSystem, causing state corruption.

[MAJOR] src/decrypters/DrmEngine.cpp (176-184): Silent failure when all DRMs fail initialization
When Initialize() fails for all DRMs, the loop silently erases them and leaves m_drms empty. Later calls to GetDrmInstance() return nullptr, but the root cause (why initialization failed) is lost. Consider logging a summary error if all DRMs fail.

Suggestions

• Future consideration: You might want to consider whether Android should create separate DRM instances per key system instead of sharing one instance, to avoid the state collision issue identified above.

Verdict

🔴 Address before merging -- 3 blocking issue(s) found (CRITICAL/MAJOR)

Review Details

• Files reviewed: 17
• Lines changed: +193 -77
• Profile: balanced (auto, lines changed: 270)
• Author: core (adapted tone)
• Findings: 1 critical, 2 major, 0 medium, 0 minor (includes 3 from summary observations)
• Review completed: 2026-03-15T13:58:32.648Z
• Keyword parsing:
  • breaking change in body

[github-actions]

Review

Must Fix

• Mutable dependency ref (depends/android/libandroidjni/libandroidjni.txt:1) — Points to a personal fork branch via a branch-name tarball URL. Branch tarballs are mutable; a force-push silently changes content while the SHA256 becomes stale. Must be updated to the upstream xbmc/libandroidjni repo pinned to a specific commit SHA before merging.

• DRMInstance::keySystem is std::string_view (DrmEngineDefines.h:128) — When Initialize() reorders keySystemsPrio using GetDrmConfigs() entries, it replaces constexpr KS_* views with views pointing into the config map's std::string keys. These views are then stored long-term in m_drms. Any config map reload or destruction yields dangling views. Change keySystem to std::string.

Medium

• Faulty already-initialized guard (DrmEngine.cpp:133) — !m_drms.empty() cannot distinguish between not yet initialized and initialized but no DRM supported any key system. In the second case, every subsequent call to PreInitializeDRM/InitializeSession re-runs the full initialization loop. Add a dedicated m_initialized bool flag.

Minor

• Typo (DrmEngine.h:127) — Comment says Choosen, should be Chosen.

[CastagnaIT]

@kodiai about your first draft review i have tried to address the reported issues
despite the fact that only one keysystem at time can be used to play a stream,
to note that i reworked the OpenDRMSystem method to avoid that adapters will be overwritten by new calls

please do another draft review

[kodiai]

kodiai response

Good progress on addressing the previous issues. Here's the status:

Fixed Issues

(1) std::string_view lifetime — The keySystem field in DRMInstance at src/decrypters/DrmEngineDefines.h:128 is now std::string instead of std::string_view, fixing the dangling reference issue when GetDrmConfigs() entries were used to reorder keysystems.

(2) Adapter reuse logic — At src/decrypters/widevineandroid/WVDecrypter.cpp:69-83, adapters are now cached in m_wvAdapters map (defined at src/decrypters/widevineandroid/WVDecrypter.h:95) and reused by keysystem, preventing the crash from recreating MediaDrm objects with unsupported UUIDs.

(3) isCryptoSchemeSupported check — At src/decrypters/widevineandroid/WVCdmAdapter.cpp:365-374, the static check is correctly used before instantiation.

Remaining Issues

(1) [CRITICAL] Mutable dependency — depends/android/libandroidjni/libandroidjni.txt:1 still points to a personal fork branch tarball (https://github.com/CastagnaIT/libandroidjni/archive/refs/heads/crypto_support.tar.gz). Branch tarballs are mutable; any force-push changes the content while the SHA256 becomes stale. This must be changed to the upstream xbmc/libandroidjni repo pinned to a specific commit SHA before merge.

(2) [MINOR] Adapter map not cleared — src/decrypters/widevineandroid/WVDecrypter.h:95 shows m_wvAdapters is never cleared in any destructor or dispose method. While shared_ptr lifetime management may handle cleanup, explicit clearing in a dispose/cleanup path would be more maintainable.

Next Step

The main blocker is the mutable dependency URL. Once the libandroidjni PR is merged upstream, update the dependency to use a commit-pinned tarball from xbmc/libandroidjni.

[CastagnaIT]

@kodiai i updated the PR code, please review

[kodiai]

kodiai response

Good work addressing the previous issues. The core fix looks solid - the isCryptoSchemeSupported check at src/decrypters/widevineandroid/WVCdmAdapter.cpp:373 successfully prevents crashes from unsupported DRMs, and the adapter caching at src/decrypters/widevineandroid/WVDecrypter.cpp:73-88 prevents re-creating MediaDrm objects.

Remaining Critical Issue

(1) [critical] depends/android/libandroidjni/libandroidjni.txt:1 — Still points to mutable branch tarball https://github.com/CastagnaIT/libandroidjni/archive/refs/heads/crypto_support.tar.gz. Branch tarballs change if the branch is force-pushed, making the SHA256 stale and the build unreproducible. Before merging, this must point to upstream xbmc/libandroidjni pinned to a specific commit SHA (e.g., archive/{commit-sha}.tar.gz).

Observations

(2) [minor] src/decrypters/DrmEngine.cpp:104 — Early-return check if (!m_drms.empty()) assumes the engine is already initialized, but if a prior Initialize() call failed partway through (e.g., decrypter path missing at line 138-145), m_drms could be non-empty yet the engine is incomplete. Consider also checking m_status != EngineStatus::DRM_ERROR or clearing m_drms on failure paths.

(3) [info] src/decrypters/DrmEngine.cpp:113 — The comment says "ClearKey is set as last since real DRMs should be preferred" which aligns with the default priority order.

All previously reported issues have been addressed. Once the dependency reference is fixed to point to a stable commit SHA, this should be ready.

[github-actions]

Review

Must Fix

Missing JNI exception check after isCryptoSchemeSupported (src/decrypters/widevineandroid/WVCdmAdapter.cpp:373)
CJNIMediaDrm::isCryptoSchemeSupported() can throw a JNI exception (e.g. on older API levels or unsupported schemes). Unlike all other JNI calls in this file, there is no xbmc_jnienv()->ExceptionCheck() / ExceptionClear() guard. A pending exception will cause every subsequent JNI call in the same thread to fail immediately, meaning if the first key system check throws, all remaining checks in the Initialize() loop will silently fail and no DRM will be detected as supported.

---

Minor

HasLicenseKey returns false instead of std::nullopt on cast failure (src/decrypters/clearkey/ClearKeyDecrypter.cpp:49)
When decrypter is null or the cast fails, returning false signals "key definitely not present" to callers (e.g. InitializeSession at DrmEngine.cpp:393). The intended semantics for an error/unknown state is std::nullopt.

virtual + override used together (src/decrypters/widevineandroid/WVDecrypter.h:33,48,50,55,... and src/decrypters/widevine/WVDecrypter.h:19-58)
Code guidelines prohibit using both virtual and override simultaneously — use override alone.