[Bug 1931] Review request: audacious-plugins-nonfree - Audacious media player plugins with non free dependencies
http://bugzilla.rpmfusion.org/show_bug.cgi?id=1931
--- Comment #2 from Hans de Goede <j.w.r.degoede(a)gmail.com> 2011-09-24 20:27:41
---
Hi,
Thanks for the review!
I've some questions / remarks to your remarks, see below.
(In reply to comment #1)
> audacious-plugins-3.0.2-hardsid.patch
Doubtful patch. src/sid/xmms-sid.h says:
| /* HardSID-support is not working and is untested, thus we disable it here.
| */
| #undef HAVE_HARDSID_BUILDER
This overrides config.h settings, and in turn, conditional code for HardSID
support is not compiled. Linking the harsid-builder lib is useless.
Ok, still once this get fixed, the configure check needs to be fixed too. But
indeed useless for the packages atm then, I'll send this upstream instead.
Further, the plugin's GUI pieces are #if'ed out because they
have not been
ported to Audacious Preferences Widgets. This implies one cannot reconfigure
the plugin with the GUI, but only by editing Audacious' config file.
Apart from that, HardSID has been (or still is) a niche market soundcard that
can be populated with real SID chips. I can't resolve http://www.hardsid.com
currently. It could be down or gone. I don't know anything about the more
recent devices from that project, but the early ones have had timing issues
with critics claiming that they have had varying results compared with either
emulation or The Original sids.
I know what the HardSID is, it just seemed wrong to me that the configure
check failed even though I did have a libsidplay 2 with HardSID support...
[...]
Only slightly related, refer to
http://lists.rpmfusion.org/pipermail/rpmfusion-developers/2011-September/...
for a comment on libsidplayfp, which I've been pointed at. It could be that
some projects will switch to it.
[...]
You could trim down the %description as it isn't necessary to describe
Audacious here, and that description is out-of-date anyway.
Will do.
[...]
> --disable-sse2 \
This option has been dropped.
Will drop.
[...]
You could conflict explicitly with the "audacious-plugins-sid" package.
Ah, you put the sid plugin build with just libsidplay 1 in its own sub-package,
I was under the mistaken impression you had dropped it completely, my bad.
Since c offers a "better" audacious-plugins-sid, wouldn't it be better to
obsolete it instead, I really like to avoid conflicts
where ever possible. The question then becomes Obsolete which version I guess
anything < %{version} of audacious-plugins-nonfree?
[...]
Have built this successfully (x86_64), installed it, and it works for playing a
couple of test files in PSID v2 format. Don't have quick access to any PSID v3
files.
Dropping the hardsid patch and related spec sections is trivial and doesn't
need a re-review.
APPROVED
Thanks!
--
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.