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

ffmpeg: refactor, ffmpeg -> ffmpeg-full & add ffmpeg (minimal) #7160

Closed
wants to merge 1 commit into from

Conversation

codyopel
Copy link
Member

@codyopel codyopel commented Apr 3, 2015

With this PR the default ffmpeg build now only includes dependencies required by builds that link against or use ffmpeg. The current feature-complete build is moved to ffmpeg-full and backwards compatibility has been removed. ffmpeg-full will use the latest version of ffmpeg available which allows the versioning to be removed from the build simplifying things. The default ffmpeg build is now simpler and easier to maintain, and still supports all versions of ffmpeg.

I did not add network support or x11 screen grabbing support because I couldn't find any builds that use it, if anyone knows of any please let me know and I will re-add it. Most link directly against libX11, etc... instead of using ffmpeg.

@edolstra
This PR should resolve 3779aa0 in a clean way and help with #7117.

The mixed hyphen and camel case has been removed, it was caused because I copied optionals directly from the configure script and appended a suffix. The faacSupport vs faacExtlib suffix is because all optionals are seperated into categories, Extlib, Programs, Library, Licensing, Documentation, Build & Developer, however only the external libraries that are disabled by default use this.

@peti
I migrated it to use stdenv.lib.enableFeature, I wasn't aware a builtin function existed.

@vcunat
I also added the inherit (stdenv) isLinux etc... changes you mentioned

@spwhitt @henrytill
I changed the way the darwin fixes were implemented a bit for ffmpeg & ffmpeg-full so if the changes break anything on Darwin let me know.

@copumpkin @Fuuzetsu

@codyopel codyopel force-pushed the ffmpeg-minimize branch 2 times, most recently from 1106164 to 9a89a59 Compare April 4, 2015 00:58
@spwhitt
Copy link
Contributor

spwhitt commented Apr 4, 2015

Everything builds and works great on Darwin except ffmpeg_0_5 which fails at configure with

Unknown option "--disable-libvpx".

But that's a minor issue. This looks awesome. I'm excited.

@codyopel
Copy link
Member Author

codyopel commented Apr 4, 2015

Yeah, the darwin work-around always passes the disable flag. I'll fix it for libvpx and pulseaudio.

@vcunat
Copy link
Member

vcunat commented Apr 4, 2015

Yes, I expect most packages only need a light version. This not only reduces the closure of the output, but also build-time closure, which may simplify some rebuilds nontrivially.

This won't allow using extra features with older ffmpeg versions – we'll see in future if that's needed or not. Of course, not having to maintain support for it seems good, if we can avoid it.

@codyopel
Copy link
Member Author

codyopel commented Apr 4, 2015

@spwhitt
I updated the commit, this should fix the darwin work-arounds and now it will work for versions < 0.9

@vcunat
Yeah, kinda, the only disadvantage to not maintaining "full" older versions is the changes the the cli syntax, otherwise you aren't losing any features. Programs that link against ffmpeg don't use the extra features, I have included all the features that they do use though.

@vcunat
Copy link
Member

vcunat commented Apr 4, 2015

Dow do you determine what features are used by programs linking against ffmpeg?

@codyopel
Copy link
Member Author

codyopel commented Apr 4, 2015

@vcunat
Painfully browsing their configure scripts/source.

@henrytill
Copy link
Member

On Darwin, trying to build ffmpeg_0_5 with these changes, during the configure phase, I get:

Unknown C compiler clang
clang is unable to create an executable file.

@henrytill
Copy link
Member

Otherwise, with these changes,

  • ffmpeg_0_10
  • ffmpeg_0
  • ffmpeg_1
  • ffmpeg_2_2
  • ffmpeg(2.5.4)
  • ffmpeg-full

all build on Darwin (10.10.2) without error.

@codyopel
Copy link
Member Author

codyopel commented Apr 5, 2015

@henrytill
I don't think we tested 0.5 with the darwin changes since nothing builds against it (only broken packages technically do). It probably doesn't have a --cc=clang option in that older version which is the reason, I'll look into it and see what I can do, but as long as 0.10+ work we should be fine. Thanks for testing and letting me know.

+ adds a minimal dependency version of ffmpeg as the default
+ the current ffmpeg changes have been moved to ffmpeg-full
+ ffmpeg default 2.5 -> 2.6
+ removed ffmpeg 0.5 & 2.5 (unused versions)
@codyopel
Copy link
Member Author

codyopel commented Apr 7, 2015

Only two packages used ffmpeg 0.5, both were broken so I removed it. I fixed fuppes and got the latest version working (still need to fix the service), but ffmpeg support is explicitly disabled in their configure script since it is broken at the moment so it isn't needed and ultrastardx I don't think has ever built successfully (but should work with 0.10 after reviewing their source).

Since @spwhitt fixed rtmpdump & wavpack on darwin I re-enabled them.

@vcunat
I also went ahead and switched the default version to 2.6. The library versions haven't changed, the last major revision to any libraries was in the 2.2 -> 2.3 update (which broke vlc) so everything should build against it just fine, there shouldn't be any api breakage.

Added a final fix for ffmpeg-full to use libv4l instead of v4l-utils which will reduce dependency bloat significantly.

@vcunat vcunat self-assigned this Apr 7, 2015
@vcunat
Copy link
Member

vcunat commented Apr 7, 2015

OK, in staging now.

@edolstra
Copy link
Member

edolstra commented Apr 7, 2015

So this depends on Qt4, JACK etc. again? IMHO that's not desirable even for a "full" version.

Also, doesn't this increase the maintenance burden by adding yet another version of ffmpeg?

@vcunat
Copy link
Member

vcunat commented Apr 7, 2015

Actually, the maintenance seems slightly simplified in comparison to the state before this PR (perhaps it's subjective, and git/hub doesn't show the diff well): there was one huge expression supporting all possible features on all versions. Both expressions are now simpler than the one before.

@codyopel
Copy link
Member Author

codyopel commented Apr 7, 2015

@edolstra
Only ffmpeg-full uses v4l, but I switched it from v4l-utils -> libv4l so it won't pull in QT, mariadb, etc...

@edolstra @vcunat
Yeah, I simplified maintenance quite a bit because the default version (ffmpeg) includes backwards compatibility for all versions, but now only includes features required by anything that builds against it. Because the features are required by something building against it they are not optional (thus removing most optionals). The full ffmpeg (ffmpeg-full) keeps all the optionals, but no longer includes backwards compatibility which simplifies that version.

ffmpeg-full is only used if you want to use ffmpeg directly, not to build against so you only pull in the dependency bloat if you want additional ffmpeg features (some of these could be disabled).

@vcunat
Copy link
Member

vcunat commented Apr 8, 2015

I assumed Qt was pulled by qtFaststartProgram ? true, but I don't even see qt explicitly added to buildInputs.

@codyopel
Copy link
Member Author

codyopel commented Apr 8, 2015

@vcunat
Nah, qt-faststart is quicktime, QT was pulled in by v4l-utils, but I changed it to use libv4l so it will no longer pull QT, and its deps.

@vcunat
Copy link
Member

vcunat commented Apr 8, 2015

:-) in the meantime I checked it doesn't pull Qt (not even in build-time closure). Jack seems rather small in comparison to many other dependencies. IMO much more will be gained by #7117 techniques which should mainly get rid of development stuff (e.g. now we have libtool in closure and many others).

@vcunat vcunat closed this in 033472c May 11, 2015
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.

5 participants