Bug 73163 – Remove platform/audio/fftw

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adam Barth]

Remove platform/audio/fftw

[Adam Barth]

Created attachment 116666 [details]
Patch

[Philippe Normand]

Is there a discussion about a possible alternative to it?

[Philippe Normand]

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.

[Adam Barth]

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

[Philippe Normand]

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

[Soo-Hyun Choi]

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

[Philippe Normand]

(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

[Kenneth Russell]

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

[Adam Barth]

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.

[Chris Rogers]

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

[Adam Barth]

Thanks Chris.

[Philippe Normand]

(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!

[Philippe Normand]

(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!

[Adam Barth]

Ok.  It sounds like we should land this patch then (possibly including a change to configure.ac).

[Philippe Normand]

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

[Kenneth Russell]

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.

[Chris Rogers]

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.

[Adam Barth]

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.

[Chris Rogers]

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.

[Soo-Hyun Choi]

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.

[Adam Barth]

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.

[WebKit Review Bot]

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

[Adam Barth]

Committed r102622: <http://trac.webkit.org/changeset/102622>