Taking this review.

A few notes of things that should be fixed:

* Please don't explicitly require "pkgconfig" in the devel subpackage. "/usr/bin/pkg-config" gets added as a dependency anyway, so it's redundant.

* On your autoreconf command, please add "-v", as we want the output to be captured in build logs.

* The conditional for "--enable-fpm=64bit" looks like it should be "%ifarch x86_64", as it looks like 64bit is specifically talking about 64-bit x86. Thus, the current isa bits conditional is likely wrong.

%if 0%{__isa_bits} == 64 or smth like this..

(In reply to Igor Gnatenko from comment #3)
> %if 0%{__isa_bits} == 64 or smth like this..
It's already using that, but the Neal concern is that 64Bit would be Intel specific.

@Neal
Looking at the code, I don't read that, looking at line 132 in fixed.h define a non asm version of mad_f_mul. Also it's mentioned that the 64Bit FPM should work as soon a a compiler has 64bit type. But my understanding is that it will not perform better if not a native 64bit arch.
So using FPM=64bit seems a safe improvement over using ASM specific code on 64bit arches.

That been said, FPM=Intel is indeed x86 asm specific. I've never tested that option.

For arm, it's defined in line 253 and is asm specific. It's also advertised as performing better than FPM=64bit. I've only selected that option because it was worked on recently (on the referenced bug) and I've made a runtime test that it worked fine at the time I've bootstrapped RPM Fusion for arm. (was armv5tel and armv7hl at that time).

Fixed the others issues locally.

@Neal, any comment on the FPM option ?

Well, I'll trust your judgement on the FPM option. You can leave it as is.

Spec URL: http://dl.kwizart.net/review/libmad.spec
SRPM URL: http://dl.kwizart.net/review/libmad-0.15.1b-18.fc26.src.rpm
Description: MPEG audio decoder library

Changelog:
- fixed verbose autoreconf
- Add pkgconfig only for el5

Scratch build with el6:
http://koji.fedoraproject.org/koji/taskinfo?taskID=16511746
There pkgconfig is automatically added as requires indeed, but not for el5.

fedora-review went mostly okay, there were a few points, though:

* COPYRIGHT needs to be under %license, not %doc

* Can you please put a comment explaining the arch-specific patch?
libmad.src: W: %ifarch-applied-patch Patch0: libmad-0.15.1b-multiarch.patch

* Sources are missing from the libmad-debuginfo subpackage. This needs to be fixed.

(In reply to Neal Gompa from comment #7)
> * Sources are missing from the libmad-debuginfo subpackage. This needs to be
> fixed.

I suggest the following patches:

https://gitweb.gentoo.org/repo/gentoo.git/plain/media-libs/libmad/files/libmad-0.15.1b-cflags.patch
https://gitweb.gentoo.org/repo/gentoo.git/plain/media-libs/libmad/files/libmad-0.15.1b-cflags-O2.patch

@Yaakov, I do not see how that fixes the fact that the source code is missing in the debuginfo subpackage...

The sources are missing in the debug package because configure is messing around with CFLAGS.  However, given Fedora's default %{optflags}, you are correct that this isn't quite sufficient; --disable-debugging needs to be dropped from the arguments to %configure.

Spec URL: http://dl.kwizart.net/review/libmad.spec
SRPM URL: http://dl.kwizart.net/review/libmad-0.15.1b-19.fc26.src.rpm
Description: MPEG audio decoder library

koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=16576156

Changelog:
- Add patches from gentoo
- Allow debuginfo generation

To speedup things a little I'm going to steal this 

REVIEW:

Legend: + = PASSED, - = FAILED, 0 = Not Applicable

+ rpmlint is not silent, but its messages are safe to ignore:

Auriga ~/rpmbuild/SPECS: rpmlint ../RPMS/x86_64/libmad-* ../SRPMS/libmad-0.15.1b-19.fc25.src.rpm 
libmad.x86_64: E: incorrect-fsf-address /usr/share/licenses/libmad/COPYRIGHT
libmad.x86_64: E: incorrect-fsf-address /usr/share/licenses/libmad/COPYING
libmad-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/libmad-0.15.1b/layer12.c
libmad-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/libmad-0.15.1b/layer3.c
libmad-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/libmad-0.15.1b/version.c
libmad-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/libmad-0.15.1b/decoder.c
libmad-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/libmad-0.15.1b/version.h
libmad-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/libmad-0.15.1b/synth.c
libmad-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/libmad-0.15.1b/frame.h
libmad-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/libmad-0.15.1b/stream.h
libmad-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/libmad-0.15.1b/fixed.c
libmad-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/libmad-0.15.1b/timer.h
libmad-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/libmad-0.15.1b/timer.c
libmad-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/libmad-0.15.1b/frame.c
libmad-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/libmad-0.15.1b/huffman.c
libmad-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/libmad-0.15.1b/decoder.h
libmad-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/libmad-0.15.1b/bit.h
libmad-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/libmad-0.15.1b/synth.h
libmad-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/libmad-0.15.1b/bit.c
libmad-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/libmad-0.15.1b/huffman.h
libmad-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/libmad-0.15.1b/stream.c
libmad-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/libmad-0.15.1b/fixed.h

^^^ This is safe to ignore. Regarding mentioning upstream - the application is ~12 years old, so it's likely it won't be updated anytime soon regardless of attempts to contact upstream.

libmad-devel.x86_64: W: only-non-binary-in-usr-lib

^^^ false positive.

libmad-devel.x86_64: W: no-documentation

^^^ That's how things are. It would be better if the package has documentation. unfortunately it hasn't.

libmad-devel.x86_64: E: incorrect-fsf-address /usr/include/mad.h

^^^ See above.

libmad.src: W: %ifarch-applied-patch Patch0: libmad-0.15.1b-multiarch.patch

^^^ This is intentional.

4 packages and 0 specfiles checked; 23 errors, 3 warnings.
Auriga ~/rpmbuild/SPECS: 

+ The package is named according to the  Package Naming Guidelines.
+ The spec file name matches the base package %{name}, in the format %{name}.spec.
+ The package meets the Packaging Guidelines. All the issues mentioned in the comments above were addressed.
+ The package is licensed with a Fedora approved license (GPLv2 or later) and meets the Licensing Guidelines.
+ The License field in the package spec file matches the actual license (GPLv2 or later).
+ The file, containing the text of the license(s) for the package, is included as %license.
+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package, match the upstream source, as provided in the spec URL.

Auriga ~/rpmbuild/SOURCES: sha256sum libmad-0.15.1b.tar.gz*
bbfac3ed6bfbc2823d3775ebb931087371e142bb0e9bb1bee51a76a6e0078690  libmad-0.15.1b.tar.gz
bbfac3ed6bfbc2823d3775ebb931087371e142bb0e9bb1bee51a76a6e0078690  libmad-0.15.1b.tar.gz.1
Auriga ~/rpmbuild/SOURCES: 

+ The package successfully compiles and builds into binary rpms on at least one primary architecture.
+ All build dependencies are listed in BuildRequires.
0 No need to handle locales.
+ The package stores shared library files in some of the dynamic linker's default paths, and it calls ldconfig in %post and %postun.
+ The package does NOT bundle copies of system libraries.
0 The package is not designed to be relocatable.
+ The package owns all directories that it creates.
+ The package does not list a file more than once in the spec file's %files listings.
+ Permissions on files are set properly.
+ The package consistently uses macros.
+ The package contains code, or permissible content.
0 No extremely large documentation files.
+ Anything, the package includes as %doc, does not affect the runtime of the application.
+ Header files are stored in a -devel package.
0 No static libraries.
+ The pkgconfig(.pc) files are stored in a -devel package.
+ The library file(s) that end in .so (without suffix) is(are) stored in a -devel package.
+ The -devel package requires the base package using a fully versioned dependency: Requires: %{name}%{?_isa} = %{version}-%{release}
+ The package does NOT contain any .la libtool archives.
0 Not a GUI application.
+ The package does not own files or directories already owned by other packages.
+ All filenames in rpm packages are valid UTF-8.


This package is


APPROVED.

A little notes from me regarding this package.

I personally believe that the time for this library is gone. 12-15 years ago MAD was ahead of mpg123 in terms of accuracy, performance, and features. Since them MAD development was stalled, while mpg123 was fully reloaded. Now mpg123 is confirming to MP3 standards, fast, feature-reach, and with active upstream.

So if you in doubts, then use mpg123. That said there are some apps which use libmad exclusively. We should let them to do so. So that's why we have to have libmad in our repository.

Thx Peter for the review.
However you will need to pick the assignment also. As this is later checked before the git module creation.

About libmad versus mpg123, I perfectly agree with that.
Maybe we should make a note on the package.
I know at least qtractor can optionally use libmad in fedora IIRC.

@Neal
Do you have any remaining issue in mind ?

(In reply to Nicolas Chauvet (kwizart) from comment #14)
> 
> @Neal
> Do you have any remaining issue in mind ?

No, I'm fine.

Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/libmad