Bugzilla – Bug 4932
$bitrate: Argument "\x{31}\x{36}..." isn't numeric in numeric
Last modified: 2008-12-18 11:11:58 UTC
SlimServer_6.5_v2007-04-21/Slim/Buttons/TrackInfo.pm line 278 $bitrate may contain something like: $VAR1 = \'160kbps CBR'; which leads to warn: Argument "\x{31}\x{36}..." isn't numeric in numeric gt (>) at ../SlimServer_6.5_v2007-04-21/Slim/Buttons/TrackInfo.pm line 278. which is obvious when comparing it like: if ( '160kbps CBR' > 0 ) suggested fix... hmmm... defined($bitrate) ? ne '' ?
I'm not sure if this is the best overall fix . It was added in change 8699 by andy in order to block an infinite loop. I can't see where that loop actually comes from, but it might make more global sense to move that block into $track->prettyBitrate so that the returned string will include the conversion status for all cases. This will combine with some code in Slim::Web::Pages::Status, and perhaps can also be reworked to address the request in bug 1277.
when QA gets to setting a target, it might make sense to use the simple fix: $bitrate ne '' or get rid of the check altogether (server now uses prettyBitRate, which will never be zero thus not trigger the infinite loop that needed the check as a bypass) then 7.0 can be set for the full rework, rolling in the aspects covered by 1277. I'll attach a proposed diff which could be an option for how the ui would look. It should be discussed whether we want to show the format at all times or ONLY if it is being converted from the native format
Created attachment 1911 [details] show bitrate as "160 kbps VBR (converted to 128kbps mp3)" tweak pretty bitrate to do the handling of adding the "converted to" text so that all bitrate displays have the same look. shows as: 878kbps CBR 878kbps CBR (Converted to 160kbps ABR) with the patch, while playing we easily get the output format as: 878kbps CBR (Converted to 160kbps ABR mp3) it will take a bit more work to get the output type in idle conditions.
Does this often happen in 6.5.2? This is the first time I've run across it. Or is it the root cause of some other of our problems?
I'm only ever able to see a string, as opposed to a stringref, so I don't get the warning either. It is rather harmless, other then to always block the "converted to..." part of the string. That would only be applicable when bitrate limiting, however. Peter, what type of track were you using for this example? It would probably be safe to punt post 6.5.2
(In reply to comment #3) > Created an attachment (id=1911) [edit] > show bitrate as "160 kbps VBR (converted to 128kbps mp3)" > > tweak pretty bitrate to do the handling of adding the "converted to" text so > that all bitrate displays have the same look. shows as: hmm either something is wrong with bugzilla, i'm blind or i'm missing something essential. the only thing i find inside this attached patchfile is a version-number bump from 6.5.1 to 6.5.2 this doesn't fix it, does it ? since i'm not a friend of OO programming; is this some oo-magic ?! 8-)
(In reply to comment #5) > I'm only ever able to see a string, as opposed to a stringref, so I don't get > the warning either. [...] > Peter, what type of track were you using for this example? I'm running SlimServer_6.5_v2007-04-21 (is this 6.5.3 ?) on an linux-box, external mysqld, reading exactly 5 different plain mp3s from a nfs-mounted directory. The Plugin-directory is almost empty (well TT and DigitalInput are there, which I declare as a bug, since they are essential). The bug gets triggered when I enter "now playing" pressing "right" to get into the file-information-menu. the mp3 is id3v2 clean, does only have a id3v1-tag appended and isn't converted to anything at all. "if ( defined($bitrate) ) {" works fine here, anyways 8-) looks like slimserver is getting the "featureitis"-disease ... 8-(
Created attachment 1915 [details] sorry, linked the wrong patch initially. It would be useful if you could save the derrogatory comments and perhaps describe what exact bit of code gave you the $VAR1 = \'160kbps CBR'; and a maybe even attach a small sample file that gives you the warning. I think we need to be seeing the same warning (are you running manually with perl -w perhaps?, or dumping \$bitrate?). on another note, Digital Input is a plugin because it's written in a way that is perfectly acceptable to disable (not much use for those with no Transporters on their network for example). TT is done this way because it is handled by TT as a plugin (also should be fully acceptable to disable in order to save memory use if not using skins that rely on that plugin). Hopefully the future folder structure will help relieve the confusion by moving such "native" plugins to Slim/Plugin and leaving Plugins/ for third party packages only.
Hopefully Andy will be able to point out why there was an infinite loop that needed to be avoided by this section of code, as I am not seeing it right now. It may be that simply deleting the IF wrapper is the solution for 6.5.2
(In reply to comment #8) > Created an attachment (id=1915) [edit] > sorry, linked the wrong patch initially. ah 8-) But I have several problems applying this patchfile to SlimServer_6.5_v2007-04-27, what version do you suggest to apply it to ? Should I use the svn tree ? I thought it would be enough to just get the latest nightly tarball and work with it. at least this worked fine for the '0' directory bug. any clues ? > It would be useful if you could save the derrogatory comments and perhaps > describe what exact bit of code gave you the $VAR1 = \'160kbps CBR'; Sorry, I didn't mean to offense you. My initial statement was "SlimServer_6.5_v2007-04-21/Slim/Buttons/TrackInfo.pm line 278" and that $bitrate might contain a string and this is futile for a comparism with "($bitrate < 0)". The "$VAR1" line comes from added code in TrackInfo.pm: "use Data::Dumper;print STDERR Dumper($bitrate);" which I added to debug the warning. > and a maybe even attach a small sample file that gives you the warning. Sorry, this wouldn't have helped, since it does not matter. ( '160kbps CBR' > 0 ) is buggy code. It triggers a __WARN__. That was the only reason why I reported it. Nothing else. I described my setup, I even removed (tried to) all Plugins to restrict the error to only the main code. But I now learn, (because of the problems with the patchfile) that we might be using a different codebase. So what should I do now ? Which is the current source of Slimserver to report bugs ? A hint would be enough. I browsed the wiki and up until now I thought it would be enough to download the nightly tarball. Help would be appreciated. I'm not saying its not documented, I just didn't find it (maybe fast enough). Concerning DigitalInput and TT: Again, no offense meant. As you stated, the future structure will fix this. It's not a bug and noone ever expects a huge software-project like this one to be perfect. I'm trying to help by reporting bugs to making it at least nearly perfect 8-) So lets concentrate on coding and kill some nasty bugs 8-) And sorry again, english isn't my native language. I often try to be either funny or sarcastic or both. and mostly this comes out wrong, when I try this in a language my mother didn't teach me 8-)
The proper solution is to ensure that $bitrate is always a number, never the 'pretty' string. My fault for not doing that. $bitrate can be -1 in the case where bitrate can't be detected, and that's why this code is there. There used to be a loop on streams where bitrate couldn't be detected, it would just keep trying to read the bitrate. I will look at fixing this soon.
Fixed in change 11892.
Fixed in 6.5.2, which is now released and available for download at http://www.slimdevices.com/su_downloads.html If you're still experiencing this bug, please re-open it!