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.