Radium-compressor is a standalone audio compressor for use with JACK. It features an intuitive UI using the Qt toolkit and uses Faust generated classes to handle DSP. SPEC: http://bsjones.fedorapeople.org/reviews/radium-compressor.spec SRPM: http://bsjones.fedorapeople.org/reviews/radium-compressor-0.5.1-1.fc18.src.rpm rpmlint /home/bsjones/rpmbuild/SRPMS/radium-compressor-0.5.1-1.fc18.src.rpm /home/bsjones/rpmbuild/RPMS/x86_64/radium-compressor-0.5.1-1.fc18.x86_64.rpm ~/rpmbuild/SPECS/radium-compressor.spec radium-compressor.x86_64: W: no-manual-page-for-binary radium_compressor 2 packages and 1 specfiles checked; 0 errors, 1 warnings.
For 0.5.1-1 * License - Some files are under GPLv3+, and some of them are actually used to build the binary: ----------------------------------------------------- audio/faudiostream/architecture/faust/audio/alsa-dsp.h audio/faudiostream/architecture/faust/audio/coreaudio-dsp.h audio/faudiostream/architecture/faust/audio/netjack-dsp.h audio/faudiostream/architecture/faust/audio/portaudio-dsp.h audio/faudiostream/architecture/faust/gui/console.h audio/faudiostream/architecture/faust/gui/faustqt.h audio/faudiostream/architecture/faust/gui/meta.h faudiostream/architecture/faust/audio/alsa-dsp.h faudiostream/architecture/faust/audio/coreaudio-dsp.h faudiostream/architecture/faust/audio/netjack-dsp.h faudiostream/architecture/faust/audio/portaudio-dsp.h faudiostream/architecture/faust/gui/console.h faudiostream/architecture/faust/gui/faustqt.h faudiostream/architecture/faust/gui/meta.h ----------------------------------------------------- So the license tag should be "GPLv3+" (just GPLv3+ is okay). * Compiler flags - Fedora specific compilation flags are not used correctly. build.log says: ----------------------------------------------------- 33 Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.ESDXyl 34 + umask 022 35 + cd /builddir/build/BUILD 36 + cd radium_compressor-0.5.1 37 + make 'OPTS=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i686 -mtune=atom -fasynchronous-unwind-tables' -j5 38 cd Qt && ./create_source_from_ui.sh `../find_moc_and_uic_paths.sh uic` `../find_moc_and_uic_paths.sh moc` compressor_widget 39 g++ -DDEBUG -O3 -Wall -msse -mfpmath=sse -DUSE_QT_REQTYPE -DUSE_QT4 -g -I. -ffast-math -IQt Qt/Qt_SliderPainter.cpp `pkg-config --cflags Qt3Support` -c 40 g++ -DDEBUG -O3 -Wall -msse -mfpmath=sse -DUSE_QT_REQTYPE -DUSE_QT4 -g -I. -ffast-math -IQt main.cpp Qt_SliderPainter.o -Iaudio/faudiostream/architecture/ `pkg-config --libs --cflags Qt3Support` -ljack -o radium_compressor 41 + exit 0 ----------------------------------------------------- Also, "-O3" is used, this should be changed to -O2. And, is it safe for this package to use "-ffast-math"? -ffast-math does some "optimization" which breaks IEEE rules for example, which may result in confusing behavior. * rpmlint issue - incorrect-fsf-address (not a blocker) - Some source files contain incorrest FSF address, which causes rpmlint complaint on debuginfo rpm. Not a blocker, however it is recommend to tell upstream - spurious-executable-perm - rpmlint complains about some header files in debuginfo rpm for having non-proper permission. You can suppress this rpmlint complaint by changing the permission of header files to 0644 at %prep. * Desktop file https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Desktop_files - It seems that "radium_compressor" is a GUI program, so installing proper desktop file is required (unless there is some reason, e.g. this program usually needs proper argument, for example)
As far as I can read from http://gcc.gnu.org/onlinedocs/gcc-4.7.0/gcc/Optimize-Options.html, -ffast-math should be safe, and benchmarking showed that -ffast-math makes the program run 13% faster. (I'm the upstream author)
Oh, I also see that -O3 makes the program run twice as fast as -O2 (at least on my computer): make benchmark && ./benchmark. It's -fpredictive-commoning and -ftree-vectorize that makes the difference. "-O2 -fpredictive-commoning -ftree-vectorize" runs at the same speed as "-O3".
(In reply to comment #3) > Oh, I also see that -O3 makes the program run twice as fast as -O2 (at least > on my computer): make benchmark && ./benchmark. It's -fpredictive-commoning > and -ftree-vectorize that makes the difference. "-O2 -fpredictive-commoning > -ftree-vectorize" runs at the same speed as "-O3". Well, -O2 vs -O3 is rather Fedora packaging (policy) issue. I can accept "-O2 -fpredictive-commoning -ftree-vectorize" instead.
(In reply to comment #3) > Oh, I also see that -O3 makes the program run twice as fast as -O2 (at least > on my computer): make benchmark && ./benchmark. It's -fpredictive-commoning > and -ftree-vectorize that makes the difference. "-O2 -fpredictive-commoning > -ftree-vectorize" runs at the same speed as "-O3". Oh, https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Compiler_flags says "Overriding these flags for performance optimizations (for instance, -O3 instead of -O2) is generally discouraged. If you can present benchmarks that show a significant speedup for this particular code, this could be revisited on a case-by-case basis. " So as you kindly presented benckmarks, I can accept -O3 and -ffast-math. Still other Fedora specific compilation flags (such as -Wp,-D_FORTIFY_SOURCE=2 and so on) must be honored.
Thanks guys for the throguh investigation of the compiler flags. Really appreciated. Kjetil: there are some incorrect FSF addresses in your source headers (correct is 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA). You can update this at your disgression License has been ajusted and file permissions addressed. I have added the desktop file and will liase with upstream regarding a suitable icon. SPEC: http://bsjones.fedorapeople.org/reviews/radium-compressor.spec SRPM: http://bsjones.fedorapeople.org/reviews/radium-compressor-0.5.1-2.fc18.src.rpm
Some notes: * Optimization level - For changing optimization level, please add some comments on your spec file about the discussion on this review request (especially the comments / explanation from Kjetil) for justification. ! Honoring Fedora specific compilation flags - Well, it seems somewhat strange that you have to use sed for Makefile and also change "OPTS" variables to honor Fedora specific compilation flags, however not a blocker. ------------------------------------------------------------- This package (radium-compressor) is APPROVED by mtasaka -------------------------------------------------------------
(In reply to comment #7) > Some notes: > > * Optimization level > - For changing optimization level, please add some comments on > your spec file about the discussion on this review request > (especially the comments / explanation from Kjetil) > for justification. Good call - will do. > > ! Honoring Fedora specific compilation flags > - Well, it seems somewhat strange that you have to use sed for Makefile > and also change "OPTS" variables to honor Fedora specific compilation > flags, however not a blocker. > I think this is a feature in the build script. I'll send a patch upstream. Thanks for the review!
New Package SCM Request ======================= Package Name: radium-compressor Short Description: An audio compressor for JACK Owners: bsjones Branches: f17 f18
Git done (by process-git-requests).
radium-compressor-0.5.1-3.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/radium-compressor-0.5.1-3.fc17
radium-compressor-0.5.1-3.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/radium-compressor-0.5.1-3.fc18
radium-compressor-0.5.1-3.fc17 has been pushed to the Fedora 17 testing repository.
radium-compressor-0.5.1-4.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/radium-compressor-0.5.1-4.fc17
radium-compressor-0.5.1-4.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/radium-compressor-0.5.1-4.fc18
Closing.
radium-compressor-0.5.1-4.fc17 has been pushed to the Fedora 17 stable repository. If problems still persist, please make note of it in this bug report.
radium-compressor-0.5.1-4.fc18 has been pushed to the Fedora 18 stable repository. If problems still persist, please make note of it in this bug report.