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.25k stars 1.01k forks source link

[bug] useActiveTrack() doesnt reflect metadata changes #2158

Closed lovegaoshi closed 8 months ago

lovegaoshi commented 12 months ago

Describe the Bug I use useActiveTrack() to display track metadata on the player screen, but when updating metadata via updateCurrentPlayingItem or updateMetadata with an index, useActiveTrack() does not refresh.

Steps To Reproduce its here in the code: https://github.com/doublesymmetry/react-native-track-player/blob/e28536c4b3312d6ccb76c43cc7f7fdd86d1fac41/src/hooks/useActiveTrack.ts#L28 useActieTrack only listens Event.ActiveTrackChanged, not when CurrentPlaying metadata is changed i think

Code To Reproduce as above.

Replicable on Example App? fairly sure the update track metadata button in the action panel doesnt change anything in the p.ayer screen

Environment Info: not relevant

How I can Help I will make a fix

dcvz commented 12 months ago

This is an interesting issue. In v4 we have two methods that can modify the data presented on the notification:

updateMetadataForTrack will update the metadata for the underlying track itself, while updateNowPlayingMetadata will only update the data in the notification while not touching the actual track object.

activeTrack() internally relies on Event.PlaybackActiveTrackChanged which is only really being fired when a media item transitions (index changes). But I guess you could expect that at least for updateMetadataForTrack the activeTrack should update, given that the object itself has been modified. I'm not sure updateNowPlayingMetadata should update activeTrack though given that its a method specifically meant to skip direct track modification.

First thought would be to introduce an Event.ActiveTrackMetadataChanged which the useActiveTrack() hook is listening to but I'll think some more on it overnight and see where my thoughts are in the morning.

dcvz commented 12 months ago

So I've been thinking about this some more and I'm a bit torn. I still believe the above, which is that since useActiveTrack() is returning an actual Track object. It really only makes sense for updateMetadataForTrack to update this but not updateNowPlayingMetadata.

However, our example showcases useActiveTrack() as a way for you to also present the metadata of the currently playing track.. so viewing it with those lenses, you'd expect both of them to update the hook.

So I think our options are as follows:

  1. Leave useActiveTrack() as a hook that fires when the current active track is changed but add a new hook for keeping track of the current metadata displayed in notification/control center.

  2. Always update useActiveTrack. This will be messy and hacky because underneath we return the track object we already have and as stated updateNowPlayingMetadata does not update that object.

  3. Leave it entirely up to the user to track and not promote useActiveTrack as the way for displaying to your UI. I don't like this idea though because we're missing a chance of providing good value to our users since we know all the data underneath.

Looking back at the list now, I think option 1 is probably the best. Introducing a new hook for getting up-to-date current metadata presented on notification / control center.

dcvz commented 12 months ago

@puckey tagging you here since you were part of of these changes and may also have some thoughts on the subject.

lovegaoshi commented 12 months ago

I'd prefer a new metadata hook/event emitted, now i think about it since I also listen to Event.ActiveTrackChanged, wouldn't want that listener to trigger on just metadata changes. An event emitted for current metadata changes would work nicely. or maybe I missed this in the doc somewhere?

dcvz commented 12 months ago

I'd prefer a new metadata hook/event emitted, now i think about it since I also listen to Event.ActiveTrackChanged, wouldn't want that listener to trigger on just metadata changes. An event emitted for current metadata changes would work nicely. or maybe I missed this in the doc somewhere?

No. It doesn't exist yet, we'd have to make it. But yeah, I think a new hook is likely the way to go.

puckey commented 11 months ago

How about adding a param object to specify that you also want to receive track updates: useActiveTrack({ updates: true })

neoassyrian commented 9 months ago

I was about to request similar thing. I use useActiveTrack to display the track metadata on screens. This works fine with normal tracks, however if we use a live audio such as a radio stream, once we receive the new icy metadata we update the track metadata accordingly, however the details don't update on screen because the active track didn't change.

Having a separate hook to manage that, would be a great idea. We can use that to track changes of the track metadata.

tomislav-arambasic commented 8 months ago

@dcvz Any update on this?

My use case: I too have screens rendering metadata. First time radio stream screen mounts, metadata UI is rendered because I receive metadata common/timed event and update my UI and notification centre accordingly. Metadata UI updates whenever new metadata events are incoming, as long as screen is mounted.

However, if I unmount the screen and come back (stream keeps playing), I don't have the access to metadata until radio stream updates it. As you said, updating active track metadata is not reflected in activeTrack getter / hook - there's no way of obtaining metadata if stream is already playing, until timed metadata event is emitted.

I think, for now, I'll be able to resolve this using redux - inside playback service, once metadata is received, I'll have to save it to redux state and load it on screen mount, as a temporary metadata, until new timed metadata is received. But this is hacky, a temporary workaround (hopefully).

My opinion, for whatever it's worth - it definetely makes sense to have access to metadata updates for active track.

About actual implementation - I understand your option 2 might be messy, but that would be the right way. As a consumer, I'm expecting to have track updates returned within useActiveTrack and TrackPlayer.getActiveTrack.

Option 1 is viable too, consumers won't have trouble as long as they're reading documentation - as long as they're aware that useActiveTrack fires only when track has changed.

Also, in cases when isLiveStream:true, it might be legit to update active track's title & artist when timed metadata is received. Once this issue is resolved, this won't be necessary, but it still might be the right thing to do? 🤷

P.S. Thanks for all the work. I've just updated our RNTP from version v2 to v4 and refactored everything. Oh boy it was fun 😄 Code looks so much cleaner using RNTP v4.

Edit: I'm working on this feature, I'll try to submit PR within next few days.

tomislav-arambasic commented 8 months ago

@lovegaoshi If https://github.com/doublesymmetry/react-native-track-player/pull/2250 doesn't get accepted, or until it's merged, you can use this patch if it works for you. Check PR for description on what's done.

diff --git a/node_modules/react-native-track-player/android/src/main/java/com/doublesymmetry/trackplayer/module/MusicEvents.kt b/node_modules/react-native-track-player/android/src/main/java/com/doublesymmetry/trackplayer/module/MusicEvents.kt
index 9a2ee90..851af89 100644
--- a/node_modules/react-native-track-player/android/src/main/java/com/doublesymmetry/trackplayer/module/MusicEvents.kt
+++ b/node_modules/react-native-track-player/android/src/main/java/com/doublesymmetry/trackplayer/module/MusicEvents.kt
@@ -51,6 +51,7 @@ class MusicEvents(private val reactContext: ReactContext) : BroadcastReceiver()
         const val METADATA_TIMED_RECEIVED = "metadata-timed-received"
         const val METADATA_COMMON_RECEIVED = "metadata-common-received"
         const val METADATA_PAYLOAD_KEY = "metadata"
+        const val NOW_PLAYING_METADATA_CHANGED = "now-playing-metadata-changed"

         // Other
         const val PLAYER_ERROR = "player-error"
diff --git a/node_modules/react-native-track-player/android/src/main/java/com/doublesymmetry/trackplayer/module/MusicModule.kt b/node_modules/react-native-track-player/android/src/main/java/com/doublesymmetry/trackplayer/module/MusicModule.kt
index b2409a0..5240ef8 100644
--- a/node_modules/react-native-track-player/android/src/main/java/com/doublesymmetry/trackplayer/module/MusicModule.kt
+++ b/node_modules/react-native-track-player/android/src/main/java/com/doublesymmetry/trackplayer/module/MusicModule.kt
@@ -371,9 +371,17 @@ class MusicModule(reactContext: ReactApplicationContext) : ReactContextBaseJavaM
             callback.reject("no_current_item", "There is no current item in the player")

         musicService.clearNotificationMetadata()
+        musicService.updateNowPlayingMetadata(bundleToTrack(Bundle()))
         callback.resolve(null)
     }

+    @ReactMethod
+    fun getNowPlayingMetadata(callback: Promise) = scope.launch {
+        if (verifyServiceBoundOrReject(callback)) return@launch
+
+        callback.resolve(Arguments.fromBundle(musicService.getNowPlayingMetadata()))
+    }
+
     @ReactMethod
     fun removeUpcomingTracks(callback: Promise) = scope.launch {
         if (verifyServiceBoundOrReject(callback)) return@launch
diff --git a/node_modules/react-native-track-player/android/src/main/java/com/doublesymmetry/trackplayer/service/MusicService.kt b/node_modules/react-native-track-player/android/src/main/java/com/doublesymmetry/trackplayer/service/MusicService.kt
index 9d6d869..a400b26 100644
--- a/node_modules/react-native-track-player/android/src/main/java/com/doublesymmetry/trackplayer/service/MusicService.kt
+++ b/node_modules/react-native-track-player/android/src/main/java/com/doublesymmetry/trackplayer/service/MusicService.kt
@@ -46,6 +46,7 @@ class MusicService : HeadlessJsTaskService() {
     private val binder = MusicBinder()
     private val scope = MainScope()
     private var progressUpdateJob: Job? = null
+    private var nowPlayingMetadata: Bundle? = Bundle()

     /**
      * Use [appKilledPlaybackBehavior] instead.
@@ -434,6 +435,13 @@ class MusicService : HeadlessJsTaskService() {
     @MainThread
     fun updateNowPlayingMetadata(track: Track) {
         player.notificationManager.overrideMetadata(track.toAudioItem())
+
+        nowPlayingMetadata = track.originalItem
+
+        val eventPayload = Bundle()
+        eventPayload.putBundle("metadata", nowPlayingMetadata!!.clone() as Bundle)
+
+        emit(MusicEvents.NOW_PLAYING_METADATA_CHANGED, eventPayload);
     }

     @MainThread
@@ -441,6 +449,11 @@ class MusicService : HeadlessJsTaskService() {
         player.notificationManager.hideNotification()
     }

+    @MainThread
+    fun getNowPlayingMetadata(): Bundle {
+        return nowPlayingMetadata ?: Bundle()
+    }
+
     private fun emitPlaybackTrackChangedEvents(
         index: Int?,
         previousIndex: Int?,
diff --git a/node_modules/react-native-track-player/ios/RNTrackPlayer/RNTrackPlayer.swift b/node_modules/react-native-track-player/ios/RNTrackPlayer/RNTrackPlayer.swift
index 32df9a5..9ee835e 100644
--- a/node_modules/react-native-track-player/ios/RNTrackPlayer/RNTrackPlayer.swift
+++ b/node_modules/react-native-track-player/ios/RNTrackPlayer/RNTrackPlayer.swift
@@ -26,6 +26,7 @@ public class RNTrackPlayer: RCTEventEmitter, AudioSessionControllerDelegate {
     private var sessionCategoryMode: AVAudioSession.Mode = .default
     private var sessionCategoryPolicy: AVAudioSession.RouteSharingPolicy = .default
     private var sessionCategoryOptions: AVAudioSession.CategoryOptions = []
+    private var nowPlayingMetadata: [String: Any] = [:]

     // MARK: - Lifecycle Methods

@@ -754,6 +755,11 @@ public class RNTrackPlayer: RCTEventEmitter, AudioSessionControllerDelegate {

         player.nowPlayingInfoController.clear()
         resolve(NSNull())
+
+        nowPlayingMetadata = [:]
+        emit(event: EventType.NowPlayingMetadataChanged, body: [
+                "metadata": [:],
+            ] as [String : Any])
     }

     @objc(updateNowPlayingMetadata:resolver:rejecter:)
@@ -761,9 +767,22 @@ public class RNTrackPlayer: RCTEventEmitter, AudioSessionControllerDelegate {
         if (rejectWhenNotInitialized(reject: reject)) { return }

         Metadata.update(for: player, with: metadata)
+
+        nowPlayingMetadata = metadata
+        emit(event: EventType.NowPlayingMetadataChanged, body: [
+                "metadata": metadata,
+            ] as [String : Any])
+
         resolve(NSNull())
     }

+    @objc(getNowPlayingMetadata:rejecter:)
+    public func getNowPlayingMetadata(resolve: RCTPromiseResolveBlock, reject: RCTPromiseRejectBlock) {
+        if (rejectWhenNotInitialized(reject: reject)) { return }
+
+        resolve((nowPlayingMetadata))
+    }
+
     private func getPlaybackStateErrorKeyValues() -> Dictionary<String, Any> {
         switch player.playbackError {
             case .failedToLoadKeyValue: return [
diff --git a/node_modules/react-native-track-player/ios/RNTrackPlayer/RNTrackPlayerBridge.m b/node_modules/react-native-track-player/ios/RNTrackPlayer/RNTrackPlayerBridge.m
index 7741994..a6e3e0b 100644
--- a/node_modules/react-native-track-player/ios/RNTrackPlayer/RNTrackPlayerBridge.m
+++ b/node_modules/react-native-track-player/ios/RNTrackPlayer/RNTrackPlayerBridge.m
@@ -164,4 +164,7 @@ @interface RCT_EXTERN_REMAP_MODULE(TrackPlayerModule, RNTrackPlayer, NSObject)
 RCT_EXTERN_METHOD(clearSleepTimer:(RCTPromiseResolveBlock)resolve
               rejecter:(RCTPromiseRejectBlock)reject);

+RCT_EXTERN_METHOD(getNowPlayingMetadata:(RCTPromiseResolveBlock)resolve
+                  rejecter:(RCTPromiseRejectBlock)reject);
+
 @end
diff --git a/node_modules/react-native-track-player/ios/RNTrackPlayer/Utils/EventType.swift b/node_modules/react-native-track-player/ios/RNTrackPlayer/Utils/EventType.swift
index 6226222..f8273e6 100644
--- a/node_modules/react-native-track-player/ios/RNTrackPlayer/Utils/EventType.swift
+++ b/node_modules/react-native-track-player/ios/RNTrackPlayer/Utils/EventType.swift
@@ -26,6 +26,7 @@ enum EventType: String, CaseIterable {
     case MetadataChapterReceived = "metadata-chapter-received"
     case MetadataTimedReceived = "metadata-timed-received"
     case MetadataCommonReceived = "metadata-common-received"
+    case NowPlayingMetadataChanged = "now-playing-metadata-changed"

     static func allRawValues() -> [String] {
         return allCases.map { $0.rawValue }
diff --git a/node_modules/react-native-track-player/src/constants/Event.ts b/node_modules/react-native-track-player/src/constants/Event.ts
index 0914946..b072036 100644
--- a/node_modules/react-native-track-player/src/constants/Event.ts
+++ b/node_modules/react-native-track-player/src/constants/Event.ts
@@ -131,4 +131,8 @@ export enum Event {
    * See https://rntp.dev/docs/api/events#commonmetadatareceived
    **/
   MetadataCommonReceived = 'metadata-common-received',
+  /**
+   * Fired when metadata of active track has changed.
+   **/
+   NowPlayingMetadataChanged = 'now-playing-metadata-changed',
 }
diff --git a/node_modules/react-native-track-player/src/hooks/index.ts b/node_modules/react-native-track-player/src/hooks/index.ts
index c914e68..1140419 100644
--- a/node_modules/react-native-track-player/src/hooks/index.ts
+++ b/node_modules/react-native-track-player/src/hooks/index.ts
@@ -4,3 +4,4 @@ export * from './usePlayWhenReady';
 export * from './usePlaybackState';
 export * from './useProgress';
 export * from './useTrackPlayerEvents';
+export * from './useNowPlayingMetadata';
diff --git a/node_modules/react-native-track-player/src/hooks/useNowPlayingMetadata.ts b/node_modules/react-native-track-player/src/hooks/useNowPlayingMetadata.ts
new file mode 100644
index 0000000..2de1058
--- /dev/null
+++ b/node_modules/react-native-track-player/src/hooks/useNowPlayingMetadata.ts
@@ -0,0 +1,34 @@
+import { useEffect, useState } from 'react';
+import { Event } from '../constants';
+import { useTrackPlayerEvents } from './useTrackPlayerEvents';
+import { NowPlayingMetadata } from '../interfaces';
+import TrackPlayer from '..';
+
+export const useNowPlayingMetadata = (): NowPlayingMetadata | undefined => {
+  const [metadata, setMetadata] = useState<NowPlayingMetadata | undefined>();
+
+  useEffect(() => {
+    let unmounted = false;
+
+    if (unmounted) return;
+
+    TrackPlayer.getNowPlayingMetadata()
+    .then(setMetadata)
+      .catch(() => {
+        /** Only throws while you haven't yet setup, ignore failure. */
+      });
+
+      return () => {
+        unmounted = true;
+      };
+  }, [])
+
+  useTrackPlayerEvents(
+    [Event.NowPlayingMetadataChanged],
+    async (event) => {
+      setMetadata(event.metadata);
+    }
+  );
+
+  return metadata;
+};
diff --git a/node_modules/react-native-track-player/src/interfaces/events/EventPayloadByEvent.ts b/node_modules/react-native-track-player/src/interfaces/events/EventPayloadByEvent.ts
index a9d1192..a16bf59 100644
--- a/node_modules/react-native-track-player/src/interfaces/events/EventPayloadByEvent.ts
+++ b/node_modules/react-native-track-player/src/interfaces/events/EventPayloadByEvent.ts
@@ -19,6 +19,7 @@ import type { RemotePlaySearchEvent } from './RemotePlaySearchEvent';
 import type { RemoteSeekEvent } from './RemoteSeekEvent';
 import type { RemoteSetRatingEvent } from './RemoteSetRatingEvent';
 import type { RemoteSkipEvent } from './RemoteSkipEvent';
+import type { NowPlayingMetadataChangedEvent } from './NowPlayingMetadataChangedEvent';

 export type EventPayloadByEvent = {
   [Event.PlayerError]: PlayerErrorEvent;
@@ -49,6 +50,7 @@ export type EventPayloadByEvent = {
   [Event.MetadataChapterReceived]: AudioMetadataReceivedEvent;
   [Event.MetadataTimedReceived]: AudioMetadataReceivedEvent;
   [Event.MetadataCommonReceived]: AudioCommonMetadataReceivedEvent;
+  [Event.NowPlayingMetadataChanged]: NowPlayingMetadataChangedEvent;
 };

 // eslint-disable-next-line
diff --git a/node_modules/react-native-track-player/src/interfaces/events/NowPlayingMetadataChangedEvent.ts b/node_modules/react-native-track-player/src/interfaces/events/NowPlayingMetadataChangedEvent.ts
new file mode 100644
index 0000000..183fc95
--- /dev/null
+++ b/node_modules/react-native-track-player/src/interfaces/events/NowPlayingMetadataChangedEvent.ts
@@ -0,0 +1,5 @@
+import type { NowPlayingMetadata } from "../NowPlayingMetadata";
+
+export interface NowPlayingMetadataChangedEvent {
+  metadata: NowPlayingMetadata;
+}
diff --git a/node_modules/react-native-track-player/src/interfaces/events/index.ts b/node_modules/react-native-track-player/src/interfaces/events/index.ts
index 92b5df2..c4421ca 100644
--- a/node_modules/react-native-track-player/src/interfaces/events/index.ts
+++ b/node_modules/react-native-track-player/src/interfaces/events/index.ts
@@ -14,3 +14,4 @@ export * from './RemotePlaySearchEvent';
 export * from './RemoteSeekEvent';
 export * from './RemoteSetRatingEvent';
 export * from './RemoteSkipEvent';
+export * from './NowPlayingMetadataChangedEvent';
diff --git a/node_modules/react-native-track-player/src/trackPlayer.ts b/node_modules/react-native-track-player/src/trackPlayer.ts
index 4af7b18..6b0ada4 100644
--- a/node_modules/react-native-track-player/src/trackPlayer.ts
+++ b/node_modules/react-native-track-player/src/trackPlayer.ts
@@ -281,6 +281,13 @@ export function updateNowPlayingMetadata(
   });
 }

+/**
+ * Gets the metadata content of the notification (Android) and the Now Playing Center (iOS).
+ */
+export async function getNowPlayingMetadata(): Promise<NowPlayingMetadata | undefined> {
+  return (await TrackPlayer.getNowPlayingMetadata()) ?? undefined;
+}
+
 // MARK: - Player API

 /**
lovegaoshi commented 8 months ago

awesome thank you! for another flavor I have a pure js hook implementation myself.