Bug 501006 - Review Request: xine-ui - A skinned xlib-based gui for xine-lib
Summary: Review Request: xine-ui - A skinned xlib-based gui for xine-lib
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords:
Depends On:
Blocks: 500926
TreeView+ depends on / blocked
 
Reported: 2009-05-15 13:38 UTC by Susi Lehtola
Modified: 2009-08-20 17:28 UTC (History)
5 users (show)

(edit)
Clone Of:
(edit)
Last Closed: 2009-05-18 13:46:45 UTC
mtasaka: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Susi Lehtola 2009-05-15 13:38:27 UTC
Spec URL:
http://theory.physics.helsinki.fi/~jzlehtol/rpms/xine-ui.spec

SRPM URL:
http://theory.physics.helsinki.fi/~jzlehtol/rpms/xine-ui-0.99.5-7.fc10.src.rpm

Upstream url: http://www.xine-project.org/

Description:
xine-ui is the default GUI for xine-lib.

rpmlint output is clean.

Comment 1 Mamoru TASAKA 2009-05-15 16:33:50 UTC
Some notes:

* About Patch1
  - Would you explain why you want to change Name= item
    from "xine" to "Xine"?

?? About dlopen'ing libX11.so in src/aaui/main.c
  - ... however:
--------------------------------------------------------
[tasaka1@localhost bin]$ ldd -r ./aaxine  | grep libX11
	libX11.so.6 => /usr/lib/libX11.so.6 (0x00793000)
--------------------------------------------------------
    I may be wrong, however would you explain why this
    dlopen() is really needed?

* Timestamps
  - Add INSTALL="install -p" on 'make install' to keep timestamps
    on installed files

* desktop-file-install
  - You don't have to delete desktop file under %_builddir
    (--delete-original). Note that currently
    $ rpmbuild -bi --short-circuit will fail due to this.

* Scriptlets
  - Update GTK icon cache update scriptlets:
    https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache

* %files
  - Would you explain why two different desktop files are installed
    under different places?
----------------------------------------------------------
/usr/share/applications/xine.desktop
/usr/share/xine/desktop/xine.desktop
----------------------------------------------------------
    Note that these two differs slightly.

