flac dev - Sep 2004 - [st.n@gmx.net: Bug#200435: xmms-flac: doesn't properly support long files]

> > -		file_info->length_in_msec = file_info->total_samples * 10 /
(file_info->sample_rate / 100);
> > +		file_info->length_in_msec =
(FLAC__uint64)file_info->total_samples * 10 / (file_info->sample_rate /
100);
...
> It seems like would be simpler to do something like:
> 
> (file_info->total_samples / file_info->sample_rate) * 1000;
Without a float cast, this truncates the length so it's always a multiple
of 1000ms. That's cool now, but it wouldn't be if the xmms gui changed
to
display track lengths with, say, tenth-of-a-second precision.

,steven.

On Tue, Jul 08, 2003 at 05:17:50PM -0400, Steven Richman wrote:
> > > -		file_info->length_in_msec = file_info->total_samples *
10 / (file_info->sample_rate / 100);
> > > +		file_info->length_in_msec =
(FLAC__uint64)file_info->total_samples * 10 / (file_info->sample_rate /
100);
> ...
> > It seems like would be simpler to do something like:
> > 
> > (file_info->total_samples / file_info->sample_rate) * 1000;
> 
> Without a float cast, this truncates the length so it's always a
multiple
> of 1000ms. That's cool now, but it wouldn't be if the xmms gui
changed to
> display track lengths with, say, tenth-of-a-second precision.
This seems no worse than the fact that the sampling rate ends up being
rounded to the nearest multiple of 100, but I see your point.

-- 
 - mdz

On Tue, Jul 08, 2003 at 10:11:49PM +0200, Miroslav Lichvar wrote:
> On Tue, Jul 08, 2003 at 11:07:09AM -0400, Matt Zimmerman wrote:
> > severity 200435 normal
> > thanks
> > 
> > I received this bug report from a Debian user.  I can't think of
any reason
> > offhand why the command line tool would work while the xmms plugin
would
> > fail.
> ...
> 
> It's an overflow, this patch will fix it. 
> 
> --- plugin.c.orig	2003-05-20 21:57:04.000000000 +0200
> +++ plugin.c	2003-07-08 22:03:37.000000000 +0200
> @@ -537,7 +537,7 @@
>  		file_info->bits_per_sample =
metadata->data.stream_info.bits_per_sample;
>  		file_info->channels = metadata->data.stream_info.channels;
>  		file_info->sample_rate = metadata->data.stream_info.sample_rate;
> -		file_info->length_in_msec = file_info->total_samples * 10 /
(file_info->sample_rate / 100);
> +		file_info->length_in_msec = (FLAC__uint64)file_info->total_samples
* 10 / (file_info->sample_rate / 100);
>  	}
>  	else if(metadata->type == FLAC__METADATA_TYPE_VORBIS_COMMENT) {
>  		double gain, peak;
It seems like would be simpler to do something like:

(file_info->total_samples / file_info->sample_rate) * 1000;

-- 
 - mdz

I am the before mentioned Debian user who reported this bug.

On Tue, Jul 08, 2003 at 17:17:50 -0400, Steven Richman
wrote:
> > > -		file_info->length_in_msec = file_info->total_samples *
10 / (file_info->sample_rate / 100);
> > > +		file_info->length_in_msec =
(FLAC__uint64)file_info->total_samples * 10 / (file_info->sample_rate /
100);
> ...
> > It seems like would be simpler to do something like:
> > 
> > (file_info->total_samples / file_info->sample_rate) * 1000;
> 
> Without a float cast, this truncates the length so it's always a
multiple
> of 1000ms. That's cool now, but it wouldn't be if the xmms gui
changed to
> display track lengths with, say, tenth-of-a-second precision.
Here is a suggestion to avoid this:

       length_in_msec = total_samples / sample_rate * 1000 +
               total_samples % sample_rate / ( sample_rate / 1000 );

    This should be exact to the msec.  No more parentheses than above are
needed since * / % are left-associative, although more parentheses wouldn't
hurt of course. :-)

    While looking at this, I also noticed that in include/FLAC/format.h,
total_samples is defined as FLAC__uint64, which is used in
src/plugin_xmms/plugin.c line 304, while in the latter file, line 57,
total_samples is defined as unsigned (used in line 538), which is only a
32 bit value in most cases. Wouldn't it be better to make it FLAC__uint64,
too?

Thanks,

    - Stephan.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url :
http://lists.xiph.org/pipermail/flac-dev/attachments/20030709/abcaa9df/attachment.pgp