libmpdclient errors with MPD response lines longer than 4KiB

[hindumagic]

ashuffle reports connecting to MPD version 0.22.0

Does ashuffle have a version number? Can't report that.

Raspberry Pi 2B with MPD library on NAS via SMB and accessing through wifi.

Start ashuffle on the commandline:

ian@boombox:~ $ ashuffle
MPD error: Response line too large

The resulting MPD log at verbose level:

Feb 09 16:33 : client: [11] opened from 127.0.0.1:44122
Feb 09 16:33 : client: [11] process command "notcommands"
Feb 09 16:33 : client: [11] command returned 0
Feb 09 16:33 : client: [11] process command "listallinfo"
Feb 09 16:33 : client: [11] command returned 0
Feb 09 16:33 : client: [11] closed

This is supposed to be running with moode audio version 7.0.1.

This bug looks to be the same issue as

[joshkunz]

Thanks for the detailed bug report.

Does ashuffle have a version number? Can't report that.

ashuffle does have a version number, but it's not easily reportable from the CLI. I've filed issue #90 to add it. IIUC, moOde 7.0.1 is using a fairly recent version of ashuffle (I believe v3), so I suspect this exists at HEAD. To verify you can try downloading an ashuffle binary from the latest release page: https://github.com/joshkunz/ashuffle/releases/tag/v3.10.0

The resulting MPD log at verbose level: ...

So it looks like MPD successfully processed the listallinfo command, but ashuffle did not successfully receive all output. The error being printed by ashuffle comes from libmpdclient, the library we use to communicate with MPD. libmpdclient has a fixed 4KiB buffer that is used to read lines sent by MPD. It looks like one of the response lines received in the listallinfo command was longer than 4KiB. There is an old PR open on libmpdclient to fix this, but it looks like it hasn't been updated in almost 9mo.

To be honest, a line longer than 4KiB sounds a little far-fetched to me, so I'm curious if you could get the raw output of listallinfo for me. You should be able to capture it with a command like:

$ echo 'listallinfo' | nc -q 30 $MPD_HOST $MPD_PORT > mpd-listallinfo.text

Replacing $MPD_HOST with the MPD IP (typically localhost) and $MPD_PORT with the port MPD is listening on (typically 6600). Note nc is the "netcat" utility which is widely available, but you may have to install it on your platform.

[hindumagic]

Haha, just looked at my bug report.. kinda left it hanging cause I forgot to refer to the previously closed bug report I found (which was unresolved).

Tried the latest binary you linked to with the same result.

I captured the output of listallinfo. All 4.5 MB. I found this hosting service, should be good for 30 days...: https://ufile.io/479texhc

Let me know if you need the file through some other means. Could be a funky filename, I have a few ':' in the library and samba mangles the names on my linux boxes.

[hindumagic]

Whoops, and to piggyback on what I just uploaded, I did a quick "wc -L" on the output (word count max line length) and it was... 4469! Ack. I'll need to find an easy method to see exactly which line it is...

But I think we just bumped into the issue. A quick fix is to just rename the offending file (or dir). Wonder if we can pressure upstream to bump up the buffer...

[hindumagic]

Found the offending line - a very detailed comment on a Cars album.

