Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Media3 MediaSession custom error messages #543

Open
abouda opened this issue Jul 26, 2023 · 35 comments
Open

Media3 MediaSession custom error messages #543

abouda opened this issue Jul 26, 2023 · 35 comments
Assignees

Comments

@abouda
Copy link

abouda commented Jul 26, 2023

Is there an equivalent in media3 for these?

  • MediaSessionConnector.setCustomErrorMessage() to send a custom error message regardless of player state.
  • MediaSessionConnector.setErrorMessageProvider(PlayerErrorMessageProvider) to convert PlaybackException to a custom error message for connected controllers.
@marcbaechinger
Copy link
Contributor

marcbaechinger commented Jul 26, 2023

There is no such functionality in Media3.

Who would be the consumer of the error messages?

@abouda
Copy link
Author

abouda commented Jul 26, 2023

  • The consumer for MediaSessionConnector.setCustomErrorMessage() is AndroidAuto. It sends respective error messages to be displayed on the AndroidAuto screen when something goes wrong with a play action. (for example user not logged in, no book available for resumption, playback error custom message). Right now with media3, the error message just says "Source error" and I want it to be more user friendly.

  • The consumer for MediaSessionConnector.setErrorMessageProvider(PlayerErrorMessageProvider) is the MediaControllerCompat in my app (I haven´t migrated that part to media3 yet). I map the PlaybackException to a respective error message that is user friendly, and display it in an alert dialog.
    I see that media3 MediaController has a onPlayerError(PlaybackException) so I am guessing that could be the solution and handle the PlaybackException on the MediaBrowser side.

@abouda
Copy link
Author

abouda commented Jul 26, 2023

Also another point is although I could handle the error onPlayerError(PlaybackException) in my app, it does not solve it for legacy controllers like AndroidAuto.

@marcbaechinger
Copy link
Contributor

Yes, this is a regression compared with what's possible with the legacy API.
We need to look into this. Marked as enhancement.

@Bwaim
Copy link

Bwaim commented Feb 9, 2024

@marcbaechinger do you hav any news on this ?

@marcbaechinger
Copy link
Contributor

Yeah, sorry no news yet.

I don't think this will be part of 1.3 I'm afraid. We will update this issue as soon as we pushed a change that adds this.

@Flyktsodan
Copy link

Our app update got rejected in playstore review because we didn't send error messages when using android auto. In other words this should be a blocker for many media app(s).

@benjaminVadon
Copy link

@marcbaechinger have you any update about this limitation ? Is it planned for the next version ?

@marcbaechinger
Copy link
Contributor

This is planned for 1.4 which is the next release.

copybara-service bot pushed a commit that referenced this issue May 14, 2024
This allows to set custom error message for instance on Android
Auto/Automotive OS.

Issue: #543
PiperOrigin-RevId: 633610089
@marcbaechinger
Copy link
Contributor

The commit above added an API to send non-fatal errors to a MediaController.

When sent to all controllers or specifically to the media notification controller (MediaSession.getMediaNotificationControllerInfo()), the error code, message and extras are included in the PlaybackStateCompat with state=STATE_ERROR (7) and sent to the platform/legacy session from where a platform or legacy controller (for instance Android Auto) can read it.

copybara-service bot pushed a commit that referenced this issue May 14, 2024
This change resets the error in the platform error state immediately
to make sure that the custom error is reflected only very briefly
and then gets back to the playback state that actually reflects the
player state.

Issue: #543
PiperOrigin-RevId: 633626180
@icutvaric
Copy link

Hello!

Any updates on this?

Or any suggestion how to do authentication on Automotive OS with Media3: https://developer.android.com/training/cars/media/automotive-os#require-sign-in?

Thanks!

@kelmer44
Copy link

Is there a reason why a resource Id is necessary? Could the method not accept just a plain string as well?

@marcbaechinger
Copy link
Contributor

This API has changed. The change that includes this is on its way into the main branch.

There will be a SessionError instead of the three parameters: MediaSession.sendError(ControllerInfo, SessionError). The message argument of the constructor of MediaSession is a String instead of a resource ID. Please use a localized string either way for the benefit of your users.

