Re: [Rhythmbox-devel] playlist source burner 0.2

[William Jon McCann <mccannwj pha jhu edu>]

Hi,

Bastien Nocera wrote:
...
> "This exceeds the 80 minute length of a standard audio CD."
>
> You don't know that the CD is 80 minutes long. Just say that it doesn't
> fit on the CD, it's better than lying on purpose.

That text is displayed, before converting the audio, iff the size of the 
media could not be determined and the length of the playlist is greater 
than 80 minutes.  You are right that the 80 is pretty arbitrary.  I'll 
remove this check and rely on the check at burn time when the disc size 
is known.

> MiB is 1000 kiB, not 1024.

Binary prefixes are all powers of 2. 
http://physics.nist.gov/cuu/Units/binary.html

> +        track->contents.audio.cdtext = g_path_get_basename (filename);
>
> We don't use the cdtext bit yet, but it would be better to get that info
> from the metadata in Rhythmbox. It would also mean that you could avoid
> non-ASCII filenames for the temporary files (shouldn't be a problem, but
> better safe than sorry).

OK.

> +rb_recorder_get_media_length
>
> In there, why aren't you using the supplied macros, instead of the magic
> numbers?

I didn't know about the macro :)  I figured out what the conversion was.

> The check for the RIFFWAVEfmt header should probably return
> ACB_ERROR_NOT_WAVE_FORMAT if it fails (it's a "bug" in my original
> code).

OK.

> What's the GINT_TO_LE for? It's probably not what you want to do (ie. I
> don't think a big endian machine would like being fed a little endian
> integer)

That is your code.  I don't know.

> Bear in mind, that I haven't tried, or seen this patch in action, so
> there might be more work needed than that. But thanks for doing that,
> many have failed where you might succeed :)

Hope so.  Thanks for your feedback.


Jon