tidal-music / tidal-sdk-ios

TIDAL SDK for iOS
Apache License 2.0
16 stars 1 forks source link

Add logic in Player to handle issue when media services are reset #124

Closed ceciliasaraiva closed 2 weeks ago

ceciliasaraiva commented 3 weeks ago

We've got reports of an issue where the media services are reset. This causes for the player to stop working. By following Apple documentation, in this situation, we must reset (I've added this inside the sdk) and set up again the audio session (I've delegated this to client of the sdk since this is set up outside currently). Apart from that, from my testing, it seems sometimes we also could get "mediaServicesWereLost", so I've added logging for both and we can monitor what actually is happening with our users.


This pull request introduces several new features and improvements to the player engine, particularly handling media services reset scenarios. The changes include adding new error types, logging enhancements, and a new monitor for media services reset notifications.

Handling Media Services Reset:

Error Handling:

Logging Enhancements:

Listener Updates:

Notification Handling:

These changes collectively improve the robustness of the player engine in handling media service interruptions and provide better logging and error handling for such scenarios.

ziad-halabi9 commented 3 weeks ago

we must reset (I've added this inside the sdk) and set up again the audio session (I've delegated this to client of the sdk since this is set up outside currently)

Have you considered applying this solution in the client instead of the sdk since that's where the audio session setup lives currently? (i.e. the app should listen to the mediaServicesWereReset notification and act by calling reset on the player and resetting up the audio session). Or is the plan eventually to move the audio session handling to inside the sdk?

it seems sometimes we also could get "mediaServicesWereLost"

According to the documentation, we don't really need to listen to this notification. It can only serve as a cue before the server restarts. We can log it and see the number of occurrences of media services lost and compare them to when it resets. Do you see any other benefits?

ceciliasaraiva commented 3 weeks ago

we must reset (I've added this inside the sdk) and set up again the audio session (I've delegated this to client of the sdk since this is set up outside currently)

Have you considered applying this solution in the client instead of the sdk since that's where the audio session setup lives currently? (i.e. the app should listen to the mediaServicesWereReset notification and act by calling reset on the player and resetting up the audio session). Or is the plan eventually to move the audio session handling to inside the sdk?

My initial idea was to perform the reset immediately in the sdk and not delay it any further. And since we don't set up the audio session in the sdk, I figured it made sense to delegate that to the listener, so they can be set it up as desired. I'm fine if we decide to move all to one place. I'm not aware if we want to move the audio session setup to the sdk.

it seems sometimes we also could get "mediaServicesWereLost"

According to the documentation, we don't really need to listen to this notification. It can only serve as a cue before the server restarts. We can log it and see the number of occurrences of media services lost and compare them to when it resets. Do you see any other benefits?

Yes, I've only added the "lost" because I noticed some inconsistent behavior sometimes when testing (for example, I received lost but not reset notification a few times - could be issue with simulating the reset, not sure), so would like to observe this in the wild, it's just for logging and my intention would be to remove it completely after we get (hopefully) positive insights; otherwise, it could possibly help with further investigations.