Bug 256593 - multimedia/vlc: Enable support for subtitles by default and add missing deps
Summary: multimedia/vlc: Enable support for subtitles by default and add missing deps
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-multimedia (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-06-14 08:45 UTC by Daniel Engberg
Modified: 2021-09-06 18:26 UTC (History)
3 users (show)

See Also:
bugzilla: maintainer-feedback? (multimedia)

Attachments
Patch for vlc (1.58 KB, patch)
2021-06-14 08:45 UTC, Daniel Engberg 		no flags Details | Diff
vlc: Add missing dependencies (2.00 KB, patch)
2021-07-05 19:23 UTC, Philipp Ost 		no flags Details | Diff
Patch for vlc v2 (2.53 KB, patch)
2021-09-06 12:42 UTC, Daniel Engberg 		no flags Details | Diff
Remove comment about C++11 support (724 bytes, patch)
2021-09-06 15:11 UTC, Philipp Ost 		no flags Details | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Engberg freebsd_committer freebsd_triage 2021-06-14 08:45:06 UTC 
Created attachment 225795 [details]
Patch for vlc

Enable subtitle support by default ((lib)ass and harfbuzz) for a better user experience
Add missing dependencies reported by poudriere

Poudriere testport OK 12.2-RELEASE (amd64)
Comment 1 Philipp Ost 2021-07-05 19:23:29 UTC 
Created attachment 226246 [details]
vlc: Add missing dependencies
Comment 2 Daniel Engberg freebsd_committer freebsd_triage 2021-07-06 05:52:57 UTC 
Hi Philipp,

The mpg123 gets picked up unintentionally and shouldn't be enabled by default. I'll update my patch to reflect on that.

Best regards,
Daniel
Comment 3 Daniel Engberg freebsd_committer freebsd_triage 2021-09-06 12:42:31 UTC 
Created attachment 227708 [details]
Patch for vlc v2

Refresh patch
Disable mpg123 support (autodetected by default and gets pulled in if found)
Add iconv:wchar_t from Philipp Ost (Thanks!)

Poudriere testport OK 12.2-RELEASE (amd64)
Comment 4 Philipp Ost 2021-09-06 15:11:43 UTC 
Created attachment 227711 [details]
Remove comment about C++11 support

Thank you for working on this! The patch looks good to me on first glance, but I didn't do any proper testing yet.

I'm wondering if we really need the comment about explicitly enabling C++11. FreeBSD 10 is EOL and directly above said comment we set USES+=compiler:c++17-lang.
Comment 5 commit-hook freebsd_committer freebsd_triage 2021-09-06 18:23:51 UTC 
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=fe3ca40c7e298d3c2246684f7817e6c52a0e20ca

commit fe3ca40c7e298d3c2246684f7817e6c52a0e20ca
Author:     Daniel Engberg <diizzy@FreeBSD.org>
AuthorDate: 2021-09-06 18:12:40 +0000
Commit:     Daniel Engberg <diizzy@FreeBSD.org>
CommitDate: 2021-09-06 18:22:25 +0000

    multimedia/vlc: Enable support for subtitles and add missing deps

    Additionally disable autodetection of (lib)mpg123 to avoid undefined
    dependencies and define wchar_t (iconv) usage
    Commit also includes suggestions by Philipp Ost

    PR:             256593
    Approved by:    arrowd (mentor)
    Differential Revision:  https://reviews.freebsd.org/D31859

 multimedia/vlc/Makefile | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)
Comment 6 Daniel Engberg freebsd_committer freebsd_triage 2021-09-06 18:26:10 UTC 
(In reply to Philipp Ost from comment #4)
We'll leave it for now I'll have it in mind next time I'm touching this port