Comment 2 Susi Lehtola 2009-05-15 18:58:40 UTC
(In reply to comment #1)
> Some notes:
> 
> * About Patch1
>   - Would you explain why you want to change Name= item
>     from "xine" to "Xine"?

Good question. This was in the rpmfusion package. I dropped the patch altogether.

> ?? About dlopen'ing libX11.so in src/aaui/main.c
>   - ... however:
> --------------------------------------------------------
> [tasaka1@localhost bin]$ ldd -r ./aaxine  | grep libX11
>  libX11.so.6 => /usr/lib/libX11.so.6 (0x00793000)
> --------------------------------------------------------
>     I may be wrong, however would you explain why this
>     dlopen() is really needed?

I don't know, this is from the rpmfusion package and I don't know why it is necessary. The sed is needed however since the unversioned .so file is provided by libX11-devel instead of libX11.

> * Timestamps
>   - Add INSTALL="install -p" on 'make install' to keep timestamps
>     on installed files

Fixed, thanks for the reminder (I was going to fix this earlier but forgot).

I also fixed the charset conversion to preserve timestamps.


> * desktop-file-install
>   - You don't have to delete desktop file under %_builddir
>     (--delete-original). Note that currently
>     $ rpmbuild -bi --short-circuit will fail due to this.

Done.

> * Scriptlets
>   - Update GTK icon cache update scriptlets:
>     https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache

This was already partly done, but I rewrote the part to use the snippets above. Also added the missing mimetype refresh.

> 
> * %files
>   - Would you explain why two different desktop files are installed
>     under different places?
> ----------------------------------------------------------
> /usr/share/applications/xine.desktop
> /usr/share/xine/desktop/xine.desktop
> ----------------------------------------------------------
>     Note that these two differs slightly.  

Good question. Removed the other one.


http://theory.physics.helsinki.fi/~jzlehtol/rpms/xine-ui.spec
http://theory.physics.helsinki.fi/~jzlehtol/rpms/xine-ui-0.99.5-8.fc10.src.rpm

Comment 3 Zarko (grof) 2009-05-16 17:33:53 UTC
Is this  xine-ui.spec taken from RPM Fusion?

If it is, then why we must reviewing again something what is reviewed earlier?

Comment 4 Mamoru TASAKA 2009-05-16 18:00:57 UTC
Because Fedora and rpmfusion are different...

Comment 5 Mamoru TASAKA 2009-05-16 18:19:47 UTC
Assigning.

Comment 6 Mamoru TASAKA 2009-05-16 18:41:09 UTC
For -8:

* Desktop file
  - Remove "Application" category. This category is marked as
    deprecated. ref:
    http://www.redhat.com/archives/fedora-extras-list/2006-October/msg00723.html

* desktop-file-install
  - Again "--remove-original" is not needed for this case.

?? About aaxine
  - Well, currently launching aaxine on CUI (with framebuffer, vga=791)
    by $ aaxine <some_file>
    just displays nothing and only sound can be heard.
    Also with GUI launching aaxine on terminal emulator (such as
    gnome-terminal) launching another Window and show some AA images
    there, not on the terminal emulator.
    Is this the expected behavior?

Comment 7 Susi Lehtola 2009-05-16 19:20:25 UTC
(In reply to comment #6)
> For -8:
> 
> * Desktop file
>   - Remove "Application" category. This category is marked as
>     deprecated. ref:
>    
> http://www.redhat.com/archives/fedora-extras-list/2006-October/msg00723.html

Removed.

> * desktop-file-install
>   - Again "--remove-original" is not needed for this case.

Sorry, didn't see it since it was on the same line as desktop-install :) Removed.

> ?? About aaxine
>   - Well, currently launching aaxine on CUI (with framebuffer, vga=791)
>     by $ aaxine <some_file>
>     just displays nothing and only sound can be heard.
>     Also with GUI launching aaxine on terminal emulator (such as
>     gnome-terminal) launching another Window and show some AA images
>     there, not on the terminal emulator.
>     Is this the expected behavior?  

After toying around for some time I found out that aalib output requires support from xine-lib. This is in the xine-lib-extras package, which I have now added as a Requires: and aaxine works.

http://theory.physics.helsinki.fi/~jzlehtol/rpms/xine-ui.spec
http://theory.physics.helsinki.fi/~jzlehtol/rpms/xine-ui-0.99.5-9.fc10.src.rpm

Comment 8 Zarko (grof) 2009-05-16 19:41:11 UTC
Im getting this issue when pressing DVD or DVB navigator buttons:

cannot find input plugin for MRL [dvd:/]
input plugin cannot open MRL [dvd:/]

but I did not try the latest buid.

Comment 9 Zarko (grof) 2009-05-16 19:50:39 UTC
Is this OK (at config time)?

checking for NVTVSIMPLE... *** nvtvsimple support will be disabled ***

Comment 10 Susi Lehtola 2009-05-16 20:13:05 UTC
(In reply to comment #8)
> Im getting this issue when pressing DVD or DVB navigator buttons:
> 
> cannot find input plugin for MRL [dvd:/]
> input plugin cannot open MRL [dvd:/]
> 
> but I did not try the latest buid.  

For dvb you need to have a working channels.conf in your ~/.xine directory.
For dvd you must have a dvd in your drive.

Otherwise you get these errors.

Stupid, yes. A packaging bug, no.


(In reply to comment #9)
> Is this OK (at config time)?
> 
> checking for NVTVSIMPLE... *** nvtvsimple support will be disabled ***  

Yes, since there's been no support in rpmfusion either and xine works without it. NVTV is for enabling TV-out on Nvidia cards.

nvtv upstream is here: http://sourceforge.net/project/showfiles.php?group_id=33758

nvtvsimple has been updated last in 2004 (nvtv-runtime in 2006), so I don't think it's an important addition.

Comment 11 Zarko (grof) 2009-05-16 20:25:30 UTC
(In reply to comment #10)
> 
> Stupid, yes. A packaging bug, no.
> 
> 
> nvtvsimple has been updated last in 2004 (nvtv-runtime in 2006), so I don't
> think it's an important addition.  


OK than. Now it's clear to me, thanks :)

Comment 12 Mamoru TASAKA 2009-05-17 09:14:07 UTC
(In reply to comment #6)
> ?? About aaxine
>   - Well, currently launching aaxine on CUI (with framebuffer, vga=791)
>     by $ aaxine <some_file>

! Well, I found that aaxine accepts the option used
  by aalib as shown in src/aaui/main.c:
---------------------------------------------------------
   638  #ifdef AA
   639    /* 
   640     * AALib help and option-parsing
   641     */
   642    if (!aa_parseoptions(NULL, NULL, &argc, argv)) {
   643      print_usage();
   644      goto failure;
   645    }
   646  #endif
---------------------------------------------------------

Then:
(In reply to comment #7)
> (In reply to comment #6)
> > For -8:
> > 
> > * Desktop file
> >   - Remove "Application" category. This category is marked as
> >     deprecated. ref:
> >    
> > http://www.redhat.com/archives/fedora-extras-list/2006-October/msg00723.html
> 
> Removed.
- You can use "--remove-category" option of "desktop-file-install"
  (as you are already using --add-category) instead of using sed.

-----------------------------------------------------
  This package (xine-ui) is APPROVED by mtasaka
-----------------------------------------------------

Comment 13 Susi Lehtola 2009-05-17 09:43:37 UTC
True. Thanks for the review!

New Package CVS Request
=======================
Package Name: xine-ui
Short Description: A skinned xlib-based gui for xine-lib
Owners: jussilehtola
Branches: F-9 F-10 F-11 EL-5
InitialCC:

Comment 14 Kevin Fenzi 2009-05-18 04:27:16 UTC
cvs done.

Comment 15 Fedora Update System 2009-05-18 06:15:25 UTC
xine-ui-0.99.5-10.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/xine-ui-0.99.5-10.fc9

Comment 16 Fedora Update System 2009-05-18 06:15:59 UTC
xine-ui-0.99.5-10.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/xine-ui-0.99.5-10.fc11

Comment 17 Fedora Update System 2009-05-18 06:16:05 UTC
xine-ui-0.99.5-10.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/xine-ui-0.99.5-10.fc10

Comment 18 Ville Skyttä 2009-05-18 12:58:02 UTC
(In reply to comment #2)
> (In reply to comment #1)
> > * Scriptlets
> >   - Update GTK icon cache update scriptlets:
> >     https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache
> 
> This was already partly done, but I rewrote the part to use the snippets 
> above.

The "touch --no-create ..." %post part appears to be missing.

Comment 19 Mamoru TASAKA 2009-05-18 13:10:41 UTC
Oops, I also missed it on my review, sorry...

Comment 20 Mamoru TASAKA 2009-05-18 13:46:45 UTC
Now closing.
(Please fix GTK icon cache updating scriptlets, sorry)

Comment 21 Susi Lehtola 2009-05-18 18:30:37 UTC
(In reply to comment #20)
> Now closing.
> (Please fix GTK icon cache updating scriptlets, sorry)  

Whoops, the %post scriplet was missing. Fixed in the -11 release.

Comment 22 Fedora Update System 2009-06-02 14:20:23 UTC
xine-ui-0.99.5-11.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 23 Fedora Update System 2009-06-02 14:21:20 UTC
xine-ui-0.99.5-11.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 24 Fedora Update System 2009-06-02 14:35:30 UTC
xine-ui-0.99.5-11.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 25 Fedora Update System 2009-07-24 22:52:51 UTC
xine-ui-0.99.5-15.el5 has been submitted as an update for Fedora EPEL 5.
http://admin.fedoraproject.org/updates/xine-ui-0.99.5-15.el5

Comment 26 Fedora Update System 2009-08-20 17:27:55 UTC
xine-ui-0.99.5-15.el5 has been pushed to the Fedora EPEL 5 stable repository.  If problems still persist, please make note of it in this bug report.


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