Bug 428959 - Review Request: coriander - Control a 1394 digital camera interactively
Review Request: coriander - Control a 1394 digital camera interactively
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Hans de Goede
Fedora Extras Quality Assurance
Depends On: 239043
  Show dependency treegraph
Reported: 2008-01-16 09:42 EST by Tim Niemueller
Modified: 2008-01-28 11:01 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2008-01-21 03:46:58 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
hdegoede: fedora‑review+
dennis: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Tim Niemueller 2008-01-16 09:42:52 EST
Spec URL: http://fedorapeople.org/~timn/coriander/coriander.spec
SRPM URL: http://fedorapeople.org/~timn/coriander/coriander-2.0.0-rc5.4.cvs20080116.fc8.src.rpm
Description: Coriander is a GUI that let you control your 1394 digital video camera interactively. It features SDL display, FTP image posting, file saving, and Real streaming. It is for IIDC cameras, not for consumer grade DV cameras.
Comment 1 Hans de Goede 2008-01-18 09:11:45 EST
Full review done, results:

Must Fix
- Remove the following lines from src/build_menus.c:
  // add edge sense option
  glade_menuitem = gtk_menu_item_new_with_label (_("Edge Sense"));
  gtk_widget_show (glade_menuitem);
  gtk_menu_append (GTK_MENU (new_menu), glade_menuitem);
  g_signal_connect ((gpointer) glade_menuitem, "activate",
                      G_CALLBACK (on_bayer_menu_activate),

  As libdc1394-2.0.1 no longer supports this

- Change Release to "0.4.rc5%{?cvs_snapshot}%{?dist}", as per the versioning 
  guidelines so that upgrading to 2.0.0 final can happen without epoch.

- Drop the unnecessary explicit: "Requires: libdc1394 >= 2.0.0" 2.0.0 has a 
  different soname then libdc1394 v1, so there is no need for this.

- Reword "and Real streaming" in description, this made me think of Real
  video (realmedia, .rm files) streaming which is patented.

- Don't use an url for cvs snapshots, as those cannot be downloaded from such an

- Drop buildrequires ffmpeg-devel, we don't have ffmpeg-devel in Fedora

- Change "--vendor rpmforge" desktop-file-install argument to "--vendor fedora"
  and drop the obsolete "--add-category X-Red-Hat-Base" argument

- Add (as comment above Source0) instructions on howto recreate the tarbal from 
  CVS (so cvs co command, any commands used to clean the checkout, etc.)

Should Fix

- Add an Icon= line to the .desktop fil, install a suitable icon under
  /usr/share/icons/hicolor/48x48/apps, and add icon cache update scriptlets  

Not checked

- sources matching due to missing reproduction instructions for the tarbal.
Comment 2 Tim Niemueller 2008-01-18 19:27:41 EST
First, thanks for the thorough review!

- Using the just-released version 2.0.0-rc6. No CVS snapshot no more
- EdgeSense auto-detection has been added in this version which eliminates the
first remark
- Changed the release accordingly to the mentioned version number
- Dropped explicit require for libdc1394 >= 2.0.0
- Reworded to "video streaming"
- URL is now ok since we are using the release candidate tarball
- ffmpeg-devel no longer required
- desktop-file-install parameters fixed
- Added Icon line to desktop file
- Added ftplib to BR to build coriander with FTP support

The new package and spec file can be found at
Comment 3 Hans de Goede 2008-01-19 13:40:05 EST
All must fix items fixed: Approved.

I still have one should fix left though, please consider fixing this before import:

* The preferred Source URL for sourceforge downloads (see guidelines) says that 
  the Source tag should be:
Source: http://downloads.sourceforge.net/coriander/coriander-%{real_version}.tar.gz

Notice that I tried the currently used URL with "spectool -g coriander.spec",
and it failed, whereas the prefered form as above works.
Comment 4 Tim Niemueller 2008-01-19 15:23:32 EST
I have changed the spec file accordingly and uploaded the new files.
Comment 5 Tim Niemueller 2008-01-19 15:24:02 EST
New Package CVS Request
Package Name: coriander
Short Description: Control a 1394 digital camera interactively
Owners: timn
Branches: F-8
InitialCC: timn
Cvsextras Commits: yes
Comment 6 Kevin Fenzi 2008-01-19 15:54:22 EST
cvs done.
Comment 7 Tim Niemueller 2008-01-19 19:02:12 EST
I have imported the sources. I tried a build not realizing that it cannot find
libdc1394-devel until it's pushed... Going to resubmit the build as soon as the
libdc1394 package has landed.
Comment 8 Hans de Goede 2008-01-20 03:56:26 EST
Note that for F-8 you can send a mail to rel-eng <the-symbol-knows-as-at>
fedoraproject.org asking the package to be included into the F-8 buildroot, even
though it hasn't hit updates yet. This is usually much faster then waiting for
it to be pushed as an update.

The proper wording is tagging into the buildroot and always include full name
and EVR, so a possible subject for such a mail would be:
"Please tag libdc1394-2.0.1-3.fc8 for F-8 buildroot inclusion"

And then some explanation why (to build coriander) ihn the body.
Comment 9 Tim Niemueller 2008-01-21 03:46:58 EST
Builds done. Update push is requested at

Closing this as NEXTRELEASE. Thanks for the review Hans.
Comment 10 Tim Niemueller 2008-01-28 09:39:51 EST
Package Change Request
Package Name: coriander
New Branches: F-7 EL-5
Comment 11 Dennis Gilmore 2008-01-28 11:01:49 EST
CVS Done

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