Bug 204830

Summary: [NEW PORT] audio/optimfrog-sse2: SSE2 enabled slave port, audio/optimfrog: Update to 5.003
Product: Ports & Packages Reporter: Matthew Rezny <rezny>
Component: Individual Port(s)Assignee: Thomas Zander <riggs>
Status: Closed FIXED    
Severity: Affects Some People CC: rezny, riggs
Priority: --- Keywords: feature, patch, patch-ready
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
update audio/optimfrog to 5.003 and native builds
rezny: maintainer-approval+
i386 poudriere log
none
i386-sse2 poudriere log
none
amd64 poudriere log
none
Updated patch with LICENSE* rezny: maintainer-approval+

Description Matthew Rezny freebsd_committer freebsd_triage 2015-11-26 19:01:24 UTC
Created attachment 163555 [details]
update audio/optimfrog to 5.003 and native builds

The Linux build of OptimFROG which has been ports could not be updated to the current version because the newer Linux binaries require a higher Linux kernel interface version than our Linuxulator provides. Fortunately, the author was receptive to the idea of a native port when I contacted him.

This is the update to native FreeBSD builds of the current version of OptimFROG. There are three sets of binaries, x86 (with auto-detection of SSE), x86+SSE2 (hard dependency), and amd64. In order to ensure that all variants are available as packages, I have added a slave for the SSE2 variant on i386. Also set myself as  maintainer with this update.

Many thanks are due to Florin Ghido for creating the lossless audio codec with the best compression ratio and for his support of FreeBSD.
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2015-11-27 03:19:15 UTC
@Matthew, if you could confirm this change passes QA (portlint, poudriere) that would be great.
Comment 2 Matthew Rezny freebsd_committer freebsd_triage 2015-11-27 03:39:37 UTC
Created attachment 163566 [details]
i386 poudriere log
Comment 3 Matthew Rezny freebsd_committer freebsd_triage 2015-11-27 03:40:02 UTC
Created attachment 163567 [details]
i386-sse2 poudriere log
Comment 4 Matthew Rezny freebsd_committer freebsd_triage 2015-11-27 03:40:31 UTC
Created attachment 163568 [details]
amd64 poudriere log
Comment 5 Matthew Rezny freebsd_committer freebsd_triage 2015-11-27 03:55:07 UTC
(In reply to Kubilay Kocak from comment #1)

/usr/ports/audio/optimfrog # portlint
WARN: Makefile: SSE2 appears in PORT_OPTIONS:M, but is not listed in OPTIONS_DEFINE. [FP]
WARN: Makefile: unless this is a master port, COMMENT has to be set by "=", not by "?=". [FP]
WARN: Makefile: Consider defining LICENSE. [1]
WARN: Makefile: "DISTNAME" has to appear earlier. [FP]
0 fatal errors and 4 warnings found.

[FP] False Positive
[1] Custom license that is free to use bit minor distribution restrictions. Unsure on the NO_CDROM requirement.
Comment 6 Thomas Zander freebsd_committer freebsd_triage 2015-12-31 14:58:16 UTC
Sorry, but I wonder if it is really necessary to have the sse2 slave port, for multiple reasons:
1) SSE2 is built into the vast majority of x86 CPUs since 2001, that's almost a solid 15 years. Granted, several newer, especially low-power chips do not have it, which justifies to have a non-SSE2 version at all. But the overwhelming majority of the users will install the sse2 version, which creates a weird situation: For amd64 users, the default port will be audio/optimfrog while for i386 users the default port will be audio/optimfrog-sse2. This is weird.
2) The codec is distributed as a binary anyway, so installing it from ports will be virtually as quick as installing a package. The minority of users with non-sse2 machines can just install the non-sse2 port from ports.
3) If the current codec is already able to perform runtime CPU features detection for sse, then it seems likely that the author adds this feature for sse2 in an upcoming release.


