Bug 501006 - Review Request: xine-ui - A skinned xlib-based gui for xine-lib
Review Request: xine-ui - A skinned xlib-based gui for xine-lib
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 500926
  Show dependency treegraph
 
Reported: 2009-05-15 09:38 EDT by Susi Lehtola
Modified: 2009-08-20 13:28 EDT (History)
5 users (show)

See Also:
Fixed In Version: 0.99.5-15.el5
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-05-18 09:46:45 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Susi Lehtola 2009-05-15 09:38:27 EDT
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 12:33:50 EDT
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 14:58:40 EDT
(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 13:33:53 EDT
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 14:00:57 EDT
Because Fedora and rpmfusion are different...
Comment 5 Mamoru TASAKA 2009-05-16 14:19:47 EDT
Assigning.
Comment 6 Mamoru TASAKA 2009-05-16 14:41:09 EDT
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 15:20:25 EDT
(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 15:41:11 EDT
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 15:50:39 EDT
Is this OK (at config time)?

checking for NVTVSIMPLE... *** nvtvsimple support will be disabled ***
Comment 10 Susi Lehtola 2009-05-16 16:13:05 EDT
(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 16:25:30 EDT
(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 05:14:07 EDT
(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 05:43:37 EDT
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 00:27:16 EDT
cvs done.
Comment 15 Fedora Update System 2009-05-18 02:15:25 EDT
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 02:15:59 EDT
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 02:16:05 EDT
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 08:58:02 EDT
(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 09:10:41 EDT
Oops, I also missed it on my review, sorry...
Comment 20 Mamoru TASAKA 2009-05-18 09:46:45 EDT
Now closing.
(Please fix GTK icon cache updating scriptlets, sorry)
Comment 21 Susi Lehtola 2009-05-18 14:30:37 EDT
(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 10:20:23 EDT
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 10:21:20 EDT
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 10:35:30 EDT
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 18:52:51 EDT
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 13:27:55 EDT
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.