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

docs: Clarify license #3130

Merged
merged 1 commit into from
Feb 10, 2021
Merged

docs: Clarify license #3130

merged 1 commit into from
Feb 10, 2021

Conversation

Herschel
Copy link
Member

@Herschel Herschel commented Feb 7, 2021

Clarify license of nellymoser as per discussion in #3004.

@Herschel
Copy link
Member Author

Herschel commented Feb 7, 2021

@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.

Choose a reason for hiding this comment

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

should be derived

@relrelb
Copy link
Contributor

relrelb commented Feb 7, 2021

Need to update paths in the CI, as well as in PKGBUILD.

LICENSE.md Outdated
Copyright (c) 2007 a840bda5870ba11f19698ff6eb9581dfb0f95fa5,
539459aeb7d425140b62a3ec7dbf6dc8e408a306, and
520e17cd55896441042b14df2566a6eb610ed444
Copyright (c) 2007 Loic Minier <lool at dooz.org>, Benjamin Larsson
Copy link

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.

Copy link
Member Author

@Herschel Herschel Feb 7, 2021

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.

Copy link

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.

Copy link
Member Author

@Herschel Herschel Feb 7, 2021

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.

Copy link
Collaborator

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.)

Copy link

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.

@Herschel Herschel force-pushed the license branch 2 times, most recently from 683e60c to 058f4bd Compare February 10, 2021 10:52
@Herschel Herschel merged commit 09c6def into ruffle-rs:master Feb 10, 2021
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