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

Remove flash between poster and video #1167

Merged
merged 1 commit into from
Sep 27, 2018
Merged

Remove flash between poster and video #1167

merged 1 commit into from
Sep 27, 2018

Conversation

cmmartin
Copy link
Contributor

@cmmartin cmmartin commented Aug 4, 2018

This resolves #1128

@n1ru4l
Copy link
Contributor

n1ru4l commented Aug 6, 2018

Could you please explain why this prevents the flashing?

@cmmartin
Copy link
Contributor Author

cmmartin commented Aug 6, 2018

If you put a breakpoint in the dealloc method of RTCVideo.m, it will be fired once the video loads and the poster is removed. So, the current implementation allocates a native video player, renders it behind a poster, then once the video loads, it de-allocates the original instance and creates a new instance. That re-instantiation causes the flash. After applying my changes, dealloc is not called when the video loads and the flash is gone.

I don't know enough about the internals of React to say for sure why it is not re-using the native video player upon video load, but I would guess the fact that the RCTVideo component changes parents.

In other words, the switch from...

 <View style={nativeProps.style}>	
        <RCTVideo	
            ref={this._assignRoot}	
            {...nativeProps}	
        />
       ...
</View>

to simply...

<RCTVideo	
   ref={this._assignRoot}	
   {...nativeProps}	
/>

leads to a creation of a new RCTVideo instance

Copy link
Contributor

@n1ru4l n1ru4l left a comment

Choose a reason for hiding this comment

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

@cobarx Lets merge this!

@cobarx
Copy link
Contributor

cobarx commented Sep 27, 2018

@n1ru4l Ok, feel free to approve any of these open PRs and I'll merge them. I've been short on time to do the extensive testing to make sure that many of these work.

Before approve anything, please review and make sure the code is readable, test it, and make sure any new functionality is documented.

@cobarx cobarx merged commit 95cea76 into TheWidlarzGroup:master Sep 27, 2018
@cmmartin cmmartin deleted the patch-2 branch March 19, 2019 04:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(Android) Black Screen for a second between loading and video playback
3 participants