Bug 454960 - Review Request: aubio - An audio labelling library
Review Request: aubio - An audio labelling library
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Hans de Goede
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-07-10 22:11 EDT by Anthony Green
Modified: 2008-07-26 18:57 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-07-14 00:45:03 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
hdegoede: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Anthony Green 2008-07-10 22:11:39 EDT
Spec URL: http://spindazzle.org/Fedora/aubio.spec
SRPM URL: http://spindazzle.org/Fedora/aubio-0.3.2-1.fc9.src.rpm
Description: 

aubio is a library for audio labelling. Its features include
segmenting a sound file before each of its attacks, performing pitch
detection, tapping the beat and producing midi streams from live
audio. The name aubio comes from 'audio' with a typo: several
transcription errors are likely to be found in the results too.

The aim of this project is to provide these automatic labelling
features to other audio softwares. Functions can be used offline in
sound editors and software samplers, or online in audio effects and
virtual instruments.


NOTE: ardour 2.5 requires this library.
Comment 1 Hans de Goede 2008-07-11 02:10:12 EDT
I think I can make some time to review this, but the URL's you've posted are
404, and my psychic powers to guess the correct URL are failing me.
Comment 2 Anthony Green 2008-07-11 08:15:17 EDT
(In reply to comment #1)
> I think I can make some time to review this, but the URL's you've posted are
> 404, and my psychic powers to guess the correct URL are failing me.
> 

Bah!  Try again.  My system last night crashed before the ftp upload was
complete.  (dbus deamon segv)

Thanks!
Comment 3 Hans de Goede 2008-07-13 03:55:21 EDT
Initial review done, needs some work:

* License: GPL, GPL is not a valid license tag, it should be "GPLv2+"
* Gives the following errors when doing a local build:
RPM build errors:
    Installed (but unpackaged) file(s) found:
   /usr/lib/python2.5/site-packages/aubio/__init__.py
   /usr/lib/python2.5/site-packages/aubio/__init__.pyc
   /usr/lib/python2.5/site-packages/aubio/__init__.pyo
   /usr/lib/python2.5/site-packages/aubio/_aubiowrapper.so
   /usr/lib/python2.5/site-packages/aubio/aubioclass.py
   /usr/lib/python2.5/site-packages/aubio/aubioclass.pyc
   /usr/lib/python2.5/site-packages/aubio/aubioclass.pyo
   <snip>

  It looks like aubio comes with python bindings, it would probably be best to
  add the necessary BR's and put the python stuff in a sub-package.

* rpmlint says:
  aubio.src: W: mixed-use-of-spaces-and-tabs (spaces: line 31, tab: line 1)
  aubio.x86_64: E: zero-length /usr/share/doc/aubio-0.3.2/NEWS
  aubio.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/libaubioext.so.2.1.1
['/usr/lib64']
  aubio.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/aubiotrack ['/usr/lib64']
  aubio.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/aubioonset ['/usr/lib64']
  aubio.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/aubionotes ['/usr/lib64']
  aubio.x86_64: E: binary-or-shlib-defines-rpath /usr/lib/python2.5/site-packages/

  You can fix this by putting the following 3 lines between %configure and make:
# Don't use rpath!
sed -i 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' libtool
sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool
Comment 4 Hans de Goede 2008-07-13 03:57:05 EDT
When I said you can fix this by ..., I meant you can fix the rpath stuff by ...,
the other 2 rpmlint issues need seperate fixing.
Comment 5 Anthony Green 2008-07-13 10:49:15 EDT
New files:

Spec URL: http://spindazzle.org/Fedora/aubio.spec
SRPM URL: http://spindazzle.org/Fedora/aubio-0.3.2-2.fc9.src.rpm

(In reply to comment #3)
> Initial review done, needs some work:
> 
> * License: GPL, GPL is not a valid license tag, it should be "GPLv2+"

Fixed.

> * Gives the following errors when doing a local build:
> RPM build errors:

I think this is because you have swig installed.  I've BuildRequired swig and
created a subpackage for the python bindings.

>     Installed (but unpackaged) file(s) found:
>    /usr/lib/python2.5/site-packages/aubio/__init__.py
>    /usr/lib/python2.5/site-packages/aubio/__init__.pyc
>    /usr/lib/python2.5/site-packages/aubio/__init__.pyo
>    /usr/lib/python2.5/site-packages/aubio/_aubiowrapper.so
>    /usr/lib/python2.5/site-packages/aubio/aubioclass.py
>    /usr/lib/python2.5/site-packages/aubio/aubioclass.pyc
>    /usr/lib/python2.5/site-packages/aubio/aubioclass.pyo
>    <snip>
> 
>   It looks like aubio comes with python bindings, it would probably be best to
>   add the necessary BR's and put the python stuff in a sub-package.
> 
> * rpmlint says:
>   aubio.src: W: mixed-use-of-spaces-and-tabs (spaces: line 31, tab: line 1)
>   aubio.x86_64: E: zero-length /usr/share/doc/aubio-0.3.2/NEWS
>   aubio.x86_64: E: binary-or-shlib-defines-rpath /usr/lib64/libaubioext.so.2.1.1
> ['/usr/lib64']
>   aubio.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/aubiotrack
['/usr/lib64']
>   aubio.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/aubioonset
['/usr/lib64']
>   aubio.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/aubionotes
['/usr/lib64']
>   aubio.x86_64: E: binary-or-shlib-defines-rpath /usr/lib/python2.5/site-packages/

All fixed.

Thanks Hans!

AG




>   You can fix this by putting the following 3 lines between %configure and make:
> # Don't use rpath!
> sed -i 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' libtool
> sed -i 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' libtool
> 

Comment 6 Hans de Goede 2008-07-13 15:06:50 EDT
I can still not approve this I'm afraid, it now fails to build on my system with:
RPM build errors:
    File not found by glob:
/var/tmp/aubio-0.3.2-2.fc10-root-hans/usr/lib64/python2.5/site-packages/aubio/*.so
    Installed (but unpackaged) file(s) found:
   /usr/lib/python2.5/site-packages/aubio/_aubiowrapper.so

Notice the lib64 in the path it is looking for, which is correct, you use 
%{python_sitearch}/%{name}/*.so

and sitearch is in lib64 on x86_64, and looking for the .so in sitearch is
correct, that is where it should be. So appearantly the aubio makefile gets this
wrong.

Note that it is allowed to put the entire python stuff in sitearch if it depends
upon a .so, so you could see if you can override the makefile install location
for the python stuff and just put it all in sitearch.

Also note that currently in your %files section, the %{python_sitearch}/%{name}
directory will be unowned when different from %{python_sitelib}/%{name}

Hmm, thinking some more about this, it would really be the cleanest to put all
of the python stuff in sitearch as it depends on a .so, if you cannot persuade
the makefile todo this you could use something like this after "make install":
if [ %{python_sitearch} != %{python_sitelib} ]; then
  mkdir -p $RPM_BUILD_ROOT%{python_sitearch}
  mv $RPM_BUILD_ROOT%{python_sitelib}/%{name} $RPM_BUILD_ROOT%{python_sitearch}
fi

This will also greatly simplify the %files for the -python package, that can
then become plain and simple:

%files python
%defattr(-,root,root,-)
%{python_sitearch}/%{name}
Comment 7 Anthony Green 2008-07-13 15:30:12 EDT
Thanks for your comments Hans.  Here we go again...

Spec URL: http://spindazzle.org/Fedora/aubio.spec
SRPM URL: http://spindazzle.org/Fedora/aubio-0.3.2-3.fc9.src.rpm

AG
Comment 8 Hans de Goede 2008-07-13 16:59:21 EDT
Looks good now, approved!
Comment 9 Anthony Green 2008-07-13 17:14:03 EDT
Thanks!

New Package CVS Request
=======================
Package Name: aubio
Short Description: An audio labelling library
Owners: green
Branches: devel
InitialCC:
Cvsextras Commits: yes
Comment 10 Kevin Fenzi 2008-07-13 23:01:59 EDT
cvs done.
Comment 11 Anthony Green 2008-07-14 04:04:40 EDT
Package Change Request
======================
Package Name: aubio
New Branches: F-9
Comment 12 Kevin Fenzi 2008-07-14 12:07:20 EDT
cvs done.
Comment 13 Hans de Goede 2008-07-26 18:31:17 EDT
Anthony,

I would be much oblidged if you could review bug 456772 in return, its a really
easy review and I need to get this into place so that I can continue my work on
the F-10 better webcam support feature:
https://fedoraproject.org/wiki/Features/BetterWebcamSupport

Thanks!
Comment 14 Hans de Goede 2008-07-26 18:57:16 EDT
Never mind, already reviewed :)

Note You need to log in before you can comment on or make changes to this bug.