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-full: nvenc doesn't imply nonfree licensing #55442

Merged
merged 3 commits into from May 10, 2019

Conversation

jacereda
Copy link
Contributor

@jacereda jacereda commented Feb 8, 2019

Motivation for this change

Looks like this assertion was added when NVENC implied non-free.
After applying this change, you can check the generated executable is distributable with:

ffmpeg -L

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@c0bw3b
Copy link
Contributor

c0bw3b commented May 3, 2019

In order to build ffmpeg with nvenv you have to bring in nvidia-video-sdk which is unfree.
Moreover official instructions to enable nVidia hardware acceleration in ffmpeg recommends using --enable-cuda --enable-cuvid --enable-nvenc --enable-nonfree configure flags.

Even if the resulting built package is redistributable, it's still unfree.

@c0bw3b c0bw3b closed this May 3, 2019
@jacereda
Copy link
Contributor Author

jacereda commented May 9, 2019

Not so fast, please reopen. nvidia-video-sdk isn't needed at all.

@c0bw3b
Copy link
Contributor

c0bw3b commented May 9, 2019

From the official LICENSE notice from ffmpeg git tree :

The NVENC library, while its header file is licensed under the compatible MIT license, requires a proprietary binary blob at run time, and is deemed to be incompatible with the GPL. We are not certain if it is compatible with the LGPL, but we require --enable-nonfree even with LGPL configurations in case it is not.

@jacereda
Copy link
Contributor Author

jacereda commented May 9, 2019

"We are not certain" doesn't sound too convincing.

https://devtalk.nvidia.com/default/topic/1015990/video-codec-sdk/lgpl-ffmpeg-and-nvenc-in-a-closed-source-commercial-application-/

I'd say the LICENSE file is obsolete, since they take care of all the nonfree stuff in the configure script and it allows to enable nvenc in a LGPL setting.

@jacereda
Copy link
Contributor Author

jacereda commented May 9, 2019

@c0bw3b
Copy link
Contributor

c0bw3b commented May 9, 2019

"We require" is convincing enough, and I don't consider a git message or a forum post to be more official and "convincing". As a distro we can't disregard a licence notice, as I'm sure you understand.
You should take this issue upstream and ask for their notice to be updated if the situation has changed.

@c0bw3b c0bw3b reopened this May 9, 2019
@grahamc
Copy link
Member

grahamc commented May 9, 2019

I'd propose closing this PR until it is more clear.

Copy link
Contributor

@c0bw3b c0bw3b left a comment

Choose a reason for hiding this comment

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

Clarify licensing with upstream

@jacereda
Copy link
Contributor Author

jacereda commented May 9, 2019

https://trac.ffmpeg.org/ticket/7893#ticket

@jacereda
Copy link
Contributor Author

Clarified. The LICENSE file has been fixed upstream.

@c0bw3b
Copy link
Contributor

c0bw3b commented May 10, 2019

They were fast to react! Thanks for your effort to clarify.
It makes a lot more sense when replacing nvidia-video-sdk with nv-codec-headers entirely.

@jacereda would you care to also update nv-codec-headers and correct its license (MIT and not GPL3) in another commit?
Or it can be another PR if you prefer (it would also impact handbrake and mpv but I don't expect much breakage from this bump)

@jacereda
Copy link
Contributor Author

Fixed the license field.
The update is less important, it will be required at some point when building a newer FFmpeg, but for now it's OK.

@c0bw3b c0bw3b merged commit 6cd0603 into NixOS:release-18.09 May 10, 2019
vcunat added a commit that referenced this pull request May 11, 2019
vcunat added a commit that referenced this pull request May 11, 2019
It's a port of #55442 to master.

(cherry picked from commit b1565e5)
@vcunat
Copy link
Member

vcunat commented May 11, 2019

Eh, well, ported to master and 19.03, along with a couple other fixes.

@c0bw3b
Copy link
Contributor

c0bw3b commented May 11, 2019

Woah I totally missed this was targeted at release-18.09 and not master..
Thanks @vcunat

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants