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)
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 :)),
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
OK, so I'll keep those ideas for the next update then :)
The other changes (apart from the version bump) are needed as it fails to compile otherwise at least on my 13-CURRENT box.
OK, so I'll have a look at it ASAP, stay tuned :)
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 ?
It fails to locate libdvdcss, localbase fixes it (included in the patch) as you mentioned.
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.
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?
Created attachment 221322 [details]
Patch for libdvdcss - link option
Adds option to link with libdvdcss
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.
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 ?
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.
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 ?
Created attachment 221350 [details]
Patch for libdvdcss - link option - RADIO version
Thanks! I managed to miss the RADIO variant...
A commit references this bug:
Date: Thu Jan 7 16:10:12 UTC 2021
New revision: 560720
Provide an option to link to libdvdcss instead of using dlopen()
- clean up master sites
- remove useless dependency to gmake
- add a license file
Submitted by: firstname.lastname@example.org