Page MenuHomeFreeBSD

Update multimedia/handbrake to 1.1.0
ClosedPublic

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

Details

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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

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

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
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?

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

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?

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.

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".

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.

  • Remove --disable-gst option
  • Add gstreamer libav dependency
multimedia/handbrake/Makefile
100 ↗(On Diff #41492)

It works for me now.

Thanks!

Looks good to me. If you are also fine with it @naito.yuichiro_gmail.com 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.
head/multimedia/handbrake/Makefile
14

Add ONLY_FOR_ARCHS_REASON with what was in all the BROKEN_arch before.

Reopening to address mat's comment.

Add ONLY_FOR_ARCHS_REASON.

Pointed out by mat@

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.