smithdtyler / prettygoodmusicplayer

A music player app for Android hits the basics hard
GNU General Public License v3.0
51 stars 23 forks source link

Delay in detecting unplugged headphones #66

Closed deveee closed 9 years ago

deveee commented 9 years ago

I have enabled property which detects if my headphones are unplugged during playing music. In this case player state is changed to "paused", as expected. But I can observe 2-3 seconds delay before state is changed. During this time music is playing with phone's speaker. Ideally it shouldn't be played at all when headphones are unplugged.

smithdtyler commented 9 years ago

This is actually intentional - I was frustrated with my phone pausing immediately when it sensed a disconnect, so I built in a "grace period" which allowed the phone to keep playing even if the cable was bumped and the phone briefly thought it was disconnected.

Sounds like you'd like this behavior to be optional?

deveee commented 9 years ago

In other music players I saw "resume when headphones inserted" function which should solve it. At least it shouldn't be that frustrating.

"pause when headphones removed" feature is useful for example:

In both cases people will hear my hard death metal sounds before player is paused. Okay, I don't listen to death metal :D But problem exists.

deveee commented 9 years ago

Quick patch:

diff --git a/src/com/smithdtyler/prettygoodmusicplayer/MusicBroadcastReceiver.java b/src/com/smithdtyler/prettygoodmusicplayer/MusicBroadcastReceiver.java
index 7b5a31e..bea13d9 100644
--- a/src/com/smithdtyler/prettygoodmusicplayer/MusicBroadcastReceiver.java
+++ b/src/com/smithdtyler/prettygoodmusicplayer/MusicBroadcastReceiver.java
@@ -42,14 +42,14 @@ public class MusicBroadcastReceiver extends BroadcastReceiver {
            if (intent.getIntExtra("state", -1) == 0) {
                Log.i(TAG, "headphones disconnected, pausing in 1 seconds");
                Intent msgIntent = new Intent(context, MusicPlaybackService.class);
-               msgIntent.putExtra("Message", MusicPlaybackService.MSG_PAUSE_IN_ONE_SEC);
+               msgIntent.putExtra("Message", MusicPlaybackService.MSG_FORCE_PAUSE);
                context.startService(msgIntent);
                // If the headphone is plugged back in quickly after being
                // unplugged, keep playing
            } else if (intent.getIntExtra("state", -1) == 1) {
                Log.i(TAG, "headphones plugged back in, cancelling disconnect");
                Intent msgIntent = new Intent(context, MusicPlaybackService.class);
-               msgIntent.putExtra("Message", MusicPlaybackService.MSG_CANCEL_PAUSE_IN_ONE_SEC);
+               msgIntent.putExtra("Message", MusicPlaybackService.MSG_CANCEL_FORCE_PAUSE);
                context.startService(msgIntent);
            }
        } else if (BluetoothDevice.ACTION_ACL_DISCONNECT_REQUESTED
@@ -143,4 +143,4 @@ public class MusicBroadcastReceiver extends BroadcastReceiver {
        }

    }
-}
\ No newline at end of file
+}
diff --git a/src/com/smithdtyler/prettygoodmusicplayer/MusicPlaybackService.java b/src/com/smithdtyler/prettygoodmusicplayer/MusicPlaybackService.java
index d9a9734..e202f03 100644
--- a/src/com/smithdtyler/prettygoodmusicplayer/MusicPlaybackService.java
+++ b/src/com/smithdtyler/prettygoodmusicplayer/MusicPlaybackService.java
@@ -64,8 +64,8 @@ public class MusicPlaybackService extends Service {
    static final int MSG_PREVIOUS = 5;
    static final int MSG_SET_PLAYLIST = 6;
    static final int MSG_PAUSE = 7;
-   static final int MSG_PAUSE_IN_ONE_SEC = 8;
-   static final int MSG_CANCEL_PAUSE_IN_ONE_SEC = 9;
+   static final int MSG_FORCE_PAUSE = 8;
+   static final int MSG_CANCEL_FORCE_PAUSE = 9;
    static final int MSG_TOGGLE_SHUFFLE = 10;
    static final int MSG_SEEK_TO = 11;

@@ -131,7 +131,7 @@ public class MusicPlaybackService extends Service {
    private int lastDuration = 0;
    private int lastPosition = 0;
    public long audioFocusLossTime = 0;
-   private long pauseTime = Long.MAX_VALUE;
+   private boolean forcePause = false;
    private boolean _shuffle = false;
    private List<Integer> shuffleFrontList = new ArrayList<Integer>();
    private Random random;
@@ -265,10 +265,16 @@ public class MusicPlaybackService extends Service {
                timer.cancel();
                stopForeground(true);
                stopSelf();
-           } else if (command == MSG_PAUSE_IN_ONE_SEC) {
-               pauseTime = System.currentTimeMillis() + 1000;
-           } else if (command == MSG_CANCEL_PAUSE_IN_ONE_SEC) {
-               pauseTime = Long.MAX_VALUE;
+           } else if (command == MSG_FORCE_PAUSE) {
+               if (mp.isPlaying()) {
+                   forcePause = true;
+                   pause();
+               }
+           } else if (command == MSG_CANCEL_FORCE_PAUSE) {
+               if (forcePause == true) {
+                   forcePause = false;
+                   play();
+               }
            }
            return START_STICKY;
        }
@@ -360,9 +366,6 @@ public class MusicPlaybackService extends Service {

    private void onTimerTick() {
        long currentTime = System.currentTimeMillis();
-       if (pauseTime < currentTime) {
-           pause();
-       }
        updateResumePosition();
        sendUpdateToClients();
    }
@@ -537,9 +540,9 @@ public class MusicPlaybackService extends Service {
        if (mp.isPlaying()) {
            pause();
        } else {
-           pauseTime = Long.MAX_VALUE;
            play();
        }
+       forcePause = false;
    }

We still sometimes can hear music after unplugging headphones for a while. This is probably caused by checking state once per a piece of time (1 second?). But this is enough for me.

Now player also automatically plays music when headphones are plugged in again.

smithdtyler commented 9 years ago

Thanks for putting this together, I think it's a pretty good solution. I'll merge it in and try it out.

smithdtyler commented 9 years ago

I think ultimately I'd like to make the behavior configurable, e.g.

On headphone disconnect:

  1. Pause immediately
  2. Pause after 1 sec grace period
  3. Pause, but resume on re-insertion within one second
smithdtyler commented 9 years ago

Brainstorming implementation options: As a menu option:

"Headphone Disconnect Behavior"

deveee commented 9 years ago

Maybe also add an option to resume on reconnect without time limit? This is what I did in my patch. When player was paused by unplugging phones, it can be resumed by plugging it again. In most players I see two separated properties:

The second option isn't necessary, but some peopel may use it.

Btw. your player is quite good :) Did you write it from scratch or is it based on code from other players?

smithdtyler commented 9 years ago

Sure, I can add that option.

Thanks! I mostly wrote it from scratch. I used a couple of open source projects as reference examples:

Media Button Router

Android-File-Explore

Thanks for the feedback! It's fun to work on a project like this with engaged and helpful users :)

smithdtyler commented 9 years ago

Nominal implementation is in this branch

I haven't tested it yet. I will close this ticket once I've successfully tested all of the modes and merged the fix into master.

smithdtyler commented 9 years ago

Tested and merged into master.

I occasionally still had an issue with a split second of music playing after the headphones are unplugged, but I don't think there's anything I can do about it - I'm sending a "pause" as soon as I get an "unplugged" notification.

deveee commented 9 years ago

Thanks a lot for adding this feature! I will test current version today.

Atm. I am rather busy because we are before Supertuxkart release, but I can try to find few minutes to make Polish translations :) Are you interested in it?

smithdtyler commented 9 years ago

Absolutely, that would be great! On Feb 3, 2015 2:16 AM, "Deve" notifications@github.com wrote:

Thanks a lot for adding this feature! I will test current version today.

Atm. I am rather busy because we are before Supertuxkart release, but I can try to find few minutes to make Polish translations :) Are you interested in it?

— Reply to this email directly or view it on GitHub https://github.com/smithdtyler/prettygoodmusicplayer/issues/66#issuecomment-72610710 .