Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=456038
Summary: Review Request: DarkIce - Live Audio Streamer Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: low Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: herlo1@gmail.com QAContact: extras-qa@fedoraproject.org CC: fedora-package-review@redhat.com,notting@redhat.com
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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: DarkIce - Live Audio Streamer
https://bugzilla.redhat.com/show_bug.cgi?id=456038
herlo1@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- OtherBugsDependingO| |177841 nThis| |
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: DarkIce - Live Audio Streamer
https://bugzilla.redhat.com/show_bug.cgi?id=456038
------- Additional Comments From herlo1@gmail.com 2008-07-21 02:15 EST ------- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: DarkIce - Live Audio Streamer
https://bugzilla.redhat.com/show_bug.cgi?id=456038
------- Additional Comments From timc@inf.ed.ac.uk 2008-07-25 05:20 EST ------- 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).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: DarkIce - Live Audio Streamer
https://bugzilla.redhat.com/show_bug.cgi?id=456038
------- Additional Comments From herlo1@gmail.com 2008-07-25 15:44 EST ------- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=456038
Rahul Sundaram sundaram@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |sundaram@redhat.com
--- Comment #4 from Rahul Sundaram sundaram@redhat.com 2008-08-14 22:43:38 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=456038
--- Comment #5 from Clint Savage herlo1@gmail.com 2008-08-14 23:48:15 EDT --- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=456038
--- Comment #6 from Rahul Sundaram sundaram@redhat.com 2008-08-15 00:01:33 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=456038
--- Comment #7 from Clint Savage herlo1@gmail.com 2008-08-15 00:13:12 EDT --- 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?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=456038
Itamar Reis Peixoto itamar@ispbrasil.com.br changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |itamar@ispbrasil.com.br Alias| |DarkIce
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=456038
Brian Pepple bdpepple@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |bdpepple@gmail.com AssignedTo|nobody@fedoraproject.org |bdpepple@gmail.com Flag| |fedora-review+
--- Comment #8 from Brian Pepple bdpepple@gmail.com 2008-10-12 00:20:49 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=456038
Brian Pepple bdpepple@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Blocks|177841 |
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=456038
Clint Savage herlo1@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #9 from Clint Savage herlo1@gmail.com 2008-10-15 17:49:46 EDT --- New Package CVS Request ======================= Package Name: darkice Short Description: Live audio streamer Owners: herlo Branches: F-9, F-10 InitialCC:
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=456038
Kevin Fenzi kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
--- Comment #10 from Kevin Fenzi kevin@tummy.com 2008-10-15 18:02:13 EDT --- cvs done.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=456038
Brian Pepple bdpepple@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |needinfo?(herlo1@gmail.com)
--- Comment #11 from Brian Pepple bdpepple@gmail.com 2008-12-03 20:09:20 EDT --- Hey Clint, if this has been built you can close the bug.
Step #13: http://fedoraproject.org/wiki/PackageMaintainers/NewPackageProcess
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=456038
Clint Savage herlo1@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |VERIFIED
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=456038
Clint Savage herlo1@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|VERIFIED |CLOSED Resolution| |NEXTRELEASE Flag|needinfo?(herlo1@gmail.com) |
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla