Bug 222248
Summary: | Review Request: alsa-plugins - backend plugins for alsa sound system | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Eric Moret <eric.moret> |
Component: | Package Review | Assignee: | Matthias Clasen <mclasen> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | bnocera, lpoetter, mcepl, mclasen |
Target Milestone: | --- | Flags: | mclasen:
fedora-review+
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2007-08-08 06:11:24 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Attachments: |
Description
Eric Moret
2007-01-11 01:02:52 UTC
Created attachment 145302 [details]
alsa-plugins spec file
Okay, no problem, I'll try to check it this week. Created attachment 145932 [details]
an updated spec file
There's an updated spec file, I've fixed some minor problems and updated it to rc2. But I can't sponsor you because I don't have any package in extras. Created attachment 146155 [details]
New spec file
There's a problem in your spec file: Checking for unpackaged file(s): /usr/lib/rpm/check-files /var/tmp/alsa-plugins-1.0.14-0.1.rc2-root-komat error: Installed (but unpackaged) file(s) found: /usr/lib/alsa-lib/libasound_module_ctl_dsp_ctl.so /usr/lib/alsa-lib/libasound_module_pcm_alsa_dsp.so RPM build errors: Installed (but unpackaged) file(s) found: /usr/lib/alsa-lib/libasound_module_ctl_dsp_ctl.so /usr/lib/alsa-lib/libasound_module_pcm_alsa_dsp.so Created attachment 146329 [details]
Fixing missing dbus-devel dependency and adding new package alsa-plugins-maemo
I have created a new package including the unpackaged files called
alsa-plugins-memo. Also I have added Requires tags based on the BuildRequires
libraries but there may be a better way to figure out what are the runtime
dependencies and I may be missing more.
(In reply to comment #7) > Created an attachment (id=146329) [edit] > Fixing missing dbus-devel dependency and adding new package alsa-plugins-maemo This one looks fine. Experimental packages with this SPEC (which is at http://www.ceplovi.cz/matej/progs/rpms/alsa-plugins/alsa-plugins.spec), are at http://www.ceplovi.cz/matej/progs/rpms/alsa-plugins/ and SRPM is http://www.ceplovi.cz/matej/progs/rpms/alsa-plugins/alsa-plugins-1.0.14-0.1.rc2.src.rpm The packages work fine for me after adding the necessary configuration. Is there any way that we can make that configuration come with the rpm ? (In reply to comment #10) > The packages work fine for me after adding the necessary configuration. > Is there any way that we can make that configuration come with the rpm ? What configuration you have on mind? /etc/asound.conf? I am just working on making it work on my notebook and I am not done yet. Moreover, I am not sure where it should go -- here on in the pulseaudio? Created attachment 147713 [details]
ALSA system configuration
This is probably not the right package (and right bug) to attach it to, but it
certainly fits into whole complex of making Pulseaudio working. This makes ALSA
using Pulseaudio, so that any application using ALSA will go through PA. The
first part is hardware specific for my Intel ICH6 audio chip (kernel module
snd_intel8x0), but the rest should be fairly general.
sorry, better explanation -- no, this is probably the right bug to talk about it, but I am not sure, whether this file should be installed by this package (alsa-plugins could be used by sound systems and applications other than PA). Should it be part of PA package itself? Ok, i sponsored Eric so we can get this in. Ideally the configuration should be specific to each plugin package. ie for alsa-plugins-pulseaudio, the following should be appended to /etc/asound.conf: pcm.pulse { type pulse } ctl.pulse { type pulse } This can be done easily in the %postin script but how do you cleanly remove those specific configuration entries from asound.conf in the %postun script? You can include configuration files, see /etc/alsa/alsa.conf @hooks [ { func load files [ "/etc/asound.conf" "~/.asoundrc" ] errors false } ] One more question, the a52 plugin requires ffmpeg to build. ffmpeg is not part of core or extra, it is in the livna repository. Should I still package that plugin? (In reply to comment #17) > One more question, the a52 plugin requires ffmpeg to build. ffmpeg is not part > of core or extra, it is in the livna repository. Should I still package that > plugin? No, it shouldn't be packaged (at least not in Fedora). (In reply to comment #17) > One more question, the a52 plugin requires ffmpeg to build. ffmpeg is not part > of core or extra, it is in the livna repository. Should I still package that > plugin? I tried to find out whether it is possible to switch compilation of this one plugin, but it would be probably issue of changing Makefile.am/C-code stuff. OK, I've removed the a52 package and included basic configuration files. The configuration files are in /etc/alsa/pcm which may or may not be the right place. Do I need to do something else besides dumping them here? In any event I'm interrested in your feedback. Does it work? What package have you tested? ftp://zouric.com/public/alsa-plugins/ Created attachment 148174 [details]
Adding configuration files and removing a52 package
Created attachment 149908 [details]
ugly hack emphasizing problem with the package
Packages from zouric.com don't even rpmbuild for me on x86_64. The problem is
that option --without-a52 does exactly nothing for me (it is strange that it
worked for the reporter, but it really doesn't for me). With the attached patch
(which is of course unacceptable in FE) it builds. Will try to investigate some
more proper solution.
(In reply to comment #22) > Packages from zouric.com don't even rpmbuild for me on x86_64. The problem is > that option --without-a52 does exactly nothing for me (it is strange that it > worked for the reporter, but it really doesn't for me). With the attached patch > (which is of course unacceptable in FE) it builds. Will try to investigate some > more proper solution. OK, I've asked my colleague (it's nice to be in an office with somebody who is an autoconf upstream maintainer ;-)) to find out what's going on. The first one is -- there is no option --without-a52 in ./configure, so running ./configure with this option does exactly nothing. I have made new release of the package and it is available on SPEC: http://matej.ceplovi.cz/progs/rpms/alsa-plugins/alsa-plugins.spec SRPMS: http://matej.ceplovi.cz/progs/rpms/alsa-plugins/alsa-plugins-1.0.14-0.3.rc2.src.rpm any news here? will be this package included in extras? Adding Lennart to the CC:. As those bits are necessary to get PulseAudio running, I'd gather he would be the best person to handle them. As said earlier, I need a sponsor. Until then, I can't add this to Fedora. Have you asked on the mailing list for a sponsor ? That might help. Now that I am sponsored, I believe I should get this package reviewed but I can't seem to be able to change its status to REVIEW REQUEST. Should I open a review request from scratch and ask for an official review there? Is this what you want? (In reply to comment #29) > Is this what you want? Yes, thank you. BTW, I was trying to access your source rpm at http://matej.ceplovi.cz/progs/rpms/alsa-plugins/alsa-plugins-1.0.14-0.3.rc2.src.rpm but the link seems to be dead. Would you mind to bring it backup. I'd like to update to latest upstream. Still waiting for an official review! Created attachment 159824 [details] Updated spec to latest upstream version Updating to latest upstream version: ftp://zouric.com/public/alsa-plugins/alsa-plugins.spec ftp://zouric.com/public/alsa-plugins/alsa-plugins-1.0.14-1.src.rpm Bastien, I am happy if Eric maintains this package. I am not directly involved upstream with this code, so I'd prefer to stay a specatator on this one. This is a really confusing ticket. It's under review, but blocking FE-NEW, which I'll fix. mcepl set the fedora-review flag, but it's assigned to stransky. And I'm not sure I've seen a ticket in the "POST" state before. So, who's reviewing? And what does "POST" mean in the context of a review? Created attachment 160236 [details]
New spec file with added dist tag
Added dist tag: ftp://zouric.com/public/alsa-plugins/alsa-plugins.spec ftp://zouric.com/public/alsa-plugins/alsa-plugins-1.0.14-1.fc7.src.rpm The above questions regarding "POST" state and reviewer still stand. Hmm, why this -maemo package? Has anyone ported Fedora to the N800 now? Ok, I'm going to take this review over, since nobody else seems to be serious about this. We really want alsa-plugins-pulseaudio in F8... Initial comments: - I second Lennarts comment about maemo - is it useful to package that plugin without any other part of maemo in fedora ? - OSSO DSP SW <--> ALSA DSP plugin ======================== Don't put ascii formatting into the %description, please. - BuildRequires are per source package, there is no reason to put them into the subpackage sections (of course, it doesn't hurt either) - Requires: alsa-lib is unnecessary, since library dependencies pull it in anyway - The license tag needs some work; GPL is no longer a valid value for that field, it should probably be GPLv2+. But some of the plugins appear to be LGPLv2+, so maybe it would be better to put license tags in the subpackages according to their actual licenses. In fact, all but the samplerate plugin appear to be LGPL, not GPL. - What is the touching in %prep about ? (In reply to comment #38) > Initial comments: > > - I second Lennarts comment about maemo - is it useful to package that plugin > without any other part of maemo in fedora ? Removed Maemo > - OSSO DSP SW <--> ALSA DSP plugin > ======================== > Don't put ascii formatting into the %description, please. Fixed > - BuildRequires are per source package, there is no reason to put them > into the subpackage sections (of course, it doesn't hurt either) I did it this way so build dependencies would not be forgotten when a package would be removed, like above for the maemo plugin. > - Requires: alsa-lib > is unnecessary, since library dependencies pull it in anyway Fixed > - The license tag needs some work; GPL is no longer a valid value for > that field, it should probably be GPLv2+. But some of the plugins > appear to be LGPLv2+, so maybe it would be better to put license > tags in the subpackages according to their actual licenses. In fact, > all but the samplerate plugin appear to be LGPL, not GPL. Tried to address it but the global License field seems to be mandatory. > - What is the touching in %prep about ? Not sure where this came from. I removed it. Created attachment 160836 [details]
Updated spec file following initial review
rpmlint is silent on all generated package, good. package name: ok spec name: ok packaging guidelines: ok license: ok license field: ok license file: ok spec language: ok spec legible: yes upstream source: ok buildable: yes ExludeArch: n/a build deps: ok shared libs: n/a relocatable: n/a directory ownership: ok duplicate files: ok permissions: ok %clean: ok macro use: consistent content: permissible large docs: n/a %doc: ok headers: n/a static libs: n/a pkgconfig files: n/a shared lib symlinks: n/a devel package: n/a libtool archives: ok gui apps: n/a directory ownership: ok %install: ok utf8 filenames: ok Looking good. Approved. New Package CVS Request ======================= Package Name: alsa-plugins Short Description: backend plugins for alsa sound system Owners: eric.moret Branches: F-7 InitialCC: cvs done. |