TheWidlarzGroup / react-native-video

A <Video /> component for react-native
http://thewidlarzgroup.github.io/react-native-video/
MIT License
7.12k stars 2.88k forks source link

Migration to Fabric #2650

Open cortinico opened 2 years ago

cortinico commented 2 years ago

Feature Request

Hey all, I'm wondering if someone is looking into migrating React Native Video to use Fabric (i.e. the React Native New Architecture). I'm opening this issue to understand what's the current status of this. I saw that some of the work future work is planned here https://github.com/react-native-video/react-native-video/issues/2642 but nothing is mentioned related to Fabric. I'm wondering if it was intentionally excluded or not.

Why it is needed

Originally requested here https://github.com/reactwg/react-native-new-architecture/discussions/6#discussioncomment-2584626

Code sample

Not a real code sample, but https://github.com/react-native-community/RNNewArchitectureLibraries contains a sample of a library that is backward compatible with both the old and the new arch of React Native.

EDIT: typo

douglowder commented 2 years ago

I'm willing to take a look at this since I maintain the RN TV repo, and video is kind of important for TV :)

hueniverse commented 2 years ago

It's on my list but the top priority is to get to zero pending PRs and less than 100 open issues.

mk0116 commented 2 years ago

I can help you @hueniverse. Let me know if you need any help from me to migrate to new arch.

hueniverse commented 2 years ago

@mk0116 thanks for the offer! Let's get v6 out first and when things are a bit more stable, we can move forward with it.

capezzbr commented 2 years ago

Hey @mk0116 @hueniverse, any update on supporting the new architecture?

hueniverse commented 2 years ago

@capezzbr unless someone shows up and does the work, I don't see it happening in the near future. So far no one has expressed interest in taking a lead on this.

mk0116 commented 2 years ago

I'm waiting for v6 to roll out. It seems like v6 has a lot of changes with breaking changes. So that after rolling out v6, I guess that's a better timing we can aim to new arch migration.

cipolleschi commented 2 years ago

Hey there! @mk0116 is there any estimated date for the v6 to be released?

mk0116 commented 1 year ago

I'm not a maintainer of this library @cipolleschi I just suggested to help them migrate to new arch and @hueniverse said rolling out v6 is the priority right now.

hari-mohan-choudhary commented 1 year ago

I am using v6.0.0-alpha.4 but this is not support fabric. any way I can use in fabric

samjayhk commented 1 year ago

@hari-mohan-choudhary Hello, did you dose any additional step for using react-native-video under Fabric mode? I have tried v6.0.0-alpha.4 and v6.0.0-alpha.5, but it still showing Unimplemented Components <Video>. Thanks.

hari-mohan-choudhary commented 1 year ago

@samjayhk I am getting the same error.

prashantchothani commented 1 year ago

@douglowder request you to please let me know if the new architecture support for tvos react native for react native video is done ? Referring to below.

I'm willing to take a look at this since I maintain the RN TV repo, and video is kind of important for TV :)

freeboub commented 1 year ago

New architecture is not yet considered as stable, and nobody open a PR to support it. So RNV doesn't support it yet. But I agree this is a must have in the future!

douglowder commented 1 year ago

@prashantchothani I have not yet picked up this work

Sunbreak commented 1 year ago

Working on fabric support: https://github.com/react-native-video/react-native-video/pull/3056

BraveEvidence commented 1 year ago

This will help https://www.youtube.com/watch?v=vVI7ZAZq5e0

jiyong1 commented 1 year ago

could you please take a look at this PR

sneh2608 commented 1 year ago

could you please take a look at this PR @jiyong1 I have used your PR in my Project...It seems that there is some bug in the implementation. Even when the video component is unmounted the video plays in background even when playInBackground flag is passed as false.I am currently implementing a carousel which has video,lottie and image support. Tried various method but the video instance won't get destroyed Could you please look into this?

<Video
repeat={false}
ref={videoRef}
resizeMode="contain"
paused={pause || loading}
volume={1}
source={{uri: convertToProxyURL(url)}}
onEnd={onVideoEnd}
onError={(_error: any) => {
setLoading(false);
}}
onProgress={data => {
if (isCurrentIndex) {
onVideoProgress?.(data);
}
}}
bufferConfig={{
minBufferMs: BUFFER_TIME,
maxBufferMs: BUFFER_TIME,
bufferForPlaybackMs: BUFFER_TIME,
bufferForPlaybackAfterRebufferMs: BUFFER_TIME,
}}
onBuffer={onBuffer}
onLoadStart={onLoadStart}
onLoad={(item: OnLoadData) => {
videoData.current = item;
loadVideo();
}}
style={styles.contentVideoView}
playInBackground={false}
/>

