media icon indicating copy to clipboard operation
media copied to clipboard

KEYCODE_HEADSETHOOK and double click behaviour

Open DavidEdwardsBL opened this issue 1 year ago • 1 comments

Version

Media3 main branch

More version details

No response

Devices that reproduce the issue

Devices that have a headset attached to them and send KEYCODE_HEADSETHOOK when clicking on the headsets primary button.

Concrete error device: OPPO CPH1951.

Devices that do not reproduce the issue

No response

Reproducible in the demo app?

Not tested

Reproduction steps

  1. Play media
  2. Double click on headset primary button (usually on single button headsets it is play / pause)

On all devices, the issue can be reproduced by sending the keycode for KEYCODE_HEADSETHOOK twice (replicating a double tap) over ADB.

We took a look at the code, and can see why there is an issue. At this stage, we are testing for PLAY_PAUSE and HEADSETHOOK. It then sets a handler to fire to detect a double tap. Once the next tap comes through, the double tap flag is set.

After the flag is set, it will be checked either here or here. Unfortunately, neither cases take KEYCODE_HEADSETHOOK into account. This leads to next media not being set.

Expected result

The system converts to a seek to next.

Actual result

The system makes a play / pause.

Media

Any

Bug Report

  • [ ] You will email the zip file produced by adb bugreport to [email protected] after filing this issue.

DavidEdwardsBL avatar Jun 24 '24 09:06 DavidEdwardsBL

Thanks for reporting - it looks like HEADSETHOOK is handled for double tap by media1: https://cs.android.com/android/platform/superproject/+/androidx-main:frameworks/support/media/media/src/main/java/android/support/v4/media/session/MediaSessionCompat.java;l=1147-1150;drc=1991cf1992f8ec2331848b8f0b6c26a433da5f54

I also noticed our code seems to consider KEYCODE_MEDIA_PLAY for the second of a double tap (in one of the branches), but not the first: https://github.com/androidx/media/blob/be2d68c2b373384529cc6b359d658d022942ec71/libraries/session/src/main/java/androidx/media3/session/MediaSessionImpl.java#L1252-L1253

I've sent a change to make all 3 places handle all 3 of:

  • HEADSETHOOK
  • PLAY_PAUSE
  • PLAY

icbaker avatar Jun 25 '24 16:06 icbaker

After a bit of back and forth in review, I dropped the handling of PLAY for consistency with what we already document. The change is linked above.

icbaker avatar Jul 10 '24 16:07 icbaker