feat(android): replace deprecated ExoPlayer2 with AndroidX media3 #3337

YangJonghun · 2023-11-08T14:11:13Z

Update the changelog

feat(android): replace deprecated ExoPlayer2 with AndroidX media3

Describe the changes

replace deprecated ExoPlayer2 with AndroidX media3
remove AndroidX dependencies
remove okhttp dependency
remove duplicate gradle function named getExtOrDefault
remove duplicate class named ReactBridgeUtils
Indent and align some Java codes

FYI)

This PR is based on v6.0.0-alpha.9 code
official migration guide

android/src/main/java/com/brentvatne/receiver/AudioBecomingNoisyReceiver.java

android/build.gradle

freeboub · 2023-11-08T17:48:54Z

Thank you for the PR, it looks easy to merge ! Just 2 small comments to check

KrzysztofMoch

@YangJonghun please do rebase. On master there are important changes like move versions to gradle.properties. Thanks for your PR 🙏

android/build.gradle

Conflicts: android/build.gradle android/src/main/java/com/brentvatne/common/react/VideoEventEmitter.java

freeboub · 2023-11-08T20:55:56Z

android/src/main/java/com/brentvatne/ReactBridgeUtils.java

@@ -1,22 +0,0 @@
-package com.brentvatne;


Can you please restore this file.
It will be used soon !
Thank you

freeboub · 2023-11-08T20:56:54Z

android/src/main/java/com/brentvatne/common/API/SubtitleStyle.kt

@@ -1,6 +1,6 @@
 package com.brentvatne.common.API

-import com.brentvatne.ReactBridgeUtils
+import com.brentvatne.common.toolbox.ReactBridgeUtils


look like you move the file, but not re add it ?

Actually I just connect ReactBridgeUtils.kt and remove old file(ReactBridgeUtils.java)
these files are already existed

Ok thank you, good catch then !

freeboub · 2023-11-08T20:58:00Z

android/src/main/java/com/brentvatne/exoplayer/ExoPlayerView.java

@@ -266,7 +264,7 @@ public void onCues(List<Cue> cues) {
            subtitleLayout.setCues(cues);
        }

-        // ExoPlayer.VideoListener implementation
+        // SimpleExoPlayer.VideoListener implementation


Suggested change

// SimpleExoPlayer.VideoListener implementation

// ExoPlayer.VideoListener implementation

Hmm.. I think we don't need comments anymore. because ComponentListener implement only one interface (It was required in v5)

If you don't want to remove it, please revert a29d85d commit

freeboub · 2023-11-08T21:05:20Z

one last point, and the code is Ok for me (I will need to test it also)

KrzysztofMoch · 2023-11-09T13:30:22Z

@YangJonghun It's me or native controls don't work ?

freeboub · 2023-11-09T21:58:22Z

@YangJonghun It's me or native controls don't work ?

I confirm that controls are broken.
At boot we receive this message in the console

LOG {"error":{"errorStackTrace":"android.view.InflateException: Binary XML file line #63 in com.videoplayer:layout/exo_player_control_view: Binary XML file line #63 in com.videoplayer:layout/exo_player_control_view: Error inflating class com.google.android.exoplayer2.ui.DefaultTimeBar\nCaused by: android.view.InflateException: Binary XML file line #63 in com.videoplayer:layout/exo_player_control_view: Error inflating class com.google.android.exoplayer2.ui.DefaultTimeBar\nCaused by: java.lang.ClassNotFoundException: com.google.android.exoplayer2.ui.DefaultTimeBar\n\tat java.lang.Class.classForName(Native Method)\n\tat java.lang.Class.forName(Class.java:454)\n\tat android.view.LayoutInflater.createView(LayoutInflater.java:819)\n\tat android.view.LayoutInflater.createViewFromTag(LayoutInflater.java:1010)\n\tat android.view.LayoutInflater.createViewFromTag(LayoutInflater.java:965)\n\tat android.view.LayoutInflater.rInflate(LayoutInflater.java:1127)\n\tat android.view.LayoutInflater.rInflateChildren(LayoutInflater.java:1088)\n\tat android.view.LayoutInflater.rInflate(LayoutInflater.java:1130)\n\tat android.view.LayoutInflater.rInflateChildren(LayoutInflater.java:1088)\n\tat android.view.LayoutInflater.inflate(LayoutInflater.java:686)\n\tat android.view.LayoutInflater.inflate(LayoutInflater.java:538)\n\tat android.view.LayoutInflater.inflate(LayoutInflater.java:485)\n\tat androidx.media3.ui.PlayerControlView.(PlayerControlView.java:419)\n\tat androidx.media3.ui.PlayerControlView.(PlayerControlView.java:355)\n\tat androidx.media3.ui.PlayerControlView.(PlayerControlView.java:351)\n\tat androidx.media3.ui.PlayerControlView.(PlayerControlView.java:347)\n\tat com.brentvatne.exoplayer.ReactExoplayerView.initializePlayerControl(ReactExoplayerView.java:394)\n\tat com.brentvatne.exoplayer.ReactExoplayerView.finishPlayerInitialization(ReactExoplayerView.java:717)\n\tat com.brentvatne.exoplayer.ReactExoplayerView.initializePlayerSource(ReactExoplayerView.java:712)\n\tat com.brentvatne.exoplayer.ReactExoplayerView.lambda$initializePlayer$4$com-brentvatne-exoplayer-ReactExoplayerView(ReactExoplayerView.java:573)\n\tat com.brentvatne.exoplayer.ReactExoplayerView$$ExternalSyntheticLambda12.run(Unknown Source:6)\n\tat android.os.Handler.handleCallback(Handler.java:942)\n\tat android.os.Handler.dispatchMessage(Handler.java:99)\n\tat android.os.Looper.loopOnce(Looper.java:201)\n\tat android.os.Looper.loop(Looper.java:288)\n\tat android.app.ActivityThread.main(ActivityThread.java:7898)\n\tat java.lang.reflect.Method.invoke(Native Method)\n\tat com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)\n\tat com.android.internal.os.ZygoteInit.main(ZygoteInit.java:936)\nCaused by: java.lang.ClassNotFoundException: Didn't find class "com.google.android.exoplayer2.ui.DefaultTimeBar" on path: DexPathList[[zip file "/data/app/~~vUs5ytyVyQva3WXiYoOUEg==/com.videoplayer-rwz9E_zQ5RLHJaxAM-DRQg==/base.apk"],nativeLibraryDirectories=[/data/app/~~vUs5ytyVyQva3WXiYoOUEg==/com.videoplayer-rwz9E_zQ5RLHJaxAM-DRQg==/lib/arm64, /data/app/~~vUs5ytyVyQva3WXiYoOUEg==/com.videoplayer-rwz9E_zQ5RLHJaxAM-DRQg==/base.apk!/lib/arm64-v8a, /system/lib64, /system_ext/lib64]]\n\tat dalvik.system.BaseDexClassLoader.findClass(BaseDexClassLoader.java:259)\n\tat java.lang.ClassLoader.loadClass(ClassLoader.java:379)\n\tat java.lang.ClassLoader.loadClass(ClassLoader.java:312)\n\t... 29 more\n","errorCode":"1001","errorException":"android.view.InflateException: Binary XML file line #63 in com.videoplayer:layout/exo_player_control_view: Binary XML file line #63 in com.videoplayer:layout/exo_player_control_view: Error inflating class com.google.android.exoplayer2.ui.DefaultTimeBar","errorString":"android.view.InflateException: Binary XML file line #63 in com.videoplayer:layout/exo_player_control_view: Binary XML file line #63 in com.videoplayer:layout/exo_player_control_view: Error inflating class com.google.android.exoplayer2.ui.DefaultTimeBar"}}

freeboub · 2023-11-09T22:02:02Z

@YangJonghun in file: ./android/src/main/res/layout/exo_player_control_view.xml
There are still references to exoplayer2 ! I that is the root cause.
It can be tested easily with basic sample in the repository

YangJonghun · 2023-11-10T15:43:43Z

@KrzysztofMoch @freeboub
Thanks for letting me know. I was a little busy to check the comments.
Anyway, the PlayerControlView for ExoPlayer2 was changed to a LegacyPlayerControlView and I applied it.
(also, default layout id was changed too)

FYI) https://developer.android.com/reference/androidx/media3/ui/LegacyPlayerControlView

KrzysztofMoch

Looks like all is working 🚀 Thanks for your PR!

YangJonghun · 2023-11-16T11:37:39Z

@KrzysztofMoch @freeboub
Can I write code migrated to Kotlin based on this branch?
I think the combination of Kotlin and Coroutine(or RxKotlin) is a way to fix a potential bug in the initializePlayer function (specifically the asynchronous call).

And if possible, it would be nice if you could comment on why this PR was held.

KrzysztofMoch · 2023-11-16T12:18:51Z

