This service will be undergoing maintenance at 00:00 UTC, 2016-09-28. It is expected to last about 1 hours
Bug 583236 - (vlc) Review Request: vlc - The cross-platform open-source multimedia framework, player and server
Review Request: vlc - The cross-platform open-source multimedia framework, pl...
Status: CLOSED NOTABUG
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Rex Dieter
Fedora Extras Quality Assurance
NotReady
:
Depends On: 518546
Blocks: FE-Legal
  Show dependency treegraph
 
Reported: 2010-04-17 05:32 EDT by Nicolas Chauvet (kwizart)
Modified: 2014-05-09 00:40 EDT (History)
15 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-09-02 11:02:11 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
rdieter: fedora‑review?


Attachments (Terms of Use)
workaround to make buildable in mock (787 bytes, patch)
2010-04-21 09:49 EDT, Rex Dieter
no flags Details | Diff

  None (edit)
Description Nicolas Chauvet (kwizart) 2010-04-17 05:32:30 EDT
Spec URL: http://rpms.kwizart.net/fedora/reviews/vlc/vlc.spec
SRPM URL: http://rpms.kwizart.net/fedora/reviews/vlc/vlc-1.1.0-0.3.pre1.fc13.src.rpm
Description: The cross-platform open-source multimedia framework, player and server

There is need to sort if some part of the source code provided by vlc can pass Fe-Legal. What I expect is that most of the problematic code will be within ffmpeg.
Comment 1 Rex Dieter 2010-04-17 07:05:35 EDT
I had a recent conversation in #phonon on irc,

