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

Fix ffmpeg compilation for rtd1296 architecture #3119

Closed
wants to merge 4 commits into from

Conversation

m4tt075
Copy link
Contributor

@m4tt075 m4tt075 commented Jan 20, 2018

Motivation: ffmpeg compilation has been broken for rtd1296 platform and all packages depending on ffmpeg. This PR fixes those issue. x264 has been upgraded to the most recent stable snapshot in the process as well. Untested so far, as I do not own a rtd1296 device.

Checklist

  • Build rule completed successfully for rtd1296 architecture
  • Build rule all-supported completed successfully
  • Package upgrade completed successfully
  • New installation of package completed successfully

@m4tt075
Copy link
Contributor Author

m4tt075 commented Jan 20, 2018

@ymartin59 Gave it another try and got it working, but please review...

@@ -42,6 +42,10 @@ ifeq ($(findstring $(ARCH),$(ARM7_ARCHES)),$(ARCH))
CONFIGURE_ARGS += --arch=arm --disable-neon --disable-armv6 --disable-armv6t2 --disable-vfp --disable-armv5te --disable-yasm --disable-asm
endif

ifeq ($(findstring $(ARCH),$(ARM8_ARCHES)),$(ARCH))
CONFIGURE_ARGS += --arch=arm64 --disable-neon --disable-yasm --disable-asm
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the disable-nepn required? Because Neon can make a huge performance impact, as far as I know from general programming. It's like SSE2/3 of ARM, so it seems very important on low power devices and I'm pretty sure this platform should support it.

Copy link
Contributor

Choose a reason for hiding this comment

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

According to https://developer.arm.com/technologies/neon we should consider NEON is available for both ARMv7 and ARMv8.
I propose to give a try to remove --disable-neon flag for these two architecture.
May you please build for rtd1297 this PR and submit to requester for "beta" testing ?

@ymartin59
Copy link
Contributor

Reference to requesters: #2997 #3083 #2886 #2981 #3079

@ymartin59
Copy link
Contributor

I have built and submitted package for testing

@m4tt075
Copy link
Contributor Author

m4tt075 commented Jan 21, 2018

Yes, makes sense. Just pushed the changes. Building here.

@Idree
Copy link

Idree commented Jan 21, 2018

Hello, i am betatesting your package :)

I just installed it on my DS216play which is packed with an rtd1296 and video's with DTS seems to be working.

Amazing work 👍

@Safihre
Copy link
Contributor

Safihre commented Jan 21, 2018

@m4tt075 any idea why all the x86/64 toolchains fail? Can't really read the log well here on mobile.

@m4tt075
Copy link
Contributor Author

m4tt075 commented Jan 21, 2018

@Safihre Yes, found it. Working on it. Pushing update soon...

@m4tt075
Copy link
Contributor Author

m4tt075 commented Jan 21, 2018

Next build attempts here

@ymartin59
Copy link
Contributor

Already one user reported rtd1296 package works well

@ymartin59
Copy link
Contributor

I have to review my pending branches with some updates related to ffmpeg, mediainfo and chromaprint

@m4tt075
Copy link
Contributor Author

m4tt075 commented Jan 21, 2018

OK, all 6.1 builds are green. Test packages are here.

@ymartin59 Feel free to cherry-pick my commits into a bigger ffmpeg update PR if you like to. I will be AFK for most of next week. And I know there is more we could do (update to 3.4.1, hw acceleration support for transcoding, specific patches which are only applied when it is compiled as part of TVH).

@ymartin59
Copy link
Contributor

@m4tt075 Thanks a lot. That is my idea. I take opportunity to update as much as possible because of publication process cost...

@Safihre
Copy link
Contributor

Safihre commented Jan 21, 2018

@ymartin59 why not use the (Docker based) Travis image to do the publishing? You can add your SynoCommunity login details as a Travis secret/encrypted environment variable.
This is also how we do it for the SABnzbd releases.

@ymartin59
Copy link
Contributor

Replaced by #3136. Unfortunately no longer builds for DSM 5.2

@ymartin59 ymartin59 closed this Feb 4, 2018
@m4tt075 m4tt075 deleted the rtd-test branch March 17, 2018 11:14
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