it would be nice if you could comment on why this PR was held.

PR is hold because we would like to bump minor version number for migration from ExoPlayer2 to AndroidX media 3. The problem is that we are in alpha and we don't really have a way to do it.

Can I write code migrated to Kotlin based on this branch?

I don't mind if you do migration based on this branch but in this case kotlin migration will be also held to moment where we will merge this PR. Lets see what @freeboub think about it

Thanks for you PR and Sorry for the trouble 🙏

freeboub · 2023-11-16T17:56:48Z

@YangJonghun First Thank you for the proposal !

Regarding to the hold state, It should not be too long. We clearly should move to media3 soon. It would be great to go out of alpha state before merging this PR.

Keep in mind that there is another PR which introduce more kotlin : #3204
I also fixed the real time issue on my side also, but it is very hard to backport ! (I also allow react native video to support multiple player).
I fully agree this code shall move to kotlin, but we should go step by step to mitigate regression risks !

Regarding to RxKotlin, I am not really fan of it (I guess this is the same philosophy than RxJs). but I have no experience with coroutines.

You can create a discussion if you want advices or way to progress, but please don't do a PR which moves all the code in kotlin, it will be too hard to review !

YangJonghun · 2023-11-16T18:40:35Z

@freeboub
First of all, thank you very much for the detailed explanation.

However, I don't think there is much risk in just migrating to Kotlin, and if you think there is, I could create a PR for each file or exclude large files like ReactExoPlayer and migrate them first.

But I respect your opinion, so I won't create a PR if you still don't want to.

I'm not sure if this is the right place for this, but I'm a fan of this library and have been using it for almost 4 years, but I think the update rate is slow, which implies that the alpha version is already unstable, and I think the alpha version needs to change faster and be used and exposed to more users to avoid more risks.

I hope you don't take this as a blame, as I trust and appreciate your contributions and hard work very much.

freeboub · 2023-11-16T21:13:13Z

@YangJonghun I agree it should not be risky to switch to kotlin, but who really knows :)
Just before I come to this project someone refactor objectiveC implementation to swift implementation, and just leave without maintenance... It was painful, and this is partially the reason why this alpha step is so long.
The second reason of this so long alpha phase is the lack of contributor. I stayed 1 year alone as maintainer. It is not safe to review my own PR :) since few month @KrzysztofMoch join to help, this really better not to be alone !

Moreover I think this repo is very important for react native ecosystem

That said, I don't take your comment as a blame, but you highlight the necessity of more contributors, and I fully agree with you !

So Yes you can update code to kotlin, I will review PRs and merge them with pleasure !
But please refactor component by component or small part by small part. I started to split and clean some files in my last PRs.
Let's continue kotlin discussions here: #3356

freeboub · 2023-11-16T22:34:21Z

media3 1.2.0 has been released yesterday 😂

freeboub · 2023-11-16T22:39:16Z

media3 1.2.0 has been released yesterday 😂

It is a joke, 1.1.1 looks good for now !

…ia3-migration

freeboub · 2023-11-18T13:13:28Z

@YangJonghun We discussed internally, and we agree to merge this PR.
So let's do it !

asharamseervi · 2023-11-21T19:26:18Z

media3 1.2.0 has been released

Can't we support the recent release v1.2.0, or any point release before v6 or future beta versions? I hope, the most challenging part is already achieved, isn't it?
CC: @YangJonghun Thank you for this great work <3

freeboub · 2023-11-21T19:42:47Z

@asharamseervi yes we can use it, I don't think this is a major issue, but it will force us to use the last android sdk, where react native core team doesn't support it officially.
I will revert the change tonight, @YangJonghun correct me if I am wrong and thanks for the notice!

YangJonghun added 4 commits November 8, 2023 22:57

feat(android): implement AndroidX media3 dependencies

715b10f

refactor(android): remove duplicate code

b724690

refactor(android): remove unused codes

c63716f

feat(android): replace ExoPlayer2 with AndroidX media3

81276a7

YangJonghun changed the title ~~Replace deprecated ExoPlayer2 with AndroidX media3~~ feat(android): replace deprecated ExoPlayer2 with AndroidX media3 Nov 8, 2023

freeboub requested changes Nov 8, 2023

View reviewed changes

android/src/main/java/com/brentvatne/receiver/AudioBecomingNoisyReceiver.java Outdated Show resolved Hide resolved

android/build.gradle Outdated Show resolved Hide resolved

KrzysztofMoch self-requested a review November 8, 2023 17:53

