chore(android): update UI controller according to media3 #3914

seyedmostafahasani · 2024-06-14T23:37:34Z

Summary

Upgrade player UI to align with media3 demo app

Motivation

Using the native controller to close these issues. (#4019, #3301,#3232,#2599)

Changes

Test plan

freeboub

It looks like you didn't merge conflicts correctly (around drm props and allowchunklesspreparation. Can you please check that ?
I would like to test it afterward

bulkinav · 2024-06-15T14:42:31Z

Hi @seyedmostafahasani. Can you also add a screenshot of the new UI here? I would like to see what it looks like now )

seyedmostafahasani · 2024-06-16T21:48:30Z

Hi @seyedmostafahasani. Can you also add a screenshot of the new UI here? I would like to see what it looks like now )

@bulkinav thanks for your suggestion. I think it is better to implement the basics of the UI first, then we can implement the screenshot feature in the future in a new PR.
@freeboub what do you think?

seyedmostafahasani · 2024-06-16T21:51:37Z

@freeboub I think we can remove the fullscreen prop in V7. The user can handle fullscreen mode with the setFullScreen method, so we don’t need an additional prop like fullscreen. What is your opinion?

android/src/main/java/com/brentvatne/common/api/Source.kt

docs/pages/component/props.mdx

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

seyedmostafahasani · 2024-06-18T10:24:33Z

@freeboub, if everything works correctly, please don’t merge this branch to master yet. First, let me remove the additional code, then we can merge this branch to master.

seyedmostafahasani · 2024-06-20T15:16:59Z

@freeboub, did you have enough time to test this PR?

ashnfb · 2024-06-20T20:51:44Z

I've tried this version of UI with a video that has captions and multiple audio options
https://bitmovin-a.akamaihd.net/content/sintel/hls/playlist.m3u8

Does the new UI have the ability to show a textTracks selection menu? I could not see a way to turn on / off captions

The audio selection menu also appears buggy, as it repeats the options twice

seyedmostafahasani · 2024-06-21T07:43:52Z

I've tried this version of UI with a video that has captions and multiple audio options https://bitmovin-a.akamaihd.net/content/sintel/hls/playlist.m3u8

Does the new UI have the ability to show a textTracks selection menu? I could not see a way to turn on / off captions

The audio selection menu also appears buggy, as it repeats the options twice

@ashnfb Yeah, it's possible to show the caption button in the new UI. I added it. Please check it out.

seyedmostafahasani · 2024-06-21T08:03:51Z

@ashnfb regarding the audio selection menu, I think this URL has an issue: https://bitmovin-a.akamaihd.net/content/sintel/hls/playlist.m3u8. I've tested it with other URLs, as shown below, and the audio menu works correctly.

freeboub · 2024-06-21T09:53:09Z

Just tested, I found 2 issues fo now:

in the sample app, the controls are displayed by default even if controls={false} by default
in the sample app, when I go to "no view" item and then channel up to 'another live sample', the video shows a big resizing glitch

# Conflicts: # android/src/main/java/com/brentvatne/exoplayer/AudioOutput.java # android/src/main/java/com/brentvatne/exoplayer/ExoPlayerView.java # android/src/main/java/com/brentvatne/exoplayer/FullScreenPlayerView.java # android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerView.java # android/src/main/java/com/brentvatne/react/VideoManagerModule.kt # src/specs/VideoNativeComponent.ts

seyedmostafahasani · 2024-07-21T22:55:07Z

