Skip to content
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

Updates for ffmpeg, mediainfo and chromaprint #3136

Merged

Conversation

ymartin59
Copy link
Contributor

@ymartin59 ymartin59 commented Feb 4, 2018

Motivation: Deliver packages to new architectures rtd1296, broadwell, broadwellnk, apollolake...
Linked issues: #2718 #3119

  • Build rule all-supported completed successfully (except chromaprint 1.4.3 on 88f6281 because of GCC version and C++ standard)
  • Package upgrade completed successfully
  • New installation of package completed successfully

@m4tt075
Copy link
Contributor

m4tt075 commented Feb 5, 2018

@ymartin59 Thanks for picking this up! Much appreciated.

I have two questions:

  1. This is related to HW transcoding: Would it be possible to enable VAAPI support on appropriate platforms (I assume that there are no Syno platforms utilizing NVENC, MMAL or OMX)?
  2. The statically linked ffmpeg version Tvheadend is using is patched. See here for the patches applied. I would like to add them if (and only if) ffmpeg is cross-compiled as a dependency of Tvheadend. Could you create some hook to achieve that? Happy to add the patches via PRing against your branch, if you are fine with it. Just let me know...

@ymartin59
Copy link
Contributor Author

@m4tt075 I am working to get mediainfo and chromaprint dynamically linked against ffmpeg package libraries... If OK, sure it sounds relevant to "improve" ffmpeg so that tvheadend can also use that libraries.
I have reviewed https://www.ffmpeg.org/legal.html because of "License: nonfree and unredistributable" reported after configure... and I am quite uncomfortable about it

@m4tt075
Copy link
Contributor

m4tt075 commented Feb 5, 2018

OK, interesting, and very relevant. Tvheadend is already linking ffmpeg dynamically and has been for some time. It is just not utilizing all optimizations. But I see your point, and that it is way more important to figure out. I will just keep on tracking this PR. Maybe we can get back to the optimizations at a later stage. No urgency...

@ymartin59
Copy link
Contributor Author

@m4tt075 I have investing --enable-vaapi, adding https://github.com/intel/libva which requires libdrm... and I just guess all this stuff is intel only. Am I right ?

@m4tt075
Copy link
Contributor

m4tt075 commented Feb 15, 2018

@ymartin59 I believe you are right that vaapi is for Intel only. I'm not sure which libraries are needed for transcoding support though. Looking at the ffmpeg configuration for TVH, libva does not seem to be required. But I'm purely guessing here. I don't know enough about ffmpeg to judge. Maybe @cytec knows? I think he has been working on ffmpeg quite a lot...

@ymartin59
Copy link
Contributor Author

@m4tt075 Following these instructions https://gist.github.com/Brainiarc7/eb45d2e22afec7534f4a117d15fe6d89
I fear VA API requires building and release additional kernel modules...

@m4tt075
Copy link
Contributor

m4tt075 commented Feb 15, 2018

@ymartin59 Yikes! That looks way more complicated than what it takes for TVH. Look, don't spend too much time on this, please. If it works, great, if it doesn't, please don't worry...

@ymartin59 ymartin59 force-pushed the update-ffmpeg-mediainfo-chromaprint branch from 0edca0a to 9acf2a3 Compare February 17, 2018 06:53
@ymartin59 ymartin59 changed the title WIP updates for ffmpeg, mediainfo and chromaprint Updates for ffmpeg, mediainfo and chromaprint Feb 17, 2018
@ymartin59 ymartin59 requested review from Diaoul and cytec February 17, 2018 06:55
Copy link
Contributor Author

@ymartin59 ymartin59 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cytec @Diaoul I have found a way to build and run chromaprint against ffmpeg package libraries... But I am not completely satisfied about Makefile code. Have you got some ideas to improve it ?

You should only review chromaprint commit: 9acf2a3

@ymartin59
Copy link
Contributor Author

ymartin59 commented Feb 17, 2018

Just found one issue with:

xpenology52> /usr/local/ffmpeg/bin/x265 -version
/usr/local/ffmpeg/bin/x265: error while loading shared libraries: libx265.so.130: cannot open shared object file: No such file or directory

But works with export LD_LIBRARY_PATH=/var/packages/ffmpeg/target/lib/.
It looks like that binary (and only that one) is linked without install prefix !

@ymartin59
Copy link
Contributor Author

ymartin59 commented Feb 17, 2018

@m4tt075 About vaapi, I think configure is looking for VA availability on hardware thanks to loaded modules like drm or /sys. Of course, there are troubles with Docker environment and cross-compilation context. I have postponed until I have new ideas: ymartin59#6

@ymartin59 ymartin59 mentioned this pull request Feb 17, 2018
3 tasks
@ymartin59
Copy link
Contributor Author

Now I have investigated x265 support... I face another issue: multibit support is quite an odd compilation process: ymartin59#7
I cannot left behind defaults 8 bits to replace by 10 or 12... it does not make sense. Package should be provided with these three options available.

@cytec
Copy link
Member

cytec commented Feb 19, 2018

@ymartin59 regarding the chromaprint makefile: why are you checking for the spk to be present on the build machine? personally i don't care if it gets rebuilt everytime, sure it takes a while but most tools don't need to be published that often (et least not those who link against ffmpeg) so i'd be fine with them building it again... i don't know if dynamic linking makes any sense there. The reason i didn't do it back then was to keep it simple. One should be able to install chromaprint without the need of installing ffmpeg first which he might not even need except for the libs that are needed by chromaprint ...

PS: afaik vaapi is intel only and not needed for tvh

@ymartin59
Copy link
Contributor Author

@cytec Thanks for your answer. I investigated this shared library concept because of compilation time and package volumes managed in repository - ffmpeg libraries are required in chromaprint and tvheadend. Now chromaprint package are small and built faster. In general both are delivered together when ffmpeg package is updated. vaapi was requested by ffmpeg users.

@ymartin59
Copy link
Contributor Author

ymartin59 commented Feb 20, 2018

Now I have enabled multibit x265 support... it no longer builds for rtd1296. A work-around with -DENABLE_ASSEMBLY=OFF but not satisfying.

@cytec
Copy link
Member

cytec commented Feb 21, 2018

im perfectly fine with using shared lib's for ffmpeg ;)
but im not really sure how the best approach would be for building those SPK's i think the way you currently doing it is fine for now... another way would be to just use shared libs but rebuild them everytime which might help getting rid of the makefile stuff... but imho that's something we'd have to consider: smaller packages which depent on each other with slightly less beautiful make files or bigger packages with longer build times and a cleaner makefile...

just a random thought: what about doing something like the native depend stuff for those cases if we get more of them? so something like "platform shared" or similar?

@m4tt075
Copy link
Contributor

m4tt075 commented Feb 22, 2018

@ymartin59 I just ran some tests. It seems all arm-platforms are broken. At least I could not compile them. As ppc-platforms are broken too, basically only x86/x64 platforms are left. Do you want depth or breadth? Or we compile 10 and 12 bit for Intel platforms only, but then it is getting more and more cumberrsome and complex which is not ideal either.

- enable neon for ARMv7 and ARMv8 and increment SPK version
- upgrade nasm to 2.13.02 to fix x264 compilation on x86 and x64 platforms
@ymartin59 ymartin59 force-pushed the update-ffmpeg-mediainfo-chromaprint branch from 4a2f830 to cbf9d26 Compare February 24, 2018 09:00
@m4tt075
Copy link
Contributor

m4tt075 commented Feb 24, 2018

@ymartin59 Thanks for adding all those Tvheadend patches. I don't understand enough of the code, so I'd rather ask: Is it ok to apply them independent of which spk is actually compiled? Or should we only apply them conditionally for the case that the ffmpeg cross-package is compiled as part of the Tvheadend spk? Just worried we might break non-TVH packages by applying those patches across the board. What do you think?

@ymartin59
Copy link
Contributor Author

@m4tt075 I have reviewed TVH patches to ffmpeg and related libraries. They are "harmless" and only add some switches and events for progress notification. That is why I applied them unconditionally.
I invite you to reuse make command to link against ffmpeg package instead of rebuild and include it in TVH package: see spk/chromaprint/Makefile and cross/chromaprint/Makefile from cbf9d26

@m4tt075
Copy link
Contributor

m4tt075 commented Feb 24, 2018

@ymartin59 Cool! Thanks. Great idea to link to the ffmpeg package. Should save a considerable amount of time building Tvheadend again and again.

* expat 2.2.5
* fontconfig 2.12.6
* freetype 2.9
* lame 3.100
* libass 0.14.0
* libbluray 1.0.2
* libogg 1.3.3
* libpng 1.6.34
* libxml2 2.9.7
* openjpeg 2.3.0
* applies tvheadend patches
* x265 2.6 multilib with bit depth support 8, 10 and 12
Allow building against ffmpeg package libraries
@ymartin59 ymartin59 force-pushed the update-ffmpeg-mediainfo-chromaprint branch from cbf9d26 to f3d629f Compare February 25, 2018 06:10
@ymartin59 ymartin59 merged commit 1aaf89b into SynoCommunity:master Feb 25, 2018
@ymartin59 ymartin59 deleted the update-ffmpeg-mediainfo-chromaprint branch February 25, 2018 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants