Bug 239043 - Review Request: libdc1394 - IEEE 1394 based Digital Camera control library
Summary: Review Request: libdc1394 - IEEE 1394 based Digital Camera control library
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Dominik 'Rathann' Mierzejewski
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
: 239184 (view as bug list)
Depends On:
Blocks: 239490 428959
TreeView+ depends on / blocked
 
Reported: 2007-05-04 16:09 UTC by Matthias Saou
Modified: 2017-03-20 14:20 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-01-20 00:01:02 UTC
Type: ---
Embargoed:
dominik: fedora-review+
kwizart: fedora-cvs-


Attachments (Terms of Use)

Description Matthias Saou 2007-05-04 16:09:49 UTC
Spec URL: http://ftp.es6.freshrpms.net/tmp/extras/libdc1394/libdc1394.spec
SRPM URL: http://ftp.es6.freshrpms.net/tmp/extras/libdc1394/libdc1394-2.0.0-0.1.rc5.src.rpm
Description:
Libdc1394 is a library that is intended to provide a high level programming
interface for application developers who wish to control IEEE 1394 based
cameras that conform to the 1394-based Digital Camera Specification.

Comment 1 Dominik 'Rathann' Mierzejewski 2007-05-05 04:30:58 UTC
I'll review this this weekend.

Comment 2 Stephanos Manos 2007-05-05 13:44:36 UTC
Many apps that use libdc1394 (ie ekiga - pwlib) require the older (stable)
version 1.2.1

Also it is useless (and corresponding apps like coriander) in current rawhide
due to the new experimental ieee1394 stuck that does not provide the raw1394 &
video1394 and the devices /dev/raw1394 & /dev/video1394/0...

Also when i asked in the list about it i was given the following answer 
>Please DO NOT propagate RPMs before a final 2.0 release (it's fine to
>use your own RPMs for internal purpose though). 


Comment 3 Nicolas Chauvet (kwizart) 2007-05-05 19:53:42 UTC
Well i was also working on it at the same time but on libdc1394(-1)
See: https://bugzilla.redhat.com/239184

I think that would be fine (as state by Stephanos) not to ship this version.
But we can to make a dual installation for testing if needed...

As such:
i've noticed that svn trunk:
"changed libdc1394.pc to libdc1394-2.pc to avoid pkg-config conflicts"
that would be good to have svn snapshoot to integrate this change.

only this binary (and i suppose libaries also) will conflict with -1:
%{_bindir}/dc1394_vloopback
We may uses suffix if needed...
no need to changes about includedir as :
-1 will have libdc1394
-2 will have dc1394

I wonder why libXext-devel was in -1 and not in -2 (will check for -1)

I would suggest to rename this package to libdc1394_2 to avoid conflict...




Comment 4 Dominik 'Rathann' Mierzejewski 2007-05-08 21:11:12 UTC
A brief look into libdc1394-devel archives suggests that the API hasn't been
finalized yet. And it should be possible to install it alongside libdc1394-1.x,
like Nicolas suggests.

Unfortunately, it appears to contain patent-encumbered code:
dc1394/bayer.c:
...
/* Edge Sensing Interpolation II from http://www-ise.stanford.edu/~tingchen/ */
/*   (Laroche,Claude A.  "Apparatus and method for adaptively
     interpolating a full color image utilizing chrominance gradients"
     U.S. Patent 5,373,322) */
void
dc1394_bayer_EdgeSense(const uint8_t *restrict bayer, uint8_t *restrict rgb, int
sx, int sy, int tile)
...

Which is a simple interpolation algorithm. Software patents need to die!


Comment 5 Nicolas Chauvet (kwizart) 2007-07-16 22:56:13 UTC
Rathann, can you confirm that something has changed to:
  476 /* coriander's Bayer decoding (GPL) */
  477 /* Edge Sensing Interpolation II from
http://www-ise.stanford.edu/~tingchen/ ...

With the possibilty for a 2.6.22 kernel to hit Fedora Core 6 threre is a need to
solve this... 

Comment 6 Dominik 'Rathann' Mierzejewski 2007-07-22 17:39:47 UTC
The mail archives seem to be inaccessible (I'm getting internal server errors
from sourceforge), but the main site doesn't list any new releases, so I'm
guessing nothing's changed so far.

Comment 7 Jarod Wilson 2007-12-04 18:37:50 UTC
Looks like the latest version is libdc1394 2.0.0-rc7 now. Builds just fine for me with a minor tweak to 
Matthais' spec file (pkgconfig file was renamed).


Comment 8 Dominik 'Rathann' Mierzejewski 2007-12-06 00:05:19 UTC
That is good to know and I've just tested it myself. However, we still can't
allow this in until we get a 'go ahead' from Legal.

Comment 9 Nicolas Chauvet (kwizart) 2007-12-06 00:41:26 UTC
Also, most projects still use libdc1394-1 (which cannot support juju firewire
for now...). If libdc1394-2 can be approved I expect it won't solve anything...
But maybe that will be a more valuable task to patches apps to support -2 than
to have -1 made compatible with juju...

Comment 10 Dominik 'Rathann' Mierzejewski 2008-01-07 18:46:40 UTC
FYI: there's a patch to add libdc1394-2 support to ffmpeg floating around
ffmpeg-devel with good chances to be accepted. What applications still don't
support libdc1394-2?

Maybe we could get this into livna/rpmfusion first while we wait for the legal
review?


Comment 11 Jarod Wilson 2008-01-07 19:37:37 UTC
Also note that libdc1394 2.0.0 was officially released a few days ago.

Comment 12 Nicolas Chauvet (kwizart) 2008-01-07 21:00:19 UTC
vlc still isn't compatible with libdc1394-2
I will try to see if this can be patched...

Comment 13 Tim Niemueller 2008-01-07 22:49:10 UTC
We are using libdc1394-2 now for quite a while now internally and I'm
maintaining RPMs thereof (based initially on atrpms packages). I have put builds
with the official 2.0.0 release to http://fedorapeople.org/~timn/libdc1394. They
replace the old version though. We use them on robots where we don't care about
other media software that depends on version 1 libs.

Just if you want to use it until legal questions have been answered.

Comment 14 Tim Niemueller 2008-01-08 23:32:20 UTC
The packages have been removed for the time being because of the possible patent
problems. The issue has been discussed on the libdc1394-devel mailing list and a
patch was proposed to remove the problematic piece of code. I'm currently
waiting for a reply from the Fedora Board if that is a sufficient solution
(freetype1 package is a precedence). If that is the case I will upload the
packages with the patch again. Stay tuned.

Comment 15 Tim Niemueller 2008-01-16 10:35:17 UTC
After discussions on the libdc1394-devel mailing list it was decided that the
patent encumbered code be dropped from libdc1394 (see changeset 535). Version
2.0.1 which has been released last night does no longer contain this code.

Because of this I have uploaded the 2.0.1 RPMs again to
http://fedorapeople.org/~timn/libdc1394/. Please have a look and try them.

Comment 16 Nicolas Chauvet (kwizart) 2008-01-16 12:21:07 UTC
Few notes (not a full review - I let this to Rathann).

You are missing to BuildRequires: libtool, doxygen
And I don't see where libX11 is used (not in libs nor in example binaries)
It could be dropped probably.

Maybe the autotools regeneration could be dropped, which will also drop the
libtool needs.

I would think it is valuable to build the docs into a separate package to avoid
multilibs problems. It is Also required to split the binaries from the libs for
the same purpose. (there is a need to check if some binary is mandatory for the
software to work or if they are only examples).
See http://fedoraproject.org/wiki/PackagingDrafts/MultilibTricks
(specially you could use make install DESTDIR=%{buildroot} INSTALL="%{__install}
-p" )

* tested against pre gcc 4.3.0 - it works - good

* about the juju include - as a userland application, we shouldn't search for
headers from within the kernel-devel. But in this case we want to link from the
internal version: dc1394/juju (which is done as a falback as kernel-devel isn't
BR). It might be possible to have it from the kernel-headers when available. 

* Not a complete review anyway, I let Rathann do it.
(will be AFK next week).


Comment 17 Matthias Saou 2008-01-16 12:26:41 UTC
Just a quick note to say that I have no objection in seeing Tim Niemueller take
ownership off this package, quite the opposite :-)

Time to remove the blocker on FE-Legal, no?

Comment 18 Hans de Goede 2008-01-16 12:30:48 UTC
(In reply to comment #17)
> Time to remove the blocker on FE-Legal, no?

We are waiting on an OK from Spot for this.


Comment 19 Tim Niemueller 2008-01-16 15:38:31 UTC
(In reply to comment #16)
> You are missing to BuildRequires: libtool, doxygen

Added.

> And I don't see where libX11 is used (not in libs nor in example binaries)
> It could be dropped probably.

dropped

> Maybe the autotools regeneration could be dropped, which will also drop the
> libtool needs.

I want to keep it around to not loose the ability to build svn-based packages by
only setting the svn snapshot.

> I would think it is valuable to build the docs into a separate package to avoid
> multilibs problems. It is Also required to split the binaries from the libs for
> the same purpose. (there is a need to check if some binary is mandatory for the
> software to work or if they are only examples).
> See http://fedoraproject.org/wiki/PackagingDrafts/MultilibTricks
> (specially you could use make install DESTDIR=%{buildroot} INSTALL="%{__install}
> -p" )

I have created -docs and -tools sub-packages to containt the doxygen HTML
documentation and the binaries.

> * about the juju include - as a userland application, we shouldn't search for
> headers from within the kernel-devel. But in this case we want to link from the
> internal version: dc1394/juju (which is done as a falback as kernel-devel isn't
> BR). It might be possible to have it from the kernel-headers when available. 

I don't think I really understood what you mean. What I did now was adding
kernel-headers as a BR and us /usr/include instead of the build-in-mods dir.

> * Not a complete review anyway, I let Rathann do it.
> (will be AFK next week).

I have uploaded the new packages and spec file to
http://fedorapeople.org/~timn/libdc1394/.

Please have a look and let me know if there are still caveats.

The coriander packages have been filed for review as bug #428959.


Comment 20 Dominik 'Rathann' Mierzejewski 2008-01-18 10:45:21 UTC
+ rpmlint OK:
libdc1394-tools.x86_64: W: no-documentation
+ md5sum matches upstream:
6b5abeed347e7f6682f4fa9f06437a5f  libdc1394-2.0.1.tar.gz
- redundant BuildRequires: automake autoconf (pulled in by libtool)
- missing Requires: pkgconfig in -devel
? wouldn't autoreconf -i work just as well in %prep?

Otherwise it looks fine.


Comment 21 Hans de Goede 2008-01-18 12:12:25 UTC
I would like to advocate to not use autoXXX at all when not absolutely
necessary, using it when not needing could mean potential breakage in the future
when a new version of automake / autoconf / libtool hits the repo.

So please use %if %{svn} blocks around both the calling and the BuildRequiring
of the autoxxx tools, atleast if you want to preserve the posibility to easily
switch to an svn snapshot (do we really need that?) otherwise just remove the
autoxxx stuff completely, please.


Comment 22 Jarod Wilson 2008-01-18 14:38:45 UTC
+1 for the %if %{svn} blocks around the auto bits (including the dependencies,
since they shouldn't be needed for a release build), was going to suggest
something similar.

Comment 23 Tim Niemueller 2008-01-18 23:58:55 UTC
- The tools do in fact have no own documentation.
- Removed redundant BR for automake and autoconf
- -devel package now requires pkgconfig
- autotools are now only used if svn_snapshot is actually set. I want to keep
the svn functionality around at least for a while. libtool is also only BR'ed in
that case.

New packages and spec file available at http://fedorapeople.org/~timn/libdc1394/

Comment 24 Dominik 'Rathann' Mierzejewski 2008-01-19 14:51:20 UTC
(In reply to comment #23)
> - The tools do in fact have no own documentation.

Yes, I said it was OK.

[...]
> New packages and spec file available at http://fedorapeople.org/~timn/libdc1394/

Well, specfile isn't there, actually, but that's not much of a problem. I got
the src.rpm.

Current (2.0.1-3) version is APPROVED, pending green light from spot.


Comment 25 Tim Niemueller 2008-01-19 16:15:24 UTC
(In reply to comment #24)
> (In reply to comment #23)
> > - The tools do in fact have no own documentation.
> Yes, I said it was OK.

Ok. Just wanted to state that I actually read that line :-)

> [...]
> > New packages and spec file available at http://fedorapeople.org/~timn/libdc1394/
> 
> Well, specfile isn't there, actually, but that's not much of a problem. I got
> the src.rpm.

Sorry, forgot to scp it. Uploaded it now for reference.

> Current (2.0.1-3) version is APPROVED, pending green light from spot.

Great! I'll ping spot on this.


Comment 26 Tom "spot" Callaway 2008-01-19 17:10:09 UTC
Lifting FE-Legal, looks good to me.

Comment 27 Tim Niemueller 2008-01-19 17:30:51 UTC
New Package CVS Request
=======================
Package Name: libdc1394
Short Description: IEEE 1394 based Digital Camera control library
Owners: timn
Branches: F-8
InitialCC: timn
Cvsextras Commits: yes

Comment 28 Kevin Fenzi 2008-01-19 18:13:33 UTC
Could we get the formal setting of the fedora-review flag to + here from Dominik
before doing cvs?


Comment 29 Dominik 'Rathann' Mierzejewski 2008-01-19 19:44:41 UTC
But of course.

Comment 30 Kevin Fenzi 2008-01-19 20:48:49 UTC
Thanks. 

cvs done.

Comment 31 Tim Niemueller 2008-01-20 00:01:02 UTC
Thanks Kevin.

I have imported the sources and build the packages for rawhide and F-8. I have
requested an update, please see
https://admin.fedoraproject.org/updates/F8/pending/libdc1394-2.0.1-3.fc8, try
the packages and raise the update karma to get it pushed.

Thanks to all reviewers and especially to the libdc1394 guys for throwing away
the potentially patent encumbered code which made the inclusion in Fedora possible!

Closing this as NEXTRELEASE, please re-open if there is anything directly
regarding the packaging.

Comment 32 Hans de Goede 2008-01-20 08:57:56 UTC
(In reply to comment #31)
> I have imported the sources and build the packages for rawhide and F-8. I have
> requested an update, please see
> https://admin.fedoraproject.org/updates/F8/pending/libdc1394-2.0.1-3.fc8, try
> the packages and raise the update karma to get it pushed.
> 

Why? You can just select push to stable as submitter of the update, you can even
designate an update to directly go to stable when creating an update (which is
what I always do when pushing a new package, as that is not really an update).


Comment 33 Tim Niemueller 2008-01-20 18:17:05 UTC
I cannot push the package. Maybe that needs special privileges in the FAS? I'm
going to mail rel-eng as suggested in the coriander rr.

If three "vote" for "works for me" this should get pushed automatically,
shouldn't it?

Comment 34 Hans de Goede 2008-01-20 20:27:43 UTC
(In reply to comment #33)
> I cannot push the package. Maybe that needs special privileges in the FAS? I'm
> going to mail rel-eng as suggested in the coriander rr.
> 
> If three "vote" for "works for me" this should get pushed automatically,
> shouldn't it?

I think you're confusing 2 things here. An update can either be (headed for)
testing or stable this can be chosen when creating it and changed at will by the
submitter of the update. An update in testing will automatically be (headed for)
stable after it gets +3 carma. The actual signing and putting the .rpm in the
repo (the pushing) gets done by one of the few humans with access to the signing
key, this thus is always done manually and happens when a human gets around to it.

If you chose to make the update go directly to stable when creating it as soon
as a human gets around to it it will show up in the stable / normal updates
repository.


Comment 35 Dominik 'Rathann' Mierzejewski 2008-01-27 17:55:53 UTC
I'd like to request F-7 branch (if it makes sense)?

Comment 36 Tim Niemueller 2008-01-27 22:12:06 UTC
Joup, should work on F-7. If I'm going to request the new branch anyway, would
it make sense to request EL-4/5 as well? Can someone please tell which firewire
stack is used in these distros? I would think that EL-4 does not use the juju
stack? In that case I would need to tweak the spec a bit.

Comment 37 Jarod Wilson 2008-01-28 03:32:15 UTC
No firewire at all in RHLE4, juju firewire in RHEL5.1 and later.

Comment 38 Tim Niemueller 2008-01-28 12:09:56 UTC
What would be the appropriate way to require RHEL 5.1 and later in the EL-5
branch? Or should we skip EL-5 for libdc1394 altogether?

Comment 39 Jarod Wilson 2008-01-28 14:32:22 UTC
For EL5, you could add 'Conflicts: kernel < 2.6.18-53.el5' to let folks know they need at least that kernel 
or newer for the firewire kernel bits.

Comment 40 Tim Niemueller 2008-01-28 14:38:49 UTC
Package Change Request
======================
Package Name: libdc1394
New Branches: F-7 EL-5


Comment 41 Dennis Gilmore 2008-01-28 15:35:46 UTC
CVS Done

Comment 42 Tim Niemueller 2010-11-07 00:14:39 UTC
Package Change Request
======================
Package Name: libdc1394
New Branches: el6
Owners: timn

Comment 43 Jason Tibbitts 2010-11-07 15:39:12 UTC
Git done (by process-git-requests).

Comment 44 Nicolas Chauvet (kwizart) 2010-11-07 15:48:25 UTC
I don't understand what's happened here, but libdc1394 is part from base in el6
It shouldn't be requested for el6

@Jason, can you remove this branch ?

Comment 45 Nicolas Chauvet (kwizart) 2010-11-07 15:58:57 UTC
<tibbs|h> They'll have to open an infrastructure ticket to get rid of the branch they should not have requested.

@timn, you need to request the branch to be removed here:
https://fedorahosted.org/fedora-infrastructure/

Comment 46 Tim Niemueller 2010-11-07 23:56:08 UTC
*sigh* Is there a list of the packages which are now in RHEL? I'd really wish they would use the same infrastructure for such packages to a) allow contributions from non-RH people and b) make it more obvious what's happening. A note to the former maintainer that the package will be in core and thus no longer in EPEL would have been nice as well. In general I'm happy how RH is doing things, but this procedure clearly would benefit from some improvements and more communication.

https://fedorahosted.org/fedora-infrastructure/ticket/2465

Comment 47 Jason Tibbitts 2010-11-08 03:07:55 UTC
If you have the means to maintain and properly test an el6 branch of your package, surely you could just run 'yum search' on your test machine.

Comment 48 Peter Lemenkov 2017-03-20 11:48:42 UTC
*** Bug 239184 has been marked as a duplicate of this bug. ***


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