Page MenuHomeFreeBSD

Update multimedia/handbrake to 1.1.0

Authored by fernape on Apr 13 2018, 10:17 PM.



Related to PR 227374
Submitter ( 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

rP FreeBSD ports repository
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

fernape created this revision.Apr 13 2018, 10:17 PM
tcberner added inline comments.Apr 14 2018, 6:30 AM
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

fernape updated this revision to Diff 41488.Apr 15 2018, 5:22 PM

Adressing issues pointed out by tcberner:

  • 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

tcberner added a comment.EditedApr 15 2018, 6:00 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

fernape updated this revision to Diff 41492.Apr 15 2018, 9:04 PM
  • Fix X11 option block.
  • Use DISTVERSION instead of PORTVERSION added inline comments.
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
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
100 ↗(On Diff #41492)


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?

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
100 ↗(On Diff #41492)

This is the first msg:

Then, a second one shows up:

In this system I have the following gstreamer packages:

$ pkg info -x 'gstreamer.*'

In the console I see only this message:
$ ./HandBrake
Xlib: extension "GLX" missing on display ":0.0".

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.

fernape updated this revision to Diff 41627.Apr 18 2018, 4:22 PM
  • Remove --disable-gst option
  • Add gstreamer libav dependency
fernape added inline comments.Apr 18 2018, 5:33 PM
100 ↗(On Diff #41492)

It works for me now.


tcberner accepted this revision.Apr 18 2018, 5:53 PM

Looks good to me. If you are also fine with it as the maintainer, this is good to go.

This revision is now accepted and ready to land.Apr 18 2018, 5:53 PM
This revision was automatically updated to reflect the committed changes.
mat added inline comments.May 14 2018, 10:26 PM

Add ONLY_FOR_ARCHS_REASON with what was in all the BROKEN_arch before.

fernape reopened this revision.May 15 2018, 3:57 PM

Reopening to address mat's comment.

fernape updated this revision to Diff 42580.May 15 2018, 4:08 PM


Pointed out by mat@

tcberner accepted this revision.May 16 2018, 5:42 AM

Looks good.

This revision is now accepted and ready to land.May 16 2018, 5:42 AM
This revision was automatically updated to reflect the committed changes.