[Bug 118] Review Request: cinelerra-cv - Advanced audio and video capturing, compositing, and editing

http://bugzilla.rpmfusion.org/show_bug.cgi?id=118 Hans de Goede <j.w.r.degoede(a)gmail.com&gt; changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |3 Status|NEW |ASSIGNED --- Comment #29 from Hans de Goede <j.w.r.degoede(a)gmail.com&gt; 2010-12-24 14:27:47 --- (In reply to comment #28) Good! I just took a quick look, I notice that you use a git snapshot, but you don't provide instructions on how to generate the tarbal, making it impossible to verify if the sources in the SRPM match upstream, which is a must. I also noticed that CinelerraCV 2.1.5 is out (and from a later date then your git snapshot), so perhaps you can upgrade to that rather then using a git snapshot? That will also fix my verify problem. Other then that from a quick glance everything looks good. Some remarks about the shmmax tweaking though: This: if [ $(grep kernel.shmmax %{_sysconfdir}/sysctl.conf |wc -l) == 0 ] ; then Can be done simpler as this: if ! grep -q kernel.shmmax %{_sysconfdir}/sysctl.conf; then I also wonder if differentiating between the runtime and boottime config with the 2 checks is a good idea. If the user has already set a shmmax in his sysctl.conf, I agree we should not overwrite it, but if it is too low, this means cinelerra will work directly after install, but not after the first boot. Would it not be better to remove this: if [ $(/sbin/sysctl -n -q kernel.shmmax) -lt 2147483647 ] ; then # Hack for the package to work "out of the box". /sbin/sysctl -q -w kernel.shmmax=0x7fffffff fi And change the second if block to: if ! grep -q kernel.shmmax %{_sysconfdir}/sysctl.conf; then # Hack for the package to work "out of the box". /sbin/sysctl -q -w kernel.shmmax=0x7fffffff # Hack for the package to work on reboot. echo -e "\n# Added by Cinelerra RPM from RPM Fusion\nkernel.shmmax = 0x7fffffff" >> %{_sysconfdir}/sysctl.conf fi This way either shmmax is left completely unchanged, or it is set to the same value directly after install as it will be after a reboot. I guess this should be combined with a small sh wrapper script for cinelerra-cv which does the "if [ $(/sbin/sysctl -n -q kernel.shmmax) -lt 2147483647 ] ; then" check, and if there is not enough shm, then prints an error using zenity --error --text="..." and exits, or does cinelerra already checks for this itself ? I hope to see a package updated to 2.1.5 from you soon, and then I'll do a full review. Regards, Hans -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug.