Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to completion
By that I mean, anywhere that MM:SS:FF is accepted, so is MM:SS (whose meaning is MM:SS:00) , and further, allowing MM:SS to be used regardless of the sample rate. Then the MM:SS:00 special case can be removed; the user will always use MM:SS when the sample rate is not 44100. The tests probably also have to be augmented to cover MM:SS, and possibly the man page and html docs. > > From: Earl Chew >To: Josh Coalson ; "flac-dev@xiph.org" >Sent: Wednesday, May 2, 2012 11:13 AM >Subject: Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to >completion > > >Josh, > > >Sure. I can try. Would you give me a more detailed description of the >requirement ? > > >What exactly does "general MM:SS handling" mean ? > > >Earl > > > > > > From: Josh Coalson >To: Earl Chew ; "flac-dev@xiph.org" >Sent: Tuesday, May 1, 2012 8:25:34 PM >Subject: Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to >completion > > >Ah, I don't remember but it might have been a negative test that was supposed >to give an error when used with the wrong sample rate. > > >Anyway, could you do another patch that updates to general MM:SS handling? > > > > >>________________ >> From: Earl Chew >>To: Josh Coalson ; "flac-dev@xiph.org" >>Sent: Thursday, April 26, 2012 3:12 PM >>Subject: Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to >>completion >> >> >>Josh, >> >> >>I see my name is mentioned in passing ;-) >> >> >>I submitted that patch because I thought it preferable to keep the test >>harness running. >> >> >>Backing this change out would also entail changing metaflac_test.sh >>accordingly, which at the time, I was hesitant to do. >>At the time I figured that at some point metaflac_test.sh was passing, so the >>code should probably allow it to pass. >> >> >>I'm ok either way. >> >> >> >>Earl >> >> >> >> >> From: Josh Coalson >>To: "flac-dev@xiph.org" >>Sent: Wednesday, April 25, 2012 3:23:35 PM >>Subject: Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to >>completion >> >> >>I haven't checked git yet but I hope this patch has not gone in. I don't >>like the special case that this is creating. >> >> >>It would be better to allow MM:SS everywhere but I consider that low priority. >> >> >> >> >>> >>> From: Earl Chew >>>To: "flac-dev@xiph.org" >>>Sent: Thursday, January 5, 2012 8:27 PM >>>Subject: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to >>>completion >>> >>> >>>When reading the INDEX from the cue sheet, the format MM:SS:FF format is >>>disallowed if the sample frequency is not a multiple of 75 because the index >>>would only be approximate. However, 00:00:00 is _exact_ because it denotes >>>the start of the track, so allow it as a special case. This allows >>>metaflac_test.sh to pass. >>> >>> >>>___ >>>flac-dev mailing list >>>flac-dev@xiph.org >>>http://lists.xiph.org/mailman/listinfo/flac-dev >>> >>> >>> >>___ >>flac-dev mailing list >>flac-dev@xiph.org >>http://lists.xiph.org/mailman/listinfo/flac-dev >> >> >> >> >> > > > >___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to completion
Josh, Sure. I can try. Would you give me a more detailed description of the requirement ? What exactly does "general MM:SS handling" mean ? Earl From: Josh Coalson To: Earl Chew ; "flac-dev@xiph.org" Sent: Tuesday, May 1, 2012 8:25:34 PM Subject: Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to completion Ah, I don't remember but it might have been a negative test that was supposed to give an error when used with the wrong sample rate. Anyway, could you do another patch that updates to general MM:SS handling? > > From: Earl Chew >To: Josh Coalson ; "flac-dev@xiph.org" >Sent: Thursday, April 26, 2012 3:12 PM >Subject: Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to >completion > > >Josh, > > >I see my name is mentioned in passing ;-) > > >I submitted that patch because I thought it preferable to keep the test >harness running. > > >Backing this change out would also entail changing metaflac_test.sh >accordingly, which at the time, I was hesitant to do. >At the time I figured that at some point metaflac_test.sh was passing, so the >code should probably allow it to pass. > > >I'm ok either way. > > > >Earl > > > >____________________ > From: Josh Coalson >To: "flac-dev@xiph.org" >Sent: Wednesday, April 25, 2012 3:23:35 PM >Subject: Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to >completion > > >I haven't checked git yet but I hope this patch has not gone in. I don't like >the special case that this is creating. > > >It would be better to allow MM:SS everywhere but I consider that low priority. > > > > >> >> From: Earl Chew >>To: "flac-dev@xiph.org" >>Sent: Thursday, January 5, 2012 8:27 PM >>Subject: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to >>completion >> >> >>When reading the INDEX from the cue sheet, the format MM:SS:FF format is >>disallowed if the sample frequency is not a multiple of 75 because the index >>would only be approximate. However, 00:00:00 is _exact_ because it denotes >>the start of the track, so allow it as a special case. This allows >>metaflac_test.sh to pass. >> >> >>___ >>flac-dev mailing list >>flac-dev@xiph.org >>http://lists.xiph.org/mailman/listinfo/flac-dev >> >> >> >___ >flac-dev mailing list >flac-dev@xiph.org >http://lists.xiph.org/mailman/listinfo/flac-dev > > > > >___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to completion
Ah, I don't remember but it might have been a negative test that was supposed to give an error when used with the wrong sample rate. Anyway, could you do another patch that updates to general MM:SS handling? > > From: Earl Chew >To: Josh Coalson ; "flac-dev@xiph.org" >Sent: Thursday, April 26, 2012 3:12 PM >Subject: Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to >completion > > >Josh, > > >I see my name is mentioned in passing ;-) > > >I submitted that patch because I thought it preferable to keep the test >harness running. > > >Backing this change out would also entail changing metaflac_test.sh >accordingly, which at the time, I was hesitant to do. >At the time I figured that at some point metaflac_test.sh was passing, so the >code should probably allow it to pass. > > >I'm ok either way. > > > >Earl > > > >________ > From: Josh Coalson >To: "flac-dev@xiph.org" >Sent: Wednesday, April 25, 2012 3:23:35 PM >Subject: Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to >completion > > >I haven't checked git yet but I hope this patch has not gone in. I don't like >the special case that this is creating. > > >It would be better to allow MM:SS everywhere but I consider that low priority. > > > > >>____ >> From: Earl Chew >>To: "flac-dev@xiph.org" >>Sent: Thursday, January 5, 2012 8:27 PM >>Subject: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to >>completion >> >> >>When reading the INDEX from the cue sheet, the format MM:SS:FF format is >>disallowed if the sample frequency is not a multiple of 75 because the index >>would only be approximate. However, 00:00:00 is _exact_ because it denotes >>the start of the track, so allow it as a special case. This allows >>metaflac_test.sh to pass. >> >> >>___ >>flac-dev mailing list >>flac-dev@xiph.org >>http://lists.xiph.org/mailman/listinfo/flac-dev >> >> >> >___ >flac-dev mailing list >flac-dev@xiph.org >http://lists.xiph.org/mailman/listinfo/flac-dev > > > > >___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to completion
Josh, I see my name is mentioned in passing ;-) I submitted that patch because I thought it preferable to keep the test harness running. Backing this change out would also entail changing metaflac_test.sh accordingly, which at the time, I was hesitant to do. At the time I figured that at some point metaflac_test.sh was passing, so the code should probably allow it to pass. I'm ok either way. Earl From: Josh Coalson To: "flac-dev@xiph.org" Sent: Wednesday, April 25, 2012 3:23:35 PM Subject: Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to completion I haven't checked git yet but I hope this patch has not gone in. I don't like the special case that this is creating. It would be better to allow MM:SS everywhere but I consider that low priority. > > From: Earl Chew >To: "flac-dev@xiph.org" >Sent: Thursday, January 5, 2012 8:27 PM >Subject: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to >completion > > >When reading the INDEX from the cue sheet, the format MM:SS:FF format is >disallowed if the sample frequency is not a multiple of 75 because the index >would only be approximate. However, 00:00:00 is _exact_ because it denotes the >start of the track, so allow it as a special case. This allows >metaflac_test.sh to pass. > > >___ >flac-dev mailing list >flac-dev@xiph.org >http://lists.xiph.org/mailman/listinfo/flac-dev > > > ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to completion
Josh Coalson wrote: > > From: Erik de Castro Lopo > > To: flac-dev@xiph.org > > Cc: Josh Coalson > > Sent: Wednesday, April 25, 2012 4:30 PM > > Subject: Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to > > completion > > > > Josh Coalson wrote: > > > >> I haven't checked git yet but I hope this patch has not gone in. I > >> don't like the special case that this is creating. > >> > >> It would be better to allow MM:SS everywhere but I consider that low > > priority. > > > > I modified version of that patch did go in. See the following commits: > > > > https://git.xiph.org/?p=flac.git;a=commit;h=19050f74eaa1aa6d609ca065e1a854ada5bb6b4c > > https://git.xiph.org/?p=flac.git;a=commit;h=7ee908403e8be011b8a7dd3bf511408b40312c2e > > https://git.xiph.org/?p=flac.git;a=commit;h=ce8a75134cace056f6c436d54b57bad1a1d93797 > > > > > > Erik > > I saw. I like almost all the progress on recent patches but not this one. > Can it be > backed out or replaced with proper MM:SS handling? I really like the idea of fixing it with proper MM:SS handling. Erik -- -- Erik de Castro Lopo http://www.mega-nerd.com/ ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to completion
> From: Erik de Castro Lopo > To: flac-dev@xiph.org > Cc: Josh Coalson > Sent: Wednesday, April 25, 2012 4:30 PM > Subject: Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to > completion > > Josh Coalson wrote: > >> I haven't checked git yet but I hope this patch has not gone in. I >> don't like the special case that this is creating. >> >> It would be better to allow MM:SS everywhere but I consider that low > priority. > > I modified version of that patch did go in. See the following commits: > > https://git.xiph.org/?p=flac.git;a=commit;h=19050f74eaa1aa6d609ca065e1a854ada5bb6b4c > https://git.xiph.org/?p=flac.git;a=commit;h=7ee908403e8be011b8a7dd3bf511408b40312c2e > https://git.xiph.org/?p=flac.git;a=commit;h=ce8a75134cace056f6c436d54b57bad1a1d93797 > > > Erik I saw. I like almost all the progress on recent patches but not this one. Can it be backed out or replaced with proper MM:SS handling? ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to completion
Josh Coalson wrote: > I haven't checked git yet but I hope this patch has not gone in. I > don't like the special case that this is creating. > > It would be better to allow MM:SS everywhere but I consider that low priority. I modified version of that patch did go in. See the following commits: https://git.xiph.org/?p=flac.git;a=commit;h=19050f74eaa1aa6d609ca065e1a854ada5bb6b4c https://git.xiph.org/?p=flac.git;a=commit;h=7ee908403e8be011b8a7dd3bf511408b40312c2e https://git.xiph.org/?p=flac.git;a=commit;h=ce8a75134cace056f6c436d54b57bad1a1d93797 Erik -- -- Erik de Castro Lopo http://www.mega-nerd.com/ ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to completion
I haven't checked git yet but I hope this patch has not gone in. I don't like the special case that this is creating. It would be better to allow MM:SS everywhere but I consider that low priority. > > From: Earl Chew >To: "flac-dev@xiph.org" >Sent: Thursday, January 5, 2012 8:27 PM >Subject: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to >completion > > >When reading the INDEX from the cue sheet, the format MM:SS:FF format is >disallowed if the sample frequency is not a multiple of 75 because the index >would only be approximate. However, 00:00:00 is _exact_ because it denotes the >start of the track, so allow it as a special case. This allows >metaflac_test.sh to pass. > > >___ >flac-dev mailing list >flac-dev@xiph.org >http://lists.xiph.org/mailman/listinfo/flac-dev > > >___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to completion
Erik de Castro Lopo wrote: > Ok, using a bit of git bisect magic, I find that if your cuesheet patch > had been applied on top of commit 19e3918d4e, then this patch: > > commit ce8a75134cace056f6c436d54b57bad1a1d93797 > Author: Erik de Castro Lopo > Date: Wed Feb 1 20:34:04 2012 +1100 > > Fix a bunch of printf format warnings. > > Broke it again. There was a minor bug in that commit which I have now fixed. Your cuesheet patch has also been commited. Thanks for your hepl with this Ear. Cheers, Erik -- -- Erik de Castro Lopo http://www.mega-nerd.com/ ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to completion
Earl Chew wrote: > Erik, > > My working view has this as the last commit: > > commit 19e3918d4e35f4ab99e7fcc58c169025c576dd16 > Author: David Schleef > Date: Thu Aug 25 18:40:29 2011 -0700 > > Fix md5 structure clearing in previous commit > > > > Using this, test_metaflac.sh runs cleanly with my cuesheet.c change. > Do you have any more insight ? Ok, using a bit of git bisect magic, I find that if your cuesheet patch had been applied on top of commit 19e3918d4e, then this patch: commit ce8a75134cace056f6c436d54b57bad1a1d93797 Author: Erik de Castro Lopo Date: Wed Feb 1 20:34:04 2012 +1100 Fix a bunch of printf format warnings. Broke it again. I'll work on this some more. Cheers, Erik -- -- Erik de Castro Lopo http://www.mega-nerd.com/ ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to completion
Erik, My working view has this as the last commit: commit 19e3918d4e35f4ab99e7fcc58c169025c576dd16 Author: David Schleef Date: Thu Aug 25 18:40:29 2011 -0700 Fix md5 structure clearing in previous commit Using this, test_metaflac.sh runs cleanly with my cuesheet.c change. I set up another view using HEAD at git.xiph.org, and in this other view test_metaflac.sh fails. With my change in place, the failure is a mismatch between information stored in the golden file. --- metaflac-test-files/case43-expect.meta 2012-02-02 08:52:57.0 -0800 +++ metaflac-test-files/out.meta 2012-02-03 11:37:13.0 -0800 @@ -10,9 +10,8 @@ METADATA block #1 type: 3 (SEEKTABLE) is last: false - length: 18 - seek points: 1 - point 0: sample_number=0 + length: 0 + seek points: 0 METADATA block #2 type: 4 (VORBIS_COMMENT) is last: false Do you have any more insight ? Earl From: Earl Chew To: "flac-dev@xiph.org" Sent: Thursday, February 2, 2012 8:50:17 AM Subject: Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to completion Erik, I'll resync and have another look. Earl From: Erik de Castro Lopo To: flac-dev@xiph.org Cc: Earl Chew Sent: Wednesday, February 1, 2012 10:30:21 PM Subject: Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to completion Earl Chew wrote: > When reading the INDEX from the cue sheet, the format MM:SS:FF format > is disallowed if the sample frequency is not a multiple of 75 because > the index would only be approximate. However, 00:00:00 is _exact_ because > it denotes the start of the track, so allow it as a special case. This > allows metaflac_test.sh to pass. Thanks for this patch Earl, but unfortunately even with this patch in place (and the exit 0 I added the script removed), test_metaflac.sh still fails with: test case43: --import-cuesheet-from... ERROR: metadata does not match expected metaflac-test-files/case43-expect.meta Would be great if you could have another look at this. Cheers, Erik -- -- Erik de Castro Lopo http://www.mega-nerd.com/___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to completion
Erik, I'll resync and have another look. Earl From: Erik de Castro Lopo To: flac-dev@xiph.org Cc: Earl Chew Sent: Wednesday, February 1, 2012 10:30:21 PM Subject: Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to completion Earl Chew wrote: > When reading the INDEX from the cue sheet, the format MM:SS:FF format > is disallowed if the sample frequency is not a multiple of 75 because > the index would only be approximate. However, 00:00:00 is _exact_ because > it denotes the start of the track, so allow it as a special case. This > allows metaflac_test.sh to pass. Thanks for this patch Earl, but unfortunately even with this patch in place (and the exit 0 I added the script removed), test_metaflac.sh still fails with: test case43: --import-cuesheet-from... ERROR: metadata does not match expected metaflac-test-files/case43-expect.meta Would be great if you could have another look at this. Cheers, Erik -- -- Erik de Castro Lopo http://www.mega-nerd.com/___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
Re: [flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to completion
Earl Chew wrote: > When reading the INDEX from the cue sheet, the format MM:SS:FF format > is disallowed if the sample frequency is not a multiple of 75 because > the index would only be approximate. However, 00:00:00 is _exact_ because > it denotes the start of the track, so allow it as a special case. This > allows metaflac_test.sh to pass. Thanks for this patch Earl, but unfortunately even with this patch in place (and the exit 0 I added the script removed), test_metaflac.sh still fails with: test case43: --import-cuesheet-from... ERROR: metadata does not match expected metaflac-test-files/case43-expect.meta Would be great if you could have another look at this. Cheers, Erik -- -- Erik de Castro Lopo http://www.mega-nerd.com/ ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
[flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to completion
When reading the INDEX from the cue sheet, the format MM:SS:FF format is disallowed if the sample frequency is not a multiple of 75 because the index would only be approximate. However, 00:00:00 is _exact_ because it denotes the start of the track, so allow it as a special case. This allows metaflac_test.sh to pass. flac.diff Description: Binary data ___ flac-dev mailing list flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
[Flac-dev] Fix cuesheet.c to allow metaflac_test.sh to run to completion
When reading the INDEX from the cue sheet, the format MM:SS:FF format is disallowed if the sample frequency is not a multiple of 75 because the index would only be approximate. However, 00:00:00 is _exact_ because it denotes the start of the track, so allow it as a special case. This allows metaflac_test.sh to pass. flac.diff Description: Binary data ___ Flac-dev mailing list Flac-dev@xiph.org http://lists.xiph.org/mailman/listinfo/flac-dev
