-
Notifications
You must be signed in to change notification settings - Fork 361
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
COMMAND_SET_DEVICE_VOLUME_WITH_FLAGS and COMMAND_ADJUST_DEVICE_VOLUME_WITH_FLAGS does not trigger related handlers in SimpleBasePlayer when using Media3 1.1.0 #554
Comments
Hi @ashutoshgngwr, Could you confirm that you are now overriding the handler that has the following signature: media/libraries/common/src/main/java/androidx/media3/common/SimpleBasePlayer.java Lines 3060 to 3062 in 5328d64
You can see that this is the handler that is triggered in both the new and deprecated methods in lines 2725 and 2738: media/libraries/common/src/main/java/androidx/media3/common/SimpleBasePlayer.java Lines 2716 to 2740 in 5328d64
|
@oceanjules Hi there! Yes, I did implement the updated handlers (with flags). Moreover, the updated handlers appear to be working with deprecated commands but not with their replacements. Let me try to reproduce this in a minimal setting. |
@oceanjules Here's a minimal Android project demonstrating this issue. If you uncomment these lines, you see log entries for the handlers in question on running the app and pressing volume rockers on the device. But handlers don't get invoked on commenting out the deprecated commands. |
We also have a test that should check this behaviour (that also currently works) which you can use as a template for your minimal example: media/libraries/common/src/test/java/androidx/media3/common/SimpleBasePlayerTest.java Lines 3183 to 3218 in 5328d64
|
Couldn't that imply that the problem might be somewhere else?
Why? |
Having tried your minimal example, I can only see this through logcat:
which comes from the MediaSessionRecord's "canHandleVolumeKey". I guess because isPlaybackTypeLocal() is false. Setting the device info type to Local, gives us:
So I think all these key events are not going through Exoplayer at all, which is why I am not sure how you managed to get those log statement in the handlers working with the provided minimal setup. |
I am getting the following logs when running my example on an Android 11 (actual device) with the remote device type.
If I replace
I am not getting any entries with the |
Hi @ashutoshgngwr, Thank you for providing the Android version, that was the key piece of information that helped me reproduce it. I have found the issue and we will be patching up a bugfix for it. For reference, these if-statements, should have had the new commands as well. Otherwise, the volumeControlType was set as FIXED, i.e. not changeable media/libraries/session/src/main/java/androidx/media3/session/PlayerWrapper.java Lines 1032 to 1037 in 5328d64
Also for reference, the reason it didn't work for me was because I was testing with Android 12 and 13 and it's a known issue that the hardware buttons and volume of a cast device are currently not working: https://issuetracker.google.com/issues/201546605 |
When hardware buttons are used to control the volume of the remote device, the call propagates to `MediaSessionCompat.setPlaybackToRemote(volumeProviderCompat)`. However, `volumeProviderCompat` was created incorrectly when the new device volume commands were present (COMMAND_SET_DEVICE_VOLUME_WITH_FLAGS and COMMAND_ADJUST_DEVICE_VOLUME_WITH_FLAGS), i.e. with volumeControlType = `VOLUME_CONTROL_FIXED`. This resulted in `VolumeProviderCompat` which doesn't call `onSetVolumeTo` or `onAdjustVolume` and hence doesn't propagate the calls to the `Player`. Instead, it only worked with the deprecated commands which ensured the volumeControlType was `VOLUME_CONTROL_ABSOLUTE`. This bug was introduced in c71e4bf (1.0 media 3 release) when `PlayerWrapper`'s call to `createVolumeProviderCompat` was mostly rewritten to handle the new commands, but the two if-statements were not amended. Note: this change fixes the bug only for Android 11 and below. For 12 and above, there is a tracking bug for the regression that was introduced: https://issuetracker.google.com/issues/201546605 http://Issue: #554 #minor-release PiperOrigin-RevId: 554966361
When hardware buttons are used to control the volume of the remote device, the call propagates to `MediaSessionCompat.setPlaybackToRemote(volumeProviderCompat)`. However, `volumeProviderCompat` was created incorrectly when the new device volume commands were present (COMMAND_SET_DEVICE_VOLUME_WITH_FLAGS and COMMAND_ADJUST_DEVICE_VOLUME_WITH_FLAGS), i.e. with volumeControlType = `VOLUME_CONTROL_FIXED`. This resulted in `VolumeProviderCompat` which doesn't call `onSetVolumeTo` or `onAdjustVolume` and hence doesn't propagate the calls to the `Player`. Instead, it only worked with the deprecated commands which ensured the volumeControlType was `VOLUME_CONTROL_ABSOLUTE`. This bug was introduced in c71e4bf (1.0 media 3 release) when `PlayerWrapper`'s call to `createVolumeProviderCompat` was mostly rewritten to handle the new commands, but the two if-statements were not amended. Note: this change fixes the bug only for Android 11 and below. For 12 and above, there is a tracking bug for the regression that was introduced: https://issuetracker.google.com/issues/201546605 http://Issue: #554 PiperOrigin-RevId: 554966361 (cherry picked from commit dedccc5)
When hardware buttons are used to control the volume of the remote device, the call propagates to `MediaSessionCompat.setPlaybackToRemote(volumeProviderCompat)`. However, `volumeProviderCompat` was created incorrectly when the new device volume commands were present (COMMAND_SET_DEVICE_VOLUME_WITH_FLAGS and COMMAND_ADJUST_DEVICE_VOLUME_WITH_FLAGS), i.e. with volumeControlType = `VOLUME_CONTROL_FIXED`. This resulted in `VolumeProviderCompat` which doesn't call `onSetVolumeTo` or `onAdjustVolume` and hence doesn't propagate the calls to the `Player`. Instead, it only worked with the deprecated commands which ensured the volumeControlType was `VOLUME_CONTROL_ABSOLUTE`. This bug was introduced in androidx@c71e4bf (1.0 media 3 release) when `PlayerWrapper`'s call to `createVolumeProviderCompat` was mostly rewritten to handle the new commands, but the two if-statements were not amended. Note: this change fixes the bug only for Android 11 and below. For 12 and above, there is a tracking bug for the regression that was introduced: https://issuetracker.google.com/issues/201546605 http://Issue: androidx#554 #minor-release PiperOrigin-RevId: 554966361
With Media3 1.0.0, we could handle cast device volume by implementing
COMMAND_GET_DEVICE_VOLUME
,COMMAND_SET_DEVICE_VOLUME
andCOMMAND_ADJUST_DEVICE_VOLUME
commands in SimpleBasePlayer. We used cast API to get/set device volume. For implementation, see this and this. Here,RemoteDeviceVolumeProvider
is a simple abstraction that deals with the cast API. As a result, we could adjust the cast device volume using volume rockers during an active cast session.With Media3 1.1.0, the latter two commands are now deprecated in favour of
COMMAND_SET_DEVICE_VOLUME_WITH_FLAGS
andCOMMAND_ADJUST_DEVICE_VOLUME_WITH_FLAGS
. However, when we replaced the deprecated commands with the new ones in our existing implementation, theSimpleBasePlayer
stopped invokinghandleSetDeviceVolume
and other related handlers. As a result, nothing happens on pressing volume rockers during a cast playback session.But if we continue using the deprecated commands, everything works as expected.
Is it a bug or an intended change? More importantly, how do we continue to use volume rockers to adjust cast device volume when the cast session is active while incorporating the latest changes?
The text was updated successfully, but these errors were encountered: