PDA

View Full Version : Ogg VBR/CBR detect patch, bug report, etc.



Joshua Uziel
2005-01-20, 05:08
Hey, just got a Squeezebox today... I have to say that I'm impressed as
it does exactly what I want and how I want it to. A quick patch:


diff -urPN SlimServer_v5.4.0-orig/CPAN/Ogg/Vorbis/Header/PurePerl.pm SlimServer_v5.4.0/CPAN/Ogg/Vorbis/Header/PurePerl.pm
--- SlimServer_v5.4.0-orig/CPAN/Ogg/Vorbis/Header/PurePerl.pm 2004-11-15 11:59:27.000000000 -0800
+++ SlimServer_v5.4.0/CPAN/Ogg/Vorbis/Header/PurePerl.pm 2005-01-20 03:17:30.000000000 -0800
@@ -24,7 +24,7 @@
# check that the file exists and is readable
unless ( -e $file && -r _ )
{
- warn "File does not exist or cannot be read.";
+ warn "File $file does not exist or cannot be read";
# file does not exist, can't do anything
return undef;
}
diff -urPN SlimServer_v5.4.0-orig/Slim/Formats/Ogg.pm SlimServer_v5.4.0/Slim/Formats/Ogg.pm
--- SlimServer_v5.4.0-orig/Slim/Formats/Ogg.pm 2004-11-15 11:59:26.000000000 -0800
+++ SlimServer_v5.4.0/Slim/Formats/Ogg.pm 2005-01-20 03:03:39.000000000 -0800
@@ -87,6 +87,8 @@
$tags->{'STEREO'} = $ogg->info('channels') == 2 ? 1 : 0;
$tags->{'CHANNELS'} = $ogg->info('channels');
$tags->{'RATE'} = $ogg->info('rate') / 1000;
+ $tags->{'VBR_SCALE'}= $ogg->info('bitrate_upper') !=
+ $ogg->info('bitrate_lower');

# temporary for now - Ogg:: doesn't expose this yet.
$tags->{'OFFSET'} = $ogg->info('offset') || 0;


First off, I figured CPAN/Ogg/Vorbis/Header/PurePerl.pm could use a
slightly better output there. I'm running slimserver as "nobody" and
some files were owned by my user and with the file perm of 600.

Secondly, it's cosmetic, but the Song Info page for my oggs were showing

Bitrate: 192kbps CBR

when they were clearly VBR. I'm not sure if that method was best, but
it seems to work for me.

Finally, a bug report... if you set a rate limit in the client (like
320kbps) and you have a lame binary on your server, and then you
deselect "Ogg Vorbis MP3 oggdec/lame" in the "File Format
Conversion Setup" page, nothing plays. Yes, there's a limited transfer
rate set, so raw data won't come in under that rate... but if you
(re)move the lame binary, it works again.

This is all with SlimServer v5.4.0 and firmware 40. Thanks!

kdf
2005-01-20, 11:32
Quoting Joshua Uziel <uzi (AT) uzix (DOT) org>:


> Finally, a bug report... if you set a rate limit in the client (like
> 320kbps) and you have a lame binary on your server, and then you
> deselect "Ogg Vorbis MP3 oggdec/lame" in the "File Format
> Conversion Setup" page, nothing plays. Yes, there's a limited transfer
> rate set, so raw data won't come in under that rate... but if you
> (re)move the lame binary, it works again.
>
> This is all with SlimServer v5.4.0 and firmware 40. Thanks!

Hi Josh,

The intent here is to create the best overall experience out of the box. If the
LAME binary is not installed, which it may not for a new user, the server will
attempt to use RAW playback, regardless of bitrate limit (squeezebox wireless
defaults to 320kbps when wireless, but it wouldn't be nice to be silent on
first use). If there is a bitrate limit, and the file type setting is
specifically disabled, then it is assumed that the user has chosen specifically
to disable that format for playback as mp3. If there is also a bitrate limit
in effect, then the server will fail to find a conversion that fits the user
requirements and move on to the next song. Of course, by installing lame, then
removing it, you end up fooling the server into thinking you are a new user so
it goes back to trying to play no matter what :)

What do you suggest would help eliminate the confusion for this case?

-kdf

Joshua Uziel
2005-01-20, 12:07
* kdf <slim-mail (AT) deane-freeman (DOT) com> [050120 10:33]:
> Of course, by installing lame, then removing it, you end up fooling
> the server into thinking you are a new user so it goes back to trying
> to play no matter what :)
>
> What do you suggest would help eliminate the confusion for this case?

