Bug 229098 - Review Request: openjpeg - JPEG 2000 codec library
Review Request: openjpeg - JPEG 2000 codec library
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Xavier Lamien
Fedora Package Reviews List
:
Depends On:
Blocks: secondlife
  Show dependency treegraph
 
Reported: 2007-02-16 19:26 EST by Callum Lerwick
Modified: 2012-04-16 13:59 EDT (History)
7 users (show)

See Also:
Fixed In Version: 1.2-3.20071114svn480.fc7
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-11-26 13:38:16 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
lxtnow: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Callum Lerwick 2007-02-16 19:26:37 EST
Spec URL: http://www.haxxed.com/rpms/secondlife/openjpeg.spec
SRPM URL: http://www.haxxed.com/rpms/secondlife/openjpeg-1.1-1.src.rpm

Description: 
The OpenJPEG library is an open-source JPEG 2000 codec written in C language.
It has been developed in order to promote the use of JPEG 2000, the new
still-image compression standard from the Joint Photographic Experts Group
(JPEG).


This is required by the Second Life client. As such, it has some patches needed to make it all work. spot deserves credit for finding these.
Comment 1 Xavier Lamien 2007-02-18 22:01:22 EST
from spec:

* You should add timestamps INSTALL="install -p" to your make install.
* replace openjpeg by it macros %{name}
* you forgot to comment why patch0 was set.


Comment 2 Xavier Lamien 2007-02-18 22:02:29 EST
rpmlint ouput error on -devel package:

no-documentation.


Comment 3 Xavier Lamien 2007-02-22 22:43:50 EST
ping ?
Comment 4 Callum Lerwick 2007-02-23 19:10:31 EST
Sorry, I got tied up with other things. Updated package:

http://www.haxxed.com/rpms/secondlife/openjpeg-1.1-2.src.rpm
http://www.haxxed.com/rpms/secondlife/openjpeg.spec

* Sat Feb 17 2007 Callum Lerwick <seg@haxxed.com> 1.1-2
- Move header to a subdirectory.
- Fix makefile patch to preserve timestamps during install.

I hate using macros unless there's a really good reason. The package name is
very unlikely to change, and on the rare occasion that it might, a search and
replace isn't a big deal. So I avoid using %{name}

The no-doc warning on the devel package is ignoreable. The main package has
docs. Hmmm, I actually need to go over the docs, README.linux shouldn't be
packaged and ChangeLog might be better in the devel package. And where's the
license...

I figure the purpose of the makefile patch is self evident, based on the name
and by taking a look at it. I put in a comment for the others because otherwise
it would not be clear that they're there for the benefit of another package.
Comment 5 Callum Lerwick 2007-02-23 19:45:59 EST
And... looks like OpenJPEG 1.1.1 was just released. Which seems to have the SL
patches merged in. Working on it now...
Comment 6 Callum Lerwick 2007-03-11 15:53:45 EDT
http://www.haxxed.com/rpms/secondlife/openjpeg-1.1.1-1.src.rpm
http://www.haxxed.com/rpms/secondlife/openjpeg.spec

* Fri Feb 23 2007 Callum Lerwick <seg@haxxed.com> 1.1.1-1
- New upstream version, which has the SL patches merged.


I still need to email upstream about including the license.
Comment 7 Xavier Lamien 2007-03-11 22:22:50 EDT
> I still need to email upstream about including the license.

You should add the licence text as source in the meantime you're waitting an
answer from upstream.
Can be approuved without this.

For the rest i'll check out your files.
Comment 8 Xavier Lamien 2007-03-12 08:09:46 EDT
s/Can/Can't
Comment 9 Xavier Lamien 2007-03-14 15:40:12 EDT
Ping ?

Comment 10 Callum Lerwick 2007-03-30 04:19:25 EDT
http://www.haxxed.com/rpms/secondlife/openjpeg.spec
http://www.haxxed.com/rpms/secondlife/openjpeg-1.1.1-2.src.rpm

* Fri Mar 16 2007 Callum Lerwick <seg@haxxed.com> 1.1.1-2
- Link with libm, fixes building on ppc. i386 and x86_64 are magical.


I still need to talk to upstream about including the license. They do seem to be
interested in the performance tweaks I came up with. :)

