Details

Reviewers

tcberner
tz

Commits

rP470133: multimedia/handbrake: Add ONLY_FOR_ARCHS_REASON
rP467738: Update multimedia/handbrake to 1.1.0

Summary

Related to PR 227374
Submitter (naito.yuichiro@gmail.com) wants to take maintainership of the port

Test Plan

portlint -AC OK
poudriere builds {10.3,10.4,11.1}{amd64,i386}, 12i386 OK
run test in 12i386 OK

Diff Detail

Repository

rP FreeBSD ports repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

fernape created this revision.Apr 13 2018, 10:17 PM

Herald added a subscriber: mat. · View Herald TranscriptApr 13 2018, 10:17 PM

Harbormaster completed remote builds in B16134: Diff 41454.Apr 13 2018, 10:17 PM

tcberner added inline comments.Apr 14 2018, 6:30 AM

multimedia/handbrake/Makefile
5 ↗	(On Diff #41454)	^ as the line already changes, you can switch it to DISTVERSION.
14 ↗	(On Diff #41454)	`ONLY_FOR_ARCH=i386 amd64` seems more idiomatic to express that. Porters Handbook
101 ↗	(On Diff #41454)	This whole option block should be changed to use the options helpers

Adressing issues pointed out by tcberner:

Use ONLY_FOR_ARCHS
Convert to use option helpers

Can you help me out with this? What am I doing wrong?
The --disable-gst and --disable-gtk-update-checks are not added to
CONFIGURE_ARGS. I didn't use OPT_CONFIGURE_ENABLE because I want the exact
opposite logic, this is to disable when X11 is set. portlint doesn't complain
and the build logs don't show any obviuos errors :S

Harbormaster completed remote builds in B16149: Diff 41488.Apr 15 2018, 5:22 PM

The options are X11, not MX11 :), so it should be X11_USES and so on

The M came from the :M expansion of make: Variable modifiers

Fix X11 option block.
Use DISTVERSION instead of PORTVERSION

Harbormaster completed remote builds in B16152: Diff 41492.Apr 15 2018, 9:04 PM

naito.yuichiro_gmail.com added a subscriber: naito.yuichiro_gmail.com.Apr 16 2018, 2:19 PM

naito.yuichiro_gmail.com added inline comments.

multimedia/handbrake/Makefile
100 ↗	(On Diff #41492)	Excuse me. I'm Yuichiro NAITO, submitter of this patch. Why did you add --disable-gst ? It disables preview feature of HandBrake. Are there any problem for this feature?

tcberner added inline comments.Apr 16 2018, 2:56 PM

multimedia/handbrake/Makefile
100 ↗	(On Diff #41492)	It seems to be already there in the current version in the ports tree: `CONFIGURE_ARGS+= --disable-gtk-update-checks --disable-gst`

fernape added inline comments.Apr 16 2018, 5:48 PM

multimedia/handbrake/Makefile
100 ↗	(On Diff #41492)	Hi, I compiled on 11.1 with and without --disable-gst. In both cases, the preview of the main window show a frame of the video. Clicking on "show preview" plays that only frame for the length of the video (mp4). It also warns about some gstreamer missing plugins. Can you confirm this behavior?

naito.yuichiro_gmail.com added inline comments.Apr 16 2018, 11:23 PM

multimedia/handbrake/Makefile
100 ↗	(On Diff #41492)	No, I can't see any warnings on my HandBrake except of 'inhibited 0'. 'inhibited 0' is a debug message that upsteram missed deleting before 1.1.0 release. Could you show me the message? I believe preview requires gstreamer1-plugins-gdkpixbuf. I want to check if another plugin is required or not.

fernape added inline comments.Apr 17 2018, 4:00 PM

multimedia/handbrake/Makefile
100 ↗	(On Diff #41492)	This is the first msg: https://www.dropbox.com/s/nadocvs1cwoiof1/msg.png?dl=0 Then, a second one shows up: https://www.dropbox.com/s/7jut7j2bw2c5n4c/msg2.png?dl=0 In this system I have the following gstreamer packages: $ pkg info -x 'gstreamer.*' gstreamer-0.10.36_6 gstreamer-plugins-0.10.36_8,3 gstreamer1-1.12.3 gstreamer1-plugins-1.12.3 gstreamer1-plugins-gdkpixbuf-1.12.3 gstreamer1-plugins-good-1.12.3 In the console I see only this message: $ ./HandBrake Xlib: extension "GLX" missing on display ":0.0".

naito.yuichiro_gmail.com added inline comments.Apr 18 2018, 12:13 PM

multimedia/handbrake/Makefile
100 ↗	(On Diff #41492)	Thank you so much. I found gstreamer1-libav plugin is needed for preview. Gstreamer1-libav plugin depends on ffmpeg package. It seems redundant because HandBrake is a video encoder, too. But I don't mind the redundancy. GUI interface includes preview feature is important. So please include libav plugin like as following line. "X11_USE= gstreamer1=gdkpixbuf,libav" If someone claims the redundancy, I will make a port option to switch preview on/off. It's a future work not now.