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

feat: add full screen support based on expo-av implementation #2763

Merged
merged 10 commits into from
Sep 11, 2022

Conversation

wood1986
Copy link
Contributor

I saw #2688. Based on the description, it seems the implementation affected the stability of v6. I have an implementation which touches the minimal code. My implementation is based on expo-av implementation and use Dialog to do full screen.

Kapture 2022-07-13 at 11 38 58

@wood1986
Copy link
Contributor Author

@reinismu
Copy link

reinismu commented Jul 21, 2022

Nice nice :)

Tried it out in my application. It goes fullscreen fine, but when I go back video doesn't work anymore (Audio still is fine). After that if I go back to fullscreen video still works there.

FullScreen

Could be emulator issue. Or something else on my end. Haven't investigated too much yet.

I wonder if it could bypass portrait lock in android and go forced landscape

React Native: 0.66.3
React Native Video: 5.2.0

@wood1986
Copy link
Contributor Author

Can you try using the built in control?

@reinismu
Copy link

reinismu commented Jul 21, 2022

Tested on a real device now. Same thing. Using built-in controls doesn't work.
I have a minimized feature for video. When I use it fixes the issue. Seems like any style change brings video back.
Or if I change the style in my component and react native hot reload kicks in.

Video style

    video: (floatingPlayer) => {
        const baseStyles = {
            width: '100%',
            aspectRatio: 16 / 9,
        };
        if (floatingPlayer) {
            return {
                ...baseStyles,
                borderTopLeftRadius: 3,
                borderTopRightRadius: 3,
                borderWidth: 3,
                borderBottomWidth: 0,
                borderColor: colors.videoTimerBackground,
            };
        }
        return {
            ...baseStyles,
        };
    },

I added

const baseStyles = {
            width: '100%',
            height: '100%',
            aspectRatio: 16 / 9,
        };

And now it works. Tho video now takes all the screen. Seems like when it goes back to the screen it can't guess the right height.

@reinismu
Copy link

reinismu commented Jul 21, 2022

Maybe I figured it out. When it puts view back in the parent it breaks my overlay controls. It puts video not under the controls, but below. I will check if I see a way to fix it

Seems like another issue is if I use the back button while in fullscreen I can't open fullscreen again.

@lewatt23
Copy link

lewatt23 commented Aug 8, 2022

Hello everyone,
Justed the PR but am unable to get the fullscreen working, also the fullscreen button doesn't even show on my side. Is there any special configuration to be done for the button to display? or am i testing the PR wrongly ? ^^!.

<Video
      source={{
        uri: url,
      }}
      ref={(ref: any) => (videoPlayer = ref)}
      onLoad={onLoad}
      controls
      onBuffer={() => {}} // Callback when remote video is buffering
      onError={() => {}}
      style={{
        width: '100%',
        aspectRatio: 16 / 9,
      }}

    />

React Native: 0.68.2
Android version : 12,
Device : Redmi Note 10

FILE.2022-08-09.00.44.19.mp4

@wood1986
Copy link
Contributor Author

wood1986 commented Aug 9, 2022

@lewatt23 did you include exo_player_control_view.xml? If yes, try clean build

@lewatt23
Copy link

lewatt23 commented Aug 9, 2022

@wood1986 no i have not, please can you expain to me how i can include it? thanks.

@wood1986
Copy link
Contributor Author

wood1986 commented Aug 9, 2022

How did you pick my change and then apply to your branch?

@lewatt23
Copy link

lewatt23 commented Aug 9, 2022

I did clone your branch and added it to my project using yarn, not sure if its the correct way ^^!.

@wood1986
Copy link
Contributor Author

@lewatt23
Copy link

@wood1986 thanks i was able to test it, it works perfectly :)

@rafaelbrun
Copy link

Great work, tested in my project and it's working just fine.
Just a question, you got a WIP commit on the PR, is that still a wip?

@wood1986
Copy link
Contributor Author

wood1986 commented Aug 12, 2022

You can ignore my comment. it is done

@reinismu
Copy link

  • If I use the back button while in fullscreen I can't open fullscreen again
  • It seems that putting the video back in its view loses previous style

@wood1986
Copy link
Contributor Author

Please take a small video

@wood1986
Copy link
Contributor Author

wood1986 commented Aug 18, 2022

I think the back button issue is fixed. Please try the latest commit

@reinismu
Copy link

Noticed reLayout(playerControlView); and tried PR again, but video still doesn't appear after exiting fullscreen :/
Will try to find time and open PR to reproduce this bug here https://github.com/wood1986/react-native-fullscreen-example.git

@wood1986
Copy link
Contributor Author

I have not update my branch to use latest commit. But I cannot reproduce

@wood1986
Copy link
Contributor Author

Kapture 2022-08-23 at 14 47 53

@freeboub
Copy link
Collaborator

freeboub commented Sep 7, 2022

Just tested on sample , It doesn't build (just a matter of depenency I think):
/home/olivier/work/repo/workspace/lib/react-native-video_official/react-native-video/examples/basic/node_modules/react-native-video/android/src/main/java/com/brentvatne/exoplayer/FullScreenPlayerView.java:9: error: package androidx.activity does not exist
import androidx.activity.OnBackPressedCallback;
^
/home/olivier/work/repo/workspace/lib/react-native-video_official/react-native-video/examples/basic/node_modules/react-native-video/android/src/main/java/com/brentvatne/exoplayer/FullScreenPlayerView.java:18: error: cannot find symbol
private final OnBackPressedCallback onBackPressedCallback;
^
symbol: class OnBackPressedCallback
location: class FullScreenPlayerView
/home/olivier/work/repo/workspace/lib/react-native-video_official/react-native-video/examples/basic/node_modules/react-native-video/android/src/main/java/com/brentvatne/exoplayer/FullScreenPlayerView.java:20: error: cannot find symbol
public FullScreenPlayerView(Context context, ExoPlayerView exoPlayerView, PlayerControlView playerControlView, OnBackPressedCallback onBackPressedCallback) {
^
symbol: class OnBackPressedCallback
location: class FullScreenPlayerView
/home/olivier/work/repo/workspace/lib/react-native-video_official/react-native-video/examples/basic/node_modules/react-native-video/android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerView.java:20: error: package androidx.activity does not exist
import androidx.activity.OnBackPressedCallback;
^
/home/olivier/work/repo/workspace/lib/react-native-video_official/react-native-video/examples/basic/node_modules/react-native-video/android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerView.java:674: error: cannot find symbol
fullScreenPlayerView = new FullScreenPlayerView(getContext(), exoPlayerView, playerControlView, new OnBackPressedCallback(true) {
^
symbol: class OnBackPressedCallback
location: class ReactExoplayerView
/home/olivier/work/repo/workspace/lib/react-native-video_official/react-native-video/examples/basic/node_modules/react-native-video/android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerView.java:675: error: method does not override or implement a method from a supertype
@OverRide
^

can you either add the dependancy in sample & in doc
Or add the dependency in main package ?

@freeboub
Copy link
Collaborator

freeboub commented Sep 7, 2022

Please also add a line in README.md

@wood1986
Copy link
Contributor Author

wood1986 commented Sep 7, 2022

What information do I need to add to README.md?

@wood1986
Copy link
Contributor Author

wood1986 commented Sep 7, 2022

@freeboub I updated the dependency. I should run now. You can also check https://github.com/wood1986/react-native-fullscreen-example.git to test it

@freeboub
Copy link
Collaborator

Tested on Android TV emulator with the sample.
I think you need to add this ! :)

diff --git a/android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerView.java b/android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerView.java
index 7e087bb8..dec0dbef 100644
--- a/android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerView.java
+++ b/android/src/main/java/com/brentvatne/exoplayer/ReactExoplayerView.java
@@ -1811,6 +1811,7 @@ class ReactExoplayerView extends FrameLayout implements
                 if (controls) {
                     fullScreenPlayerView.dismiss();
+                     reLayout(exoPlayerView);
                 }
                 eventEmitter.fullscreenDidDismiss();
             });
         }

I add control support in the sample in another PR

@wood1986
Copy link
Contributor Author

added

@freeboub freeboub merged commit c8b54f3 into TheWidlarzGroup:master Sep 11, 2022
@evoactivity
Copy link
Contributor

@freeboub this needs an entry in the changelog and updating the documentation about android fullscreen

@freeboub
Copy link
Collaborator

Thank you @evoactivity, I pushed a PR to update the basic sample to make controls testable here : #2852
Regarding to the documentation, I think controls are not documentated at all.
Let's open a ticket to follow this.
Thank you

@wood1986
Copy link
Contributor Author

Finally!! Thanks for the merge

@wood1986
Copy link
Contributor Author

BTW, when will you publish the alpha or beta or final to npm?

@freeboub
Copy link
Collaborator

freeboub commented Sep 11, 2022

@wood1986 I should sync up on Monday for this. There are some ios blocking issues with build which makes me threaten ...

@KestasVenslauskas
Copy link

Why full screen works only while using default controls but settings 'fullscreen' prop just hides status bar?

@wood1986
Copy link
Contributor Author

Because people may have their own full screen implementation which is before my full screen work. One of the example is the player is not inside the scroll view.

@KestasVenslauskas
Copy link

@wood1986 So there is no way of having fullscreen player popup with custom controls?

@wood1986
Copy link
Contributor Author

are you asking the controls inside fullscreen or the control for opening fullscreen?

@KestasVenslauskas
Copy link

Controls opening fullscreen. Because calling player.presentFullScreen() is not working

@oferRounds
Copy link

Question: we’re using react-native-version 0.67.4, and thus can not yet upgrade to the latest react-native-video version who includes the feature
My question: how easy would it be to create fork/patch with this change applied to the 5.2.0 version?

@ahmettopal
Copy link

ahmettopal commented Dec 2, 2022

ı compare my library with yours everythinks are same but full screen button still not showing. what ım missing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants