libcdio contains quite a bit of code copied from libcdparanoia, so it seems very unlikely to me  that is is going to change license unless libcdparanoia is also relicensed.

Out of curiosity, why would we move the cdio plugin to -ugly, but not the cdparanoia plugin which has the same issue?

Couldn't we just add some API to core that prevents the core from making plugins based on GPL-ed code available to this application?

I think both should move.  I didn't notice cdparanoia since it is in gst-plugins-base rather than in gst-plugins-good.

When you say "couldn't we add some API to core that prevents the core from making plugins base on GPL'ed code available to the application" do you mean that the application would register its license with GStreamer and say "I am a GPL'ed program", and then GPL plugins would be available.  Otherwise they would not be?
This might resolve the problem.

That said, is it a license violation for LGPL'ed code (e.g. GStreamer) to access a GPL library?  While it is okay for a GPL program to link against a LGPL library, I didn't think you were supposed to link the other way around.  If you do, doesn't this effectively make GStreamer a GPL library?  Or is it only if you *use* the code?

Neither - the GPL and LGPL are only about distribution. An end-user is entirely free to add whatever code the like, under whatever license, as long as they don't *distribute* the sum together. This approach of declaring a license and rejecting conflicting licenses doesn't encapsulate the nature of the problem properly.

Created attachment 84865 [details] [review]
patch adding a new cddasrc plugin based on ioctls and LGPL


Artem Kachitchkine and I wrote the attached plugin which implements CDDA using Solaris ioctls.  It might be nice if GStreamer had a plugin that used CDDA without the GPL issues, so this is a start.

I believe Linux ioctls are slightly different for a few of the operations, so it would likely be necessary to modify the code a bit with "#ifdef __sun" and put the Linux code in the #else block.

It is missing some features that would be nice, like jitter protection and damage recovery.  But perhaps this could be implemented later, or a LGPL library may become available that could be used for this part.

One idea may be to use the LGPL'ed version of libparanoia which is a part of OpenSolaris here:

http://src.opensolaris.org/source/xref/sfw/usr/src/cmd/cdrtools/cdrtools-2.01.01/libparanoia/

According to Joerg Schilling (who manages the cdda2wav project at Sun), he was able to relicense this bit of libcdparanoia as LGPL with the permission of the original author.  This version of libparanoia only has code for improving quality of the data read from the CD, and does not have any code for interacting with the CD device directly.  cdda2wav has this logic just in the binary code for cdda2wav.

This cdda2wav version of libcdparanoia is currently statically linked into the Solaris cdda2wav project, so it would likely be necessary to package up this library separately or include needed code in the plugin directly.  Probably should work with Joerg to make this happen.   His email is Joerg.Schilling@Sun.COM.

Oh, I realize that this plugin is currently in the "ext" directory.  This might not be the right location for something that doesn't depend on anything external, though if we make it depend on libcdparanoia as mentioned above, I guess "ext" might be the right place.

At any rate, I propose this for discussion.  As I said, I think having a CDDA plugin that doesn't depend on any GPL code in GStreamer would be nice to have, even if it doesn't work as well as the others.  I'm sure people will improve the code if it gets used.  I'd expect other distros besides Sun would want to avoid linking GPL code into GStreamer.

Created attachment 84866 [details] [review]
updated patch including needed ext/cddasrc/Makefile.am file


Sorry, last version of the patch was not complete.  This one is better.

Also not sure about the name, currently calling it simply cddasrc, but perhaps a there could be a better name?

Created attachment 86150 [details] [review]
patch with a fix


This patch just modifies the gst_cddasrc_read_sector function so instead of calling calloc like this:

   if ((buf = calloc (1, len))) != NULL) { 

It calls it like this:

  if ((buf = calloc (1, (sizeof (void *) * len))) != NULL) { 

This fixes a core dumping issue on Sparc, where it was crashing because later on in the function the gst_buffer_new_and_alloc function was reading into uninitialized memory since the allocated buffer wasn't really long enough.

This code has been soaking in OpenSolaris for a few weeks now, and aside from this issue, it seems to be working great, fyi.

The solution is simple, let's wait for gpl3.

Created attachment 103380 [details] [review]
upated patch


Cleaned up the patch a little bit, especially in the read_sector function.  Now it doesn't allocate two buffers since one is good.

hmm, just saw this bug. If these plugins are based on GPL libraries they belong in Ugly. The fact that they have managed to go into/stay in -good is an error. The goal of good and base is to ensure that when one distribute any plugins from these packages you generally don't need to think about licensing conflicts, which is why plugins using GPL libraries are among those meant to go to -ugly. 

Setting this as a blocker move bug, to make sure we get at it before next release cylce.

I think we should just add an --enable/--disable-gpl switch to configure and make it default to --disable-gpl. This should be quite easy to do now and is the least intrusive, vendors can then do things as they like.

I don't know what to do with this bug for the current -good release cycle.

Let it be for this round, going to look into if we can get cdparanoia relicensed which would solve the problem.

Punting this until the next release

Created attachment 109793 [details] [review]
updated patch


Here is an update patch.  We found the function cddasrc_byte_swap was badly written and causing crashing issues on big endian machines.  This version of the code fixes the problem.

I suspect with a little work to make this code support Linux ioctls it could function as a LGPL CDDA plugin for GStreamer.  I know the CDDA ioctls on Linux and Solaris are a bit different, but I don't believe by that much.

I talked to Monty and the next release of cdparanoia will be available under the LGPL :) problems solved.

What exactly does "next release" mean? In the past it sometimes took more than a year until a new cdparanoia release ;)

My impression was that he would do it quickly, but we might have to nag a bit :)

> Created an attachment (id=109793) [edit]
> updated patch
>  
> Here is an update patch. (...)

Could you please open a separate new bug for this new plugin?

It should probably be added to -bad first rather than -good directly. Also, I'm not particularly fond of the overly generic name.

Okay, I created bug #529741 for the new plugin.  Yes, the name is generic, but it is sort of a generic CDDA plugin.  If you have any better suggestions for the name, please feel free to update bug #529741 and it shouldn't be hard to change the name.

libcdparanoia is currently in gst-plugins-base.  I suspect when libcdparanoia is relicensed, that gstcdparanoisrc.c will need to be updated so that GST_PLUGIN_DEFINE specifies "LGPL" rather than "GPL", and the configure script might need to be updated to require the new version that is LGPL.  Shouldn't there be a separate bug for tracking libcdparanoia issues?

This bug is about the fact that gst-plugins-good inappropriately contains a GPL'ed plugin.

Ok, so the libcdio plugin should be moved to -ugly next time... cloning this bug for cdparanoia now :)

For those not on the mirrored bug LGPL cdparanoia is now out and available here:
http://downloads.xiph.org/releases/cdparanoia/

0.10 version is LGPL

This is no longer a blocker, right?

Well, this one still is. The cdio plugin from gst-plugins-good should be moved to -ugly as libcdio is still GPL. cdparanoia is not needed to be moved to -ugly because cdparanoia is LGPL now.

OK. I think I'll handle this by leaving things as they are for this release, and aiming to do a limited gst-plugins-good release simultaneous with gst-plugins-ugly next month.

cdiocddasrc is now in gst-plugins-ugly.