-
-
Notifications
You must be signed in to change notification settings - Fork 843
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
docs: Clarify license #3130
docs: Clarify license #3130
Conversation
@cyanreg, could you verify that this correctly and clearly obeys the license terms for FFmpeg's nellymoser decoder? Thank you! |
LICENSE.md
Outdated
|
||
# nellymoser-rs | ||
|
||
nellymoser-rs is licensed under the MIT License and dervied from the nelly2pcm and FFmpeg projects. |
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.
should be derived
LICENSE.md
Outdated
Copyright (c) 2007 a840bda5870ba11f19698ff6eb9581dfb0f95fa5, | ||
539459aeb7d425140b62a3ec7dbf6dc8e408a306, and | ||
520e17cd55896441042b14df2566a6eb610ed444 | ||
Copyright (c) 2007 Loic Minier <lool at dooz.org>, Benjamin Larsson |
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.
Not a lawyer, but our code is LGPL. To import code with other licenses we have to keep around their original license. Sometimes we put it under our regular license header, but in this case we kept it as-is. But as far as I know, that doesn't cover any changes we make to it, which would still be under the LGPL.
Again, really not a lawyer, but last time I wanted to use some MIT-licensed code in FFmpeg I was told to put the LGPL license on top and the original MIT license below. And I think since the LGPL header missing in this case, it's implicit from the project's overall license?
Someone else could compare what changes we've made and whether any of our changes made it in, since if they didn't, the decoder would be purely based on nelly2pcm, I guess?
But if you're already intending to use some FFmpeg decoders through a wrapper, why not use this one as well? We have more optimizations this code doesn't have since we share handwritten SIMD between decoders.
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.
The original nelly2pcm is attributed to the three MD5ed authors. In FFmpeg, the additional authors attributed are Benjamin Larsson and Loic Minier, via commits to FFmpeg, and this attribution is specifically placed as part of the MIT copyright, so I assume this was under the MIT License.
Additionally, the FFmpeg LICENSE.md states that "Most files in FFmpeg are under the GNU Lesser General Public License version 2.1 or later" (not all files), and then later "There are a handful of files under other licensing terms". Nellymoser is not explicitly named in the LICENSE.md, but given that the nellymoser files do not contain a (L)GPL License like most other FFmpeg files, and only contains an MIT License, it seems more explicit to me that it is MIT licensed.
Naturally, IANAL/YANAL, and not arguing in bad faith; only showing how I arrived at the POV that this is MIT licensed. Additionally, we started this work based on nelly2pcm before switching to FFmpeg's version, seeing that it was more up to date and still under MIT. (I haven't specifically diffed the nelly2pcm with FFmpeg's sources).
But if you're already intending to use some FFmpeg decoders through a wrapper, why not use this one as well? We have more optimizations this code doesn't have since we share handwritten SIMD between decoders.
Mainly for ease-of-compilation to Wasm. We are using the wasm32-unknown-unknown target, so pure Rust solutions allow for ease of use, compilation, and smaller file size, avoiding emscripten (and naturally SIMD in Wasm is still a fantasy for the near future). However, for desktop, I would indeed prefer to link to FFmpeg/system libs, although Nellymoser is small enough to not be a huge deal. Additionally, some of the contributor PRs are showing that it may be worthwhile to link to these C libraries without the whole of emscripten by stubbing out some of libc, so we may indeed pursue this route.
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.
Additionally, the FFmpeg LICENSE.md states that "Most files in FFmpeg are under the GNU Lesser General Public License version 2.1 or later" (not all files), and then later "There are a handful of files under other licensing terms". Nellymoser is not explicitly named in the LICENSE.md, but given that the nellymoser files do not contain a (L)GPL License like most other FFmpeg files, and only contains an MIT License, it seems more explicit to me that it is MIT licensed.
Sound correct to me. We do have some hard-MIT license code in our repo (libavutil/x86/x86util.asm) after all.
So the PR looks good to me.
Mainly for ease-of-compilation to Wasm. We are using the wasm32-unknown-unknown target, so pure Rust solutions allow for ease of use, compilation, and smaller file size, avoiding emscripten (and naturally SIMD in Wasm is still a fantasy for the near future).
Not a fantasy. I'm actually working on a transassembler for arm64 asm to wasm for our SIMD right now. The idea is to use this in both FFmpeg, dav1d and other projects too, since it's just a GNU 'as' slot-in replacement.
SIMD support across Wasm implementations is pretty extensive already, and definitely usable.
Should allow for pretty much near-native decoding of everything once done and integrated. Which is kind of why I brought this up.
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.
Correct me if I'm wrong, but Wasm SIMD is still disabled behind experimental flags in Chrome+Firefox and will be for the foreseeable future. I know it's in-progress, but we've also been stuck with the same story regarding other features for a long time (WebGPU, etc.), so I generally plan assuming these won't be available for some time (a year+). It becomes useful to us when it is widely available (i.e. iOS Safari. :-). Regardless, SIMD would make it worthwhile to link to FFmpeg implementations, and getting that in before it's commonplace would be a benefit.
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.
Not a fantasy. I'm actually working on a transassembler for arm64 asm to wasm for our SIMD right now. The idea is to use this in both FFmpeg, dav1d and other projects too, since it's just a GNU 'as' slot-in replacement.
SIMD support across Wasm implementations is pretty extensive already, and definitely usable.
Should allow for pretty much near-native decoding of everything once done and integrated. Which is kind of why I brought this up.
As another point of view: So far, my experience with any non-default wasm features has been discouraging. From https://webassembly.org/roadmap/ , one widely supported feature is mutli-value, and currently LLVM segfaults when trying to compile simple Rust code with it (rust-lang/rust#73916 , I'm hoping LLVM 12 upgrade will fix it). Another, bulk memory operations, is unsupported on Safari - and while marked as "standardized", on another page (https://github.com/WebAssembly/proposals) is still in stage 4 and not actually merged to the spec yet. This state hasn't changed for the past six months; the next poll on actually standardizing them is supposed to be in 3 days.
(Plus, aside from LLVM and browsers themselves, toolchain components in between also seem vastly unprepared for these extensions. Only recently I managed to go the entire path from compiled wasm file with bulk-memory
to working demo page, and only because we happened - for unrelated reasons - to stop using wasm-pack and webpack's wasm loader.)
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.
It's not stable yet, but it's backwards compatible, and we can runtime toggle it depending on whether the implementation supports it, similar to how we handle architectures and whether they support various SIMD extensions.
I think it's adoption depends on having a large enough use case for it, testing it thoroughly and expanding the spec where lacking. So hopefully we'll help with that.
683e60c
to
058f4bd
Compare
Clarify license of nellymoser as per discussion in #3004.