Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 207685 - Review Request: gstreamer-plugins-farsight - GStreamer plug-ins for farsight protocol
Summary: Review Request: gstreamer-plugins-farsight - GStreamer plug-ins for farsight ...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jef Spaleta
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-09-22 15:24 UTC by Brian Pepple
Modified: 2008-04-23 16:20 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-12-23 16:36:30 UTC
Type: ---
Embargoed:
j: fedora-cvs+


Attachments (Terms of Use)

Description Brian Pepple 2006-09-22 15:24:24 UTC
Spec URL: http://piedmont.homelinux.org/fedora/telepathy/gstreamer-plugins-farsight.spec
SRPM URL: http://piedmont.homelinux.org/fedora/telepathy/gstreamer-plugins-farsight-0.10.2-1.src.rpm
Description: This is a set of plugins for GStreamer that will be used by Farsight for Audio/Video conferencing.

Comment 1 Jef Spaleta 2006-12-03 02:57:20 UTC
The above urls don't seem to work.  Can you give me updated url info for these?

-jef

Comment 2 Brian Pepple 2006-12-03 03:13:22 UTC
(In reply to comment #1)
> The above urls don't seem to work.  Can you give me updated url info for these?

I just retired my web server this week, and haven't got around to obtaining any
new space to host this at. 



Comment 3 Jef Spaleta 2006-12-03 03:21:28 UTC
email them to me and I can put them up for you.

-jef

Comment 4 Jef Spaleta 2006-12-03 06:04:08 UTC
Here we go.

SRPM:
http://jspaleta.thecodergeek.com/Fedora%20SRPMS/gstreamer-plugins-farsight/gstreamer-plugins-farsight-0.10.2-1.src.rpm
SPEC:
http://jspaleta.thecodergeek.com/Fedora%20SRPMS/gstreamer-plugins-farsight/gstreamer-plugins-farsight.spec

Let me grab a beer and I'll start a mock build and if that passes I'll start a
formal review.

One thing, is there any way to test the functionality of this plugin currently?

-jef


Comment 5 Jef Spaleta 2006-12-03 06:19:34 UTC
One minor quibble.
Would it make sense to use the majorminor define in the version tag, like you do
later in the file location strings? It reduces the potential for out of sync
strings if/when upstream moves to 0.11

-jef 

Comment 6 Jef Spaleta 2006-12-03 06:38:50 UTC
A couple of questions with regard to the configure options

--disable-jasper	
jasper is in Extras currently. Is there a specific problem associated with
enabling it?  

--with-plugins=rtpdemux,rtpjitterbuffer
Are there other optional plugins to explore?

-jef

Comment 7 Brian Pepple 2006-12-03 15:16:08 UTC
(In reply to comment #6)
> A couple of questions with regard to the configure options
> 
> --disable-jasper	
> jasper is in Extras currently. Is there a specific problem associated with
> enabling it?  
> 
> --with-plugins=rtpdemux,rtpjitterbuffer
> Are there other optional plugins to explore?

Most of the other plugins are still fairly experimental, and probably not stable
enought to build currently.

The best way to test this is with Stream Engine, which I haven't yet submitted
to FE (September was fairly busy, and it slipped my mind).  I'll send you the
spec & SRPM for it once I update it to the latest version.

Once you have gstreamer-plugins-farsight & Stream Engine installed you can test
it by running 'STREAM_ENGINE_PERSIST=1 telepathy-stream-engine'

Comment 8 Brian Pepple 2006-12-03 17:21:11 UTC
Here's my review request for telepathy-stream-engine:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=218217

BTW, I've brought my server back online for now for these packages.

Comment 9 Jef Spaleta 2006-12-23 02:19:41 UTC
GOOD:
*Built in mock on i386 against fedora-development
*rpmlint runs cleanly against it
*follows naming guidelines for addon packages
*licnese is GPL and spec tag matches
*COPYING file in %doc
*spec in US/english and legible
*Include source matches upstream md5sum
77c92554c2bd57ad1b426e5ba50eb9a8  gst-plugins-farsight-0.10.2.tar.gz
*buildrequires look good
*no locales to worry about
*only gstreamer plugin so's no shared libs in default linker path
*not relocatable
*does not create any unowned directories. Package requires   
  gstreamer-plugins-base which in turns requires gstreamer which owns 
  /usr/lib/gstreamer-0.10
*no duplicates in %files
*file permissions seem fine
*install and clean sections look good
*macro use is consistent
*docs is good
*no -devel subpackage
*la files removed in install section
*not a gui, no desktop file needed
*appears to meet all packaging guidelines


One small thing I'd like to see changed. Can you rename the macro at from
majorminor to gst_majorminor?  I was slightly confused initially because its the
same numerical value as the package version majorminor. Its clear now its a
macro to define the gstreamer majorminor, but it would be helpful for clarity to
change the name to gst_majorminor.  This is not a blocker however, but it may
help save a little time if someone has to pick this package up from you later.

This package is APPROVED for entry into the development tree.


I haven't tested this yet, I'm be building stream-engine next so I can test this. 


Comment 10 Brian Pepple 2006-12-23 16:36:30 UTC
Jef, thanks for the review.

Comment 11 Brian Pepple 2008-04-23 15:42:48 UTC
Package Change Request
======================
Package Name: gstreamer-plugins-farsight
New Branches: OLPC2
Updated Fedora Owners: gdesmott,bpepple

Comment 12 Jason Tibbitts 2008-04-23 16:20:59 UTC
CVS done.


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