[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: RFR: chromaprint (Adoption)




* Simon Chopin <chopin.simon@gmail.com>, 2011-12-26, 15:19:
 http://mentors.debian.net/package/chromaprint

Alternatively, one can download the package with dget using this command:

 dget -x http://mentors.debian.net/debian/pool/main/c/chromaprint/chromaprint_0.6-1.dsc

Here's my review:

+    - The python bindings have been moved to the pyacoustid package,
+      available in Debian as python-acoustid.

s/python/Python/. I'd explicitly write here that python-chromaprint is not being built from this source package anymore.

+  * Add fpcalc binary to libchromaprint0 (Closes: #649292)

That'd be violation of Policy 8.2 if you actually did that. But in fact, the binary was added to a new binary package libchromaprint-bin. (BTW, the Policy suggests <libraryname>-tools for such package name.)

+  * Enable multiarch

Please be specific about what changes were required to enable multi-arch.

+libchromaprint.so.0 libchromaprint0 #MINVER#
+ (c++|optional)"std::vector<char, std::allocator<char> >::_M_insert_aux(__gnu_cxx::__normal_iterator<char*, std::vector<char, std::allocator<char> > >, char const&)@Base" 0.6
+ (c++|optional)"std::vector<double, std::allocator<double> >::_M_fill_insert(__gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > >, unsigned long, double const&)@Base" 0.6
+ (c++|optional)"std::vector<double, std::allocator<double> >::operator=(std::vector<double, std::allocator<double> > const&)@Base" 0.6
+ (c++|optional)"void std::__adjust_heap<__gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > >, long, double>(__gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > >, long, long, double)@Base" 0.6
+ (c++|optional)"void std::__heap_select<__gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > > >(__gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > >, __gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > >, __gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > >)@Base" 0.6
+ (c++|optional)"void std::__insertion_sort<__gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > > >(__gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > >, __gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > >)@Base" 0.6
+ (c++|optional)"void std::__introsort_loop<__gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > >, long>(__gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > >, __gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > >, long)@Base" 0.6
+ (c++|optional)"void std::__move_median_first<__gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > > >(__gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > >, __gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > >, __gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > >)@Base" 0.6

It's slightly worrisome that these symbols are exported in the first place. OTOH I don't know if there's a simple and upstreamable way to hide them. (Upstream is using cmake.)

But anyway, on i386 I get this lintian error:
E: libchromaprint0: symbols-file-contains-current-version-with-debian-revision on symbol _ZNSt6vectorIdSaIdEE14_M_fill_insertEN9__gnu_cxx17__normal_iteratorIPdS1_EEjRKd@Base and 2 others

+ chromaprint_dealloc@Base 0.6
+ chromaprint_decode_fingerprint@Base 0.6
+ chromaprint_encode_fingerprint@Base 0.6
+ chromaprint_feed@Base 0.6
+ chromaprint_finish@Base 0.6
+ chromaprint_free@Base 0.6
+ chromaprint_get_fingerprint@Base 0.6
+ chromaprint_get_raw_fingerprint@Base 0.6
+ chromaprint_get_version@Base 0.6
+ chromaprint_new@Base 0.6
+ chromaprint_start@Base 0.6

Why 0.6? As far as I can tell, there were no interface changes in this release, to this should be 0.5 or even lower.

--- chromaprint-0.5/debian/patches/use_libav_public_interfaces.patch	1970-01-01 01:00:00.000000000 +0100
+++ chromaprint-0.6/debian/patches/use_libav_public_interfaces.patch	2011-12-26 01:21:00.000000000 +0100
@@ -0,0 +1,72 @@
+Description:
+ Make use of the intended libavcodec public API instead of using bundled
+ headers and private functions.

In DEP-3, short description is obligatory (as far as I understand it).

+ #ifdef HAVE_AV_AUDIO_CONVERT
+-		convert_ctx = av_audio_convert_alloc(SAMPLE_FMT_S16, codec_ctx->channels,
+-		                                     codec_ctx->sample_fmt, codec_ctx->channels, NULL, 0);
++        printf("Conversion\n");

A stray debug print?

++        convert_ctx = av_audio_resample_init(1, 1, codec_ctx->sample_rate,
++                                             codec_ctx->sample_rate, SAMPLE_FMT_S16,
++                                             codec_ctx->channels, 16, 10, 0, 0.8);

You changed tabs to spaces (here, and in another place).

At least the following changes are not documented in the changelog:
- Addition of python-docutils to B-D; addition of manual page. (BTW, rst2man was added in docutils 0.6; please consider using versioned B-D.)
- Changes to Vcs-* fields.
- Changes to debian/copyright.
- Disabling DH_VERBOSE.

--
Jakub Wilk


Reply to: