Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore(android): refactor drm props #3846

Merged
merged 22 commits into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
d3fc4fd
perf: ensure we do not provide callback to native if no callback prov…
freeboub May 5, 2024
e1da32d
chore: rework bufferConfig to make it more generic and reduce ReactEx…
freeboub May 6, 2024
df3da43
Merge branch 'master' of github.com:react-native-video/react-native-v…
freeboub May 8, 2024
0f33ad1
chore: improve issue template
freeboub May 8, 2024
17f2385
Merge branch 'TheWidlarzGroup:master' into master
freeboub May 10, 2024
1066898
fix(android): avoid video view flickering at playback startup
freeboub May 10, 2024
485f867
Merge branch 'master' of github.com:react-native-video/react-native-v…
freeboub May 11, 2024
a5e10a3
Merge branch 'master' of github.com:freeboub/react-native-video into …
freeboub May 11, 2024
9ce1d95
Merge branch 'master' of github.com:react-native-video/react-native-v…
freeboub May 15, 2024
fa27db7
Merge branch 'master' of github.com:react-native-video/react-native-v…
freeboub May 17, 2024
be681d3
Merge branch 'master' of github.com:react-native-video/react-native-v…
freeboub May 21, 2024
2fadb26
Merge branch 'master' of github.com:react-native-video/react-native-v…
freeboub May 23, 2024
92df10d
Merge branch 'master' of github.com:react-native-video/react-native-v…
freeboub May 24, 2024
aa61388
Merge branch 'master' of github.com:react-native-video/react-native-v…
freeboub May 28, 2024
41e2577
chore(android): refactor DRM props into a dedicated class
freeboub May 28, 2024
755b834
Update android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerV…
freeboub May 28, 2024
dc12ff8
chore: fix linter
freeboub May 28, 2024
cdb77e5
Merge branch 'chore/refactorDrmProps' of github.com:freeboub/react-na…
freeboub May 28, 2024
25eaedb
Merge branch 'master' of github.com:react-native-video/react-native-v…
freeboub May 30, 2024
1e155fc
fix: ensure drm prop is correctly cleaned
freeboub May 30, 2024
8b0e2a6
Merge branch 'master' of github.com:react-native-video/react-native-v…
freeboub Jun 7, 2024
3f37e6d
chore: revert unnecessary change
freeboub Jun 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 66 additions & 0 deletions android/src/main/java/com/brentvatne/common/api/DRMProps.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package com.brentvatne.common.api

import com.brentvatne.common.toolbox.ReactBridgeUtils.safeGetArray
import com.brentvatne.common.toolbox.ReactBridgeUtils.safeGetString
import com.facebook.react.bridge.ReadableMap
import java.util.UUID

/**
* Class representing DRM props for host.
* Only generic code here, no reference to the player.
*/
class DRMProps {
/**
* string version of configured UUID for drm prop
*/
var drmType: String? = null

/**
* Configured UUID for drm prop
*/
var drmUUID: UUID? = null

/**
* DRM license server to be used
*/
var drmLicenseServer: String? = null

/**
* DRM Http Header to access to license server
*/
var drmLicenseHeader: Array<String> = emptyArray<String>()
companion object {
private const val PROP_DRM_TYPE = "type"
private const val PROP_DRM_LICENSE_SERVER = "licenseServer"
private const val PROP_DRM_HEADERS = "headers"
private const val PROP_DRM_HEADERS_KEY = "key"
private const val PROP_DRM_HEADERS_VALUE = "value"

/** parse the source ReadableMap received from app */
@JvmStatic
fun parse(src: ReadableMap?): DRMProps? {
var drm: DRMProps? = null
if (src != null && src.hasKey(PROP_DRM_TYPE)) {
drm = DRMProps()
drm.drmType = safeGetString(src, PROP_DRM_TYPE)
drm.drmLicenseServer = safeGetString(src, PROP_DRM_LICENSE_SERVER)
val drmHeadersArray = safeGetArray(src, PROP_DRM_HEADERS)
if (drm.drmType != null && drm.drmLicenseServer != null) {
if (drmHeadersArray != null) {
val drmKeyRequestPropertiesList = ArrayList<String?>()
for (i in 0 until drmHeadersArray.size()) {
val current = drmHeadersArray.getMap(i)
drmKeyRequestPropertiesList.add(safeGetString(current, PROP_DRM_HEADERS_KEY))
drmKeyRequestPropertiesList.add(safeGetString(current, PROP_DRM_HEADERS_VALUE))
}
val array = emptyArray<String>()
drm.drmLicenseHeader = drmKeyRequestPropertiesList.toArray(array)
}
} else {
return null
}
}
return drm
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@
import com.brentvatne.common.api.BufferConfig;
import com.brentvatne.common.api.BufferingStrategy;
import com.brentvatne.common.api.ControlsConfig;
import com.brentvatne.common.api.DRMProps;
import com.brentvatne.common.api.ResizeMode;
import com.brentvatne.common.api.SideLoadedTextTrack;
import com.brentvatne.common.api.SideLoadedTextTrackList;
Expand Down Expand Up @@ -238,9 +239,7 @@ public class ReactExoplayerView extends FrameLayout implements
private float mProgressUpdateInterval = 250.0f;
private boolean playInBackground = false;
private boolean mReportBandwidth = false;
private UUID drmUUID = null;
private String drmLicenseUrl = null;
private String[] drmLicenseHeader = null;
private DRMProps drmProps;
private boolean controls;
private Uri adTagUrl;

Expand Down Expand Up @@ -774,10 +773,11 @@ private void initializePlayerCore(ReactExoplayerView self) {

private DrmSessionManager initializePlayerDrm(ReactExoplayerView self) {
DrmSessionManager drmSessionManager = null;
if (self.drmUUID != null) {
if (self.drmProps != null) {
try {
drmSessionManager = self.buildDrmSessionManager(self.drmUUID, self.drmLicenseUrl,
self.drmLicenseHeader);
drmSessionManager = self.buildDrmSessionManager(self.drmProps.getDrmUUID(),
self.drmProps.getDrmLicenseServer(),
self.drmProps.getDrmLicenseHeader());
} catch (UnsupportedDrmException e) {
int errorStringId = Util.SDK_INT < 18 ? R.string.error_drm_not_supported
: (e.reason == UnsupportedDrmException.REASON_UNSUPPORTED_SCHEME
Expand All @@ -794,7 +794,7 @@ private void initializePlayerSource() {
return;
}
DrmSessionManager drmSessionManager = initializePlayerDrm(this);
if (drmSessionManager == null && drmUUID != null) {
if (drmSessionManager == null && drmProps != null && drmProps.getDrmUUID() != null) {
// Failed to intialize DRM session manager - cannot continue
DebugLog.e(TAG, "Failed to initialize DRM Session Manager Framework!");
eventEmitter.error("Failed to initialize DRM Session Manager Framework!", new Exception("DRM Session Manager Framework failure!"), "3003");
Expand Down Expand Up @@ -2232,7 +2232,7 @@ public void setFullscreen(boolean fullscreen) {
}

public void setUseTextureView(boolean useTextureView) {
boolean finallyUseTextureView = useTextureView && this.drmUUID == null;
boolean finallyUseTextureView = useTextureView && drmProps == null;
exoPlayerView.setUseTextureView(finallyUseTextureView);
}

Expand All @@ -2259,16 +2259,11 @@ public void setBufferConfig(BufferConfig config) {
initializePlayer();
}

public void setDrmType(UUID drmType) {
this.drmUUID = drmType;
}

public void setDrmLicenseUrl(String licenseUrl){
this.drmLicenseUrl = licenseUrl;
}

public void setDrmLicenseHeader(String[] header){
this.drmLicenseHeader = header;
public void setDrm(DRMProps drmProps) {
this.drmProps = drmProps;
if (drmProps != null && drmProps.getDrmType() != null) {
this.drmProps.setDrmUUID(Util.getDrmUuid(drmProps.getDrmType()));
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

        if (drmProps != null && drmProps.getDrmType() != null) {
            this.drmProps.setDrmUUID(Util.getDrmUuid(drmProps.getDrmType()));
        }

I think above code could be moved inside the DrmProps.parse fn.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but my problem with that is that I will have to import Utils from media3 in drmProp.kt. I really want to avoid including player specific code in the utils / data management classes ...

Copy link
Collaborator

@YangJonghun YangJonghun Jun 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand. but for that reason, it seems like parse function should be categorized in a different class. (for example, DRMUtils)
And I think removing dependencies from Utils seems hard to avoid because there's so much to prepare to implement as the SDK requires.🥲

If you worried deep dependencies, I recommend copying media3's implementation.

However, I'm not suggesting this because I think it's a serious issue, so you don't have to fix it.

}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.brentvatne.common.api.BufferConfig;
import com.brentvatne.common.api.BufferingStrategy;
import com.brentvatne.common.api.ControlsConfig;
import com.brentvatne.common.api.DRMProps;
import com.brentvatne.common.api.ResizeMode;
import com.brentvatne.common.api.SideLoadedTextTrackList;
import com.brentvatne.common.api.Source;
Expand Down Expand Up @@ -39,9 +40,7 @@ public class ReactExoplayerViewManager extends ViewGroupManager<ReactExoplayerVi
private static final String PROP_SRC = "src";
private static final String PROP_AD_TAG_URL = "adTagUrl";
private static final String PROP_DRM = "drm";
private static final String PROP_DRM_TYPE = "type";
private static final String PROP_DRM_LICENSE_SERVER = "licenseServer";
private static final String PROP_DRM_HEADERS = "headers";
private static final String PROP_SRC_HEADERS = "requestHeaders";
private static final String PROP_RESIZE_MODE = "resizeMode";
private static final String PROP_REPEAT = "repeat";
private static final String PROP_SELECTED_AUDIO_TRACK = "selectedAudioTrack";
Expand Down Expand Up @@ -117,28 +116,9 @@ public void onDropViewInstance(ReactExoplayerView view) {

@ReactProp(name = PROP_DRM)
public void setDRM(final ReactExoplayerView videoView, @Nullable ReadableMap drm) {
if (drm != null && drm.hasKey(PROP_DRM_TYPE)) {
String drmType = ReactBridgeUtils.safeGetString(drm, PROP_DRM_TYPE);
String drmLicenseServer = ReactBridgeUtils.safeGetString(drm, PROP_DRM_LICENSE_SERVER);
ReadableArray drmHeadersArray = ReactBridgeUtils.safeGetArray(drm, PROP_DRM_HEADERS);
if (drmType != null && drmLicenseServer != null && Util.getDrmUuid(drmType) != null) {
UUID drmUUID = Util.getDrmUuid(drmType);
videoView.setDrmType(drmUUID);
videoView.setDrmLicenseUrl(drmLicenseServer);
if (drmHeadersArray != null) {
ArrayList<String> drmKeyRequestPropertiesList = new ArrayList<>();
for (int i = 0; i < drmHeadersArray.size(); i++) {
ReadableMap current = drmHeadersArray.getMap(i);
String key = current.hasKey("key") ? current.getString("key") : null;
String value = current.hasKey("value") ? current.getString("value") : null;
drmKeyRequestPropertiesList.add(key);
drmKeyRequestPropertiesList.add(value);
}
videoView.setDrmLicenseHeader(drmKeyRequestPropertiesList.toArray(new String[0]));
}
videoView.setUseTextureView(false);
}
}
DRMProps drmProps = DRMProps.parse(drm);
videoView.setDrm(drmProps);
videoView.setUseTextureView(false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if drm props is null, we have to follow injected prop (useSecureView or useTextureView)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes you are right, thank you. I am also wondering if it can make sense to use Surface View by default? What is your opinion?

Copy link
Collaborator

@YangJonghun YangJonghun May 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's hard question. because it depends on what user using this library for
I prefer SurfaceView from a user perspective. In most situations, this is beneficial.
However, it can cause to make issue threads being created by people who don't read the documentation or don't know much about Android.

Decision is yours🫡

Copy link
Collaborator Author

@freeboub freeboub May 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am working to remove this videoView.setUseTextureView(false);
It is a bit. The ugly solution is to add new variables recording the last calls to setUseTextureView and setUseSecureView and apply it in the else.

I would prefer implement it in another way:

  • move DRM & viewType inside the source prop.
  • It will decrease the bridge usage, avoid restarting playback if we receive DRM prop after source and make cleaner code.

I will try to implement it, but I think we can merge this part now, and the next fix will come soon !

I propose to merge this PR as the setUseTextureView is already present in the code !

Copy link
Collaborator Author

@freeboub freeboub May 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@YangJonghun here is The patch I would to do for ensure we correctly reset the view state in the app:
#3867
initially I didn't plan to it right now, but you make me accelerate :)
Still in draft as I need to do the same thing on ios, but that's the idea.
Let me know you thought please !
And ping @KrzysztofMoch

Copy link
Collaborator

@YangJonghun YangJonghun Jun 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I apologize for late response.
I left related comment in #3867 .

}

@ReactProp(name = PROP_SRC)
Expand Down
Loading