androidx / media

Jetpack Media3 support libraries for media use cases, including ExoPlayer, an extensible media player for Android
https://developer.android.com/media/media3
Apache License 2.0
1.6k stars 380 forks source link

MediaSession.Callback onMediaButtonEvent receives wrong intent extras when service started #1527

Open Digipom opened 3 months ago

Digipom commented 3 months ago

Version

Media3 1.3.1

More version details

androidx.media3:media3-session:1.3.1

Devices that reproduce the issue

Pixel 7

Devices that do not reproduce the issue

No response

Reproducible in the demo app?

Not tested

Reproduction steps

Tap Play from a Bluetooth device while the app process is ended.

Expected result

Two button events should match.

Actual result

First button event:

controller package: com.example.myapp android.intent.extra.PACKAGE_NAME=com.google.android.bluetooth

Second button event:

controller package: com.google.android.bluetooth android.intent.extra.PACKAGE_NAME=not set

Media

N/A

Bug Report

oceanjules commented 3 months ago

Could you expand on the reproduction steps a bit? In the title, you say "when the service is started", but later you say "when the app process is ended".

Digipom commented 3 months ago

Here's when I was observing it in our app: 1) Start the app and observe the service also starts. 2) Swipe the app away. Observe that the process ends, but then is restarted again with a KEYCODE_MEDIA_STOP. 3) Open the app, and now swipe it away again. This time the process will end and not start again. 4) Press Play on the Bluetooth device. This will cause the process to start again (since it was the last one with a media session).

When the process starts, the service will also start and will receive a button event with these parameters:

controller package: com.example.myapp android.intent.extra.PACKAGE_NAME=com.google.android.bluetooth

If the service was already running, then pressing play on a Bluetooth device results in these parameters, instead:

controller package: com.google.android.bluetooth android.intent.extra.PACKAGE_NAME=not set

So the two don't match, which trips up detection logic that is trying to figure out where media button events are coming from.

More details:

**First** Bluetooth play event:

MediaSession callback - onMediaButtonEvent: Intent {
  act=android.intent.action.MEDIA_BUTTON
  flg=0x10000010
  cmp=com.example.myapp/.MyService (has extras)
},
controller package: com.example.myapp,
extras: {
  android.intent.extra.PACKAGE_NAME=com.google.android.bluetooth,
  android.intent.extra.KEY_EVENT=KeyEvent {
    action=ACTION_DOWN,
    keyCode=KEYCODE_MEDIA_PLAY,
    scanCode=0,
    metaState=0,
    flags=0x0,
    repeatCount=0,
    eventTime=0,
    downTime=0,
    deviceId=-1,
    source=0x0,
    displayId=-1
  }
}
**Second** Bluetooth play event:

MediaSession callback - onMediaButtonEvent: Intent {
  act=android.intent.action.MEDIA_BUTTON (has extras)
},
controller package: com.google.android.bluetooth,
extras: {
  android.intent.extra.KEY_EVENT=KeyEvent {
    action=ACTION_DOWN,
    keyCode=KEYCODE_MEDIA_PLAY,
    scanCode=0,
    metaState=0,
    flags=0x0,
    repeatCount=0,
    eventTime=0,
    downTime=0,
    deviceId=-1,
    source=0x0,
    displayId=-1
  }
}
marcbaechinger commented 3 months ago

Thanks for reporting and the detailed information! Interesting problem. :)

From what I understand you've added the MediaButtonReceiver to your manifest to enable playback resumption.

The first call to MediaSession.Callback.onMediaButtonEvent is originated through the MediaButtonReceiver. Bluetooth is sending the key event to the system, the system is figuring out which media button receiver is on top of the stack and delivers it accordingly. It sends an intent to the MediaButtonReceiver that arrives in onReceive of the receiver. From your output you can see that the first intent has the component set to cmp=com.example.myapp/.MyService which we do here in the receiver. We add that, but besides this we delegate the Intent instance that we receive from the system. The service is then started and that Intent is delivered to MediaSessionService.onStartCommand. There we recognize the media action and use the internal media notification controller to call onMediaButtonEvent which is where you get the call and you have dumped the output from above I think. The media notification controller has the package name of your app as shown above.

The second call (aka the second key event sent from BT) arrives at the system when the session is already available and active. In such a case, the system doesn't send it through the receiver, but create a platform controller with the BT package name , connects against the session and calls onMediaButtonEvent which arrives in MediaSessionLegacyStub where we use the unchanged controller info to call onMediaButtonEvent.

I think the above explains the output your seeing, and I agree this isn't consistent and makes recognizing difficult.