`ian@oldgold ~/Downloads $ cat mpd-listallinfo.text |awk '{print length, $0}'|sort -nr|head -1

4469 Comment: The Cars - 1978 - The Cars (2016 HDtracks) [FLAC@192khz24bit] Artist: The Cars Title: The Cars Producer: Roy Thomas Baker Release Date: June 6, 1978 Recorded: February 1978, AIR Studios, London Label: Rhino Records, Elektra Records Genres: Rock, New Wave, Pop Rock Duration: 35:10 Website: http://www.hdtracks.com/the-cars-291230 The Cars: Wikipedia: The Cars are an American rock band that emerged from the new wave scene in the late 1970s. The band originated in Boston, Massachusetts in 1976, with singer, rhythm guitarist and songwriter Ric Ocasek, singer and bassist Benjamin Orr, lead guitarist Elliot Easton, keyboardist Greg Hawkes and drummer David Robinson. The Cars were at the forefront in merging 1970s guitar-oriented rock with the new synthesizer-oriented pop that was then becoming popular and which would flourish in the early 1980s. Robert Palmer, music critic for The New York Times and Rolling Stone, described the Cars' musical style by saying: "they have taken some important but disparate contemporary trends—punk minimalism, the labyrinthine synthesizer and guitar textures of art rock, the '50s rockabilly revival and the melodious terseness of power pop—and mixed them into a personal and appealing blend." The Cars were named "Best New Artist" in the 1978 Rolling Stone Readers' Poll and won "Video of the Year" for "You Might Think" at the first MTV Video Music Awards in 1984. Their debut album, The Cars, sold six million copies and appeared on the Billboard 200 album chart for 139 weeks. As of 2001, the Cars have sold over 23 million albums in the United States. The band broke up in 1988, and Ocasek had always discouraged talk of a reunion since then. Orr died in 2000 from pancreatic cancer. In 2005, Easton and Hawkes joined with Todd Rundgren to form a spin-off band, the New Cars, which performed classic Cars and Rundgren songs alongside new material. The original surviving members reunited in 2010 to record a new album, Move Like This, which was released in May 2011, followed by a short tour. In 2015, they were nominated for induction into the Rock and Roll Hall of Fame. The Cars: Wikipedia: The Cars is the eponymous debut studio album by the American new wave band the Cars. It was released on June 6, 1978 on Elektra Records. The album, which featured the three charting singles "Just What I Needed," "My Best Friend's Girl," and "Good Times Roll," as well as several album-oriented rock radio hits, was a major success for the band, remaining on the charts for 139 weeks. It has been recognized as one of the band's best albums. AllMusic Review by Greg Prato: The Cars' 1978 self-titled debut, issued on the Elektra label, is a genuine rock masterpiece. The band jokingly referred to the album as their "true greatest-hits album," but it's no exaggeration -- all nine tracks are new wave/rock classics, still in rotation on rock radio. Whereas most bands of the late '70s embraced either punk/new wave or hard rock, the Cars were one of the first bands to do the unthinkable -- merge the two styles together. Add to it bandleader/songwriter Ric Ocasek's supreme pop sensibilities, and you had an album that appealed to new wavers, rockers, and Top 40 fans. One of the most popular new wave songs ever, "Just What I Needed," is an obvious highlight, as are such familiar hits as "Good Times Roll," "My Best Friend's Girl," and "You're All I've Got Tonight." But like most consummate rock albums, the lesser-known compositions are just as exhilarating: "Don't Cha Stop," "Bye Bye Love," "All Mixed Up," and "Moving in Stereo," the latter featured as an instrumental during a steamy scene in the popular movie Fast Times at Ridgemont High. With flawless performances, songwriting, and production (courtesy of Queen alumni Roy Thomas Baker), the Cars' debut remains one of rock's all-time classics. Tracklist: 01 - Good Times Roll - 03:45 02 - My Best Friend's Girl - 03:44 03 - Just What I Needed - 03:44 04 - I'm In Touch With Your World - 03:30 05 - Don't Cha Stop - 03:03 06 - You're All I've Got Tonight - 04:13 07 - Bye Bye Love - 04:13 08 - Moving In Stereo - 04:43 09 - All Mixed Up - 04:15 Personnel: Ric Ocasek – lead vocals, rhythm guitar Elliot Easton – lead guitar, backing vocals Greg Hawkes – keyboards, percussion, saxophone, backing vocals Benjamin Orr – lead vocals, bass David Robinson – drums, percussion, Syndrums, backing vocals`

[joshkunz]

Thanks for the investigation! This definitely looks like a bug in libmpdclient. As I mentioned in the previous comment, there is a PR upstream to get this fixed. I filed a new issue with libmpdclient to see if they're open to moving forward with that PR.

In the meantime, I've opened PR #91 to add a more helpful error message in this case.

Fixing this upstream is definitely my preference, since there are many clients that use libmpdclient and this issue can affect any of them. If libmpdclient is unwilling to fix this for whatever reason, then the only alternative would be implementing and maintaining our own. Hopefully that isn't required.

For you, or others, affected by this issue: The short-term work-around is to decrease the length of the problematic field. Unfortunately, since this is part of libmpdclient's protocol layer, there's not much ashuffle itself can do to help with this.

Let me know if you need the file through some other means.

Just a tip for future reference: GitHub actually allows you to attach files directly to issue comments. No external host needed 😄

[hindumagic]

Thanks for the tip - I'll check out the details.

Not sure if you're looking for advice, but I would take the jump and convert this to a daemon with a proper log file. As long as you've got a connection to mpd, you have a variable point of failure. If you daemonize the program, you can do some nice stuff like just log any service interruption and continue with reconnection attempts. I think that logging would help with debugging issues like this as well. Even better, external control of the daemon to start/stop it for cleaner integration into other systems like MoodeAudio.

