Antennapod: Preference to fast-forward or skip on hardware button not respected with long-press of volume key

[e2jk]

PR #1424 has added the ability for users to indicate if they want their hardware forward button to skip instead of fast forwarding.

Regardless of the state of this preference, AP always skips to the next episode when I long-press the volume up button on my phone when the screen is off.
It used to be that when long-pressing the volume up, the episode would be forwarded X number of seconds (that time is definable in the preferences). I use[d to use] this extensively to forward ads when driving my car, it's easy as you can keep your eyes on the road and just need one finger to long-press for 2 seconds. Now I can't, as doing so manually requires me to unlock the screen, look at my phone, go to AP (if it wasn't the last app I used), click on the currently playing section at the bottom of the screen, and click on the fast forward button a couple of times. All way too dangerous while driving...

Anyway, looking at the code, I see the following on line 1259 of core/service/playback/PlaybackServiceMediaPlayer.java:

                case KeyEvent.KEYCODE_MEDIA_NEXT: {
                    if(event.getSource() == InputDevice.SOURCE_CLASS_NONE ||
                            UserPreferences.shouldHardwareButtonSkip()) {
                        // assume the skip command comes from a notification or the lockscreen
                        // a >| skip button should actually skip
                        endPlayback(true);
                    } else {
                        // assume skip command comes from a (bluetooth) media button
                        // user actually wants to fast-forward
                        seekDelta(UserPreferences.getFastFowardSecs() * 1000);
                    }
                    return true;
                }

I assume UserPreferences.shouldHardwareButtonSkip() returns the value of the checkbox in the preferences, but as it is preceded by a OR on event.getSource() == InputDevice.SOURCE_CLASS_NONE, it looks like on my device long-pressing the volume up had an event source of SOURCE_CLASS_NONE and the code thus always goes to endPlayback(true) instead of seekDelta(UserPreferences.getFastFowardSecs() * 1000).

I've noticed that the PR and the pref_hardwareForwardButtonSkips_sum string mentions hardware buttons, but the comment on line 1261 mentions the command coming from "a notification or the lockscreen" - maybe a residue from a previous commit? IN any case, I've also tried to use the >| button that appears on the lockscreen, but regardless of this checkbox the player always skips to the next track.

Now I'm not sure what the first part in the if statement is supposed to do, but (unless I haven't understood the feature correctly) on my phone it's not working as I imagine it should. The description of this preference reads:

When pressing a hardware forward button skip to the next track instead of fast-forwarding

The way I'm understanding this is that when the checkbox is not set, long-pressing my _hardware_ volume-up button should fast-forward.

Looking at different bug reports (e.g. #1407) I understand there was some rough discussions around skipping vs. fast-forwarding, with two camps each preferring one of the behaviours. It looks to me that this setting was introduced to appease this, but it looks like it's not yet completely working as expected?

Again, I'm not really sure what the first condition in this if is supposed to do, but maybe it could be removed so that the code would do (pseudo-code):

    when pressing KeyEvent.KEYCODE_MEDIA_NEXT:
        if UserPreferences.shouldHardwareButtonSkip()
            skip to next episode
        else
            fast forward

AP version 1.4.2.2
Samsung GT-I9195 (Galaxy S4 Mini)
Android 5.1.1 (Cyangenmod 12.1)

[mfietz]

Whoa, wall of text.
Maybe this clears things up: we expect event.getSource() == InputDevice.SOURCE_CLASS_NONE to only be true if the KeyEvent comes from a notification.
Don't think we can fix this, there doesn't seem to be another way to differentiate notification from device.

[e2jk]

Wall indeed ;)
Why is it needed to check if the event comes from a notification?
Wouldn't it be possible to use the flow I described in the pseudo-code at the end? When an event comes with that keycode, skip if the user wants to skip else fast-forward, without having to check if it comes from a notification?

[mfietz]

Why is it needed to check if the event comes from a notification?

Most bluetooth devices cannot send "fast-forward", only "next". So people want us to pretend that next means fast-forward. So we only interpret KEYCODE_MEDIA_NEXT as next if we think it came from a notification or it the setting "forward button skips" is enabled

[e2jk]

So if it's a notification (event.getSource() == InputDevice.SOURCE_CLASS_NONE) the code will skip _regardless_ of whether the setting "forward button skips" is enabled or not. Is that the expected behaviour?
Is there a way to get back to the original behaviour, which was to fast-forward a number of seconds when long-pressing the volume up?

At this moment the behaviour is not consistent: if you long-press the volume down, you go 30 seconds back (for example to re-listen to a bit you weren't paying attention), but if you long-press the volume up, you don't go 30 seconds forward (for example to skip ads) but you go to the next episode.
The previous behaviour used to be consistent.

[mfietz]

the code will skip regardless of whether the setting "forward button skips" is enabled or not. Is that the expected behaviour?

Yes. The setting only matters for bluetooth devices. But your bluetooth device clearly is a notification, so it does not apply.

Is there a way to get back to the original behaviour, which was to fast-forward a number of seconds when long-pressing the volume up?

Hm, tell your notification it is a bluetooth device? Joke aside... fork the code? Find another way we can distinguish bluetooth device from notification.

At this moment the behaviour is not consistent

It is consistent from the "this is a notification" point of view. Volume up means "the next button on the notiication was pressed", not fast-forward (how we would interpret it if your device wasn't a notification in disguise).

[e2jk]

The setting only matters for bluetooth devices.

OK, that wasn't clear to me by reading the setting's description. I understand now that by "hardware" it is meant "bluetooth". I don't own a bluetooth device, I'm only operating the hardware volume buttons on the side of my phone.

My point about not being consistent comes looking from the user experience side, and based on how the app has been behaving since Daniel started developing it: long-press on volume down goes back, long-press on volume up goes forward. The behavior of this button has changed: from 2012 until somewhere around the summer/fall of 2015, a long press had the fast-forward behavior, while now it skips.

I understand that some people want to be able to skip the episode (as explained by @TomHennen in issue #732 in April 2015) and I'm completely fine with it, unless it breaks the behaviour that folks have been used to since the app was started.

What would break if the "this is a notification" check was removed? i.e. remove event.getSource() == InputDevice.SOURCE_CLASS_NONE || so that the code becomes:

                case KeyEvent.KEYCODE_MEDIA_NEXT: {
                    if(UserPreferences.shouldHardwareButtonSkip()) {
                        // The user wants to skip
                        endPlayback(true);
                    } else {
                        // The user wants to fast-forward
                        seekDelta(UserPreferences.getFastFowardSecs() * 1000);
                    }
                    return true;
                }

With this code, everything behaves the same way, regardless if it's a notification or if the device is Bluetooth the preference set by the user is respected:

• Skip if the preference is enable
• Fast forward if the preference is not set

Are there any drawbacks you can think of, or can I submit a pull request with this change?

In the case that there would be any drawbacks, is there a way to detect that the event comes wither from a Bluetooth device or from a volume up long-press? Then the code could become something like this (separate else if clauses to make it more readable here):

when pressing KeyEvent.KEYCODE_MEDIA_NEXT:
    if IsBluetooth and UserPreferences.shouldHardwareButtonSkip()
        skip to next episode
    else if IsVolumeUpLongPress and UserPreferences.shouldHardwareButtonSkip()
        skip to next episode
    else if isNotification
        skip to next episode
    else
        // We get here if it's not a notification (i.e. Bluetooth or Long Press on volume up key)
        // and the user has shouldHardwareButtonSkip set to False
        fast forward

[TomHennen]

The notification control has both FF and Skip. If we weren't able to tell if it came from the notification then they'd both just FF. I don't think we actually do anything to detect if it's a volume up long press. I suspect this is something your handset or ROM is doing for you. I don't believe it's built in to AntennaPod. (For example, long press on volume doesn't do anything for me except change the volume, Nexus 6, Android 6.0)

[e2jk]

So it's likely a CyanogenMod-specific feature that sends the KEYCODE_MEDIA_NEXT (and not KEYCODE_MEDIA_FAST_FORWARD apparently) event when the volume up key is long-pressed. This action used to have the effect of fast-forwarding, but with the recent changes to support KEYCODE_MEDIA_NEXT specifically to skip, that effect has changed.

Again on a technical level I agree it is the right event to skip (i.e. >| skips and >> forwards), but it's unfortunate that I was used to the previous behavior. I'm not trying to say that skipping doesn't make sense, as I do sometimes encounter episodes that are uninteresting and skip them manually.

Maybe the solution is to reach out to the CM folks and see if it would be possible to put an option to send KEYCODE_MEDIA_FAST_FORWARD instead of KEYCODE_MEDIA_NEXT on a long press. I'll try to see how one would go about reaching out to CM (I have low expectations about just posting a random message in the forums). I think for now I'll just go with my own fork and remove the skip behavior, but I'm really not looking forward to having to maintain a fork over the long term, just too much hassle.

I would still suggest you clarify the preference's message to specify this is a Bluetooth-only parameter. When I saw this issue appear, I had good hopes this would fix my issues with long-pressing the hardware volume buttons, alas ;)

[e2jk]

FWIW: On my phone (running CyanogenMod version 12.1) in Settings > Buttons there is this option, which (translated from French to English) clearly says that it _skips_ songs (the options "Contrôle de la musique"):

When the screen is turned off, a long press on the volume buttons allows to change the track that is being played

[mfietz]

@TomHennen wontfix? (Would not even know how to)

[e2jk]

Let me investigate the coming weekend, if anything is passed in the event that could identify reliably that this is coming from a long-press on the hardware volume button, which could be use in the if statement the same way it's done for Bluetooth devices.
This used to work perfectly well, until the changes this fall for skip vs. fast forward ;)

[mfietz]

Blame those people that want us to pretend their stupid bluetooth devices send a different media button signal ;)

[e2jk]

Well, yes and no ;)

Technically yes: the buttons are sending the Skip signal, so it's correct to skip to the next track.

But from a usability standpoint, due to the original implementation that fast-forwarded the current podcast, users have been accustomed to the FF instead of the Skip behaviour. With the implementation in the fall of the differentiation between FF and Skip, all the users that were used to Fast Forward got the rug pulled under their feet.

I don't think these discussions would have been as prominent if the first implementation had been to skip, but then again (and I know it's not everybody's standpoint on this issue, which I respect) I think it is more frequent to fast-forward (ad skipping scenario, for instance) rather than to skip the odd episode that is boring. Hence the reason why Daniel originally implemented the Fast Forward behaviour instead of the skip.

But let's not start this debate again. I'll see if there is a way to differentiate the events, otherwise we'll just close this bug and I'll accept losing this feature that I'm using for my security while driving for my commute.

[BenBergman]

I am also affected by this issue (OnePlus One running Cyanogen OS 12.0).

Thanks for digging into this @e2jk . If you do end up maintaining a fork, please post a link here.

[blkeller]

Here's my "me too" report on this bug: OnePlus One running Cyanogen OS 11S. I really, really miss this feature.

If there's a good way to differentiate between handset button events and headset button events, then of course that's an ideal solution. If it turns out that isn't possible, though, then I'd like to advocate for a new checkbox in the preferences that would allow users to optionally restore the old button behavior, and which would be disabled by default. Perhaps the preference could even be worded in such a way that users would know that checking the box would mean breaking the expected behavior of their bluetooth headset buttons.

In summary, if it's not possible to automatically determine proper button behavior across different devices and accessories, then please consider allowing the user to manually select his or her preference, with the new button behavior as the default.

Thanks to you all for your efforts on this bug.

[BenBergman]

Seconding the checkbox option.

[e2jk]

I have still not been able to start hacking around the code, and don't think I'll be able to do so in the coming months (starting businesses and family, you know).
Such a checkbox would be completely fine with me. I know it represents yet another option, but this feature is so useful (as I tried to convey in my wall of text above). Unfortunately, I think the main devs (@TomHennen and @mfietz) are not running CyanogenMod, and thus weren't aware of this "extra" feature that we were enjoying and making our commutes safer before this got broken to support the behavior that >> skips to the next track instead of advancing the playback, as had been the case for the last years.
(re-reading myself, this could come over as a criticism towards the devs, but I assure you it's not what I mean. Just trying to indicate why I think we users of this "feature" are so passionate about it, while for the devs this seems to be just a non-issue ;) )

[mfietz]

new checkbox in the preferences

The big problem is: what would it actually say? Every skip/next track would become a fast-forward, that includes notifications. I think this is more than weird...

In my opinion, you have the following options:

1. Tell CM to fix it. They can either replace skip with actual fast-forward or they could set the source class, e.g. SOURCE_CLASS_BUTTON
2. Fork and compile your own version of AP
3. Acceptance.

I won't implement something I cannot test - and I don't run CM at the moment.

[e2jk]

How about this: modify the checkbox that now points at the hardware button (which means Bluetooth devices) to be generic: "Skip instead of Fast-Forward", and put this option to True by default.
That would be a further implementation of PR #1424 which allowed [only] Bluetooth devices to FF instead of Skip.

The following would be achieved:

• Users that depended on the original behavior of the button/functionality to FF will have an ability to get back to that behavior
• The new behavior implemented last fall to Skip will still be the default

The only people noticing any change would be folks that have a Bluetooth device that do use the software button to skip (making a bold assumption: probably a minority of people). Instead of being able to skip to the next episode directly, they would need to press the Play button from the episode list. That is just one extra click, or none if you are already looking at the list of episodes.

To summarize, there would be 2 behaviors:

• [default: True] >> button (either from the notification or from hardware) skips, << goes back 30 seconds
• with option set to False, >> button fast-forwards 30 seconds, << goes back 30 seconds

Implementing it this way, you are able to test it yourself and are not dependent on a CyanogenMod feature.

The change is likely easy:

• Rename UserPreferences.shouldHardwareButtonSkip -> UserPreferences.shouldButtonSkip
• Modify the string to replace the reference to "hardware" button with "notification or hardware" button
• Replace line 1259 of core/service/playback/PlaybackServiceMediaPlayer.java:
  if(event.getSource() == InputDevice.SOURCE_CLASS_NONE || UserPreferences.shouldHardwareButtonSkip()) {-> if !UserPreferences.shouldButtonSkip()) {

What do you think about this?

It's too bad that I didn't take the time to report this issue when it happened, as the Bluetooth folks were faster they were able to get their change implemented. But if you look at the root of the issue it's the same change that caused both the Bluetooth and this issue: both were used to the historical fact that the >> behavior was to skip 30 sec ahead (just as << goes back 30 seconds, which is luckily still working fine).

[yzerman19]

@e2jk I don't have a OnePlus nor any device with CM, so this volume button thing has never been in my radar. I have a question- have you tried other podcast apps? How do they handle this?
I know for the Bluetooth button scenario, I have used other podcast apps and they all jump forward/back 30s (or specified time). That is part of the reason for this app I prefer that functionality over skipping episodes.

[e2jk]

I've created PR #1722 that modifies the current Bluetooth preference to make it unconditional. Default behavior: Skip to next file, but if you set this to False you can fast-Forward regardless of if you're doing it from a Bluetooth hardware button, CyanogenMod's Volume Up long-press, or any other >> button.

I've created a test APK for those that would like to try this: https://files.klein.st/AntennaPod-debug-issue1560.apk

I'm interested in your feedback, please let me know if I need to change anything. Thanks for your time on this issue.

[e2jk]

@yzerman19 I haven't used another podcast player than AntennaPod since somewhere at the end of 2012, so can't tell.

I can understand that the Bluetooth headsets (or the CM devs with the Volume Up long-press) would send the "Next" signal, since most smartphone users on the planet are listening to music instead of podcasts (yep, still rather niche, just ask any random person on the Metro what a podcast is ;) ): for music, it makes more sense to skip to the next soundtrack ("I don't want to listen to this song just now") rather than fast-forward (what? "I don't like this part of the lyrics" or "this bass segment is boring" ?).

But for a podcast, I see different reasons why fast-forward can make more sense than skipping (yes, I am fully aware not everybody agrees on this point, and that's why in my PR I made skipping the default as that is how it's currently working):

• Skipping ads (e.g. Leo Laporte with his 5 minutes ads on Security Now in the middle of episodes)
• Skipping lengthy introductions/ads (e.g. Tim Ferriss' podcasts usually begins with actual content at around 2m30s)
• Going to the next topics on podcasts that don't use chapters (e.g. segments on the Linux Voice or Security Now podcasts that don't interest me)

[eddyp]

I am seeing a similar issue on the always-on-screen on my Samsung Galaxy A5 2017. The buttons on the always-on-screen are rewind-to-start+ skip-to-next instead of skip-back + ff , although I have configured the buttons correctly. Please note that after I exit the always-on-screen, I see the notification screen which has the right buttons.

Is this caused by the same issue or is it something specific to the always-on-screen feature?

[eddyp]

It doesn't work for me on a Bluetooth device where it used to work.

[LatinSuD]

Just got a Pixel XL with Android Oreo 8.1.0.

All my bluetooth headsets now skip instead of fast-forwarding, regardless of what i have configured.

I tried compiling current git version and it works. I'll stick with it until official version catches up.

[eddyp]

The same happens for me as for @LatinSuD. And it also is a problem for me since I regularly listen to podcasts while driving

[tomchiverton]

On Android 7.x it worked fine (long press skips forward).
Since OTA update to 8.1 its broken.

V1.6.5 from FDroid

[eddyp]

I noticed recently that it is working again on my Samsung Galaxy A5 2017 with Android 8.0.0 and AntennaPod Version: 1.7.1
Commit: d26d21260

[LatinSuD]

It works in 1.7.2b (although it stopped working for a few minutes today, not sure what happened).

[ByteHamster]

I don't think this should be an issue anymore because we no longer check event.getSource() to determine if the event comes from the notification.

[Slinger]

@ByteHamster I'm still seeing this issue on my device. I'm not sure it can be fixed, because onStartCommand() seems to get called when holding volume up ("Received media button event").

But I've suggested a simple workaround for the "Forward Button Skips" setting in #4075. Originally I opened it as a bug report but I realize that was wrong. I'm not sure if there is any point in reopening this bug, but if you want I'd be happy to help. I can try ideas and (small) changes in the code. But I don't have a bluetooth controller so I can't promise I can check everything.

edit: sorry I meant onStartCommand() not handleKeycode.

[Slinger]

@ByteHamster thank you so much for reopening this issue. I think I've found something useful!

I found no way to distinguish the two situations from onStartCommand(), but I looked at the intent sent to MediaButtonReceiver:

When holding hardware volume up button:

D/MediaButtonReceiver( 2985): android.intent.extra.KEY_EVENT : KeyEvent { action=ACTION_DOWN, keyCode=KEYCODE_MEDIA_NEXT, scanCode=0, metaState=0, flags=0x0, repeatCount=0, eventTime=1484017, downTime=1484017, deviceId=-1, source=0x0 }

And when pressing next on the lock screen:

D/MediaButtonReceiver( 2985): android.intent.extra.KEY_EVENT : KeyEvent { action=ACTION_DOWN, keyCode=KEYCODE_MEDIA_NEXT, scanCode=0, metaState=0, flags=0x0, repeatCount=0, eventTime=0, downTime=0, deviceId=-1, source=0x0 }

So I think it's safe to say that if eventTime or downTime is zero the key press is purely artificial. I hope the solution is as easy as I think it is. :-)

(((For full clarity, I added the following code to onReceive() in core/src/main/java/de/danoeh/antennapod/core/receiver/MediaButtonReceiver.java:

        Bundle bundle = intent.getExtras();
        if (bundle != null) {
                    for (String key : bundle.keySet()) {
                                    Log.d(TAG, key + " : " + (bundle.get(key) != null ? bundle.get(key) : "NULL"));
                                        }
        }

)))

[Slinger]

I'm not sure how quickly this will be looked at (I understand resources are always limited). So if I get some time I might look into trying to change this.

Essentially my idea is:

1. Check if eventTime and downTime are both > 0 in onReceive(), if so add another "extra" to the intent to indicate that it's a physical button (probably a boolean? but mere existence of it should be enough...)
2. Check for the "extra" in onStartCommand() and set notificationButton when calling handleKeycod() appropriately (instead of always set to "true").

Does this seem ok?

[Slinger]

Ok, I had some time to try my idea and it does seem to work perfectly. I've made a pull request #4256

It would be great if someone with a bluetooth music controller could test that it doesn't cause any unwanted changes. From what I understand bluetooth skip was already interpreted as jumping forward instead of skipping, so it probably isn't a problem.