Ok, so your description makes sense in that it won't play if you select
a rate limit but have no means of meeting that limit. My suggestion
would be to add some output to the log for this case. Putting something
like "Error: couldn't limit audio bitrate, check file formats setting."
to the log would at least give a little insight into what's going on.
Once you have something like that, you can either go ahead and play it
or not, that doesn't matter too much to me. I just think that some
diagnostics would be helpful.

In my case, all my music is in 192kbps ogg format, and I don't want to
re-encode what's already been encoded once. So I've gone and set "No
Limit" and I'm done with it... but new users *with* lame installed might
be confused why their wireless SB that defaulted to 320kbps isn't
playing their music when they deselect "oggdec/lame" from the file
formats.

kdf
2005-01-20, 12:28
Quoting Joshua Uziel <uzi (AT) uzix (DOT) org>:

> * kdf <slim-mail (AT) deane-freeman (DOT) com> [050120 10:33]:
> > Of course, by installing lame, then removing it, you end up fooling
> > the server into thinking you are a new user so it goes back to trying
> > to play no matter what :)
> >
> > What do you suggest would help eliminate the confusion for this case?
>
> Ok, so your description makes sense in that it won't play if you select
> a rate limit but have no means of meeting that limit. My suggestion
> would be to add some output to the log for this case. Putting something
> like "Error: couldn't limit audio bitrate, check file formats setting."
> to the log would at least give a little insight into what's going on.
> Once you have something like that, you can either go ahead and play it
> or not, that doesn't matter too much to me. I just think that some
> diagnostics would be helpful.

Great, I'll take a look at that section and forward a patch to Slim Devices.

> In my case, all my music is in 192kbps ogg format, and I don't want to
> re-encode what's already been encoded once. So I've gone and set "No
> Limit" and I'm done with it... but new users *with* lame installed might
> be confused why their wireless SB that defaulted to 320kbps isn't
> playing their music when they deselect "oggdec/lame" from the file
> formats.

Good Point. How does an added note at the top of the web UI when a user
disables an MP3 format sound? If a user tries to enable a file type that has a
missing binary, we put up a warning. Something similar could be done when
disabling a format that would affect bitrate limiting.

how about something like:
"note: bitrate limiting for this format also disabled"

-kdf

Joshua Uziel
2005-01-20, 12:43
* kdf <slim-mail (AT) deane-freeman (DOT) com> [050120 11:29]:
> Great, I'll take a look at that section and forward a patch to Slim Devices.

Thanks! And don't forget my patches. ;)

> Good Point. How does an added note at the top of the web UI when a
> user disables an MP3 format sound? If a user tries to enable a file
> type that has a missing binary, we put up a warning. Something
> similar could be done when disabling a format that would affect
> bitrate limiting.
>
> how about something like:
> "note: bitrate limiting for this format also disabled"

Yep, that sounds good. That, in tandem with some log output would be
great.

It's funny... I used to work on the Cobalt server appliances, and this
whole device/appliance/whatever controlled via a web interface, coupled
with digging into the code is giving me some nostalgia here. :)

kdf
2005-01-20, 12:49
Quoting Joshua Uziel <uzi (AT) uzix (DOT) org>:

> * kdf <slim-mail (AT) deane-freeman (DOT) com> [050120 11:29]:
> > Great, I'll take a look at that section and forward a patch to Slim
> Devices.
>
> Thanks! And don't forget my patches. ;)

I haven't, but I dont have to worry about those. You've provided the right
format, so any of the Slim Devices guys can review and merge at will. I'll
forward them anyway.

-kdf

Joshua Uziel
2005-01-20, 12:53
* kdf <slim-mail (AT) deane-freeman (DOT) com> [050120 11:50]:
> I haven't, but I dont have to worry about those. You've provided the right
> format, so any of the Slim Devices guys can review and merge at will. I'll
> forward them anyway.

Heh, you mean there's a format other than "diff -u"? ;) Thanks! :)

kdf
2005-01-20, 13:14
Quoting Joshua Uziel <uzi (AT) uzix (DOT) org>:

> * kdf <slim-mail (AT) deane-freeman (DOT) com> [050120 11:50]:
> > I haven't, but I dont have to worry about those. You've provided the right
> > format, so any of the Slim Devices guys can review and merge at will. I'll
> > forward them anyway.
>
> Heh, you mean there's a format other than "diff -u"? ;) Thanks! :)
>
not for slimserver :)

btw, if you are interested in digging into more of the code in the future, feel
free to join the developers list. It's the better place for dropping patches
and getting involved in the code plans.

cheers,
kdf

kdf
2005-01-20, 13:32
Quoting Joshua Uziel <uzi (AT) uzix (DOT) org>:

> Once you have something like that, you can either go ahead and play it
> or not, that doesn't matter too much to me. I just think that some
> diagnostics would be helpful.

it just occurred to me, have you tried looking at the log output when you check
d_source in server settings, debugging? That does put out a great deal of info
specifically for transcoding formats at play time.

-kdf

Joshua Uziel
2005-01-20, 15:45
* kdf <slim-mail (AT) deane-freeman (DOT) com> [050120 12:32]:
> btw, if you are interested in digging into more of the code in the
> future, feel free to join the developers list. It's the better place
> for dropping patches and getting involved in the code plans.

I'm strongly fighting the urge to join that list, but it'll probably
happen eventually. :)

> it just occurred to me, have you tried looking at the log output when
> you check d_source in server settings, debugging? That does put out a
> great deal of info specifically for transcoding formats at play time.

Ok, I'll check it out. Thanks for the tip! I think I mentioned that I
just got the SB yesterday, so I'm not done exploring yet. :)

kdf
2005-01-20, 15:50
Quoting Joshua Uziel <uzi (AT) uzix (DOT) org>:

> * kdf <slim-mail (AT) deane-freeman (DOT) com> [050120 12:32]:
> > btw, if you are interested in digging into more of the code in the
> > future, feel free to join the developers list. It's the better place
> > for dropping patches and getting involved in the code plans.
>
> I'm strongly fighting the urge to join that list, but it'll probably
> happen eventually. :)

haha! yes, it does tend to have its effects on one's schedule :)

>
> > it just occurred to me, have you tried looking at the log output when
> > you check d_source in server settings, debugging? That does put out a
> > great deal of info specifically for transcoding formats at play time.
>
> Ok, I'll check it out. Thanks for the tip! I think I mentioned that I
> just got the SB yesterday, so I'm not done exploring yet. :)

np...I was looking for a place to add a debug message and I realised that there
is a lot there already. Please do keep reporting back on anything that catches
your interest.

-kdf

Joshua Uziel
2005-01-20, 16:06
* kdf <slim-mail (AT) deane-freeman (DOT) com> [050120 14:50]:
> np...I was looking for a place to add a debug message and I realised
> that there is a lot there already. Please do keep reporting back on
> anything that catches your interest.

Will do. I also have some initial nitpicks/wishlist items that lie
outside the realm of SlimServer, such as:

1) The device seems to be made for being at a slightly lower vantage
point than the user as it faces a little upward. I would like for there
to be little kickstands towards the back of the device similar to what
you have on most keyboards so that you can face it a little downwards as
well. In my case, the SB is on top of my center speaker, which is on
top of my tv... so it's high up. I've stuck an eraser underneath the
back of the device for a temporary solution. :)

2) Open versus shared key WEP... no need to discuss this further.

3) The wireless SBs have both wired and wireless... it might be cool if
you could use the SB as an access point. Of course, at least 10/100 and
802.11g would be preferred, so I'm talking about a future spin of the
device.

Just in case anyone cares... :)

kdf
2005-01-20, 16:33
Quoting Joshua Uziel <uzi (AT) uzix (DOT) org>:

> 1) The device seems to be made for being at a slightly lower vantage
> point than the user as it faces a little upward. I would like for there
> to be little kickstands towards the back of the device similar to what
> you have on most keyboards so that you can face it a little downwards as
> well. In my case, the SB is on top of my center speaker, which is on
> top of my tv... so it's high up. I've stuck an eraser underneath the
> back of the device for a temporary solution. :)

got something similar myself :)

> 2) Open versus shared key WEP... no need to discuss this further.

There are a few requests related to this on http://bugs.slimdevices.com.
WEP Key index (646), open system(224) and WPA (247).

> 3) The wireless SBs have both wired and wireless... it might be cool if
> you could use the SB as an access point. Of course, at least 10/100 and
> 802.11g would be preferred, so I'm talking about a future spin of the
> device.

bug 11 for 802.11G. As for future spins, that's the one area where Slim Devices
reserves the right to be not so open. It is their policy not to comment on
future hardware. They managed to get us from Slimp3, to squeezebox and then the
new graphics displays, so there is sure to be lots more to come.

> Just in case anyone cares... :)

absolutely! Just about every employee of Slim Devices reads these. If you check
the archives, you'll see a lot of @slimdevices.com addy in and around. I'm
just quicker than they are today :)

-kdf