I think that this program is a great idea! Um, and please excuse the above if I'm out of line - I haven't even fixed my library to get ashuffle to work yet! (so I don't even know if it is a daemon or not - haha)

[joshkunz]

I'll keep it brief since it's not really related to the issue at hand. Feel free to open a new issue to discuss new enhancements/features you'd like to see in ashuffle. We can continue talking there.

I would take the jump and convert this to a daemon...

Again feel free to open an issue, but IMO, daemonization is overrated, and in some cases harmful. Things like restarts and log aggregation are much better handled by daemon managers like systemd, daemontools, or any other system-specific daemon manager. A helpful ashuffle user has actually contributed systemd configuration scripts that can help with running ashuffle in the background.

I think that logging would help with debugging issues like this as well.

Unfortunately, no amount of additional logging in ashuffle would help with this particular issue, since the bug is in a library we depend on. I'm not opposed to adding logging, but I'm not sure what additional logging would be helpful.

Even better, external control of the daemon to start/stop it for cleaner integration into other systems like MoodeAudio.

IMO the current "start/stop" control of opening/closing ashuffle seems to work pretty well, and it's generic across all tools that a system like moOde would depend on. A custom control protocols seems more annoying for users to adapt too. ashuffle has a bit of a startup time problem (see issue #78 for example), but nobody has specifically claimed this is a problem for them in the context of controlling ashuffle. If a user had a proposal for a different control protocol I'm open to it.

[cricalix]

I'm seeing this problem where no single tag has a value over 1500 bytes (but I suspect there are files with several long tags that must be adding up to > 4096 bytes and triggering libmpdclient), and found a useful workaround that others may be able to use. Instead of relying on ashuffle's call to mpd to get all tracks in the library, use mpc search and pipe it to ashuffle. Then, change the ashuffle arguments to include "-n --file -". The "-n" is critical, since that stops ashuffle from getting the validation data from mpd and triggering the bug in libmpdclient.

I'm doing this all via systemd, so my ExecStart looks like

/bin/bash -c "/usr/bin/mpc search filename 'NAS' | /usr/local/bin/ashuffle -n --queue-buffer 2 --file -"

This only really works if you have a single source I suppose, but that's my setup and it works for me :)

/bin/bash -c bit is important, since ExecStart cannot include pipes and redirections.

This also avoids having to mod moOde's php code and deal with re-applying updates on update.

[NebuPookins]

If you want to search your entire music library (instead of only songs whose filename contain "NAS"), I think you can do something like this:

mpc listall | ashuffle --no-check --file -

[NebuPookins]

There are pros and cons to the two workarounds listed here:

1. Edit your tags to be less than 4K.
   • Pros: Works natively with mpd, e.g. if the library gets updated, ashuffle will see the new songs?
   • Cons: Have to edit your tags, which may mean throwing away data you want to preserve.
2. Pipe mpc listall into ashuffle.
   • Pros: No need to edit tags.
   • Cons: mpc listall runs once. If library updates, ashuffle won't see the new songs unless you kill the command and re-run it.

@joshkunz I think there'd be a solution to get the best of both worlds if you tweaked the way --only and --queue-buffer interacted. I tried to do a workaround where I executed mpc listall | ashuffle --no-check --only 3 --queue-buffer 3 --file - in a loop. My expectation is that it would try to maintain a queue size of 3, and after adding 3 more songs to the queue, ashuffle would exit.

So like in the first iteration of the loop, it would add songs 'a', 'b', 'c', and then quit. Then because I'm running the command in a loop, it would immediately execute again and then block, because there's already 3 songs in the queue since I said --queue-buffer 3. Then, once 'a' finished playing, it would enqueue 'd' and block again. When 'b' finished playing, it would enqueue 'e'. Then when 'c' finished playing, it would enqueue 'f', and then quit since I said --only 3. And then my script would loop, and it ashuffle would block again.

The idea is that way, we refresh the list that mpc listall returns every 3 songs.

However, it looks like currently if you use --only, the --queue-buffer flag is ignored, and ashuffle just immediately enqueues the number of songs specified by --only even if that would cause the queue to become bigger than the value specified in --queue-buffer.

Is this a change you'd be willing to make?

[joshkunz]

High level comment for this thread: I'm pretty bummed that there hasn't been any movement on this on the libmpdclient side. I'd really prefer not needing weird work-arounds to solve the issue.

To @NebuPookins:

If you want to search your entire music library (instead of only songs whose filename contain "NAS"), I think you can do something like this:

mpc listall | ashuffle --no-check --file -

AFAIK, mpc listall also uses libmpdclient so I'd be kind-of surprised if that worked... I did make some optimizations to speed up startup that might be over-eager in fetching track metadata. Maybe I could trim some of that down when no exclusion rules are present... If someone can confirm that mpc listall is working but ashuffle is not, then I can try optimize this to avoid the mpc listall step.

My expectation is that it would try to maintain a queue size of 3, and after adding 3 more songs to the queue, ashuffle would exit.

I'm not sure that makes a ton of sense, I'd really prefer to just leave --only's behavior really straight forward. Having it block when --queue-buffer is set is pretty unintuitive. Honestly, I should probably add some extra validation to make --only and --queue-buffer exclusive...

I tried to do a workaround where I executed mpc listall | ashuffle --no-check --only 3 --queue-buffer 3 --file - in a loop.

This is duplicating ashuffle's core loop. If this is a work-around for this issue there has to be a better solution here. For example, I'd consider adding something a --db-command so that ashuffle would know what command to re-run when the DB is updated to refresh the track list. Or, say --tweak exit-on-db-refresh to force an exit when the DB is changed so that ashuffle can be re-started with a new track list. That seems more straightforward.

[NebuPookins]

High level comment for this thread: I'm pretty bummed that there hasn't been any movement on this on the libmpdclient side. I'd really prefer not needing weird work-arounds to solve the issue.

Agreed. I'm just trying to be pragmatic here as without any workarounds, ashuffle is essentially unusable (crashes on startup) because of this issue (if you're unwilling to edit your tags).

If someone can confirm that mpc listall is working but ashuffle is not, then I can try optimize this to avoid the mpc listall step.

Just to be clear (since I never actually explicitly said this in my previous comments), ashuffle doesn't work for me but mpc listall does.

[09:53:33] nebu@Enshrined /home/nebu  
> ashuffle
libmpdclient received a malformed response from the server.
This may be because a song's metadata attribute (for example,
a comment) was longer than 4KiB.
See https://github.com/joshkunz/ashuffle/issues/89 for
details or updates.
MPD error: Response line too large
[09:53:35] nebu@Enshrined /home/nebu [1] 
> mpc listall | ashuffle --no-check --queue-buffer 10 --tweak window-size=100 --file -
Picking random songs out of a pool of 34178.

But it's fine with me if you're waiting for a 2nd person to confirm this.

This is duplicating ashuffle's core loop. If this is a work-around for this issue there has to be a better solution here.

Any of these sound fine to me.

[joshkunz]

Agreed. I'm just trying to be pragmatic here as without any workarounds, ashuffle is essentially unusable (crashes on startup) because of this issue (if you're unwilling to edit your tags).

I mean too be fair, it took ~10yrs before someone ran into this. But yeah I understand that for users with long tags the situation is not great.

Just to be clear (since I never actually explicitly said this in my previous comments), ashuffle doesn't work for me but mpc listall does.

Thanks for confirming! Yeah, I was just looking for someone to say that they tried it explicitly. I'll look into seeing if we can use whatever strategy mpc listall uses when no exclusion rules are set. I think folks will still be sunk if they need to do exclusions because we'll have to fetch the tags.

Any of these sound fine to me.

Awesome, I just sent out a PR to add a --tweak exit-on-db-update=yes option that can be used to work-around this, or in any situation where you want to re-run an external command when the db is updated. I'll merge that and cut a release tomorrow when the integration tests have run.

[joshkunz]

I took a look at mpc listall and it looks like they call mpd_send_list_all in the case where there is no special formatting. That should work great for ashuffle, so I'll work on adding that in as well...

[joshkunz]

I'll merge that and cut a release tomorrow when the integration tests have run.

This has been cut into v3.12.0, but I'm having some trouble with the automation. I guess this is the first release post switch to GitHub actions, so there's no binaries yet. I'm working on debugging the release issue.

[joshkunz]

Github issues have been ironed out, and this has now been cut into an official release v3.12.3.

[joshkunz]

The optimization to skip loading tags when no rules or groups are specified has been released as v3.12.5. This won't address the problem when rules (--exclude) or groups (--by-album or --group-by) are used but that should at least avoid the need to use the mpc listall work-around mentioned above.