Ogg VBR/CBR detect patch, bug report, etc.

[Joshua Uziel]

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]

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]

* 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]

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]

* 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]

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]

* 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]

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]

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]

* 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.