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.

Sign up for GitHub

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

Jump to bottom

demo-session crashing when connected from "Media Controller Tester" #78

Open
loki666 opened this issue May 20, 2022 · 10 comments
Open

demo-session crashing when connected from "Media Controller Tester" #78

loki666 opened this issue May 20, 2022 · 10 comments
Assignees
@marcbaechinger
Labels
bug

Comments

@loki666
Copy link
Contributor

loki666 commented May 20, 2022 •
edited by marcbaechinger

Demo session sample app is crashing when conneced from https://github.com/googlesamples/android-media-controller

Tested against both release and main branches

Crash with Android API 24
On API 26 & 28 it doesn’t crash, but returns no results when browsing
Works with Android API 29 & 30

Step to reproduce
deploy both apps on device
launch media controller test app
click "control" on Media3 Session Demo in the list

2022-05-20 11:50:09.972 9872-9872/androidx.media3.demo.session E/AndroidRuntime: FATAL EXCEPTION: main
    Process: androidx.media3.demo.session, PID: 9872
    java.lang.UnsupportedOperationException: It is not supported to send an error for [rootID]
        at androidx.media.MediaBrowserServiceCompat$Result.onErrorSent(MediaBrowserServiceCompat.java:944)
        at androidx.media.MediaBrowserServiceCompat$Result.sendError(MediaBrowserServiceCompat.java:889)
        at androidx.media3.session.MediaLibraryServiceLegacyStub.onLoadChildren(MediaLibraryServiceLegacyStub.java:178)
        at androidx.media3.session.MediaLibraryServiceLegacyStub.onLoadChildren(MediaLibraryServiceLegacyStub.java:168)
        at androidx.media.MediaBrowserServiceCompat$MediaBrowserServiceImplApi21.onLoadChildren(MediaBrowserServiceCompat.java:427)
        at androidx.media.MediaBrowserServiceCompat$MediaBrowserServiceImplApi21$MediaBrowserServiceApi21.onLoadChildren(MediaBrowserServiceCompat.java:518)
        at android.service.media.MediaBrowserService.performLoadChildren(MediaBrowserService.java:662)
        at android.service.media.MediaBrowserService.addSubscription(MediaBrowserService.java:600)
        at android.service.media.MediaBrowserService.-wrap3(MediaBrowserService.java)
        at android.service.media.MediaBrowserService$ServiceBinder$3.run(MediaBrowserService.java:272)
        at android.os.Handler.handleCallback(Handler.java:751)
        at android.os.Handler.dispatchMessage(Handler.java:95)
        at android.os.Looper.loop(Looper.java:154)
        at android.app.ActivityThread.main(ActivityThread.java:6077)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:866)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:756)
@marcbaechinger
Copy link
Contributor

marcbaechinger commented May 25, 2022 •
edited

Thanks for reporting!

I think this is because the PlaybackService of the session demo advertises COMMAND_CODE_LIBRARY_SUBSCRIBE but actually doesn't override MediaLibrarySessionCallback.subscribe(). We already have implemented subscribe in the meanwhile and this will be fixed in the main branch soon and part of the next release.

see this commit also.

@loki666
Copy link
Contributor Author

loki666 commented May 28, 2022

Ok thanks.
I'll give it a try and let you know

@loki666
Copy link
Contributor Author

loki666 commented May 28, 2022

mmmh, still crashing the same way.

I think the issue is because controller is null

media/libraries/session/src/main/java/androidx/media3/session/MediaLibraryServiceLegacyStub.java

Line 178 in 72a4fb0

result.sendError(/* extras= */ null);

@marcbaechinger
Copy link
Contributor

Thanks for checking and the feedback.

I think there are two issues. The first is the crash at connection time that you report and the second is that browsing the category tree isn't working.

1) Crash at connection to session

When I test with API 27 with Android 13 (SP2A.220505.002) and T (TP1A.220519.003) I can succefully connect with the media controller test app that is using the legacy media session API and with the media controller test app that is using the Media3 API. After connecting I can see the current state of the player, the available actions and I can use the functionality of MediaSessionCompat. transport controls including custom actions.

I see you reported the crash above for API 24 so I'm going to test with that API as well. Can you confirm this only happens on API 24?

2) Browsing not working

I noticed that with the media test controller app that is using the legacy controller API, I can connect but the MediaBrowserService functionality is not working properly. I wasn't successful to browse the tree with any app I tried of which I know browsing works with other clients (UAMP, SoundCloud). I found that it works when I set a break point in the session demo app in onGetChildren. So it looks like there is a threading issue somewhere.

When testing with UAMP that is using the legacy MediaSession still I get the same results (meaning I can not browse UAMP or Soundcloud but transport controls and metadata work).

Can you browse your app with the media controller test app (like MediabrowserCOmpat accessing your MediaBrowserCompatService)?

@loki666
Copy link
Contributor Author

loki666 commented Jun 2, 2022 •
edited

Ok

for the connection crash, it seems to only happens with API 24 and API 26 (probably lower API too)
API 27+ don't throw the UnsupportedOperationException

for the browsing issue, it seems to only works on API 28+ (even with onSubscribe implemented)

I can browse my app, or the demo-session app with media controller test which (by the look of the deps) is still using MediabrowseCompat, only on API 28+ devices

@iTaysonLab
Copy link

