[New Arch] Support New Architecture Fabric Component #3122

yungblud · 2023-05-08T10:54:47Z

Update the documentation

README.md was not updated yet. It will be updated soon.

Update the changelog

CHANGELOG.md was not updated yet. It will be updated soon.

Provide an example of how to test the change

First, we created new FabricExample app in examples/FabricExample
Go to new fabric example app: cd examples/FabricExample
- bundle install: bundle install
- iOS pod install: cd ios and RCT_NEW_ARCH_ENABLED=1 bundle exec pod install
start metro server: yarn start
- run iOS: yarn ios
- run android: yarn android

we created some video-testing UI for the new fabric example app.

Focus the PR on only one area

Only for Fabric support

Describe the changes

Notes

This PR supports both new arch and old arch native component view
But, for now react-native codegen for new arch is not supporting array type of event method callback parameters. see this issue
- onTimedMetadata
- onAudioTracks
- onTextTracks
- onVideoTracks
- we marked comment for upper methods with @todo: fix type

Splitting PR by features

더나은 방식의 example 앱 추가

Fabric iOS base

…t-oldarch-ios Support old arch iOS

…t-oldarch-android Support oldarch android

yungblud · 2023-07-11T02:22:57Z

@freeboub We finished our tasks including old arch support.
So do you want to split this PR step by step?

chore: modified npm files

freeboub · 2023-07-11T06:28:06Z

android/src/fabric/java/com/brentvatne/exoplayer/events/OnAudioTracksEvent.kt

+
+    fun arrayToObject(field: String?, array: WritableArray?): WritableMap? {
+        val event = Arguments.createMap()
+        // @todo: temporarily remove put array on event callback parameter (codegen issue)


still not possible to do it with new arch ?

freeboub · 2023-07-11T06:28:52Z

android/src/fabric/java/com/brentvatne/exoplayer/events/OnTextTracksEvent.kt

+    fun arrayToObject(field: String?, array: WritableArray?): WritableMap? {
+        val event = Arguments.createMap()
+        // @todo: temporarily remove put array on event callback parameter (codegen issue)
+        // event.putArray(field!!, array)


still not possible with new arch ? If it is an issue with new Arch, it would be good to disable it only for old arch

freeboub · 2023-07-11T06:29:35Z

android/src/fabric/java/com/brentvatne/exoplayer/events/OnTimedMetadataEvent.kt

+
+        val event = Arguments.createMap()
+        // @todo: temporarily remove put array on event callback parameter (codegen issue)
+        // event.putArray(EVENT_PROP_TIMED_METADATA, metadataArray)


to reactivate on old arch

freeboub · 2023-07-11T06:30:30Z

android/src/fabric/java/com/brentvatne/exoplayer/events/OnVideoLoadEvent.kt

+        val naturalSize: WritableMap? = aspectRatioToNaturalSize(videoWidth, videoHeight)
+        event.putMap(EVENT_PROP_NATURAL_SIZE, naturalSize)
+        event.putString(EVENT_PROP_TRACK_ID, trackId)
+        // @todo: temporarily remove put array on event callback parameter (codegen issue)


to reactivate on old arch

freeboub · 2023-07-11T06:32:02Z

android/src/fabric/java/com/brentvatne/exoplayer/events/OnVideoPlaybackStateChangedEvent.kt

+    }
+
+    companion object {
+        const val EVENT_NAME = "topOnVideoPlaybackStateChanged"


are you sure of this EVENT_NAME ? I think you did a copy past issue :) (top)

freeboub · 2023-07-11T06:32:26Z

android/src/fabric/java/com/brentvatne/exoplayer/events/OnVideoProgressEvent.kt

+    }
+
+    companion object {
+        const val EVENT_NAME = "topOnVideoProgress"


topOnVideoProgress instead of onVideoProgress

freeboub · 2023-07-11T06:32:45Z

android/src/fabric/java/com/brentvatne/exoplayer/events/OnVideoProgressEvent.kt

+    private val EVENT_PROP_ORIENTATION = "orientation"
+    private val EVENT_PROP_VIDEO_TRACKS = "videoTracks"
+    private val EVENT_PROP_AUDIO_TRACKS = "audioTracks"
+    private val EVENT_PROP_TEXT_TRACKS = "textTracks"


you can remove unused constant here

freeboub · 2023-07-11T06:33:20Z

android/src/fabric/java/com/brentvatne/exoplayer/events/OnVideoTracksEvent.kt

+    fun arrayToObject(field: String?, array: WritableArray?): WritableMap? {
+        val event = Arguments.createMap()
+        // @todo: temporarily remove put array on event callback parameter (codegen issue)
+        // event.putArray(field!!, array)


to reactivate in old arch

freeboub · 2023-07-11T06:35:28Z

@yungblud thank you very much !
Yes if you have time to split the PR it would be wonderful!
Sorry, but I merged some other changes past week which causes conflicts...
Do you want me to guide you on the steps ?
you still not detail API changes in README, but you can just comment here, we will reintegrate it once it will be merged.
Thank you again !

The events management on android seems a good first step in my point of view, I just review this part.

why did you prefix all event by top ?
for the array issue, this is not possible to disable tracks management... can you at least reenable it for old architecture.

freeboub · 2023-07-11T06:44:00Z

src/types/events.ts

@@ -0,0 +1,31 @@
+import type { OnBandwidthUpdateData, OnBufferData, OnLoadData, OnLoadStartData, OnProgressData, OnSeekData, OnPlaybackData, OnExternalPlaybackChangeData, OnPictureInPictureStatusChangedData, OnReceiveAdEventData, OnVideoErrorData, OnPlaybackStateChangedData, OnAudioFocusChangedData, OnTimedMetadataData, OnAudioTracksData, OnTextTracksData, OnVideoTracksData } from '../fabric/VideoNativeComponent';


should this file be in fabric folder ?

yungblud · 2023-07-14T04:57:11Z

@freeboub Yep. I think it would be better to follow your guide about splitting PR.
So, we will make the first PR for Android Event management.
I will mention you after we create new PR!

And for the existing reviews that you wrote, we will ignore it for now.
I think it would be happy for us if you do same review again on the new PR.

yungblud · 2023-07-19T08:01:00Z

@freeboub
I am splitting PR.
Can you check this PR?

…itialize-player 안드로이드 initialize fix

Fix/ios props

freeboub · 2023-12-08T21:47:53Z

ios/Video/ReactVideoManager.swift

@@ -36,8 +51,8 @@ class RCTVideoManager: RCTViewManager {
        })
    }

