Bug 252322 - multimedia/libdvdread: Add option to link with libdvdcss
Summary: multimedia/libdvdread: Add option to link with libdvdcss
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: Ganael LAPLANCHE
URL: https://code.videolan.org/videolan/li...
Keywords:
Depends on:
Blocks:
 
Reported: 2021-01-01 11:28 UTC by daniel.engberg.lists
Modified: 2021-01-07 16:12 UTC (History)
0 users

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


Attachments
Patch for libdvdread (2.06 KB, patch)
2021-01-01 11:28 UTC, daniel.engberg.lists
no flags Details | Diff
Patch for libdvdcss - link option (1.62 KB, patch)
2021-01-06 11:46 UTC, daniel.engberg.lists
no flags Details | Diff
Patch for libdvdcss - link option - RADIO version (1.51 KB, patch)
2021-01-07 12:33 UTC, Ganael LAPLANCHE
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description daniel.engberg.lists 2021-01-01 11:28:46 UTC
Created attachment 221156 [details]
Patch for libdvdread

Update libdvdread to 6.1.1

Other minor fixes:
Switch from PORTVERSION to DISTVERSION
Use HTTPS for MASTER_SITES and remove stale mirrors
Add USES= localbase to properly detect libdvdread
Use CONFIGURE_WITH to enable/disable libdvdcss

Compile tested on FreeBSD 13.0-CURRENT main-c1-g63f93c7e1 (amd64) (make, make check-plist)
Run-time tested on FreeBSD 13.0-CURRENT #0: Wed Dec 30 11:54:07 CET 2020 (arm64) (hash unavailable)
Poudriere testport OK 12.2-RELEASE (amd64)
Poudriere testport OK 11.4-RELEASE (amd64)
Comment 1 Ganael LAPLANCHE freebsd_committer 2021-01-01 12:15:01 UTC
Hello Daniel,

Thanks a lot for your contribution.

I had skipped that update because there is no code change between 6.1.0 and 6.1.1, only a library version bump.

If there is no specific reason for upgrading it, I would prefer waiting for the next release to update the port, to avoid having to bump and rebuild all dependencies again.

Anyway, you provide other improvements to the port which are interesting, I will integrate them ASAP.

Best regards (and happy new year by the way :)),

Ganael.
Comment 2 daniel.engberg.lists 2021-01-01 12:34:12 UTC
Hi,

It was more of a "while I'm at it" thing to be honest (because it's a dependency of PR 252323). I'm fine with not bumping if you feel it's unnecessary.

Happy new year

Best regards,
Daniel
Comment 3 Ganael LAPLANCHE freebsd_committer 2021-01-01 15:54:43 UTC
OK, so I'll keep those ideas for the next update then :)

Thanks again,

Best regards,

Ganael.
Comment 4 daniel.engberg.lists 2021-01-01 16:00:17 UTC
The other changes (apart from the version bump) are needed as it fails to compile otherwise at least on my 13-CURRENT box.
Comment 5 Ganael LAPLANCHE freebsd_committer 2021-01-01 16:46:25 UTC
OK, so I'll have a look at it ASAP, stay tuned :)
Comment 6 Ganael LAPLANCHE freebsd_committer 2021-01-04 11:22:19 UTC
Hello Daniel,

Can you tell me more ? How does it fail on 13-CURRENT, do you have a build log ? The only change that could fix a build failure seems to be usage of 'localbase'. Is that right ?
Comment 7 daniel.engberg.lists 2021-01-04 20:03:56 UTC
Hi,

It fails to locate libdvdcss, localbase fixes it (included in the patch) as you mentioned.

Best regards,
Daniel
Comment 8 Ganael LAPLANCHE freebsd_committer 2021-01-05 11:19:03 UTC
Hello Daniel,

Thanks for your feedback.

OK, I see. I'd like to avoid linking directly against libdvdcss (--with-libdvdcss). That library is currently dlopen'ed and I think it should remain that way because libdvdcss is RESTRICTED ("CSS code may violate the DMCA"). It allows to install/remove libdvdcss without having to rebuild the libdvdread port and makes the whole thing much more flexible.

The remaining of the patch (MASTER_SITES update, LICENSE_FILE specification and gmake removal from USES) are good ideas that I'll keep for a future commit.

Best regards,

Ganael.
Comment 9 daniel.engberg.lists 2021-01-06 07:24:41 UTC
I think we should have a clear distinction between the options, "follow" upstream and as for the requirement to rebuild it's already the case in ~99.95%(?) of all ports already if you want different options than default.

Having a quick look at repology.org

Adélie, Gentoo, NetBSD (kinda, http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/multimedia/libdvdread/ ), Void Linux links (usually optional)

Debian and OpenSUSE clearly states that their package doesn't have decss support

With all that in mind and trying to find a middleground, how about adding an option that allows linking but doesn't set --without would that be acceptable?
Comment 10 daniel.engberg.lists 2021-01-06 11:46:32 UTC
Created attachment 221322 [details]
Patch for libdvdcss - link option

Adds option to link with libdvdcss
Comment 11 Ganael LAPLANCHE freebsd_committer 2021-01-06 15:16:05 UTC
Hello Daniel,

If dvdcss remains disabled for package building and dlopen() is the default behaviour, yes I think we can offer a user option to link to dvdcss. I'll look at your patch ASAP, thanks.

Stay tuned,
Best regards,

Ganael.
Comment 12 Ganael LAPLANCHE freebsd_committer 2021-01-06 15:40:45 UTC
But I still do not see a clear use case for the need to explicitly link against the dvdcss library and not use dlopen() ? Would you have an example ?
Comment 13 daniel.engberg.lists 2021-01-06 22:14:30 UTC
I might be a bit old-fashioned but if it's linked you can assume that it's utilized by libdvdread as opposed where it would just potentially fail to decode  (becasue it cannot locate libdvdcss) but I might be reading the code incorrectly.
Comment 14 Ganael LAPLANCHE freebsd_committer 2021-01-07 12:32:04 UTC
Hello Daniel,

Well, reading src/dvd_input.c:183, it seems that you would get a warning telling that Encrypted DVD support is unavailable. Anyway, I understand your concern. Here is an updated patch that will offer the option to explicitly link against dvdcss but as a RADIO option and will set --without-libdvdcss if DVDCSS_DLOPEN is selected (because in this case libdvdread will still attempt to dlopen() it). I also renamed the options to make them more explicit.

Would that patch fit your needs ?

Best regards,

Ganael.
Comment 15 Ganael LAPLANCHE freebsd_committer 2021-01-07 12:33:10 UTC
Created attachment 221350 [details]
Patch for libdvdcss - link option - RADIO version
Comment 16 daniel.engberg.lists 2021-01-07 12:41:22 UTC
Thanks! I managed to miss the RADIO variant...

LGTM!
Comment 17 commit-hook freebsd_committer 2021-01-07 16:11:03 UTC
A commit references this bug:

Author: martymac
Date: Thu Jan  7 16:10:12 UTC 2021
New revision: 560720
URL: https://svnweb.freebsd.org/changeset/ports/560720

Log:
  Provide an option to link to libdvdcss instead of using dlopen()

  While here:
  - clean up master sites
  - remove useless dependency to gmake
  - add a license file

  PR:		252322
  Submitted by:	daniel.engberg.lists@pyret.net

Changes:
  head/multimedia/libdvdread/Makefile
Comment 18 Ganael LAPLANCHE freebsd_committer 2021-01-07 16:12:11 UTC
Committed,thanks!