iTaysonLab commented May 12, 2023 •
edited

Can confirm that this issue is reproducible on all Samsungs with Samsung Experience ROM (pre-One UI, also SDK 24), and it directly happens if you try to connect a MediaController directly to your application's MediaLibraryService.

Is there any workaround of it at the moment? Production users are reporting about the issues on such devices, and there is no possible way to override onSubscribe without disabling MediaLibraryService features (like losing Android Auto support)

Update: my bad, you actually can override both onSubscribe and onUnsubscribe, so I just replaced them with Futures.immediateFuture(LibraryResult.ofVoid()) so the application won't crash on SE7. I hope it works.

@victor-munoz victor-munoz mentioned this issue Jul 17, 2023
Closed
@marcbaechinger
Copy link
Contributor

I can not repro the crash when using an emulator running Android 7/API 24.

There is a bug in the media controller app that does not allow to browse the categories. Once this bug is fixed, I can connect to the session demo and navigate through the library. I can also start and pause playback of the current item and add a new item for playback when using the browser.

I have file a pull request against the media controller app to fix this: googlesamples/android-media-controller#50

rohitjoins pushed a commit that referenced this issue Aug 2, 2023
@marcbaechinger @rohitjoins
Only use result.sendError where supported by legacy media library
17ee527 
`MediaLibraryServiceLegacyStub` handles various edge cases by calling
`result.sendError(null)` with the intention to send back an error to
the legacy browser [1].

`MediaBrowserServiceCompat` of the legacy media1 Compat library has an
inner base class `Result` that has a default implementation of
`onErrorSent` that throws an `UnsupportedOperationException` [2].
However, most anonymous inner classes for `Result` created in
`MediaBrowserServiceCompat` do not override `onErrorSent` [3].

Hence Media3 must not call `sendError` in these cases. Instead we call
`sendResult(null)` according to what the default implementation of
the callbacks in `MediaBrowserServiceCompat` do ([4] as an example).

Issue: #78
Issue: #334

[1] https://github.com/androidx/media/blob/release/libraries/session/src/main/java/androidx/media3/session/MediaLibraryServiceLegacyStub.java#L200
[2] https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:media/media/src/main/java/androidx/media/MediaBrowserServiceCompat.java;l=872
[3] https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:media/media/src/main/java/androidx/media/MediaBrowserServiceCompat.java;l=578-604?q=MediaBrowserServiceCompat&ss=androidx
[4] https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:media/media/src/main/java/androidx/media/MediaBrowserServiceCompat.java;l=1395

PiperOrigin-RevId: 551210137
tianyif pushed a commit that referenced this issue Aug 14, 2023
@marcbaechinger @tianyif
Only use result.sendError where supported by legacy media library
ab2505d 
`MediaLibraryServiceLegacyStub` handles various edge cases by calling
`result.sendError(null)` with the intention to send back an error to
the legacy browser [1].

`MediaBrowserServiceCompat` of the legacy media1 Compat library has an
inner base class `Result` that has a default implementation of
`onErrorSent` that throws an `UnsupportedOperationException` [2].
However, most anonymous inner classes for `Result` created in
`MediaBrowserServiceCompat` do not override `onErrorSent` [3].

Hence Media3 must not call `sendError` in these cases. Instead we call
`sendResult(null)` according to what the default implementation of
the callbacks in `MediaBrowserServiceCompat` do ([4] as an example).

Issue: #78
Issue: #334

[1] https://github.com/androidx/media/blob/release/libraries/session/src/main/java/androidx/media3/session/MediaLibraryServiceLegacyStub.java#L200
[2] https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:media/media/src/main/java/androidx/media/MediaBrowserServiceCompat.java;l=872
[3] https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:media/media/src/main/java/androidx/media/MediaBrowserServiceCompat.java;l=578-604?q=MediaBrowserServiceCompat&ss=androidx
[4] https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:media/media/src/main/java/androidx/media/MediaBrowserServiceCompat.java;l=1395

PiperOrigin-RevId: 551210137
(cherry picked from commit 17ee527)
@bubenheimer
Copy link

At the time of media3 1.0 release, the release announcement said that android-media-controller app would be updated within weeks for media3 support. Is that still on the radar? I really would like to poke at my controller pretty hard one of these days, including the media3-specific parts.

https://android-developers.googleblog.com/2023/03/media3-is-ready-to-play.html#:~:text=our%20testing%20tool%2C%20the%20Media%20Controller%20Test%20app%2C%20will%20also%20be%20updated%20to%20Media3%20on%20their%20main%20branches%20in%20the%20coming%20weeks

@tonihei
Copy link
Collaborator

tonihei commented Jun 10, 2024

@bubenheimer Thanks for the reminder! That's still on our radar and planned for the upcoming 1.4.0 release, sorry we didn't get around to publish the test app earlier.

copybara-service bot pushed a commit that referenced this issue Jun 11, 2024
@icbaker
Publish the media3 session controller test app
f54991e 
Issue: #78
PiperOrigin-RevId: 642277900
@icbaker
Copy link
Collaborator

icbaker commented Jun 11, 2024

@bubenheimer The app is now published in f54991e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@marcbaechinger marcbaechinger

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@loki666 @marcbaechinger @bubenheimer @icbaker @iTaysonLab @tonihei