Bug 207685 - Review Request: gstreamer-plugins-farsight - GStreamer plug-ins for farsight protocol
Review Request: gstreamer-plugins-farsight - GStreamer plug-ins for farsight ...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jef Spaleta
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-09-22 11:24 EDT by Brian Pepple
Modified: 2008-04-23 12:20 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-12-23 11:36:30 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Brian Pepple 2006-09-22 11:24:24 EDT
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-02 21:57:20 EST
The above urls don't seem to work.  Can you give me updated url info for these?

-jef
Comment 2 Brian Pepple 2006-12-02 22:13:22 EST
(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-02 22:21:28 EST
email them to me and I can put them up for you.

-jef
Comment 4 Jef Spaleta 2006-12-03 01:04:08 EST
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 01:19:34 EST
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 01:38:50 EST
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 10:16:08 EST
(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 12:21:11 EST
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-22 21:19:41 EST
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 11:36:30 EST
Jef, thanks for the review.
Comment 11 Brian Pepple 2008-04-23 11:42:48 EDT
Package Change Request
======================
Package Name: gstreamer-plugins-farsight
New Branches: OLPC2
Updated Fedora Owners: gdesmott,bpepple
Comment 12 Jason Tibbitts 2008-04-23 12:20:59 EDT
CVS done.

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