Bug 501006

Summary: Review Request: xine-ui - A skinned xlib-based gui for xine-lib
Product: [Fedora] Fedora Reporter: Susi Lehtola <susi.lehtola>
Component: Package ReviewAssignee: Mamoru TASAKA <mtasaka>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, mtasaka, notting, ville.skytta, zarko.pintar
Target Milestone: ---Flags: mtasaka: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 0.99.5-15.el5 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-05-18 13:46:45 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 500926    

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.