Hi Vector, I am not lawyer here :) . My understanding is that most of the distributions d'nt ship any mp3 encoder/decoder, as part of the base distribution due to licensing issues. You might want to double-check on this.
thanks, Pradhap.D Paul Cunningham wrote: > Vector, > > See below for my comments ... > > Paul > > Vector Li wrote: >> I am porting "mpg123", which is a fast console MPEG audio player. >> Would you please have a code review for me? >> webrev at: >> http://cr.opensolaris.org/~vector/mpg123/ > > === Start of Comments === > > 1. usr/src/cmd/mpg123/METADATA > Change as follows ... > 1 NAME: mpg123 High Performance MPEG Audio Player > 12 COMMENTS: > > 2. usr/src/cmd/Makefile > & usr/src/pkgdefs/Makefile > Try to add stuff alphabetically > > 3. usr/src/cmd/mpg123/install-sfw > Replace ... > 46 . ${SRC}/tools/install.subr > with ... > source ${SRC}/tools/install.subr > > You could have put these dirs ... > 43 [[ -d ${MPG123LIBDIR} ]] || mkdir -m 0755 ${MPG123LIBDIR} > 44 [[ -d ${PKGCONFIGDIR} ]] || mkdir -m 0755 ${PKGCONFIGDIR} > in Targetdirs > > Do these need the 777 ... > 64 _install L libmpg123.so.0.11.2 libmpg123.so.0 777 > 65 _install L libmpg123.so.0.11.2 libmpg123.so 777 > > 4. usr/src/cmd/mpg123/sunman-stability > Does this need the CDDL HEAD stuff, its not normal to have it? > > 5. CDDL HEADER and file tops in various files > Try to make them conform to those in ... > "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/" > > > 6. usr/src/pkgdefs/SUNWmpg123/copyright > Does this need the sun disclaimer and package src copyright > stuff added at top see .. > "http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWmeld/copyright" > > > > 7. usr/src/pkgdefs/SUNWmpg123/depend > This looks default 'depend', have you checked you have > no other dependencies. If not delete this file and add > 'DATAFILES = depend' into SUNWmpg123/Makefile. Otherwise add > other dependencies here > > 8. usr/src/pkgdefs/SUNWmpg123/pkginfo.tmpl > Change DESC= line to ... > "mpg123 - Fast console MPEG Audio Player and decoder library (1.6.2)" > > === End of Comments =====