KrzysztofMoch requested changes Nov 8, 2023

View reviewed changes

android/build.gradle Outdated Show resolved Hide resolved

YangJonghun added 3 commits November 9, 2023 03:50

Merge branch 'master'

f49cf30

Conflicts: android/build.gradle android/src/main/java/com/brentvatne/common/react/VideoEventEmitter.java

fix(android): move default properties to gradle.properties

53ade18

revert(android): prevent security exception

a1a7d30

YangJonghun requested review from freeboub and KrzysztofMoch November 8, 2023 20:04

freeboub reviewed Nov 8, 2023

View reviewed changes

YangJonghun added 3 commits November 9, 2023 13:15

chore: align indent

413033c

chore: remove redundant comments

a29d85d

chore: reorder import

2ab85b4

fix: apply media3's legacy player control view

ab0a751

KrzysztofMoch approved these changes Nov 13, 2023

View reviewed changes

freeboub approved these changes Nov 13, 2023

View reviewed changes

KrzysztofMoch added the hold PR is on hold for some important reason label Nov 16, 2023

Merge commit '1ba93f9e9d33f653f0e01214f220e1e5eda819f5' into feat/med…

321a482

…ia3-migration

freeboub merged commit f2e80e9 into TheWidlarzGroup:master Nov 18, 2023
1 check passed

YangJonghun deleted the feat/media3-migration branch November 19, 2023 10:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(android): replace deprecated ExoPlayer2 with AndroidX media3 #3337

feat(android): replace deprecated ExoPlayer2 with AndroidX media3 #3337

YangJonghun commented Nov 8, 2023 •

edited

freeboub commented Nov 8, 2023

KrzysztofMoch left a comment

freeboub Nov 8, 2023

freeboub Nov 8, 2023

YangJonghun Nov 9, 2023

freeboub Nov 9, 2023

freeboub Nov 8, 2023

YangJonghun Nov 9, 2023

freeboub commented Nov 8, 2023

KrzysztofMoch commented Nov 9, 2023

freeboub commented Nov 9, 2023

freeboub commented Nov 9, 2023

YangJonghun commented Nov 10, 2023 •

edited

KrzysztofMoch left a comment •

edited

YangJonghun commented Nov 16, 2023

KrzysztofMoch commented Nov 16, 2023

freeboub commented Nov 16, 2023

YangJonghun commented Nov 16, 2023 •

edited

freeboub commented Nov 16, 2023

freeboub commented Nov 16, 2023

freeboub commented Nov 16, 2023

freeboub commented Nov 18, 2023

asharamseervi commented Nov 21, 2023

freeboub commented Nov 21, 2023

	// SimpleExoPlayer.VideoListener implementation
	// ExoPlayer.VideoListener implementation

feat(android): replace deprecated ExoPlayer2 with AndroidX media3 #3337

feat(android): replace deprecated ExoPlayer2 with AndroidX media3 #3337

Conversation

YangJonghun commented Nov 8, 2023 • edited

Update the changelog

Describe the changes

freeboub commented Nov 8, 2023

KrzysztofMoch left a comment

Choose a reason for hiding this comment

freeboub Nov 8, 2023

Choose a reason for hiding this comment

freeboub Nov 8, 2023

Choose a reason for hiding this comment

YangJonghun Nov 9, 2023

Choose a reason for hiding this comment

freeboub Nov 9, 2023

Choose a reason for hiding this comment

freeboub Nov 8, 2023

Choose a reason for hiding this comment

YangJonghun Nov 9, 2023

Choose a reason for hiding this comment

freeboub commented Nov 8, 2023

KrzysztofMoch commented Nov 9, 2023

freeboub commented Nov 9, 2023

freeboub commented Nov 9, 2023

YangJonghun commented Nov 10, 2023 • edited

KrzysztofMoch left a comment • edited

Choose a reason for hiding this comment

YangJonghun commented Nov 16, 2023

KrzysztofMoch commented Nov 16, 2023

freeboub commented Nov 16, 2023

YangJonghun commented Nov 16, 2023 • edited

freeboub commented Nov 16, 2023

freeboub commented Nov 16, 2023

freeboub commented Nov 16, 2023

freeboub commented Nov 18, 2023

asharamseervi commented Nov 21, 2023

freeboub commented Nov 21, 2023

YangJonghun commented Nov 8, 2023 •

edited

YangJonghun commented Nov 10, 2023 •

edited

KrzysztofMoch left a comment •

edited

YangJonghun commented Nov 16, 2023 •

edited