doublesymmetry / react-native-track-player

A fully fledged audio module created for music apps. Provides audio playback, external media controls, background mode and more!
https://rntp.dev/
Apache License 2.0
3.18k stars 980 forks source link

chore: improve bridgeless interop layer compatibility #2290

Open behenate opened 2 months ago

behenate commented 2 months ago

Why

In react-native 0.74 when new architecture is enabled bridgeless mode will be enabled by default. By default modules which use the bridge will use an interop layer but it's not perfect. After creating a project, which uses react-native 0.74.0-rc.6 both Android and iOS don't work at all under bridgeless.

How

I managed to get the features of the example app working on iOS by removing some declarations from RNTrackPlayerBridge.m: getSleepTimerProgress, setSleepTimer, sleepWhenActiveTrackReachesEnd and clearSleepTimer are declared, but never implemented (correct me if I'm wrong), which causes a segmentation fault when using bridgeless. Removing them fixes the issue. All of the functionality seems to be working ok after the fix.

On Android some of the @ReactMethod annotated functions are assigned a value of scope.launch. This causes the interop layer to unnecessarily try to pass the returned Jobs to the JS side, which fails. Added a wrapper for scope.launch, so that the assignment can be kept for better readability, but the function return type is changed to Unit.

Unfortunately some things still don't work well with bridgeless on Android. Some of the events aren't sent correctly (eq. the PlaybackActiveTrackChanged). For some reason all of the notification controls become broken. Those issues are likely easy fixes, but I currently don't have time to learn the codebase and fix them 😕

behenate commented 2 months ago

@dcvz It would be good to have bridgeless working before 0.74 is released. Do you guys have plans for fixing the Android issues I mentioned?

brentvatne commented 2 months ago

@dcvz - any chance you could review this? react-native-track-player came up as one of the top 400 most popular packages in the react-native and we're hoping to get as many popular libraries as possible compatible leading up to the launch of react-native@0.74 - https://github.com/reactwg/react-native-new-architecture/discussions/167

lovegaoshi commented 2 months ago

thank you for the work @behenate I put together a bare minimum example app https://github.com/lovegaoshi/RNTPExampleNewArch

for the event issue, it's actually none of the hooks/useEvents work anymore; the addEventListener inside doesnt do anything I believe. however TP.addEventListener as in SetupService works. tbh i'm completely lost with the new arch stuff, if anyone have any insight, thank you very much in advance hooks are set up as https://github.com/doublesymmetry/react-native-track-player/blob/main/src/hooks/useTrackPlayerEvents.ts

notification is a bigger issue imo bc all of that should be native.

lovegaoshi commented 2 months ago

I guess somehow bridge and bridgeless are running concurrently? RNTP internal hooks is registered as NOBRIDGE and listeners added via AppRegistry.registerHeadlessTask is registered in the bridge? they use different NativeEventEmitters and the nobridge ones arent updating at all. the ... count logs are console.log(emitter.listenerCount(event), event, 'count')

(NOBRIDGE) LOG  Bridgeless mode is enabled
 (NOBRIDGE) LOG  Running "RNTPExampleNewArch" with {"rootTag":241,"initialProps":{},"fabric":true}
 (NOBRIDGE) LOG  deb playback-progress-updated
 (NOBRIDGE) LOG  0 playback-progress-updated count
 LOG  0 remote-pause count
 LOG  0 remote-play count
 LOG  0 remote-next count
 (NOBRIDGE) LOG  getInitialURL null
 LOG  0 remote-previous count
 LOG  0 remote-jump-forward count
 LOG  0 remote-jump-backward count
 LOG  0 remote-seek count
 LOG  0 remote-duck count
 LOG  0 playback-queue-ended count
 LOG  0 playback-active-track-changed count
 LOG  0 playback-progress-updated count
 LOG  0 playback-play-when-ready-changed count
 LOG  0 playback-state count
 LOG  0 playback-metadata-received count
 LOG  0 metadata-chapter-received count
 LOG  0 metadata-timed-received count
 LOG  0 metadata-common-received count
 LOG  1 playback-metadata-received count
 LOG  Event.PlaybackProgressUpdated {"buffered": 1626.357, "duration": 0, "position": 1575.25, "track": 5}
 (NOBRIDGE) LOG  deb playback-active-track-changed
 (NOBRIDGE) LOG  0 playback-active-track-changed count
 (NOBRIDGE) LOG  0 playback-state count
 (NOBRIDGE) LOG  0 playback-play-when-ready-changed count
 (NOBRIDGE) LOG  1 playback-state count
 LOG  Event.PlaybackProgressUpdated {"buffered": 1657.469, "duration": 0, "position": 1577.249, "track": 5}
 LOG  Event.PlaybackProgressUpdated {"buffered": 1659.454, "duration": 0, "position": 1579.249, "track": 5}
 LOG  Event.PlaybackProgressUpdated {"buffered": 1661.466, "duration": 0, "position": 1581.251, "track": 5}
 LOG  Event.PlaybackProgressUpdated {"buffered": 1663.451, "duration": 0, "position": 1583.252, "track": 5}
 LOG  Event.PlaybackProgressUpdated {"buffered": 1665.462, "duration": 0, "position": 1585.251, "track": 5}
 LOG  Event.PlaybackProgressUpdated {"buffered": 1667.448, "duration": 0, "position": 1587.252, "track": 5}
 LOG  Event.PlaybackProgressUpdated {"buffered": 1669.459, "duration": 0, "position": 1589.251, "track": 5}
 LOG  Event.PlaybackProgressUpdated {"buffered": 1671.444, "duration": 0, "position": 1591.247, "track": 5}
 LOG  Event.PlaybackProgressUpdated {"buffered": 1673.456, "duration": 0, "position": 1593.251, "track": 5}