-
-
Notifications
You must be signed in to change notification settings - Fork 3k
bug/#3640-android-poster-not-working #4478
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
base: master
Are you sure you want to change the base?
bug/#3640-android-poster-not-working #4478
Conversation
@@ -570,7 +570,7 @@ const Video = forwardRef<VideoRef, ReactVideoProps>( | |||
); | |||
|
|||
const _onReadyForDisplay = useCallback(() => { | |||
hasPoster && setShowPoster(false); | |||
hasPoster && setShowPoster(!!hasPoster); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have any idea why it's works? Basically i can see, that !!hasPoster is always true here, so when the video is ready for display, we still show the poster?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, this was an issue we investigated long time ago and it was funny because it only happens on Android. If I remember right, yes, this was the behaviour.
onReadyForDisplay
👉 if it hasPoster
then setShowPoster(true)
Unfortunately at the moment we don't have time do deep dive into it, but in our project we patched this change in all react-native-video
versions in order to fix the black-screen
poster issue on Android.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also don't understand how it can fix something... In your app, do you change the poster value during playback initialization? It may be a corner case, but setting it to false is the expected behavior here...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@freeboub Yes, depending if the file is a video or audio. If it's audio, we're looking for it's corresponding poster and update the poster after we find it. I believe this might happen during initialization, yes, just like you say.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see why this can fix issues. It's fine for me to merge this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should implement some more advanced logic? With this change, I think the poster will always be visible under the video whenever a user provides one
This PR should fix: