Bug 215569

Summary: Review Request: beryl-vidcap - Beryl OpenGL window and compositing manager video capture utility
Product: [Fedora] Fedora Reporter: Jarod Wilson <jarod>
Component: Package ReviewAssignee: Mamoru TASAKA <mtasaka>
Status: CLOSED NOTABUG QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: chitlesh, martin.sourada, mtasaka, tom
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-06-28 14:12:13 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On: 209259    
Bug Blocks: 201449    
Attachments:
Description Flags
spec file to try to aviod using chrpath
none
spec file (more correct)
none
Mock build log of beryl-vidcap 0.1.3-1 on FC-devel i386
none
objdump log of libseom.so.0.0.0
none
Mock build log of beryl-vidcap-0.1.4-1.fc7
none
Mock build log of beryl-vidcap-0.1.99.2-1.fc7 none

Description Jarod Wilson 2006-11-14 12:28:59 EST
Spec URL: http://wilsonet.com/packages/beryl/beryl-vidcap.spec
SRPM URL: http://wilsonet.com/packages/beryl/beryl-vidcap-0.1.2-3.fc6.src.rpm
Description:
Beryl is a combined window manager and compositing
manager that runs on top of Xgl or AIGLX using OpenGL
to provide effects accelerated by a 3D graphics card
on the desktop. Beryl is a community-driven fork of
Compiz.

This package provides a utility capable of capturing
beryl-enabled desktop sessions as video.


Depends on beryl-core, submitted for FE-review under bug 209259.
Comment 1 Martin Sourada 2006-11-14 12:51:50 EST
You should fix this rpmlint output:
E: beryl-vidcap binary-or-shlib-defines-rpath /usr/lib/beryl/libcapture.so
'/build/BUILD/beryl-vidcap-0.1.2/seom/.libs']
W: beryl-vidcap no-documentation
E: beryl-vidcap library-without-ldconfig-postin /usr/lib/libseom.so.0.0.0
E: beryl-vidcap library-without-ldconfig-postun /usr/lib/libseom.so.0.0.0
E: beryl-vidcap library-without-ldconfig-postin /usr/lib/libseom.so.0
E: beryl-vidcap library-without-ldconfig-postun /usr/lib/libseom.so.0
Comment 2 Martin Sourada 2006-11-14 13:43:40 EST
About the rpath check this:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-a1dfb5f46bf4098841e31a75d833e6e1b3e72544
Comment 3 Jarod Wilson 2006-11-14 14:57:12 EST
Ugh. I hate this package. And its utterly crappy Makefiles. Latest build works
around everything in comment #1, as well as making proper symlinks instead of
copying all the libseom .so's. Well, except there's still no documentation, but
nothing I can do about that one at the moment, since there is none. :)

http://wilsonet.com/packages/beryl/beryl-vidcap-0.1.2-4.fc6.src.rpm
Comment 4 Mamoru TASAKA 2006-11-16 00:38:43 EST
Just a quick look at this:

* %{_libdir}/beryl/ is not owned by any package.
* Fedora specific compilation flags is not passed and -debuginfo
  rpm is of no use.
Comment 5 Jarod Wilson 2006-11-16 01:00:36 EST
(In reply to comment #4)
> Just a quick look at this:
> 
> * %{_libdir}/beryl/ is not owned by any package.

Actually, it is. Its owned by beryl-plugins. I believe I should have Requires:
beryl-plugins.

> * Fedora specific compilation flags is not passed

Eep. Will add that next build.

> -debuginfo rpm is of no use.

Hrm, might be the Makefile installing stuff with -s...

I'll take a look at fixing all of these in the morning, thanks much!
Comment 6 Jarod Wilson 2006-11-16 14:47:16 EST
I have yet to figure out why there's no source files being included in the
debuginfo package, but it does have .debug files in it... (Also need to trim
down the BuildReqs per your comments in the beryl-vidcap bug). But I have pushed
a new build that adds a dep on beryl-plugins and enabled the proper compile flags:

http://wilsonet.com/packages/beryl/beryl-vidcap-0.1.2-5.fc6.src.rpm
Comment 7 Jarod Wilson 2006-11-16 17:38:59 EST
New build with trimmed BR: (requires new beryl-core-devel, which I haven't
pushed yet).:

http://wilsonet.com/packages/beryl/beryl-vidcap-0.1.2-6.fc6.src.rpm
Comment 8 Mamoru TASAKA 2006-11-17 03:32:46 EST
Created attachment 141457 [details]
spec file to try to aviod using chrpath

Well, using chrpath is somewhat unwilling, I think.
How do you think about the spec file I attached?

Note: The change for 'make' command in %install process
is needed to pass fedora specific compilation flags.

NOTE: I only checked about chrpath and $RPM_OPT_FLAGS
issues and not checked anything else!!
Comment 9 Mamoru TASAKA 2006-11-17 03:38:01 EST
Created attachment 141458 [details]
spec file (more correct)

Perhaps more correct spec file (use exec)
Comment 10 Mamoru TASAKA 2006-11-17 11:56:58 EST
Well, during review on beryl-vipcap, several issues are
found.

* %{_libdir}/beryl/libcapture.so contains undefined
  non-weak symbols.
----------------------------------------------------
[tasaka1@localhost ~]$ ldd -r /usr/lib/beryl/libcapture.so > /dev/null
undefined symbol: glEnd (/usr/lib/beryl/libcapture.so)
undefined symbol: glEnable      (/usr/lib/beryl/libcapture.so)
undefined symbol: glColor4us    (/usr/lib/beryl/libcapture.so)
undefined symbol: glEnableClientState   (/usr/lib/beryl/libcapture.so)
undefined symbol: glDisable     (/usr/lib/beryl/libcapture.so)
undefined symbol: glRecti       (/usr/lib/beryl/libcapture.so)
undefined symbol: addScreenAction       (/usr/lib/beryl/libcapture.so)
undefined symbol: compSetFloatOption    (/usr/lib/beryl/libcapture.so)
undefined symbol: getIntOptionNamed     (/usr/lib/beryl/libcapture.so)
(and others)
----------------------------------------------------
  Please check the linkage against this package.

* Related to above, -devel package are missing necessary
  Requires.
  %{_includedir}/seom/seom.h reads:
----------------------------------------------------
#include <GL/gl.h>
#include <GL/glext.h>
#include <GL/glx.h>

#include <X11/Xlib.h>
#include <X11/Xatom.h>
#include <X11/keysym.h>
----------------------------------------------------
  This means -devel package requires at least the following packages
---------------------------------------------------
libGL-devel (this is provided by mesa-libGL-devel)
libX11-devel
xorg-x11-proto-devel (this is required by mesa-libGL-devel so
                      redundant for writing to Requires)
---------------------------------------------------
  and also this implies that the missing linkage on
  %{_libdir}/beryl/libcapture.so _may_ be for
  libGL.so and libX11.so (I have not checked).

* Fedora specific compilation flags are not passed.
----------------------------------------------------
+ umask 022
+ cd /builddir/build/BUILD
+ cd beryl-vidcap-0.1.2
+ LANG=C
+ export LANG
+ unset DISPLAY
+ rm -rf /var/tmp/beryl-vidcap-0.1.2-6.fc7-root-mockbuild
+ pushd seom
~/build/BUILD/beryl-vidcap-0.1.2/seom ~/build/BUILD/beryl-vidcap-0.1.2
+ make DESTDIR=/var/tmp/beryl-vidcap-0.1.2-6.fc7-root-mockbuild install
cc -W -Wall -std=c99 -Iinclude -ldl -lpthread -L.libs -lseom  -o seom-filter
src/filter/main.c
cc -W -Wall -std=c99 -Iinclude -ldl -lpthread -L.libs -lseom -lX11 -lXv -o
seom-player src/player/main.c
cc -W -Wall -std=c99 -Iinclude -ldl -lpthread -L.libs -lseom  -o seom-server
src/server/main.c
------------------------------------------------------
 (This uses original 0.1.2-6 spec file). Please fix this.

* License:
  I cannot find any GPL license document file in the source
  files. Only I can read is in capture.c, however, it is not
  GPL (looks like "free" license, however, what is this?)

  Well, other files than capture.c don't contain any license
  terms and no license document is provided, so, the license
  of this package is really questionable.......
Comment 11 Jarod Wilson 2006-11-17 16:31:12 EST
I like the rpath-killer there, very crafty, nice to not have to resort to
chrpath. I've put that in my local spec for the next build. I've also fully
fixed the cflags and -devel dependencies.

As for the undefined symbols... The gl* ones are indeed resolved by linking
against libGL, but I'm not having any luck figuring out exactly what to do for
the others. I did find that essentially the same unresolved symbols show up for
all beryl and compiz plugins though... Probably going to have to poke both
upstreams about this one...

On the license front... I see a #define _GNU_SOURCE, but yeah, the license block
is a little vague. I'll prod upstream for clarification.
Comment 12 tomc 2006-12-04 10:21:11 EST
(In reply to comment #11)
> On the license front... I see a #define _GNU_SOURCE, but yeah, the license block
> is a little vague. I'll prod upstream for clarification.

Do you mean the files in seom or the plugin itself? capture.c has a standard
header just like any other file in beryl-plugins, and the seom files have no
header, just a LICENSE file in the root directory. If you require that every
file needs to have a GPL header, I can add it, no problem.

I would also suggest, if somehow possible, to specify ARCH when building seom,
see http://www.neopsis.com/projects/yukon/wiki/YukonCompile - "Architecture
Optimizations"
Comment 13 Mamoru TASAKA 2006-12-12 23:34:50 EST
Well, any state change on this package?
Comment 14 Jarod Wilson 2006-12-13 16:03:06 EST
I've incorporated the asm stuff Tom suggested in comment #12 and done a 0.1.3
build, but it still suffers from the same undefined symbols problem, which I've
not been able to trace the root cause of just yet.

http://wilsonet.com/packages/beryl/beryl-vidcap-0.1.3-1.fc6.src.rpm
Comment 15 Mamoru TASAKA 2006-12-14 02:08:07 EST
(In reply to comment #14)
> but it still suffers from the same undefined symbols problem, which I've
> not been able to trace the root cause of just yet.

Well, for this I change my opinion. This library 
(/usr/lib/beryl/libcapture.so) is a plugin module for beryl and
undefined non-weal symbols issue can be ignored.

Then please check this rpmlint error.
-------------------------------------------
E: beryl-vidcap shlib-with-non-pic-code /usr/lib/libseom.so.0.0.0
-------------------------------------------
Comment 16 Chitlesh GOORAH 2006-12-14 06:03:24 EST
(In reply to comment #15)

> Then please check this rpmlint error.
> -------------------------------------------
> E: beryl-vidcap shlib-with-non-pic-code /usr/lib/libseom.so.0.0.0
> -------------------------------------------
> 

If beryl-vidcap is installed, each time yum performs a ldconfig, it spits an
error dur to usr/lib/libseom.so.0.0.0
Comment 17 Jarod Wilson 2006-12-14 17:08:24 EST
Ugh. I get no such errors (no rpmlint error and no ldconfig spew) at all on my
system, but its x86_64. I have a feeling this is an issue with the x86 asm...
Comment 18 tomc 2006-12-14 17:22:47 EST
It could be.. the .rodata section on top of frame.asm could cause this, but I'm
not an assembler expert so I can't tell for sure. but I could convert the
[yuv]Mul variables to 'immediates' (or how they are called), simply use the
values directly in the code instead of referencing the variables. Just need to
figure out how to do that correctly WRT endianess :)
Comment 19 Mamoru TASAKA 2006-12-14 23:06:51 EST
Created attachment 143732 [details]
Mock build log of beryl-vidcap 0.1.3-1 on FC-devel i386

Well, I am working on FC-devel i386 and
on this environ rpmlint surely complains about
beryl-vidcap 0.1.3-1.

I don't know about yasm, however I suspect this does
something because mockbuild log says:

----------------------------------------------------------------
libtool --tag=CC --mode=compile yasm -f elf -m x86 -o src/arch/x86/frame.lo
-prefer-non-pic src/arch/x86/frame.asm
mkdir src/arch/x86/.libs
 yasm -f elf -m x86 src/arch/x86/frame.asm -o src/arch/x86/.libs/frame.o
 yasm -f elf -m x86 src/arch/x86/frame.asm -o src/arch/x86/frame.o >/dev/null
2>&1
libtool --tag=CC --mode=link gcc -Wl,--as-needed -rpath /usr/lib -o libseom.la
src/buffer.lo src/client.lo src/codec.lo src/frame.lo src/server.lo
src/stream.lo src/arch/x86/frame.lo -ldl -lpthread
---------------------------------------------------------------------------
Comment 20 tomc 2006-12-15 04:59:05 EST
(In reply to comment #19)
> libtool --tag=CC --mode=compile yasm -f elf -m x86 -o src/arch/x86/frame.lo
> -prefer-non-pic src/arch/x86/frame.asm
> mkdir src/arch/x86/.libs
>  yasm -f elf -m x86 src/arch/x86/frame.asm -o src/arch/x86/.libs/frame.o
>  yasm -f elf -m x86 src/arch/x86/frame.asm -o src/arch/x86/frame.o >/dev/null
> 2>&1

Don't get fooled by the '-prefer-non-pic' because that's only there to tell
libtool not to add -DPIC to the commandline when executing yasm. If there is a
better way of doing that, please tell me - executing anything else than gcc with
libtool just asks for trouble.. :(
Comment 21 Garrick Staples 2006-12-15 23:35:35 EST
(In reply to comment #17)
> Ugh. I get no such errors (no rpmlint error and no ldconfig spew) at all on my
> system, but its x86_64. I have a feeling this is an issue with the x86 asm...

Definitely happens on i686.

[root@localhost ~]# ldconfig
ldconfig: /usr/lib/libseom.so.0 is not a symbolic link

[root@localhost ~]# ls -al /usr/lib/libseom.so*
-rwxr-xr-x 1 root root 21948 Nov 13 11:58 /usr/lib/libseom.so.0
-rwxr-xr-x 1 root root 20436 Nov 13 11:58 /usr/lib/libseom.so.0.0.0
[root@localhost ~]# rpm -q beryl-vidcap
beryl-vidcap-0.1.2-3
Comment 22 Mamoru TASAKA 2006-12-16 10:47:40 EST
(In reply to comment #21)
> [root@localhost ~]# ls -al /usr/lib/libseom.so*
> -rwxr-xr-x 1 root root 21948 Nov 13 11:58 /usr/lib/libseom.so.0
> -rwxr-xr-x 1 root root 20436 Nov 13 11:58 /usr/lib/libseom.so.0.0.0
> [root@localhost ~]# rpm -q beryl-vidcap
> beryl-vidcap-0.1.2-3

Please try 0.1.3-1. My complaint about shlib-with-non-pic-code is
for 0.1.3-1.

Comment 23 tomc 2006-12-17 20:05:24 EST
As of right now, the seom svn:extenals reference has been removed from
beryl-vidcap and the preferred way is to have separate seom and beryl-vidcap
packages.

But it's still unclear whether the beryl-vidcap plugin should be merged with
beryl-plugins or not, hopefully we'll have this resolved before the 0.1.4 release.
Comment 24 Mamoru TASAKA 2007-01-03 06:30:27 EST
Well, a Happy new year to everyone.

Jarod, it seems that you updated beryl related packages to
0.1.4, so how about this package?
Comment 25 Jarod Wilson 2007-01-03 10:45:05 EST
Yeah, once I get all the 0.1.4 bits pushed for FC6, I plan to start working on
this one again. Looks as if it may be a bit of work with seom broken out now (in
the long run, it would appear seom needs to be packaged by itself and BR/R by
beryl-vidcap).
Comment 26 Mamoru TASAKA 2007-01-12 11:01:38 EST
Well, would you once create srpm for 0.1.4?

Maybe someone (including me) can point out how beryl-vidcap
should be fixed if there is any problem.
Comment 27 Jarod Wilson 2007-01-12 17:54:17 EST
Sorry, sick wife and kids, plus my day job kept getting in the way... Finally
got around to it, SRPM available here:

http://people.redhat.com/jwilson/packages/beryl-vidcap/beryl-vidcap-0.1.4-1.fc6.src.rpm

At the moment, I'm just including a seom svn checkout in the package, will look
into packaging seom on its own in the future...
Comment 28 Mamoru TASAKA 2007-01-13 13:58:24 EST
Created attachment 145532 [details]
objdump log of libseom.so.0.0.0

Well, I tried 0.1.4-1.fc7 (on FC-devel i386), however,
libseom.so.0.0.0 still contains non-pic code.
Comment 29 Mamoru TASAKA 2007-01-13 14:00:02 EST
Created attachment 145533 [details]
Mock build log of beryl-vidcap-0.1.4-1.fc7

I attach a mockbuild log of 0.1.4-1 on FC-devel i386.
Any ideas, anyone?
Comment 30 tomc 2007-01-13 18:28:58 EST
(In reply to comment #28)
> Well, I tried 0.1.4-1.fc7 (on FC-devel i386), however,
> libseom.so.0.0.0 still contains non-pic code.

How do I find out which code (symbols?) is non-PIC?

Comment 31 Mamoru TASAKA 2007-01-14 12:26:00 EST
(In reply to comment #30)
> (In reply to comment #28)
> > Well, I tried 0.1.4-1.fc7 (on FC-devel i386), however,
> > libseom.so.0.0.0 still contains non-pic code.
> 
> How do I find out which code (symbols?) is non-PIC?

Actually I don't know _which_ code is non-PIC, however,
the existence of the line of "TEXTREL" in  objump result 
(in comment 28) means that some codes are non-PIC.

Comment 32 tomc 2007-01-31 19:30:46 EST
No TEXTRELs anymore in x86 code.
Btw, making code x86 PIC-conform is really hard.. in amd64 I just write 'mov
rax,[var wrt rip]', in x86 I have to use call/pop, _GLOBAL_OFFSET_TABLE_, 'wrt
..gotoff/..gotpc' and such weird stuff.. I decided to use a different (easier)
technique, but it's still much more difficult then a simple 'wrt rip' :(
Comment 33 Jarod Wilson 2007-02-02 09:34:15 EST
(In reply to comment #32)
> No TEXTRELs anymore in x86 code.
> Btw, making code x86 PIC-conform is really hard.. in amd64 I just write 'mov
> rax,[var wrt rip]', in x86 I have to use call/pop, _GLOBAL_OFFSET_TABLE_, 'wrt
> ..gotoff/..gotpc' and such weird stuff.. I decided to use a different (easier)
> technique, but it's still much more difficult then a simple 'wrt rip' :(

Heh, good stuff. Were these updates included with the 0.1.99.2 snap of beryl, or
sometime after? I've got an updated srpm for 0.1.99.2 sitting here:

http://people.redhat.com/jwilson/packages/beryl-vidcap/beryl-vidcap-0.1.99.2-1.fc6.src.rpm
Comment 34 Mamoru TASAKA 2007-02-02 11:13:08 EST
Created attachment 147234 [details]
Mock build log of  beryl-vidcap-0.1.99.2-1.fc7

Mockbuild log of beryl-vidcap-0.1.99.2-1 on
FC-devel i386.

I just tried to rebuild, not checked anything else.
However please check the log why rebuilding failed.
Comment 35 tomc 2007-02-02 11:27:14 EST
> ./seom.pc.in /usr lib
> make: *** [seom.pc] Error 1

you have to either build seom from an official tarball
(http://dbservice.com/ftpdir/tom/seom/tarballs/) or use a svn checkout and run
'make' from the directory. The reason is because seom uses either the file
VERSION  (which is automatically created when I build the tarball) or the output
of 'svn info' to get the version, and the version is needed to build seom.pc.
Comment 36 Mamoru TASAKA 2007-02-02 13:07:19 EST
(In reply to comment #35)
> > ./seom.pc.in /usr lib
> > make: *** [seom.pc] Error 1
> 
> you have to either build seom from an official tarball
> (http://dbservice.com/ftpdir/tom/seom/tarballs/) or 

Umm?? This is the first time I heard that there is
a official _seperated_ tarball of seom!!

Then another review request for seom should be submitted,
should make this review request blocked by the seom review
request, and the review request for seom should be reviewed
first.
Comment 37 Jarod Wilson 2007-02-04 17:28:15 EST
Ah, first I'd heard of an official tarball as well... I'm just putting the finishing touches on a stand-alone 
seom package, which I'll hopefully get submitted for review shortly.
Comment 38 Jarod Wilson 2007-02-05 12:16:28 EST
This review is now blocked on the acceptance of build requirement seom, which is
being tracked in bug 227309.
Comment 39 Jarod Wilson 2007-03-29 13:28:04 EDT
Just a minor update: I'm told all the unresolved symbols in comment #10 are
normal, not something we should worry about, per bug 216232.
Comment 40 tomc 2007-06-05 15:48:02 EDT
Just now, as beryl is slowly being replaced by compcomm/compiz/coral (or
whatever because I don't follow the incredibly stupid discussions anymore) you
decide to work on this package again? I wouldn't bother anymore trying to push
this into the main distribution, unless you intend to support beryl in it (whose
support will stop after the compcomm release..).

What does 'fedora-review?' mean anyway?
Comment 41 Jason Tibbitts 2007-06-05 19:28:44 EDT
fedora-review? means the package is being reviewed.
Comment 42 Mamoru TASAKA 2007-06-28 13:39:14 EDT
So how should I treat this review request? Should I close this
with NOTABUG?
Comment 43 Jarod Wilson 2007-06-28 14:12:13 EDT
Yeah, with beryl going away, lets just drop this one.
Comment 44 Mamoru TASAKA 2007-06-28 14:17:47 EDT
Okay, thank you.