Closed jonathangreco closed 3 years ago
@cobarx ping :)
Hi.
This bug has been tested on Ipad air 2 and Iphone 8. I can give a quick video that shows the issue.
here's multiple screens that tries to explain how to reproduce it (taken from Iphone 8 IOS 12.1):
Initial state of the screen
on click we show the video, and native controls set to true we go to fullscreen :
Then 2 bugs are happening here (on both Ipad Air & Iphone 8) when we go out of fullscreen :
You can see now that the layout is broken, means that it's now take half of the screen. The upper side is scrollable tho. If we not go to fullscreen this problem never happen.
And if we stay on fullscreen until the video ends, on Ipad Air only screen freeze. We never go back to the app and need to restart it to take control again.
Is someone managed to reproduce this bug ? Is possible to deactivate fullscreen option in controls, but still have native controls available on IOS ?
I tried to reproduce it with a snack.expo.io but it seems that react-native-video does not work with snack : https://snack.expo.io/@jonathan01/iphone-fullscreen
@cobarx ping
What do you think about this issue ? Did you ever seen it ?
Still happens. Tested on iPad. After a video goes full screen and back, the view is not resized anymore when the tablet orientation change.
@cristiangu So its reproducable, that's a good thing. Thanks for your feedback on this.
@cristiangu Do you have a little basic white project that we can launch that reproduce this error ? With this it will be easier for maintainers like @cobarx to fix it.
same here, layout is broken after the video goes full screen and back on IOS device
I found that if i pressing the full screen button in the control bar, although the layout is broken, the full screen is take the whole screen that what i need.
If i run this code this.player.presentFullscreenPlayer();
the layout are not broken again and return to previous, but the video is not take the whole screen, it just change the screen to black and place the video into top-center , i tried to use resizeMode with "contain, cover, stretch" , the fullscreen style is still not same as the full screen button.
@cobarx Hi, is this issue followed by anyone ? Since 2 month now and no maintainers take this issue.
I'm having similar issues. When controls={true} fullscreen layout is broken. Everything is fine however when the controls prop is unspecified.
@cobarx @7nickgzzjr @ashnfb Sorry for the multiple ping but since this issue remains quiet from any contributor's intervention I need to know how this issue can be improved to be valid.
Thanks a lot for your future help on this.
@jonathangreco do you have a git repo with your code for testing?
Hi sorry for not responding sooner. It is hard to keep up with the volume of issues that this project generates. I don't get to put in much time on this project for my work, so it's mostly in my free time and there isn't much activity from other maintainers to help with the load. I end up putting most of my time into reviewing PRs.
However, since this is a crash it needs to get fixed asap. I should be able to take a look at this this week and see if I can sort it out. If you happen to be able to put together a PR that would be great, but I should be able to tackle it.
@ashnfb Hi, I've not any git repo for testing at the moment. But maybe this snack https://snack.expo.io/@jonathan01/iphone-fullscreen can be mounted as a project on IOS to test it. (Snack not compile react-native-video, i can't make it work)
@cobarx Don't be sorry, no problem, you may notice that I'm a patient guy,I completely understand how it's complicated to handle a community repository in addition of your work, I'm glad to see you in this thread now :)
I'm not an objective C nor a Java developper sorry so I can't make a PR in order to fix this.
It seems that it's easy to reproduce, because a lot of people came on this thread to confirm this is still happening.
I hope you'll be able to reproduce easily.
@cobarx @jonathangreco if you can wait until early next week, I will try and put together a PR for this Monday or Tuesday
That's fine I won't have time to work on anything before next week. I'll look forward to your PR when it's ready. Thanks for tackling this!
On Wed, Jan 16, 2019, 8:58 AM Ash Mishra notifications@github.com wrote:
@cobarx https://github.com/cobarx @jonathangreco https://github.com/jonathangreco if you can wait until early next week, I will try and put together a PR for this Monday or Tuesday
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/react-native-community/react-native-video/issues/1319#issuecomment-454855970, or mute the thread https://github.com/notifications/unsubscribe-auth/AD4AF4Hdzhxx2FtWCZdpWyvLGtfy3CJ2ks5vD1oygaJpZM4YWa34 .
Example code is at this branch: https://github.com/nfb-onf/react-native-video/tree/1319-fullscreen-rotation-issues
Confirmed there are issues when controls={true}. Doesn't happen if controls are set to false.
Then it's confirmed, happy here :)
@jonathangreco I'm working on it. It looks like react-native is not getting onLayout (rotation changes) when in full-screen mode via the built-in controls, so when exiting it messes up the layout. Might need a hack to fix.
@jonathangreco @cobarx fixed PR https://github.com/react-native-community/react-native-video/pull/1441 Took about 2 days :(
I'm not very happy with the fix; but it's the best I could come up with.
Here's the diagnosis and solution:
Normally, whenever we rotate the device, react-native components receive an "onLayout" event.
In this code, there are 2 different methods for rendering video: AVPlayerLayer (doesn't have controls), and an embedded AVPlayerViewController (when controls are on). The problem is when AVPlayerViewController is in fullscreen and a rotation occurs, the "onLayout" is never sent to react-native's shadowViews.
The solution is to force the reactViewController.view (the rootViewController) to update it's bounds to the rotated screen's bounds and mark the view as needing layout. This causes the right sequence of events -- ie. react-native calls onLayout correctly and layouts out all other subviews.
Couple of other notes. The basic example, isn't very basic. I've added a new example which is really barebones, and necessary for testing bugs. Feel free to rename it in the PR as appropriate.
whoaaa really thanks for the fix. For what it's worth, I really appreciate your work, even if the fix not makes you happy.
@ashnfb Hi is this planned to be merged soon ?
Hi @cobarx this can be merged
From: Jonathan notifications@github.com Sent: February 15, 2019 1:43:23 AM To: react-native-community/react-native-video Cc: Mishra Ash; Mention Subject: Re: [react-native-community/react-native-video] fullscreen mode on IOS crash layout when video ends (#1319)
@ashnfbhttps://github.com/ashnfb Hi is this planned to be merged soon ?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/react-native-community/react-native-video/issues/1319#issuecomment-463972451, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AbpNLu8r80GBVpruMhpvYNhBavu4spbyks5vNoE7gaJpZM4YWa34.
Hi @cobarx what do you need on this issue in order to merge this ? It would be nice if I can get this for my deployment of my app on the apple store next week :)
Pleeeease :)
I'm experiencing the same issue. Looking forward to the fix.
@cobarx another ping from here, please do your best on this.
I'm not having the best of luck reproducing this, but I trust @ashnfb's judgment on this and it sounds like the fix works for you @jonathangreco so I went ahead and merged it.
I did find one issue with the fix. If I go fullscreen using the button on the onscreen controls, everything works fantastic. However if I use this.video.presentFullscreenPlayer()
, it often produces the following red screen:
@ashnfb would you mind looking and seeing if you can diagnose this?
When I test going fullscreen via the method on 4.4.0 it doesn't produce the red screen, however the controls are the wrong size and in the wrong position.
@cobarx @ashnfb what's the status on this? When is it going to be released? Thanks!
Hi all, I'm sorry for the delay. I'm on parental leave for a newborn, but I will try and have a look at the issue Hampton found.
@Phil, and others - the code has been merged into master, so you can make a direct reference to that commit (or later) in your packages for now.
From: Phil Gebauer notifications@github.com Sent: March 20, 2019 10:54:30 PM To: react-native-community/react-native-video Cc: Mishra Ash; Mention Subject: Re: [react-native-community/react-native-video] fullscreen mode on IOS crash layout when video ends (#1319)
@cobarxhttps://github.com/cobarx @ashnfbhttps://github.com/ashnfb what's the status on this? When is it going to be released? Thanks!
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/react-native-community/react-native-video/issues/1319#issuecomment-475120048, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AbpNLoaOc25iDFGzBfdKS2tCbdn3n1X3ks5vYx6WgaJpZM4YWa34.
@ashnfb Hi, I tested today your fix, and didn't see any changes about the related issue.
Using the this.video.presentFullscreenPlayer()
I also encountered the error shown by @Cobarx
Well it seems your fix didn't make the trick :p
@fubar Did you manage to make this work in your project ?
@jonathangreco just to be sure... where did you pull? the fix is on the master of my fork.
@ashnfb Yes but it has been merged since and released, right ?
@jonathangreco I haven't gotten back to this yet but @ashnfb's fix (https://github.com/react-native-community/react-native-video/pull/1441) was merged on Mar 12 and a new version was released on Apr 4 (4.4.1), so the fix is out. I'll get back to this and check myself soon.
@fubar @jonathangreco as mentioned, there are additional fixes in my fork, that were not merged into the master. https://github.com/nfb-onf/react-native-video/commits/master. These were committed May 1, 2019, after the 4.4.1 release.
@ashnfb Hi, thanks for your time for your answer, I take the time to test your fork, so I started a fresh project to install your master version
Here is my package.json
{
"name": "verifCamera4",
"version": "0.0.1",
"private": true,
"scripts": {
"start": "node node_modules/react-native/local-cli/cli.js start",
"test": "jest"
},
"dependencies": {
"react": "16.8.3",
"react-native": "0.59.8",
"react-native-video": "git+https://github.com/nfb-onf/react-native-video.git#master"
},
"devDependencies": {
"@babel/core": "^7.4.5",
"@babel/runtime": "^7.4.5",
"babel-jest": "^24.8.0",
"jest": "^24.8.0",
"metro-react-native-babel-preset": "^0.54.1",
"react-test-renderer": "16.8.3"
},
"jest": {
"preset": "react-native"
}
}
What I did is react-native init verifCamera4
then
react-native link react-native-video
Then I fixed some provisionning profile by creating a new app on my developper account. Same on TestTarget...blablabla
... 1 Hour later ...
here's is my App.js (for simplicity)
/**
* Sample React Native App
* https://github.com/facebook/react-native
*
* @format
* @flow
*/
import React, {Component} from 'react';
import {Platform, ScrollView, StyleSheet, Text, View} from 'react-native';
import Video from "react-native-video";
const instructions = Platform.select({
ios: 'Press Cmd+R to reload,\n' + 'Cmd+D or shake for dev menu',
android:
'Double tap R on your keyboard to reload,\n' +
'Shake or press menu button for dev menu',
});
type Props = {};
export default class App extends Component<Props> {
render() {
return (
<View style={styles.container}>
<ScrollView>
<Text style={styles.welcome}>Welcome to React Native!</Text>
<Text style={styles.instructions}>To get started, edit App.js</Text>
<Text style={styles.instructions}>{instructions}</Text>
<View style={{flex:1, flexDirection: "row"}}>
<Video controls
source={{uri: "https://app.dollycast.io/films/102/maestro_product-1551264704/movie_maestro_product-1551264704_1.mp4"}}
style={{borderWidth: 1, width:300, height: 300}}/>
</View>
<Text style={styles.welcome}>Welcome to React Native!</Text>
<Text style={styles.instructions}>To get started, edit App.js</Text>
<Text style={styles.instructions}>{instructions}</Text>
</ScrollView>
</View>
);
}
}
const styles = StyleSheet.create({
container: {
flex: 1,
justifyContent: 'center',
alignItems: 'center',
},
welcome: {
fontSize: 20,
textAlign: 'center',
margin: 10,
},
instructions: {
textAlign: 'center',
color: '#333333',
marginBottom: 5,
},
});
For testing purpose I give you a video.mp4 to test directly in your app from my main project
The bug is still here, to reproduce :
Install the project with correct dependancies (take my package.json)
Can you confirm the bug still persist ?
I tested this on Iphone XR on version 12.2.
React Native Environment Info:
System:
OS: macOS 10.14.3
CPU: (4) x64 Intel(R) Core(TM) i5-7360U CPU @ 2.30GHz
Memory: 606.13 MB / 16.00 GB
Shell: 5.3 - /bin/zsh
Binaries:
Node: 8.9.0 - /usr/local/bin/node
Yarn: 1.15.2 - /usr/local/bin/yarn
npm: 6.9.0 - /usr/local/bin/npm
Watchman: 4.9.0 - /usr/local/bin/watchman
SDKs:
iOS SDK:
Platforms: iOS 12.2, macOS 10.14, tvOS 12.2, watchOS 5.2
Android SDK:
API Levels: 23, 24, 26, 27, 28
Build Tools: 26.0.3, 27.0.3, 28.0.0, 28.0.2, 28.0.3
System Images: android-23 | Google APIs Intel x86 Atom, android-26 | Google APIs Intel x86 Atom, android-28 | Google Play Intel x86 Atom
IDEs:
Android Studio: 3.3 AI-182.5107.16.33.5264788
Xcode: 10.2.1/10E1001 - /usr/bin/xcodebuild
npmPackages:
react: 16.8.3 => 16.8.3
react-native: 0.59.8 => 0.59.8
npmGlobalPackages:
create-react-native-app: 1.0.0
react-native-cli: 2.0.1
react-native: 0.56.0
@jonathangreco I think some of your additional views and scrollviews are causing the problem. Rotation was broken for your project without going into fullscreen video.
The code below works perfectly fine. See attached video.
return (
<View style={styles.container}>
<Text style={styles.welcome}>Welcome to React Native!</Text>
<Text style={styles.instructions}>To get started, edit App.js</Text>
<Text style={styles.instructions}>{instructions}</Text>
<Video controls
source={{uri: "https://app.dollycast.io/films/102/maestro_product-1551264704/movie_maestro_product-1551264704_1.mp4"}}
style={{ width:300, height: 300}}/>
<Text style={styles.welcome}>Welcome to React Native!</Text>
<Text style={styles.instructions}>To get started, edit App.js</Text>
<Text style={styles.instructions}>{instructions}</Text>
</View>
);
@ashnfb You are right about extra view on simulator and a device that broke the layout. Still I think developpers want to style their screen accordingly to their graphic model.
Can you give me hints about what should we do to fix that ?
If I need the video to be encapsulated in a ScrollView.. Am I screwed ? :p
Finally the issue is still here because of a ScrollView, on the little project we've tested it's working.
P.S : I noticed some weird things, when I update my code, for example change using ScrollView and View, and made some changes between the code which works and the one wich make it broken.
Once I broked it, to make it work again I have to kill the app.
Hi @jonathangreco, I think the problem is the scrollview needs a different set of styling to resize correctly in a parent view. The way I discovered the issue was giving all the views a different background color, and I was able to see the scrollview wasn't resizing. I haven't explored a solution, but I'll try and have a look at it this week. cheers
@ashnfb Hope you can do it, really thank you for your work, I will be able to test anything you'll push on my test repo
@jonathangreco just dropping an update. I haven't been able to fix this with a scrollview yet. Normally onLayout is called when a view has a flex=1. This continues to work with Views and fullscreen, however, when a ScrollView is involved, exiting Fullscreen stops the flex=1 from triggering an onLayout change, which results in the layout being incorrect. It seems to be a rendering issue deeper in the React code; and I'm not sure there is a easy workaround. Maybe @cobarx can help us?
I don't know if @cobarx can help.
I'm only able to test your work on this one. I'm not a native developper yet, but a simple react-native user :)
I can jus hope that this isssue will find an happy ending, since it's been a 8 month now. My app does not look the same without fullscreen. It's missing the "Wahouuu" effect ;p
Again many thanks to you for the time passed on this.
I don't know if it helps you, but I am using https://www.npmjs.com/package/react-native-orientation-locker for that.
When the component is unmounted or when navigation occurs, I set the orientation lock back to the value I need.
I am also experiencing this issue where the layout does not change back to portrait after rotating it to landscape. However, I noticed it does NOT happen on iPhone X (and any other screens with a notch) but, on the iPhone 6/7/8 it experiences this.
Could this be a device related issue?
@jonathangreco just dropping an update. I haven't been able to fix this with a scrollview yet. Normally onLayout is called when a view has a flex=1. This continues to work with Views and fullscreen, however, when a ScrollView is involved, exiting Fullscreen stops the flex=1 from triggering an onLayout change, which results in the layout being incorrect. It seems to be a rendering issue deeper in the React code; and I'm not sure there is a easy workaround. Maybe @cobarx can help us?
Do you have an idea about the fix to this issue? @cobarx
@jonathangreco I now have this working correctly! @CHaNGeTe was correct, and this is an orientation problem we're dealing at the react-native level, when re-rendering the layout of views. There are no code changes to react-native-video, it simply needs re-rendering on the js side - which you can do with a package, or listen to a change in Dimensions.
I've attached an example for you here https://jsfiddle.net/nunavut/vpzLg0ac/
p.s. Make sure to use my fork to test. https://github.com/nfb-onf/react-native-video
@ashnfb I tested it on an Iphone XR, the content is now centered when back to the app (exit fullscreen) but there still a white side...unusable. The rendering do its job, centered the content again. But the app use the half of the screen after rotating.
So there is progress but it's still unusable :p
I paste below my code of the screen, I also tested if your snippet, same issue.
I did a yarn upgrade to pull your last commit from your repo, but no luck.
import React, {Component} from 'react';
import {Button, FlatList, ScrollView, StyleSheet, Text, View, Dimensions} from 'react-native';
import VideoComponent from "../component/VideoComponent";
import {connect} from "react-redux";
import {bindActionCreators} from "redux";
import * as defaultActions from "../actions/defaultActions";
import * as velibActions from "../actions/velibActions";
import Icon from 'react-native-vector-icons/FontAwesome';
class MainScreen extends Component {
constructor(props) {
super(props);
this.state = {
width: Dimensions.get('window').width,
height: Dimensions.get('window').height,
videoStatus: "Not ended",
};
Dimensions.addEventListener("change", (e) => {
this.setState(e.window);
});
}
fetchData = () => {
let results = fetch(
"https://opendata.paris.fr/api/records/1.0/search/?dataset=velib-disponibilite-en-temps-reel&facet=overflowactivation&facet=creditcard&facet=kioskstate&facet=station_state"
);
results.then(response => {
response
.json()
.then(data => {
this.setState({
data: data
});
});
});
};
componentDidMount() {
this.fetchData();
this.props.actions.acceptCgu(true);
}
renderRow = ({item}) => {
let isBookmarked = (this.props.velib_bookmarked.find(velib => velib.id === item.recordid));
let jsx = <Icon.Button
name="plus"
size={20}
iconStyle={{marginRight: 0}}
backgroundColor="green"
onPress={() => this.props.velibActions.addToBookmark({id : item.recordid, station_name: item.fields.station_name}) }
/>;
if (isBookmarked) {
jsx = <Icon.Button
name="minus"
size={20}
iconStyle={{marginRight: 0}}
backgroundColor="red"
onPress={() => this.props.velibActions.removeFromBookmark(item.recordid) }
/>
}
return (
<View style={styles.rowItem}>
<View style={styles.icon}>
{jsx}
</View>
<Text>{item.fields.station_name}</Text>
</View>
);
};
onVideoEnd = (uri) => {
this.setState({
videoStatus: "Vidéo terminée! Avez vous aimé : " + uri
})
};
render() {
return (
<View style={{flex: 1, backgroundColor: 'red', width: this.state.width, height:this.state.height, alignItems: 'center',
justifyContent: 'center',}}>
<ScrollView vertical style={{flex: 1, backgroundColor: 'yellow'}}
onLayout={() => console.log('rendering scrollview')}>
<Text style={styles.welcome}>Welcome to React Native!</Text>
<VideoComponent
paused={true}
onVideoEnd={(uri) => this.onVideoEnd(uri)}
size={30}
controls={true}
uri={"https://app.dollycast.io/films/102/maestro_product-1551264704/movie_maestro_product-1551264704_1.mp4"}
/>
{this.state.data && <FlatList
data={this.state.data.records}
renderItem={(item) => this.renderRow(item)}
keyExtractor={(item) => "" + item.recordid}
style={styles.listContainer}
extraData={this.props.velib_bookmarked}
/>}
<Text style={styles.welcome}>{this.state.videoStatus}</Text>
<Button
title="Go to Home"
onPress={() => this.props.navigation.navigate('HomeScreen')}
/>
<Button
title="Go to Flex"
onPress={() => this.props.navigation.navigate('FlexScreen')}
/>
</ScrollView>
</View>
);
}
}
function mapStateToProps(state) {
return {
user: state.defaultReducer.user,
cgu_accepted: state.defaultReducer.cgu_accepted,
velib_bookmarked: state.velibReducer.velib_bookmarked
};
}
function mapDispatchToProps(dispatch) {
return {
actions: bindActionCreators(defaultActions, dispatch),
velibActions : bindActionCreators(velibActions, dispatch)
};
}
export default connect(
mapStateToProps,
mapDispatchToProps
)(MainScreen);
const styles = StyleSheet.create({
container: {
flex: 1,
justifyContent: 'center',
alignItems: 'center',
},
listContainer: {
},
rowItem: {
flexDirection: "row",
flex: 1,
height: 50,
justifyContent: "space-between",
alignItems: "center",
},
icon: {
marginRight: 10
},
welcome: {
fontSize: 20,
textAlign: 'center',
margin: 10,
},
});
@jonathangreco I was also getting the 1/2 white; it's a bit tricky because scrollview is quite broken. Send me your project by email at a.mishra@nfb.ca
From emails between @ashnfb and myself it appears that React-navigation can cause the 1/2 white screen.
But remove it (since it's a major components) does not seems to be a good idea, a package wich is incompatible with another is bad and have to be fixed from one side or the other.
But even if the white screen can be fixed for now by removing the navigation system, we still have the issue when the video finishes (the root view is destroyed ?) Also, there still exist a scroll view issue when after rotates occurred the scrool view is blocked
So far, I don't think this workaround is fully stable, it needs more investigation and work around the component ScrollView.
Maybe we can post an issue on react-native main package for the scroll view component ? We can also post an issue on react-navigation package if we are sure that it's on their side (for the white screen)
@cobarx since you have a lot to catch up on this issue, I understand you don't want to involve, but we are really stuck on this at the moment, even if @ashnfb did a really great job to investigate on this, your help would be a great thing.
We are near the solution, I'm sure we are
In case this helps anyone: I have tried https://github.com/abbasfreestyle/react-native-af-video-player as well as Expo's video module (https://docs.expo.io/versions/latest/sdk/video/) and they both have the exact same issue. Some debugging has lead me to react-navigation-stack
's StackViewLayout.js file; all of this in addition to previous comments support the conclusion that it's a lower-level issue and probably isolated to React Navigation.
I ended up implementing a custom controls interface on top of Expo's video module so that I can prevent fullscreen mode. This turned out to be a much bigger undertaking than I had hoped for but at least it's a somewhat acceptable UX. I started with https://github.com/ihmpavel/expo-video-player for the controls interface but ended up copying and heavily modifying its code to get it to an acceptable level of UX as it's rather, well, quirky out of the box, and has some bugs and outdated dependencies. If anyone is interested I'd be happy to share my modified version.
I discovered a new thing about this issue, this can maybe give a boost to its resolve. Look below a demo : https://youtu.be/tYxY1WJkuO8
@ashnfb @cobarx @CHaNGeTe any hint about this now ?
Current behavior
On IOS with the native players buttons I read a video and go to fullscreen. When the video finishes, i go back to my app and then I switch from portrait to landscape (or vice versa)
Reproduction steps
Then your app not taking the whole screen.
Expected behavior
The app should take the whole screen.
Platform
Which player are you experiencing the problem on: