Bug 951823 - Review Request: sidplayfp - SID chip music module player
Summary: Review Request: sidplayfp - SID chip music module player
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Michael Schwendt
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 951820
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-04-13 13:01 UTC by Hans de Goede
Modified: 2013-05-02 03:53 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2013-04-27 03:21:44 UTC
Type: ---
Embargoed:
bugs.michael: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Hans de Goede 2013-04-13 13:01:24 UTC
Spec URL: http://people.fedoraproject.org/~jwrdegoede/sidplayfp.spec
SRPM URL: http://people.fedoraproject.org/~jwrdegoede/sidplayfp-1.0.1-1.fc15.src.rpm
Description:
A player for playing SID music modules originally created on the Commodore 64
and compatibles.

Fedora Account System Username: jwrdegoede

Note this package depends on libsidplayfp, which review request is bug 951820.

Comment 1 Michael Schwendt 2013-04-19 11:35:23 UTC
* rpmlint only finds a few incorrect-fsf-address issues:
  https://fedoraproject.org/wiki/Common_Rpmlint_issues#incorrect-fsf-address

  Reported: https://sourceforge.net/tracker/?group_id=257241&atid=1551608


* sidplayfp options --none and --nosid segfault. Reported:
  https://sourceforge.net/tracker/?func=detail&aid=3611237&group_id=257241&atid=1551608


* stilview usage txt refers to USAGE.txt file that isn't packaged. Reported:
  https://sourceforge.net/tracker/?group_id=257241&atid=1551608

Run-time test:
$ HVSC_BASE=/home/ms19a/Music/INCOMING/HVSC/C64Music stilview -e=/DEMOS/0-9/3_Oversample.sid 
  TITLE: Get Ready
 ARTIST: Jeroen Tel
COMMENT: Same as /MUSICIANS/T/Tel_Jeroen/HCS_etc/Get_Ready.sid, but with digis
         added and with voice 1 largely muted.


* Build with pulseaudio and ALSA?

  $ grep alsa sidplayfp.spec 
  BuildRequires:  libsidplayfp-devel alsa-lib-devel pulseaudio-libs-devel
  $ rpm -qR sidplayfp|grep alsa
  $

Somebody has reported it already:
https://sourceforge.net/tracker/?func=detail&aid=3609103&group_id=257241&atid=1551608


* Run-time test: It's expected that with no ROM images available, some sids that did play with libsidplay1 are just silent (since old libsidplay set up fake interrupts, e.g. CIA Timer IRQs, for non VBI-speed playback). Installing the non-free "vice-data" package makes those test tunes work. Since a few users have asked about such silent sids before in upstream tracker, I will contact upstream about whether it is known how much exactly is missing (e.g. just default IRQ handlers in kernal space or larger parts?).


* Untested: The "Songlength Database" which is also touched by one of the patches. I dunno what may have changed over the past years, whether it works and whether it is still updated.


> make install DESTDIR=$RPM_BUILD_ROOT

Just for the record, nowadays there's %make_install for that.


* Other than that, I see no packaging mistakes.

APPROVED

Comment 2 Hans de Goede 2013-04-19 18:12:26 UTC
(In reply to comment #1)
> * Untested: The "Songlength Database" which is also touched by one of the
> patches. I dunno what may have changed over the past years, whether it works
> and whether it is still updated.

I've tested this and it works :)  The patch is only so that the Songlenghts.txt file can be optionally put under /usr/share/sidplayfp, so that it is available for all users, rather then each user needing to put it in ~/.local/share/sidplayfp separately. Also it has been send upstream and accepted there.

> > make install DESTDIR=$RPM_BUILD_ROOT
> 
> Just for the record, nowadays there's %make_install for that.

I know, but whomever came up with that should have given it a better name, this is way too much like the bad %makeinstall, so I prefer the old way.

> APPROVED

Thanks for the review!

Comment 3 Hans de Goede 2013-04-19 18:18:17 UTC
New Package SCM Request
=======================
Package Name: sidplayfp
Short Description: SID chip music module player
Owners: jwrdegoede
Branches: f17 f18 f19
InitialCC: mschwendt

Comment 4 Gwyn Ciesla 2013-04-22 13:29:45 UTC
Git done (by process-git-requests).

Comment 5 Fedora Update System 2013-04-22 21:26:27 UTC
libsidplayfp-1.0.1-2.fc19,sidplayfp-1.0.1-1.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/libsidplayfp-1.0.1-2.fc19,sidplayfp-1.0.1-1.fc19

Comment 6 Fedora Update System 2013-04-22 21:30:38 UTC
libsidplayfp-1.0.1-2.fc18,sidplayfp-1.0.1-1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/libsidplayfp-1.0.1-2.fc18,sidplayfp-1.0.1-1.fc18

Comment 7 Fedora Update System 2013-04-22 21:30:47 UTC
libsidplayfp-1.0.1-2.fc17,sidplayfp-1.0.1-1.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/libsidplayfp-1.0.1-2.fc17,sidplayfp-1.0.1-1.fc17

Comment 8 Fedora Update System 2013-04-23 02:58:23 UTC
libsidplayfp-1.0.1-2.fc19, sidplayfp-1.0.1-1.fc19 has been pushed to the Fedora 19 testing repository.

Comment 9 Fedora Update System 2013-04-27 03:21:45 UTC
libsidplayfp-1.0.1-2.fc19, sidplayfp-1.0.1-1.fc19 has been pushed to the Fedora 19 stable repository.

Comment 10 Fedora Update System 2013-05-02 03:51:54 UTC
libsidplayfp-1.0.1-2.fc18, sidplayfp-1.0.1-1.fc18 has been pushed to the Fedora 18 stable repository.

Comment 11 Fedora Update System 2013-05-02 03:53:04 UTC
libsidplayfp-1.0.1-2.fc17, sidplayfp-1.0.1-1.fc17 has been pushed to the Fedora 17 stable repository.


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