Bug 205798 - Review Request: xine-lib - The Xine library
Review Request: xine-lib - The Xine library
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ville Skyttä
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-09-08 12:11 EDT by Aurelien Bompard
Modified: 2007-11-30 17:11 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-11-01 01:12:30 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Aurelien Bompard 2006-09-08 12:11:57 EDT
Spec URL: http://gauret.free.fr/fichiers/rpms/fedora/xine-lib.spec
SRPM URL: http://gauret.free.fr/fichiers/rpms/fedora/xine-lib-1.1.2-12.src.rpm
Description: 
This package contains the Xine library. Xine is a free multimedia player.
It can play back various media. It also decodes multimedia files from local
disk drives, and displays multimedia streamed over the Internet. It
interprets many of the most common multimedia formats available - and some
of the most uncommon formats, too.  Non-default rpmbuild options:
--without imagemagick:  Disable ImageMagick support
--with    directfb:     Enable DirectFB support
Comment 1 Ville Skyttä 2006-09-08 13:17:32 EDT
This probably needs an ack from legal before it makes much sense to spend time
on a review -> adding blocker on FE-Legal.

(Removing explicit Cc to me, I already receive the traffic through the
fedora-package-review list.)
Comment 2 Dennis Gilmore 2006-09-08 13:28:28 EDT
as with other media packages  you will need to use a modified tarball with all 
of the patented codec support removed.
Comment 3 Ville Skyttä 2006-09-08 13:39:42 EDT
That is already done, see comments in the specfile.  I think a legal ack is
needed nevertheless.
Comment 4 Dennis Gilmore 2006-09-08 13:54:55 EDT
Source0:        
http://download.sourceforge.net/xine/%{name}-%{version}.tar.bz2
Patch4:         xine-lib-1.1.2-new-ffmpeg.patch

doesn't look like it   

I see the comment in %prep  it should be up with the sources.

# The tarball is generated from the upstream tarball using
# the script in SOURCE1. It prunes potentially patented code

comment should be moved and  id have a guess that the ffmpeg  patch dropped
Comment 5 Aurelien Bompard 2006-09-12 07:27:15 EDT
Ah yes, sorry. This version actually builds...

* Tue Sep 12 2006 Aurelien Bompard <abompard@fedoraproject.org> 1.1.2-13
- drop patches 2 and 4
Comment 7 Aurelien Bompard 2006-09-15 11:29:33 EDT
Do you know how long the legal audit is likely to take ? Could we expect
xine-lib in FC6, or is it much too close ?
The SuSE specfile contains a mini-audit, if it can be of any help:
http://gauret.free.fr/fichiers/rpms/fedora/xine-lib.suse.spec
Comment 8 Ville Skyttä 2006-09-15 12:06:58 EDT
No idea.  Some packages have been waiting a long time, eg. mkvtoolnix 9+ months
in bug 177134.

See also http://www.redhat.com/archives/fedora-extras-list/2006-August/msg00199.html

There was a word from someone that getting the legal queue moving would be put
on the Board's agenda, but I haven't heard more about that, and summaries for
the two last Board meetings seem to be missing from Wiki.  Rex, any news?
Comment 9 Rex Dieter 2006-09-15 12:25:50 EDT
>  Rex, any news?

Other than the last 2 FPB meetings weren't held (for various reasons).  I'll 
make sure it's discussed by FPB at next opportunity.
Comment 10 Denis Leroy 2006-09-15 12:31:38 EDT
How is this package going to coexist with the various xine-libs available from
"other repos" and legally used by people in countries with no software patents ?
Since xine-lib has to be crippled to make it into Extras, won't it create an
unnecessary annoyance ? Does it add real value to FE ?
Comment 11 Dennis Gilmore 2006-09-15 12:48:07 EDT
xine-lib  is the only viable engine  on some arches for amarok. other repos  
should be able to have a xine-lib-extras package  that has non free bits for 
those that can use them.
Comment 12 Rex Dieter 2006-09-15 12:48:58 EDT
Re: comment #10
> other repos

If this makes it in, there are plans to also make a xine-libs-extras-nonfree for
"other" repos. 

> Does it add real value to FE ?

IMO, a *definite* yes.  We'll be able to include apps that depend on
xine-lib-devel to build (otherwise they wouldn't be included in Extras *at all*).
Comment 13 Denis Leroy 2006-09-15 12:50:59 EDT
Excellent! thanks for the clarification.
Comment 14 Tom "spot" Callaway 2006-10-14 17:59:51 EDT
The liba52 code needs to go, it is patent encumbered and still present in the
tarball. Otherwise, it looks ok. Lifting FE-Legal, on the grounds that the
liba52 directory will be removed from the source.
Comment 15 Aurelien Bompard 2006-10-15 16:15:49 EDT
Great news ! Thanks !
I've updated the pruning script to remove liba52 too :
http://gauret.free.fr/fichiers/rpms/fedora/xine-lib.spec
http://gauret.free.fr/fichiers/rpms/fedora/xine-lib-1.1.2-14.src.rpm
Comment 16 Ville Skyttä 2006-10-16 16:46:01 EDT
Preliminary review:

- src/libdts: most likely needs to be removed.  The code contains links to 
http://www.videolan.org/dtsdec.html - check it out for more info.

- The option summary after ./configure contains lots of things containing the 
word "mpeg" - is it just the message or does the source tarball require 
further cleanup?

- The cleanup script removes src/libspudec twice, maybe the other one was 
supposed to remove something else?

- xine-lib-dxropts.patch is not needed because src/dxr3 is pruned.

- Build fails on x86_64:
    File not 
found: /var/tmp/xine-lib-1.1.2-14.fc5-buildroot/usr/lib64/xine/plugins/1.1.2/xineplug_decode_gdk_pixbuf.so
    File not 
found: /var/tmp/xine-lib-1.1.2-14.fc5-buildroot/usr/lib64/xine/plugins/1.1.2/xineplug_vo_out_vidix.so
Comment 17 Marcin Garski 2006-10-16 17:30:42 EDT
(In reply to comment #16)
> - src/libdts: most likely needs to be removed.  The code contains links to 
> http://www.videolan.org/dtsdec.html - check it out for more info.

Probably need to be removed.
From libdts website: "Provisional Warning: DTS Inc. claims that use of libdca
software, to decode DTS compressed sound data on a DVD could violate DTS's
patent rights. If you are unsure about the legality of using and distributing
this code in your country, in particular in the USA, please consult your lawyer
before downloading it".
Comment 18 Aurelien Bompard 2006-10-16 17:44:41 EDT
- removed libdts
- fixed the cleanup script to remove libspudec only once
- dropped dxr patch
- listed xineplug_decode_gdk_pixbuf.so and xineplug_vo_out_vidix.so only on x86

http://gauret.free.fr/fichiers/rpms/fedora/xine-lib.spec
http://gauret.free.fr/fichiers/rpms/fedora/xine-lib-1.1.2-15.src.rpm

The mpeg-related configure switches are there to skip the mpeg tests (saves me
from hacking even more the configure script)
Comment 19 Ville Skyttä 2006-10-16 18:16:06 EDT
(In reply to comment #18)
> - listed xineplug_decode_gdk_pixbuf.so and xineplug_vo_out_vidix.so only
> on x86

This doesn't look like the correct fix to me.  vidix is x86 only IIRC so that 
part is maybe ok, but gdk-pixbuf appears to be just a matter of missing BR: 
gtk2-devel.

> The mpeg-related configure switches are there to skip the mpeg tests

Note that I was talking about the printed configuration summary 
after ./configure is run, not switches passed to it.  Eg.

xine-lib summary:
[...]
 * demultiplexer plugins:
   - avi           - mpeg
   - mpeg_block    - mpeg_audio
   - mpeg_elem     - mpeg_pes
   - mpeg_ts       - qt/mpeg-4
[...]
etc

dts removal looks incomplete:

RPM build errors:
    File not 
found: /var/tmp/xine-lib-1.1.2-15.fc5-buildroot/usr/lib64/xine/plugins/1.1.2/xineplug_decode_dts.so

More later tomorrow - others' feel free to chime in on the review.
Comment 20 Aurelien Bompard 2006-10-17 06:33:48 EDT
You're right about gdk_pixbuf, I thought you meant you tried on both x86 and
x86_64 and it failed only on the latter.

- add BR gtk2-devel
- remove xineplug_decode_gdk_pixbuf.so from x86-only

http://gauret.free.fr/fichiers/rpms/fedora/xine-lib.spec
http://gauret.free.fr/fichiers/rpms/fedora/xine-lib-1.1.2-16.src.rpm

find -name "*mpeg*" in the sources dir yields some results, but I don't think
it's mpeg decoding code. It looks more like interfacing with ffmpeg or code for
the mpg container. In any case, I trust Legal in this.

I tried to build the package in mock, and it fails because of an improper
selinux label on /usr/lib/libSDL-1.2.so.0.7.2. Is it fixable ? Will it fail in
the Fedora Buildsystem ?
Comment 21 Ville Skyttä 2006-10-17 17:49:58 EDT
Ok, getting closer.  It now builds and a q'n'd-modified and rebuilt FE5 amarok 
works nicely with flacs on x86_64.

Just an observation: xine-lib appears to ship and use its private slightly 
customized copies of libmatroska and libmpcdec (and possibly other things), 
and doesn't provide an option to use the system ones.  Not nice, but not a 
blocker IMO either.

The installed docs should be reviewed for usefulness, at least README.dxr3 and 
README.MINGWCROSS look like clear candidates for removal.

About some input plugins: are MMS, RTP, and RTSP ok to be included?  AFAIK MMS 
is proprietary (libmms was not submitted to FE, I think Hans asked about it 
sometime ago), and a quick Googling session revealed these for the two latter.
http://www.ietf.org/ietf/IPR/RTSP
http://www1.ietf.org/ietf/IPR/microsoft-ipr-draft-ietf-avt-rtp-vc1-02.txt

spot, your thoughts on the above?

About the SDL issue, no idea.  No problems building it for FC5 here, using 
mach.
Comment 22 Tom "spot" Callaway 2006-10-17 18:17:55 EDT
RTP looks ok because Microsoft is stating in that document that they are
granting a "Royalty-Free, Reasonable, and Non-Discriminatory License to All
Implementers" in that RFC. Likewise, the Real Networks document seems to cover
RTSP sufficiently.

The MMS protocol implementation on the other hand is definitely not
royalty-free, and needs to go.
Comment 23 Aurelien Bompard 2006-10-18 04:42:12 EDT
- cleanup docs
- remove mms
- delete more source files in the cleanup script
- drop patch3 (affecting mms)

About the 3rd line, I realized I did not remove all the source code files in the
input plugins dir, so I changed the script to do that.

http://gauret.free.fr/fichiers/rpms/fedora/xine-lib.spec
http://gauret.free.fr/fichiers/rpms/fedora/xine-lib-1.1.2-17.src.rpm

The failure in mock may be beacause my FC5 is a yum-upgraded FC4, I don't know.
Comment 24 Ville Skyttä 2006-10-19 02:48:26 EDT
Ok, looks even better.  One more question about pruned stuff:

What's the problem with including the DVD reading functionality in this 
package?  The capability to read data from encrypted DVDs is not implemented 
directly in xine-lib as far as I can tell.  Based on some recent discussions 
in private mail with spot, that would be the only real problem with including 
DVD reading libraries in Fedora.

The ability to read unencrypted DVDs would be useful to have.  And even though 
the actual MPEG data on them couldn't be decoded in software with packages in 
Fedora, there are hardware solutions to that, such as the em8300/dxr3 cards 
supported by the em8300* packages in FE, and probably full-featured DVB cards 
(ie. ones that have a MPEG decoder chip).  Enabling libdvdnav in xine-lib 
would also allow enabling the dxr3 hardware MPEG decoding functionality in it.  

The dxr3 functionality here is associated with libavcodec, rte, and fame, all 
of which can't be included in Fedora, but they're only used to re-encoded 
non-MPEG content to MPEG for feeding to the em8300 devices.  MPEG playback 
with em8300 requires none of those.
Comment 25 Aurelien Bompard 2006-10-29 02:15:04 EST
At the moment, libdvdnav is not in Extras, so we need to review and accept it
before enabling dvd support in xine-lib. Since the libdvdnav sumbission will
have to go through Legal review, it may take some time.
So I'd rather have xine-lib without dvdnav support imported in the close future,
and add the support when libdvdnav is in Extras.
Are you guys OK with that ?

Ville, would you like to co-maintain xine-lib with me in Extras ?
Comment 26 Ville Skyttä 2006-10-29 04:20:53 EST
Yeah, feel free to add me as a co-maintainer when this goes in.

Note that xine-lib uses a bundled/modified libdvdnav by default, dunno how 
feasible would it be to try to convert it to use an external one.  But yeah, 
that can be looked into later.

I have nothing to add here at the moment, so as far as I'm concerned, this is 
approved.  I'll set this to FE-ACCEPT tomorrow in order to provide a bit of 
time in case someone (spot?) wishes to take one more look at the package 
contents.  Please delay importing until this is in FE-ACCEPT state.

Oh, one little thing, I think xineplug_decode_gdk_pixbuf.so and 
xineplug_vo_out_caca.so are good candidates for moving into the list of 
plugins potentially to be moved to a xine-lib-extras subpackage later.
Comment 27 Ville Skyttä 2006-10-30 12:00:57 EST
Moving to FE-ACCEPT.
Comment 28 Aurelien Bompard 2006-11-01 01:12:30 EST
Imported and built, thanks a lot !
Comment 29 Ville Skyttä 2007-08-14 07:35:59 EDT
Package Change Request
======================
Package Name: xine-lib
Updated Fedora Owners: abompard,scop
Updated Fedora CC: [none]

I've been already co-maintaining this package for a while, so I suppose it
wouldn't hurt to move me from initialcc to a real co-maintainer.
Comment 30 Kevin Fenzi 2007-08-14 15:58:41 EDT
cvs done.
Comment 31 Ville Skyttä 2007-09-07 10:32:41 EDT
Package Change Request
======================
Package Name: xine-lib
New Branches: EL-5
Updated EPEL Owners: scop,abompard
Comment 32 Kevin Fenzi 2007-09-07 14:39:39 EDT
cvs done.

Note You need to log in before you can comment on or make changes to this bug.