http://www.haxxed.com/code/openjpeg-1.1.1-t1-optimize.20070330.patch
Comment 11 Xavier Lamien 2007-04-05 21:43:49 EDT
Well, 

any answers from upstream about license ?
Comment 12 Callum Lerwick 2007-04-06 16:49:47 EDT
I finally managed to catch upstream's attention. :) The license is in svn now
and a new release should be coming soonish, with the license and some of the
optimizations me and others have been working on...
Comment 13 Xavier Lamien 2007-04-17 10:53:51 EDT
ping ?

Any news about new release ?
Comment 14 Mamoru TASAKA 2007-05-14 20:28:13 EDT
I will close this bug as NOTABUG if any response from
the reporter is gained within one week

http://fedoraproject.org/wiki/Extras/Policy/StalledReviews
Comment 15 Xavier Lamien 2007-05-21 09:02:21 EDT
re: ping ?

Callum, any news from upstream ?
if not, what about svn trunck ?
Comment 16 Callum Lerwick 2007-05-21 17:29:47 EDT
I just submitted a bunch of patches yesterday, they may have been waiting on
that. I have a package based on SVN, however the ABI has sort of broken due to a
bunch of stuff for Cinema4k being added to the encoder parameter structure.
Upstream tried to prevent it, but on Fedora, it still crashes apps with "stack
smashing detected". Not sure how to deal with this.
Comment 17 Hans de Goede 2007-05-21 17:36:36 EDT
(In reply to comment #16)
> I just submitted a bunch of patches yesterday, they may have been waiting on
> that. I have a package based on SVN, however the ABI has sort of broken due to a
> bunch of stuff for Cinema4k being added to the encoder parameter structure.
> Upstream tried to prevent it, but on Fedora, it still crashes apps with "stack
> smashing detected". Not sure how to deal with this.

The only way to solve this is to give the new version a new soname (preferably
working with upstream), and then do a compat package for the old version, if the
new version is still API compatible, the compat package doesn't need a parallel
installable -devel and thus should be easy.
Comment 18 Nicolas Chauvet (kwizart) 2007-06-06 16:53:06 EDT
Hi!
4 June 2007: Version 1.2 Released
Comment 19 Bryan O'Sullivan 2007-06-13 19:42:06 EDT
Callum, any news?
Comment 20 Callum Lerwick 2007-06-13 20:44:31 EDT
My upgrade to F7 went smoothly as usual. (note: sarcasm) I'll upload an updated
package shortly.
Comment 21 Callum Lerwick 2007-06-14 04:17:17 EDT
http://www.haxxed.com/rpms/secondlife/openjpeg.spec
http://www.haxxed.com/rpms/secondlife/openjpeg-1.2-1.fc7.src.rpm

* Sun Jun 10 2007 Callum Lerwick <seg@haxxed.com> 1.2-1
- Build the mj2 tools as well.
- New upstream version, ABI has broken, upstream has bumped soname.
Comment 22 Hans de Goede 2007-06-14 14:27:58 EDT
(In reply to comment #21)
> - New upstream version, ABI has broken, upstream has bumped soname.

Since this is still under review, and thius not in Fedora, we do not have
anything depending on this already in the repo, right? Then this shouldn't be a
problem.
Comment 23 Xavier Lamien 2007-06-14 16:32:58 EDT
(in reply to comment #22)
>> Since this is still under review, and thius not in Fedora, we do not have
>> anything depending on this already in the repo, right? Then this shouldn't be a
>> problem.

indeed, this souldn't

well, 
starting the review...
Comment 24 Xavier Lamien 2007-06-15 04:57:22 EDT
 OK - Mock : Built on F-7 (x86_64)
 ? - Package meets naming and packaging guidelines
 OK - Spec file matches base package name.
 OK - Spec has consistant macro usage.
 OK - Meets Packaging Guidelines.
 OK - License field in spec matches
 OK - License is GPL
 OK - License match extras packaging policy licenses allowed
 OK - License file is included in package
 OK - Spec in American English
 OK - Spec is legible.
 OK - Sources SHOULD match upstream md5sum:
4973c564c96683a921a7f6759906da4e  openjpeg_v1_2.tar.gz 
 OK - Package has correct buildroot.
 OK - extras BuildRequires are not redundant.
 OK - %build and %install stages are correct and work.
 OK - Package has %defattr and permissions on files is good.
 OK - Package has a correct %clean section.
 OK - Package is code or permissible content.
 OK - Packages %doc files don't affect runtime.
 OK - Package has no duplicate files in %files.
 OK - Package doesn't own any directories that other packages own.
 OK - Changelog section is correct. 

 OK - Should function as described.
 OK - Should package latest version

-------------------------------------------
Rpmlint output:
-------------------------------------------
OK - silent on both srpm and main/sub package rpm


-------------------------------------------
Sub-packages:
-------------------------------------------
They need some improvment.
according to the scheme of opengjpeg
the main package should contain all bin files, that imply to remove -bin
subpackage (which not really usefull).
And if other packages work with/need-to  openjpeg lib for developping work,
provide a subpackage named -lib or -n libopenjpeg.

Also Changelog file should be in the main package.
Comment 25 Xavier Lamien 2007-06-20 17:34:26 EDT
Callum: ping ?
Comment 26 Mamoru TASAKA 2007-07-03 14:03:03 EDT
Again ping?
Comment 27 Xavier Lamien 2007-07-23 13:31:59 EDT
Last ping !
Comment 28 Callum Lerwick 2007-07-24 15:41:05 EDT
Hmmm, I'm trying to get some patches upstreamed. I should probably get a new
package out in the mean time.
Comment 29 Callum Lerwick 2007-08-10 01:25:49 EDT
http://www.haxxed.com/rpms/secondlife/openjpeg.spec
http://www.haxxed.com/rpms/secondlife/openjpeg-1.2-2.20070808svn.fc7.src.rpm

* Thu Aug 09 2007 Callum Lerwick <seg@haxxed.com> 1.2-2.20070808svn
- Put binaries in main package, move libraries to -libs subpackage.

Based on an SVN snapshot, with a whole load of patches that I'm trying to get
upstream, that speeds it up about 25%...
Comment 30 Xavier Lamien 2007-08-14 13:49:48 EDT
Okay, 
All issues above has been fixed.

However,
the release tag need to be fix.
As a pre-release svn checkout, its should be set like this: 0.%{X}.%{alphatag}
So move it to "0.1.%{snapshot}%{?dist}" instead of "2.%{snapshot}%{?dist}".

Also note that as an snapshot version, you should comment above the source0 the
svn external link to get the source.
Comment 31 Nicolas Chauvet (kwizart) 2007-08-14 16:10:35 EDT
@Xavier
This is a post release, so 2 is right in this case...

@Callum
About the snapshoot, it is often recommanded to use a bash script to retrieve
the sources (and remove CVS or .svn ), so you can have a method to compare
differents snapshoots... This script have to be provided also (Source10)...

You can find an sample in the dirac cvs directory if you want...
Comment 32 Till Maas 2007-09-08 03:38:41 EDT
(In reply to comment #31)

> @Callum
> About the snapshoot, it is often recommanded to use a bash script to retrieve
> the sources (and remove CVS or .svn ), so you can have a method to compare

The Guidelines demand a documented way how to create a snapshot tarball:
http://fedoraproject.org/wiki/Packaging/SourceURL#head-615f6271efb394ab340a93a6cf030f2d08cf0d49
And a hint for svn:
You can use "svn export" instead of "svn co" to avoid the creation of .svn
directories.
Comment 33 Mamoru TASAKA 2007-09-24 09:19:17 EDT
ping?
Comment 34 Mamoru TASAKA 2007-10-03 05:29:18 EDT
Again ping?
Comment 35 Xavier Lamien 2007-10-03 05:30:22 EDT
it's ok for me.
Comment 36 Nicolas Chauvet (kwizart) 2007-10-14 19:38:51 EDT
This package needs cvs to be Requested by the Reporter. See:
http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure
Comment 37 Mamoru TASAKA 2007-11-02 03:01:24 EDT
ping again?
Comment 38 Mamoru TASAKA 2007-11-02 05:24:10 EDT
By the way:

(In reply to comment #29)
> http://www.haxxed.com/rpms/secondlife/openjpeg.spec
> http://www.haxxed.com/rpms/secondlife/openjpeg-1.2-2.20070808svn.fc7.src.rpm
> 
> * Thu Aug 09 2007 Callum Lerwick <seg@haxxed.com> 1.2-2.20070808svn
> - Put binaries in main package, move libraries to -libs subpackage.

Rebuild fails at least on ppc and ppc64.
http://koji.fedoraproject.org/koji/taskinfo?taskID=224231
Comment 39 Callum Lerwick 2007-11-02 06:34:38 EDT
Hmmm, yeah that's something I've fixed already. :) I've got a bit distracted by
high memory usage and serious memory leakage in recent Second Life clients. The
leakage seemed to have been a Mesa/DRI bug or something, but I guess we just
have to live with higher memory usage for now. SL really doesn't run very well
on 512mb anymore.
Comment 40 Mamoru TASAKA 2007-11-16 07:00:48 EST
Again ping, Callum?

If this package is okay, please do CVS procedure
( http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure )
Comment 41 Callum Lerwick 2007-11-16 22:54:13 EST
http://www.haxxed.com/rpms/secondlife/openjpeg.spec
http://www.haxxed.com/rpms/secondlife/openjpeg-1.2-3.20071114svn480.fc7.src.rpm

* Thu Aug 09 2007 Callum Lerwick <seg@haxxed.com> 1.2-3.20071114svn480
- Build using cmake.
- New snapshot.

I think I'm doing snapshots the way the guidelines want me to now. Not that I
want to make a habit of using snapshots. The major performance optimizations got
merged upstream, and a new release should be coming in the next week. :)
Comment 42 Callum Lerwick 2007-11-16 23:05:28 EST
New Package CVS Request
=======================
Package Name: openjpeg
Short Description: JPEG 2000 codec library
Owners: seg
Branches: F-7 F-8
InitialCC: seg
Cvsextras Commits: yes
Comment 43 Kevin Fenzi 2007-11-16 23:23:11 EST
cvs done.
Comment 44 Fedora Update System 2007-11-20 12:56:38 EST
openjpeg-1.2-3.20071114svn480.fc7 has been pushed to the Fedora 7 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update openjpeg'
Comment 45 Fedora Update System 2007-11-20 13:06:16 EST
openjpeg-1.2-3.20071114svn480.fc8 has been pushed to the Fedora 8 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update openjpeg'
Comment 46 Gus Wirth 2007-11-25 14:15:37 EST
openjpeg-devel-1.2-3.20071114svn480.fc8 openjpeg.h header file and library do
not match. Installed on Fedora 8 system fully updated as of 11/25/2007

Simple program: openjpegtest.c

#include <stdio.h>
#include <openjpeg/openjpeg.h>
int main( void ) { return 0; }

Compile:
gcc -o tmp.out openjpegtest.c -lopenjpeg

Results:

/usr/lib/gcc/i386-redhat-linux/4.1.2/../../../libopenjpeg.so: undefined
reference to `floor'
/usr/lib/gcc/i386-redhat-linux/4.1.2/../../../libopenjpeg.so: undefined
reference to `ceilf'
/usr/lib/gcc/i386-redhat-linux/4.1.2/../../../libopenjpeg.so: undefined
reference to `floorf'
/usr/lib/gcc/i386-redhat-linux/4.1.2/../../../libopenjpeg.so: undefined
reference to `lrintf'
/usr/lib/gcc/i386-redhat-linux/4.1.2/../../../libopenjpeg.so: undefined
reference to `pow'
collect2: ld returned 1 exit status

Looks like a math include got left out
Comment 47 Hans de Goede 2007-11-25 14:24:18 EST
(In reply to comment #46)
> openjpeg-devel-1.2-3.20071114svn480.fc8 openjpeg.h header file and library do
> not match. Installed on Fedora 8 system fully updated as of 11/25/2007
> 

<snip>

You need to add -lm to the cmdline when linking
Comment 48 Callum Lerwick 2007-11-25 15:08:07 EST
Yeah, that's a link error. gcc *should* be replacing those functions with inline
code on i386, though.
Comment 49 Fedora Update System 2007-11-26 13:38:14 EST
openjpeg-1.2-3.20071114svn480.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 50 Fedora Update System 2007-11-26 13:58:03 EST
openjpeg-1.2-3.20071114svn480.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 51 Adam Goode 2009-09-23 14:02:34 EDT
Package Change Request
======================
Package Name: openjpeg
New Branches: EL-5
Owners: agoode
Comment 52 Kevin Fenzi 2009-09-24 00:40:32 EDT
cvs done.

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