search

mikebrady / shairport-sync

AirPlay and AirPlay 2 audio player
Other
7.3k stars 574 forks source link

Incorrect metadata (caching?) #972

Closed hifiberry closed 4 years ago

hifiberry commented 4 years ago

When playing songs from the same album, artist name isn't always updated. Sometimes, it just uses the artist name from the previous song

An example is the album "Random access memories" from Daft Punk.

Song 7: Touch (feat. Paul Williams)

         dict entry(
            string "xesam:title"
            variant                string "Touch"
         )
         dict entry(
            string "xesam:album"
            variant                string "Random Access Memories"
         )
         dict entry(
            string "xesam:artist"
            variant                array [
                  string "Daft Punk (feat. Paul Williams)"
               ]
         )

Song 8: Get Lucky (feat. Pharell Williams)

         dict entry(
            string "xesam:title"
            variant                string "Get Lucky"
         )
         dict entry(
            string "xesam:album"
            variant                string "Random Access Memories"
         )
         dict entry(
            string "xesam:artist"
            variant                array [
                  string "Daft Punk (feat. Pharrell Williams)"
               ]
         )

Song 9 Beyond should be just "Daft Punk", but it still display "feat. Pharell Williams":

         dict entry(
            string "xesam:title"
            variant                string "Beyond"
         )
         dict entry(
            string "xesam:album"
            variant                string "Random Access Memories"
         )
         dict entry(
            string "xesam:artist"
            variant                array [
                  string "Daft Punk (feat. Pharrell Williams)"
               ]
         )

Interestingly when playing a song from another album before this, it displays the correct data:

         dict entry(
            string "xesam:title"
            variant                string "Beyond"
         )
         dict entry(
            string "xesam:album"
            variant                string "Random Access Memories"
         )
         dict entry(
            string "xesam:artist"
            variant                array [
                  string "Daft Punk"
               ]
         )

This looks like a (rather strange) caching issue. It looks like artist name won't be changed if the new artist name matches the beginning of the artist name of the previous song.

mikebrady commented 4 years ago

Thanks for the report. Could you let me have the configuration string please? The string that is returned by:

$ shairport-sync -V
hifiberry commented 4 years ago

3.3.3-OpenSSL-Avahi-ALSA-stdout-pipe-metadata-mpris-sysconfdir:/etc

mikebrady commented 4 years ago

Thanks. I'll see if I can reproduce the problem.

hifiberry commented 4 years ago

If you don't have the Daft Punk album, try 2 songs from the one album with 2 songs: first one from "artist feat. artist 2" and second one "artist". I haven't tried it, but my guess would be that this will trigger it.

mikebrady commented 4 years ago

It would be really interesting to see what's coming from the music player itself, and the best way to do that would be to build the Shairport Sync Metadata Reader.

If you could try it, it would be much simpler, for me at least, than trying to guess which part of the metadata was used to hold the artist information – I'd say this depends on whether the album is a compilation or not (I guess it isn't), whether an "album artist" field is used or not and maybe other variables.

Would that be possible? You would not have to recompile Shairport Sync itself, just build the metadata reader and hook it up to the metadata pipe.

hifiberry commented 4 years ago

I don't have the metadata reader on the system and it will be a bit complicated to add it as this is a buildroot-based system. How can I dump the data into a file? shairport-sync is compiled with the metadata option, but I can't find a configuration option to configure a pipe or file where this will be send to. How can I dump these (even unprocessed) to a file?

mikebrady commented 4 years ago

Ah, I see. Let me do a quick check and give you exact instructions. By default, the pipe is /tmp/shairport-sync-metadata.

mikebrady commented 4 years ago

Actually it’s going to take a couple of days to get to this, I’m afraid. Apologies for the delay.

mikebrady commented 4 years ago

Okay, what works for me is to go to the /tmp folder. Delete the pipe shairport-sync-metadata and replace it with an empty file of the same name. Set the permissions of the file to 666. This is something like how it might appear (this is on a Pi Zero):

pi@zeropi:/tmp $ ls -al
total 48
drwxrwxrwt  8 root root  4096 Feb 17 11:44 .
drwxr-xr-x 21 root root  4096 Dec  3  2018 ..
drwxrwxrwt  2 root root  4096 Dec 23 14:52 .font-unix
drwxrwxrwt  2 root root  4096 Dec 23 14:52 .ICE-unix
-rw-rw-rw-  1 root root     0 Feb 17 11:47 shairport-sync-metadata
drwx------  3 root root  4096 Dec 23 14:52 systemd-private-997280b9f74c4f0c9bf80107eec352c2-systemd-timesyncd.service-ZwxDK1
drwxrwxrwt  2 root root  4096 Dec 23 14:52 .Test-unix
drwxrwxrwt  2 root root  4096 Dec 23 14:52 .X11-unix
drwxrwxrwt  2 root root  4096 Dec 23 14:52 .XIM-unix
pi@zeropi:/tmp $

Then [re]start shairport-sync. The metadata will now be written into the file. It may get quite big! It can them be played back into the shairport-sync-metadata-reader on another device.

hifiberry commented 4 years ago

Unfortunately in my setup it didn't create a pipe and it doesn't write to the /tmp/shairport-sync file even if I create it. I even checked the whole file system and can't find the file anywhere (just to be sure that it isn't using a different directory).

However, it is compiles with the "--with-metadata" option: https://github.com/hifiberry/hifiberry-os/blob/master/buildroot/package/hifiberry-shairport/hifiberry-shairport.mk

Are there any other configuration options required to log metadata to this pipe/file?

mikebrady commented 4 years ago

Yes indeed, you should enable it in the configuration file, which is by default at /etc/shairport-sync.conf. This is an example of some settings in the metadata section:

// How to deal with metadata, including artwork
metadata =
{
        enabled = "yes"; // set this to yes to get Shairport Sync to solicit metadata from the source and to pass it on via a pipe
        include_cover_art = "yes"; // set to "yes" to get Shairport Sync to solicit cover art from the source and pass it via the pipe. You must also set "enabled" to "yes".
        pipe_name = "/tmp/shairport-sync-metadata";
//      pipe_timeout = 5000; // wait for this number of milliseconds for a blocked pipe to unblock before giving up
//      socket_address = "226.0.0.1"; // if set to a host name or IP address, UDP packets containing metadata will be sent to this address. May be a multicast address. "socket-port" must be non-zero and "enabled" must be set to yes"
//      socket_port = 5555; // if socket_address is set, the port to send UDP packets to
//      socket_msglength = 65000; // the maximum packet size for any UDP metadata. This will be clipped to be between 500 or 65000. The default is 500.
};
hifiberry commented 4 years ago

Find attached the output to the metadata file for 4 songs from this album.

shairport-sync-metadata.txt

mikebrady commented 4 years ago

Thanks. This is what that metadata looks like when processed by shairport-sync-metadata-reader with spaces added before and after each piece of metadata (bracketed by mdst (MetaDataSTart) and mden (MetaDataENd)):

mike@ubuntu:~/shairport-sync-metadata-reader$ ./shairport-sync-metadata-reader < ~/Downloads/shairport-sync-metadata.txt 
"ssnc" "snua": "iTunes/12.10.4 (Windows; Microsoft Windows 10 x64 Professional Edition (Build 18362); x64) (dt:2)".
"ssnc" "acre": "1067329903".
"ssnc" "daid": "910EE8BD27EA8226".
Client's IP: "192.168.30.107".
"ssnc" "svip": "192.168.30.126".
"ssnc" "pend": "".
"ssnc" "pbeg": "".
"ssnc" "pvol": "-20.00,-41.11,-90.20,-0.95".
"ssnc" "pvol": "-20.00,-41.11,-90.20,-0.95".
"ssnc" "pvol": "-20.00,-41.11,-90.20,-0.95".

"ssnc" "mdst": "2660495789".
Album Name: "Random Access Memories".
Artist: "Daft Punk (feat. Pharrell Williams)".
Comment: "".
Composer: "Guy-Manuel De Homem-Christo/Guy-Manuel De Homem-Christo/Thomas Bangalter".
Genre: "Electronica/Dance".
File kind: "AAC-Audiodatei".
Title: "Lose Yourself to Dance".
Persistent ID: "4d8d35f99ec53eb3".
Sort as: "".
"ssnc" "mden": "2660495789".

"ssnc" "prgr": "2659749357/2660495789/2675356053".
"ssnc" "dapo": "3689".
"ssnc" "pffr": "".
"ssnc" "prsm": "".
"ssnc" "flsr": "2661094541".
"ssnc" "pfls": "".
"ssnc" "pffr": "".

"ssnc" "mdst": "2661094541".
Album Name: "Random Access Memories".
Artist: "Daft Punk (feat. Paul Williams)".
Comment: "".
Composer: "Guy-Manuel De Homem-Christo/Guy-Manuel De Homem-Christo/Thomas Bangalter".
Genre: "Electronica/Dance".
File kind: "AAC-Audiodatei".
Title: "Touch".
Persistent ID: "9dd4774ca256fb2".
Sort as: "".
"ssnc" "mden": "2661094541".

"ssnc" "prgr": "2661079181/2661094541/2683083317".
"ssnc" "prsm": "".
"ssnc" "flsr": "2661523981".
"ssnc" "pfls": "".
"ssnc" "pffr": "".

"ssnc" "mdst": "2661523981".
Album Name: "Random Access Memories".
Artist: "Daft Punk (feat. Pharrell Williams)".
Comment: "".
Composer: "Guy-Manuel De Homem-Christo/Guy-Manuel De Homem-Christo/Thomas Bangalter".
Genre: "Electronica/Dance".
File kind: "AAC-Audiodatei".
Title: "Get Lucky".
Persistent ID: "680b63f053218198".
Sort as: "".
"ssnc" "mden": "2661523981".

"ssnc" "prgr": "2661508621/2661523981/2677809157".
"ssnc" "prsm": "".
"ssnc" "flsr": "2661764749".
"ssnc" "pfls": "".
"ssnc" "pffr": "".

"ssnc" "mdst": "2661764749".
Album Name: "Random Access Memories".
Artist: "Daft Punk".
Comment: "".
Composer: "Guy-Manuel De Homem-Christo/Guy-Manuel De Homem-Christo/Thomas Bangalter".
Genre: "Electronica/Dance".
File kind: "AAC-Audiodatei".
Title: "Beyond".
Persistent ID: "710907d996cf4d6f".
Sort as: "".
"ssnc" "mden": "2661764749".

"ssnc" "prgr": "2661749389/2661764749/2674548973".
"ssnc" "prsm": "".
"ssnc" "flsr": "2661966797".
"ssnc" "pfls": "".
"ssnc" "pffr": "".
"ssnc" "mdst": "2661966797".

Album Name: "Random Access Memories".
Artist: "Daft Punk".
Comment: "".
Composer: "Guy-Manuel De Homem-Christo/Guy-Manuel De Homem-Christo/Thomas Bangalter".
Genre: "Electronica/Dance".
File kind: "AAC-Audiodatei".
Title: "Motherboard".
Persistent ID: "2c6aed4603d477bc".
Sort as: "".
"ssnc" "mden": "2661966797".

"ssnc" "prgr": "2661951437/2661966797/2677018349".
"ssnc" "prsm": "".
mikebrady commented 4 years ago

Let me see if I can simulate that here.

hifiberry commented 4 years ago

The incorrect title is this:

