Red Hat Bugzilla – Bug 456038
Review Request: DarkIce - Live Audio Streamer
Last modified: 2008-12-03 20:32:43 EST
SRPM URL: http://herlo.fedorapeople.org/files/darkice-0.19-1.fc9.src.rpm Spec URL: http://herlo.fedorapeople.org/files/darkice.spec Description: DarkIce is a live audio streamer. It records audio from an audio interface (e.g. sound card), encodes it and sends it to a streaming server. DarkIce can record from: * OSS audio devices * ALSA audio devices * Solaris audio interface * Jack sources * uLaw audio input through a serial interface As an aside, it supports lame and faac libs, but these are not included. This is my first package, I need a sponsor.
While working on this spec file, I was attempting to get it to build with mock. Because of this, there is a small patch that was generated as well. Thanks to ricky for the patch and ianweller for general help. Patch0: http://herlo.fedorapeople.org/files/darkice-0.19-configure.patch I've also updated the src.rpm and the spec file to match.
Hi. This is just an informal review with some comments you might find helpful. * The description line starting "DarkIce is a live ..." exceeds 79 characters and wraps around on a standard terminal, you should break this up. * Your %changelog entry version is 0.19.1-1 but your package name and version is 0.19-1 - they should be consistent. * The "GPL" license is invalid it should be "GPLv2" or "GPLv2+" for example, see https://fedoraproject.org/wiki/Licensing. * The Group "Applications/Sound and Video" does not exist in the official list in /usr/share/doc/rpm-4.4.2.3/GROUPS (or equivalent rpm version) whereas it should be one from this list according to http://docs.fedoraproject.org/drafts/rpm-guide-en/ch13s02.html (section 13.2.2.7), although I don't know how current that is. * You should have a %config in front of %{_sysconfdir}/darkice.cfg as its a configuration file (then changes to it will be preserved across rpm upgrades). * The "Requires: libogg" and "Requires: libvorbis" lines are superfluous, you should let rpm find these lib dependencies on its own. * Consider using %{name}/%{version} in the Source0 line (since you have used them in the Patch0 line)? * You use $RPM_BUILD_ROOT in the %install block but %{buildroot} in the %clean block - choose one or the other and be consistent. * Provided configuration file is broken when program run as I got the following error: DarkIce: ConfigSection.cpp:117: format missing in section icecast-0 [0] It seems inconsistent with the man page which says "format" applies to [icecast2-x] sections not [icecast-x] sections. * Not sure you need an explicit BuildRequires line for "pkgconfig", nor libogg and libvorbis (as these are pre-requisites of the equivalent -devel packages anyway).
Thank you for the review. Updated SPEC/SRPM are now available in the same location as above. I've updated and fixed all of the above issues, with the exception of the following: * Provided configuration file is broken when program run as I got the following error: DarkIce: ConfigSection.cpp:117: format missing in section icecast-0 [0] It seems inconsistent with the man page which says "format" applies to [icecast2-x] sections not [icecast-x] sections. The issue with this error is that you are attempting to use darkice to connect with an icecast 1.x.x server. In the man page, it specifically states that the [icecast-x] sections are to be used only with the lame encoder. From the man page: ---------------------------------------------------------------------------- [icecast-x] This section describes an output to an IceCast 1.3.x server or Darwin Streaming Server , while encoding with a lame encoder. --------------------------------------------------------------------------- Since Fedora doesn't support lame, twolame or faac, this support has been removed from the package. My question is, does that mean we should modify the man page to match? Or leave it in place? Cheers, Clint
Can't the lame related items be run-time detected? Would you ask upstream if they are interested in something like this or gstreamer? If it can't be done, might be worth it to patch the man page or add a README.dist file explaining what has been changed or maybe provide a macro to rebuild it.
Rahul, What do you mean 'lame related items be run-time detected'? The application can be built without the lame libraries altogether, no patch needed. Why would we want to include them? Cheers, Clint
I didn't say we would want to include them. I was thinking of a xine-lib like solution to the problem where third repos can add what is being removed here.
Rahul, Oh, if a thirdparty repo wants to build darkice-nonfree or something, its a simple matter of requiring the right libraries. The configure script would take care of that already. I have simply omitted those libraries in my build. Make more sense?
MD5Sum: 590c152cde2d62fef422f9f773560e95 darkice-0.19.tar.gz Good: * Source URL is canonical * Upstream source tarball verified * Package name conforms to the Fedora Naming Guidelines * Group Tag is from the official list * Buildroot has all required elements * All paths begin with macros * Make succeeds even when %{_smp_mflags} is defined * Files have appropriate permissions and owners * Rpmlint produces only the following warning, which can be safely ignored: darkice.x86_64: W: conffile-without-noreplace-flag /etc/darkice.cfg * Package installs and uninstalls cleanly Bad: * License should be GPLv2+, which you can fix when you import the package into CVS. +1 APPROVED, and I'll also be you sponsor.
New Package CVS Request ======================= Package Name: darkice Short Description: Live audio streamer Owners: herlo Branches: F-9, F-10 InitialCC:
cvs done.
Hey Clint, if this has been built you can close the bug. Step #13: http://fedoraproject.org/wiki/PackageMaintainers/NewPackageProcess