feat(iOS): rewrite DRM Module #4136

KrzysztofMoch · 2024-09-02T13:34:40Z

Summary

I have re-implemented DRM module for Apple devices. New Implementation use AVContentKeySession instead of resource loader.

fixes #4177 #3675 #3467

Motivation

Motivation is to fix issue with DRM Airplay and add visionOS support (in old implementation it was not possible)

Test plan

I have test app with Fairplay stream (which unfortunately I cannot share)
Also to play DRM content you need to have real device, simulator will not work

Tested on

iOS is tested by me
tvOS is not tested and I don't have real device to do it
visionOS is not tested and I don't have real device to do it

ios/Video/Features/DRMManager+AVContentKeySessionDelegate.swift

ios/Video/Features/RCTVideoErrorHandling.swift

KrzysztofMoch · 2024-09-04T15:59:33Z

@freeboub should I add someone else to reviewers ?

freeboub · 2024-09-04T16:39:15Z

Maybe @paul-rinaldi or @paulrinaldi would like to test this version ? I can review but not test on my side ...

ios/Video/Features/DRMManager+AVContentKeySessionDelegate.swift

ios/Video/Features/RCTResourceLoaderDelegate.swift

ios/Video/Features/RCTVideoDRM.swift

ios/Video/RCTVideo.swift

freeboub · 2024-09-04T16:50:31Z

I added some small comments, but it looks like good job 👍
If you are looking for other testers, you can try to ping last issue openers which are linked to DRM on ios. they may have a look to the PR or test it !

paul-rinaldi · 2024-09-04T16:55:57Z

I added some small comments, but it looks like good job 👍
If you are looking for other testers, you can try to ping last issue openers which are linked to DRM on ios. they may have a look to the PR or test it !

I don't have a tv with iOS on it or vision OS. I have physical iPhones and physical iPads I could test with if you would like me to but it sounds like the PR creator already did that.

Sorry I could not be more helpful.

freeboub · 2024-09-04T18:22:02Z

I added some small comments, but it looks like good job 👍
If you are looking for other testers, you can try to ping last issue openers which are linked to DRM on ios. they may have a look to the PR or test it !

I don't have a tv with iOS on it or vision OS. I have physical iPhones and physical iPads I could test with if you would like me to but it sounds like the PR creator already did that.

Sorry I could not be more helpful.

My proposal was linked to your message on discord in fact !

rohankm · 2024-09-06T08:51:30Z

I added some small comments, but it looks like good job 👍
If you are looking for other testers, you can try to ping last issue openers which are linked to DRM on ios. they may have a look to the PR or test it !

I have tested it and it works fine also it solved buffering issues in drm videos.

src/Video.tsx

freeboub · 2024-09-06T13:35:48Z

Looks good to me. Please build a release after merging this PR :)

KrzysztofMoch · 2024-09-06T13:37:42Z

Sure I will give this PR to tests for our client and after it will do it! Thanks!

rohankm · 2024-09-18T00:49:00Z

Waiting for the merge

ios/Video/RCTVideo.swift

docs/pages/component/props.mdx

KrzysztofMoch · 2024-09-18T15:38:13Z

ios/Video/RCTVideo.swift