[10:31] <j-b> rdieter: audited vlc source code
[10:31] <j-b> and tarballs
[10:31] <j-b> Some parts will give you headaches, but not many of them
[10:32] <rdieter> j-b: boo, was afraid of headaches, but we've dealt with similar before.  any details?
[10:33] <j-b> rdieter: mp3 and wma fixed-point decoder/encoder for embedded devices.
[10:33] <j-b> rdieter: and DVB sub decoder/encoder (but I would say those are not licensed)
[10:36] <rdieter> j-b: thanks!  I'll make sure to review that prior to our splitting it up.
[10:41] <j-b> rdieter: confirmed. shine+wmafixed. DVBsub should be ok.
Comment 2 Rex Dieter 2010-04-18 12:28:52 EDT
I'll start reviewing this, approval pending any FE-LEGAL issues of course.
Comment 3 Peter Lemenkov 2010-04-19 01:45:00 EDT
Folks, I'm just curious, what's the story behind this review? I mean, VLC functionality must be greatly degraded in order to be submitted to Fedora, so is it worth at all?
Comment 4 Itamar Reis Peixoto 2010-04-19 08:58:01 EDT
(In reply to comment #3)

I agree, it  probably conflict with vlc from rpmfusion.
Comment 5 Rex Dieter 2010-04-19 09:16:06 EDT
The primary purpose (from my POV) is to allow phonon-vlc backend into fedora.

It will not conflict, and will (hopefully) be splittable into free/unpatented bits in fedora, with the nonfree/patented bits elsewhere, much like what is currently done with gsstreamer and xine-lib.
Comment 6 Rex Dieter 2010-04-19 09:16:45 EDT
Fwiw, other distros are (apparently) already splitting it, and vlc is modular enough these days to allow this.
Comment 7 Tom "spot" Callaway 2010-04-19 14:25:39 EDT
These files are patent-encumbered, and cannot be in the source tarball that Fedora uses:

modules/codec/a52.*
modules/codec/dvbsub.c
modules/codec/shine/*
modules/codec/spudec/*
modules/codec/wmafixed/*

Everything else that I see is either okay, or is dependent on third party libraries (or DLL files) that we can't include in Fedora.

Show me a package that removes those files and builds cleanly and I'll lift FE-Legal.
Comment 8 Rex Dieter 2010-04-21 09:49:23 EDT
Created attachment 408073 [details]
workaround to make buildable in mock

Seems vlc's plugin cache generator needs access to dbus, and this fails when building in mock.  Here's a crude patch to make the failure non-fatal, to allow the build to continue.
Comment 9 Rex Dieter 2010-04-21 10:52:22 EDT
Scratch build with the aforementioned mock patch:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2130404

$ rpmlint vlc-1.1.0-0.3.pre1.fc12.rex.src.rpm x86_64/*.rpm | grep -v rpath
vlc.src: W: spelling-error %description -l en_US unicast -> uni cast, uni-cast, unicameral
vlc.src: W: spelling-error %description -l en_US multicast -> Multics, multicultural, multitask
vlc.src:9: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 9)
vlc.src: W: invalid-url Source0: http://download.videolan.org/pub/videolan/vlc/1.1.0/vlc-1.1.0-pre1.tar.bz2 HTTP Error 404: Not Found
mozilla-vlc.x86_64: W: spelling-error Summary(en_US) plugin -> plug in, plug-in, plugging
mozilla-vlc.x86_64: W: spelling-error %description -l en_US plugin -> plug in, plug-in, plugging
mozilla-vlc.x86_64: W: no-documentation
vlc.x86_64: W: spelling-error %description -l en_US unicast -> uni cast, uni-cast, unicameral
vlc.x86_64: W: spelling-error %description -l en_US multicast -> Multics, multicultural, multitask
vlc.x86_64: W: incoherent-version-in-changelog 1.1.0-0.3.pre1 ['1.1.0-0.3.pre1.fc13.rex', '1.1.0-0.3.pre1.rex']
vlc.x86_64: W: dangling-relative-symlink /usr/share/vlc/skins2/fonts/FreeSans.ttf ../../../fonts/dejavu/DejaVuSans.ttf
vlc.x86_64: W: dangling-relative-symlink /usr/share/vlc/skins2/fonts/FreeSansBold.ttf ../../../fonts/dejavu/DejaVuSans-Bold.ttf
vlc.x86_64: W: file-not-utf8 /usr/share/doc/vlc-1.1.0/ChangeLog
vlc-core.x86_64: W: hidden-file-or-dir /usr/share/vlc/lua/http/dialogs/.hosts
vlc-core.x86_64: W: hidden-file-or-dir /usr/share/vlc/http/.hosts
vlc-core.x86_64: W: hidden-file-or-dir /usr/share/vlc/http/dialogs/.hosts
vlc-core.x86_64: W: hidden-file-or-dir /usr/share/vlc/lua/http/.hosts
vlc-devel.x86_64: W: no-dependency-on vlc/vlc-libs/libvlc
vlc-nox.x86_64: W: spelling-error %description -l en_US framebuffer -> frame buffer, frame-buffer, framework
vlc-nox.x86_64: W: no-documentation
vlc-plugin-jack.x86_64: W: no-documentation
7 packages and 0 specfiles checked; 293 errors, 21 warnings.

I grepped out the bazillion rpath errors, for brevity.
Comment 10 Rex Dieter 2010-04-21 11:13:35 EDT
1.  MUST : fix rpath issues

naming: ok

license: ok

macros: ok

scriptlets: ok (though the icon scriptlets could be updated to use the newer, optimized version currently in the packaging guidelines)

static libs: none, ok

libtool archives: none, ok

-devel: properly includes headers, and lib*.so symlinks

2.  SHOULD adjust current dep
Requires: qt-x11%{_isa} >= 1:4.5.2
to 
Requires: qt4%{?_isa} >= %{_qt4_version}

3.  SHOULD include the kde4 solid/action items, instead of omitting them.  I'd recommend dropping
BuildRequires: kdelibs4-devel
and use instead
BuildRequires: kde-filesystem
Requires: kde-filesystem
with configure option:
--with-kde-solid=%{_kde4_appsdir}/solid/actions

I will mention that I *was* going to suggest some more package splits for shlibs and codecs, mainly for multilib reasons, but the more I think about it, the current way does make sense.

4. SHOULD clarify why libpulse_plugin.so is in vlc, and not vlc-core


that's all I have so far, will do a final analysis, when we have a new submission that addressed these items as well as using a stripped tarball per comment #7.
Comment 11 Chen Lei 2010-04-23 04:21:20 EDT
Scilab 5.2 has been released. 
http://download.videolan.org/pub/videolan/testing/vlc-1.1.0-pre2/
Comment 12 Chen Lei 2010-04-23 04:21:52 EDT
s/Scilab 5.2/vlc 1.1.0-pre2
Comment 13 Rex Dieter 2010-05-04 12:38:59 EDT
Nicolas, ping, can we have an update?  sync' up to 1.1.0-pre3 and address the legal and items I raised, and we should be fairly close.
Comment 14 Nicolas Chauvet (kwizart) 2010-05-04 13:08:24 EDT
(In reply to comment #13)
> Nicolas, ping, can we have an update?  sync' up to 1.1.0-pre3 and address the
> legal and items I raised, and we should be fairly close.    

Sorry for the late answear:
I don't think we are that close. Usability test is still need to be proven.
Also I wonder how will behave gui option when a given option support is disabled. (does that will make the player crash ?). Specially as the fact that some option is disabled is completely not tested upstream.
For example no 'client streaming' will be offered (needs live555), and no dvb support either (needs libdvbpsi) which are core tasks of the vlc framework.

Also, it is still not known how this can be restored correctly (using the vlc-gen-cache that leaded the rpath issue previously); but the plugin-cache*-*.dat is odd.

I'm still searching how to make the offending code removal reliable. That could be a ./configure conditional that will remove the directories at make dist time. Anyone to pick that task ?

But given the amount of work for F-13 I don't think it is suitable to rush.
Comment 15 Rex Dieter 2010-05-04 13:23:29 EDT
of, course, this is F-14 territory, but I'd rather get stuff into cvs and repos for broader testing/feedback sooner rather than later.
Comment 16 nucleo 2010-05-28 10:07:22 EDT
Is hal-devel needed in VLC 1.1?

There is in vlc.spec "BuildRequires: hal-devel" but in VLC 1.1 uses libudev instead HAL.

http://git.videolan.org/?p=vlc/vlc-1.1.git;a=blob_plain;f=NEWS;hb=HEAD

Removed modules:
 * HAL: Use libudev instead.
Comment 17 Manuel Faux 2010-08-16 09:17:36 EDT
Don't you think shipping this minimal version of VLC with Fedora might confuse new users? If an experienced user installs VLC and is unable to play his MP3s or MPEGs, it might be more difficult for him to find a solution than if VLC is not in the Repo at all, I think.
Comment 18 Rex Dieter 2010-08-16 09:31:35 EDT
A little, but that's not unique to vlc. The same can be said of many patent-encumbered multimedia apps and frameworks, including gstreamer and xine-lib.  This has been overcome for them, and the same can (and hopefully will soon) be done for vlc too.  It'll involve education and documentation in the least, and a growing institutional knowledge.
Comment 19 Tomas Miljenović (TomasM) 2010-09-08 08:27:13 EDT
As an interim measure, can the GUI be excluded?  This would only require updating the documentation, as I don't think the ncurses interface supports features such as streaming and dvb.

If this is only about the phonon backend, how much of vlc asides from libvlc.so.* is required?
Comment 20 Rex Dieter 2010-09-08 10:43:14 EDT
It's mostly about the runtime libraries and (unpatented) plugins, from my phonon-clouded POV, yes.
Comment 21 Rex Dieter 2010-12-02 09:14:53 EST
comment #19 is a good suggestion.  I'll be happy to work on bringing only libvlc + free codecs to fedora.  

Nicolas, is that agreeable?  Do you have a preference whether I re-use the vlc-core pkg name or have another suggestion?
Comment 22 Nicolas Chauvet (kwizart) 2010-12-02 11:41:26 EST
As warned at Spot on Fudcond Zurich, but not on this tread actually.

vlc playback hightly depends on FFmpeg. If you cannot offer to have that library in fedora that will make vlc users <<suffer>> even with free software codecs.

Will you be able to read libvpx with such vlc-free ?
No, because it depends on ffmpeg itself as no specific module is ever planed to be made from vlc but using the ffmpeg libavcodec decoder.

Same for some chroma conversion, thoses have been removed from next vlc branch because they were duplicating with ffmpeg ones.

With that said, there is no common point of comparing Gstreamer and vlc as a Framework when there is to deal with patent concern. Whereas Gstreamer pick out the single code they want from FFmpeg to make a Gstreamer module. VLC removes everything that is already present in FFmpeg to avoid duplication, so it makes a big difference.

You won't solve that using modules, when such specific module doesn't even exists for the single task you want. Or such sigle modules would need to be compiled two time, (one patentless and another patent version).

So, such ffmpeg-less vlc will probably not work well and be a pain to maintain, when it will have to deal with playbacks. Which is probably not what targets phonon backend. And I'm not yet sure that the server only side in fedora can be completed without functionality loss.

At least, I would like to have some feedback as the conditional to make it build in a fedora mock are worked on from the current rpmfusion spec file. Just simple tasks:
- It would be interesting to list which codecs work at playback, which one doesn't.
- list the vlc-core modules built from a fedora mock from those built from current package.
- checks from the source code of the fedora vlc-core modules if they have preprocessor macro from optional patented dependencies.
- check if such module is actually usefull for something.
- etc.

Last, there is still a real problem with the cache plugin at runtime. (the patch from rex was at build time). It would have allowed for new vlc modules to be correctly registered. For now, new modules sometime won't be seen because vlc-cache-gen is crashing while installing a new module.
(in which case yum reinstall vlc-core strangely solve things sometime).
Comment 23 Rahul Sundaram 2010-12-24 15:11:52 EST
It seems easier if phonon-vlc along with vlc stays in RPM Fusion and let phonon-gstreamer be the default for Fedora and KDE SIG can focus testing and getting the bugs fixed so that we can finally settle on one default multimedia framework from Fedora 15 onwards.
Comment 24 Tom "spot" Callaway 2011-06-30 13:03:54 EDT
If this is dead, please close it out.
Comment 25 Rex Dieter 2011-09-02 11:02:11 EDT
OK, stick a fork in this, for now. rpmfusion it stays.

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