Re: New Package Proposal: File_Ogg

On 27 Aug 2003 at 21:19, David Grant wrote: > This is a proposal for File_Ogg, a PEAR group of classes for accessing > media information within an Ogg physical bitstream, to be added to > PEAR. > > Sources can be found at: > > http://jalapeno.dyndns.org/File_Ogg/ > > together with the packaged source and a package definition file. > > File_Ogg is packaged with File_Ogg_Vorbis_PEAR (for an explanation of > the PEAR suffix, check this mailing list for previous comments by > myself and others on this package), which is a class for accessing > Vorbis logical streams. It is anticipated that a number of other > logical streams will become accessible in the near future. > > Comments and +1s (in that order) are very welcome. Hi David, as discussed earlier I like the idea very much. And I very much appreciate the new, open concept - e.g. also providing a pecl- interface. But should you really distribute an empty pecl-file and require_once it even if it hasn't anything in it's body? Maybe think that over again before doing a release. Maybe the require_once-statements could be done inside the if- statement to load the appropriate files only when needed? In your comments at the beginning of each file there is: // $Id Do you intend to put this file in cvs? This tag is only there for cvs- purpose. It needs to be $Id$ to be correct. Then having a look at Ogg/PEAR.phps: As discussed earlier also, shouldn't probably the streamnames be lowercase alltogether? Have a look at flac? If you don't do it lowercase (which I think might be a bad idea - but looks better when you want to display it to the user) you should have written Vorbis insteand of vorbis because that's the way xiph.org writes it afaik. theora maybe the same? Why does speex have that many whitespaces in the name? In listStreams() you mention the "stream serial number". In getStream it's called $streamSerial - but in listStreams that's $stream_id. Maybe rename for consistency? Looking at File_Ogg/Ogg/Vorbis/PEAR.phps: Is $_avgBitrate really an integer as you state? Or could it return fractions and therefor be a double? But these are only a few minor things that I found. Altogether: Well done! If you iron out these few things I'm +1 for File_Ogg. Stefan