Thinking about how to make this consistent, I guess routing through the ControllerInfo unchanged in the second case feels to be the right thing to do in comparison to invent something there on the library side. Hence, a change for making this consistent should be on the other code site, which is onStartCommand.

While I don't think we can make things completely consistent, it seems feasible to fake the ControllerInfo there instead of using the media controller info. From your valuable post I learned that we can find the sender package in the extras of the intent with key Intent.EXTRA_PACKAGE_NAME. So in theory we could do something like this and call your Callback.onMediaButtonEvent(ControllerInfo, Intent) with something like this:

ControllerInfo callerInfo;
if (intent.getExtras().containsKey(Intent.EXTRA_PACKAGE_NAME)) {
  callerInfo =
      new ControllerInfo(
          new MediaSessionManager.RemoteUserInfo(
              /* packageName= */ intent
                  .getExtras()
                  .getString(Intent.EXTRA_PACKAGE_NAME),
              /* pid= */ 0,
              /* uid= */ 0),
          MediaLibraryInfo.VERSION_INT,
          MediaControllerStub.VERSION_INT,
          false,
          null,
          Bundle.EMPTY);
} else {
  callerInfo = sessionImpl.getMediaNotificationControllerInfo();
}
sessionImpl.onMediaButtonEvent(callerInfo, intent);

Like I said, that still not consistent because we can't know the pid and the uid of the BT process. Comparing what you get in the first and the second call would still not be the same. It would have the same package name indeed.

Further, the package name we read in that extras Bundle could come from any malious user, and Media3 would help such a user with inpersonating BT that way. So you can probably understand, that this may not be a viable solution for us.

The same applies for being consisten in the extras with the package name that's keyed with Intent.EXTRAS_PACKAGE_NAME.

We'll discuss this internally based on this analysis and will update this issue if we have news. Thanks again for your report.

Sorry for the long post. The context info may be useful for responding to the other two issues you filed which seems to be around starting the service with media button receiver as well.


As an aside and for my education: What other clients apart from Bluetooth do you expect to send a media button event to your app? Do you accept random media button events broadcasted from other apps to you? Apart from these, I can think of the remote control on Android TV. Since API 26 (I think), key events are always delivered to the session directly (except the first one), not through the receiver so since then there is nothing else to expect there. Covering these API levels would include knowing the BT package name which changed at some point (before 26 I think) and on API 21 the package name is always android.media.session.MediaController when a platform controller connects. Can you educate me how you recognize BT for these cases?

Digipom commented 3 months ago

As an aside and for my education: What other clients apart from Bluetooth do you expect to send a media button event to your app? Do you accept random media button events broadcasted from other apps to you? Apart from these, I can think of the remote control on Android TV. Since API 26 (I think), key events are always delivered to the session directly (except the first one), not through the receiver so since then there is nothing else to expect there. Covering these API levels would include knowing the BT package name which changed at some point (before 26 I think) and on API 21 the package name is always android.media.session.MediaController when a platform controller connects. Can you educate me how you recognize BT for these cases?

Thank you for the detailed follow-up on your part. I'll try to clarify: a frequent complaint that we get from users is that the app starts automatically when their phone connects to their car, because the car is sending an auto-play event to the phone. Ideally, the car itself should not be doing this but not every car supports configuring that. At the same time, we don't want to exclude users from being able to intentionally control the app via Bluetooth.

The solution we arrived at for now is to add a setting to the app that allows toggling this behavior, and we called it "Background playback" / "Start playback via accessories, including headsets or car Bluetooth"

When the app is in the foreground (checked using ProcessLifecycleOwner), we allow all events regardless of that setting. When the app is in the background, and that setting is turned off (thus disallowing background playback), then we check the following:

1) is "allow_start_play_in_background" false? 2) Is the controller package name different than our package? Or, is the controller package the same, but the intent contains Intent.EXTRA_PACKAGE_NAME and that doesn't match our package? (this accounts for onStartCommand as you described it above). On Android versions 13+, notification events appear to go straight to the player, but on Android 12 and below, they are routed through onMediaButtonEvent so that's why we still need to check the controller info. 3) Is the key code KEYCODE_MEDIA_PLAY? 4) Is the ProcessLifecycleOwner lifecycle below "Lifecycle.State.STARTED"?

If all of the above is true, then we suppress the event by returning true from onMediaButtonEvent. Unfortunately, by default, that also causes a "ForegroundServiceDidNotStartInTimeException ". https://github.com/androidx/media/issues/1528. For now I suppress that by calling startForeground with a fake notification, then immediately calling stopForeground, but this is a hack I'd rather not have to do.

The min. SDK we deploy to is 26, so we don't need to worry about any concerns that apply before then.