Bugzilla – Bug 3807
Using "All Songs" as seed crashes the server
Last modified: 2008-09-15 14:39:24 UTC
Using MusicIP 1.6 on Linux and Windows XP When I press & hold "PLAY" on "All Songs" (Player UI), Slimserver crashes with the following message: Can't locate object method "tracks" via package "ALL_SONGS" (perhaps you forgot to load "ALL_SONGS"?) at D:/slimpy63/Plugins/MusicMagic/Plugin.pm line 870. While the same error occurs in 6.5, too, it doesn't crash the server: 2006-07-24 12:44:03.9426 ERROR: Request: Error when trying to run function coderef: [Can't locate object method "tracks" via package "ALL_SONGS" (perhaps you forgot to load "ALL_SONGS"?) at D:\slimpy/Plugins/MusicMagic/Plugin.pm line 507.]
Created attachment 1364 [details] wrap critical section in eval{} Here's a patch against the trunk which can fetch these problems. Don't know whether this is a good solution, but I didn't like comparing $currentItem against a static string 'ALL_SONGS'.
Comment on attachment 1364 [details] wrap critical section in eval{} Forget it... isn't working.
Created attachment 1365 [details] wrap critical lines in eval{} This one should work. (Note to author: you have to be running MusicIP for successful testing...)
we often check for $all conditions in other places, so the concept isn't bad. I take it that the result here is a bumpRight on an ALL* item? other option, of course, could be a random seed, or ensuring that ALL* isn't mixable in the first place (I was fairly sure this was already the case, and that the mixers shouldn't react unless ->mixable)
(In reply to comment #4) > we often check for $all conditions in other places, so the concept isn't bad. > I take it that the result here is a bumpRight on an ALL* item? It should bumpRight on any item which does represent a mixable item (not a song/artist/album object or why ever the code might cough up).
BrowseDB is checking for mixable in 6.5. Browse Albums->all songs, and I don't see the overlay and nothing happens when pressing and holding play. As such, I can't reproduce in 6.5, so it would seem to be already fixed in my tests.
> While the same error occurs in 6.5, too, it doesn't crash the server: I mentioned it in the initial report... I must be tired these days (the Transporter hype just cost me another hour of sleep :-)). Shall I fix it for 6.3.x - just in case anybody else came across the issue?
running latest from svn, I cannot see any mixer icon on any All Songs item in the player UI. Press and hold PLAY does nothing, so I cannot reproduce even the error that doesn't crash. I have no input on 6.3.1. I'm personally not spending any time with it at this point
I just updated my copy, went (on softsqueeze, this is, don't have the HW at work) to All Albums/Artists/Songs, press and hold PLAY: $ perl -w slimserver.pl 2006-07-25 08:12:57.6828 NoSetup: let's free the hijacked pages... 2006-07-25 08:13:07.7205 ERROR: Request: Error when trying to run function coderef: [Can't locate object method "tracks" via package "ALL_SONGS" (perhaps you forgot to load "ALL_SONGS"?) at D:\slimpy/Plugins/MusicMagic/Plugin.pm line 507.] 2006-07-25 08:13:07.7269 Request: Command [fb:a4:77:33:53:38->button] (Bad dispatch!) 2006-07-25 08:13:07.7285 Param: [_buttoncode] = [passback] 2006-07-25 08:13:07.7302 Param: [_time] = [180.343] 2006-07-25 08:13:07.7318 Param: [_orFunction] = [1] 2006-07-25 08:13:19.6888 ERROR: Request: Error when trying to run function coderef: [Can't locate object method "name" via package "ALL_ALBUMS" (perhaps you forgot to load "ALL_ALBUMS"?) at D:\slimpy/Plugins/MusicMagic/Plugin.pm line 513.] 2006-07-25 08:13:19.6952 Request: Command [fb:a4:77:33:53:38->button] (Bad dispatch!) 2006-07-25 08:13:19.6969 Param: [_buttoncode] = [passback] 2006-07-25 08:13:19.6985 Param: [_time] = [192.312] 2006-07-25 08:13:19.7002 Param: [_orFunction] = [1] 2006-07-25 08:13:24.1266 ERROR: Request: Error when trying to run function coderef: [Can't locate object method "name" via package "ALL_ARTISTS" (perhaps you forgot to load "ALL_ARTISTS"?) at D:\slimpy/Plugins/MusicMagic/Plugin.pm line 517.] 2006-07-25 08:13:24.1344 Request: Command [fb:a4:77:33:53:38->button] (Bad dispatch!) 2006-07-25 08:13:24.1361 Param: [_buttoncode] = [passback] 2006-07-25 08:13:24.1378 Param: [_time] = [196.75] 2006-07-25 08:13:24.1394 Param: [_orFunction] = [1] One nice side effect with my patch compared to the current state is the bumpRight :-)
Created attachment 1368 [details] shortcut to save time right, ok with you now. we're detecting mixable items in browsedboverlay like so: # A text item generally means ALL_, so overlay an arrow elsif (!ref($item)) { return (undef, Slim::Display::Display::symbol('rightarrow')); } this bypasses the rest of the routine to check mixable status, since !ref means ALL_* we could simply add that to the 'create_mix' function, and have a similar effect, and avoid the time spent on an eval.
> we could simply add that to the 'create_mix' function, and have a similar > effect, and avoid the time spent on an eval. Should this already be in your patch? I still get that warning/error message with the patch applied. I think we won't spend much time in the eval as it's backing out real soon anyway...
with the patch, I get no error. it just plays all.
tell you what. I'll work on a refactor of the plugins. I can make a few new class methods for title and prefName. These can be used for display and pref, then we can add accessors for the mixable state and refer to those. This way, only the actual valid mixers for the given item will show, instead of all activated mixers with a bump if it's not valid for that item. Of course, this is if you happen to have two mixers enabled. I'll notify erland of yet more changes. the eval patch could work for 6.3.1, if the nightlies are still coming through.
Subject: Re: Using "All Songs" as seed crashes the server We are not planning to do any more work on the 6.3.x series.
Created attachment 1371 [details] plugin accessors basic refactor. This creates new mthods for grabbing title, prefname and mixable status. This allows the play-and-hold feature to create a dynamic list of mixer options, if only one of two mixers are usable on the currentItem. The side effect of this fixes the problem from the summary as none of the ALL_* items will register as mixable, so the old fallback of play all will catch them. This also adds a benefit of dropping a case of a hardwired 'mixable' test for the browsedb overlay, in case we ever add more mixers.
Created attachment 1372 [details] fixed up a bit must test after cut and paste. a few more safety checks and corrected vars.
> fixed up a bit I guess this is from before the big merge? I can't apply. Will fix my 6.3.1 myself :-)
it's mostly for reference for me later...since as always, I'm buggered behind a corporate firewall all day so have to dump a patch here as simply ftp-ing files won't be much use with the big merge today. one hunk of browseDB seems to be a problem post-shocker.
Created attachment 1373 [details] might work post-merge this is an attempt to make something that might work. unfortunately, it might be worse if the line numbers aren't right. should be able to patch each hunk manually, worst case. This can wait anyway, so once I'm home and can mrege this patch in with an updated svn, I'll reup an accurate patch for testing.
Commited as change 8658. Michael - can you check and see if that fixes it? Thanks.
Idiot me - I tried to apply the patch again :-/. Yes, this seems fine. One more thing changed, though: the "MusicMagic Mixing..." animation appears on both lines. Due to the changes in the general display code? By design? Thanks!
seems to be someting in the new display code. search button does it sometimes, also noticed it in some other showbriefly mesages. I think it is the code filling in empty array indeces
Closing this - if there's a display issue, please open a new bug. Or just commit :)