Edit - I was able to solve this issue by implementing the onDropViewInstance method in ReactExoplayerViewmanager file and clearing the view resources when the component unmounts @Override public void onDropViewInstance(ReactExoplayerView view) { view.cleanUpResources(); ((ThemedReactContext) view.getContext()).removeLifecycleEventListener(view); }

AbdouEB commented 11 months ago

Hey, any updates on fabric support on Android as it seems like it is still an issue..

freeboub commented 10 months ago

could you please take a look at this https://github.com/react-native-video/react-native-video/pull/3122

jessiemblack commented 7 months ago

Having this issue still with RN 0.72.6. Not able to use the library

theyanniss23002 commented 7 months ago

How long will it take before you get around to developing an update to support fabric?

cipolleschi commented 7 months ago

Hello there! Have you tried to run the react-native-video component through the interop layers we provided starting from 0.72? This should be the way to use the component in a New Architecture app while it is still on the old architecture.

Also, if you tried that path, and there are issues with react-native-video, we would love to look at them, as we might be able to address them in the interop layer and that would help also other libraries/components to work better in the New Architecture through the interop layers.

theyanniss23002 commented 7 months ago

@cipolleschi I'm using react-native version 0.73, is it ok?

cipolleschi commented 7 months ago

sure, even better. We improved the Interop Layers in 0.73!

freeboub commented 7 months ago

How long will it take before you get around to developing an update to support fabric?

Nobody can provide an eta, It would be faster with more reviewers / contributors 😅 That said there are some pull requests open to integrate it!

theyanniss23002 commented 7 months ago

sure, even better. We improved the Interop Layers in 0.73!

Then I ran into a problem... version 0.73, maybe something needs to be changed in this case, but so far I have no idea what exactly

Снимок экрана 2024-01-25 в 10 28 06 Снимок экрана 2024-01-25 в 10 28 17 Снимок экрана 2024-01-25 в 10 28 32
cipolleschi commented 7 months ago

@theyanniss23002 have you registered your component in the react-native.config.js as explained in the How to use the interop layer section?

How to use the interop layer

The interop layer is shipped with React Native 0.72. If no components are registered, it does nothing. The only step required to use it is to register a component in the layer.

To register a component, you have to edit the react-native.config.js file in your application project to list the legacy components required by the app, in the Android and iOS sections:

module.exports = {
   project:{
     android: {
       unstable_reactLegacyComponentNames: [ "ComponentName" ]
     },
     ios: {
       unstable_reactLegacyComponentNames: [ "ComponentName" ]
     }
   },
};
theyanniss23002 commented 7 months ago

@theyanniss23002 have you registered your component in the react-native.config.js as explained in the How to use the interop layer section?

How to use the interop layer

The interop layer is shipped with React Native 0.72. If no components are registered, it does nothing. The only step required to use it is to register a component in the layer. To register a component, you have to edit the react-native.config.js file in your application project to list the legacy components required by the app, in the Android and iOS sections:

module.exports = {
   project:{
     android: {
       unstable_reactLegacyComponentNames: [ "ComponentName" ]
     },
     ios: {
       unstable_reactLegacyComponentNames: [ "ComponentName" ]
     }
   },
};

Thank you! I did it

Mahmoud412 commented 6 months ago

@cipolleschi i have been trying to use interop layer for react native video 6.0.0-alpha.1 and i follow How to use the interop layer section but still not working. i'm using React Native 0.72 react-native.config.js

module.exports = {
   project:{
     android: {
       unstable_reactLegacyComponentNames: [ "RCTVideo" ]
     },
     ios: {
       unstable_reactLegacyComponentNames: [ "" ]
     }
   },
};
theyanniss23002 commented 6 months ago

@cipolleschi i have been trying to use interop layer for react native video 6.0.0-alpha.1 and i follow How to use the interop layer section but still not working. i'm using React Native 0.72 react-native.config.js

module.exports = {
   project:{
     android: {
       unstable_reactLegacyComponentNames: [ "RCTVideo" ]
     },
     ios: {
       unstable_reactLegacyComponentNames: [ "" ]
     }
   },
};

Use this config

module.exports = {
  project: {
    android: {
      unstable_reactLegacyComponentNames: ['Video'],
    },
    ios: {
      unstable_reactLegacyComponentNames: ['Video'],
    },
  },
};
Mahmoud412 commented 6 months ago

@theyanniss23002 now i'm getting this error and the app crashing right after it.

android
theyanniss23002 commented 6 months ago

@theyanniss23002 now i'm getting this error and the app crashing right after it. android

I can't imagine why you are getting this error. contact the library creators

KrzysztofMoch commented 6 months ago

@Mahmoud412 You need to update to 6.0.0-beta.5 (it includes fix for that)