Remove global AVPlayer instance #382
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ephemer
approved these changes
Dec 8, 2022
| } | ||
| } | ||
|
|
||
| exoPlayer.addListener(listener) | ||
| } | ||
|
|
||
| fun setUserContext(ctx: Long) { | ||
| this.userContext = ctx |
There was a problem hiding this comment.
Can you explain the naming of userContext? If the idea based on C APIs which take a void * userCtx parameter, then I think the naming might be too generic for what it really is.
From my current understanding, I would just call this "pointerToSwiftAVPlayerInstance" or something equally specific. userContext in the C meaning of the word is something very generic that could be anything at all, which is why it's usually defined as void*. That doesn't really fit our use case here 🙏🏼
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #
Type of change:
Motivation (current vs expected behavior)
We have a global AVPlayer instance in order to be able to reference our AVPlayer inside JNI callbacks. While this works it's not a good practice and wouldn't work anymore once there are more than on AVPlayer instances.
This PR saves the memory address of our AVPlayer inside the kotlin implementation (userContext) which we can use to later retrieve our player instance in the native callbacks.
Please check if the PR fulfills these requirements