-        if source.drm != nil || _localSourceEncryptionKeyScheme != nil {
-            _resouceLoaderDelegate = RCTResourceLoaderDelegate(
+        if source.drm?.type != nil {


@freeboub source.drm is never nil is structure with nils even if we don't pass anything are you okay with checking drm.type ?

I am not sure to understand, you want to change
let drm: DRMParams?
by
let drm: DRMParams

And check only type ?
if that's the case, I agree !

Something like this patch:

diff --git a/ios/Video/DataStructures/VideoSource.swift b/ios/Video/DataStructures/VideoSource.swift index e672929f..45e5b2d3 100644 --- a/ios/Video/DataStructures/VideoSource.swift +++ b/ios/Video/DataStructures/VideoSource.swift @@ -10,7 +10,7 @@ struct VideoSource { let cropEnd: Int64? let customMetadata: CustomMetadata? /* DRM */ - let drm: DRMParams? + let drm: DRMParams var textTracks: [TextTrack] = [] let json: NSDictionary? @@ -28,7 +28,7 @@ struct VideoSource { self.cropStart = nil self.cropEnd = nil self.customMetadata = nil - self.drm = nil + self.drm = DRMParams(nil) return } self.json = json diff --git a/ios/Video/RCTVideo.swift b/ios/Video/RCTVideo.swift index 908e3eb8..64130675 100644 --- a/ios/Video/RCTVideo.swift +++ b/ios/Video/RCTVideo.swift @@ -421,7 +421,7 @@ class RCTVideo: UIView, RCTVideoPlayerViewControllerDelegate, RCTPlayerObserverH "type": _source?.type ?? NSNull(), "isNetwork": NSNumber(value: _source?.isNetwork ?? false), ], - "drm": source.drm?.json ?? NSNull(), + "drm": source.drm.json ?? NSNull(), "target": reactTag as Any, ])

I meant:
in current impl source.drm is never null it will be always struct with some values (if drm is not setted but user all values will be just null). But now as I think I can aloso check if source.drm.json != nil bcs if json is null it means that users notting passed to it

agastiazoro · 2024-09-18T22:34:24Z

ios/Video/Features/RCTVideoErrorHandling.swift

+        case .unsupportedDRMType:
+            return NSLocalizedString("Verifiy that you are using fairplay (on Apple devices)", comment: "")
+        case .simulatorDRMNotSuported:
+            return NSLocalizedString("You need to test DRM conent on real device", comment: "")


@KrzysztofMoch there is a typo in "content"

maksymhcode-care · 2024-09-19T05:05:38Z

Is this ready to be merged?

freeboub · 2024-09-19T10:37:29Z

ios/Video/RCTVideo.swift

@@ -1329,11 +1326,11 @@ class RCTVideo: UIView, RCTVideoPlayerViewControllerDelegate, RCTPlayerObserverH
    }

    func setLicenseResult(_ license: String!, _ licenseUrl: String!) {


@KrzysztofMoch I am reviewing use of String! and it is a bad practice and can lead to some issues, see: #4181

It is just hide the warning, so no need to test the value ...
I think it would be better to change like I propose (not sure of the consequences inside _drmManager)

Suggested change

func setLicenseResult(_ license: String!, _ licenseUrl: String!) {

func setLicenseResult(_ license: String?, _ licenseUrl: String?) {

freeboub · 2024-09-19T10:37:49Z

ios/Video/RCTVideo.swift

@@ -1329,11 +1326,11 @@ class RCTVideo: UIView, RCTVideoPlayerViewControllerDelegate, RCTPlayerObserverH
    }

    func setLicenseResult(_ license: String!, _ licenseUrl: String!) {
-        _resouceLoaderDelegate?.setLicenseResult(license, licenseUrl)
+        _drmManager?.setJSLicenseResult(license: license, licenseUrl: licenseUrl)
    }

    func setLicenseResultError(_ error: String!, _ licenseUrl: String!) {


Same here:

Suggested change

func setLicenseResultError(_ error: String!, _ licenseUrl: String!) {

func setLicenseResultError(_ error: String?, _ licenseUrl: String?) {

agastiazoro · 2024-09-19T13:15:03Z

ios/Video/Features/RCTVideoErrorHandling.swift

+        case .offlineDRMNotSuported:
+            return NSLocalizedString("You tried to use Offline DRM but it is not supported yet", comment: "")
+        case .unsupportedDRMType:
+            return NSLocalizedString("You tried to use unsupporeted DRM type", comment: "")


@KrzysztofMoch there is a typo in "unsupported"

freeboub reviewed Sep 2, 2024

View reviewed changes

ios/Video/Features/DRMManager+AVContentKeySessionDelegate.swift Show resolved Hide resolved

freeboub reviewed Sep 2, 2024

View reviewed changes

ios/Video/Features/RCTVideoErrorHandling.swift Show resolved Hide resolved

moskalakamil mentioned this pull request Sep 2, 2024

[BUG]: IOS- keeps buffering #3675

Open

KrzysztofMoch changed the title ~~[WIP] feat(iOS): rewrite DRM Module~~ feat(iOS): rewrite DRM Module Sep 4, 2024

KrzysztofMoch marked this pull request as ready for review September 4, 2024 15:58

freeboub reviewed Sep 4, 2024

View reviewed changes

ios/Video/Features/DRMManager+AVContentKeySessionDelegate.swift Outdated Show resolved Hide resolved

freeboub reviewed Sep 4, 2024

View reviewed changes

ios/Video/Features/RCTResourceLoaderDelegate.swift Show resolved Hide resolved

freeboub reviewed Sep 4, 2024

View reviewed changes

ios/Video/Features/RCTVideoDRM.swift Show resolved Hide resolved

freeboub reviewed Sep 4, 2024

View reviewed changes

ios/Video/RCTVideo.swift Show resolved Hide resolved

KrzysztofMoch force-pushed the feat/ios-rewrite-drm branch from 3bd5834 to 6be407d Compare September 6, 2024 10:10

freeboub reviewed Sep 6, 2024

View reviewed changes

src/Video.tsx Show resolved Hide resolved

KrzysztofMoch force-pushed the feat/ios-rewrite-drm branch from 4cb8b05 to 516335d Compare September 6, 2024 13:16

freeboub approved these changes Sep 6, 2024

View reviewed changes

KrzysztofMoch force-pushed the feat/ios-rewrite-drm branch from 516335d to 59ba0fd Compare September 9, 2024 13:48

freeboub mentioned this pull request Sep 9, 2024

Move startup props into a single configuration #2693

Open

7 tasks

freeboub mentioned this pull request Sep 17, 2024

[BUG]: IOS DRM Custom License Request Not Working #4177

Open

KrzysztofMoch added 5 commits September 18, 2024 07:55

minimal api

fd2f106

add suport for getLicense

689d8d6

update logic for obtaining assetId

346ece9

add support for localSourceEncryptionKeyScheme

3117953

fix typo

b38be40

KrzysztofMoch added 13 commits September 18, 2024 07:55

fix pendingLicenses key bug

bbb96bb

lint code

75678c4

code clean

31b2ddd

code clean

cf74c1b

remove old files

ad950f4

fix tvOS build

7dcdb48

fix errors loop

a7f2f28

move localSourceEncryptionKeyScheme into drm params

07836fd

add check for drm type

a7957e4

use DebugLog

f74e2a6

lint

b915f13

update docs

059b09a

lint code

2dedcd2

KrzysztofMoch force-pushed the feat/ios-rewrite-drm branch from 59ba0fd to 2dedcd2 Compare September 18, 2024 06:01

freeboub reviewed Sep 18, 2024

View reviewed changes

ios/Video/RCTVideo.swift Outdated Show resolved Hide resolved

freeboub reviewed Sep 18, 2024

View reviewed changes

docs/pages/component/props.mdx Outdated Show resolved Hide resolved

fix bad rebase

e41a178

KrzysztofMoch force-pushed the feat/ios-rewrite-drm branch from 346f441 to e41a178 Compare September 18, 2024 08:34

update docs

c1afb61

freeboub self-requested a review September 18, 2024 14:35

freeboub approved these changes Sep 18, 2024

View reviewed changes

fix crashes on simulators

c60756c

KrzysztofMoch commented Sep 18, 2024

View reviewed changes

show error on simulator when using DRM

85a027c

KrzysztofMoch force-pushed the feat/ios-rewrite-drm branch from ead3023 to 85a027c Compare September 18, 2024 20:06

agastiazoro reviewed Sep 18, 2024

View reviewed changes

freeboub reviewed Sep 19, 2024

View reviewed changes

agastiazoro reviewed Sep 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(iOS): rewrite DRM Module #4136

feat(iOS): rewrite DRM Module #4136

KrzysztofMoch commented Sep 2, 2024 •

edited

Loading

KrzysztofMoch commented Sep 4, 2024

freeboub commented Sep 4, 2024

freeboub commented Sep 4, 2024

paul-rinaldi commented Sep 4, 2024

freeboub commented Sep 4, 2024

rohankm commented Sep 6, 2024

freeboub commented Sep 6, 2024

KrzysztofMoch commented Sep 6, 2024

rohankm commented Sep 18, 2024

KrzysztofMoch Sep 18, 2024 •

edited

Loading

freeboub Sep 18, 2024

KrzysztofMoch Sep 18, 2024 •

edited

Loading

agastiazoro Sep 18, 2024

maksymhcode-care commented Sep 19, 2024

freeboub Sep 19, 2024

freeboub Sep 19, 2024

agastiazoro Sep 19, 2024

		@@ -1329,11 +1326,11 @@ class RCTVideo: UIView, RCTVideoPlayerViewControllerDelegate, RCTPlayerObserverH
		}

		func setLicenseResult(_ license: String!, _ licenseUrl: String!) {

	func setLicenseResult(_ license: String!, _ licenseUrl: String!) {
	func setLicenseResult(_ license: String?, _ licenseUrl: String?) {

	func setLicenseResultError(_ error: String!, _ licenseUrl: String!) {
	func setLicenseResultError(_ error: String?, _ licenseUrl: String?) {

feat(iOS): rewrite DRM Module #4136

Are you sure you want to change the base?

feat(iOS): rewrite DRM Module #4136

Conversation

KrzysztofMoch commented Sep 2, 2024 • edited Loading

Summary

Motivation

Test plan

Tested on

KrzysztofMoch commented Sep 4, 2024

freeboub commented Sep 4, 2024

freeboub commented Sep 4, 2024

paul-rinaldi commented Sep 4, 2024

freeboub commented Sep 4, 2024

rohankm commented Sep 6, 2024

freeboub commented Sep 6, 2024

KrzysztofMoch commented Sep 6, 2024

rohankm commented Sep 18, 2024

KrzysztofMoch Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

freeboub Sep 18, 2024

Choose a reason for hiding this comment

KrzysztofMoch Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

agastiazoro Sep 18, 2024

Choose a reason for hiding this comment

maksymhcode-care commented Sep 19, 2024

freeboub Sep 19, 2024

Choose a reason for hiding this comment

freeboub Sep 19, 2024

Choose a reason for hiding this comment

agastiazoro Sep 19, 2024

Choose a reason for hiding this comment

KrzysztofMoch commented Sep 2, 2024 •

edited

Loading

KrzysztofMoch Sep 18, 2024 •

edited

Loading

KrzysztofMoch Sep 18, 2024 •

edited

Loading