-    @objc(setLicenseResultError:reactTag:)


Why did you change this ? Is there an issue ?
please see: #3406

yungblud and others added 30 commits April 13, 2023 14:41

feat: ⚡ added .nvmrc

cb5c8fb

feat: ⚡ added laftel video example app

9e6d828

feat: ⚡ added setup for exapmle

7671f80

feat: ⚡ set path

e3697d7

feat: ⚡ Pod install

03d4020

feat: ⚡ modified setup

69c917d

feat: ⚡ modified setup

efdfc7c

feat: ⚡ added example video

569abf7

Merge pull request #1 from laftel-team/feature/add-better-examples

77e79da

더나은 방식의 example 앱 추가

feat: ⚡ added mm extension for podspec

2f26d32

feat: ⚡ added support fabric podspec

8631592

feat: ⚡ added objc++ files

f6195ac

feat: ⚡ added codegen files with package json

7e4636e

feat: ⚡ moved js files to src

10d1fb9

feat: ⚡ support fabric

3ee6cf5

feat: ⚡ support objc header files

60e7e3e

feat: ⚡ export

49ea9d1

chore: ⚡ chore

be88cfd

feat: ⚡ modified RCTVideo.h

902ba2f

feat: ⚡ added prop update

70b1db3

feat: ⚡ refactored export

76caa45

feat: ⚡ typescript init

58e4024

feat: ⚡ modified Podfile.lock

7a4685c

Merge pull request TheWidlarzGroup#2 from laftel-team/feature/fabric

2d6cdcb

Fabric iOS base

refactor: js to ts

605f81e

chore: update pacakge

c081f28

chore: update example package

42157c1

example app change

d6ef859

change pod spec

f3e95a8

add video event delegate

505cb2e

yungblud added 12 commits July 10, 2023 20:02

fix: fixed compile error swif

8d91ae7

feat: added finished

876df63

feat: modified drmType, selected audio, text type oldarch

002a9ef

chore: 7.0.0-alpha.15

0679397

fix: fixed finished

23bf8a1

chore: 7.0.0-alpha.16

f04eb3e

chore: 7.0.0-alpha.17

d851bef

chore: added .npmignore on android

dba4f0c

chore: 7.0.0-alpha.18

0d17826

Merge pull request TheWidlarzGroup#25 from laftel-team/feature/suppor…

fb7274b

…t-oldarch-ios Support old arch iOS

Merge pull request TheWidlarzGroup#26 from laftel-team/feature/suppor…

536d926

…t-oldarch-android Support oldarch android

Merge branch 'laftel-master' into feature/new-arch-fabric

0469c07

yungblud added 3 commits July 11, 2023 14:34

chore: modified npm files

e110ad4

Merge pull request TheWidlarzGroup#27 from laftel-team/fix/android-build

51b6d3c

chore: modified npm files

Merge branch 'laftel-master' into feature/new-arch-fabric

e4edcaf

freeboub reviewed Jul 11, 2023

View reviewed changes

yungblud mentioned this pull request Jul 19, 2023

[New Arch] Fabric Example #3175

Merged

jiyong1 and others added 6 commits August 9, 2023 14:28

fix: add initialized flag

4208364

chore: 7.0.0-alpha.20

80bd52e

fix: ios props name

e754019

Merge pull request TheWidlarzGroup#28 from laftel-team/fix/android-in…

816598f

…itialize-player 안드로이드 initialize fix

Merge pull request TheWidlarzGroup#29 from laftel-team/fix/ios-props

fbd7452

Fix/ios props

Merge branch 'laftel-master' into feature/new-arch-fabric

e2d414d

freeboub mentioned this pull request Oct 5, 2023

[Project. Status] Version 6.0.0-alpha #2723

Closed

freeboub reviewed Dec 8, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[New Arch] Support New Architecture Fabric Component #3122

[New Arch] Support New Architecture Fabric Component #3122

yungblud commented May 8, 2023 •

edited

Loading

yungblud commented Jul 11, 2023

freeboub Jul 11, 2023

freeboub Jul 11, 2023

freeboub Jul 11, 2023

freeboub Jul 11, 2023

freeboub Jul 11, 2023

freeboub Jul 11, 2023

freeboub Jul 11, 2023

freeboub Jul 11, 2023

freeboub commented Jul 11, 2023

freeboub Jul 11, 2023

yungblud commented Jul 14, 2023

yungblud commented Jul 19, 2023

freeboub Dec 8, 2023

		@@ -0,0 +1,31 @@
		import type { OnBandwidthUpdateData, OnBufferData, OnLoadData, OnLoadStartData, OnProgressData, OnSeekData, OnPlaybackData, OnExternalPlaybackChangeData, OnPictureInPictureStatusChangedData, OnReceiveAdEventData, OnVideoErrorData, OnPlaybackStateChangedData, OnAudioFocusChangedData, OnTimedMetadataData, OnAudioTracksData, OnTextTracksData, OnVideoTracksData } from '../fabric/VideoNativeComponent';

[New Arch] Support New Architecture Fabric Component #3122

Are you sure you want to change the base?

[New Arch] Support New Architecture Fabric Component #3122

Conversation

yungblud commented May 8, 2023 • edited Loading

Update the documentation

Update the changelog

Provide an example of how to test the change

Focus the PR on only one area

Describe the changes

Splitting PR by features

yungblud commented Jul 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

freeboub commented Jul 11, 2023

Choose a reason for hiding this comment

yungblud commented Jul 14, 2023

yungblud commented Jul 19, 2023

Choose a reason for hiding this comment

yungblud commented May 8, 2023 •

edited

Loading