Repository: beetbox/beets

Issue:
mpdstats records a skip when stopping playback
<!--
Describe your problem, feature request, or discussion topic here.

Please fill out this and the "Setup" section below and remember to include
enough detail so that other people can reproduce the problem.
-->

### Problem

I am using the mpdstats plugin.  Running `beet mpdstats` and then starting a song playing in mpd and then stopping it makes mpdstats record a skip and adjust the song rating downwards, even though that song remains at the beginning of the mpd playlist (i.e. it's not actually skipped).  Interestingly pausing and then later stopping seems to not cause this problem.

### Setup

* OS: Arch Linux
* Python version: 3.8.5
* beets version: b89a2650c
* Turning off plugins made problem go away (yes/no): Well... yes?

My configuration (output of `beet config`) is:

```yaml
plugins: parentwork scrub info mbsync alternatives chroma mpdstats
directory: ~/music/beets/

paths:
    default: '%ifdef{parent_composer_sort,$parent_composer_sort,[unknown]}/%ifdef{parentwork_date,$parentwork_date - }%ifdef{parentwork,$parentwork,[unknown]}/%ifdef{year,$year - }$album%aunique{album year,albumartist albumtype label catalognum albumdisambig releasegroupdisambig}/$track $title'

import:
    from_scratch: yes
    move: yes
acoustid:
    apikey: REDACTED
alternatives:
    aphone:
        directory: /home/user/music/aphone/
        formats: opus
        query: ''
parentwork:
    auto: no
    force: no
chroma:
    auto: yes
mpd:
    music_directory: /home/user/music/beets
    rating: yes
    rating_mix: 0.75
    host: localhost
    port: 6600
    password: REDACTED
scrub:
    auto: yes
```


Discussion:
sampsyo: Seems pretty good to me! Would you mind adding a changelog entry describing the change?

Patch:
diff --git a/beetsplug/mpdstats.py b/beetsplug/mpdstats.py
index f232d87e9..39b045f9b 100644
--- a/beetsplug/mpdstats.py
+++ b/beetsplug/mpdstats.py
@@ -108,8 +108,9 @@ class MPDClientWrapper(object):
         return self.get(command, retries=retries - 1)
 
     def currentsong(self):
-        """Return the path to the currently playing song.  Prefixes paths with the
-        music_directory, to get the absolute path.
+        """Return the path to the currently playing song, along with its
+        songid.  Prefixes paths with the music_directory, to get the absolute
+        path.
         """
         result = None
         entry = self.get('currentsong')
@@ -118,7 +119,7 @@ class MPDClientWrapper(object):
                 result = os.path.join(self.music_directory, entry['file'])
             else:
                 result = entry['file']
-        return result
+        return result, entry.get('id')
 
     def status(self):
         """Return the current status of the MPD.
@@ -240,7 +241,9 @@ class MPDStats(object):
     def on_stop(self, status):
         self._log.info(u'stop')
 
-        if self.now_playing:
+        # if the current song stays the same it means that we stopped on the
+        # current track and should not record a skip.
+        if self.now_playing and self.now_playing['id'] != status.get('songid'):
             self.handle_song_change(self.now_playing)
 
         self.now_playing = None
@@ -251,7 +254,7 @@ class MPDStats(object):
 
     def on_play(self, status):
 
-        path = self.mpd.currentsong()
+        path, songid = self.mpd.currentsong()
 
         if not path:
             return
@@ -286,6 +289,7 @@ class MPDStats(object):
             'started':    time.time(),
             'remaining':  remaining,
             'path':       path,
+            'id':         songid,
             'beets_item': self.get_item(path),
         }
 
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 0dfa2c5f8..47159ddc2 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -245,6 +245,10 @@ Fixes:
 * Fix a bug that caused metadata starting with something resembling a drive
   letter to be incorrectly split into an extra directory after the colon.
   :bug:`3685`
+* :doc:`/plugins/mpdstats`: Don't record a skip when stopping MPD, as MPD keeps
+  the current track in the queue.
+  Thanks to :user:`aereaux`.
+  :bug:`3722`
 
 For plugin developers:
 


Test patch:
diff --git a/test/test_mpdstats.py b/test/test_mpdstats.py
index 0117e22aa..20226927f 100644
--- a/test/test_mpdstats.py
+++ b/test/test_mpdstats.py
@@ -62,10 +62,11 @@ class MPDStatsTest(unittest.TestCase, TestHelper):
                 {'state': u'stop'}]
     EVENTS = [["player"]] * (len(STATUSES) - 1) + [KeyboardInterrupt]
     item_path = util.normpath('/foo/bar.flac')
+    songid = 1
 
     @patch("beetsplug.mpdstats.MPDClientWrapper", return_value=Mock(**{
         "events.side_effect": EVENTS, "status.side_effect": STATUSES,
-        "currentsong.return_value": item_path}))
+        "currentsong.return_value": (item_path, songid)}))
     def test_run_mpdstats(self, mpd_mock):
         item = Item(title=u'title', path=self.item_path, id=1)
         item.add(self.lib)