-
Notifications
You must be signed in to change notification settings - Fork 888
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
[rbp] Add extra FFmpeg HEVC dependencies #4620
Conversation
Does this require a specific ffmpeg version? What else is required for this to work? Just some patches from the newclock5 branch? |
export CFLAGS="-I$SYSROOT_PREFIX/usr/include/interface/vcos/pthreads -I$SYSROOT_PREFIX/usr/include/interface/vmcs_host/linux -DRPI=1 $CFLAGS" | ||
export FFMPEG_LIBS="-lbcm_host -lvcos -lvchiq_arm -lmmal -lmmal_core -lmmal_util -lvcsm" | ||
else | ||
export LDFLAGS="$LDFLAGS -fPIC" |
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.
Why -fPIC
for everything else?
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.
Actually I can't remember why it was done like that. I've just tested with -fPIC
for RPi2 and it's working for HEVC and non-HEVC so I'll update the commit back to how it was with -fPIC
for everything.
No, wait. There's a collection of ffmpeg patches that implement the HEVC optimisations that are required as well (on top of 2.8.4). I forgot about them as they're being automatically applied in my builds after being extracted from newclock5 (where they apply against the Kodi ffmpeg) - I then apply them against the external OpenELEC ffmpeg package. We (I'll) need to add those extra HEVC ffmpeg patches to this PR once popcornmix is happy with them. Until then leave this PR to one side (although it could be merged as it currently stands, it shouldn't do any harm). I'm not sure if the plan is for the HEVC FFmpeg optimisations for RBP to go upstream - hopefully that is the case. |
[rbp] Add extra FFmpeg HEVC dependencies
Was this ok to merge? I thought there was supposed to be some extra patches added? |
Should be, but wasn't meant to be....
Yes there are, but currently not ETA on that so merging this now was completely unnecessary. |
Obviously submitting PRs that are not meant for immediate merging is now a risky business unless people actually read the PR, so this practice is probably best avoided, which most likely will mean fewer PRs (as they won't be pushed until the author believes them to be 100% ready for merging) and less visibility on upcoming changes/opportunity for comment. |
Then we have to create and Maintain a - next branch which will be more work. This PR is harmless. But if its not complete and ready to merge dort do such PRs. Dont want collect to many for the future. (except we Maintain a branch for this) |
Not really buying your argument but I will steer clear of submitting "next" type PRs in future - I really don't understand why you can't keep your finger off the "merge" button and just allow some PRs to evolve as intended. |
I've had this patch in my builds for quite a while. Latest newclock5 RPi FFmpeg HEVC optimisations require these extra libraries, includes and defines. Should be safe to merge ahead of the very latest HEVC optimisations, which will eventually appear in in the rpb-backports patches.