@kelmer44
Copy link

@marcbaechinger thats great! I do plan on using a localized string, only we dont use the android resources in our project, but an alternative solution

@chillbrodev
Copy link

This is planned for 1.4 which is the next release.

Is there a potential release date for 1.4?

@marcbaechinger
Copy link
Contributor

We are currently preparing the beta01. The stable release of 1.4.0 is expected to be in mid July.

@Flyktsodan
Copy link

I tried the latest version and it works in terms of showing a error message. The android auto companion app still shows "Getting your selection" and users have to navigate back and try again. Is there any way of communicating that the ListenableFuture failed and it should close the "player view" in android auto? See screenshot for reference:
Screenshot 2024-08-21 at 11 17 50

@marcbaechinger
Copy link
Contributor

Thanks for reporting.

What did you do this in the old world to make 'close the "player view" in android auto'?

Not sure if this is what you are asking about, but I mean to remember that you can send a PendingIntent back to Auto that then opens that activity for resolving the error in some ways. Is this what you are asking for?

This is documented here: https://developers.google.com/cars/design/create-apps/media-apps/signin-flow

If this is the case, then you can add the same extras key/value to the LibraryResult that does report the authentication error code. Something like this:

return Futures.immediateFuture(
  LibraryResult.ofError(
    LibraryResult.RESULT_ERROR_SESSION_AUTHENTICATION_EXPIRED,
    LibraryParams.Builder()
       .setExtras(getExpiredAuthenticationResolutionExtras()).build()
  )
)

private fun getExpiredAuthenticationResolutionExtras(): Bundle {
        return Bundle().also {
            it.putString(
                EXTRAS_KEY_ERROR_RESOLUTION_ACTION_LABEL_COMPAT,
                getString(R.string.login_button_label))
            val signInIntent = Intent(this, SignInActivity::class.java)
            @OptIn(UnstableApi::class)
            val flags = if (Util.SDK_INT >= 23) PendingIntent.FLAG_IMMUTABLE else 0
            it.putParcelable(
                EXTRAS_KEY_ERROR_RESOLUTION_ACTION_INTENT_COMPAT,
                PendingIntent.getActivity(this, /* requestCode= */ 0, signInIntent, flags))
        }
    }

@Flyktsodan
Copy link

Flyktsodan commented Aug 21, 2024

Thanks for the quick reply, with our old ExoPlayer implementation we would authenticate for playback, if it failed we would call mediaSessionConnector.setCustomErrorMessage(errorMessage). That resulted in a message being shown in the android auto ui (see attached screenshot).

With media3, we authenticate for playback in onSetMediaItems (when it's called from an Android auto controller). The difference here is calling mediaSession.sendError() displays a toast instead and the screen remains in "Getting your selection" (as shown in first screenshot). I've tried returning a Futures.immediateFailedFuture in onSetMediaItems but it doesn't have any effect. The only way I can get the screen to show the "Source error" is by returning a broken MediaItem that can't be played.

Screenshot 2024-08-21 at 12 06 36

@Flyktsodan
Copy link

@marcbaechinger can you think of any way in how to replicate the old behaviour described above?
I'm worried app will be rejected in playstore review if this is not fixed somehow.

@marcbaechinger
Copy link
Contributor

I think Media3 is currently missing an API to send a custom fatal exception in an easy way.

The current version allows to send non-fatal errors as you do and it allows to customize error code and error message of a PlaybackException. However, there isn't an easy way do originate a fatal PlaybackException that isn't coming from the Player.

https://developer.android.com/media/media3/session/control-playback#error-handling

Sorry for not having a better answer than that we should prioritize this to provide this with the next release.

@manthan34
Copy link

Thanks for the quick reply, with our old ExoPlayer implementation we would authenticate for playback, if it failed we would call mediaSessionConnector.setCustomErrorMessage(errorMessage). That resulted in a message being shown in the android auto ui (see attached screenshot).

With media3, we authenticate for playback in onSetMediaItems (when it's called from an Android auto controller). The difference here is calling mediaSession.sendError() displays a toast instead and the screen remains in "Getting your selection" (as shown in first screenshot). I've tried returning a Futures.immediateFailedFuture in onSetMediaItems but it doesn't have any effect. The only way I can get the screen to show the "Source error" is by returning a broken MediaItem that can't be played.

Screenshot 2024-08-21 at 12 06 36

Hi @Flyktsodan ,

I’m facing a similar issue as mentioned in your post and would greatly appreciate your help with the following:

  1. How did you manage to display a custom error message on the Android Auto screen?
  2. You mentioned returning a broken MediaItem. Could you please provide a sample code or more details on how you handled this?

Thanks in advance!

@NikSatyr
Copy link

@marcbaechinger considering that 1.5.0 is almost out, and there are no mentions of the requested feature, could you please let us know if and when there is going to be a way of managing non-player-originated fatal errors?

This currently is a huge risk in terms of passing the Google Play review for us, thus stalling the migration to media3

Sorry for not having a better answer than that we should prioritize this to provide this with the next release.

@NikSatyr
Copy link

to re-iterate on the importance of this

we need a way to customize the error message shown for the Android Auto, since Google Play reviewers reject the app if they are presented with a generic "Source error" message. We need to be able to tweak this according to our logic (e.g. see the screenshot below)

image

This includes 2 places where we'd like to initiate this error.

first one is during the media items request: the user might be logged out, using a non-premium account etc. so I believe that convenient APIs would look smth like this:

override fun onAddMediaItems(
        mediaSession: MediaSession,
        controller: MediaSession.ControllerInfo,
        mediaItems: MutableList<MediaItem>
    ): ListenableFuture<MutableList<MediaItem>> = coroutineScope.future {
        // We load the media items according to our needs
        val items = androidAutoCallbackHandler.addMediaItems(mediaItems)

        val shouldPresentError = isUserUnauthorized

        if (items.isEmpty() || shouldPresentError) {
            throw IllegalStateException("Customized error message")
        }

        return@future items.toMutableList()
    }

secondly, some of our content might be geo-blocked, resulting in an ExoPlaybackException to be thrown by player. in this case, the error message would be just "Source error", which is not good enough for the Google Play review. this one is coming from ExoPlaybackException#deriveMessage, which is not modifeable. we'd like to have a way to define this one by ourselves, based on the cause of the ExoPlaybackException

I'd be happy to provide any additional info you might need

@marcbaechinger
Copy link
Contributor

@NikSatyr
Copy link

NikSatyr commented Nov 22, 2024

they do, although this is a really cumbersome way and I think a simpler alternative would be much appreciated.

however, this is not applicable to the 1st use-case with the onAddMediaItems way

@marcbaechinger
Copy link
Contributor

marcbaechinger commented Nov 22, 2024

Ok thanks.

Do I understand correctly that Auto is requesting you to play a song that the user isn't allow to play? So it was presented in the UI, then the user logged out or similar and then the user presses to play that item?

@NikSatyr
Copy link

Yes, you are correct.

One of the possible follow-ups: why don't I just refresh the content tree to show no items for the logged out user?

This is simply not an option because of our UI/UX requirements, and would be a regression from the legacy media session where we had such kind of control (by the means mentioned by some of the authors above)

@marcbaechinger
Copy link
Contributor

marcbaechinger commented Nov 22, 2024

Ah, ok.

I would indeed have suggested to trigger a onChildrenChanged event for this parentId and then return an error from onGetChildren() with LibraryResult.RESULT_ERROR_SESSION_AUTHENTICATION_EXPIRED. That should trigger a fatal error in the platform media session which is similar or the same what you were able to do with the legacy PlaybackStateCompat.

But if this isn't an option for UX reasons we need to seek for other ways.

I think a non-fatal is probably not working and I also don't know the impact of that regarding UX:

session.mediaNotificationControllerInfo?.let {
   session.sendError(
      it,
      SessionError(SessionError.ERROR_SESSION_AUTHENTICATION_EXPIRED, "message", Bundle()),
   )
}

Sorry, for asking questions for my education:

What was the impact on the UI with the legacy library? You kept the listing, then the user clicked and you created an error with PlaybackStateCompat. I think that's what you did.

What was AAOS then displaying, or more generally what are the UX guidelines in this case?

@NikSatyr
Copy link

NikSatyr commented Nov 25, 2024

no problems with asking questions, I'm glad to help :)

first of all, the impact of the non-fatal you are describing above with the session.sendError. since we cannot provide the media items for the unauthorized user, we return an empty list

    override fun onAddMediaItems(
        mediaSession: MediaSession,
        controller: MediaSession.ControllerInfo,
        mediaItems: MutableList<MediaItem>
    ): ListenableFuture<MutableList<MediaItem>> = coroutineScope.future {
        if (isUnauthorized) {
            mediaSession.sendError(
                controller,
                SessionError(
                    SessionError.ERROR_SESSION_AUTHENTICATION_EXPIRED,
                    "You are unauthorized"
                )
            )
            return@future mutableListOf()
        }

        val items = androidAutoCallbackHandler.addMediaItems(mediaItems)
        return@future items.toMutableList()
    }

this will result in a stuck "Getting your selection" screen (unless you tap the back button), and an error toast shown at the bottom for a couple of seconds:

389551322-702d7148-bd33-4c25-bf94-c6d3e30e5753

this approach has a serious flaw when used from onAddMediaItems: this error is displayed for a short period time, while the driver might be unable to give a quick look at the AA display due to the traffic or whatever. this means that once they finally are able to take a look at why the music has stopped, they won't see any error message. in terms of Google Play Store reviewers, this is unacceptable

You kept the listing, then the user clicked and you created an error with PlaybackStateCompat. I think that's what you did.

yes, that's correct. here's a flashback to our previous media2 implementation. things were simpler since we could hack into session.sessionCompat and set the error playback state ourselves. the code for this used to look smth like this:

    @SuppressLint("RestrictedApi")
    @WorkerThread
    override fun onSetMediaUri(
        session: MediaSession,
        controller: MediaSession.ControllerInfo,
        uri: Uri,
        extras: Bundle?
    ): Int {
        runBlocking {
            if (isUnauthorized) {
                val errorState = PlaybackStateCompat.Builder()
                    .setState(PlaybackStateCompat.STATE_ERROR, 0, 0f)
                    .setErrorMessage(PlaybackStateCompat.ERROR_CODE_AUTHENTICATION_EXPIRED, "You are unauthorized")
                    .build()

                session.sessionCompat.setPlaybackState(errorState)
            } else {
                someOurHelper.play(uri)
            }
        }
        return SessionResult.RESULT_SUCCESS
    }

this would result in opening a fullscreen AA player with the error state that would persist until the user took some action (e.g. navigating back and playing smth else)

389510808-9cfbcd08-0c9e-47bd-9c7e-15a03442871f

this behavior based on the following guidelines: https://developer.android.com/training/cars/media#errors

please let me know if you need any additional input from me

@NikSatyr
Copy link

@marcbaechinger kind reminder in case this got lost

@marcbaechinger
Copy link
Contributor

marcbaechinger commented Nov 27, 2024

Thanks I have nothing to add I'm afraid.

When you return a LibraryResult.RESULT_ERROR_SESSION_AUTHENTICATION_EXPIRED for one of the library session callbacks, a fatal error is written into the PlaybackState of the platform session. If this isn't working please file a bug. If this isn't a solution for your use case the I'm afraid, but I can't drop everything else to work on this now.

@NikSatyr
Copy link

@marcbaechinger I'd be happy to return LibraryResult.RESULT_ERROR_SESSION_AUTHENTICATION_EXPIRED for onAddMediaItems, but the problem is this method expects a list of MediaItem to be returned. so there's simply no way to return the error here.

would you like me to file a separate issue for you to take a look once you have capacity?

in the meantime, here are my workarounds to make this work leveraging the current APIs:

  1. Use https://developer.android.com/media/media3/session/control-playback#fatal-errors as a base to customize the error messages. I managed to simplify this a bit:
    private val listeners: MutableList<Listener> = mutableListOf()

    private var customizedPlaybackException: PlaybackException? = null

    private val errorCustomizationListener = ErrorCustomizationListener()

    init {
        exoPlayer.addListener(errorCustomizationListener)
    }

    override fun addListener(listener: Listener) {
        listeners.add(listener)
        super.addListener(listener)
    }

    override fun removeListener(listener: Listener) {
        listeners.remove(listener)
        super.removeListener(listener)
    }

    override fun getPlayerError(): PlaybackException? = customizedPlaybackException


    private inner class ErrorCustomizationListener : Listener {

        override fun onPlayerErrorChanged(error: PlaybackException?) {
            customizedPlaybackException = error?.let { customizePlaybackException(it) }
            listeners.forEach { it.onPlayerErrorChanged(customizedPlaybackException) }
        }

        override fun onPlayerError(error: PlaybackException) {
            listeners.forEach { it.onPlayerError(customizedPlaybackException ?: error) }
        }

        private fun customizePlaybackException(error: PlaybackException): PlaybackException {
            return PlaybackException(
                errorInfoProvider.provideErrorInfo(error).message,
                error.cause,
                error.errorCode,
            )
        }
    }
  1. In onAddMediaItems I return a media item with a broken url that will never play but trigger a source error:

    override fun onAddMediaItems(
        mediaSession: MediaSession,
        controller: MediaSession.ControllerInfo,
        mediaItems: MutableList<MediaItem>
    ): ListenableFuture<MutableList<MediaItem>> = coroutineScope.future {
        val items = // load the items

        if (items.isEmpty()) {
            val metadata = MediaMetadata.Builder()
                .setTitle("Loading...") // This is for a bit prettier UI on Android Auto
                .setIsPlayable(true)
                .setIsBrowsable(false)
                .setExtras(bundleOf("error_message" to "Failed to play because of your reasoning"))
                .build()

            val brokenItem = MediaItem.Builder()
                .setUri("https://non-existing-url.com/customize-for-your-needs")
                .setMediaMetadata(metadata)
                .build()

            return@future mutableListOf(brokenItem)
        }

        return@future items.toMutableList()
    }
  1. this source error will be passed through the error customizer from the step 1. here's the chance to tweak the error that would look smth like this:
    override fun provideErrorInfo(exception: PlaybackException): PlaybackErrorInfoProvider.ErrorInfo {
        return when {
            playbackManager.getCurrentMetadata().extras.getString("error_message") != null -> {
                PlaybackErrorInfoProvider.ErrorInfo(
                    SessionError.ERROR_BAD_VALUE,
                    "Failed to play due to some customized reason"
                )
            }

            // Regular customizations unrelated to AA
            else -> return PlaybackErrorInfoProvider.ErrorInfo(
                SessionError.ERROR_SESSION_PREMIUM_ACCOUNT_REQUIRED,
                "Whatever"
            )
        }

and here's how it would look like after tapping on the media item that is not allowed to play in Android Auto:

loading error

@marcbaechinger
Copy link
Contributor

marcbaechinger commented Nov 27, 2024

Yeah, I meant returning that from onGetChildren() from the MediaLibrarySession.Callback.

I think the remaining piece of this issue is adding ways for apps to generate their own custom PlaybackException even if there isn't a PlaybackException coming from the player. This is a remaining of the legacy library that leaks into the new API with this. In media1 policy errors in the MediaBrowserServiceCompat have been reported as fatal exception in the playback state. This wasn't nicely separated in concerns IMO. So we added the workaround that returning an authentication error from the MediaLibraryCallback.onXyz() triggers a fatal exception in the platform playback state.

So I think regarding the custom PlaybackException there isn't a need to create a new issue.

I'll do my best to deliver this asap.

@c-erdos
Copy link

c-erdos commented Jan 14, 2025

Hi - I'm a PM on the Atlas team checking in on status of this fix? We're building a new instance of Android Auto and need to implement different error messages for different scenarios: https://www.figma.com/design/aFsWpw4nybSQQzUqA4C5fz/CarPlay-%26-Android-Auto%3A-Eng-Spec?node-id=10575-22035&t=kH0E3tldfKnDaKxU-4

I was told this is not currently possible on Android Auto. What's timeline look like for fixing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests