Le mercredi 01 septembre 2004 à 16:55 -0400, William Jon McCann a
écrit :
> Hi Christophe,
>
> > +AM_CONDITIONAL(MKDTEMP_MISSING, test x$mkdtemp_missing = xtrue)
> > Won't compilation fail on systems without mkdtemp ?
>
> I think I copied this right out of nautilus-cd-burner. The check is
> used to pull in a header file in case the function is not defined on the
> system. Is this function provided in libnautilus-burn if not by the system?
Yeah, you are right, I missed that part, libnautilus-burn provides this
function if the system doesn't have it.
> > Should probably be marked for translation
>
> Do we translate warnings sent to the console?
I think translators generally says it should be translatable.
> > + if ((media_duration < 0) && (duration > 4800)) {
> >
> > Fwiw, the CD could be a 74 min CD, and I think there are 90 and 99
> > minutes CDs iirc. Maybe we should ask the user what to do when we
> > couldn't read the media size and the length is greater than 74*60?
>
> I'm fine with changing back to 74 minutes. I suppose asking the user is
> ok but I hate to add more dialogs. Maybe it is better to ask the user
> to insert the media and verify the size correctly?
When I suggested to add a dialog, I thought there were cases when there
is a blank cd in the recorder, but you can't read its size properly. If
the media_duration < 0 case only happens when there is no media or when
the media isn't blank, then there shouldn't be a dialog :)
>
> Not sure. It seems to work as it is.
Did you try with files that were not encoded at 44.1 16bit stereo?
>
> I think that is only true if you play audio before attempting to write a
> CD. I think I'll keep it for now.
Should probably be moved in rhythmbox initialization process then
instead of making it in several places
> > +static char *
> > +get_dest_from_uri (const char *tmp_dir,
> >
> Mostly true. We do need the files to have a .wav extension. From the
> cdrecord man page:
>
> It is extremely unlikely that a filename with the same name exists.
Agreed, it was only that I wasn't sure what the goal of the functions
were. The function had been meant to be a "create a file which we are
100% sure is unique and is the one we created" function then it wouldn't
have worked, but it seems it's a "generate a file name which 99.99%
likely to not exist" function, then it works ;)
>
> >
> > +static gint64
> > +get_tracks_length (RBRecorder *recorder,
> > + GError **error)
> >
> > We should already know the track length because we converted the track
> > to wav, maybe we don't need to reread it from the wav file? Actually,
> > the checks being done in rb_recorder_burn have already been done
> > previous in rb_playlist_source_recorder_start, didn't they?
>
> Sorta. We can't trust those durations.
Well, if you ask gstreamer about the stream duration after it has
finished writing the wav file, I hope it will give the same result as
the one it wrote in the wav header ;) Btw, there used to be bugs about
gstreamer not updating properly the wav length when using wavenc, I
assume they are fixed now?
> This check was recommended by
> Bastien. I think it is a good one. It is really the only authoritative
> check we do. Also, if this object is used for something else it is
> useful to do the check there.
Yeah, makes sense if the object is to be reused in other contexts. What
is bothering me is that we have 2 length checks using 2 different
methods to calculate the lengths. As long as bad things don't happen
(like the user saying he is ok to burn stuff, and then getting an error
message about rb not being able to burn, or the user getting 2 error
dialogs in a row) when the 2 tests disagree, that's fine with me.
Christophe
Attachment:
signature.asc
Description: Ceci est une partie de message =?ISO-8859-1?Q?num=E9riquement?= =?ISO-8859-1?Q?_sign=E9e?=