Also I would like to include LICENSE-* tags. Especially with non-standard licenses we should make the extra effort to ensure we distribute software only within the permissions of the copyright holder.
Comment 7 Matthew Rezny freebsd_committer freebsd_triage 2016-01-02 02:52:17 UTC
The SSE2 slave is not necessary, it is there to provide a package of that build.

1: Almost the same can be said of AMD64, however that is irrelevant, our x86 branch must run on processors without any SSE or MMX at all, so the default for i386 MUST NOT be SSE2. Case in point: less than a quarter of my i386 systems have SSE2 support, and those are not in use (power hungry P4), while the much lower power systems with just MMX and maybe SSE are actually still getting use.

2: The reason to provide packages is for users who don't want to install the whole ports tree to get one package. Personally I don't care because I don't use the official packages at all, but many people seem to care about providing packages so I wanted to make all the binaries available through appropriate packages. Also, when the author wrote to inform me that the binaries were publicly available, he said he would like to have these new native builds available through FreeBSD ports packages, and he has graciously provided us three of them. As we still don't have flavors/variants support, the slave port to enable the non-default option is the best solution at the moment to provide packages for all.

3: The author has a very good reason to do the separate builds. This isn't just optional SSE2 routines, rather the SSE2 build uses no x87 FPU instructions, it does all FP via SSE2. The plain i386 build uses x87 FPU instructions and has some SSE optimizations that can be enabled with runtime detection. Trying to do runtime detection for x87 vs SSE2 FP is not reasonable. This isn't going to change and it's not at all uncommon for binary distribution.

I didn't use LICENSE_FILE because the license file should be plaint text as far as I know but the license file included in the release package is HTML. Is there any solution other than to put plaintext version in LICENSE_TEXT? I would rather not use LICENSE_TEXT because I would have to ensure that the plaintext is updated if the license ever changes, and that is a grey area itself; the license text is copyrighted, the license allows only unmodified files to be redistributed, and there are only specific allowances for what may be changed when repackaging for distribution. We are complying with the terms as I understand them and I assume we set NO_CDROM because there may be cases where there is a fee for the CD.
Comment 8 Thomas Zander freebsd_committer freebsd_triage 2016-01-02 11:20:07 UTC
Created attachment 164951 [details]
Updated patch with LICENSE*

Thanks for the explanation.
Of course I cannot claim to have authoritative numbers on the absolute distribution of SSE2 vs non-SSE2 enabled i386 CPUs, so I am ok with providing a non-sse2 default port. I still don't think it is truly necessary to have the sse2-enabled i386 package (and hence the extra slave port), but since you put in the work as maintainer, it's fine too.
Regarding the license, of course we would like to have the LICENSE_FILE as plain text, but if it it's some other format, then so be it. It's still better to have the license as html file in the pkg than to leave it out.

Can you have a look a the attached diff (relative to ${PORTSDIR}/audio) and let me know if you are ok with it? I also added the license permissions according to my understanding of the author's license file.
Comment 9 Matthew Rezny freebsd_committer freebsd_triage 2016-01-02 13:26:42 UTC
Comment on attachment 164951 [details]
Updated patch with LICENSE*

The updated patch looks good. Thank you for adding the license lines, and making it clear that HTML is not a problem for LICENSE_FILE. The permissions look correct with my understanding of the license as well, we may redistribute in our packages as long as we do not charge a fee.
Comment 10 commit-hook freebsd_committer freebsd_triage 2016-01-02 15:55:03 UTC
A commit references this bug:

Author: riggs
Date: Sat Jan  2 15:54:08 UTC 2016
New revision: 405077
URL: https://svnweb.freebsd.org/changeset/ports/405077

Log:
  Update to upstream version 5.003, introduce SSE2-enabled i386 slave port

  While on it:
  - Add LICENCE_* tags

  PR:		204830
  Submitted by:	matthew@reztek.cz (maintainer)

Changes:
  head/audio/Makefile
  head/audio/optimfrog/Makefile
  head/audio/optimfrog/distinfo
  head/audio/optimfrog/pkg-plist
  head/audio/optimfrog-sse2/
  head/audio/optimfrog-sse2/Makefile