Bug 222248 - Review Request: alsa-plugins - backend plugins for alsa sound system
Review Request: alsa-plugins - backend plugins for alsa sound system
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Matthias Clasen
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-10 20:02 EST by Eric Moret
Modified: 2007-11-30 17:11 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-08-08 02:11:24 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mclasen: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)
alsa-plugins spec file (5.21 KB, text/plain)
2007-01-10 20:02 EST, Eric Moret
no flags Details
an updated spec file (5.63 KB, text/plain)
2007-01-18 12:24 EST, Martin Stransky
no flags Details
New spec file (4.86 KB, text/plain)
2007-01-22 03:05 EST, Eric Moret
no flags Details
Fixing missing dbus-devel dependency and adding new package alsa-plugins-maemo (5.73 KB, text/plain)
2007-01-23 13:31 EST, Eric Moret
no flags Details
ALSA system configuration (335 bytes, text/plain)
2007-02-08 17:54 EST, Matěj Cepl
no flags Details
Adding configuration files and removing a52 package (6.11 KB, text/plain)
2007-02-16 02:03 EST, Eric Moret
no flags Details
ugly hack emphasizing problem with the package (456 bytes, patch)
2007-03-13 04:49 EDT, Matěj Cepl
no flags Details | Diff
Updated spec to latest upstream version (6.30 KB, text/plain)
2007-07-23 19:49 EDT, Eric Moret
no flags Details
New spec file with added dist tag (6.31 KB, text/plain)
2007-07-30 07:39 EDT, Eric Moret
no flags Details
Updated spec file following initial review (5.90 KB, text/plain)
2007-08-07 14:25 EDT, Eric Moret
no flags Details

  None (edit)
Description Eric Moret 2007-01-10 20:02:52 EST
With pulseaudio being introduced in fedora as a replacement for esd, some
applications can't work without backend plugins. Among those backend plugins is
a alsa to pulseaudio output plugin that is part of alsa-plugins currently
unpackaged.

I would like to add that package to fedora extra but I am currently not a
maintainer of any package. I guess I would need sponsorship and probably a bit
of mentoring help with the attached spec file.
Comment 1 Eric Moret 2007-01-10 20:02:52 EST
Created attachment 145302 [details]
alsa-plugins spec file
Comment 2 Martin Stransky 2007-01-15 08:02:47 EST
Okay, no problem, I'll try to check it this week.
Comment 3 Martin Stransky 2007-01-18 12:24:41 EST
Created attachment 145932 [details]
an updated spec file
Comment 4 Martin Stransky 2007-01-18 12:26:31 EST
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.
Comment 5 Eric Moret 2007-01-22 03:05:13 EST
Created attachment 146155 [details]
New spec file
Comment 6 Martin Stransky 2007-01-22 05:03:36 EST
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
Comment 7 Eric Moret 2007-01-23 13:31:13 EST
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.
Comment 8 Martin Stransky 2007-01-25 12:21:12 EST
(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.

Comment 10 Matthias Clasen 2007-02-08 14:30:36 EST
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 ?
Comment 11 Matěj Cepl 2007-02-08 17:05:15 EST
(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?
Comment 12 Matěj Cepl 2007-02-08 17:54:42 EST
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.
Comment 13 Matěj Cepl 2007-02-08 17:56:53 EST
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?
Comment 14 Alexander Larsson 2007-02-12 09:31:47 EST
Ok, i sponsored Eric so we can get this in.
Comment 15 Eric Moret 2007-02-15 03:09:20 EST
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?
Comment 16 Martin Stransky 2007-02-15 03:15:20 EST
You can include configuration files, see /etc/alsa/alsa.conf

@hooks [
        {
                func load
                files [
                        "/etc/asound.conf"
                        "~/.asoundrc"
                ]
                errors false
        }
]
Comment 17 Eric Moret 2007-02-15 13:57:12 EST
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?
Comment 18 Bastien Nocera 2007-02-15 14:03:52 EST
(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).
Comment 19 Matěj Cepl 2007-02-15 16:11:32 EST
(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.
Comment 20 Eric Moret 2007-02-16 02:00:59 EST
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/
Comment 21 Eric Moret 2007-02-16 02:03:29 EST
Created attachment 148174 [details]
Adding configuration files and removing a52 package
Comment 22 Matěj Cepl 2007-03-13 04:49:33 EDT
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.
Comment 23 Matěj Cepl 2007-03-13 10:29:42 EDT
(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
Comment 24 Martin Stransky 2007-06-06 02:50:12 EDT
any news here? will be this package included in extras?
Comment 25 Bastien Nocera 2007-06-06 06:52:24 EDT
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.
Comment 26 Eric Moret 2007-06-06 10:36:08 EDT
As said earlier, I need a sponsor. Until then, I can't add this to Fedora.
Comment 27 Matthias Clasen 2007-06-06 11:23:54 EDT
Have you asked on the mailing list for a sponsor ? That might help.
Comment 28 Eric Moret 2007-07-19 20:12:44 EDT
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?
Comment 29 Matěj Cepl 2007-07-20 05:39:15 EDT
Is this what you want?
Comment 30 Eric Moret 2007-07-23 17:30:13 EDT
(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!
Comment 31 Eric Moret 2007-07-23 19:49:37 EDT
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
Comment 32 Lennart Poettering 2007-07-24 17:09:00 EDT
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. 
Comment 33 Jason Tibbitts 2007-07-28 11:50:09 EDT
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?
Comment 34 Eric Moret 2007-07-30 07:39:53 EDT
Created attachment 160236 [details]
New spec file with added dist tag
Comment 35 Eric Moret 2007-07-30 07:45:09 EDT
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.
Comment 36 Lennart Poettering 2007-08-06 16:23:03 EDT
Hmm, why this -maemo package? Has anyone ported Fedora to the N800 now?
Comment 37 Matthias Clasen 2007-08-06 16:33:08 EDT
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...
Comment 38 Matthias Clasen 2007-08-06 19:59:35 EDT
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 ?
Comment 39 Eric Moret 2007-08-07 14:24:11 EDT
(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.
Comment 40 Eric Moret 2007-08-07 14:25:39 EDT
Created attachment 160836 [details]
Updated spec file following initial review
Comment 41 Matthias Clasen 2007-08-07 15:22:31 EDT
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.


Comment 42 Eric Moret 2007-08-07 18:33:54 EDT
New Package CVS Request
=======================
Package Name: alsa-plugins
Short Description: backend plugins for alsa sound system
Owners: eric.moret@epita.fr
Branches: F-7
InitialCC: 
Comment 43 Kevin Fenzi 2007-08-07 19:56:53 EDT
cvs done.

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