| Summary: | Remove platform/audio/fftw | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Adam Barth <abarth> | ||||
| Component: | New Bugs | Assignee: | Adam Barth <abarth> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | crogers, donggwan.kim, dwim79, dw.im, kbr, pnormand, s.choi, webkit.review.bot | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Adam Barth
2011-11-26 22:16:32 PST
Created attachment 116666 [details]
Patch
Is there a discussion about a possible alternative to it? It'd be great if the patch updates configure.ac as well, no need to check for libfftw there if support for it is removed. > Is there a discussion about a possible alternative to it?
Are you using this code? If so, I'm happy to leave it in the tree, but you should check whether you're complying with the various licenses. IANAL, but s.choi pointed out that there might be some license compatibility problems between FFTW and WebKit.
(In reply to comment #4) > > Is there a discussion about a possible alternative to it? > > Are you using this code? If so, I'm happy to leave it in the tree, but you should check whether you're complying with the various licenses. IANAL, but s.choi pointed out that there might be some license compatibility problems between FFTW and WebKit. I'll test your patch in my webaudio branch and report back :) If needed we can hook in the FFT GStreamer lib anyway, it's LGPL. (In reply to comment #5) > (In reply to comment #4) > > > Is there a discussion about a possible alternative to it? > > > > Are you using this code? If so, I'm happy to leave it in the tree, but you should check whether you're complying with the various licenses. IANAL, but s.choi pointed out that there might be some license compatibility problems between FFTW and WebKit. > > I'll test your patch in my webaudio branch and report back :) > If needed we can hook in the FFT GStreamer lib anyway, it's LGPL. I would think FFT GStreamer can be a nice alternative. FFTW.org is GPL, so it may lead into potential license conflict with WebKit. (In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > > Is there a discussion about a possible alternative to it? > > > > > > Are you using this code? If so, I'm happy to leave it in the tree, but you should check whether you're complying with the various licenses. IANAL, but s.choi pointed out that there might be some license compatibility problems between FFTW and WebKit. > > > > I'll test your patch in my webaudio branch and report back :) > > If needed we can hook in the FFT GStreamer lib anyway, it's LGPL. > > I would think FFT GStreamer can be a nice alternative. Yes, for the ports already using GStreamer :) We indeed need a replacement for FFTW, I'll try to provide one using gstfft for the folks interested. > FFTW.org is GPL, so it may lead into potential license conflict with WebKit. Yes. I think Chris Rogers chosed FFTW initially because it's very very fast :) It seems we could contact MIT and get a unlimited-use license, see http://www.fftw.org/faq/section1.html#isfftwfree (In reply to comment #7) > (In reply to comment #6) > > (In reply to comment #5) > > > (In reply to comment #4) > > > > > Is there a discussion about a possible alternative to it? > > > > > > > > Are you using this code? If so, I'm happy to leave it in the tree, but you should check whether you're complying with the various licenses. IANAL, but s.choi pointed out that there might be some license compatibility problems between FFTW and WebKit. > > > > > > I'll test your patch in my webaudio branch and report back :) > > > If needed we can hook in the FFT GStreamer lib anyway, it's LGPL. > > > > I would think FFT GStreamer can be a nice alternative. > > Yes, for the ports already using GStreamer :) > We indeed need a replacement for FFTW, I'll try to provide one using gstfft for the folks interested. > > > FFTW.org is GPL, so it may lead into potential license conflict with WebKit. > > Yes. I think Chris Rogers chosed FFTW initially because it's very very fast :) > It seems we could contact MIT and get a unlimited-use license, see http://www.fftw.org/faq/section1.html#isfftwfree There were some attempts to secure a non-GPL license for the use of FFTW in WebKit, but these were so far unsuccessful. There's been no activity on this front for several months. Chromium is currently using ffmpeg's FFT on Windows and Linux, because that library is already being linked in to the browser, so there was no increase in code size by doing so. I personally don't mind r+'ing this patch, though think it would be best if Chris Rogers would be given the chance to comment. He's currently on vacation but due back in a week or two. We can wait for him to get back. There's no particular rush on this patch. It just seems like a good cleanup so folks don't accidentally walk into a licensing trap. Adam, please don't remove this code as the GTK port relies on it. Presumably their license obligations are less stringent than the other ports since much of their code is GPL. The mac port uses Accelerate.framework, while chromium uses FFmpeg Thanks Chris. (In reply to comment #10) > Adam, please don't remove this code as the GTK port relies on it. Presumably their license obligations are less stringent than the other ports since much of their code is GPL. > Our code is LGPL actually. We are in violation of the LGPL if we link against FFTW indeed. But this is unlikely to happen currently because our WebAudio support is not fully landed and not yet enabled by default. Reopening then! (In reply to comment #12) > (In reply to comment #10) > > Adam, please don't remove this code as the GTK port relies on it. Presumably their license obligations are less stringent than the other ports since much of their code is GPL. > > > > Our code is LGPL actually. > We are in violation of the LGPL if we link against FFTW indeed. But this is unlikely to happen currently because our WebAudio support is not fully landed and not yet enabled by default. > > Reopening then! I created bug 73295 to disable WebAudio on GTK. We'll re-enable it when the gstfftw-based implementation lands. So GTK is not blocking this patch anymore! Ok. It sounds like we should land this patch then (possibly including a change to configure.ac). (In reply to comment #14) > Ok. It sounds like we should land this patch then (possibly including a change to configure.ac). The configure.ac change is not needed I think. I can just fix it up when we add the GStreamer fft implementation. Comment on attachment 116666 [details]
Patch
I'd still like crogers to give a thumbs up to this removal, but otherwise seems fine to me.
MIT *does* have commercial license options for FFTW which make it compatible with LGPL, so in theory a port could use FFTW. And since, performance-wise, FFTW is one of the better options we have, it would be nice to avoid removing this code. But, I'm not a lawyer and I don't know how best to call out this code as needing special care. At the least, maybe we should add some precautionary comments in the header file? Maybe we can bring this discussion up on webkit-dev, so that the more legal-oriented folks can weigh in. It would be nice to keep the code if possible, for potential use by ports with the appropriate commercial license. Rather than having a bunch of code that someone might use one day, we can add the code back if someone actually wants to use it. That seems reasonable, although it makes it difficult for ports to know that the code is available if it's not actually visible in the tree. I don't want to hold this back if everybody else wants it removed. As said in their homepage, FFTW can be purchased to be compatible with LGPL, and we are aware of it. But as I pointed earlier in the chrome-dev, this could potentially introduce a license trap, leading to a serous license violation. I would like it to be removed from WebKit source tree, leaving it to a browser vendor's decision whether or not to implement a necessary port as their necessary. I don't mind bring this issue to webkit-dev, though. Comment on attachment 116666 [details]
Patch
Given that no one is using this code today, I think we're better off removing it. If someone wants to use it in the future, then we can figure out how to let them do that without creating a license trap for others.
Comment on attachment 116666 [details] Patch Rejecting attachment 116666 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: LED at 1305. Hunk #3 FAILED at 1920. 3 out of 3 hunks FAILED -- saving rejects to file Source/WebCore/WebCore.gyp/WebCore.gyp.rej patching file Source/WebCore/platform/audio/FFTFrame.h patching file Source/WebCore/platform/audio/FFTFrameStub.cpp patching file Source/WebCore/platform/audio/fftw/FFTFrameFFTW.cpp rm 'Source/WebCore/platform/audio/fftw/FFTFrameFFTW.cpp' Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Eric Seidel', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/10827924 Committed r102622: <http://trac.webkit.org/changeset/102622> |