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

iOS Fullscreen with Controls Crash #1568

Closed
jonas-app opened this issue Apr 24, 2019 · 25 comments
Closed

iOS Fullscreen with Controls Crash #1568

jonas-app opened this issue Apr 24, 2019 · 25 comments
Labels
stale Closed due to inactivity or lack or resources

Comments

@jonas-app
Copy link

@ashnfb @cobarx

Current behavior

The following change crashes our current implementation on iOS:
https://github.com/react-native-community/react-native-video/pull/1441/files#diff-42e8804735be64e84d6dd1fb103210a1R566
We display the video in full screen as soon as it is loaded.
After updating this implementation crashes.
A fix is not urgent for us, as we do not need the controls in non fullscreen mode on iOS.
As a workaround we just disable the controls on iOS, which resolves the issue.

Reproduction steps

<Video source={this.props.source} resizeMode='contain' paused={false} repeat={false} controls={true} onLoad={() => this.player && this.player.presentFullscreenPlayer()} ref={this.handleRef} style={fullscreen} />

Additional info: We render the video in a react navigation dialog.

Expected behavior

It does not crash.

Platform

  • iOS
Exception thrown while executing UI block: Application tried to present modally an active controller <XXX.ReactNativeViewController: 0x7ff09b6182c0>.

__44-[RCTUIManager flushUIBlocksWithCompletion:]_block_invoke
    RCTUIManager.m:1112
__44-[RCTUIManager flushUIBlocksWithCompletion:]_block_invoke.461
__RCTExecuteOnMainQueue_block_invoke
_dispatch_call_block_and_release
_dispatch_client_callout
_dispatch_main_queue_callback_4CF
__CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__
__CFRunLoopRun
CFRunLoopRunSpecific
GSEventRunModal
UIApplicationMain
main
start
0x0
@ashnfb
Copy link
Contributor

ashnfb commented Apr 24, 2019

@jonas-arkulpa can you try this fix in my master branch before I do a pull request
https://github.com/nfb-onf/react-native-video

cc @cobarx @fubar

@jonas-app
Copy link
Author

@ashnfb Thx for the really fast response.
I tried your fix with positive and negative results. :)

With the change our implementation does not crash if the controls prop is set true.
Atm onEnd and onFullscreenPlayerWillDismiss do not get called.

With active controls another issue we've since the update is partly fixed.
The player does start in expanded fullscreen by default (I have no clue how this functionality is properly called). Also the state of the button is wrong: First press just toggles the button state and the second shrinks the video. Not sure why the control prop changes this behaviour. I think this existed before and might be better handled in a different issue.

controls=true:
fullscreen-controls-true

controls=false:
fullscreen-controls-false

The grey background is the video. Tested on an iPhone X.

@ashnfb
Copy link
Contributor

ashnfb commented May 1, 2019

@jonas-arkulpa @cobarx
Try my master branch again --> https://github.com/nfb-onf/react-native-video, it includes fixes for the onXXX methods. I did not look at the controls issue.

iOS 11+ has much better methods for method-based entry into fullscreen mode. The code in the repo is much more complex than it needs to be because of legacy iOS support.

@jonas-app
Copy link
Author

@kaybutter Do you have the time to test this the following weeks?

@kaybutter
Copy link

@jonas-arkulpa Yup. Should find some time next week :)

@kaybutter
Copy link

Sorry, haven't had time to look at this yet. Will try my best to get to it quickly.

@kaybutter
Copy link

kaybutter commented Jun 26, 2019

@ashnfb Sorry that it took so long. I've tried using your branch. I noticed that the video refuses the play in fullscreen now, if I set controls to true :(

@joemewes
Copy link

@ashnfb any update on this? I have an issue with fullscreen via presentFullscreenPlayer and dismissFullscreenPlayer with controls set to true || false. This may be related. Im gonna report on another issue with attached links to example repos. thanks.

@joemewes
Copy link

joemewes commented Nov 4, 2019

We have this issue too when setting fullscreen via presentFullscreenPlayer():

Simulator/Device (iOS)

  • Tested with local .mp4, remote .mp4 and remote .m3u8.

  • with controls set to true the simulator throws the following error:

Error setting property 'fullscreen' of RCTVideo with tag #3: Exception thrown while executing UI block: Application tried to present modally an active controller <UIViewController: 0x7fabe8c0fea0>.
  • with controls set to false the simulator does not throw and error but the video loses frame playback and gets stuck, forcing faster playback/catchup when you programmatically dismiss full screen mode via dismissFullscreenPlayer().

I've setup a test repo to reproduce and debug : https://github.com/4AllDigital/RNV-iOS

I'm happy to do any work/testing on this, but Im not an Objective-C developer and it seems that the issue is on that side, possibly around https://github.com/react-native-community/react-native-video/blob/master/ios/Video/RCTVideo.m#L1257, but I'd be guessing.

joemewes added a commit to joemewes/react-native-video that referenced this issue Nov 4, 2019
@ashnfb
Copy link
Contributor

ashnfb commented Nov 12, 2019

Hi @joemewes, I've been away for 2 months, and haven't been looking into react-native-video for some time. I will try and have a look in the next few weeks at this issue if I can find the time; I recall it being pretty ugly to fix because of legacy iOS APIs. Apologies that I can't help more immediately.

