music-assistant / hass-music-assistant

Turn your Home Assistant instance into a jukebox, hassle free streaming of your favorite media to Home Assistant media players.
Apache License 2.0
1.3k stars 48 forks source link

junk after document element #1064

Closed HuismAndre closed 1 year ago

HuismAndre commented 1 year ago

What version of Music Assistant has the issue?

2022.12.1

The problem

I have the following playlist: [quote]

EXTM3U

/media/Various Artists/Back To the 80's Volume 3/410 Michael Jackson - One Day In Your Life.mp3 /media/Various Artists/Back To the 80's Volume 3/411 Tom Tom Club - Wordy Rappinghood.mp3 /media/Various Artists/Back To the 80's Volume 3/412 Earth, Wind & Fire - Let's Groove.mp3 [/quote] This playlist fails to load. When I remove this Michael Jackson track from the m3u list, the list works. I also checked the id tags on the file in question (using Tag&Rename) and found no garbage.

How to reproduce

I guess you'll have to have my mp3 file for this.

Relevant log output

2023-01-11 21:57:05.553 ERROR (MainThread) [music_assistant.music.file] Error processing /media/Various Artists/Back To the 80's Volume 3/410 Michael Jackson - One Day In Your Life.mp3 - junk after document element: line 25, column 0 Traceback (most recent call last): File "/usr/local/lib/python3.10/site-packages/music_assistant/music_providers/filesystem/filesystem.py", line 253, in sync_library track = await self._parse_track(entry.path) File "/usr/local/lib/python3.10/site-packages/music_assistant/music_providers/filesystem/filesystem.py", line 549, in _parse_track artist = await self._parse_artist(track_artist_str) File "/usr/local/lib/python3.10/site-packages/music_assistant/music_providers/filesystem/filesystem.py", line 679, in _parse_artist info = await self.mass.loop.run_in_executor(None, xmltodict.parse, data) File "/usr/local/lib/python3.10/concurrent/futures/thread.py", line 58, in run result = self.fn(*self.args, **self.kwargs) File "/usr/local/lib/python3.10/site-packages/xmltodict.py", line 378, in parse parser.Parse(xml_input, True) xml.parsers.expat.ExpatError: junk after document element: line 25, column 0

Additional information

There are more tracks in my music database with this issue. The originals are in my possession and ripped by me to mp3 years ago (can't even remember which program I used back then). Foobar has no problem with my music database. Neither does VLC. But those can't run on my NAS that's why I was over joyed when I saw your great program.

BTW: It looks like your program uses the ID3v1 tag instead of the newer ID3v2 tag.

What version of Home Assistant Core are your running

2023.1.1

What type of installation are you running?

Home Assistant Container

On what type of hardware are you running?

Linux

OzGav commented 1 year ago

We recommend tagging with MusicBrainz Picard. Can you try that?

HuismAndre commented 1 year ago

OK, will see if that helps. Could take a while (being an ex DJ my collection is a bit above average).

OzGav commented 1 year ago

Just try that one file and see if that fixes it.

marcelveldt commented 1 year ago

There seems to be something within the tags. It would be great if you can save that file somewhere save to send it to me some day so I can try to reproduce and maybe fix it in the code.

HuismAndre commented 1 year ago

@marcelveldt Thanks. I checked the file with Foobar, mp3val and MusicBrainz and neither of them found anything wrong with the file. You can download the file from the following location: https://djdj.nl/xtra/410%20Michael%20Jackson%20-%20One%20Day%20In%20Your%20Life.mp3

BTW: With a short playlist, the playlist fails. With a longer playlist the playlist ends with the last correct song. Processing the playlist (500 items) takes some 20-30 minutes. I find that strange since the songs themselves are already indexed (unless that's screwed up too by the 'junk' that some songs seem to have).

Edit: I'm a bit further. It seems this track has a tracknumber of 154 when looking at ID3v1 and 410 when looking at Id3v2. Now in now way can I fix this 154. I've tried with all the tools I could find (including the mentioned MusicBrainz). I could however change the 410 to 154 and when I do that the track appears in the playlist. Why look at ID3v1 in 2023? Isn't that something that's dead since 2000 or something?

HuismAndre commented 1 year ago

Is there any update on this?

OzGav commented 1 year ago

Please be patient (in the order of a few weeks) while Marcel does the rewrite.

OzGav commented 1 year ago

MA V2 Beta is available. Please advise if this is still an issue

HuismAndre commented 1 year ago

I run HA in a docker container so it seems I can't run your add-on version.

OzGav commented 1 year ago

There is a docker version https://github.com/music-assistant/hass-music-assistant/issues/1143

OzGav commented 1 year ago

Please advise if this is still an issue.

HuismAndre commented 1 year ago

Tried to install the docker version on my DS220+ NAS but wasn't succesful. I'll now wait for your program to get out of beta, Maybe then the howto (docker) will also be a tad more useable.

OzGav commented 1 year ago

Others are using docker just fine. If you can be more specific about what wasn’t successful we might be able to help.

HuismAndre commented 1 year ago

So last night I was finally able to install and run Music Assistant 2.0 in a docker container with my music attached. The problem hasn't changed. The log is still filled with "xml.parsers.expat.ExpatError: junk after document element" errors. Playlists still fail because of this (only items up to the first error get loaded). Tracks with this error still don't end up in the database (foobar2000 etc have no problem with it). No known (to me) mp3 tag editor (including those suggested in this thread) finds anything wrong with the track in question.

I too now consider this matter closed. MA has moved away from HA integration and for me, that was the biggest plus of the program. Too bad.

marcelveldt commented 1 year ago

@HuismAndre I think you have misinterpret something because we're not moving away from HA integration at all, it just is not ready yet, I am working hard to get that part ready again. We just have moved the core of Music Assistant into its own process for stability and maintenance reasons that's all. Of course we're not going to drop HA support, that is the core reason of the existence of MA ;-)

I found the link to your file above, I will review it and report back. I have never seen this error before so very curious what is causing it.

marcelveldt commented 1 year ago

Quickly ran the file through our tag scanner,it found no issues but then I re-read your initial post and it occured to me where it is failing. It is failing on reading the actual artist folder on disk, more specifically the "artist.nfo" file with additional metadata. That is what is crashing. I will add an additional guard in our code to not crash completely when reading that file fails but at least now you know that the artist.nfo file is causing the malfunction.

So in this case you will have an artist folder called Michael Jackson and in there will be some additional images and a file "artist.nfo"

HuismAndre commented 1 year ago

Thanks for looking into it. Yes, I have a music database with complete artist info in nfo files (created by Kodi years ago, using the "export music database" option). I just checked the file from Michael Jackson and it's bad, it's real bad (and no, that's not the song). It contains entries of other artists as wel. No idea how Kodi could have made such a mess of it. I'll take the easy way out and rename the nfo files for now (and maybe someday will look into it some more). Scanner is running as we speak. Let's hope this solves my issue.

BTW: Good to hear you don't move away from Home Assistant. ;-)

marcelveldt commented 1 year ago

Ah, great to hear that we found the culprit, strange that the file was so malformed. Well at least it exposed a bug on the MA side too: that we should ignmore malformed nfo files :-)

And no my dayjob is actually Home Assistant (I'm a Nabu Casa employee) so it would be really strange if MA would not be a killing extension to HA ;-)