"ssnc" "mden": "2661523981". "ssnc" "prgr": "2661508621/2661523981/2677809157". "ssnc" "prsm": "". "ssnc" "flsr": "2661764749". "ssnc" "pfls": "". "ssnc" "pffr": "". "ssnc" "mdst": "2661764749". Album Name: "Random Access Memories". Artist: "Daft Punk". Comment: "". Composer: "Guy-Manuel De Homem-Christo/Guy-Manuel De Homem-Christo/Thomas Bangalter". Genre: "Electronica/Dance". File kind: "AAC-Audiodatei". Title: "Beyond". Persistent ID:`"710907d996cf4d6f".

Looks like the metadata are correct here, just shown incorrectly in MPRIS

mikebrady commented 4 years ago

Looks like the metadata are correct here, just shown incorrectly in MPRIS

Exactly. Thanks for your sleuthing. I'll see what I can figure out now.

mikebrady commented 4 years ago

However, it is compiles with the "--with-metadata" option: https://github.com/hifiberry/hifiberry-os/blob/master/buildroot/package/hifiberry-shairport/hifiberry-shairport.mk

I see that these build instructions include the configuration option --with-mpris-test-client. That causes a separate executable called shairport-sync-mpris-clientto be built in the same folder as the shairport-sync executable. When you run it, it listens to the Shairport Sync MPRIS interface (on the D-Bus system bus by default) and reports changes. Here is a sample:

mike@ubuntu:~/shairport-sync$ ./shairport-sync-mpris-test-client 
Listening on the D-Bus system bus...
  *** Properties Changed:
      Metadata -> {'mpris:trackid': <objectpath '/org/gnome/ShairportSync/mper_465857810'>, 'xesam:title': <'Anon/Lawson: Veni Emmanuel '>, 'xesam:album': <'Hyperion sampler - November 2019 Vol. 1'>, 'xesam:artist': <['Owain Park: The Gesualdo Six']>, 'xesam:genre': <['Classical']>, 'mpris:length': <int64 209400000>}
 *** Properties Changed:
      PlaybackStatus -> 'Paused'
 *** Properties Changed:
      PlaybackStatus -> 'Playing'
 *** Properties Changed:
      Metadata -> {'mpris:trackid': <objectpath '/org/gnome/ShairportSync/mper_53254254'>, 'xesam:title': <"Leontovych: Carol of the bells, 'Hark! How the bells, sweet silver bells' ">, 'xesam:album': <'Hyperion sampler - November 2019 Vol. 1'>, 'xesam:artist': <["James Vivian: St George's Chapel Choir Windsor"]>, 'xesam:genre': <['Classical']>, 'mpris:length': <int64 80746000>}
 *** Properties Changed:
      PlaybackStatus -> 'Paused'
 *** Properties Changed:
      PlaybackStatus -> 'Playing'
 *** Properties Changed:
      Metadata -> {'mpris:trackid': <objectpath '/org/gnome/ShairportSync/mper_1845494595'>, 'xesam:title': <'Bauldeweyn: Missa Da pacem - Movement 5c: Agnus Dei III '>, 'xesam:album': <'Hyperion sampler - November 2019 Vol. 1'>, 'xesam:artist': <['Peter Phillips: The Tallis Scholars']>, 'xesam:genre': <['Classical']>, 'mpris:length': <int64 175192000>}
 *** Properties Changed:
      PlaybackStatus -> 'Paused'
 *** Properties Changed:
      Metadata -> {'mpris:trackid': <objectpath '/org/gnome/ShairportSync/mper_1845494595'>, 'xesam:title': <'Parsons: In nomine IV a 7 '>, 'xesam:album': <'Hyperion sampler - November 2019 Vol. 1'>, 'xesam:artist': <['Fretwork']>, 'xesam:genre': <['Classical']>, 'mpris:length': <int64 149199000>}
 *** Properties Changed:
      PlaybackStatus -> 'Playing'
 *** Properties Changed:
      Metadata -> {'mpris:trackid': <objectpath '/org/gnome/ShairportSync/mper_2104015886'>, 'xesam:title': <'Parsons: In nomine IV a 7 '>, 'xesam:album': <'Hyperion sampler - November 2019 Vol. 1'>, 'xesam:artist': <['Fretwork']>, 'xesam:genre': <['Classical']>, 'mpris:length': <int64 149199000>}
 *** Properties Changed:
      PlaybackStatus -> 'Paused'
 *** Properties Changed:
      PlaybackStatus -> 'Playing'
      Metadata -> {'mpris:trackid': <objectpath '/org/gnome/ShairportSync/mper_76894302'>, 'xesam:title': <"Purcell: King Arthur, or The British Worthy, Z628 - No 33. Passacaglia: Verse & Chorus: How happy the lover ? For love ev'ry creature ">, 'xesam:album': <'Hyperion sampler - November 2019 Vol. 1'>, 'xesam:artist': <['Carolyn Sampson, Jeremy Budd, Roderick Williams; Paul McCreesh: Gabrieli Players, Gabrieli Consort']>, 'xesam:genre': <['Classical']>, 'mpris:length': <int64 306266000>}
 *** Properties Changed:
      Metadata -> {'mpris:trackid': <objectpath '/org/gnome/ShairportSync/mper_76894302'>, 'xesam:title': <"Sumsion: Magnificat and Nunc dimittis in A - Canticle 2: Nunc dimittis, 'Lord, now lettest thou thy servant depart in peace' ">, 'xesam:album': <'Hyperion sampler - November 2019 Vol. 1'>, 'xesam:artist': <["Andrew Nethsingha: St John's College Choir Cambridge"]>, 'xesam:genre': <['Classical']>, 'mpris:length': <int64 189400000>}
...

Does it happen to be installed on your system? If so, it would offer more evidence as to what is coming out of Shairport Sync's MPRIS interface on the D-Bus.

mikebrady commented 4 years ago

I might have found something. I’ll push an update tonight or tomorrow.

mikebrady commented 4 years ago

Okay, I found a silly bug which I think could account for the behaviour you experienced – it was a length-limited string comparison which didn't check that the overall strings were the same length. I've fixed it and pushed an update out in the development branch, version 3.3.6d9. It would be great if you could test it and let us know if the problem is resolved.

hifiberry commented 4 years ago

Checked out the latest commit from the development branch and built a new version based on this. Looks like it got worse now ;-) I can only get metadata from the first song. After this , it always reports these metadata now.

mikebrady commented 4 years ago

Yikes, that’s not what I expected. Let me check back.

mikebrady commented 4 years ago

Apologies – that was one of those "it's-so-simple-it-doesn't-need-to-be-tested" fixes. There's an updated and tested-for-sanity version on the development branch now.

tuomashamalainen commented 4 years ago

I originally ran into this and inspired the opening of the issue. Testing the new version, I'm happy to report that it appears to be fixed and artist names are updating correctly through MPRIS. Thank you for your swift action on this 🙂

mikebrady commented 4 years ago

Thanks Tuomas. Apologies for the glitch and thanks for reporting the issue.

hifiberry commented 4 years ago

Hi Mike,

I think, we can close this.

Thanks for the help!

mikebrady commented 4 years ago

Great, thanks!