@joemewes
Copy link

@ashnfb no worries. That's great and no stress about timings. I still think open-source projects and maintainers amazing! lol.

bitmoji

@mattpocock
Copy link

mattpocock commented Nov 25, 2019

Hello all, this is also happening for us, but on an iPad 7th gen running iOS 13.1.

Our (stripped-back for clarity) implementation:

<Video
  ref={(ref) => (videoElementRef.current = ref)}
  source={{ uri: source }}
  onReadyForDisplay={() => {
    videoElementRef.current.presentFullscreenPlayer();
  }}
  allowsExternalPlayback={false}
/>

Thanks for your time :)

EDIT: rolling back to 3.1.0 fixed this for us.

@luco
Copy link

luco commented Dec 3, 2019

+1 here.

@Rehubbard
Copy link

Any update on this change? I am getting the same error. I am trying to make the video automatically go full screen if the user switches to landscape during playback.

@ekzotech
Copy link

I have this issue too, with react-native-video version 5.0.2.
I can fullscreen player on iOS by clicking native control, but I get error if I'm calling presentFullscreenPlayer().

@skykingit
Copy link

i have the same error on IOS,
but when i set control={false},it works for me
when can fix the issues?

@Saad9624
Copy link

Saad9624 commented May 4, 2020

source={{ uri: source }}

hello can you specify your source ?

@KalumDog
Copy link

same issue with react-native-video version@5.0.2

@iamsantoshyadav
Copy link

I have the same issue in the TVOS app, any update?

@fernandopascoalbr
Copy link

to fullscreen work fine you need set absoluteFill (top:0, bottom:0, left:0, right:0) in Video component

@iamsantoshyadav
Copy link

to fullscreen work fine you need set absoluteFill (top:0, bottom:0, left:0, right:0) in Video component

yes, I did it already. I'm facing the same issue which was mentioned by @joemewes in his earlier comment #1568 (comment)

@iamsantoshyadav
Copy link

iamsantoshyadav commented Dec 12, 2020

I've observed one thing here which allows playing video on fullscreen mode, I hope it will help others

use onFullscreenPlayerDidDismiss event with video like following

<Video 
    source={{
        uri: "your_video_url",
        headers: buildHeader()
      }}
     ref={(ref) => {
         this.player = ref
      }}
     repeat={false}
     onEnd={() => { console.log('onEnd') }}
     onFullscreenPlayerDidDismiss={() => { console.log('onFullscreenPlayerDidDismiss') }}
     onProgress={(evt) => {  }}
     onBuffer={this.onBuffer}
     onError={this.videoError}
     style={{
        position: 'absolute',
        top: 0,
        left: 0,
        bottom: 0,
        right: 0,
     }} 
/>

As you can see here I'm not doing anything here with this event but fullscreen working perfectly once I mentioned this event

@Jkettler
Copy link

It's annoying but for now I'm just handling it like:

<Video
  controls={Platform.OS === 'android' || !fullScreen}
   // ...other props
>

where fullScreen is a boolean state variable.

Works for me on version 5.1.1, but since the fullscreen prop doesn't seem to work on either platform, I also need to manage the toggling of fullscreen with videoPlayer.current.presentFullscreenPlayer();

@ghCarol
Copy link

ghCarol commented Mar 25, 2021

I've observed one thing here which allows playing video on fullscreen mode, I hope it will help others

use onFullscreenPlayerDidDismiss event with video like following

<Video 
    source={{
        uri: "your_video_url",
        headers: buildHeader()
      }}
     ref={(ref) => {
         this.player = ref
      }}
     repeat={false}
     onEnd={() => { console.log('onEnd') }}
     onFullscreenPlayerDidDismiss={() => { console.log('onFullscreenPlayerDidDismiss') }}
     onProgress={(evt) => {  }}
     onBuffer={this.onBuffer}
     onError={this.videoError}
     style={{
        position: 'absolute',
        top: 0,
        left: 0,
        bottom: 0,
        right: 0,
     }} 
/>

As you can see here I'm not doing anything here with this event but fullscreen working perfectly once I mentioned this event

I can't even see the log information here using the same code. It seems onFullscreenPlayerDidDismiss() on my ios simulator device does not work at all.

@AdamLee321
Copy link

AdamLee321 commented Nov 15, 2021

Does anyone have a solution for this? I am using and PR to have fullscreen for Android so I can't rollback to 3.1.0.

Edit: Current solution

<Video
  ref={player => (this.player = player)}
  repeat={false}
  fullscreen={this.state.fullscreen}
  onPlaybackRateChange={({playbackRate}) => {
    if (playbackRate == 1 && Platform.OS != 'ios') {
      this.setState({fullscreen: !this.state.fullscreen});
    }
  }}
  source={{
    uri: item.url,
  }}
  paused={true}
  controls
  style={styles.imageBackground}
/>

@freeboub freeboub added Platform: iOS triage needed Help needed to confirm the issue labels Jun 4, 2022
@hueniverse hueniverse added stale Closed due to inactivity or lack or resources and removed triage needed Help needed to confirm the issue labels Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Closed due to inactivity or lack or resources
Projects
None yet
Development

Successfully merging a pull request may close this issue.