-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Addition of package HandBrake CLI #3937
base: master
Are you sure you want to change the base?
Conversation
New Update
dependencies: libdvdread libdvdnav dav1d harfbuff jansson ffmpeg updated to 4.2.2
This reverts commit a33f7d8.
addition of new package HandBrake CLI (Command Line Interface) update of ffmpeg configuration options according to HandBrake package requirement. Update of ffmpeg to version 4.2.2
@jvega1976 Welcome and thanks for this contribution. Please create a new branch and do not create a pull request from your master branch. Otherwise you will get troubles to update your fork from the upstream repo and to create other (independent) PRs from your fork. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this package is for x64 archs only, this is obsolete. IMHO it would be better to use a docker container for this.
This PR changes some common cross modules that are related to other packages that need to be verified with build and functional testing.
- cross/bzip2 (this change breaks all 32-bit archs if not adjusted)
- cross/ffmpeg (revert changes and wait for official update with PR FFMPEG 4.2.2: Update, fixes and cleanup #3873)
- cross/fontconfig
- cross/freetype
- cross/glib (this is a breaking change for all dependent packages, as introduces depependency on native/python3)
- cross/gmmlib
- cross/intel-media-driver
- cross/intel-media-sdk
- cross/libaom
- cross/libva-utils
- cross/libxml2
- cross/util-linux
- cross/x265
General request:
revert the remove of targets in cross module PLIST files, as other packages may depend on it.
- cross/libaom
- cross/libva-utils
@@ -15,7 +15,7 @@ CONFIGURE_TARGET = nop | |||
COMPILE_TARGET = bzip2_compile | |||
INSTALL_TARGET = bzip2_install | |||
|
|||
ADDITIONAL_CFLAGS = -fpic -fPIC -O3 -D_FILE_OFFSET_BITS=64 | |||
ADDITIONAL_CFLAGS = -m64 -fpic -fPIC -O3 -D_FILE_OFFSET_BITS=64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add the flag -m64
for 64-bit archs only
@@ -75,7 +75,7 @@ tc_vars: patch | |||
@echo TC_ENV += CPPFLAGS=\"$(CPPFLAGS) $$\(ADDITIONAL_CPPFLAGS\)\" | |||
@echo TC_ENV += CXXFLAGS=\"$(CXXFLAGS) $$\(ADDITIONAL_CXXFLAGS\)\" | |||
@echo TC_ENV += LDFLAGS=\"$(LDFLAGS) $$\(ADDITIONAL_LDFLAGS\)\" | |||
@echo TC_CONFIGURE_ARGS := --host=$(TC_TARGET) --build=i686-pc-linux | |||
@echo TC_CONFIGURE_ARGS := --host=$(TC_TARGET) --build=x86_64-pc-linux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert this change.
The official development environment uses i686 and not x64 for toolchain builds.
STARTABLE = no | ||
|
||
HOMEPAGE = https://handbrake.fr | ||
LICENSE = GPLv3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please set the LICENSE according to cross/HandBrakeCLI/Makefile
without ENABLE_FDK_AAC=YES: LICENSE = GPLv2
with ENABLE_FDK_AAC=YES:
this should be something like: LICENSE = for personal use of <your name and email>
A better approach would be to isolate the build with ENABLE_FDK_AAC=YES
to /diyspk/HandBrakeCLI/Makefile, as this is for do it yourself builds only.
And /spk/HandBrakeCLI/Makefile with ENABLE_FDK_AAC=YES
must not build and terminate with an appropriate error.
@@ -0,0 +1,206 @@ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file must be empty as all target files are taken from the dependent cross modules.
@@ -0,0 +1,203 @@ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this file.
All targets are normally included by dependent cross modules
CONFIGURE_TARGET = myConfigure | ||
COMPILE_TARGET = myCompile | ||
INSTALL_TARGET = myInstall |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use correct naming <packagename>_target
as:
HandBrake_configure
HandBrake_compile
HandBrake_install
Please apply this to all your new Makefiles
PKG_VERS = 1.3.1 | ||
PKG_EXT = tar.bz2 | ||
PKG_DIST_NAME = $(PKG_NAME)-$(PKG_VERS)-source.$(PKG_EXT) | ||
PKG_DIST_SITE = https://github.com/HandBrake/HandBrake/releases/download/1.3.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use $(PKG_VERS) in url to avoid conflicts.
.PHONY: myCompile | ||
myCompile: | ||
$(RUN) DESTDIR=$(INSTALL_DIR) ./configure $(CONFIGURE_ARGS) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use HandBreak_configure for calling ./configure
And define HandBreak_compile = nop
to suppress the compile step, instead of defining a target which does nothing.
Probably you have to use $(REAL_CONFIGURE_ARGS)
instead of $(CONFIGURE_ARGS)
to include toolchain specific args.
May be you can use the default configure task with additional definition of CONFIGURE_ARGS += DESTDIR=$(INSTALL_DIR)
? (didn't try myself).
.PHONY: myPreCopy | ||
myPreCopy: | ||
cp PLIST $(WORK_DIR)/PLIST | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PLIST is handled by build framework and must not be copied like this.
Motivation: I have been able to build the command line program of the famous open-source video transcoder program: HandBrake. I also builded it with the Intel Quick Sync option. I build it specifically for the ApolloLake architecture, but I assume it should work OK with the other x86_64 architectures. So far it works for me like a charm, and the QSV encoding works very fast and a produce files with a very good quality. Specially, it make very easy the encoding of DVD's and Blue-rays.
Linked issues: To accomplish the HandBrake compilation I had to update the FFmpeg configuration options (HandBrake require to build FFmpeg with a very specific set of options), for this, I made use of a new variable (HANDBRAKE) to determine what configurations options should be used, this variable is automatically assigned when HandBrake is being build.
It also required the addition of the new packages: libdvdread, Libdvdnav, dav1d, harfbuzz and jansson.
Package FFmpeg is updated to 4.2.2
Checklist
all-supported
completed successfully