Skip to content

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

Closed
Closed
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
Assignees
oceanjules
Labels
bugquestion

Description

@ashutoshgngwr
ashutoshgngwr
opened on Aug 1, 2023

With Media3 1.0.0, we could handle cast device volume by implementing COMMAND_GET_DEVICE_VOLUME, COMMAND_SET_DEVICE_VOLUME and COMMAND_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 and COMMAND_ADJUST_DEVICE_VOLUME_WITH_FLAGS. However, when we replaced the deprecated commands with the new ones in our existing implementation, the SimpleBasePlayer stopped invoking handleSetDeviceVolume 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?

Activity

  1. ashutoshgngwr
    added
    needs triage
    question
    on Aug 1, 2023

  2. oceanjules commented on Aug 1, 2023

    @oceanjules
    oceanjules
    on Aug 1, 2023
    Contributor

    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

    @ForOverride
    protected ListenableFuture<?> handleSetDeviceVolume(
    @IntRange(from = 0) int deviceVolume, int flags) {

    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

    @Override
    public final void setDeviceVolume(int volume) {
    verifyApplicationThreadAndInitState();
    // Use a local copy to ensure the lambda below uses the current state value.
    State state = this.state;
    if (!shouldHandleCommand(Player.COMMAND_SET_DEVICE_VOLUME)) {
    return;
    }
    updateStateForPendingOperation(
    /* pendingOperation= */ handleSetDeviceVolume(volume, C.VOLUME_FLAG_SHOW_UI),
    /* placeholderStateSupplier= */ () -> state.buildUpon().setDeviceVolume(volume).build());
    }
    @Override
    public final void setDeviceVolume(int volume, @C.VolumeFlags int flags) {
    verifyApplicationThreadAndInitState();
    // Use a local copy to ensure the lambda below uses the current state value.
    State state = this.state;
    if (!shouldHandleCommand(Player.COMMAND_SET_DEVICE_VOLUME_WITH_FLAGS)) {
    return;
    }
    updateStateForPendingOperation(
    /* pendingOperation= */ handleSetDeviceVolume(volume, flags),
    /* placeholderStateSupplier= */ () -> state.buildUpon().setDeviceVolume(volume).build());
    }

  3. oceanjules
    self-assigned this
    on Aug 1, 2023

  4. ashutoshgngwr commented on Aug 1, 2023

    @ashutoshgngwr
    ashutoshgngwr
    on Aug 1, 2023
    Author

    @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.

  5. ashutoshgngwr commented on Aug 1, 2023

    @ashutoshgngwr
    ashutoshgngwr
    on Aug 1, 2023
    Author

    @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.

  6. oceanjules commented on Aug 1, 2023

    @oceanjules
    oceanjules
    on Aug 1, 2023
    Contributor

    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

    @Test
    public void setDeviceVolume_immediateHandling_updatesStateAndInformsListeners() {
    State state =
    new State.Builder()
    .setAvailableCommands(new Commands.Builder().addAllCommands().build())
    .build();
    // Set a different one to the one requested to ensure the updated state is used.
    State updatedState = state.buildUpon().setDeviceVolume(6).build();
    AtomicInteger flagsFromHandlerRef = new AtomicInteger();
    int volumeFlags = C.VOLUME_FLAG_SHOW_UI | C.VOLUME_FLAG_VIBRATE;
    SimpleBasePlayer player =
    new SimpleBasePlayer(Looper.myLooper()) {
    private State playerState = state;
    @Override
    protected State getState() {
    return playerState;
    }
    @Override
    protected ListenableFuture<?> handleSetDeviceVolume(int volume, int flags) {
    playerState = updatedState;
    flagsFromHandlerRef.set(flags);
    return Futures.immediateVoidFuture();
    }
    };
    Listener listener = mock(Listener.class);
    player.addListener(listener);
    player.setDeviceVolume(3, volumeFlags);
    assertThat(player.getDeviceVolume()).isEqualTo(6);
    verify(listener).onDeviceVolumeChanged(6, /* muted= */ false);
    assertThat(flagsFromHandlerRef.get()).isEqualTo(volumeFlags);
    verifyNoMoreInteractions(listener);
    }

  7. ashutoshgngwr commented on Aug 1, 2023

    @ashutoshgngwr
    ashutoshgngwr
    on Aug 1, 2023
    Author

    We also have a test that should check this behaviour (that also currently works)

    Couldn't that imply that the problem might be somewhere else?

    which you can use as a template for your minimal example

    Why?

  8. oceanjules commented on Aug 6, 2023

    @oceanjules
    oceanjules
    on Aug 6, 2023
    Contributor

    Having tried your minimal example, I can only see this through logcat:

    MediaSessionRecord      system_server     D  No routing session for com.example.myapplication
MediaSessionService     system_server     D  Adjusting suggestedStream=-2147483648 by 0. flags=4116, preferSuggestedStream=false, session=null

    which comes from the MediaSessionRecord's "canHandleVolumeKey". I guess because isPlaybackTypeLocal() is false.

    Setting the device info type to Local, gives us:

    MediaSessionService     system_server D  dispatchVolumeKeyEvent, pkg=com.example.myapplication, opPkg=com.example.myapplication, pid=27424, uid=10316, asSystem=true, event=KeyEvent { action=ACTION_DOWN, keyCode=KEYCODE_VOLUME_DOWN, scanCode=114, metaState=0, flags=0x8, repeatCount=0, eventTime=434166456228000, downTime=434166456228000, deviceId=2, source=0x101, displayId=-1 }, stream=-2147483648, musicOnly=false
MediaSessionService     system_server D  Adjusting com.example.myapplication/androidx.media3.session.id.Playback (userId=0) by -1. flags=4113, suggestedStream=-2147483648, preferSuggestedStream=false

    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.

  9. ashutoshgngwr commented on Aug 7, 2023

    @ashutoshgngwr
    ashutoshgngwr
    on Aug 7, 2023
    Author

    I am getting the following logs when running my example on an Android 11 (actual device) with the remote device type.

    MediaSessionService     dispatchVolumeKeyEvent, pkg=com.example.myapplication, opPkg=com.example.myapplication, pid=13269, uid=10377, asSystem=true, event=KeyEvent { action=ACTION_DOWN, keyCode=KEYCODE_VOLUME_DOWN, scanCode=114, metaState=0, flags=0x8, repeatCount=0, eventTime=43458071, downTime=43458071, deviceId=5, source=0x101, displayId=-1 }, stream=-2147483648, musicOnly=false
MediaSessionService     Adjusting com.example.myapplication/androidx.media3.session.id.Playback (userId=0) by -1. flags=4113, suggestedStream=-2147483648, preferSuggestedStream=false
MediaSessionService     dispatchVolumeKeyEvent, pkg=com.example.myapplication, opPkg=com.example.myapplication, pid=13269, uid=10377, asSystem=true, event=KeyEvent { action=ACTION_UP, keyCode=KEYCODE_VOLUME_DOWN, scanCode=114, metaState=0, flags=0x8, repeatCount=0, eventTime=43458200, downTime=43458071, deviceId=5, source=0x101, displayId=-1 }, stream=-2147483648, musicOnly=false
MediaSessionService     Adjusting com.example.myapplication/androidx.media3.session.id.Playback (userId=0) by 0. flags=4116, suggestedStream=-2147483648, preferSuggestedStream=false

    If I replace *_WITH_FLAGS commands with their deprecated counterparts, I am getting the following.

    MediaSessionService     dispatchVolumeKeyEvent, pkg=com.example.myapplication, opPkg=com.example.myapplication, pid=13986, uid=10377, asSystem=true, event=KeyEvent { action=ACTION_DOWN, keyCode=KEYCODE_VOLUME_UP, scanCode=115, metaState=0, flags=0x8, repeatCount=0, eventTime=43798183, downTime=43798183, deviceId=2, source=0x101, displayId=-1 }, stream=-2147483648, musicOnly=false
MediaSessionService     Adjusting com.example.myapplication/androidx.media3.session.id.Playback (userId=0) by 1. flags=4113, suggestedStream=-2147483648, preferSuggestedStream=false
MediaSessionPlayer      handleIncreaseDeviceVolume: flags=1
MediaSessionService     dispatchVolumeKeyEvent, pkg=com.example.myapplication, opPkg=com.example.myapplication, pid=13986, uid=10377, asSystem=true, event=KeyEvent { action=ACTION_UP, keyCode=KEYCODE_VOLUME_UP, scanCode=115, metaState=0, flags=0x8, repeatCount=0, eventTime=43798360, downTime=43798183, deviceId=2, source=0x101, displayId=-1 }, stream=-2147483648, musicOnly=false
MediaSessionService     Adjusting com.example.myapplication/androidx.media3.session.id.Playback (userId=0) by 0. flags=4116, suggestedStream=-2147483648, preferSuggestedStream=false
MediaSessionService     dispatchVolumeKeyEvent, pkg=com.example.myapplication, opPkg=com.example.myapplication, pid=13986, uid=10377, asSystem=true, event=KeyEvent { action=ACTION_DOWN, keyCode=KEYCODE_VOLUME_DOWN, scanCode=114, metaState=0, flags=0x8, repeatCount=0, eventTime=43798951, downTime=43798951, deviceId=5, source=0x101, displayId=-1 }, stream=-2147483648, musicOnly=false
MediaSessionService     Adjusting com.example.myapplication/androidx.media3.session.id.Playback (userId=0) by -1. flags=4113, suggestedStream=-2147483648, preferSuggestedStream=false
MediaSessionPlayer      handleDecreaseDeviceVolume: flags=1
MediaSessionService     dispatchVolumeKeyEvent, pkg=com.example.myapplication, opPkg=com.example.myapplication, pid=13986, uid=10377, asSystem=true, event=KeyEvent { action=ACTION_UP, keyCode=KEYCODE_VOLUME_DOWN, scanCode=114, metaState=0, flags=0x8, repeatCount=0, eventTime=43799132, downTime=43798951, deviceId=5, source=0x101, displayId=-1 }, stream=-2147483648, musicOnly=false
MediaSessionService     Adjusting com.example.myapplication/androidx.media3.session.id.Playback (userId=0) by 0. flags=4116, suggestedStream=-2147483648, preferSuggestedStream=false
MediaSessionService     dispatchVolumeKeyEvent, pkg=com.example.myapplication, opPkg=com.example.myapplication, pid=13986, uid=10377, asSystem=true, event=KeyEvent { action=ACTION_DOWN, keyCode=KEYCODE_VOLUME_UP, scanCode=115, metaState=0, flags=0x8, repeatCount=0, eventTime=43799771, downTime=43799771, deviceId=2, source=0x101, displayId=-1 }, stream=-2147483648, musicOnly=false
MediaSessionService     Adjusting com.example.myapplication/androidx.media3.session.id.Playback (userId=0) by 1. flags=4113, suggestedStream=-2147483648, preferSuggestedStream=false
MediaSessionPlayer      handleIncreaseDeviceVolume: flags=1
MediaSessionService     dispatchVolumeKeyEvent, pkg=com.example.myapplication, opPkg=com.example.myapplication, pid=13986, uid=10377, asSystem=true, event=KeyEvent { action=ACTION_UP, keyCode=KEYCODE_VOLUME_UP, scanCode=115, metaState=0, flags=0x8, repeatCount=0, eventTime=43799950, downTime=43799771, deviceId=2, source=0x101, displayId=-1 }, stream=-2147483648, musicOnly=false
MediaSessionService     Adjusting com.example.myapplication/androidx.media3.session.id.Playback (userId=0) by 0. flags=4116, suggestedStream=-2147483648, preferSuggestedStream=false

    I am not getting any entries with the MediaSessionRecord tag. Now I am not aware of how the internals work in the framework, but I think this should add some clarity.

  10. oceanjules commented on Aug 7, 2023

    @oceanjules
    oceanjules
    on Aug 7, 2023
    Contributor

    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

    Commands availableCommands = getAvailableCommands();
    int volumeControlType = VolumeProviderCompat.VOLUME_CONTROL_FIXED;
    if (availableCommands.contains(COMMAND_ADJUST_DEVICE_VOLUME)) {
    volumeControlType = VolumeProviderCompat.VOLUME_CONTROL_RELATIVE;
    if (availableCommands.contains(COMMAND_SET_DEVICE_VOLUME)) {
    volumeControlType = VolumeProviderCompat.VOLUME_CONTROL_ABSOLUTE;

    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

  11. oceanjules
    added
    bug
    and removed
    needs triage
    on Aug 7, 2023
  12. tianyif
    added 2 commits that reference this issue on Aug 10, 2023

    Fix `PlayerWrapper`'s creation of `VolumeProviderCompat`

    dedccc5

    Fix `PlayerWrapper`'s creation of `VolumeProviderCompat`

    136f087
  13. microkatz
    added a commit that references this issue on Sep 29, 2023

    Fix `PlayerWrapper`'s creation of `VolumeProviderCompat`

    a2f51bb
  14. oceanjules
    closed this as completedon Nov 3, 2023
  15. androidx
    locked and limited conversation to collaborators on Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugquestion

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions