archseer/ruby-mpd

extraneous data leading to inconsistencies in MPD#songs

Opened this issue · 4 comments

I'm working on Dockerizing an application that uses ruby-mpd. I include a small set of Creative Commons mp3s as a kind of "database seed". I noticed that @mpd.songs.count was always 1 greater than the actual count of my library.

using some Dirty Ruby Tricks(tm), I've traced it as follows:

[1] pry(main)> @mpd = MPD.new ENV['MPD_HOST'], ENV['MPD_PORT']    
=> #<MPD:0x0000000584e6d0
 @callbacks={},
 @hostname="127.0.0.1",
 @mutex=#<Mutex:0x0000000584e590>,
 @options={:callbacks=>false},
 @password=nil,
 @port="6600",
 @socket=nil,
 @tags=nil,
 @version=nil>
[2] pry(main)> @mpd.connect    
=> true
[3] pry(main)> unless ENV['MPD_PASS'].nil?    
[3] pry(main)*   @mpd.password ENV['MPD_PASS']      
[3] pry(main)* end      
=> nil
[4] pry(main)> 
[5] pry(main)> 
[6] pry(main)> 
[7] pry(main)> 
[8] pry(main)> @mpd.send('socket').puts 'listallinfo'
=> nil
[9] pry(main)> response = @mpd.send('handle_server_response')
=> "directory: test_library\nLast-Modified: 2015-08-04T20:01:23Z\nfile: test_library/Indian_Summer-212174.mp3\nLast-Modified: 2009-04-04T13:04:47Z\nTime: 286\nArtist: canton\nTitle: Indian Summer\nAlbum: http://www.cantonbecker.com\nfile: test_library/Map_of_the_Cosmos-184608.mp3\nLast-Modified: 2009-03-16T01:37:52Z\nTime: 395\nArtist: canton\nTitle: Map of the Cosmos\nAlbum: http://www.cantonbecker.com\n"
[10] pry(main)> 
[11] pry(main)> response.lines
=> ["directory: test_library\n",
 "Last-Modified: 2015-08-04T20:01:23Z\n",
 "file: test_library/Indian_Summer-212174.mp3\n",
 "Last-Modified: 2009-04-04T13:04:47Z\n",
 "Time: 286\n",
 "Artist: canton\n",
 "Title: Indian Summer\n",
 "Album: http://www.cantonbecker.com\n",
 "file: test_library/Map_of_the_Cosmos-184608.mp3\n",
 "Last-Modified: 2009-03-16T01:37:52Z\n",
 "Time: 395\n",
 "Artist: canton\n",
 "Title: Map of the Cosmos\n",
 "Album: http://www.cantonbecker.com\n"]
[12] pry(main)> response.lines.reject {|line| line =~ /(#{[:directory, :playlist].join('|')}):/i}
=> ["Last-Modified: 2015-08-04T20:01:23Z\n",
 "file: test_library/Indian_Summer-212174.mp3\n",
 "Last-Modified: 2009-04-04T13:04:47Z\n",
 "Time: 286\n",
 "Artist: canton\n",
 "Title: Indian Summer\n",
 "Album: http://www.cantonbecker.com\n",
 "file: test_library/Map_of_the_Cosmos-184608.mp3\n",
 "Last-Modified: 2009-03-16T01:37:52Z\n",
 "Time: 395\n",
 "Artist: canton\n",
 "Title: Map of the Cosmos\n",
 "Album: http://www.cantonbecker.com\n"]

note statements 11 and 12. MPD::Parser#filter_lines removes the "directory" line, but leaves behind the "Last-Modified" line associated with the directory. this later on manifests as a bizarre, inconsistent song list:

[13] pry(main)> @mpd.songs
=> [#<MPD::Song:0x000000049a5930
  @album=nil,
  @albumartist=nil,
  @artist=nil,
  @data={:"last-modified"=>2015-08-04 20:01:23 UTC},
  @file="test_library/Indian_Summer-212174.mp3",
  @mpd=
   #<MPD:0x0000000584e6d0
    @callbacks={},
    @hostname="127.0.0.1",
    @mutex=#<Mutex:0x0000000584e590>,
    @options={:callbacks=>false},
    @password=nil,
    @port="6600",
    @socket=#<TCPSocket:fd 10>,
    @tags=nil,
    @version="0.19.0">,
  @time=nil,
  @title=nil>,
 #<MPD::Song:0x000000049a58b8
  @album="http://www.cantonbecker.com",
  @albumartist=nil,
  @artist="canton",
  @data={:"last-modified"=>2009-04-04 13:04:47 UTC},
  @file="test_library/Map_of_the_Cosmos-184608.mp3",
  @mpd=
   #<MPD:0x0000000584e6d0
    @callbacks={},
    @hostname="127.0.0.1",
    @mutex=#<Mutex:0x0000000584e590>,
    @options={:callbacks=>false},
    @password=nil,
    @port="6600",
    @socket=#<TCPSocket:fd 10>,
    @tags=nil,
    @version="0.19.0">,
  @time=[nil, 286],
  @title="Indian Summer">,
 #<MPD::Song:0x000000049a5868
  @album="http://www.cantonbecker.com",
  @albumartist=nil,
  @artist="canton",
  @data={:"last-modified"=>2009-03-16 01:37:52 UTC},
  @file=nil,
  @mpd=
   #<MPD:0x0000000584e6d0
    @callbacks={},
    @hostname="127.0.0.1",
    @mutex=#<Mutex:0x0000000584e590>,
    @options={:callbacks=>false},
    @password=nil,
    @port="6600",
    @socket=#<TCPSocket:fd 10>,
    @tags=nil,
    @version="0.19.0">,
  @time=[nil, 395],
  @title="Map of the Cosmos">]

3 songs where there should be 2, one with a nil @file variable, another with a nil @time variable.

I have no idea what a fix would/should look like. I think I've spent all of today's brain juices tracing the issue this far.

as an additional exhibit, here's a data structure that has obviously weird behavior in :"last-modified" and :time. I didn't trace it fully, but I assume this data structure will eventually become an array of MPD::Song objects. the mis-matched columns likely causes malformed objects to be created.

[15] pry(main)> @mpd.send('parse_response', 'listallinfo', response)
=> {:directory=>"test_library",
 :"last-modified"=>[23, 1, 20, 4, 8, 2015, 2, 216, false, "UTC", 2009-04-04 13:04:47 UTC, 2009-03-16 01:37:52 UTC],
 :file=>["test_library/Indian_Summer-212174.mp3", "test_library/Map_of_the_Cosmos-184608.mp3"],
 :time=>[nil, 286, [nil, 395]],
 :artist=>["canton", "canton"],
 :title=>["Indian Summer", "Map of the Cosmos"],
 :album=>["http://www.cantonbecker.com", "http://www.cantonbecker.com"]}

Yeah, seems like the parser just strips the directory declarations out, leaving the last-modified timestamps in place. The implementation will need modification to remove those out as well.

for this category of problem, I always feel like my solution is brutish and inelegant. here's a fix that seems to resolve my test case issue: neoice@577d810

given that MPD::Parser#filter_lines is dropping both :directory and :playlist, I'm not 100% sure what will happen to the Playlist objects. I'm attempting to test against that now.

root@3815993ad0f1:/# mpc lsplaylists
foobar
[1] pry(main)> @mpd = MPD.new ENV['MPD_HOST'], ENV['MPD_PORT']
=> #<MPD:0x00000003e0fdc8
 @callbacks={},
 @hostname="127.0.0.1",
 @mutex=#<Mutex:0x00000003e0fcb0>,
 @options={:callbacks=>false},
 @password=nil,
 @port="6600",
 @socket=nil,
 @tags=nil,
 @version=nil>
[2] pry(main)> @mpd.connect
=> true
[3] pry(main)> @mpd.songs.count
=> 5
[4] pry(main)> @mpd.send('socket').puts 'listallinfo'
=> nil
[5] pry(main)> response = @mpd.send('handle_server_response')
=> "directory: test_library\nLast-Modified: 2015-08-07T14:50:41Z\nfile: test_library/Nova-139889831.mp3\nLast-Modified: 2014-03-16T18:46:30Z\nTime: 238\nfile: test_library/Indian_Summer-212174.mp3\nLast-Modified: 2009-04-04T13:04:47Z\nTime: 286\nArtist: canton\nTitle: Indian Summer\nAlbum: http://www.cantonbecker.com\nfile: test_library/Invisible_Space_Journeys-212173.mp3\nLast-Modified: 2009-04-04T13:01:05Z\nTime: 410\nArtist: canton\nTitle: Invisible Space Journeys\nAlbum: http://www.cantonbecker.com\nfile: test_library/Map_of_the_Cosmos-184608.mp3\nLast-Modified: 2009-03-16T01:37:52Z\nTime: 395\nArtist: canton\nTitle: Map of the Cosmos\nAlbum: http://www.cantonbecker.com\nfile: test_library/Sunflower-184607.mp3\nLast-Modified: 2009-03-16T01:37:39Z\nTime: 289\nArtist: canton\nTitle: Sunflower\nAlbum: http://www.cantonbecker.com\n"

looks like Playlist info doesn't actually come down with listallinfo.

I looked into this and noticed that the MPD protocol documentation frowns on using listallinfo and keeping a client-side copy of the database:

http://www.musicpd.org/doc/protocol/database.html

listallinfo [URI]

Same as listall, except it also returns metadata info in the same format as lsinfo.

Do not use this command. Do not manage a client-side copy of MPD's database. That is fragile and     adds huge overhead. It will break with large databases. Instead, query MPD whenever you need something.

You can get an accurate song count from the stats command:

mpd.send_command(:stats).fetch(:songs)