multimedia/aribb24: New port A library for ARIB STD-B24, decoding JIS 8 bit characters and parsing MPEG-TS stream. Approved by: tbd (mentor), multimedia Differential Revision: https://reviews.freebsd.org/D33618
Details
Details
- Reviewers
philip 0mp - Group Reviewers
multimedia - Commits
- R11:ebf7717569bd: multimedia/aribb24: New port
Testport looks good, linters are happy.
Diff Detail
Diff Detail
- Repository
- R11 FreeBSD ports repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Tagging multimedia as this library can provide extra functionality to ffmpeg and because MAINTAINER is set to multimedia@FreeBSD.org.
| multimedia/aribb24/Makefile | ||
|---|---|---|
| 11 | I'm not sure if that's a common case that a port is added with a mailing-list maintainer. I think it only happens for well-established, critical ports. In this case, I think, the maintainer should be you. The reasoning is that it's better when there is one explicit maintainer for a given port when it gets added to the ports tree. It is a way of saying that "hey, I think that this ports is worth the effort to have it in the ports tree and I'm happy to maintain it for the time being". Adding a port with MAINTAINER set to ports@freebsd.org or another mailing list looks like "this piece of software is cool, but I'm not interested in maintaining it". This is the intuition i have. Also, if aribb24 is indeed an important part of the ffmpeg environment, then setting MAINTAINER to multimedia is not without a merit. Perhaps someone from multimedia would like to chime in on this one. | |
| 30 | Unnecessary line. | |
| multimedia/aribb24/pkg-plist | ||
| 7 | Do we need USE_LDCONFIG? We seem to be installing shared libraries here. | |
| multimedia/aribb24/Makefile | ||
|---|---|---|
| 11 | Thanks for the clear writeup! ATM I would not say that aribb24 is an important part of ffmpeg. Its an option that defaults to off. So I think, me taking maintainership is fine for this one. | |
| multimedia/aribb24/pkg-plist | ||
| 7 | Indeed, I wrongly interpreted TPH "If the software uses autotools, and specifically libtool, add USES=libtool." thinking libtool is sufficient but USE_LDCONFIG should also be set in addition to libtool, according to other ports that install shared libs which use libtool, eg atkm. | |
Comment Actions
You should probably consider using jeeb's fork (https://github.com/jeeb/aribb24/tree/pr_fixes, this also seems to be what videolan uses ) and possibly adding patches from https://github.com/scimmia9286/aribb24/commits/add-multi-DRCS-plane
You can also most likely drop the static archive unless you have a good reason to keep it around.
| multimedia/aribb24/Makefile | ||
|---|---|---|
| 8 | Common practice is to use full commit hash to avoid clashing in distfiles dir PATCHFILES= full-hash.patch:-p1 \ full-hash2.patch You might also want to consider "Example 19. Using USE_GITHUB to Access a Commit Between Two Versions" in Porters Handbook since you're referring to the same repo | |
Comment Actions
Unfortunately patches from https://github.com/scimmia9286/aribb24/commits/add-multi-DRCS-plane don't apply cleanly.
I'll port them under files/.
Comment Actions
Perhaps you could add a note in the commit message where the patches come from.
Also, I'm not sure if that's relevant for this port, but there is a very nice command for generating meaningful DISTVERSIONs for projects with Git repositories, which have at least one tag in the repo: git describe --tags. E.g.,
/tmp/aribb24$ git describe --tags v1.0.3-5-g5e9be27
then you can use -g5e9be27 as DISTVERSIONSUFFIX and skip the GH_TAGNAME completely.
Comment Actions
You "can" but I would highly suggest that we shouldn't work around Porters Handbook whether you personally like it or not, suggest changes to it instead. Consistency > Personal preference
GH_TAGNAME should be shorter to follow Porters Handbook
"Example 16. More Complete Use of USE_GITHUB"
Comment Actions
"Example 16. More Complete Use of USE_GITHUB"
See also "Example 19. Using USE_GITHUB to Access a Commit Between Two Versions"
Comment Actions
aribb24 is already supported by multimedia/ffmpeg. Please, remove the following as ARIBB24=on builds fine:
.if !exists(${.CURDIR:H:H}/multimedia/aribb24) # https://git.ffmpeg.org/gitweb/ffmpeg.git/commitdiff/100bfac6d6ec OPTIONS_EXCLUDE+= ARIBB24 .endif