@freeboub @KrzysztofMoch @YangJonghun I have updated the code. Please check it out. I think if we merge it, we can close some issues like (#4019, #3301,#3232,#2599)
I will be available to apply your comments on this PR.

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

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

YangJonghun · 2024-07-22T07:58:37Z

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

@@ -32,18 +29,15 @@

 import java.util.List;

-@SuppressLint("ViewConstructor")
-public final class ExoPlayerView extends FrameLayout implements AdViewProvider {
+public final class ExoPlayerView extends PlayerView implements AdViewProvider {


PlayerView already implements AdViewProvider internally
fyi) https://developer.android.com/reference/androidx/media3/ui/PlayerView

Thanks for sharing your knowledge.

YangJonghun · 2024-07-22T08:08:12Z

android/src/main/java/com/brentvatne/exoplayer/FullScreenPlayerView.kt

 override fun onBackPressed() {
- super.onBackPressed()
+ findViewById<ImageView>(androidx.media3.ui.R.id.exo_fullscreen)?.performClick()


Do we really need this code?

Yeah, we need it because when a user presses the back button, the full-screen button icon doesn't change. We have to add it to update the full-screen button icon.

YangJonghun · 2024-07-22T08:14:51Z

android/src/main/java/com/brentvatne/exoplayer/FullScreenPlayerView.kt

 }

 override fun onStart() {
 super.onStart()
- parent = exoPlayerView.parent as ViewGroup?
+ parent = exoPlayerView.parent as FrameLayout


It would be nice to keep the ViewGroup? if you possible🙏

KrzysztofMoch · 2024-07-22T09:57:01Z

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

We have empty file here

android/src/main/java/com/brentvatne/exoplayer/FullScreenPlayerView.kt

 }
 } catch (ex: Exception) {
- DebugLog.e("ExoPlayer Exception", "Failed to flag FLAG_KEEP_SCREEN_ON on fullscreen.")
+ DebugLog.e("ExoPlayer Exception", "Failed to flag FLAG_KEEP_SCREEN_ON on fullscreeen.")


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

@@ -281,7 +255,7 @@ public void onVideoSizeChanged(VideoSize videoSize) {

 @Override
 public void onRenderedFirstFrame() {
- shutterView.setVisibility(INVISIBLE);
+ setHideShutterView(hideShutterView);


YangJonghun · 2024-07-22T17:37:40Z

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

 }

 public void invalidateAspectRatio() {
 // Resetting aspect ratio will force layout refresh on next video size changed
- layout.invalidateAspectRatio();
+ //layout.invalidateAspectRatio();


Is it intended?

YangJonghun · 2024-07-22T17:40:03Z

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

 layoutParams = new ViewGroup.LayoutParams(
 ViewGroup.LayoutParams.MATCH_PARENT,
 ViewGroup.LayoutParams.MATCH_PARENT);

 componentListener = new ComponentListener();
-
 FrameLayout.LayoutParams aspectRatioParams = new FrameLayout.LayoutParams(
 FrameLayout.LayoutParams.MATCH_PARENT,
 FrameLayout.LayoutParams.MATCH_PARENT);
 aspectRatioParams.gravity = Gravity.CENTER;
 layout = new AspectRatioFrameLayout(context);
 layout.setLayoutParams(aspectRatioParams);


It appears that layout is created but not included in the PlayerView. Am I misread something?

YangJonghun · 2024-07-22T17:44:50Z

android/src/main/java/com/brentvatne/exoplayer/FullScreenPlayerView.kt

 if (window != null) {
- val isPlaying = fullscreenVideoPlayer.exoPlayerView.isPlaying
+ val isPlaying = it.exoPlayerView.isPlaying


Is it retrun boolean? If it doesn't, PR#3969 has same problem also.

Yeah, it returns a boolean.

# Conflicts: # android/src/main/java/com/brentvatne/exoplayer/FullScreenPlayerView.kt

seyedmostafahasani · 2024-07-27T13:58:03Z

@YangJonghun thank you for reviewing my PR. I've applied your suggestions.

YangJonghun · 2024-07-28T07:25:45Z

@seyedmostafahasani
Thanks
It seems resizeMode prop doesn't work in this PR, please check it🙏

seyedmostafahasani · 2024-07-28T16:05:07Z

@YangJonghun thanks for testing. I've fixed it. You can test it now.

YangJonghun · 2024-07-29T10:01:14Z

@seyedmostafahasani
Thanks:)
But navigation bar does not work properly when restoring from fullscreen mode. Sorry for not commenting at once🥲
(If I find more issues after testing, I'll leave them in this comment.)

seyedmostafahasani · 2024-07-29T10:30:45Z

@seyedmostafahasani Thanks:) But navigation bar does not work properly when restoring from fullscreen mode. Sorry for not commenting at once🥲 (If I find more issues after testing, I'll leave them in this comment.)

No problem. Thanks again for testing. Let me check it out. Are you sure this issue is related to this PR?

YangJonghun · 2024-07-29T13:29:21Z

No problem. Thanks again for testing. Let me check it out. Are you sure this issue is related to this PR?

Oops, it wasn't problem with this PR 😓 Also occurs on master branch

seyedmostafahasani · 2024-07-29T15:02:45Z

No problem:). I will try to fix it in another PR.

seyedmostafahasani added 2 commits June 15, 2024 03:05

chore(android): update UI controller according to media3

4f568d3

fix: import path

8c84728

seyedmostafahasani marked this pull request as draft June 15, 2024 06:20

freeboub requested changes Jun 15, 2024

View reviewed changes

seyedmostafahasani added 2 commits June 17, 2024 01:09

chore: update fullScreen prop functionality

72f5732

chore(doc): update document about fullScreen prop

bba24b1

seyedmostafahasani added 6 commits June 17, 2024 01:31

fix: allow chunckless preparation conflict

7c215c9

fix: drm prop conflict

5ab02f2

fix: build sample app

9ab93e3

fix: declare useCache variable

30d29e8

fix: registerPlayer type

edbd682

fix: import object path

be13ab2

seyedmostafahasani marked this pull request as ready for review June 16, 2024 23:00

seyedmostafahasani requested a review from freeboub June 16, 2024 23:00

freeboub requested changes Jun 18, 2024

View reviewed changes

android/src/main/java/com/brentvatne/common/api/Source.kt Outdated Show resolved Hide resolved

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

android/src/main/java/com/brentvatne/exoplayer/FullScreenPlayerView.java Outdated Show resolved Hide resolved

seyedmostafahasani added 2 commits June 18, 2024 13:28

fix: revert function and prop

dd33573

fix: import path

9fbca52

seyedmostafahasani requested a review from freeboub June 18, 2024 10:07

chore: show subtitle button

e9493a1

chore: remove additional code

9e20777

fix: adjust controller status based on the controls prop

b44b3ba

seyedmostafahasani added 4 commits July 22, 2024 00:51

fix: lint error

9a40af3

chore: update FullScreenPlayerView according to media3 controller

739d6c9

fix: lint error

15624e3

YangJonghun reviewed Jul 22, 2024

View reviewed changes

android/src/main/java/com/brentvatne/exoplayer/AudioOutput.java Outdated Show resolved Hide resolved

YangJonghun reviewed Jul 22, 2024

View reviewed changes

android/src/main/java/com/brentvatne/exoplayer/ExoPlayerView.java Outdated Show resolved Hide resolved

fix: PR feedback

6c808fc

seyedmostafahasani requested a review from YangJonghun July 22, 2024 07:10

YangJonghun reviewed Jul 22, 2024

View reviewed changes

fix: PR issues

73257a2

seyedmostafahasani requested a review from YangJonghun July 22, 2024 09:14

KrzysztofMoch reviewed Jul 22, 2024

View reviewed changes

android/src/main/java/com/brentvatne/exoplayer/FullScreenPlayerView.java Outdated

Copy link

Collaborator

KrzysztofMoch Jul 22, 2024

Choose a reason for hiding this comment

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

We have empty file here

chore: remove empty java file

87d487e

seyedmostafahasani requested a review from KrzysztofMoch July 22, 2024 10:25

YangJonghun reviewed Jul 25, 2024

View reviewed changes

seyedmostafahasani added 3 commits July 27, 2024 17:17

fix: PR feedback

68d617f

Merge branch 'master' into chore/UI-controller

d83b866

# Conflicts: # android/src/main/java/com/brentvatne/exoplayer/FullScreenPlayerView.kt

chore: refactor condition with when

90ae980

seyedmostafahasani requested a review from YangJonghun July 27, 2024 14:02

fix: resizeMode

725a5d1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(android): update UI controller according to media3 #3914

chore(android): update UI controller according to media3 #3914

seyedmostafahasani commented Jun 14, 2024 •

edited

Loading

freeboub left a comment

bulkinav commented Jun 15, 2024

seyedmostafahasani commented Jun 16, 2024

seyedmostafahasani commented Jun 16, 2024

seyedmostafahasani commented Jun 18, 2024

seyedmostafahasani commented Jun 20, 2024

ashnfb commented Jun 20, 2024

seyedmostafahasani commented Jun 21, 2024 •

edited

Loading

seyedmostafahasani commented Jun 21, 2024 •

edited

Loading

freeboub commented Jun 21, 2024

seyedmostafahasani commented Jul 21, 2024 •

edited

Loading

YangJonghun Jul 22, 2024 •

edited

Loading

seyedmostafahasani Jul 22, 2024

YangJonghun Jul 22, 2024

seyedmostafahasani Jul 22, 2024

YangJonghun Jul 22, 2024 •

edited

Loading

KrzysztofMoch Jul 22, 2024

This comment was marked as resolved.

This comment was marked as resolved.

YangJonghun Jul 22, 2024

YangJonghun Jul 22, 2024

YangJonghun Jul 22, 2024

seyedmostafahasani Jul 27, 2024

seyedmostafahasani commented Jul 27, 2024

YangJonghun commented Jul 28, 2024

seyedmostafahasani commented Jul 28, 2024

YangJonghun commented Jul 29, 2024

seyedmostafahasani commented Jul 29, 2024

YangJonghun commented Jul 29, 2024 •

edited

Loading

seyedmostafahasani commented Jul 29, 2024

chore(android): update UI controller according to media3 #3914

Are you sure you want to change the base?

chore(android): update UI controller according to media3 #3914

Conversation

seyedmostafahasani commented Jun 14, 2024 • edited Loading

Summary

Motivation

Changes

Test plan

freeboub left a comment

Choose a reason for hiding this comment

bulkinav commented Jun 15, 2024

seyedmostafahasani commented Jun 16, 2024

seyedmostafahasani commented Jun 16, 2024

seyedmostafahasani commented Jun 18, 2024

seyedmostafahasani commented Jun 20, 2024

ashnfb commented Jun 20, 2024

seyedmostafahasani commented Jun 21, 2024 • edited Loading

seyedmostafahasani commented Jun 21, 2024 • edited Loading

freeboub commented Jun 21, 2024

seyedmostafahasani commented Jul 21, 2024 • edited Loading

YangJonghun Jul 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YangJonghun Jul 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seyedmostafahasani commented Jul 27, 2024

YangJonghun commented Jul 28, 2024

seyedmostafahasani commented Jul 28, 2024

YangJonghun commented Jul 29, 2024

seyedmostafahasani commented Jul 29, 2024

YangJonghun commented Jul 29, 2024 • edited Loading

seyedmostafahasani commented Jul 29, 2024

seyedmostafahasani commented Jun 14, 2024 •

edited

Loading

seyedmostafahasani commented Jun 21, 2024 •

edited

Loading

seyedmostafahasani commented Jun 21, 2024 •

edited

Loading

seyedmostafahasani commented Jul 21, 2024 •

edited

Loading

YangJonghun Jul 22, 2024 •

edited

Loading

YangJonghun Jul 22, 2024 •

edited

Loading

YangJonghun commented Jul 29, 2024 •

edited

Loading